mbox series

[0/2] clk: gcc-sc8280xp: fix broken suspend

Message ID 20220805121250.10347-1-johan+linaro@kernel.org
Headers show
Series clk: gcc-sc8280xp: fix broken suspend | expand

Message

Johan Hovold Aug. 5, 2022, 12:12 p.m. UTC
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(-)

Comments

Stephen Boyd Aug. 23, 2022, 2:15 a.m. UTC | #1
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.
Johan Hovold Aug. 23, 2022, 11:09 a.m. UTC | #2
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