2022-08-26 01:37:06

by Matthias Kaehlcke

[permalink] [raw]
Subject: [PATCH v2 2/2] clk: qcom: gcc-sc7280: Keep the USB GDSCs always on

When the GDSC is disabled during system suspend USB is broken on
sc7280 when the system resumes. Mark the GDSC as always on to
make sure USB still works after system suspend.

Signed-off-by: Matthias Kaehlcke <[email protected]>
---

Changes in v2:
- set the flags of the GDSC not of the GDSC power domain
- updated commit message

drivers/clk/qcom/gcc-sc7280.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/gcc-sc7280.c b/drivers/clk/qcom/gcc-sc7280.c
index 7ff64d4d5920..adef68d2cb0b 100644
--- a/drivers/clk/qcom/gcc-sc7280.c
+++ b/drivers/clk/qcom/gcc-sc7280.c
@@ -3127,7 +3127,7 @@ static struct gdsc gcc_usb30_prim_gdsc = {
.name = "gcc_usb30_prim_gdsc",
},
.pwrsts = PWRSTS_OFF_ON,
- .flags = VOTABLE,
+ .flags = VOTABLE | ALWAYS_ON,
};

static struct gdsc gcc_usb30_sec_gdsc = {
--
2.37.2.672.g94769d06f0-goog


2022-08-26 07:32:05

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] clk: qcom: gcc-sc7280: Keep the USB GDSCs always on

On Thu, Aug 25, 2022 at 06:21:59PM -0700, Matthias Kaehlcke wrote:
> When the GDSC is disabled during system suspend USB is broken on
> sc7280 when the system resumes. Mark the GDSC as always on to
> make sure USB still works after system suspend.
>
> Signed-off-by: Matthias Kaehlcke <[email protected]>
> ---
>
> Changes in v2:
> - set the flags of the GDSC not of the GDSC power domain
> - updated commit message
>
> drivers/clk/qcom/gcc-sc7280.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/qcom/gcc-sc7280.c b/drivers/clk/qcom/gcc-sc7280.c
> index 7ff64d4d5920..adef68d2cb0b 100644
> --- a/drivers/clk/qcom/gcc-sc7280.c
> +++ b/drivers/clk/qcom/gcc-sc7280.c

Perhaps you can add a comment here about why this is needed similar to
what I did for sc8280xp:

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

> @@ -3127,7 +3127,7 @@ static struct gdsc gcc_usb30_prim_gdsc = {
> .name = "gcc_usb30_prim_gdsc",
> },
> .pwrsts = PWRSTS_OFF_ON,
> - .flags = VOTABLE,
> + .flags = VOTABLE | ALWAYS_ON,
> };
>
> static struct gdsc gcc_usb30_sec_gdsc = {

Look good otherwise. For both patches:

Reviewed-by: Johan Hovold <[email protected]>

Johan

2022-08-26 15:40:02

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] clk: qcom: gcc-sc7280: Keep the USB GDSCs always on

On Fri, Aug 26, 2022 at 09:16:38AM +0200, Johan Hovold wrote:
> On Thu, Aug 25, 2022 at 06:21:59PM -0700, Matthias Kaehlcke wrote:
> > When the GDSC is disabled during system suspend USB is broken on
> > sc7280 when the system resumes. Mark the GDSC as always on to
> > make sure USB still works after system suspend.
> >
> > Signed-off-by: Matthias Kaehlcke <[email protected]>
> > ---
> >
> > Changes in v2:
> > - set the flags of the GDSC not of the GDSC power domain
> > - updated commit message
> >
> > drivers/clk/qcom/gcc-sc7280.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/qcom/gcc-sc7280.c b/drivers/clk/qcom/gcc-sc7280.c
> > index 7ff64d4d5920..adef68d2cb0b 100644
> > --- a/drivers/clk/qcom/gcc-sc7280.c
> > +++ b/drivers/clk/qcom/gcc-sc7280.c
>
> Perhaps you can add a comment here about why this is needed similar to
> what I did for sc8280xp:
>
> https://lore.kernel.org/all/[email protected]/

From Bjorn's comment on the sc7180 patch it seems it's not an dwc3
implementation issue. IIUC the GDSCs should have a retention state
that would be used during suspend.

> > @@ -3127,7 +3127,7 @@ static struct gdsc gcc_usb30_prim_gdsc = {
> > .name = "gcc_usb30_prim_gdsc",
> > },
> > .pwrsts = PWRSTS_OFF_ON,
> > - .flags = VOTABLE,
> > + .flags = VOTABLE | ALWAYS_ON,
> > };
> >
> > static struct gdsc gcc_usb30_sec_gdsc = {
>
> Look good otherwise. For both patches:
>
> Reviewed-by: Johan Hovold <[email protected]>

Thanks!