mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/3] ACPI: processor: idle: Enhance LPI verification and
@ 2025-11-25  6:52 Huisong Li
  2025-11-25  6:52 ` [PATCH 1/3] ACPI: processor: idle: Relocate and verify acpi_processor_ffh_lpi_probe Huisong Li
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Huisong Li @ 2025-11-25  6:52 UTC (permalink / raw)
  To: rafael, lenb
  Cc: linux-acpi, linux-kernel, Sudeep.Holla, linuxarm,
	jonathan.cameron, zhanjie9, zhenglifeng1, yubowen8, lihuisong

This series is aimed to let LPI verification effective and redefine
two functions to void, which is a part of the series in link [1].

[1] https://lore.kernel.org/all/20251103084244.2654432-1-lihuisong@huawei.com

Huisong Li (3):
  ACPI: processor: idle: Relocate and verify
    acpi_processor_ffh_lpi_probe
  ACPI: processor: idle: Redefine acpi_processor_setup_cpuidle_dev to
    void
  ACPI: processor: idle: Redefine acpi_processor_setup_cpuidle_states to
    void

 drivers/acpi/processor_idle.c | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

-- 
2.33.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] ACPI: processor: idle: Relocate and verify acpi_processor_ffh_lpi_probe
  2025-11-25  6:52 [PATCH 0/3] ACPI: processor: idle: Enhance LPI verification and Huisong Li
@ 2025-11-25  6:52 ` Huisong Li
  2026-01-14 17:27   ` Rafael J. Wysocki
  2025-11-25  6:52 ` [PATCH 2/3] ACPI: processor: idle: Redefine acpi_processor_setup_cpuidle_dev to void Huisong Li
  2025-11-25  6:52 ` [PATCH 3/3] ACPI: processor: idle: Redefine acpi_processor_setup_cpuidle_states " Huisong Li
  2 siblings, 1 reply; 9+ messages in thread
From: Huisong Li @ 2025-11-25  6:52 UTC (permalink / raw)
  To: rafael, lenb
  Cc: linux-acpi, linux-kernel, Sudeep.Holla, linuxarm,
	jonathan.cameron, zhanjie9, zhenglifeng1, yubowen8, lihuisong

The platform used LPI need check if the LPI support and the entry
method is valid by the acpi_processor_ffh_lpi_probe(). But the return
of acpi_processor_ffh_lpi_probe() in acpi_processor_setup_cpuidle_dev()
isn't verified by any caller.

What's more, acpi_processor_get_power_info() is a more logical place for
verifying the validity of FFH LPI than acpi_processor_setup_cpuidle_dev().
So move acpi_processor_ffh_lpi_probe() from the latter to the former and
verify its return.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 drivers/acpi/processor_idle.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 5f86297c8b23..cdf86874a87a 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -1252,7 +1252,7 @@ static int acpi_processor_setup_cpuidle_dev(struct acpi_processor *pr,
 
 	dev->cpu = pr->id;
 	if (pr->flags.has_lpi)
-		return acpi_processor_ffh_lpi_probe(pr->id);
+		return 0;
 
 	acpi_processor_setup_cpuidle_cx(pr, dev);
 	return 0;
@@ -1264,7 +1264,13 @@ static int acpi_processor_get_power_info(struct acpi_processor *pr)
 
 	ret = acpi_processor_get_lpi_info(pr);
 	if (ret)
-		ret = acpi_processor_get_cstate_info(pr);
+		return acpi_processor_get_cstate_info(pr);
+
+	if (pr->flags.has_lpi) {
+		ret = acpi_processor_ffh_lpi_probe(pr->id);
+		if (ret)
+			pr_err("Processor FFH LPI state is invalid.\n");
+	}
 
 	return ret;
 }
-- 
2.33.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/3] ACPI: processor: idle: Redefine acpi_processor_setup_cpuidle_dev to void
  2025-11-25  6:52 [PATCH 0/3] ACPI: processor: idle: Enhance LPI verification and Huisong Li
  2025-11-25  6:52 ` [PATCH 1/3] ACPI: processor: idle: Relocate and verify acpi_processor_ffh_lpi_probe Huisong Li
@ 2025-11-25  6:52 ` Huisong Li
  2025-11-25  6:52 ` [PATCH 3/3] ACPI: processor: idle: Redefine acpi_processor_setup_cpuidle_states " Huisong Li
  2 siblings, 0 replies; 9+ messages in thread
From: Huisong Li @ 2025-11-25  6:52 UTC (permalink / raw)
  To: rafael, lenb
  Cc: linux-acpi, linux-kernel, Sudeep.Holla, linuxarm,
	jonathan.cameron, zhanjie9, zhenglifeng1, yubowen8, lihuisong

Notice that the acpi_processor_setup_cpuidle_dev() don't need to
return any value because their callers don't check them anyway
and the function doesn't fail to execute.
So redefine the function to void.

No intentional functional impact.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 drivers/acpi/processor_idle.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index cdf86874a87a..2804fa91c1ec 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -1244,18 +1244,17 @@ static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
  * @pr: the ACPI processor
  * @dev : the cpuidle device
  */
-static int acpi_processor_setup_cpuidle_dev(struct acpi_processor *pr,
-					    struct cpuidle_device *dev)
+static void acpi_processor_setup_cpuidle_dev(struct acpi_processor *pr,
+					     struct cpuidle_device *dev)
 {
 	if (!pr->flags.power_setup_done || !pr->flags.power || !dev)
-		return -EINVAL;
+		return;
 
 	dev->cpu = pr->id;
 	if (pr->flags.has_lpi)
-		return 0;
+		return;
 
 	acpi_processor_setup_cpuidle_cx(pr, dev);
-	return 0;
 }
 
 static int acpi_processor_get_power_info(struct acpi_processor *pr)
-- 
2.33.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/3] ACPI: processor: idle: Redefine acpi_processor_setup_cpuidle_states to void
  2025-11-25  6:52 [PATCH 0/3] ACPI: processor: idle: Enhance LPI verification and Huisong Li
  2025-11-25  6:52 ` [PATCH 1/3] ACPI: processor: idle: Relocate and verify acpi_processor_ffh_lpi_probe Huisong Li
  2025-11-25  6:52 ` [PATCH 2/3] ACPI: processor: idle: Redefine acpi_processor_setup_cpuidle_dev to void Huisong Li
@ 2025-11-25  6:52 ` Huisong Li
  2 siblings, 0 replies; 9+ messages in thread
From: Huisong Li @ 2025-11-25  6:52 UTC (permalink / raw)
  To: rafael, lenb
  Cc: linux-acpi, linux-kernel, Sudeep.Holla, linuxarm,
	jonathan.cameron, zhanjie9, zhenglifeng1, yubowen8, lihuisong

Notice that the acpi_processor_setup_cpuidle_states() don't need to
return any value because their callers don't check them anyway.
In addition, acpi_processor_setup_lpi_states() wouldn't execute with
failure. So redefine setup idle functions to void.

No intentional functional impact.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 drivers/acpi/processor_idle.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 2804fa91c1ec..686aa18bbcd2 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -1180,7 +1180,7 @@ static int acpi_idle_lpi_enter(struct cpuidle_device *dev,
 	return -EINVAL;
 }
 
-static int acpi_processor_setup_lpi_states(struct acpi_processor *pr)
+static void acpi_processor_setup_lpi_states(struct acpi_processor *pr)
 {
 	int i;
 	struct acpi_lpi_state *lpi;
@@ -1188,7 +1188,7 @@ static int acpi_processor_setup_lpi_states(struct acpi_processor *pr)
 	struct cpuidle_driver *drv = &acpi_idle_driver;
 
 	if (!pr->flags.has_lpi)
-		return -EOPNOTSUPP;
+		return;
 
 	for (i = 0; i < pr->power.count && i < CPUIDLE_STATE_MAX; i++) {
 		lpi = &pr->power.lpi_states[i];
@@ -1206,8 +1206,6 @@ static int acpi_processor_setup_lpi_states(struct acpi_processor *pr)
 	}
 
 	drv->state_count = i;
-
-	return 0;
 }
 
 /**
@@ -1216,13 +1214,13 @@ static int acpi_processor_setup_lpi_states(struct acpi_processor *pr)
  *
  * @pr: the ACPI processor
  */
-static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
+static void acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
 {
 	int i;
 	struct cpuidle_driver *drv = &acpi_idle_driver;
 
 	if (!pr->flags.power_setup_done || !pr->flags.power)
-		return -EINVAL;
+		return;
 
 	drv->safe_state_index = -1;
 	for (i = ACPI_IDLE_STATE_START; i < CPUIDLE_STATE_MAX; i++) {
@@ -1230,11 +1228,12 @@ static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
 		drv->states[i].desc[0] = '\0';
 	}
 
-	if (pr->flags.has_lpi)
-		return acpi_processor_setup_lpi_states(pr);
+	if (pr->flags.has_lpi) {
+		acpi_processor_setup_lpi_states(pr);
+		return;
+	}
 
 	acpi_processor_setup_cstates(pr);
-	return 0;
 }
 
 /**
-- 
2.33.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] ACPI: processor: idle: Relocate and verify acpi_processor_ffh_lpi_probe
  2025-11-25  6:52 ` [PATCH 1/3] ACPI: processor: idle: Relocate and verify acpi_processor_ffh_lpi_probe Huisong Li
@ 2026-01-14 17:27   ` Rafael J. Wysocki
  2026-01-15 12:09     ` lihuisong (C)
  0 siblings, 1 reply; 9+ messages in thread
From: Rafael J. Wysocki @ 2026-01-14 17:27 UTC (permalink / raw)
  To: Huisong Li
  Cc: rafael, lenb, linux-acpi, linux-kernel, Sudeep.Holla, linuxarm,
	jonathan.cameron, zhanjie9, zhenglifeng1, yubowen8

On Tue, Nov 25, 2025 at 7:52 AM Huisong Li <lihuisong@huawei.com> wrote:
>
> The platform used LPI need check if the LPI support and the entry
> method is valid by the acpi_processor_ffh_lpi_probe(). But the return
> of acpi_processor_ffh_lpi_probe() in acpi_processor_setup_cpuidle_dev()
> isn't verified by any caller.
>
> What's more, acpi_processor_get_power_info() is a more logical place for
> verifying the validity of FFH LPI than acpi_processor_setup_cpuidle_dev().
> So move acpi_processor_ffh_lpi_probe() from the latter to the former and
> verify its return.
>
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---
>  drivers/acpi/processor_idle.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 5f86297c8b23..cdf86874a87a 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -1252,7 +1252,7 @@ static int acpi_processor_setup_cpuidle_dev(struct acpi_processor *pr,
>
>         dev->cpu = pr->id;
>         if (pr->flags.has_lpi)
> -               return acpi_processor_ffh_lpi_probe(pr->id);
> +               return 0;
>
>         acpi_processor_setup_cpuidle_cx(pr, dev);
>         return 0;
> @@ -1264,7 +1264,13 @@ static int acpi_processor_get_power_info(struct acpi_processor *pr)
>
>         ret = acpi_processor_get_lpi_info(pr);
>         if (ret)
> -               ret = acpi_processor_get_cstate_info(pr);
> +               return acpi_processor_get_cstate_info(pr);
> +
> +       if (pr->flags.has_lpi) {
> +               ret = acpi_processor_ffh_lpi_probe(pr->id);
> +               if (ret)
> +                       pr_err("Processor FFH LPI state is invalid.\n");
> +       }
>
>         return ret;
>  }
> --

Please reorder this behind the next patch in the series.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] ACPI: processor: idle: Relocate and verify acpi_processor_ffh_lpi_probe
  2026-01-14 17:27   ` Rafael J. Wysocki
@ 2026-01-15 12:09     ` lihuisong (C)
  2026-01-15 13:06       ` Rafael J. Wysocki
  0 siblings, 1 reply; 9+ messages in thread
From: lihuisong (C) @ 2026-01-15 12:09 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: lenb, linux-acpi, linux-kernel, Sudeep.Holla, linuxarm,
	jonathan.cameron, zhanjie9, zhenglifeng1, yubowen8, lihuisong

Hi Rafael,

On 1/15/2026 1:27 AM, Rafael J. Wysocki wrote:
> On Tue, Nov 25, 2025 at 7:52 AM Huisong Li <lihuisong@huawei.com> wrote:
>> The platform used LPI need check if the LPI support and the entry
>> method is valid by the acpi_processor_ffh_lpi_probe(). But the return
>> of acpi_processor_ffh_lpi_probe() in acpi_processor_setup_cpuidle_dev()
>> isn't verified by any caller.
>>
>> What's more, acpi_processor_get_power_info() is a more logical place for
>> verifying the validity of FFH LPI than acpi_processor_setup_cpuidle_dev().
>> So move acpi_processor_ffh_lpi_probe() from the latter to the former and
>> verify its return.
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> ---
>>   drivers/acpi/processor_idle.c | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
>> index 5f86297c8b23..cdf86874a87a 100644
>> --- a/drivers/acpi/processor_idle.c
>> +++ b/drivers/acpi/processor_idle.c
>> @@ -1252,7 +1252,7 @@ static int acpi_processor_setup_cpuidle_dev(struct acpi_processor *pr,
>>
>>          dev->cpu = pr->id;
>>          if (pr->flags.has_lpi)
>> -               return acpi_processor_ffh_lpi_probe(pr->id);
>> +               return 0;
>>
>>          acpi_processor_setup_cpuidle_cx(pr, dev);
>>          return 0;
>> @@ -1264,7 +1264,13 @@ static int acpi_processor_get_power_info(struct acpi_processor *pr)
>>
>>          ret = acpi_processor_get_lpi_info(pr);
>>          if (ret)
>> -               ret = acpi_processor_get_cstate_info(pr);
>> +               return acpi_processor_get_cstate_info(pr);
>> +
>> +       if (pr->flags.has_lpi) {
>> +               ret = acpi_processor_ffh_lpi_probe(pr->id);
>> +               if (ret)
>> +                       pr_err("Processor FFH LPI state is invalid.\n");
>> +       }
>>
>>          return ret;
>>   }
>> --
> Please reorder this behind the next patch in the series.
Patch 2/3 depends on this patch.
So I don't know how to reorder this patch.
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] ACPI: processor: idle: Relocate and verify acpi_processor_ffh_lpi_probe
  2026-01-15 12:09     ` lihuisong (C)
@ 2026-01-15 13:06       ` Rafael J. Wysocki
  2026-01-16  6:37         ` lihuisong (C)
  0 siblings, 1 reply; 9+ messages in thread
From: Rafael J. Wysocki @ 2026-01-15 13:06 UTC (permalink / raw)
  To: lihuisong (C)
  Cc: Rafael J. Wysocki, lenb, linux-acpi, linux-kernel, Sudeep.Holla,
	linuxarm, jonathan.cameron, zhanjie9, zhenglifeng1, yubowen8

On Thu, Jan 15, 2026 at 1:09 PM lihuisong (C) <lihuisong@huawei.com> wrote:
>
> Hi Rafael,
>
> On 1/15/2026 1:27 AM, Rafael J. Wysocki wrote:
> > On Tue, Nov 25, 2025 at 7:52 AM Huisong Li <lihuisong@huawei.com> wrote:
> >> The platform used LPI need check if the LPI support and the entry
> >> method is valid by the acpi_processor_ffh_lpi_probe(). But the return
> >> of acpi_processor_ffh_lpi_probe() in acpi_processor_setup_cpuidle_dev()
> >> isn't verified by any caller.
> >>
> >> What's more, acpi_processor_get_power_info() is a more logical place for
> >> verifying the validity of FFH LPI than acpi_processor_setup_cpuidle_dev().
> >> So move acpi_processor_ffh_lpi_probe() from the latter to the former and
> >> verify its return.
> >>
> >> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> >> ---
> >>   drivers/acpi/processor_idle.c | 10 ++++++++--
> >>   1 file changed, 8 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> >> index 5f86297c8b23..cdf86874a87a 100644
> >> --- a/drivers/acpi/processor_idle.c
> >> +++ b/drivers/acpi/processor_idle.c
> >> @@ -1252,7 +1252,7 @@ static int acpi_processor_setup_cpuidle_dev(struct acpi_processor *pr,
> >>
> >>          dev->cpu = pr->id;
> >>          if (pr->flags.has_lpi)
> >> -               return acpi_processor_ffh_lpi_probe(pr->id);
> >> +               return 0;
> >>
> >>          acpi_processor_setup_cpuidle_cx(pr, dev);
> >>          return 0;
> >> @@ -1264,7 +1264,13 @@ static int acpi_processor_get_power_info(struct acpi_processor *pr)
> >>
> >>          ret = acpi_processor_get_lpi_info(pr);
> >>          if (ret)
> >> -               ret = acpi_processor_get_cstate_info(pr);
> >> +               return acpi_processor_get_cstate_info(pr);
> >> +
> >> +       if (pr->flags.has_lpi) {
> >> +               ret = acpi_processor_ffh_lpi_probe(pr->id);
> >> +               if (ret)
> >> +                       pr_err("Processor FFH LPI state is invalid.\n");
> >> +       }
> >>
> >>          return ret;
> >>   }
> >> --
> > Please reorder this behind the next patch in the series.
> Patch 2/3 depends on this patch.
> So I don't know how to reorder this patch.

I should have been more precise, sorry.

Please first convert acpi_processor_setup_cpuidle_dev() to a void
function and then make the changes from this patch on top of that.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] ACPI: processor: idle: Relocate and verify acpi_processor_ffh_lpi_probe
  2026-01-15 13:06       ` Rafael J. Wysocki
@ 2026-01-16  6:37         ` lihuisong (C)
  2026-01-16 12:22           ` Rafael J. Wysocki
  0 siblings, 1 reply; 9+ messages in thread
From: lihuisong (C) @ 2026-01-16  6:37 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: lenb, linux-acpi, linux-kernel, Sudeep.Holla, linuxarm,
	jonathan.cameron, zhanjie9, zhenglifeng1, yubowen8, lihuisong


On 1/15/2026 9:06 PM, Rafael J. Wysocki wrote:
> On Thu, Jan 15, 2026 at 1:09 PM lihuisong (C) <lihuisong@huawei.com> wrote:
>> Hi Rafael,
>>
>> On 1/15/2026 1:27 AM, Rafael J. Wysocki wrote:
>>> On Tue, Nov 25, 2025 at 7:52 AM Huisong Li <lihuisong@huawei.com> wrote:
>>>> The platform used LPI need check if the LPI support and the entry
>>>> method is valid by the acpi_processor_ffh_lpi_probe(). But the return
>>>> of acpi_processor_ffh_lpi_probe() in acpi_processor_setup_cpuidle_dev()
>>>> isn't verified by any caller.
>>>>
>>>> What's more, acpi_processor_get_power_info() is a more logical place for
>>>> verifying the validity of FFH LPI than acpi_processor_setup_cpuidle_dev().
>>>> So move acpi_processor_ffh_lpi_probe() from the latter to the former and
>>>> verify its return.
>>>>
>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>> ---
>>>>    drivers/acpi/processor_idle.c | 10 ++++++++--
>>>>    1 file changed, 8 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
>>>> index 5f86297c8b23..cdf86874a87a 100644
>>>> --- a/drivers/acpi/processor_idle.c
>>>> +++ b/drivers/acpi/processor_idle.c
>>>> @@ -1252,7 +1252,7 @@ static int acpi_processor_setup_cpuidle_dev(struct acpi_processor *pr,
>>>>
>>>>           dev->cpu = pr->id;
>>>>           if (pr->flags.has_lpi)
>>>> -               return acpi_processor_ffh_lpi_probe(pr->id);
>>>> +               return 0;
>>>>
>>>>           acpi_processor_setup_cpuidle_cx(pr, dev);
>>>>           return 0;
>>>> @@ -1264,7 +1264,13 @@ static int acpi_processor_get_power_info(struct acpi_processor *pr)
>>>>
>>>>           ret = acpi_processor_get_lpi_info(pr);
>>>>           if (ret)
>>>> -               ret = acpi_processor_get_cstate_info(pr);
>>>> +               return acpi_processor_get_cstate_info(pr);
>>>> +
>>>> +       if (pr->flags.has_lpi) {
>>>> +               ret = acpi_processor_ffh_lpi_probe(pr->id);
>>>> +               if (ret)
>>>> +                       pr_err("Processor FFH LPI state is invalid.\n");
>>>> +       }
>>>>
>>>>           return ret;
>>>>    }
>>>> --
>>> Please reorder this behind the next patch in the series.
>> Patch 2/3 depends on this patch.
>> So I don't know how to reorder this patch.
> I should have been more precise, sorry.
>
> Please first convert acpi_processor_setup_cpuidle_dev() to a void
> function and then make the changes from this patch on top of that.
The acpi_processor_ffh_lpi_probe may return an error.
And acpi_processor_setup_cpuidle_dev can pass the error code to its 
caller(Although the caller ignored it currently).
It may be inapproprate to convert acpi_processor_setup_cpuidle_dev() to 
a void function directly if we doesn't move acpi_processor_ffh_lpi_probe 
out first.
So I first relocate the position of acpi_processor_ffh_lpi_probe. Then 
changing it to a void function would be more logical.

Or we need to drop the return value of acpi_processor_ffh_lpi_probe and 
convert acpi_processor_setup_cpuidle_dev to a void function, like:
-->

-static int acpi_processor_setup_cpuidle_dev(struct acpi_processor *pr,
-					    struct cpuidle_device *dev)
+static void acpi_processor_setup_cpuidle_dev(struct acpi_processor *pr,
+					     struct cpuidle_device *dev)
  {
  	if (!pr->flags.power_setup_done || !pr->flags.power || !dev)
-		return -EINVAL;
+		return;
  
  	dev->cpu = pr->id;
  	if (pr->flags.has_lpi) {
-		return acpi_processor_ffh_lpi_probe();
+               acpi_processor_ffh_lpi_probe();
+		return;
         }
  
  	acpi_processor_setup_cpuidle_cx(pr, dev);
-	return 0;
  }

What do you think now?

>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] ACPI: processor: idle: Relocate and verify acpi_processor_ffh_lpi_probe
  2026-01-16  6:37         ` lihuisong (C)
@ 2026-01-16 12:22           ` Rafael J. Wysocki
  0 siblings, 0 replies; 9+ messages in thread
From: Rafael J. Wysocki @ 2026-01-16 12:22 UTC (permalink / raw)
  To: lihuisong (C)
  Cc: Rafael J. Wysocki, lenb, linux-acpi, linux-kernel, Sudeep.Holla,
	linuxarm, jonathan.cameron, zhanjie9, zhenglifeng1, yubowen8

On Fri, Jan 16, 2026 at 7:38 AM lihuisong (C) <lihuisong@huawei.com> wrote:
>
>
> On 1/15/2026 9:06 PM, Rafael J. Wysocki wrote:
> > On Thu, Jan 15, 2026 at 1:09 PM lihuisong (C) <lihuisong@huawei.com> wrote:
> >> Hi Rafael,
> >>
> >> On 1/15/2026 1:27 AM, Rafael J. Wysocki wrote:
> >>> On Tue, Nov 25, 2025 at 7:52 AM Huisong Li <lihuisong@huawei.com> wrote:
> >>>> The platform used LPI need check if the LPI support and the entry
> >>>> method is valid by the acpi_processor_ffh_lpi_probe(). But the return
> >>>> of acpi_processor_ffh_lpi_probe() in acpi_processor_setup_cpuidle_dev()
> >>>> isn't verified by any caller.
> >>>>
> >>>> What's more, acpi_processor_get_power_info() is a more logical place for
> >>>> verifying the validity of FFH LPI than acpi_processor_setup_cpuidle_dev().
> >>>> So move acpi_processor_ffh_lpi_probe() from the latter to the former and
> >>>> verify its return.
> >>>>
> >>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> >>>> ---
> >>>>    drivers/acpi/processor_idle.c | 10 ++++++++--
> >>>>    1 file changed, 8 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> >>>> index 5f86297c8b23..cdf86874a87a 100644
> >>>> --- a/drivers/acpi/processor_idle.c
> >>>> +++ b/drivers/acpi/processor_idle.c
> >>>> @@ -1252,7 +1252,7 @@ static int acpi_processor_setup_cpuidle_dev(struct acpi_processor *pr,
> >>>>
> >>>>           dev->cpu = pr->id;
> >>>>           if (pr->flags.has_lpi)
> >>>> -               return acpi_processor_ffh_lpi_probe(pr->id);
> >>>> +               return 0;
> >>>>
> >>>>           acpi_processor_setup_cpuidle_cx(pr, dev);
> >>>>           return 0;
> >>>> @@ -1264,7 +1264,13 @@ static int acpi_processor_get_power_info(struct acpi_processor *pr)
> >>>>
> >>>>           ret = acpi_processor_get_lpi_info(pr);
> >>>>           if (ret)
> >>>> -               ret = acpi_processor_get_cstate_info(pr);
> >>>> +               return acpi_processor_get_cstate_info(pr);
> >>>> +
> >>>> +       if (pr->flags.has_lpi) {
> >>>> +               ret = acpi_processor_ffh_lpi_probe(pr->id);
> >>>> +               if (ret)
> >>>> +                       pr_err("Processor FFH LPI state is invalid.\n");
> >>>> +       }
> >>>>
> >>>>           return ret;
> >>>>    }
> >>>> --
> >>> Please reorder this behind the next patch in the series.
> >> Patch 2/3 depends on this patch.
> >> So I don't know how to reorder this patch.
> > I should have been more precise, sorry.
> >
> > Please first convert acpi_processor_setup_cpuidle_dev() to a void
> > function and then make the changes from this patch on top of that.
> The acpi_processor_ffh_lpi_probe may return an error.
> And acpi_processor_setup_cpuidle_dev can pass the error code to its
> caller(Although the caller ignored it currently).

If all of its callers ignore its return value, it can and arguably
should be a void function.

> It may be inapproprate to convert acpi_processor_setup_cpuidle_dev() to
> a void function directly if we doesn't move acpi_processor_ffh_lpi_probe
> out first.
> So I first relocate the position of acpi_processor_ffh_lpi_probe. Then
> changing it to a void function would be more logical.
>
> Or we need to drop the return value of acpi_processor_ffh_lpi_probe and
> convert acpi_processor_setup_cpuidle_dev to a void function, like:
> -->
>
> -static int acpi_processor_setup_cpuidle_dev(struct acpi_processor *pr,
> -                                           struct cpuidle_device *dev)
> +static void acpi_processor_setup_cpuidle_dev(struct acpi_processor *pr,
> +                                            struct cpuidle_device *dev)
>   {
>         if (!pr->flags.power_setup_done || !pr->flags.power || !dev)
> -               return -EINVAL;
> +               return;
>
>         dev->cpu = pr->id;
>         if (pr->flags.has_lpi) {
> -               return acpi_processor_ffh_lpi_probe();
> +               acpi_processor_ffh_lpi_probe();
> +               return;
>          }
>
>         acpi_processor_setup_cpuidle_cx(pr, dev);
> -       return 0;
>   }
>
> What do you think now?

Well, please see above.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-01-16 12:22 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-25  6:52 [PATCH 0/3] ACPI: processor: idle: Enhance LPI verification and Huisong Li
2025-11-25  6:52 ` [PATCH 1/3] ACPI: processor: idle: Relocate and verify acpi_processor_ffh_lpi_probe Huisong Li
2026-01-14 17:27   ` Rafael J. Wysocki
2026-01-15 12:09     ` lihuisong (C)
2026-01-15 13:06       ` Rafael J. Wysocki
2026-01-16  6:37         ` lihuisong (C)
2026-01-16 12:22           ` Rafael J. Wysocki
2025-11-25  6:52 ` [PATCH 2/3] ACPI: processor: idle: Redefine acpi_processor_setup_cpuidle_dev to void Huisong Li
2025-11-25  6:52 ` [PATCH 3/3] ACPI: processor: idle: Redefine acpi_processor_setup_cpuidle_states " Huisong Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®