diff --git a/drivers/usb/usbhubd/src/main.rs b/drivers/usb/usbhubd/src/main.rs index 516a60cfc4..1ad7bb9a0c 100644 --- a/drivers/usb/usbhubd/src/main.rs +++ b/drivers/usb/usbhubd/src/main.rs @@ -543,12 +543,14 @@ fn main() { } // Linux 7.1 hub_port_debounce_be_connected(): on a - // connection-change event, require the connection bit to stay - // stable for 100ms (25ms steps, 2s budget) before enumerating. + // connection-change event, require the connection state to + // stay stable for 100ms (25ms steps, 2s budget) before + // enumerating. The change bit is cleared after attach below, + // not inside the debounce — a hung clear must not deadlock + // enumeration. if snap.connection_changed && !state.attached { match port_ops::debounce_until_connected( | | fetch_status(&handle, port, usb_3), - | | clear_feature(&handle, port, usb::HubPortFeature::CPortConnection), |ms| thread::sleep(time::Duration::from_millis(ms)), ) { Ok(s) => { @@ -595,6 +597,15 @@ fn main() { let was_attached = state.attached; state.ensure_attached(true); + if !was_attached && state.attached { + // The connection-change bit was ignored during the debounce + // (stuck/hung clears must not deadlock enumeration); clear it + // now that the device is attached, best-effort. Linux clears + // C_PORT_CONNECTION in port_event on the same path. + if snap.connection_changed { + let _ = clear_feature(&handle, port, usb::HubPortFeature::CPortConnection); + } + } // New SuperSpeed child: deliver the accumulated hub delay so the // device can compensate isochronous scheduling (Linux // usb_set_isoch_delay after enumeration, hub.c:5126-5131). diff --git a/drivers/usb/usbhubd/src/port_ops.rs b/drivers/usb/usbhubd/src/port_ops.rs index f846f1ab1a..4197194a53 100644 --- a/drivers/usb/usbhubd/src/port_ops.rs +++ b/drivers/usb/usbhubd/src/port_ops.rs @@ -115,17 +115,21 @@ impl fmt::Display for WaitError { impl std::error::Error for WaitError {} -/// Linux 7.1 `hub_port_debounce_be_connected()` (hub.c:4696-4737). +/// Linux 7.1 `hub_port_debounce_be_connected()` (hub.c:4696-4737), adapted +/// for an environment with no control-transfer timeouts. /// -/// Poll the port until the connection bit stays continuously set for +/// Poll the port until the CONNECTION STATE stays continuously set for /// `HUB_DEBOUNCE_STABLE_MS`, sampling every `HUB_DEBOUNCE_STEP_MS` with a -/// total budget of `HUB_DEBOUNCE_TIMEOUT_MS`. Any connection-change event -/// or state flip restarts the stable window. The change bit is cleared -/// during the loop so subsequent reads reflect hardware, not the -/// original event. Returns the final stable status. +/// total budget of `HUB_DEBOUNCE_TIMEOUT_MS`. Stability is judged on the +/// connection STATE alone, not on the change bit: Linux clears +/// C_PORT_CONNECTION inside its loop so later reads are clean, but a hub +/// that hangs or ignores the clear would deadlock a literal port of that +/// design (our control transfers have no timeout). A stuck change bit is +/// harmless — it only re-triggers a poll — so it is ignored here and +/// cleared once after a successful attach instead. Returns the final +/// stable status. pub fn debounce_until_connected( mut fetch: impl FnMut() -> Result, - mut clear_connection_change: impl FnMut() -> Result<(), E>, mut sleep: impl FnMut(u64), ) -> Result> { let mut stable_ms = 0u64; @@ -137,7 +141,7 @@ pub fn debounce_until_connected( loop { let cur = fetch().map_err(WaitError::Io)?; - if !cur.connection_changed && connection == Some(cur.connected) { + if connection == Some(cur.connected) { stable_ms += HUB_DEBOUNCE_STEP_MS; if stable_ms >= HUB_DEBOUNCE_STABLE_MS { return if cur.connected { @@ -151,10 +155,6 @@ pub fn debounce_until_connected( connection = Some(cur.connected); } - if cur.connection_changed { - clear_connection_change().map_err(WaitError::Io)?; - } - if total_ms >= HUB_DEBOUNCE_TIMEOUT_MS { return Err(WaitError::Timeout); } @@ -303,14 +303,12 @@ mod tests { let rig = RefCell::new(Rig::new([connected(), connected(), connected(), connected()])); let out = debounce_until_connected( || rig.borrow_mut().fetch(), - || rig.borrow_mut().clear(), |ms| rig.borrow_mut().sleep(ms), ); let rig = rig.into_inner(); assert_eq!(out, Ok(connected())); // First read seeds the window; four stable steps of 25 ms reach 100 ms. assert_eq!(rig.sleeps, vec![25, 25, 25, 25]); - assert_eq!(rig.clears, 0); } #[test] @@ -325,7 +323,6 @@ mod tests { ])); let out = debounce_until_connected( || rig.borrow_mut().fetch(), - || rig.borrow_mut().clear(), |ms| rig.borrow_mut().sleep(ms), ); let rig = rig.into_inner(); @@ -339,29 +336,44 @@ mod tests { let rig = RefCell::new(Rig::new([snap(), snap(), snap(), snap()])); let out = debounce_until_connected( || rig.borrow_mut().fetch(), - || rig.borrow_mut().clear(), |ms| rig.borrow_mut().sleep(ms), ); assert_eq!(out, Err(WaitError::Disconnected)); } #[test] - fn debounce_change_bit_cleared_and_timeout() { - let flapping = PortStatusSnapshot { + fn debounce_stuck_change_bit_still_converges() { + // The hub keeps C_PORT_CONNECTION set (clear hung or ignored). + // Stability is judged on the connection STATE, so a stuck change + // bit must not deadlock the debounce — this was the QEMU + // enumeration hang caught by test-usb-hub-qemu.sh. + let stuck = PortStatusSnapshot { connection_changed: true, ..connected() }; + let rig = RefCell::new(Rig::new([stuck, stuck, stuck, stuck, stuck])); + let out = debounce_until_connected( + || rig.borrow_mut().fetch(), + |ms| rig.borrow_mut().sleep(ms), + ); + let rig = rig.into_inner(); + assert_eq!(out, Ok(stuck)); + assert_eq!(rig.sleeps, vec![25, 25, 25, 25]); + } + + #[test] + fn debounce_timeout_when_never_stable() { + // Connection flaps between connected and disconnected every poll — + // the window never fills, so the total budget expires. let rig = RefCell::new(Rig::new( - (0..200).map(|i| if i % 2 == 0 { flapping } else { connected() }), + (0..200).map(|i| if i % 2 == 0 { connected() } else { snap() }), )); let out = debounce_until_connected( || rig.borrow_mut().fetch(), - || rig.borrow_mut().clear(), |ms| rig.borrow_mut().sleep(ms), ); let rig = rig.into_inner(); assert_eq!(out, Err(WaitError::Timeout)); - assert!(rig.clears > 0); assert_eq!(rig.sleeps.iter().sum::(), HUB_DEBOUNCE_TIMEOUT_MS); }