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

[Roy Luo <royluo@google.com>]

On Thu, Oct 23, 2025 at 3:43 PM Thinh Nguyen <Thinh.Nguyen@synopsys.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@google.com>
> > Signed-off-by: Joy Chakraborty <joychakr@google.com>
> > Co-developed-by: Naveen Kumar <mnkumar@google.com>
> > Signed-off-by: Naveen Kumar <mnkumar@google.com>
> > Signed-off-by: Roy Luo <royluo@google.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.

| 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.

>
> 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