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

Thinh Nguyen
2025-11-04T02:07:46Z

On Fri, Oct 31, 2025, Roy Luo wrote:
> On Wed, Oct 29, 2025 at 6:35 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
> > >
> > > diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> > > index 310d182e10b5..467515d5f937 100644
> > > --- a/drivers/usb/dwc3/Kconfig
> > > +++ b/drivers/usb/dwc3/Kconfig
> > > @@ -189,4 +189,14 @@ config USB_DWC3_RTK
> > > or dual-role mode.
> > > Say 'Y' or 'M' if you have such device.
> > >
> > > +config USB_DWC3_GOOGLE
> > > + tristate "Google Platform"
> > > + depends on OF && COMMON_CLK && RESET_CONTROLLER
> > > + default n
> > > + help
> > > + Support the DesignWare Core USB3 IP found on Google Tensor
> > > + SoCs, starting with the G5 generation. This driver includes
> > > + support for hibernation in host mode.
> > > + Say 'Y' or 'M' if you have one such device.
> > > +
> > > endif
> > > diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> > > index 830e6c9e5fe0..a94982630657 100644
> > > --- a/drivers/usb/dwc3/Makefile
> > > +++ b/drivers/usb/dwc3/Makefile
> > > @@ -57,3 +57,4 @@ obj-$(CONFIG_USB_DWC3_IMX8MP) += dwc3-imx8mp.o
> > > obj-$(CONFIG_USB_DWC3_XILINX) += dwc3-xilinx.o
> > > obj-$(CONFIG_USB_DWC3_OCTEON) += dwc3-octeon.o
> > > obj-$(CONFIG_USB_DWC3_RTK) += dwc3-rtk.o
> > > +obj-$(CONFIG_USB_DWC3_GOOGLE) += dwc3-google.o
> > > diff --git a/drivers/usb/dwc3/dwc3-google.c b/drivers/usb/dwc3/dwc3-google.c
> > > new file mode 100644
> > > index 000000000000..d3fec6fb4dcc
> > > --- /dev/null
> > > +++ b/drivers/usb/dwc3/dwc3-google.c
> > > @@ -0,0 +1,608 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * dwc3-google.c - Google DWC3 Specific Glue Layer
> > > + *
> > > + * Copyright (c) 2025, Google LLC
> > > + * Author: Roy Luo <royluo@google.com>
> > > + */
> > > +
> > > +#include <linux/of.h>
> > > +#include <linux/bitfield.h>
> > > +#include <linux/irq.h>
> > > +#include <linux/clk.h>
> > > +#include <linux/module.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/reset.h>
> > > +#include <linux/pm_domain.h>
> > > +#include <linux/iopoll.h>
> > > +#include "core.h"
> > > +#include "glue.h"
> > > +
> > > +/* HOST CFG registers */
> > > +#define HC_STATUS_OFFSET 0x0
> > > +#define HC_STATUS_CURRENT_POWER_STATE_U2PMU GENMASK(1, 0)
> > > +#define HC_STATUS_CURRENT_POWER_STATE_U3PMU GENMASK(4, 3)
> > > +
> > > +#define HOST_CFG1_OFFSET 0x4
> > > +#define HOST_CFG1_PME_EN BIT(3)
> > > +#define HOST_CFG1_PM_POWER_STATE_REQUEST GENMASK(5, 4)
> > > +#define HOST_CFG1_PM_POWER_STATE_D0 0x0
> > > +#define HOST_CFG1_PM_POWER_STATE_D3 0x3
> > > +
> > > +/* USBINT registers */
> > > +#define USBINT_CFG1_OFFSET 0x0
> > > +#define USBINT_CFG1_USBDRD_PME_GEN_U2P_INTR_MSK BIT(2)
> > > +#define USBINT_CFG1_USBDRD_PME_GEN_U3P_INTR_MSK BIT(3)
> > > +#define USBINT_CFG1_USBDRD_PME_GEN_U2P_INTR_INT_EN BIT(8)
> > > +#define USBINT_CFG1_USBDRD_PME_GEN_U3P_INTR_INT_EN BIT(9)
> > > +#define USBINT_CFG1_USBDRD_PME_GEN_U2_INTR_CLR BIT(14)
> > > +#define USBINT_CFG1_USBDRD_PME_GEN_U3_INTR_CLR BIT(15)
> > > +
> > > +#define USBINT_STATUS_OFFSET 0x4
> > > +#define USBINT_STATUS_USBDRD_PME_GEN_U2P_INTR_STS_RAW BIT(2)
> > > +#define USBINT_STATUS_USBDRD_PME_GEN_U3P_INTR_STS_RAW BIT(3)
> > > +
> > > +#define USBCS_TOP_CTRL_CFG1_OFFSET 0xc
> > > +#define USBCS_TOP_CTRL_CFG1_USB2ONLY_MODE BIT(5)
> > > +
> > > +#define DWC3_GOOGLE_MAX_RESETS 4
> > > +
> > > +struct dwc3_google {
> > > + struct device *dev;
> > > + struct dwc3 dwc;
> > > + struct clk_bulk_data *clks;
> > > + int num_clks;
> > > + struct reset_control_bulk_data rsts[DWC3_GOOGLE_MAX_RESETS];
> > > + int num_rsts;
> > > + struct reset_control *non_sticky_rst;
> > > + struct device *usb_psw_pd;
> > > + struct device_link *usb_psw_pd_dl;
> > > + struct notifier_block usb_psw_pd_nb;
> > > + struct device *usb_top_pd;
> > > + struct device_link *usb_top_pd_dl;
> > > + void __iomem *host_cfg_base;
> > > + void __iomem *usbint_cfg_base;
> > > + int hs_pme_irq;
> > > + int ss_pme_irq;
> > > + bool is_usb2only;
> > > + bool is_hibernation;
> > > +};
> > > +
> > > +#define to_dwc3_google(d) container_of((d), struct dwc3_google, dwc)
> > > +
> > > +static int dwc3_google_rst_init(struct dwc3_google *google)
> > > +{
> > > + int ret;
> > > +
> > > + google->num_rsts = 4;
> > > + google->rsts[0].id = "non_sticky";
> > > + google->rsts[1].id = "sticky";
> > > + google->rsts[2].id = "drd_bus";
> > > + google->rsts[3].id = "top";
> > > +
> > > + ret = devm_reset_control_bulk_get_exclusive(google->dev,
> > > + google->num_rsts,
> > > + google->rsts);
> > > +
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + google->non_sticky_rst = google->rsts[0].rstc;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int dwc3_google_set_pmu_state(struct dwc3_google *google, int state)
> > > +{
> > > + u32 reg;
> > > + int ret;
> > > +
> > > + reg = readl(google->host_cfg_base + HOST_CFG1_OFFSET);
> > > + reg &= ~HOST_CFG1_PM_POWER_STATE_REQUEST;
> > > + reg |= (FIELD_PREP(HOST_CFG1_PM_POWER_STATE_REQUEST, state) |
> > > + HOST_CFG1_PME_EN);
> > > + writel(reg, google->host_cfg_base + HOST_CFG1_OFFSET);
> > > +
> > > + ret = readl_poll_timeout(google->host_cfg_base + HC_STATUS_OFFSET, reg,
> > > + (FIELD_GET(HC_STATUS_CURRENT_POWER_STATE_U2PMU, reg) == state &&
> > > + FIELD_GET(HC_STATUS_CURRENT_POWER_STATE_U3PMU, reg) == state),
> > > + 10, 10000);
> > > +
> > > + if (ret)
> > > + dev_err(google->dev, "failed to set PMU state %d\n", state);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +/*
> > > + * Clear pme interrupts and report their status.
> > > + * The hardware requires write-1 then write-0 sequence to clear the interrupt bits.
> > > + */
> > > +static u32 dwc3_google_clear_pme_irqs(struct dwc3_google *google)
> > > +{
> > > + u32 irq_status, reg_set, reg_clear;
> > > +
> > > + irq_status = readl(google->usbint_cfg_base + USBINT_STATUS_OFFSET);
> > > + irq_status &= (USBINT_STATUS_USBDRD_PME_GEN_U2P_INTR_STS_RAW |
> > > + USBINT_STATUS_USBDRD_PME_GEN_U3P_INTR_STS_RAW);
> > > + if (!irq_status)
> > > + return irq_status;
> > > +
> > > + reg_set = readl(google->usbint_cfg_base + USBINT_CFG1_OFFSET);
> > > + reg_clear = reg_set;
> > > + if (irq_status & USBINT_STATUS_USBDRD_PME_GEN_U2P_INTR_STS_RAW) {
> > > + reg_set |= USBINT_CFG1_USBDRD_PME_GEN_U2_INTR_CLR;
> > > + reg_clear &= ~USBINT_CFG1_USBDRD_PME_GEN_U2_INTR_CLR;
> > > + }
> > > + if (irq_status & USBINT_STATUS_USBDRD_PME_GEN_U3P_INTR_STS_RAW) {
> > > + reg_set |= USBINT_CFG1_USBDRD_PME_GEN_U3_INTR_CLR;
> > > + reg_clear &= ~USBINT_CFG1_USBDRD_PME_GEN_U3_INTR_CLR;
> > > + }
> > > +
> > > + writel(reg_set, google->usbint_cfg_base + USBINT_CFG1_OFFSET);
> > > + writel(reg_clear, google->usbint_cfg_base + USBINT_CFG1_OFFSET);
> > > +
> > > + return irq_status;
> > > +}
> > > +
> > > +static void dwc3_google_enable_pme_irq(struct dwc3_google *google)
> > > +{
> > > + u32 reg;
> > > +
> > > + reg = readl(google->usbint_cfg_base + USBINT_CFG1_OFFSET);
> > > + reg &= ~(USBINT_CFG1_USBDRD_PME_GEN_U2P_INTR_MSK |
> > > + USBINT_CFG1_USBDRD_PME_GEN_U3P_INTR_MSK);
> > > + reg |= (USBINT_CFG1_USBDRD_PME_GEN_U2P_INTR_INT_EN |
> > > + USBINT_CFG1_USBDRD_PME_GEN_U3P_INTR_INT_EN);
> > > + writel(reg, google->usbint_cfg_base + USBINT_CFG1_OFFSET);
> > > +
> > > + enable_irq(google->hs_pme_irq);
> > > + enable_irq(google->ss_pme_irq);
> > > + enable_irq_wake(google->hs_pme_irq);
> > > + enable_irq_wake(google->ss_pme_irq);
> > > +}
> > > +
> > > +static void dwc3_google_disable_pme_irq(struct dwc3_google *google)
> > > +{
> > > + u32 reg;
> > > +
> > > + reg = readl(google->usbint_cfg_base + USBINT_CFG1_OFFSET);
> > > + reg &= ~(USBINT_CFG1_USBDRD_PME_GEN_U2P_INTR_INT_EN |
> > > + USBINT_CFG1_USBDRD_PME_GEN_U3P_INTR_INT_EN);
> > > + reg |= (USBINT_CFG1_USBDRD_PME_GEN_U2P_INTR_MSK |
> > > + USBINT_CFG1_USBDRD_PME_GEN_U3P_INTR_MSK);
> > > + writel(reg, google->usbint_cfg_base + USBINT_CFG1_OFFSET);
> > > +
> > > + disable_irq_wake(google->hs_pme_irq);
> > > + disable_irq_wake(google->ss_pme_irq);
> > > + disable_irq_nosync(google->hs_pme_irq);
> > > + disable_irq_nosync(google->ss_pme_irq);
> > > +}
> > > +
> > > +static irqreturn_t dwc3_google_resume_irq(int irq, void *data)
> > > +{
> > > + struct dwc3_google *google = data;
> > > + struct dwc3 *dwc = &google->dwc;
> > > + u32 irq_status, dr_role;
> > > +
> > > + irq_status = dwc3_google_clear_pme_irqs(google);
> > > + dr_role = dwc->current_dr_role;
> > > +
> > > + if (!irq_status || !google->is_hibernation ||
> >
> > Any handling of race condition with is_hibernation?
>
> IIUC, you're referring to the race between
> - pme irq handler: dwc3_google_resume_irq() reads is_hibernation
> - suspend/resume work: sets and unsets is_hibernation
>
> Possible scenarios:
> 1. Suspend into hibernation: everything is serialized.
> dwc3_google_suspend() sets is_hibernation
> -> usb psw pdom is turned off
> -> controller enters D3
> -> pme irq could be triggered
> -> pme irq handler

In dwc3_google_suspend(), looks like is_hibernation is set after you
enable pme irq, probably very unlikely, but can the interrupt be
asserted then? If so, will there be another interrupt asserted?
Otherwise the current logic may think it was spurious interrupt a miss
an event.

> 2.1 Resume from hibernation: irq handler wins the race.
> pme irq handler (is_hibernation = true) triggers resume
> -> usb psw pdom is turned on
> -> dwc3_google_resume() unset is_hibernation
> 2.2 Resume from hibernation: resume work wins the race.
> usb psw pdom is turned on
> -> dwc3_google_resume() unset is_hibernation
> -> pme irq handler (is_hibernation = false) does nothing
>
> So far I don't see any issues in any of the scenarios, please
> let me know if otherwise.
>
> >
> > > + dr_role != DWC3_GCTL_PRTCAP_HOST) {
> > > + dev_warn(google->dev, "spurious pme irq %d, hibernation %d, dr_role %u\n",
> > > + irq, google->is_hibernation, dr_role);
> >
> > Should we limit this print and do we need this to be dev_warn? It may be
> > noisy wouldn't it.
>
> Ack, will make it WARN_ONCE in the next version.

As Greg noted, don't use WARN_ONCE. I don't know how often spurious
come, if there are many, perhaps use dev_dbg_ratelimited? If not, just
stick to dev_dbg(). The regular user doesn't need to know about this
right?

>
> >
> > > + return IRQ_HANDLED;
> > > + }
> > > +
> > > + if (dwc->xhci)
> > > + pm_runtime_resume(&dwc->xhci->dev);
> > > +
> > > + return IRQ_HANDLED;
> > > +}
> > > +
> > > +static int dwc3_google_request_irq(struct dwc3_google *google, struct platform_device *pdev,
> > > + const char *irq_name, const char *req_name)
> > > +{
> > > + int ret;
> > > + int irq;
> > > +
> > > + irq = platform_get_irq_byname(pdev, irq_name);
> > > + if (irq < 0) {
> > > + dev_err(google->dev, "invalid irq name %s\n", irq_name);
> > > + return irq;
> > > + }
> > > +
> > > + irq_set_status_flags(irq, IRQ_NOAUTOEN);
> > > + ret = devm_request_threaded_irq(google->dev, irq, NULL,
> > > + dwc3_google_resume_irq,
> > > + IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> > > + req_name, google);
> > > + if (ret < 0) {
> > > + dev_err(google->dev, "failed to request irq %s\n", req_name);
> > > + return ret;
> > > + }
> > > +
> > > + return irq;
> > > +}
> > > +
> > > +static int dwc3_google_usb_psw_pd_notifier(struct notifier_block *nb, unsigned long action, void *d)
> > > +{
> > > + struct dwc3_google *google = container_of(nb, struct dwc3_google, usb_psw_pd_nb);
> > > + int ret;
> > > +
> > > + if (!google->is_hibernation)
> > > + return NOTIFY_OK;
> > > +
> > > + if (action == GENPD_NOTIFY_OFF) {
> > > + dev_dbg(google->dev, "enter D3 power state\n");
> > > + dwc3_google_set_pmu_state(google, HOST_CFG1_PM_POWER_STATE_D3);
> >
> > What happen when we failed to set the PMU state?
>
> Apparently there must be something wrong with the hardware.
> Currently we only throw some error logs as no reliable recovery
> mechanism is known.

Just checking here because you did not return early and error out. If
it's already in a bad state and no known recovery mechanism, either way
is fine.

>
> >
> > > + ret = reset_control_assert(google->non_sticky_rst);
> > > + if (ret)
> > > + dev_err(google->dev, "non sticky reset assert failed: %d\n", ret);
> > > + } else if (action == GENPD_NOTIFY_ON) {
> > > + dev_dbg(google->dev, "enter D0 power state\n");
> > > + dwc3_google_clear_pme_irqs(google);
> > > + ret = reset_control_deassert(google->non_sticky_rst);
> > > + if (ret)
> > > + dev_err(google->dev, "non sticky reset deassert failed: %d\n", ret);
> > > + dwc3_google_set_pmu_state(google, HOST_CFG1_PM_POWER_STATE_D0);
> > > + }
> > > +
> > > + return NOTIFY_OK;
> > > +}
> > > +
> > > +static void dwc3_google_pm_domain_deinit(struct dwc3_google *google)
> > > +{
> > > + if (google->usb_top_pd_dl)
> > > + device_link_del(google->usb_top_pd_dl);
> > > +
> > > + if (!IS_ERR_OR_NULL(google->usb_top_pd)) {
> > > + device_set_wakeup_capable(google->usb_top_pd, false);
> > > + dev_pm_domain_detach(google->usb_top_pd, true);
> > > + }
> > > +
> > > + if (google->usb_psw_pd_dl)
> > > + device_link_del(google->usb_psw_pd_dl);
> > > +
> > > + if (!IS_ERR_OR_NULL(google->usb_psw_pd)) {
> > > + dev_pm_genpd_remove_notifier(google->usb_psw_pd);
> > > + dev_pm_domain_detach(google->usb_psw_pd, true);
> > > + }
> > > +}
> > > +
> > > +static int dwc3_google_pm_domain_init(struct dwc3_google *google)
> > > +{
> > > + int ret;
> > > +
> > > + /*
> > > + * Establish PM RUNTIME link between dwc dev and its power domain usb_psw_pd,
> > > + * register notifier block to handle hibernation.
> > > + */
> > > + google->usb_psw_pd = dev_pm_domain_attach_by_name(google->dev, "psw");
> > > + if (IS_ERR_OR_NULL(google->usb_psw_pd)) {
> > > + dev_err(google->dev, "failed to get psw pd");
> > > + ret = google->usb_psw_pd ? PTR_ERR(google->usb_psw_pd) : -ENODATA;
> > > + return ret;
> > > + }
> > > +
> > > + google->usb_psw_pd_nb.notifier_call = dwc3_google_usb_psw_pd_notifier;
> > > + ret = dev_pm_genpd_add_notifier(google->usb_psw_pd, &google->usb_psw_pd_nb);
> > > + if (ret) {
> > > + dev_err(google->dev, "failed to add psw pd notifier");
> > > + goto err;
> > > + }
> > > +
> > > + google->usb_psw_pd_dl = device_link_add(google->dev, google->usb_psw_pd,
> > > + DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME |
> > > + DL_FLAG_RPM_ACTIVE);
> > > + if (!google->usb_psw_pd_dl) {
> > > + dev_err(google->usb_psw_pd, "failed to add device link");
> > > + ret = -ENODEV;
> > > + goto err;
> > > + }
> > > +
> > > + /*
> > > + * usb_top_pd is the parent power domain of usb_psw_pd. Keeping usb_top_pd on
> > > + * while usb_psw_pd is off places the controller in a power-gated state,
> > > + * essential for hibernation. Acquire a handle to usb_top_pd and sets it as
> > > + * wakeup-capable to allow the domain to be left on during system suspend.
> > > + */
> >
> > I'm still trying to wrap my head around how usb_top_pd, usb_psw_pd, and
> > the google->dev are working together in the glue here, particularly why
> > usb_top_pd is needed. It seems usb_top_pd shouldn't be handled by this
> > glued? Do you do anything except setting wakeup-capable?
> >
> > BR,
> > Thinh
>
> To provide more context, the underlying usb power domain has 3 power
> states: Full Power, Power Gated, Off. The usb_top_pd and usb_psw_pd
> are the logical power domains to represent the 3 power states.
> - Full Power: usb_psw_pd ON, usb_top_p ON.
> - Power Gated: usb_psw_pd OFF, usb_top_p ON.
> - Off: usb_psw_pd OFF, usb_top_p OFF.
>
> To enter hibernation, the usb power domain must enter Power Gated
> state. To achieve this, this glue driver holds a handle to usb_top_pd
> and would cast a vote to keep it ON when attempting to enter
> hibernation. In addition, the usb_psw_pd runtime PM is directly tied
> to google->dev so that usb_psw_pd would be OFF when google->dev
> suspends. Together, the usb power domain would reach Power Gated
> state when device suspends.
>
> I hope this information helps.
>

Yes. This is very helpful.

So, while the glue driver is bound, usb_top_pd is always ON? Even when
xhci driver is not bound or when in device mode?

BR,
Thinh