| Message ID | 20230706140842.18059-1-manivannan.sadhasivam@linaro.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v2] clk: qcom: gcc-sc8280xp: Allow PCIe GDSCs to enter retention state | expand |
On Thu, Jul 06, 2023 at 07:38:42PM +0530, Manivannan Sadhasivam wrote: > With the minimal system suspend support in place for the PCIe driver that > keeps the interconnect path voted, the ALWAYS_ON flag can now be dropped. > > Also, the PWRSTS_RET_ON flag should be used to allow the GDSCs to enter the > retention state when the parent domain get's turned off during system > suspend. > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > > Changes in v2: > > * Changed the patch from simple revert to changing the ALWAYS_ON flag to > PWRSTS_RET_ON. > > drivers/clk/qcom/gcc-sc8280xp.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/clk/qcom/gcc-sc8280xp.c b/drivers/clk/qcom/gcc-sc8280xp.c > index 04a99dbaa57e..c59b0f91c87d 100644 > --- a/drivers/clk/qcom/gcc-sc8280xp.c > +++ b/drivers/clk/qcom/gcc-sc8280xp.c > @@ -6786,7 +6786,7 @@ static struct gdsc pcie_2a_gdsc = { > .name = "pcie_2a_gdsc", > }, > .pwrsts = PWRSTS_OFF_ON, > - .flags = VOTABLE | ALWAYS_ON, > + .flags = VOTABLE | PWRSTS_RET_ON, > }; This is not correct either. PWRSTS_RET_ON is a pwrsts mask... Johan
On Thu, Jul 06, 2023 at 04:17:06PM +0200, Johan Hovold wrote: > On Thu, Jul 06, 2023 at 07:38:42PM +0530, Manivannan Sadhasivam wrote: > > With the minimal system suspend support in place for the PCIe driver that > > keeps the interconnect path voted, the ALWAYS_ON flag can now be dropped. > > > > Also, the PWRSTS_RET_ON flag should be used to allow the GDSCs to enter the > > retention state when the parent domain get's turned off during system > > suspend. > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > --- > > > > Changes in v2: > > > > * Changed the patch from simple revert to changing the ALWAYS_ON flag to > > PWRSTS_RET_ON. > > > > drivers/clk/qcom/gcc-sc8280xp.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/clk/qcom/gcc-sc8280xp.c b/drivers/clk/qcom/gcc-sc8280xp.c > > index 04a99dbaa57e..c59b0f91c87d 100644 > > --- a/drivers/clk/qcom/gcc-sc8280xp.c > > +++ b/drivers/clk/qcom/gcc-sc8280xp.c > > @@ -6786,7 +6786,7 @@ static struct gdsc pcie_2a_gdsc = { > > .name = "pcie_2a_gdsc", > > }, > > .pwrsts = PWRSTS_OFF_ON, > > - .flags = VOTABLE | ALWAYS_ON, > > + .flags = VOTABLE | PWRSTS_RET_ON, > > }; > > This is not correct either. PWRSTS_RET_ON is a pwrsts mask... > I should stop sending patches while feeling sleepy... - Mani > Johan
On 6.07.2023 16:17, Johan Hovold wrote: > On Thu, Jul 06, 2023 at 07:38:42PM +0530, Manivannan Sadhasivam wrote: >> With the minimal system suspend support in place for the PCIe driver that >> keeps the interconnect path voted, the ALWAYS_ON flag can now be dropped. >> >> Also, the PWRSTS_RET_ON flag should be used to allow the GDSCs to enter the >> retention state when the parent domain get's turned off during system >> suspend. >> >> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> >> --- >> >> Changes in v2: >> >> * Changed the patch from simple revert to changing the ALWAYS_ON flag to >> PWRSTS_RET_ON. >> >> drivers/clk/qcom/gcc-sc8280xp.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/clk/qcom/gcc-sc8280xp.c b/drivers/clk/qcom/gcc-sc8280xp.c >> index 04a99dbaa57e..c59b0f91c87d 100644 >> --- a/drivers/clk/qcom/gcc-sc8280xp.c >> +++ b/drivers/clk/qcom/gcc-sc8280xp.c >> @@ -6786,7 +6786,7 @@ static struct gdsc pcie_2a_gdsc = { >> .name = "pcie_2a_gdsc", >> }, >> .pwrsts = PWRSTS_OFF_ON, >> - .flags = VOTABLE | ALWAYS_ON, >> + .flags = VOTABLE | PWRSTS_RET_ON, >> }; > > This is not correct either. PWRSTS_RET_ON is a pwrsts mask... This + please rebase atop that: https://lore.kernel.org/linux-arm-msm/20230620-topic-sc8280_gccgdsc-v2-0-562c1428c10d@linaro.org/ Konrad > > Johan
On 6.07.2023 16:40, Konrad Dybcio wrote: > On 6.07.2023 16:17, Johan Hovold wrote: >> On Thu, Jul 06, 2023 at 07:38:42PM +0530, Manivannan Sadhasivam wrote: >>> With the minimal system suspend support in place for the PCIe driver that >>> keeps the interconnect path voted, the ALWAYS_ON flag can now be dropped. >>> >>> Also, the PWRSTS_RET_ON flag should be used to allow the GDSCs to enter the >>> retention state when the parent domain get's turned off during system >>> suspend. >>> >>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> >>> --- >>> >>> Changes in v2: >>> >>> * Changed the patch from simple revert to changing the ALWAYS_ON flag to >>> PWRSTS_RET_ON. >>> >>> drivers/clk/qcom/gcc-sc8280xp.c | 10 +++++----- >>> 1 file changed, 5 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/clk/qcom/gcc-sc8280xp.c b/drivers/clk/qcom/gcc-sc8280xp.c >>> index 04a99dbaa57e..c59b0f91c87d 100644 >>> --- a/drivers/clk/qcom/gcc-sc8280xp.c >>> +++ b/drivers/clk/qcom/gcc-sc8280xp.c >>> @@ -6786,7 +6786,7 @@ static struct gdsc pcie_2a_gdsc = { >>> .name = "pcie_2a_gdsc", >>> }, >>> .pwrsts = PWRSTS_OFF_ON, >>> - .flags = VOTABLE | ALWAYS_ON, >>> + .flags = VOTABLE | PWRSTS_RET_ON, >>> }; >> >> This is not correct either. PWRSTS_RET_ON is a pwrsts mask... > This + please rebase atop that: > > https://lore.kernel.org/linux-arm-msm/20230620-topic-sc8280_gccgdsc-v2-0-562c1428c10d@linaro.org/ Moreover, I tried a similar patch out. Johan's v6.4 branch + the patchset I linked + removing ALWAYS_ON from gcc (so keeping PWRSTS_OFF_ON) and PCIe survives a wakeup-s2idle-wakeup cycle. I am however running with clk_ignore_unused, so I don't know how this impacts the RPMh system suspend situation. Konrad > > Konrad >> >> Johan
diff --git a/drivers/clk/qcom/gcc-sc8280xp.c b/drivers/clk/qcom/gcc-sc8280xp.c index 04a99dbaa57e..c59b0f91c87d 100644 --- a/drivers/clk/qcom/gcc-sc8280xp.c +++ b/drivers/clk/qcom/gcc-sc8280xp.c @@ -6786,7 +6786,7 @@ static struct gdsc pcie_2a_gdsc = { .name = "pcie_2a_gdsc", }, .pwrsts = PWRSTS_OFF_ON, - .flags = VOTABLE | ALWAYS_ON, + .flags = VOTABLE | PWRSTS_RET_ON, }; static struct gdsc pcie_2b_gdsc = { @@ -6797,7 +6797,7 @@ static struct gdsc pcie_2b_gdsc = { .name = "pcie_2b_gdsc", }, .pwrsts = PWRSTS_OFF_ON, - .flags = VOTABLE | ALWAYS_ON, + .flags = VOTABLE | PWRSTS_RET_ON, }; static struct gdsc pcie_3a_gdsc = { @@ -6808,7 +6808,7 @@ static struct gdsc pcie_3a_gdsc = { .name = "pcie_3a_gdsc", }, .pwrsts = PWRSTS_OFF_ON, - .flags = VOTABLE | ALWAYS_ON, + .flags = VOTABLE | PWRSTS_RET_ON, }; static struct gdsc pcie_3b_gdsc = { @@ -6819,7 +6819,7 @@ static struct gdsc pcie_3b_gdsc = { .name = "pcie_3b_gdsc", }, .pwrsts = PWRSTS_OFF_ON, - .flags = VOTABLE | ALWAYS_ON, + .flags = VOTABLE | PWRSTS_RET_ON, }; static struct gdsc pcie_4_gdsc = { @@ -6830,7 +6830,7 @@ static struct gdsc pcie_4_gdsc = { .name = "pcie_4_gdsc", }, .pwrsts = PWRSTS_OFF_ON, - .flags = VOTABLE | ALWAYS_ON, + .flags = VOTABLE | PWRSTS_RET_ON, }; static struct gdsc ufs_card_gdsc = {
With the minimal system suspend support in place for the PCIe driver that keeps the interconnect path voted, the ALWAYS_ON flag can now be dropped. Also, the PWRSTS_RET_ON flag should be used to allow the GDSCs to enter the retention state when the parent domain get's turned off during system suspend. Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> --- Changes in v2: * Changed the patch from simple revert to changing the ALWAYS_ON flag to PWRSTS_RET_ON. drivers/clk/qcom/gcc-sc8280xp.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)