Re: [PATCH 2/2] ARM: dts: mba6ulx: Add USB hub node

[email protected]
Newsgroups dev.linux.lists.imx,org.kernel.vger.linux-devicetree
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] I2C GPIO expander used as reset-gpios for onboard_usb_dev causes PM phase mismatch, breaking reset timing during resume.
--

--- Patch [2]: [PATCH 2/2] ARM: dts: mba6ulx: Add USB hub node ---
commit 52d9db850f98165215d9ab4ba1bee0bd2993c73e
Author: Alexander Stein <[email protected]>

ARM: dts: mba6ulx: Add USB hub node

This commit replaces a fixed regulator previously used for USB hub reset with
a proper onboard USB hub device node that utilizes reset-gpios.

> diff --git a/arch/arm/boot/dts/nxp/imx/mba6ulx.dtsi b/arch/arm/boot/dts/nxp/imx/mba6ulx.dtsi
> index 244376b38835a..322fbd82531e6 100644
> --- a/arch/arm/boot/dts/nxp/imx/mba6ulx.dtsi
> +++ b/arch/arm/boot/dts/nxp/imx/mba6ulx.dtsi

[ ... ]

> @@ -379,10 +369,18 @@ &usbotg1 {
>  /* 7-port usb hub */
>  /* id, pwr, oc pins not connected */
>  &usbotg2 {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
>  	disable-over-current;
> -	vbus-supply = <&reg_otg2vbus_5v0>;
>  	dr_mode = "host";
>  	status = "okay";
> +
> +	hub_2_0: hub@1 {
> +		compatible = "usb424,2517";
> +		reg = <1>;
> +		reset-gpios = <&expander_out1 0 GPIO_ACTIVE_LOW>;
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
Does using an I2C GPIO expander for the onboard USB hub reset cause a PM phase
mismatch during system suspend and resume?

The onboard_usb_dev driver uses suspend_late() and resume_early() power
management operations, while standard I2C GPIO expanders typically use normal
suspend() and resume() operations.

During system suspend, the expander suspends first (in the suspend phase) and
puts its regmap into cache_only mode. When the hub driver suspends later in
suspend_late(), its reset GPIO assertion is merely cached.

During system resume, the hub driver attempts to deassert the reset in
resume_early() and waits for power_on_delay_us. Because the expander is
still in cache_only mode, the physical pin remains unchanged. 

Later, during the normal resume phase, the expander finally syncs its cache
to hardware and physically deasserts the reset pin. This entirely bypasses the
hub driver's power_on_delay_us wait and causes a race with the USB host
controller, which could result in enumeration failures upon wake.

> +		vdd-supply = <&reg_mba6ul_3v3>;
> +	};
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.