* [PATCH 0/2] clk: gcc-sc8280xp: fix broken suspend @ 2022-08-05 12:12 Johan Hovold 2022-08-05 12:12 ` [PATCH 1/2] clk: gcc-sc8280xp: keep PCIe power-domains always-on Johan Hovold ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Johan Hovold @ 2022-08-05 12:12 UTC (permalink / raw) To: Bjorn Andersson Cc: Manivannan Sadhasivam, Andy Gross, Konrad Dybcio, Michael Turquette, Stephen Boyd, linux-arm-msm, linux-clk, linux-kernel, Johan Hovold The Qualcomm PCIe driver currently does not implement suspend at all so we need to mark the GDSCs as always-on to prevent genpd from disabling them. Similarly, the Qualcomm dwc3 USB suspend implementation is also incomplete and the controller doesn't currently survive a suspend cycle unless the GDSC is kept on. Note that this has nothing to with whether wakeup is enabled or not (cf. [1]). With these two workarounds, we have somewhat functional suspend on the SC8280XP reference design and Lenovo Thinkpad X13s until the missing driver support is in place (even USB remote wakeup works with [2] applied). Johan [1] https://lore.kernel.org/all/20220804151001.23612-3-johan+linaro@kernel.org/ [2] https://lore.kernel.org/all/20220804151001.23612-1-johan+linaro@kernel.org/ Johan Hovold (2): clk: gcc-sc8280xp: keep PCIe power-domains always-on clk: gcc-sc8280xp: keep USB power-domains always-on drivers/clk/qcom/gcc-sc8280xp.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) -- 2.35.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] clk: gcc-sc8280xp: keep PCIe power-domains always-on 2022-08-05 12:12 [PATCH 0/2] clk: gcc-sc8280xp: fix broken suspend Johan Hovold @ 2022-08-05 12:12 ` Johan Hovold 2022-08-06 15:10 ` Manivannan Sadhasivam 2022-08-05 12:12 ` [PATCH 2/2] clk: gcc-sc8280xp: keep USB " Johan Hovold 2022-08-23 2:15 ` [PATCH 0/2] clk: gcc-sc8280xp: fix broken suspend Stephen Boyd 2 siblings, 1 reply; 7+ messages in thread From: Johan Hovold @ 2022-08-05 12:12 UTC (permalink / raw) To: Bjorn Andersson Cc: Manivannan Sadhasivam, Andy Gross, Konrad Dybcio, Michael Turquette, Stephen Boyd, linux-arm-msm, linux-clk, linux-kernel, Johan Hovold The Qualcomm PCIe driver does not yet implement suspend so to keep the PCIe power domains always-on for now to avoid crashing during resume. Signed-off-by: Johan Hovold <johan+linaro@kernel.org> --- drivers/clk/qcom/gcc-sc8280xp.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/clk/qcom/gcc-sc8280xp.c b/drivers/clk/qcom/gcc-sc8280xp.c index a2f3ffcc5849..eaeada42e13a 100644 --- a/drivers/clk/qcom/gcc-sc8280xp.c +++ b/drivers/clk/qcom/gcc-sc8280xp.c @@ -6768,6 +6768,10 @@ static struct gdsc pcie_1_tunnel_gdsc = { .flags = VOTABLE, }; +/* + * The Qualcomm PCIe driver does not yet implement suspend so to keep the + * PCIe power domains always-on for now. + */ static struct gdsc pcie_2a_gdsc = { .gdscr = 0x9d004, .collapse_ctrl = 0x52128, @@ -6776,7 +6780,7 @@ static struct gdsc pcie_2a_gdsc = { .name = "pcie_2a_gdsc", }, .pwrsts = PWRSTS_OFF_ON, - .flags = VOTABLE, + .flags = VOTABLE | ALWAYS_ON, }; static struct gdsc pcie_2b_gdsc = { @@ -6787,7 +6791,7 @@ static struct gdsc pcie_2b_gdsc = { .name = "pcie_2b_gdsc", }, .pwrsts = PWRSTS_OFF_ON, - .flags = VOTABLE, + .flags = VOTABLE | ALWAYS_ON, }; static struct gdsc pcie_3a_gdsc = { @@ -6798,7 +6802,7 @@ static struct gdsc pcie_3a_gdsc = { .name = "pcie_3a_gdsc", }, .pwrsts = PWRSTS_OFF_ON, - .flags = VOTABLE, + .flags = VOTABLE | ALWAYS_ON, }; static struct gdsc pcie_3b_gdsc = { @@ -6809,7 +6813,7 @@ static struct gdsc pcie_3b_gdsc = { .name = "pcie_3b_gdsc", }, .pwrsts = PWRSTS_OFF_ON, - .flags = VOTABLE, + .flags = VOTABLE | ALWAYS_ON, }; static struct gdsc pcie_4_gdsc = { @@ -6820,7 +6824,7 @@ static struct gdsc pcie_4_gdsc = { .name = "pcie_4_gdsc", }, .pwrsts = PWRSTS_OFF_ON, - .flags = VOTABLE, + .flags = VOTABLE | ALWAYS_ON, }; static struct gdsc ufs_card_gdsc = { -- 2.35.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] clk: gcc-sc8280xp: keep PCIe power-domains always-on 2022-08-05 12:12 ` [PATCH 1/2] clk: gcc-sc8280xp: keep PCIe power-domains always-on Johan Hovold @ 2022-08-06 15:10 ` Manivannan Sadhasivam 0 siblings, 0 replies; 7+ messages in thread From: Manivannan Sadhasivam @ 2022-08-06 15:10 UTC (permalink / raw) To: Johan Hovold Cc: Bjorn Andersson, Andy Gross, Konrad Dybcio, Michael Turquette, Stephen Boyd, linux-arm-msm, linux-clk, linux-kernel On Fri, Aug 05, 2022 at 02:12:49PM +0200, Johan Hovold wrote: > The Qualcomm PCIe driver does not yet implement suspend so to keep the > PCIe power domains always-on for now to avoid crashing during resume. > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Thanks, Mani > --- > drivers/clk/qcom/gcc-sc8280xp.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/drivers/clk/qcom/gcc-sc8280xp.c b/drivers/clk/qcom/gcc-sc8280xp.c > index a2f3ffcc5849..eaeada42e13a 100644 > --- a/drivers/clk/qcom/gcc-sc8280xp.c > +++ b/drivers/clk/qcom/gcc-sc8280xp.c > @@ -6768,6 +6768,10 @@ static struct gdsc pcie_1_tunnel_gdsc = { > .flags = VOTABLE, > }; > > +/* > + * The Qualcomm PCIe driver does not yet implement suspend so to keep the > + * PCIe power domains always-on for now. > + */ > static struct gdsc pcie_2a_gdsc = { > .gdscr = 0x9d004, > .collapse_ctrl = 0x52128, > @@ -6776,7 +6780,7 @@ static struct gdsc pcie_2a_gdsc = { > .name = "pcie_2a_gdsc", > }, > .pwrsts = PWRSTS_OFF_ON, > - .flags = VOTABLE, > + .flags = VOTABLE | ALWAYS_ON, > }; > > static struct gdsc pcie_2b_gdsc = { > @@ -6787,7 +6791,7 @@ static struct gdsc pcie_2b_gdsc = { > .name = "pcie_2b_gdsc", > }, > .pwrsts = PWRSTS_OFF_ON, > - .flags = VOTABLE, > + .flags = VOTABLE | ALWAYS_ON, > }; > > static struct gdsc pcie_3a_gdsc = { > @@ -6798,7 +6802,7 @@ static struct gdsc pcie_3a_gdsc = { > .name = "pcie_3a_gdsc", > }, > .pwrsts = PWRSTS_OFF_ON, > - .flags = VOTABLE, > + .flags = VOTABLE | ALWAYS_ON, > }; > > static struct gdsc pcie_3b_gdsc = { > @@ -6809,7 +6813,7 @@ static struct gdsc pcie_3b_gdsc = { > .name = "pcie_3b_gdsc", > }, > .pwrsts = PWRSTS_OFF_ON, > - .flags = VOTABLE, > + .flags = VOTABLE | ALWAYS_ON, > }; > > static struct gdsc pcie_4_gdsc = { > @@ -6820,7 +6824,7 @@ static struct gdsc pcie_4_gdsc = { > .name = "pcie_4_gdsc", > }, > .pwrsts = PWRSTS_OFF_ON, > - .flags = VOTABLE, > + .flags = VOTABLE | ALWAYS_ON, > }; > > static struct gdsc ufs_card_gdsc = { > -- > 2.35.1 > -- மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] clk: gcc-sc8280xp: keep USB power-domains always-on 2022-08-05 12:12 [PATCH 0/2] clk: gcc-sc8280xp: fix broken suspend Johan Hovold 2022-08-05 12:12 ` [PATCH 1/2] clk: gcc-sc8280xp: keep PCIe power-domains always-on Johan Hovold @ 2022-08-05 12:12 ` Johan Hovold 2022-08-06 15:11 ` Manivannan Sadhasivam 2022-08-23 2:15 ` [PATCH 0/2] clk: gcc-sc8280xp: fix broken suspend Stephen Boyd 2 siblings, 1 reply; 7+ messages in thread From: Johan Hovold @ 2022-08-05 12:12 UTC (permalink / raw) To: Bjorn Andersson Cc: Manivannan Sadhasivam, Andy Gross, Konrad Dybcio, Michael Turquette, Stephen Boyd, linux-arm-msm, linux-clk, linux-kernel, Johan Hovold The Qualcomm DWC3 driver suspend implementation appears to be incomplete for SC8280XP so keep the USB power domains always-on for now so that the controller survives a suspend cycle. Signed-off-by: Johan Hovold <johan+linaro@kernel.org> --- drivers/clk/qcom/gcc-sc8280xp.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/clk/qcom/gcc-sc8280xp.c b/drivers/clk/qcom/gcc-sc8280xp.c index eaeada42e13a..7768e6901dcc 100644 --- a/drivers/clk/qcom/gcc-sc8280xp.c +++ b/drivers/clk/qcom/gcc-sc8280xp.c @@ -6843,12 +6843,17 @@ static struct gdsc ufs_phy_gdsc = { .pwrsts = PWRSTS_OFF_ON, }; +/* + * The Qualcomm DWC3 driver suspend implementation appears to be incomplete + * for sc8280xp so keep the USB power domains always-on for now. + */ static struct gdsc usb30_mp_gdsc = { .gdscr = 0xab004, .pd = { .name = "usb30_mp_gdsc", }, .pwrsts = PWRSTS_OFF_ON, + .flags = ALWAYS_ON, }; static struct gdsc usb30_prim_gdsc = { @@ -6857,6 +6862,7 @@ static struct gdsc usb30_prim_gdsc = { .name = "usb30_prim_gdsc", }, .pwrsts = PWRSTS_OFF_ON, + .flags = ALWAYS_ON, }; static struct gdsc usb30_sec_gdsc = { @@ -6865,6 +6871,7 @@ static struct gdsc usb30_sec_gdsc = { .name = "usb30_sec_gdsc", }, .pwrsts = PWRSTS_OFF_ON, + .flags = ALWAYS_ON, }; static struct clk_regmap *gcc_sc8280xp_clocks[] = { -- 2.35.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] clk: gcc-sc8280xp: keep USB power-domains always-on 2022-08-05 12:12 ` [PATCH 2/2] clk: gcc-sc8280xp: keep USB " Johan Hovold @ 2022-08-06 15:11 ` Manivannan Sadhasivam 0 siblings, 0 replies; 7+ messages in thread From: Manivannan Sadhasivam @ 2022-08-06 15:11 UTC (permalink / raw) To: Johan Hovold Cc: Bjorn Andersson, Andy Gross, Konrad Dybcio, Michael Turquette, Stephen Boyd, linux-arm-msm, linux-clk, linux-kernel On Fri, Aug 05, 2022 at 02:12:50PM +0200, Johan Hovold wrote: > The Qualcomm DWC3 driver suspend implementation appears to be incomplete > for SC8280XP so keep the USB power domains always-on for now so that the > controller survives a suspend cycle. > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Thanks, Mani > --- > drivers/clk/qcom/gcc-sc8280xp.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/clk/qcom/gcc-sc8280xp.c b/drivers/clk/qcom/gcc-sc8280xp.c > index eaeada42e13a..7768e6901dcc 100644 > --- a/drivers/clk/qcom/gcc-sc8280xp.c > +++ b/drivers/clk/qcom/gcc-sc8280xp.c > @@ -6843,12 +6843,17 @@ static struct gdsc ufs_phy_gdsc = { > .pwrsts = PWRSTS_OFF_ON, > }; > > +/* > + * The Qualcomm DWC3 driver suspend implementation appears to be incomplete > + * for sc8280xp so keep the USB power domains always-on for now. > + */ > static struct gdsc usb30_mp_gdsc = { > .gdscr = 0xab004, > .pd = { > .name = "usb30_mp_gdsc", > }, > .pwrsts = PWRSTS_OFF_ON, > + .flags = ALWAYS_ON, > }; > > static struct gdsc usb30_prim_gdsc = { > @@ -6857,6 +6862,7 @@ static struct gdsc usb30_prim_gdsc = { > .name = "usb30_prim_gdsc", > }, > .pwrsts = PWRSTS_OFF_ON, > + .flags = ALWAYS_ON, > }; > > static struct gdsc usb30_sec_gdsc = { > @@ -6865,6 +6871,7 @@ static struct gdsc usb30_sec_gdsc = { > .name = "usb30_sec_gdsc", > }, > .pwrsts = PWRSTS_OFF_ON, > + .flags = ALWAYS_ON, > }; > > static struct clk_regmap *gcc_sc8280xp_clocks[] = { > -- > 2.35.1 > -- மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] clk: gcc-sc8280xp: fix broken suspend 2022-08-05 12:12 [PATCH 0/2] clk: gcc-sc8280xp: fix broken suspend Johan Hovold 2022-08-05 12:12 ` [PATCH 1/2] clk: gcc-sc8280xp: keep PCIe power-domains always-on Johan Hovold 2022-08-05 12:12 ` [PATCH 2/2] clk: gcc-sc8280xp: keep USB " Johan Hovold @ 2022-08-23 2:15 ` Stephen Boyd 2022-08-23 11:09 ` Johan Hovold 2 siblings, 1 reply; 7+ messages in thread From: Stephen Boyd @ 2022-08-23 2:15 UTC (permalink / raw) To: Bjorn Andersson, Johan Hovold Cc: Manivannan Sadhasivam, Andy Gross, Konrad Dybcio, Michael Turquette, linux-arm-msm, linux-clk, linux-kernel, Johan Hovold Quoting Johan Hovold (2022-08-05 05:12:48) > The Qualcomm PCIe driver currently does not implement suspend at all so > we need to mark the GDSCs as always-on to prevent genpd from disabling > them. > > Similarly, the Qualcomm dwc3 USB suspend implementation is also > incomplete and the controller doesn't currently survive a suspend cycle > unless the GDSC is kept on. Note that this has nothing to with whether > wakeup is enabled or not (cf. [1]). > > With these two workarounds, we have somewhat functional suspend on the > SC8280XP reference design and Lenovo Thinkpad X13s until the missing > driver support is in place (even USB remote wakeup works with [2] > applied). Are these urgently needed for this release or does suspend not really work yet on sc8280xp? I'm trying to understand if we need to apply these to the fixes tree (in which case why isn't there a Fixes tag targetting whatever commit broke suspend) or if they can wait for the next release and come through clk-next. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] clk: gcc-sc8280xp: fix broken suspend 2022-08-23 2:15 ` [PATCH 0/2] clk: gcc-sc8280xp: fix broken suspend Stephen Boyd @ 2022-08-23 11:09 ` Johan Hovold 0 siblings, 0 replies; 7+ messages in thread From: Johan Hovold @ 2022-08-23 11:09 UTC (permalink / raw) To: Stephen Boyd Cc: Bjorn Andersson, Johan Hovold, Manivannan Sadhasivam, Andy Gross, Konrad Dybcio, Michael Turquette, linux-arm-msm, linux-clk, linux-kernel On Mon, Aug 22, 2022 at 07:15:25PM -0700, Stephen Boyd wrote: > Quoting Johan Hovold (2022-08-05 05:12:48) > > The Qualcomm PCIe driver currently does not implement suspend at all so > > we need to mark the GDSCs as always-on to prevent genpd from disabling > > them. > > > > Similarly, the Qualcomm dwc3 USB suspend implementation is also > > incomplete and the controller doesn't currently survive a suspend cycle > > unless the GDSC is kept on. Note that this has nothing to with whether > > wakeup is enabled or not (cf. [1]). > > > > With these two workarounds, we have somewhat functional suspend on the > > SC8280XP reference design and Lenovo Thinkpad X13s until the missing > > driver support is in place (even USB remote wakeup works with [2] > > applied). > > Are these urgently needed for this release or does suspend not really > work yet on sc8280xp? I'm trying to understand if we need to apply > these to the fixes tree (in which case why isn't there a Fixes tag > targetting whatever commit broke suspend) or if they can wait for the > next release and come through clk-next. They are urgently needed as sc8280xp does not survive a suspend cycle without them (e.g. crashes on resume) and this is not something that users expect when closing the lid of their laptops. This is not a regression, but I guess I could have pointed to the commit adding the sc8280xp clock driver and GDSC power-domain definitions: Fixes: d65d005f9a6c ("clk: qcom: add sc8280xp GCC driver") As several Qualcomm drivers simply ignore system PM and, for example, leave clocks enabled during suspend, we also need to keep the power-domains enabled. I noticed that Bjorn applied these for 6.1, but I believe this is 6.0 material (or at the least the USB one is as PCI support for sc8280xp is being added in 6.1). Johan ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-08-23 15:29 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-08-05 12:12 [PATCH 0/2] clk: gcc-sc8280xp: fix broken suspend Johan Hovold 2022-08-05 12:12 ` [PATCH 1/2] clk: gcc-sc8280xp: keep PCIe power-domains always-on Johan Hovold 2022-08-06 15:10 ` Manivannan Sadhasivam 2022-08-05 12:12 ` [PATCH 2/2] clk: gcc-sc8280xp: keep USB " Johan Hovold 2022-08-06 15:11 ` Manivannan Sadhasivam 2022-08-23 2:15 ` [PATCH 0/2] clk: gcc-sc8280xp: fix broken suspend Stephen Boyd 2022-08-23 11:09 ` Johan Hovold
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).