[v1] ACPI: processor: idle: enhance and cleancode for cpuidle state

[PATCH v1 1/9] ACPI: processor: idle: raise up log level when evaluate LPI failed

Posted by Huisong Li 4 months, 1 week ago

According to ACPI spec, LPI package must be ACPI_TYPE_PACKAGE and
the count of package must be greater than 4. And the count contained
in package needs to be equal to the value of count field in LPI package.
All are illegal and return failure. It is better for these verification
to use error level log instead of debug so as to get detailed logs directly
when initialization fails.

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

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 22b051b94a86..5acf12a0441f 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -908,7 +908,7 @@ static int acpi_processor_evaluate_lpi(acpi_handle handle,
 	/* There must be at least 4 elements = 3 elements + 1 package */
 	if (!lpi_data || lpi_data->type != ACPI_TYPE_PACKAGE ||
 	    lpi_data->package.count < 4) {
-		pr_debug("not enough elements in _LPI\n");
+		pr_err("not enough elements in _LPI\n");
 		ret = -ENODATA;
 		goto end;
 	}
@@ -917,7 +917,7 @@ static int acpi_processor_evaluate_lpi(acpi_handle handle,
 
 	/* Validate number of power states. */
 	if (pkg_count < 1 || pkg_count != lpi_data->package.count - 3) {
-		pr_debug("count given by _LPI is not valid\n");
+		pr_err("count given by _LPI is not valid\n");
 		ret = -ENODATA;
 		goto end;
 	}
-- 
2.33.0

Re: [PATCH v1 1/9] ACPI: processor: idle: raise up log level when evaluate LPI failed

Posted by Rafael J. Wysocki 3 months, 2 weeks ago

On Mon, Sep 29, 2025 at 11:38 AM Huisong Li <lihuisong@huawei.com> wrote:
>
> According to ACPI spec, LPI package must be ACPI_TYPE_PACKAGE and
> the count of package must be greater than 4. And the count contained
> in package needs to be equal to the value of count field in LPI package.
> All are illegal and return failure. It is better for these verification
> to use error level log instead of debug so as to get detailed logs directly
> when initialization fails.
>
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---
>  drivers/acpi/processor_idle.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 22b051b94a86..5acf12a0441f 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -908,7 +908,7 @@ static int acpi_processor_evaluate_lpi(acpi_handle handle,
>         /* There must be at least 4 elements = 3 elements + 1 package */
>         if (!lpi_data || lpi_data->type != ACPI_TYPE_PACKAGE ||
>             lpi_data->package.count < 4) {
> -               pr_debug("not enough elements in _LPI\n");
> +               pr_err("not enough elements in _LPI\n");
>                 ret = -ENODATA;
>                 goto end;
>         }
> @@ -917,7 +917,7 @@ static int acpi_processor_evaluate_lpi(acpi_handle handle,
>
>         /* Validate number of power states. */
>         if (pkg_count < 1 || pkg_count != lpi_data->package.count - 3) {
> -               pr_debug("count given by _LPI is not valid\n");
> +               pr_err("count given by _LPI is not valid\n");
>                 ret = -ENODATA;
>                 goto end;
>         }
> --

They are pr_debug() on purpose because they are not useful to anyone
other than the people who work on _LPI implementations in firmware or
debug firmware issues.  They do not indicate kernel functional issues
in particular.

Re: [PATCH v1 1/9] ACPI: processor: idle: raise up log level when evaluate LPI failed

Posted by lihuisong (C) 3 months, 2 weeks ago

在 2025/10/22 3:29, Rafael J. Wysocki 写道:
> On Mon, Sep 29, 2025 at 11:38 AM Huisong Li <lihuisong@huawei.com> wrote:
>> According to ACPI spec, LPI package must be ACPI_TYPE_PACKAGE and
>> the count of package must be greater than 4. And the count contained
>> in package needs to be equal to the value of count field in LPI package.
>> All are illegal and return failure. It is better for these verification
>> to use error level log instead of debug so as to get detailed logs directly
>> when initialization fails.
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> ---
>>   drivers/acpi/processor_idle.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
>> index 22b051b94a86..5acf12a0441f 100644
>> --- a/drivers/acpi/processor_idle.c
>> +++ b/drivers/acpi/processor_idle.c
>> @@ -908,7 +908,7 @@ static int acpi_processor_evaluate_lpi(acpi_handle handle,
>>          /* There must be at least 4 elements = 3 elements + 1 package */
>>          if (!lpi_data || lpi_data->type != ACPI_TYPE_PACKAGE ||
>>              lpi_data->package.count < 4) {
>> -               pr_debug("not enough elements in _LPI\n");
>> +               pr_err("not enough elements in _LPI\n");
>>                  ret = -ENODATA;
>>                  goto end;
>>          }
>> @@ -917,7 +917,7 @@ static int acpi_processor_evaluate_lpi(acpi_handle handle,
>>
>>          /* Validate number of power states. */
>>          if (pkg_count < 1 || pkg_count != lpi_data->package.count - 3) {
>> -               pr_debug("count given by _LPI is not valid\n");
>> +               pr_err("count given by _LPI is not valid\n");
>>                  ret = -ENODATA;
>>                  goto end;
>>          }
>> --
> They are pr_debug() on purpose because they are not useful to anyone
> other than the people who work on _LPI implementations in firmware or
> debug firmware issues.  They do not indicate kernel functional issues
> in particular.
ok, get this purpose. Thanks.