[PATCH] arm64: dts: qcom: sm8250: Disable the not yet supported cluster idle state

[PATCH] arm64: dts: qcom: sm8250: Disable the not yet supported cluster idle state

[Ulf Hansson]

To support the deeper cluster idle state for sm8250 platforms, some
additional synchronization is needed between the rpmh-rsc device and the
CPU cluster PM domain. Until that is supported, let's disable the cluster
idle state.

This fixes a problem that has been reported for the Qcom RB5 platform (see
below), but most likely other sm8250 platforms suffers from similar issues,
so let's make the fix generic for sm8250.

vreg_l11c_3p3: failed to enable: -ETIMEDOUT
qcom-rpmh-regulator 18200000.rsc:pm8150l-rpmh-regulators: ldo11: devm_regulator_register() failed, ret=-110
qcom-rpmh-regulator: probe of 18200000.rsc:pm8150l-rpmh-regulators failed with error -110

Reported-by: Amit Pundir <amit.pundir@linaro.org>
Fixes: 32bc936d7321 ("arm64: dts: qcom: sm8250: Add cpuidle states")
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

This problem was reported by Amit [1] together with an attempt to fix it. It
turned, that we wanted a more generic fix, hence I posted $subject patch.

Also note that, $subject patch is also depending (from functionality point of
view) on a another for genpd [2]. Although, that should soon reach v6.1-rc[n] and
stable kernels. 

Bjorn, can you pick this up as a fix for v6.1-rc and tag it for stable kernels?

Kind regards
Ulf Hansson

[1]
https://lore.kernel.org/lkml/20221018145348.4051809-1-amit.pundir@linaro.org/
[2]
https://lore.kernel.org/lkml/CAJZ5v0hJxRiL03XDFpU8FuabsHn5i6JMksJ=dfj8B+Dm=s3LYA@mail.gmail.com/

---
 arch/arm64/boot/dts/qcom/sm8250.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index a5b62cadb129..e276eed1f8e2 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -334,6 +334,7 @@ CLUSTER_SLEEP_0: cluster-sleep-0 {
 				exit-latency-us = <6562>;
 				min-residency-us = <9987>;
 				local-timer-stop;
+				status = "disabled";
 			};
 		};
 	};
-- 
2.34.1

Re: [PATCH] arm64: dts: qcom: sm8250: Disable the not yet supported cluster idle state

[Sudeep Holla]

On Thu, Oct 27, 2022 at 01:57:45PM +0200, Ulf Hansson wrote:
> To support the deeper cluster idle state for sm8250 platforms, some
> additional synchronization is needed between the rpmh-rsc device and the
> CPU cluster PM domain. Until that is supported, let's disable the cluster
> idle state.
> 
> This fixes a problem that has been reported for the Qcom RB5 platform (see
> below), but most likely other sm8250 platforms suffers from similar issues,
> so let's make the fix generic for sm8250.
> 
> vreg_l11c_3p3: failed to enable: -ETIMEDOUT
> qcom-rpmh-regulator 18200000.rsc:pm8150l-rpmh-regulators: ldo11: devm_regulator_register() failed, ret=-110
> qcom-rpmh-regulator: probe of 18200000.rsc:pm8150l-rpmh-regulators failed with error -110
> 
> Reported-by: Amit Pundir <amit.pundir@linaro.org>
> Fixes: 32bc936d7321 ("arm64: dts: qcom: sm8250: Add cpuidle states")

Thanks for the patience and fixing it this way :). I take it is only the
cluster states that has the issue because [1] only disables the CPU level
states which was confusing and I had asked the same.

Anyways for this,

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

-- 
Regards,
Sudeep

Re: [PATCH] arm64: dts: qcom: sm8250: Disable the not yet supported cluster idle state

[Amit Pundir]

On Thu, 27 Oct 2022 at 17:27, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> To support the deeper cluster idle state for sm8250 platforms, some
> additional synchronization is needed between the rpmh-rsc device and the
> CPU cluster PM domain. Until that is supported, let's disable the cluster
> idle state.
>
> This fixes a problem that has been reported for the Qcom RB5 platform (see
> below), but most likely other sm8250 platforms suffers from similar issues,
> so let's make the fix generic for sm8250.
>
> vreg_l11c_3p3: failed to enable: -ETIMEDOUT
> qcom-rpmh-regulator 18200000.rsc:pm8150l-rpmh-regulators: ldo11: devm_regulator_register() failed, ret=-110
> qcom-rpmh-regulator: probe of 18200000.rsc:pm8150l-rpmh-regulators failed with error -110
>
> Reported-by: Amit Pundir <amit.pundir@linaro.org>
> Fixes: 32bc936d7321 ("arm64: dts: qcom: sm8250: Add cpuidle states")
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>
> This problem was reported by Amit [1] together with an attempt to fix it. It
> turned, that we wanted a more generic fix, hence I posted $subject patch.

Thanks Ulf and Sudeep. This patch along with [2], fixes the above
mentioned crash on RB5 running AOSP.

Tested-by: Amit Pundir <amit.pundir@linaro.org>

Regards,
Amit Pundir

>
> Also note that, $subject patch is also depending (from functionality point of
> view) on a another for genpd [2]. Although, that should soon reach v6.1-rc[n] and
> stable kernels.
>
> Bjorn, can you pick this up as a fix for v6.1-rc and tag it for stable kernels?
>
> Kind regards
> Ulf Hansson
>
> [1]
> https://lore.kernel.org/lkml/20221018145348.4051809-1-amit.pundir@linaro.org/
> [2]
> https://lore.kernel.org/lkml/CAJZ5v0hJxRiL03XDFpU8FuabsHn5i6JMksJ=dfj8B+Dm=s3LYA@mail.gmail.com/
>
> ---
>  arch/arm64/boot/dts/qcom/sm8250.dtsi | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index a5b62cadb129..e276eed1f8e2 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -334,6 +334,7 @@ CLUSTER_SLEEP_0: cluster-sleep-0 {
>                                 exit-latency-us = <6562>;
>                                 min-residency-us = <9987>;
>                                 local-timer-stop;
> +                               status = "disabled";
>                         };
>                 };
>         };
> --
> 2.34.1
>

Re: [PATCH] arm64: dts: qcom: sm8250: Disable the not yet supported cluster idle state

[Bjorn Andersson]

On Thu, 27 Oct 2022 13:57:45 +0200, Ulf Hansson wrote:
> To support the deeper cluster idle state for sm8250 platforms, some
> additional synchronization is needed between the rpmh-rsc device and the
> CPU cluster PM domain. Until that is supported, let's disable the cluster
> idle state.
> 
> This fixes a problem that has been reported for the Qcom RB5 platform (see
> below), but most likely other sm8250 platforms suffers from similar issues,
> so let's make the fix generic for sm8250.
> 
> [...]

Applied, thanks!

[1/1] arm64: dts: qcom: sm8250: Disable the not yet supported cluster idle state
      commit: 5c7fa5de12a31c1425cc87ba2ef27e6ae0a1788c

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>

Re: [PATCH] arm64: dts: qcom: sm8250: Disable the not yet supported cluster idle state

[Amit Pundir]

On Mon, 7 Nov 2022 at 08:43, Bjorn Andersson <andersson@kernel.org> wrote:
>
> On Thu, 27 Oct 2022 13:57:45 +0200, Ulf Hansson wrote:
> > To support the deeper cluster idle state for sm8250 platforms, some
> > additional synchronization is needed between the rpmh-rsc device and the
> > CPU cluster PM domain. Until that is supported, let's disable the cluster
> > idle state.
> >
> > This fixes a problem that has been reported for the Qcom RB5 platform (see
> > below), but most likely other sm8250 platforms suffers from similar issues,
> > so let's make the fix generic for sm8250.
> >
> > [...]
>
> Applied, thanks!
>
> [1/1] arm64: dts: qcom: sm8250: Disable the not yet supported cluster idle state
>       commit: 5c7fa5de12a31c1425cc87ba2ef27e6ae0a1788c

Hi Bjorn,

There seem to be some error while applying the patch "arm64: dts:
qcom: sm8250: Disable the not yet supported cluster idle state".
This patch is already applied in your arm64-fixes-for-6.1 tree
(commit: cadaa773bcf161184fa428180516bae33a7bc667)

The new commit: 5c7fa5de12a31c1425cc87ba2ef27e6ae0a1788c,
however, disables the Big cpu idle state instead. I'm not sure if that
is intentional though.

Regards,
Amit Pundir


>
> Best regards,
> --
> Bjorn Andersson <andersson@kernel.org>

Re: [PATCH] arm64: dts: qcom: sm8250: Disable the not yet supported cluster idle state

[Ulf Hansson]

On Mon, 7 Nov 2022 at 06:55, Amit Pundir <amit.pundir@linaro.org> wrote:
>
> On Mon, 7 Nov 2022 at 08:43, Bjorn Andersson <andersson@kernel.org> wrote:
> >
> > On Thu, 27 Oct 2022 13:57:45 +0200, Ulf Hansson wrote:
> > > To support the deeper cluster idle state for sm8250 platforms, some
> > > additional synchronization is needed between the rpmh-rsc device and the
> > > CPU cluster PM domain. Until that is supported, let's disable the cluster
> > > idle state.
> > >
> > > This fixes a problem that has been reported for the Qcom RB5 platform (see
> > > below), but most likely other sm8250 platforms suffers from similar issues,
> > > so let's make the fix generic for sm8250.
> > >
> > > [...]
> >
> > Applied, thanks!
> >
> > [1/1] arm64: dts: qcom: sm8250: Disable the not yet supported cluster idle state
> >       commit: 5c7fa5de12a31c1425cc87ba2ef27e6ae0a1788c
>
> Hi Bjorn,
>
> There seem to be some error while applying the patch "arm64: dts:
> qcom: sm8250: Disable the not yet supported cluster idle state".
> This patch is already applied in your arm64-fixes-for-6.1 tree
> (commit: cadaa773bcf161184fa428180516bae33a7bc667)
>
> The new commit: 5c7fa5de12a31c1425cc87ba2ef27e6ae0a1788c,
> however, disables the Big cpu idle state instead. I'm not sure if that
> is intentional though.

It's a mistake. There have been a lot of various patches/discussions
around this issue at LKML, not entirely easy to follow.

Anyway to make it clear, we shouldn't need to disable the
BIG_CPU_SLEEP_0 state, but only the CLUSTER_SLEEP_0.

Bjorn, can you please have a look and drop/revert commit
5c7fa5de12a31c1425cc87ba2ef27e6ae0a1788c ?

Kind regards
Uffe