Re: [PATCH RFC] ACPI: processor: idle: Do not propagate acpi_processor_ffh_lpi_probe() -ENODEV

Sudeep Holla
2026-04-14T12:25:58Z

On Tue, Apr 14, 2026 at 07:31:29PM +0800, lihuisong (C) wrote:
>
> On 4/14/2026 6:21 PM, Breno Leitao wrote:
> > Hello Huisong,
> >
> > On Tue, Apr 14, 2026 at 05:43:51PM +0800, lihuisong (C) wrote:
> > > But it is a real issue. Thanks for your report.
> > > I think the best way to fix your issue is that remove this verification in
> > > psci_acpi_cpu_init_idle().
> > > Because it is legal for platform to report one LPI state.
> > > This function just needs to verify the LPI states which are FFH.
> > Thank you for the prompt feedback.
> >
> > Would this approach work?
> >
> > commit 6c9d52840a4f778cc989838ba76ee51416e85de3
> > Author: Breno Leitao <leitao@debian.org>
> > Date: Tue Apr 14 03:16:08 2026 -0700
> >
> > ACPI: processor: idle: Allow platforms with only one LPI state
> > psci_acpi_cpu_init_idle() rejects platforms where power.count - 1 <= 0
> > by returning -ENODEV. However, having a single LPI state (WFI) is a
> > valid configuration. The function's purpose is to verify FFH idle states,
> > and when count is zero, there are simply no FFH states to validate —
> > this is not an error.
> > On NVIDIA Grace (aarch64) systems with PSCIv1.1, power.count is 1 for
> > all 72 CPUs, so the probe fails with -ENODEV. After commit cac173bea57d
> > ("ACPI: processor: idle: Rework the handling of
> > acpi_processor_ffh_lpi_probe()"), this failure propagates up and prevents
> > cpuidle registration entirely.
> > Change the check from (count <= 0) to (count < 0) so that platforms
> > with only WFI are accepted. The for loop naturally handles count == 0
> > by not iterating.
> > Fixes: cac173bea57d ("ACPI: processor: idle: Rework the handling of acpi_processor_ffh_lpi_probe()")
> > Signed-off-by: Breno Leitao <leitao@debian.org>
> >
> > diff --git a/drivers/acpi/arm64/cpuidle.c b/drivers/acpi/arm64/cpuidle.c
> > index 801f9c4501425..7791b751042ce 100644
> > --- a/drivers/acpi/arm64/cpuidle.c
> > +++ b/drivers/acpi/arm64/cpuidle.c
> > @@ -31,7 +31,7 @@ static int psci_acpi_cpu_init_idle(unsigned int cpu)
> > return -EOPNOTSUPP;
> > count = pr->power.count - 1;
> > - if (count <= 0)
> > + if (count < 0)
> > return -ENODEV;
> > for (i = 0; i < count; i++) {
>
> This count already verified in acpi_processor_get_lpi_info.
>
> I suggest modifing it as below:
>
> -->
>
> git diff
> diff --git a/drivers/acpi/arm64/cpuidle.c b/drivers/acpi/arm64/cpuidle.c
> index 801f9c450142..c68a5db8ebba 100644
> --- a/drivers/acpi/arm64/cpuidle.c
> +++ b/drivers/acpi/arm64/cpuidle.c
> @@ -16,7 +16,7 @@
>
>  static int psci_acpi_cpu_init_idle(unsigned int cpu)
>  {
> -       int i, count;
> +       int i;
>         struct acpi_lpi_state *lpi;
>         struct acpi_processor *pr = per_cpu(processors, cpu);
>
> @@ -30,14 +30,10 @@ static int psci_acpi_cpu_init_idle(unsigned int cpu)
>         if (!psci_ops.cpu_suspend)
>                 return -EOPNOTSUPP;
>
> -       count = pr->power.count - 1;
> -       if (count <= 0)
> -               return -ENODEV;
> -

It was intentionally designed this way, as there is little value in defining
only WFI in the _LPI tables. In the absence of a cpuidle driver/LPI entry,
arch_cpu_idle() is invoked, which is sufficient and avoids unnecessary
complexity, only to ultimately execute wfi() anyway.

So while I understand that the kernel did not report an error previously, that
does not mean the _LPI table is merely moot on this platform when it contains
only a WFI state.

--
Regards,
Sudeep