From c0a4448c0b7f58af75375bb78ba0e4daf8022433 Mon Sep 17 00:00:00 2001 From: Jeremy Soller Date: Tue, 16 Apr 2024 21:26:14 -0600 Subject: [PATCH] xhcid/usbhidd: remove extra set idle code, just use device request --- storage/usbscsid/src/main.rs | 1 - usbhidd/src/main.rs | 13 ++++----- xhcid/src/driver_interface.rs | 7 +---- xhcid/src/usb/setup.rs | 20 -------------- xhcid/src/xhci/scheme.rs | 50 +++++++---------------------------- 5 files changed, 18 insertions(+), 73 deletions(-) diff --git a/storage/usbscsid/src/main.rs b/storage/usbscsid/src/main.rs index aef4cde0b6..e745c0ad3f 100644 --- a/storage/usbscsid/src/main.rs +++ b/storage/usbscsid/src/main.rs @@ -77,7 +77,6 @@ fn daemon(daemon: redox_daemon::Daemon, scheme: String, port: usize, protocol: u config_desc: configuration_value, interface_desc: Some(interface_num), alternate_setting: Some(alternate_setting), - ..Default::default() }) .expect("Failed to configure endpoints"); diff --git a/usbhidd/src/main.rs b/usbhidd/src/main.rs index 6f1b8a2e1c..921de656ae 100644 --- a/usbhidd/src/main.rs +++ b/usbhidd/src/main.rs @@ -10,7 +10,7 @@ use rehid::{ report_handler::ReportHandler, usage_tables::{GenericDesktopUsage, UsagePage}, }; -use xhcid_interface::{ConfigureEndpointsReq, DevDesc, EndpDirection, EndpointTy, PortReqRecipient, XhciClientHandle, PROTOCOL_REPORT}; +use xhcid_interface::{ConfigureEndpointsReq, DevDesc, EndpDirection, EndpointTy, PortReqRecipient, XhciClientHandle}; mod keymap; mod reqs; @@ -281,14 +281,15 @@ fn main() { config_desc: conf_num as u8, interface_desc: Some(interface_num), alternate_setting: Some(if_desc.alternate_setting), - //TODO: do we need to set this? It fails for mice. protocol: Some(PROTOCOL_REPORT), - //TODO: dynamically create good values, fix xhcid so it does not block on each request - // This sets all reports to a duration of 4ms - report_idles: vec![(0, 1)], - ..Default::default() }) .expect("Failed to configure endpoints"); + //TODO: do we need to set protocol to report? It fails for mice. + + //TODO: dynamically create good values, fix xhcid so it does not block on each request + // This sets all reports to a duration of 4ms + reqs::set_idle(&handle, 1, 0, interface_num as u16).expect("Failed to set idle"); + let report_desc_len = hid_desc.desc_len; assert_eq!(hid_desc.desc_ty, REPORT_DESC_TY); diff --git a/xhcid/src/driver_interface.rs b/xhcid/src/driver_interface.rs index bd5b0e662a..c850f252d6 100644 --- a/xhcid/src/driver_interface.rs +++ b/xhcid/src/driver_interface.rs @@ -14,17 +14,12 @@ use thiserror::Error; pub use crate::usb::{EndpointTy, ENDP_ATTR_TY_MASK}; -pub const PROTOCOL_BOOT: u8 = 0; -pub const PROTOCOL_REPORT: u8 = 1; - #[derive(Clone, Debug, Default, Serialize, Deserialize)] pub struct ConfigureEndpointsReq { /// Index into the configuration descriptors of the device descriptor. pub config_desc: u8, pub interface_desc: Option, pub alternate_setting: Option, - pub protocol: Option, - pub report_idles: Vec<(u8, u8)>, } #[derive(Clone, Debug, Serialize, Deserialize)] @@ -477,7 +472,7 @@ impl XhciClientHandle { value, index, length, - transfers_data: true, + transfers_data: !matches!(data, DeviceReqData::NoData), }; let json = serde_json::to_vec(&req)?; diff --git a/xhcid/src/usb/setup.rs b/xhcid/src/usb/setup.rs index 9a9a2ef970..879a424776 100644 --- a/xhcid/src/usb/setup.rs +++ b/xhcid/src/usb/setup.rs @@ -191,24 +191,4 @@ impl Setup { length: 0, } } - - pub const fn set_protocol(interface: u8, protocol: u8) -> Self { - Self { - kind: 0b0010_0001, - request: 0x0B, - value: protocol as u16, - index: interface as u16, - length: 0, - } - } - - pub const fn set_idle(interface: u8, report_id: u8, duration: u8) -> Self { - Self { - kind: 0b0010_0001, - request: 0x0A, - value: (report_id as u16) | ((duration as u16) << 8), - index: interface as u16, - length: 0, - } - } } diff --git a/xhcid/src/xhci/scheme.rs b/xhcid/src/xhci/scheme.rs index 4c2fbf92b6..3a6f3d78ac 100644 --- a/xhcid/src/xhci/scheme.rs +++ b/xhcid/src/xhci/scheme.rs @@ -465,7 +465,7 @@ impl Xhci { } // TODO: Handle event data - debug!("EVENT DATA: {:?}", event_trb.event_data()); + trace!("EVENT DATA: {:?}", event_trb.event_data()); Ok(event_trb) } @@ -500,33 +500,6 @@ impl Xhci { ).await } - async fn set_protocol( - &self, - port: usize, - interface_num: u8, - protocol: u8, - ) -> Result<()> { - debug!("Setting idle value {} (protocol {}) to port {}", interface_num, protocol, port); - self.device_req_no_data( - port, - usb::Setup::set_protocol(interface_num, protocol), - ).await - } - - async fn set_idle( - &self, - port: usize, - interface_num: u8, - report_id: u8, - duration: u8, - ) -> Result<()> { - debug!("Setting idle value {} (report_id {}, duration {}) to port {}", interface_num, report_id, duration, port); - self.device_req_no_data( - port, - usb::Setup::set_idle(interface_num, report_id, duration), - ).await - } - async fn reset_endpoint(&self, port_num: usize, endp_num: u8, tsp: bool) -> Result<()> { let port_state = self.port_states.get(&port_num).ok_or(Error::new(EBADFD))?; @@ -867,14 +840,6 @@ impl Xhci { if let Some(alternate_setting) = req.alternate_setting { self.set_interface(port, interface_num, alternate_setting).await?; } - - if let Some(protocol) = req.protocol { - self.set_protocol(port, interface_num, protocol).await?; - } - - for (report_id, duration) in req.report_idles { - self.set_idle(port, interface_num, report_id, duration).await?; - } } Ok(()) @@ -1267,12 +1232,17 @@ impl Xhci { // TODO: Reuse buffers, or something. // TODO: Validate the size. // TODO: Sizes above 65536, *perhaps*. - let data_buffer = unsafe { self.alloc_dma_zeroed_unsized(req.length as usize)? }; - assert_eq!(data_buffer.len(), req.length as usize); + let data_buffer_opt = if req.transfers_data { + let data_buffer = unsafe { self.alloc_dma_zeroed_unsized(req.length as usize)? }; + assert_eq!(data_buffer.len(), req.length as usize); + Some(data_buffer) + } else { + None + }; Ok(match transfer_kind { - TransferKind::In => PortReqState::WaitingForDeviceBytes(data_buffer, setup), - TransferKind::Out => PortReqState::WaitingForHostBytes(data_buffer, setup), + TransferKind::In => PortReqState::WaitingForDeviceBytes(data_buffer_opt.ok_or(Error::new(EINVAL))?, setup), + TransferKind::Out => PortReqState::WaitingForHostBytes(data_buffer_opt.ok_or(Error::new(EINVAL))?, setup), TransferKind::NoData => PortReqState::TmpSetup(setup), _ => unreachable!(), })