Re: [PATCH 2/2] ARM: dts: mba6ulx: Add USB hub node
Alexander Stein <[email protected]>
| Newsgroups | dev.linux.lists.sashiko-reviews,dev.linux.lists.imx,org.kernel.vger.linux-devicetree |
|---|---|
| Organization | TQ-Systems GmbH |
| Message-ID | <5266525.iZASKD2KPV@steina-w> |
Am Donnerstag, 13. August 2026, 14:52:22 CEST schrieb [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 = <®_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. Mh, my platform doesn't support system suspend/wakeup (yet?). So there is nothing I could test here. On the other hand this sounds like a generic problem other if not all platforms affected as well if the reset is attached to a GPIO expander. Any suggestion? Best regards, Alexander > > + vdd-supply = <®_mba6ul_3v3>; > > + }; > > }; > > -- TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany Amtsgericht München, HRB 105018 Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider http://www.tq-group.com/