2022-08-02 15:32:39

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 0/8] usb: dwc3: qcom: fix wakeup implementation

This series fixes some of the fallout after the recently merged series
that added wakeup support to the Qualcomm dwc3 driver:

https://lore.kernel.org/all/[email protected]/

The first patch fixes a long standing PHY power sequencing issue in dwc3
core.

The second patch reverts a power-domain hack that was added by the above
series. There are other ways to implement this which doesn't violate the
genpd interface, and if for some reason those are not sufficient, the
genpd implementation needs to be extended, not hacked around.

The third patch fixes a couple of NULL-pointer dereferences when
suspending controllers in peripheral or OTG mode due to a hack that was
added to suspend path. Unfortunately, it seems the hack needs to stay
for now if we want functioning suspend on some Qualcomm platforms.

The fourth patch fixes another issue in the Qualcomm dwc3 implementation
that has been added a while back and which breaks runtime PM.

The remaining patches moves the wakeup-source property over from the
core node to the glue node in the binding and instead propagates the
wakeup capability to the former during probe.

Note that this incidentally also avoids adding probe-deferral hacks to
the driver as was recently proposed to deal with another problem with
the current implementation:

https://lore.kernel.org/all/[email protected]/

With this series I have functioning USB system suspend and wakeup as
well as somewhat functioning runtime PM in host mode on sc8280xp. Note
that the PHYs apparently do not need to be shutdown for wakeup on this
platform.

Some issues remain such as that the controller needs to be suspended to
handle remote wakeup during runtime PM (the wakeup interrupts probably
needs to be enabled whenever there's a wakeup-capable device connected
to the bus) and that root hub connect/disconnect events cannot
selectively be disabled.

And of course, the suspend speed hack needs to be replaced at some
point but that likely requires some more heavy lifting in the dwc3
implementation.

Johan


Johan Hovold (8):
usb: dwc3: fix PHY disable sequence
Revert "usb: dwc3: qcom: Keep power domain on to retain controller
status"
usb: dwc3: qcom: fix broken non-host-mode suspend
usb: dwc3: qcom: fix runtime PM wakeup
Revert "dt-bindings: usb: dwc3: Add wakeup-source property support"
dt-bindings: usb: qcom,dwc3: add wakeup-source property
usb: dwc3: qcom: fix wakeup implementation
usb: dwc3: qcom: clean up suspend callbacks

.../devicetree/bindings/usb/qcom,dwc3.yaml | 2 +
.../devicetree/bindings/usb/snps,dwc3.yaml | 5 --
drivers/usb/dwc3/core.c | 24 +++---
drivers/usb/dwc3/dwc3-qcom.c | 77 ++++++++++---------
4 files changed, 55 insertions(+), 53 deletions(-)

--
2.35.1



2022-08-02 16:34:34

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 1/8] usb: dwc3: fix PHY disable sequence

Generic PHYs must be powered-off before they can be tore down.

Similarly, suspending legacy PHYs after having powered them off makes no
sense.

Fix the dwc3_core_exit() (e.g. called during suspend) and open-coded
dwc3_probe() error-path sequences that got this wrong.

Note that this makes dwc3_core_exit() match the dwc3_core_init() error
path with respect to powering off the PHYs.

Fixes: 03c1fd622f72 ("usb: dwc3: core: add phy cleanup for probe error handling")
Fixes: c499ff71ff2a ("usb: dwc3: core: re-factor init and exit paths")
Cc: [email protected] # 4.8
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/dwc3/core.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index c5c238ab3083..16d1f328775f 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -833,15 +833,16 @@ static void dwc3_core_exit(struct dwc3 *dwc)
{
dwc3_event_buffers_cleanup(dwc);

+ usb_phy_set_suspend(dwc->usb2_phy, 1);
+ usb_phy_set_suspend(dwc->usb3_phy, 1);
+ phy_power_off(dwc->usb2_generic_phy);
+ phy_power_off(dwc->usb3_generic_phy);
+
usb_phy_shutdown(dwc->usb2_phy);
usb_phy_shutdown(dwc->usb3_phy);
phy_exit(dwc->usb2_generic_phy);
phy_exit(dwc->usb3_generic_phy);

- usb_phy_set_suspend(dwc->usb2_phy, 1);
- usb_phy_set_suspend(dwc->usb3_phy, 1);
- phy_power_off(dwc->usb2_generic_phy);
- phy_power_off(dwc->usb3_generic_phy);
dwc3_clk_disable(dwc);
reset_control_assert(dwc->reset);
}
@@ -1879,16 +1880,16 @@ static int dwc3_probe(struct platform_device *pdev)
dwc3_debugfs_exit(dwc);
dwc3_event_buffers_cleanup(dwc);

- usb_phy_shutdown(dwc->usb2_phy);
- usb_phy_shutdown(dwc->usb3_phy);
- phy_exit(dwc->usb2_generic_phy);
- phy_exit(dwc->usb3_generic_phy);
-
usb_phy_set_suspend(dwc->usb2_phy, 1);
usb_phy_set_suspend(dwc->usb3_phy, 1);
phy_power_off(dwc->usb2_generic_phy);
phy_power_off(dwc->usb3_generic_phy);

+ usb_phy_shutdown(dwc->usb2_phy);
+ usb_phy_shutdown(dwc->usb3_phy);
+ phy_exit(dwc->usb2_generic_phy);
+ phy_exit(dwc->usb3_generic_phy);
+
dwc3_ulpi_exit(dwc);

err4:
--
2.35.1


2022-08-02 16:35:08

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 2/8] Revert "usb: dwc3: qcom: Keep power domain on to retain controller status"

This reverts commit d9be8d5c5b032e5383ff5c404ff4155e9c705429.

Generic power-domain flags must be set before the power-domain is
initialised and must specifically not be modified by drivers for devices
that happen to be in the domain.

To make sure that USB power-domains are left enabled during system
suspend when a device in the domain is in the wakeup path, the
GENPD_FLAG_ACTIVE_WAKEUP flag should instead be set for the domain
unconditionally when it is registered.

Note that this also avoids keeping power-domains on during suspend when
wakeup has not been enabled (e.g. through sysfs).

For the runtime PM case, making sure that the PHYs are not suspended and
that they are in the same domain as the controller prevents the domain
from being suspended. If there are cases where this is not possible or
desirable, the genpd implementation may need to be extended.

Fixes: d9be8d5c5b03 ("usb: dwc3: qcom: Keep power domain on to retain controller status")
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/dwc3/dwc3-qcom.c | 28 +++++++---------------------
1 file changed, 7 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index c5e482f53e9d..be2e3dd36440 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -17,7 +17,6 @@
#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/phy/phy.h>
-#include <linux/pm_domain.h>
#include <linux/usb/of.h>
#include <linux/reset.h>
#include <linux/iopoll.h>
@@ -757,13 +756,12 @@ dwc3_qcom_create_urs_usb_platdev(struct device *dev)

static int dwc3_qcom_probe(struct platform_device *pdev)
{
- struct device_node *np = pdev->dev.of_node;
- struct device *dev = &pdev->dev;
- struct dwc3_qcom *qcom;
- struct resource *res, *parent_res = NULL;
- int ret, i;
- bool ignore_pipe_clk;
- struct generic_pm_domain *genpd;
+ struct device_node *np = pdev->dev.of_node;
+ struct device *dev = &pdev->dev;
+ struct dwc3_qcom *qcom;
+ struct resource *res, *parent_res = NULL;
+ int ret, i;
+ bool ignore_pipe_clk;

qcom = devm_kzalloc(&pdev->dev, sizeof(*qcom), GFP_KERNEL);
if (!qcom)
@@ -772,8 +770,6 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, qcom);
qcom->dev = &pdev->dev;

- genpd = pd_to_genpd(qcom->dev->pm_domain);
-
if (has_acpi_companion(dev)) {
qcom->acpi_pdata = acpi_device_get_match_data(dev);
if (!qcom->acpi_pdata) {
@@ -881,17 +877,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
if (ret)
goto interconnect_exit;

- if (device_can_wakeup(&qcom->dwc3->dev)) {
- /*
- * Setting GENPD_FLAG_ALWAYS_ON flag takes care of keeping
- * genpd on in both runtime suspend and system suspend cases.
- */
- genpd->flags |= GENPD_FLAG_ALWAYS_ON;
- device_init_wakeup(&pdev->dev, true);
- } else {
- genpd->flags |= GENPD_FLAG_RPM_ALWAYS_ON;
- }
-
+ device_init_wakeup(&pdev->dev, 1);
qcom->is_suspended = false;
pm_runtime_set_active(dev);
pm_runtime_enable(dev);
--
2.35.1


2022-08-02 16:36:39

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 8/8] usb: dwc3: qcom: clean up suspend callbacks

Clean up the suspend callbacks by separating the error and success paths
to improve readability.

Also drop a related redundant initialisation.

Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/dwc3/dwc3-qcom.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index 1bd2818b4daa..01e8c2fc6914 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -944,14 +944,15 @@ static int __maybe_unused dwc3_qcom_pm_suspend(struct device *dev)
{
struct dwc3_qcom *qcom = dev_get_drvdata(dev);
bool wakeup = device_may_wakeup(dev);
- int ret = 0;
-
+ int ret;

ret = dwc3_qcom_suspend(qcom, wakeup);
- if (!ret)
- qcom->pm_suspended = true;
+ if (ret)
+ return ret;

- return ret;
+ qcom->pm_suspended = true;
+
+ return 0;
}

static int __maybe_unused dwc3_qcom_pm_resume(struct device *dev)
@@ -961,10 +962,12 @@ static int __maybe_unused dwc3_qcom_pm_resume(struct device *dev)
int ret;

ret = dwc3_qcom_resume(qcom, wakeup);
- if (!ret)
- qcom->pm_suspended = false;
+ if (ret)
+ return ret;

- return ret;
+ qcom->pm_suspended = false;
+
+ return 0;
}

static int __maybe_unused dwc3_qcom_runtime_suspend(struct device *dev)
--
2.35.1


2022-08-02 16:37:31

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 7/8] usb: dwc3: qcom: fix wakeup implementation

It is the Qualcomm glue wakeup interrupts that may be able to wake the
system from suspend and this can now be described in the devicetree.

Move the wakeup-source property handling over from the core driver and
instead propagate the capability setting to the core device during
probe.

This is needed as there is currently no way for the core driver to query
the wakeup setting of the glue device, but it is the core driver that
manages the PHY power state during suspend.

Also don't leave the PHYs enabled when system wakeup has been disabled
through sysfs.

Fixes: 649f5c842ba3 ("usb: dwc3: core: Host wake up support from system suspend")
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/dwc3/core.c | 5 ++---
drivers/usb/dwc3/dwc3-qcom.c | 6 +++++-
2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 16d1f328775f..8c8e32651473 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1822,7 +1822,6 @@ static int dwc3_probe(struct platform_device *pdev)

platform_set_drvdata(pdev, dwc);
dwc3_cache_hwparams(dwc);
- device_init_wakeup(&pdev->dev, of_property_read_bool(dev->of_node, "wakeup-source"));

spin_lock_init(&dwc->lock);
mutex_init(&dwc->mutex);
@@ -1984,7 +1983,7 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
dwc3_core_exit(dwc);
break;
case DWC3_GCTL_PRTCAP_HOST:
- if (!PMSG_IS_AUTO(msg) && !device_can_wakeup(dwc->dev)) {
+ if (!PMSG_IS_AUTO(msg) && !device_may_wakeup(dwc->dev)) {
dwc3_core_exit(dwc);
break;
}
@@ -2045,7 +2044,7 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
spin_unlock_irqrestore(&dwc->lock, flags);
break;
case DWC3_GCTL_PRTCAP_HOST:
- if (!PMSG_IS_AUTO(msg) && !device_can_wakeup(dwc->dev)) {
+ if (!PMSG_IS_AUTO(msg) && !device_may_wakeup(dwc->dev)) {
ret = dwc3_core_init_for_resume(dwc);
if (ret)
return ret;
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index 57d3a0e6f280..1bd2818b4daa 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -771,6 +771,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
struct resource *res, *parent_res = NULL;
int ret, i;
bool ignore_pipe_clk;
+ bool wakeup_source;

qcom = devm_kzalloc(&pdev->dev, sizeof(*qcom), GFP_KERNEL);
if (!qcom)
@@ -886,7 +887,10 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
if (ret)
goto interconnect_exit;

- device_init_wakeup(&pdev->dev, 1);
+ wakeup_source = of_property_read_bool(dev->of_node, "wakeup-source");
+ device_init_wakeup(&pdev->dev, wakeup_source);
+ device_init_wakeup(&qcom->dwc3->dev, wakeup_source);
+
qcom->is_suspended = false;
pm_runtime_set_active(dev);
pm_runtime_enable(dev);
--
2.35.1


2022-08-02 18:23:44

by Krishna Kurapati PSSNV

[permalink] [raw]
Subject: Re: [PATCH 2/8] Revert "usb: dwc3: qcom: Keep power domain on to retain controller status"


On 8/2/2022 8:43 PM, Johan Hovold wrote:
> This reverts commit d9be8d5c5b032e5383ff5c404ff4155e9c705429.
>
> Generic power-domain flags must be set before the power-domain is
> initialised and must specifically not be modified by drivers for devices
> that happen to be in the domain.
>
> To make sure that USB power-domains are left enabled during system
> suspend when a device in the domain is in the wakeup path, the
> GENPD_FLAG_ACTIVE_WAKEUP flag should instead be set for the domain
> unconditionally when it is registered.
Hi Johan, Thanks for the review and suggestion.

In case we need the genpd framework to set the GENPD_FLAG_ACTIVE_WAKEUP
flag for a particular domain that will be used by a device (which is
wakeup capable) and hasn't been probed yet , can you help me understand if
there any dt-flags we  can add to convey the same the genpd  framework
so that it will set that flag during domain registration ?

Regards,
Krishna,
>
> Note that this also avoids keeping power-domains on during suspend when
> wakeup has not been enabled (e.g. through sysfs).
>
> For the runtime PM case, making sure that the PHYs are not suspended and
> that they are in the same domain as the controller prevents the domain
> from being suspended. If there are cases where this is not possible or
> desirable, the genpd implementation may need to be extended.
>
> Fixes: d9be8d5c5b03 ("usb: dwc3: qcom: Keep power domain on to retain controller status")
> Signed-off-by: Johan Hovold <[email protected]>
> ---
> drivers/usb/dwc3/dwc3-qcom.c | 28 +++++++---------------------
> 1 file changed, 7 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index c5e482f53e9d..be2e3dd36440 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -17,7 +17,6 @@
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
> #include <linux/phy/phy.h>
> -#include <linux/pm_domain.h>
> #include <linux/usb/of.h>
> #include <linux/reset.h>
> #include <linux/iopoll.h>
> @@ -757,13 +756,12 @@ dwc3_qcom_create_urs_usb_platdev(struct device *dev)
>
> static int dwc3_qcom_probe(struct platform_device *pdev)
> {
> - struct device_node *np = pdev->dev.of_node;
> - struct device *dev = &pdev->dev;
> - struct dwc3_qcom *qcom;
> - struct resource *res, *parent_res = NULL;
> - int ret, i;
> - bool ignore_pipe_clk;
> - struct generic_pm_domain *genpd;
> + struct device_node *np = pdev->dev.of_node;
> + struct device *dev = &pdev->dev;
> + struct dwc3_qcom *qcom;
> + struct resource *res, *parent_res = NULL;
> + int ret, i;
> + bool ignore_pipe_clk;
>
> qcom = devm_kzalloc(&pdev->dev, sizeof(*qcom), GFP_KERNEL);
> if (!qcom)
> @@ -772,8 +770,6 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, qcom);
> qcom->dev = &pdev->dev;
>
> - genpd = pd_to_genpd(qcom->dev->pm_domain);
> -
> if (has_acpi_companion(dev)) {
> qcom->acpi_pdata = acpi_device_get_match_data(dev);
> if (!qcom->acpi_pdata) {
> @@ -881,17 +877,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
> if (ret)
> goto interconnect_exit;
>
> - if (device_can_wakeup(&qcom->dwc3->dev)) {
> - /*
> - * Setting GENPD_FLAG_ALWAYS_ON flag takes care of keeping
> - * genpd on in both runtime suspend and system suspend cases.
> - */
> - genpd->flags |= GENPD_FLAG_ALWAYS_ON;
> - device_init_wakeup(&pdev->dev, true);
> - } else {
> - genpd->flags |= GENPD_FLAG_RPM_ALWAYS_ON;
> - }
> -
> + device_init_wakeup(&pdev->dev, 1);
> qcom->is_suspended = false;
> pm_runtime_set_active(dev);
> pm_runtime_enable(dev);

2022-08-02 21:50:02

by Andrew Halaney

[permalink] [raw]
Subject: Re: [PATCH 1/8] usb: dwc3: fix PHY disable sequence

On Tue, Aug 02, 2022 at 05:13:57PM +0200, Johan Hovold wrote:
> Generic PHYs must be powered-off before they can be tore down.
>
> Similarly, suspending legacy PHYs after having powered them off makes no
> sense.
>
> Fix the dwc3_core_exit() (e.g. called during suspend) and open-coded
> dwc3_probe() error-path sequences that got this wrong.
>
> Note that this makes dwc3_core_exit() match the dwc3_core_init() error
> path with respect to powering off the PHYs.
>
> Fixes: 03c1fd622f72 ("usb: dwc3: core: add phy cleanup for probe error handling")
> Fixes: c499ff71ff2a ("usb: dwc3: core: re-factor init and exit paths")
> Cc: [email protected] # 4.8
> Signed-off-by: Johan Hovold <[email protected]>
> ---

Reviewed-by: Andrew Halaney <[email protected]>

> drivers/usb/dwc3/core.c | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index c5c238ab3083..16d1f328775f 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -833,15 +833,16 @@ static void dwc3_core_exit(struct dwc3 *dwc)
> {
> dwc3_event_buffers_cleanup(dwc);
>
> + usb_phy_set_suspend(dwc->usb2_phy, 1);
> + usb_phy_set_suspend(dwc->usb3_phy, 1);
> + phy_power_off(dwc->usb2_generic_phy);
> + phy_power_off(dwc->usb3_generic_phy);
> +
> usb_phy_shutdown(dwc->usb2_phy);
> usb_phy_shutdown(dwc->usb3_phy);
> phy_exit(dwc->usb2_generic_phy);
> phy_exit(dwc->usb3_generic_phy);
>
> - usb_phy_set_suspend(dwc->usb2_phy, 1);
> - usb_phy_set_suspend(dwc->usb3_phy, 1);
> - phy_power_off(dwc->usb2_generic_phy);
> - phy_power_off(dwc->usb3_generic_phy);
> dwc3_clk_disable(dwc);
> reset_control_assert(dwc->reset);
> }
> @@ -1879,16 +1880,16 @@ static int dwc3_probe(struct platform_device *pdev)
> dwc3_debugfs_exit(dwc);
> dwc3_event_buffers_cleanup(dwc);
>
> - usb_phy_shutdown(dwc->usb2_phy);
> - usb_phy_shutdown(dwc->usb3_phy);
> - phy_exit(dwc->usb2_generic_phy);
> - phy_exit(dwc->usb3_generic_phy);
> -
> usb_phy_set_suspend(dwc->usb2_phy, 1);
> usb_phy_set_suspend(dwc->usb3_phy, 1);
> phy_power_off(dwc->usb2_generic_phy);
> phy_power_off(dwc->usb3_generic_phy);
>
> + usb_phy_shutdown(dwc->usb2_phy);
> + usb_phy_shutdown(dwc->usb3_phy);
> + phy_exit(dwc->usb2_generic_phy);
> + phy_exit(dwc->usb3_generic_phy);
> +
> dwc3_ulpi_exit(dwc);
>
> err4:
> --
> 2.35.1
>


2022-08-03 08:40:48

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH 2/8] Revert "usb: dwc3: qcom: Keep power domain on to retain controller status"

On Tue, Aug 02, 2022 at 11:30:34PM +0530, Krishna Kurapati PSSNV wrote:
>
> On 8/2/2022 8:43 PM, Johan Hovold wrote:
> > This reverts commit d9be8d5c5b032e5383ff5c404ff4155e9c705429.
> >
> > Generic power-domain flags must be set before the power-domain is
> > initialised and must specifically not be modified by drivers for devices
> > that happen to be in the domain.
> >
> > To make sure that USB power-domains are left enabled during system
> > suspend when a device in the domain is in the wakeup path, the
> > GENPD_FLAG_ACTIVE_WAKEUP flag should instead be set for the domain
> > unconditionally when it is registered.

> In case we need the genpd framework to set the GENPD_FLAG_ACTIVE_WAKEUP
> flag for a particular domain that will be used by a device (which is
> wakeup capable) and hasn't been probed yet , can you help me understand if
> there any dt-flags we  can add to convey the same the genpd  framework
> so that it will set that flag during domain registration ?

This can't be expressed in DT (currently), so what you need is something
like the below:

diff --git a/drivers/clk/qcom/gcc-sc7280.c b/drivers/clk/qcom/gcc-sc7280.c
index 7ff64d4d5920..4ff855269467 100644
--- a/drivers/clk/qcom/gcc-sc7280.c
+++ b/drivers/clk/qcom/gcc-sc7280.c
@@ -3125,6 +3125,7 @@ static struct gdsc gcc_usb30_prim_gdsc = {
.gdscr = 0xf004,
.pd = {
.name = "gcc_usb30_prim_gdsc",
+ .flags = GENPD_FLAG_ACTIVE_WAKEUP,
},
.pwrsts = PWRSTS_OFF_ON,
.flags = VOTABLE,
@@ -3134,6 +3135,7 @@ static struct gdsc gcc_usb30_sec_gdsc = {
.gdscr = 0x9e004,
.pd = {
.name = "gcc_usb30_sec_gdsc",
+ .flags = GENPD_FLAG_ACTIVE_WAKEUP,
},
.pwrsts = PWRSTS_OFF_ON,
.flags = VOTABLE,

to make sure that genpd keeps these domains powered during system
suspend if they are used by devices that are in the wakeup path.

Johan

2022-08-03 17:49:43

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH 1/8] usb: dwc3: fix PHY disable sequence

On Tue, Aug 02, 2022 at 05:13:57PM +0200, Johan Hovold wrote:
> Generic PHYs must be powered-off before they can be tore down.
>
> Similarly, suspending legacy PHYs after having powered them off makes no
> sense.
>
> Fix the dwc3_core_exit() (e.g. called during suspend) and open-coded
> dwc3_probe() error-path sequences that got this wrong.
>
> Note that this makes dwc3_core_exit() match the dwc3_core_init() error
> path with respect to powering off the PHYs.
>
> Fixes: 03c1fd622f72 ("usb: dwc3: core: add phy cleanup for probe error handling")
> Fixes: c499ff71ff2a ("usb: dwc3: core: re-factor init and exit paths")
> Cc: [email protected] # 4.8
> Signed-off-by: Johan Hovold <[email protected]>

I also wondered about this earlier, but didn't take action.

Reviewed-by: Matthias Kaehlcke <[email protected]>