Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 23 Aug 2022 13:09:33 +0200
From:   Johan Hovold <johan@kernel.org>
To:     Stephen Boyd <sboyd@kernel.org>
Cc:     Bjorn Andersson <bjorn.andersson@linaro.org>,
        Johan Hovold <johan+linaro@kernel.org>,
        Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
        Andy Gross <agross@kernel.org>,
        Konrad Dybcio <konrad.dybcio@somainline.org>,
        Michael Turquette <mturquette@baylibre.com>,
        linux-arm-msm@vger.kernel.org, linux-clk@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/2] clk: gcc-sc8280xp: fix broken suspend
Message-ID: <YwS1bWufBYE6buSy@hovoldconsulting.com>
References: <20220805121250.10347-1-johan+linaro@kernel.org>
 <20220823021528.7291FC433D6@smtp.kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220823021528.7291FC433D6@smtp.kernel.org>
Precedence: bulk

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