Message-ID: <CA+zupgy4qO9X=R7KqEru5kr7tYhgdw=9Z70sLNKj5DTS_J7KZw@mail.gmail.com>
Date: Tue, 4 Nov 2025 18:55:54 +0800
From: Roy Luo <royluo@...gle.com>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Cc: 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 Tue, Nov 4, 2025 at 10:07 AM Thinh Nguyen <Thinh.Nguyen@...opsys.com> wrote:
>
> On Fri, Oct 31, 2025, Roy Luo wrote:
> > On Wed, Oct 29, 2025 at 6:35 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
> > > >
> > > > 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@...gle.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.

The pme interrupt can only be asserted after controller is in
hibernation, that is, after the usb psw dom is turned off and
the dwc3_google_usb_psw_pd_notifier() callback is
completed. So no, the interrupt won't fire before is_hibernation
is set.

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

This case should be very very rare, I haven't seen it on the field.
And yes, this log isn't for regular end users.
I will change it to dev_dbg() as suggested in the next version.

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

Ack.

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

Since usb_top_pd is the parent power domain of usb_psw_pd, and
usb_psw_pd RPM is directly tied to glue device, usb_top_pd would
be ON when glue device is active (because usb_psw_pd is ON)
and would be OFF when glue device suspends in non-hibernation
scenarios (because usb_psw_pd is OFF). In hibernation scenario,
a vote is casted for usb_top_pd to keep it on even when the
glue device is suspended and usb_psw_pd is OFF.

To your question, usb_top_pd is not always ON because it would be
turned off when the glue device suspends in non-hibernation scenario.
When in device mode and provided dwc3 dev is active, usb_top_pd
would be ON because its child usb_psw_pd is ON.

Regards,
Roy Luo

>
> BR,
> Thinh

Powered by blists - more mailing lists