GDSCs cannot be transitioned into a Retention state in SW.
When either the RETAIN_MEM bit, or both the RETAIN_MEM and
RETAIN_PERIPH bits are set, and the GDSC is left ON, the HW
takes care of retaining the memory/logic for the domain when
the parent domain transitions to low power state.
The existing logic handling the PWRSTS_RET seems to set the
RETAIN_MEM/RETAIN_PERIPH bits but then explicitly turns the
GDSC OFF as part of _gdsc_disable(). Fix that by leaving the
GDSC in ON state.
Signed-off-by: Rajendra Nayak <[email protected]>
Cc: AngeloGioacchino Del Regno <[email protected]>
---
No changes in v2:
There are a few existing users of PWRSTS_RET and I am not
sure if they would be impacted with this change
1. mdss_gdsc in mmcc-msm8974.c, I am expecting that the
gdsc is actually transitioning to OFF and might be left
ON as part of this change, atleast till we hit system wide
low power state.
If we really leak more power because of this
change, the right thing to do would be to update .pwrsts for
mdss_gdsc to PWRSTS_OFF_ON instead of PWRSTS_RET_ON
I dont have a msm8974 hardware, so if anyone who has can report
any issues I can take a look further on how to fix it.
2. gpu_gx_gdsc in gpucc-msm8998.c and
gpu_gx_gdsc in gpucc-sdm660.c
Both of these seem to add support for 3 power state
OFF, RET and ON, however I dont see any logic in gdsc
driver to handle 3 different power states.
So I am expecting that these are infact just transitioning
between ON and OFF and RET state is never really used.
The ideal fix for them would be to just update their resp.
.pwrsts to PWRSTS_OFF_ON only.
drivers/clk/qcom/gdsc.c | 10 ++++++++++
drivers/clk/qcom/gdsc.h | 5 +++++
2 files changed, 15 insertions(+)
diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index d3244006c661..ccf63771e852 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -368,6 +368,16 @@ static int _gdsc_disable(struct gdsc *sc)
if (sc->pwrsts & PWRSTS_OFF)
gdsc_clear_mem_on(sc);
+ /*
+ * If the GDSC supports only a Retention state, apart from ON,
+ * leave it in ON state.
+ * There is no SW control to transition the GDSC into
+ * Retention state. This happens in HW when the parent
+ * domain goes down to a Low power state
+ */
+ if (sc->pwrsts == PWRSTS_RET_ON)
+ return 0;
+
ret = gdsc_toggle_logic(sc, GDSC_OFF);
if (ret)
return ret;
diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
index 5de48c9439b2..981a12c8502d 100644
--- a/drivers/clk/qcom/gdsc.h
+++ b/drivers/clk/qcom/gdsc.h
@@ -49,6 +49,11 @@ struct gdsc {
const u8 pwrsts;
/* Powerdomain allowable state bitfields */
#define PWRSTS_OFF BIT(0)
+/*
+ * There is no SW control to transition a GDSC into
+ * PWRSTS_RET. This happens in HW when the parent
+ * domain goes down to a low power state
+ */
#define PWRSTS_RET BIT(1)
#define PWRSTS_ON BIT(2)
#define PWRSTS_OFF_ON (PWRSTS_OFF | PWRSTS_ON)
--
2.17.1
The USB controller on sc7180 does not retain the state when
the system goes into low power state and the GDSC is
turned off. This results in the controller reinitializing and
re-enumerating all the connected devices (resulting in additional
delay while coming out of suspend)
Fix this by updating the .pwrsts for the USB GDSC so it only
transitions to retention state in low power.
Signed-off-by: Rajendra Nayak <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
Tested-by: Matthias Kaehlcke <[email protected]>
---
v2:
Updated the changelog
drivers/clk/qcom/gcc-sc7180.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/qcom/gcc-sc7180.c b/drivers/clk/qcom/gcc-sc7180.c
index c2ea09945c47..2d3980251e78 100644
--- a/drivers/clk/qcom/gcc-sc7180.c
+++ b/drivers/clk/qcom/gcc-sc7180.c
@@ -2224,7 +2224,7 @@ static struct gdsc usb30_prim_gdsc = {
.pd = {
.name = "usb30_prim_gdsc",
},
- .pwrsts = PWRSTS_OFF_ON,
+ .pwrsts = PWRSTS_RET_ON,
};
static struct gdsc hlos1_vote_mmnoc_mmu_tbu_hf0_gdsc = {
--
2.17.1
On Fri, Sep 16, 2022 at 03:54:15PM +0530, Rajendra Nayak wrote:
> GDSCs cannot be transitioned into a Retention state in SW.
> When either the RETAIN_MEM bit, or both the RETAIN_MEM and
> RETAIN_PERIPH bits are set, and the GDSC is left ON, the HW
> takes care of retaining the memory/logic for the domain when
> the parent domain transitions to low power state.
> The existing logic handling the PWRSTS_RET seems to set the
> RETAIN_MEM/RETAIN_PERIPH bits but then explicitly turns the
> GDSC OFF as part of _gdsc_disable(). Fix that by leaving the
> GDSC in ON state.
>
> Signed-off-by: Rajendra Nayak <[email protected]>
> Cc: AngeloGioacchino Del Regno <[email protected]>
Reviewed-by: Bjorn Andersson <[email protected]>
Regards,
Bjorn
> ---
> No changes in v2:
>
> There are a few existing users of PWRSTS_RET and I am not
> sure if they would be impacted with this change
>
> 1. mdss_gdsc in mmcc-msm8974.c, I am expecting that the
> gdsc is actually transitioning to OFF and might be left
> ON as part of this change, atleast till we hit system wide
> low power state.
> If we really leak more power because of this
> change, the right thing to do would be to update .pwrsts for
> mdss_gdsc to PWRSTS_OFF_ON instead of PWRSTS_RET_ON
> I dont have a msm8974 hardware, so if anyone who has can report
> any issues I can take a look further on how to fix it.
>
> 2. gpu_gx_gdsc in gpucc-msm8998.c and
> gpu_gx_gdsc in gpucc-sdm660.c
> Both of these seem to add support for 3 power state
> OFF, RET and ON, however I dont see any logic in gdsc
> driver to handle 3 different power states.
> So I am expecting that these are infact just transitioning
> between ON and OFF and RET state is never really used.
> The ideal fix for them would be to just update their resp.
> .pwrsts to PWRSTS_OFF_ON only.
>
> drivers/clk/qcom/gdsc.c | 10 ++++++++++
> drivers/clk/qcom/gdsc.h | 5 +++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> index d3244006c661..ccf63771e852 100644
> --- a/drivers/clk/qcom/gdsc.c
> +++ b/drivers/clk/qcom/gdsc.c
> @@ -368,6 +368,16 @@ static int _gdsc_disable(struct gdsc *sc)
> if (sc->pwrsts & PWRSTS_OFF)
> gdsc_clear_mem_on(sc);
>
> + /*
> + * If the GDSC supports only a Retention state, apart from ON,
> + * leave it in ON state.
> + * There is no SW control to transition the GDSC into
> + * Retention state. This happens in HW when the parent
> + * domain goes down to a Low power state
> + */
> + if (sc->pwrsts == PWRSTS_RET_ON)
> + return 0;
> +
> ret = gdsc_toggle_logic(sc, GDSC_OFF);
> if (ret)
> return ret;
> diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
> index 5de48c9439b2..981a12c8502d 100644
> --- a/drivers/clk/qcom/gdsc.h
> +++ b/drivers/clk/qcom/gdsc.h
> @@ -49,6 +49,11 @@ struct gdsc {
> const u8 pwrsts;
> /* Powerdomain allowable state bitfields */
> #define PWRSTS_OFF BIT(0)
> +/*
> + * There is no SW control to transition a GDSC into
> + * PWRSTS_RET. This happens in HW when the parent
> + * domain goes down to a low power state
> + */
> #define PWRSTS_RET BIT(1)
> #define PWRSTS_ON BIT(2)
> #define PWRSTS_OFF_ON (PWRSTS_OFF | PWRSTS_ON)
> --
> 2.17.1
>
On Fri, Sep 16, 2022 at 02:06:58PM -0500, Bjorn Andersson wrote:
> On Fri, Sep 16, 2022 at 03:54:16PM +0530, Rajendra Nayak wrote:
> > The USB controller on sc7180 does not retain the state when
> > the system goes into low power state and the GDSC is
> > turned off. This results in the controller reinitializing and
> > re-enumerating all the connected devices (resulting in additional
> > delay while coming out of suspend)
> > Fix this by updating the .pwrsts for the USB GDSC so it only
> > transitions to retention state in low power.
> >
> > Signed-off-by: Rajendra Nayak <[email protected]>
> > Reviewed-by: Matthias Kaehlcke <[email protected]>
> > Tested-by: Matthias Kaehlcke <[email protected]>
> > ---
> > v2:
> > Updated the changelog
> >
> > drivers/clk/qcom/gcc-sc7180.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/qcom/gcc-sc7180.c b/drivers/clk/qcom/gcc-sc7180.c
> > index c2ea09945c47..2d3980251e78 100644
> > --- a/drivers/clk/qcom/gcc-sc7180.c
> > +++ b/drivers/clk/qcom/gcc-sc7180.c
> > @@ -2224,7 +2224,7 @@ static struct gdsc usb30_prim_gdsc = {
> > .pd = {
> > .name = "usb30_prim_gdsc",
> > },
> > - .pwrsts = PWRSTS_OFF_ON,
> > + .pwrsts = PWRSTS_RET_ON,
>
> I presume the same should be applied to gcc_usb30_sec_gdsc?
>
Please ignore that, missed the '1' in the filename.
Reviewed-by: Bjorn Andersson <[email protected]>
Regards,
Bjorn
On Fri, Sep 16, 2022 at 03:54:16PM +0530, Rajendra Nayak wrote:
> The USB controller on sc7180 does not retain the state when
> the system goes into low power state and the GDSC is
> turned off. This results in the controller reinitializing and
> re-enumerating all the connected devices (resulting in additional
> delay while coming out of suspend)
> Fix this by updating the .pwrsts for the USB GDSC so it only
> transitions to retention state in low power.
>
> Signed-off-by: Rajendra Nayak <[email protected]>
> Reviewed-by: Matthias Kaehlcke <[email protected]>
> Tested-by: Matthias Kaehlcke <[email protected]>
> ---
> v2:
> Updated the changelog
>
> drivers/clk/qcom/gcc-sc7180.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/qcom/gcc-sc7180.c b/drivers/clk/qcom/gcc-sc7180.c
> index c2ea09945c47..2d3980251e78 100644
> --- a/drivers/clk/qcom/gcc-sc7180.c
> +++ b/drivers/clk/qcom/gcc-sc7180.c
> @@ -2224,7 +2224,7 @@ static struct gdsc usb30_prim_gdsc = {
> .pd = {
> .name = "usb30_prim_gdsc",
> },
> - .pwrsts = PWRSTS_OFF_ON,
> + .pwrsts = PWRSTS_RET_ON,
I presume the same should be applied to gcc_usb30_sec_gdsc?
Regards,
Bjorn
> };
>
> static struct gdsc hlos1_vote_mmnoc_mmu_tbu_hf0_gdsc = {
> --
> 2.17.1
>
On 16.09.2022 12:24, Rajendra Nayak wrote:
> GDSCs cannot be transitioned into a Retention state in SW.
> When either the RETAIN_MEM bit, or both the RETAIN_MEM and
> RETAIN_PERIPH bits are set, and the GDSC is left ON, the HW
> takes care of retaining the memory/logic for the domain when
> the parent domain transitions to low power state.
> The existing logic handling the PWRSTS_RET seems to set the
> RETAIN_MEM/RETAIN_PERIPH bits but then explicitly turns the
> GDSC OFF as part of _gdsc_disable(). Fix that by leaving the
> GDSC in ON state.
>
> Signed-off-by: Rajendra Nayak <[email protected]>
> Cc: AngeloGioacchino Del Regno <[email protected]>
> ---
> No changes in v2:
>
> There are a few existing users of PWRSTS_RET and I am not
> sure if they would be impacted with this change
>
> 1. mdss_gdsc in mmcc-msm8974.c, I am expecting that the
> gdsc is actually transitioning to OFF and might be left
> ON as part of this change, atleast till we hit system wide
> low power state.
> If we really leak more power because of this
> change, the right thing to do would be to update .pwrsts for
> mdss_gdsc to PWRSTS_OFF_ON instead of PWRSTS_RET_ON
> I dont have a msm8974 hardware, so if anyone who has can report
> any issues I can take a look further on how to fix it.
>
> 2. gpu_gx_gdsc in gpucc-msm8998.c and
> gpu_gx_gdsc in gpucc-sdm660.c
> Both of these seem to add support for 3 power state
> OFF, RET and ON, however I dont see any logic in gdsc
> driver to handle 3 different power states.
> So I am expecting that these are infact just transitioning
> between ON and OFF and RET state is never really used.
> The ideal fix for them would be to just update their resp.
> .pwrsts to PWRSTS_OFF_ON only.
>
> drivers/clk/qcom/gdsc.c | 10 ++++++++++
> drivers/clk/qcom/gdsc.h | 5 +++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> index d3244006c661..ccf63771e852 100644
> --- a/drivers/clk/qcom/gdsc.c
> +++ b/drivers/clk/qcom/gdsc.c
> @@ -368,6 +368,16 @@ static int _gdsc_disable(struct gdsc *sc)
> if (sc->pwrsts & PWRSTS_OFF)
> gdsc_clear_mem_on(sc);
>
> + /*
> + * If the GDSC supports only a Retention state, apart from ON,
> + * leave it in ON state.
> + * There is no SW control to transition the GDSC into
> + * Retention state. This happens in HW when the parent
> + * domain goes down to a Low power state
> + */
> + if (sc->pwrsts == PWRSTS_RET_ON)
> + return 0;
> +
> ret = gdsc_toggle_logic(sc, GDSC_OFF);
> if (ret)
> return ret;
> diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
> index 5de48c9439b2..981a12c8502d 100644
> --- a/drivers/clk/qcom/gdsc.h
> +++ b/drivers/clk/qcom/gdsc.h
> @@ -49,6 +49,11 @@ struct gdsc {
> const u8 pwrsts;
> /* Powerdomain allowable state bitfields */
> #define PWRSTS_OFF BIT(0)
> +/*
> + * There is no SW control to transition a GDSC into
> + * PWRSTS_RET. This happens in HW when the parent
> + * domain goes down to a low power state
> + */
> #define PWRSTS_RET BIT(1)
> #define PWRSTS_ON BIT(2)
> #define PWRSTS_OFF_ON (PWRSTS_OFF | PWRSTS_ON)
Adding AGDR's new email to CC.
Konrad
On Fri, Sep 16, 2022 at 03:54:16PM +0530, Rajendra Nayak wrote:
> The USB controller on sc7180 does not retain the state when
> the system goes into low power state and the GDSC is
> turned off. This results in the controller reinitializing and
> re-enumerating all the connected devices (resulting in additional
> delay while coming out of suspend)
> Fix this by updating the .pwrsts for the USB GDSC so it only
> transitions to retention state in low power.
>
> Signed-off-by: Rajendra Nayak <[email protected]>
> Reviewed-by: Matthias Kaehlcke <[email protected]>
> Tested-by: Matthias Kaehlcke <[email protected]>
> ---
> v2:
> Updated the changelog
>
> drivers/clk/qcom/gcc-sc7180.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/qcom/gcc-sc7180.c b/drivers/clk/qcom/gcc-sc7180.c
> index c2ea09945c47..2d3980251e78 100644
> --- a/drivers/clk/qcom/gcc-sc7180.c
> +++ b/drivers/clk/qcom/gcc-sc7180.c
> @@ -2224,7 +2224,7 @@ static struct gdsc usb30_prim_gdsc = {
> .pd = {
> .name = "usb30_prim_gdsc",
> },
> - .pwrsts = PWRSTS_OFF_ON,
> + .pwrsts = PWRSTS_RET_ON,
> };
>
> static struct gdsc hlos1_vote_mmnoc_mmu_tbu_hf0_gdsc = {
It seems like the above will not work unless you also provide the
registers offsets that gdsc_force_mem_on() expects.
Specifically, unless you set cxc_count for the GDSC, the above call is a
no-op and the retention setting (i.e. the RETAIN_MEM and RETAIN_PERIPH
bits) will not be updated when registering the GDSC.
Johan
On 9/19/2022 9:15 PM, Johan Hovold wrote:
> On Fri, Sep 16, 2022 at 03:54:16PM +0530, Rajendra Nayak wrote:
>> The USB controller on sc7180 does not retain the state when
>> the system goes into low power state and the GDSC is
>> turned off. This results in the controller reinitializing and
>> re-enumerating all the connected devices (resulting in additional
>> delay while coming out of suspend)
>> Fix this by updating the .pwrsts for the USB GDSC so it only
>> transitions to retention state in low power.
>>
>> Signed-off-by: Rajendra Nayak <[email protected]>
>> Reviewed-by: Matthias Kaehlcke <[email protected]>
>> Tested-by: Matthias Kaehlcke <[email protected]>
>> ---
>> v2:
>> Updated the changelog
>>
>> drivers/clk/qcom/gcc-sc7180.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/qcom/gcc-sc7180.c b/drivers/clk/qcom/gcc-sc7180.c
>> index c2ea09945c47..2d3980251e78 100644
>> --- a/drivers/clk/qcom/gcc-sc7180.c
>> +++ b/drivers/clk/qcom/gcc-sc7180.c
>> @@ -2224,7 +2224,7 @@ static struct gdsc usb30_prim_gdsc = {
>> .pd = {
>> .name = "usb30_prim_gdsc",
>> },
>> - .pwrsts = PWRSTS_OFF_ON,
>> + .pwrsts = PWRSTS_RET_ON,
>> };
>>
>> static struct gdsc hlos1_vote_mmnoc_mmu_tbu_hf0_gdsc = {
>
> It seems like the above will not work unless you also provide the
> registers offsets that gdsc_force_mem_on() expects.
That's true, but its needed only on platforms that support complete
CX domain power collapse. sc7280 (and sc7180) does not support
that and hence we can achieve GDSC RET without any of the RETAIN_MEM/
RETAIN_PERIPH bits programmed.
I explained some of that in detail on another related thread a
while back [1]
[1] https://lore.kernel.org/all/[email protected]/
>
> Specifically, unless you set cxc_count for the GDSC, the above call is a
> no-op and the retention setting (i.e. the RETAIN_MEM and RETAIN_PERIPH
> bits) will not be updated when registering the GDSC.
>
> Johan
On 9/20/2022 11:21 AM, Johan Hovold wrote:
> On Tue, Sep 20, 2022 at 09:06:22AM +0530, Rajendra Nayak wrote:
>>
>>
>> On 9/19/2022 9:15 PM, Johan Hovold wrote:
>>> On Fri, Sep 16, 2022 at 03:54:16PM +0530, Rajendra Nayak wrote:
>>>> The USB controller on sc7180 does not retain the state when
>>>> the system goes into low power state and the GDSC is
>>>> turned off. This results in the controller reinitializing and
>>>> re-enumerating all the connected devices (resulting in additional
>>>> delay while coming out of suspend)
>>>> Fix this by updating the .pwrsts for the USB GDSC so it only
>>>> transitions to retention state in low power.
>>>>
>>>> Signed-off-by: Rajendra Nayak <[email protected]>
>>>> Reviewed-by: Matthias Kaehlcke <[email protected]>
>>>> Tested-by: Matthias Kaehlcke <[email protected]>
>>>> ---
>>>> v2:
>>>> Updated the changelog
>>>>
>>>> drivers/clk/qcom/gcc-sc7180.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/clk/qcom/gcc-sc7180.c b/drivers/clk/qcom/gcc-sc7180.c
>>>> index c2ea09945c47..2d3980251e78 100644
>>>> --- a/drivers/clk/qcom/gcc-sc7180.c
>>>> +++ b/drivers/clk/qcom/gcc-sc7180.c
>>>> @@ -2224,7 +2224,7 @@ static struct gdsc usb30_prim_gdsc = {
>>>> .pd = {
>>>> .name = "usb30_prim_gdsc",
>>>> },
>>>> - .pwrsts = PWRSTS_OFF_ON,
>>>> + .pwrsts = PWRSTS_RET_ON,
>>>> };
>>>>
>>>> static struct gdsc hlos1_vote_mmnoc_mmu_tbu_hf0_gdsc = {
>>>
>>> It seems like the above will not work unless you also provide the
>>> registers offsets that gdsc_force_mem_on() expects.
>>
>> That's true, but its needed only on platforms that support complete
>> CX domain power collapse. sc7280 (and sc7180) does not support
>> that and hence we can achieve GDSC RET without any of the RETAIN_MEM/
>> RETAIN_PERIPH bits programmed.
>> I explained some of that in detail on another related thread a
>> while back [1]
>>
>> [1] https://lore.kernel.org/all/[email protected]/
>
> Thanks for the link, that was was very informative.
>
> Could you please update the commit message of patch 1/3 so that it also
> covers these systems and explains why you don't set the RETAIN_MEM and
> RETAIN_PERIPH bits for them?
>
> As that commit message stands now, it seems that those bits must be set
> for retention to work.
Agree, I will update the commit message and re-spin, thanks.
>
>>> Specifically, unless you set cxc_count for the GDSC, the above call is a
>>> no-op and the retention setting (i.e. the RETAIN_MEM and RETAIN_PERIPH
>>> bits) will not be updated when registering the GDSC.
>
> Johan
On Tue, Sep 20, 2022 at 09:06:22AM +0530, Rajendra Nayak wrote:
>
>
> On 9/19/2022 9:15 PM, Johan Hovold wrote:
> > On Fri, Sep 16, 2022 at 03:54:16PM +0530, Rajendra Nayak wrote:
> >> The USB controller on sc7180 does not retain the state when
> >> the system goes into low power state and the GDSC is
> >> turned off. This results in the controller reinitializing and
> >> re-enumerating all the connected devices (resulting in additional
> >> delay while coming out of suspend)
> >> Fix this by updating the .pwrsts for the USB GDSC so it only
> >> transitions to retention state in low power.
> >>
> >> Signed-off-by: Rajendra Nayak <[email protected]>
> >> Reviewed-by: Matthias Kaehlcke <[email protected]>
> >> Tested-by: Matthias Kaehlcke <[email protected]>
> >> ---
> >> v2:
> >> Updated the changelog
> >>
> >> drivers/clk/qcom/gcc-sc7180.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/clk/qcom/gcc-sc7180.c b/drivers/clk/qcom/gcc-sc7180.c
> >> index c2ea09945c47..2d3980251e78 100644
> >> --- a/drivers/clk/qcom/gcc-sc7180.c
> >> +++ b/drivers/clk/qcom/gcc-sc7180.c
> >> @@ -2224,7 +2224,7 @@ static struct gdsc usb30_prim_gdsc = {
> >> .pd = {
> >> .name = "usb30_prim_gdsc",
> >> },
> >> - .pwrsts = PWRSTS_OFF_ON,
> >> + .pwrsts = PWRSTS_RET_ON,
> >> };
> >>
> >> static struct gdsc hlos1_vote_mmnoc_mmu_tbu_hf0_gdsc = {
> >
> > It seems like the above will not work unless you also provide the
> > registers offsets that gdsc_force_mem_on() expects.
>
> That's true, but its needed only on platforms that support complete
> CX domain power collapse. sc7280 (and sc7180) does not support
> that and hence we can achieve GDSC RET without any of the RETAIN_MEM/
> RETAIN_PERIPH bits programmed.
> I explained some of that in detail on another related thread a
> while back [1]
>
> [1] https://lore.kernel.org/all/[email protected]/
Thanks for the link, that was was very informative.
Could you please update the commit message of patch 1/3 so that it also
covers these systems and explains why you don't set the RETAIN_MEM and
RETAIN_PERIPH bits for them?
As that commit message stands now, it seems that those bits must be set
for retention to work.
> > Specifically, unless you set cxc_count for the GDSC, the above call is a
> > no-op and the retention setting (i.e. the RETAIN_MEM and RETAIN_PERIPH
> > bits) will not be updated when registering the GDSC.
Johan