linux-kernel - Re: [PATCH v4 2/2] usb: dwc3: Add Google Tensor SoC DWC3 glue driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251030010215.q76ytn345nwwcyru@synopsys.com>
Date: Thu, 30 Oct 2025 01:02:25 +0000
From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To: Roy Luo <royluo@...gle.com>
CC: Thinh Nguyen <Thinh.Nguyen@...opsys.com>, Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Peter Griffin <peter.griffin@...aro.org>,
        André Draszik <andre.draszik@...aro.org>,
        Tudor Ambarus <tudor.ambarus@...aro.org>,
        Joy Chakraborty <joychakr@...gle.com>,
        Naveen Kumar <mnkumar@...gle.com>,
        Badhri Jagan Sridharan <badhri@...gle.com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
        "linux-samsung-soc@...r.kernel.org" <linux-samsung-soc@...r.kernel.org>
Subject: Re: [PATCH v4 2/2] usb: dwc3: Add Google Tensor SoC DWC3 glue driver

On Fri, Oct 24, 2025, Roy Luo wrote:
> On Thu, Oct 23, 2025 at 3:43 PM Thinh Nguyen <Thinh.Nguyen@...opsys.com> wrote:
> >
> > On Fri, Oct 17, 2025, Roy Luo wrote:
> > > Add support for the DWC3 USB controller found on Google Tensor G5.
> > > The controller features dual-role functionality and hibernation.
> > >
> > > The primary focus is implementing hibernation support in host mode,
> > > enabling the controller to enter a low-power state (D3). This is
> > > particularly relevant during system power state transition and
> > > runtime power management for power efficiency.
> > > Highlights:
> > > - Align suspend callback with dwc3_suspend_common() for deciding
> > >   between a full teardown and hibernation in host mode.
> > > - Integration with `psw` (power switchable) and `top` power domains,
> > >   managing their states and device links to support hibernation.
> > > - A notifier callback dwc3_google_usb_psw_pd_notifier() for
> > >   `psw` power domain events to manage controller state
> > >   transitions to/from D3.
> > > - Coordination of the `non_sticky` reset during power state
> > >   transitions, asserting it on D3 entry and deasserting on D0 entry
> > >   in hibernation scenario.
> > > - Handling of high-speed and super-speed PME interrupts
> > >   that are generated by remote wakeup during hibernation.
> > >
> > > Co-developed-by: Joy Chakraborty <joychakr@...gle.com>
> > > Signed-off-by: Joy Chakraborty <joychakr@...gle.com>
> > > Co-developed-by: Naveen Kumar <mnkumar@...gle.com>
> > > Signed-off-by: Naveen Kumar <mnkumar@...gle.com>
> > > Signed-off-by: Roy Luo <royluo@...gle.com>
> > > ---
> > >  drivers/usb/dwc3/Kconfig       |  10 +
> > >  drivers/usb/dwc3/Makefile      |   1 +
> > >  drivers/usb/dwc3/dwc3-google.c | 608 +++++++++++++++++++++++++++++++++
> > >  3 files changed, 619 insertions(+)
> > >  create mode 100644 drivers/usb/dwc3/dwc3-google.c
> > >
> >
> > Sorry, I've been tied up with some internal projects and haven't
> > reviewed this in detail yet. I think this change deserve more time and
> > attention, and thus the delay.
> >
> 
> No worry, thanks for paying attention to this patch!
> 
> > One of the things that stood out is that you're assuming the host
> > suspend is always hibernation. While it's true that xhci suspend would
> > go through the xhci hibernation flow, however, that needs to communicate
> > to the glue driver here. For example, if the xhci driver is not bound,
> > and the device goes into suspend, we may go through this hibernation
> > flow when we should not. But maybe that's already handle? I need to
> > check.
> 
> Actually the host suspend doesn't always go into hibernation.
> In dwc3_google_suspend(), this driver follows the logic in
> dwc3_suspend_common() in determining whether to do a full tear
> down or enter hibernation.

If we go through the xhci_suspend() and xhci_resume(), then there will
be saving and restoring of xHC states. And what you said is true. if we
go through the dwc3_core_exit, the states may be lost because of the phy
teardown. Maybe I should not mix the term hibernation for xhci
save/restore states.

> 
> | static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
> | ...
> | case DWC3_GCTL_PRTCAP_HOST:
> |   if (!PMSG_IS_AUTO(msg) && !device_may_wakeup(dwc->dev)) {
> |     dwc3_core_exit(dwc);
> |     break;
> |   }

> 
> The glue and the dwc3 core have to be aligned here as there's no
> way to enter hibernation if the core has been completely torn down.
> As for xhci, I don't see any logic that's conditional on hibernation
> so I didn't pay much attention to it, please correct me if I'm wrong.

There's no conditional save states for xhci. It always saves states on
suspend. But to pass the power to the PMU and to the save and restore of
the sticky states when entering D3, I think the logic you noted to check
that is fine.

BR,
Thinh

> 
> >
> > In any case, there are multiple players (xhci, xhci-plat, dwc3, glue)
> > here, and I need more time to review. Appologies if this will take
> > longer than usual.
> >
> > Thanks,
> > Thinh
> 
> Looking forward to working with you to land this patch.
> 
> Thanks,
> Roy Luo