Re: [PATCH v7 10/16] usb: hub: Power on connected M.2 E-key connectors with power sequencing API

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] hub_is_port_power_switchable() incorrectly elevates a per-port power sequencer property to a hub-wide capability.
--

--- Patch [10]: [PATCH v7 10/16] usb: hub: Power on connected M.2 E-key connectors with power sequencing API ---
commit 5b4df25e39c50f42b92c649d3b30e831a5d2dc33
Author: Chen-Yu Tsai <wenst@chromium.org>

usb: hub: Power on connected M.2 E-key connectors with power sequencing API

This commit adds support for powering on M.2 E-key connectors that have a
USB connection using the power sequencing API. It uses the usb_port device
to acquire the power sequencing descriptor, allowing the power sequence
provider to differentiate between ports on a hub.

Link: https://lore.kernel.org/all/20260610084053.2059858-1-wenst@chromium.org/
Link: https://lore.kernel.org/all/20260515090149.3169406-1-wenst@chromium.org/

[ ... ]

> 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 code incorrectly elevate a per-port property to a hub-wide
capability?

If a natively non-switchable USB hub is connected and at least one of its
ports has an external power sequencer, hub_is_port_power_switchable() will
return true for the entire hub.

This causes runtime operations like hub_port_connect_change() to act on all
ports as if they are switchable:

drivers/usb/core/hub.c:hub_port_connect_change() {
    ...
		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);
    ...
}

Will this erroneously issue SetPortFeature(PORT_POWER) commands for ports on
the hub that neither support power switching nor possess a power sequencer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724084328.3943997-1-wenst@chromium.org?part=10