| Message ID | 20250328143040.9348-2-ggherdovich@suse.cz (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | [1/2] ACPI: processor: idle: Return an error if both P_LVL{2,3} idle states are invalid | expand |
> -----Original Message----- > From: Giovanni Gherdovich <ggherdovich@suse.cz> > Sent: Friday, March 28, 2025 10:31 PM > To: Rafael J . Wysocki <rafael@kernel.org>; Zhang, Rui <rui.zhang@intel.com> > Cc: Len Brown <lenb@kernel.org>; Giovanni Gherdovich > <ggherdovich@suse.cz>; linux-acpi@vger.kernel.org; linux- > kernel@vger.kernel.org; linux-pm@vger.kernel.org > Subject: [PATCH 2/2] ACPI: processor: idle: Remove obsolete comment > Importance: High > > Since commit 496121c02127e9c460b436244c38260b044cc45a ("ACPI: > processor: > idle: Allow probing on platforms with one ACPI C-state"), the comment > doesn't reflect the code anymore; remove it. > > Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz> > --- > drivers/acpi/processor_idle.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c > index b181f7fc2090..2a076c7a825a 100644 > --- a/drivers/acpi/processor_idle.c > +++ b/drivers/acpi/processor_idle.c > @@ -482,10 +482,6 @@ static int acpi_processor_get_cstate_info(struct > acpi_processor *pr) > > pr->power.count = acpi_processor_power_verify(pr); > > - /* > - * if one state of type C2 or C3 is available, mark this > - * CPU as being "idle manageable" > - */ > for (i = 1; i < ACPI_PROCESSOR_MAX_POWER; i++) { > if (pr->power.states[i].valid) { > pr->power.count = i; > -- > 2.43.0 I think we can clean up a bit more. How about the patch below? From 115d3a07febff32eed49f9343ef111e7e1452f9d Mon Sep 17 00:00:00 2001 From: "Zhang, Rui" <rui.zhang@intel.com> Date: Mon, 31 Mar 2025 07:29:57 +0000 Subject: [PATCH] ACPI: processor: idle: Simplify acpi_processor_get_cstate_info() logic Since commit 496121c02127 ("ACPI: processor: idle: Allow probing on platforms with one ACPI C-state"), acpi_idle driver can be probed with C1 only. Optimize the logic for setting pr->power.count and pr->flags.power by 1. unconditionally set pr->flags.power leveraging the fact that C1 is always valid after acpi_processor_get_power_info_default(). 2. update acpi_processor_power_verify() to return the highest valid C-state directly. No functional change intended. Signed-off-by: Zhang Rui <rui.zhang@intel.com> --- drivers/acpi/processor_idle.c | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index 698897b29de2..7ce8c3802937 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -442,7 +442,7 @@ static int acpi_processor_power_verify(struct acpi_processor *pr) lapic_timer_check_state(i, pr, cx); tsc_check_state(cx->type); - working++; + working = i; } if (buggy_latency) { @@ -457,7 +457,6 @@ static int acpi_processor_power_verify(struct acpi_processor *pr) static int acpi_processor_get_cstate_info(struct acpi_processor *pr) { - unsigned int i; int result; @@ -477,17 +476,7 @@ static int acpi_processor_get_cstate_info(struct acpi_processor *pr) acpi_processor_get_power_info_default(pr); pr->power.count = acpi_processor_power_verify(pr); - - /* - * if one state of type C2 or C3 is available, mark this - * CPU as being "idle manageable" - */ - for (i = 1; i < ACPI_PROCESSOR_MAX_POWER; i++) { - if (pr->power.states[i].valid) { - pr->power.count = i; - pr->flags.power = 1; - } - } + pr->flags.power = 1; return 0; }
On Mon, Mar 31, 2025 at 9:38 AM Zhang, Rui <rui.zhang@intel.com> wrote: > > > -----Original Message----- > > From: Giovanni Gherdovich <ggherdovich@suse.cz> > > Sent: Friday, March 28, 2025 10:31 PM > > To: Rafael J . Wysocki <rafael@kernel.org>; Zhang, Rui <rui.zhang@intel.com> > > Cc: Len Brown <lenb@kernel.org>; Giovanni Gherdovich > > <ggherdovich@suse.cz>; linux-acpi@vger.kernel.org; linux- > > kernel@vger.kernel.org; linux-pm@vger.kernel.org > > Subject: [PATCH 2/2] ACPI: processor: idle: Remove obsolete comment > > Importance: High > > > > Since commit 496121c02127e9c460b436244c38260b044cc45a ("ACPI: > > processor: > > idle: Allow probing on platforms with one ACPI C-state"), the comment > > doesn't reflect the code anymore; remove it. > > > > Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz> > > --- > > drivers/acpi/processor_idle.c | 4 ---- > > 1 file changed, 4 deletions(-) > > > > diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c > > index b181f7fc2090..2a076c7a825a 100644 > > --- a/drivers/acpi/processor_idle.c > > +++ b/drivers/acpi/processor_idle.c > > @@ -482,10 +482,6 @@ static int acpi_processor_get_cstate_info(struct > > acpi_processor *pr) > > > > pr->power.count = acpi_processor_power_verify(pr); > > > > - /* > > - * if one state of type C2 or C3 is available, mark this > > - * CPU as being "idle manageable" > > - */ > > for (i = 1; i < ACPI_PROCESSOR_MAX_POWER; i++) { > > if (pr->power.states[i].valid) { > > pr->power.count = i; > > -- > > 2.43.0 > > I think we can clean up a bit more. How about the patch below? > > From 115d3a07febff32eed49f9343ef111e7e1452f9d Mon Sep 17 00:00:00 2001 > From: "Zhang, Rui" <rui.zhang@intel.com> > Date: Mon, 31 Mar 2025 07:29:57 +0000 > Subject: [PATCH] ACPI: processor: idle: Simplify > acpi_processor_get_cstate_info() logic > > Since commit 496121c02127 ("ACPI: processor: idle: Allow probing on > platforms with one ACPI C-state"), acpi_idle driver can be probed with > C1 only. > > Optimize the logic for setting pr->power.count and pr->flags.power by > 1. unconditionally set pr->flags.power leveraging the fact that C1 is > always valid after acpi_processor_get_power_info_default(). > 2. update acpi_processor_power_verify() to return the highest valid > C-state directly. > > No functional change intended. > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > --- > drivers/acpi/processor_idle.c | 15 ++------------- > 1 file changed, 2 insertions(+), 13 deletions(-) > > diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c > index 698897b29de2..7ce8c3802937 100644 > --- a/drivers/acpi/processor_idle.c > +++ b/drivers/acpi/processor_idle.c > @@ -442,7 +442,7 @@ static int acpi_processor_power_verify(struct acpi_processor *pr) > > lapic_timer_check_state(i, pr, cx); > tsc_check_state(cx->type); > - working++; > + working = i; What if some states are skipped because they are invalid? 'working' can be less than 'i' then AFAICS. > } > > if (buggy_latency) { > @@ -457,7 +457,6 @@ static int acpi_processor_power_verify(struct acpi_processor *pr) > > static int acpi_processor_get_cstate_info(struct acpi_processor *pr) > { > - unsigned int i; > int result; > > > @@ -477,17 +476,7 @@ static int acpi_processor_get_cstate_info(struct acpi_processor *pr) > acpi_processor_get_power_info_default(pr); > > pr->power.count = acpi_processor_power_verify(pr); > - > - /* > - * if one state of type C2 or C3 is available, mark this > - * CPU as being "idle manageable" > - */ > - for (i = 1; i < ACPI_PROCESSOR_MAX_POWER; i++) { > - if (pr->power.states[i].valid) { > - pr->power.count = i; > - pr->flags.power = 1; > - } > - } > + pr->flags.power = 1; > > return 0; > } > --
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index b181f7fc2090..2a076c7a825a 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -482,10 +482,6 @@ static int acpi_processor_get_cstate_info(struct acpi_processor *pr) pr->power.count = acpi_processor_power_verify(pr); - /* - * if one state of type C2 or C3 is available, mark this - * CPU as being "idle manageable" - */ for (i = 1; i < ACPI_PROCESSOR_MAX_POWER; i++) { if (pr->power.states[i].valid) { pr->power.count = i;
Since commit 496121c02127e9c460b436244c38260b044cc45a ("ACPI: processor: idle: Allow probing on platforms with one ACPI C-state"), the comment doesn't reflect the code anymore; remove it. Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz> --- drivers/acpi/processor_idle.c | 4 ---- 1 file changed, 4 deletions(-)