Re: [PATCH v8 10/16] usb: hub: Power on connected M.2 E-key connectors with power sequencing API
From: sashiko-bot@kernel.org
Date: 2026-09-04 09:10:20
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] hub_is_port_power_switchable() evaluates a per-port property at the hub level, leading to brittle logic and unintended side-effects for ports without power sequencers.
--
commit 839f0d077bce3f935502df5060567449e3c570bf
Author: Chen-Yu Tsai [off-list ref]
usb: hub: Power on connected M.2 E-key connectors with power sequencing API
The new M.2 E-key connector can have a USB connection. For the USB device
on this connector to work, its power must be enabled and the W_DISABLE2#
signal deasserted. The connector driver handles this and provides a
toggle over the power sequencing API.
Link: https://lore.kernel.org/all/20260610084053.2059858-1-wenst@chromium.org/ (local)
Link: https://lore.kernel.org/all/20260515090149.3169406-1-wenst@chromium.org/ (local)
quoted hunk ↗ jump to hunk
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h index de524c6da9fc4..3f403a56e5f75 100644 --- a/drivers/usb/core/hub.h +++ b/drivers/usb/core/hub.h@@ -103,7 +103,15 @@ static inline bool hub_is_port_power_switchable(struct usb_hub *hub) if (!hub) return false; hcs = hub->descriptor->wHubCharacteristics; - return (le16_to_cpu(hcs) & HUB_CHAR_LPSM) < HUB_CHAR_NO_LPSM; + if ((le16_to_cpu(hcs) & HUB_CHAR_LPSM) < HUB_CHAR_NO_LPSM) + return true; + /* check for controllable external power sequencers */ + for (unsigned int i = 0; i < hub->hdev->maxchild; i++) { + if (hub->ports[i] && hub->ports[i]->pwrseq) + return true; + } + + return false; }
[Severity: Medium] Does this loop incorrectly promote a per-port capability to a hub-wide capability? If any port on a hub has a pwrseq, hub_is_port_power_switchable() returns true for the entire hub. Could this cause callers to erroneously try to apply power switching logic to all ports on the hub, even those without a pwrseq, just because a sibling port made the hub appear switchable? For example, in drivers/usb/core/hub.c:hub_port_connect(): if (hub_is_port_power_switchable(hub) && !usb_port_is_power_on(port_dev, portstatus) && !port_dev->port_owner) usb_hub_set_port_power(hdev, hub, port1, true); Additionally, since hdev->maxchild is 0 during port creation, the loop inside hub_is_port_power_switchable() does not execute at that time. Does this make the hub-wide check redundant for its intended purpose in usb_port_allow_power_off()? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260904084158.1341550-1-wenst@chromium.org?part=10