[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <9fe902df-98a3-47cf-a199-092c9ef90ce2@huawei.com>
Date: Mon, 25 Aug 2025 14:05:02 +0800
From: "lihuisong (C)" <lihuisong@...wei.com>
To: "Rafael J. Wysocki" <rafael@...nel.org>
CC: <lenb@...nel.org>, <linux-acpi@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linuxarm@...wei.com>,
<jonathan.cameron@...wei.com>, <zhanjie9@...ilicon.com>,
<zhenglifeng1@...wei.com>, <yubowen8@...wei.com>, <liuyonglong@...wei.com>,
<lihuisong@...wei.com>
Subject: Re: [PATCH v3 2/2] ACPI: processor: idle: Optimize acpi idle driver
registration
在 2025/8/24 0:13, Rafael J. Wysocki 写道:
> On Mon, Jul 28, 2025 at 9:06 AM Huisong Li <lihuisong@...wei.com> wrote:
>> Currently, the acpi idle driver is registered from within a CPU
>> hotplug callback. Although this didn't cause any functional issues,
>> this is questionable and confusing. And it is better to register
>> the cpuidle driver when all of the CPUs have been brought up.
>>
>> So add a new function to initialize acpi_idle_driver based on the
>> power management information of an available CPU and register cpuidle
>> driver in acpi_processor_driver_init().
>>
>> Signed-off-by: Huisong Li <lihuisong@...wei.com>
>> ---
>> drivers/acpi/processor_driver.c | 3 ++
>> drivers/acpi/processor_idle.c | 65 +++++++++++++++++++++------------
>> include/acpi/processor.h | 8 ++++
>> 3 files changed, 53 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
>> index 65e779be64ff..bc9f58a02c1d 100644
>> --- a/drivers/acpi/processor_driver.c
>> +++ b/drivers/acpi/processor_driver.c
>> @@ -263,6 +263,8 @@ static int __init acpi_processor_driver_init(void)
>> if (result < 0)
>> return result;
>>
>> + acpi_processor_register_idle_driver();
>> +
>> result = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
>> "acpi/cpu-drv:online",
>> acpi_soft_cpu_online, NULL);
>> @@ -301,6 +303,7 @@ static void __exit acpi_processor_driver_exit(void)
>>
>> cpuhp_remove_state_nocalls(hp_online);
>> cpuhp_remove_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD);
>> + acpi_processor_unregister_idle_driver();
>> driver_unregister(&acpi_processor_driver);
>> }
>>
>> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
>> index 031738390f2d..c71802d42e8a 100644
>> --- a/drivers/acpi/processor_idle.c
>> +++ b/drivers/acpi/processor_idle.c
>> @@ -1360,7 +1360,48 @@ int acpi_processor_power_state_has_changed(struct acpi_processor *pr)
>> return 0;
>> }
>>
>> -static int acpi_processor_registered;
>> +void acpi_processor_register_idle_driver(void)
>> +{
>> + struct acpi_processor *pr;
>> + int ret = -ENODEV;
>> + int cpu;
>> +
>> + /*
>> + * Acpi idle driver is used by all possible CPUs.
>> + * Install the idle handler by the processor power info of one in them.
>> + * Note that we use previously set idle handler will be used on
>> + * platforms that only support C1.
>> + */
>> + for_each_cpu(cpu, (struct cpumask *)cpu_possible_mask) {
>> + pr = per_cpu(processors, cpu);
>> + if (!pr)
>> + continue;
>> +
>> + ret = acpi_processor_get_power_info(pr);
>> + if (!ret) {
>> + pr->flags.power_setup_done = 1;
>> + acpi_processor_setup_cpuidle_states(pr);
>> + break;
>> + }
>> + }
>> +
>> + if (ret) {
>> + pr_debug("No ACPI power information from any CPUs.\n");
>> + return;
>> + }
>> +
>> + ret = cpuidle_register_driver(&acpi_idle_driver);
>> + if (ret) {
>> + pr_debug("register %s failed.\n", acpi_idle_driver.name);
>> + return;
>> + }
>> + pr_debug("%s registered with cpuidle.\n", acpi_idle_driver.name);
>> +}
>> +
>> +void acpi_processor_unregister_idle_driver(void)
>> +{
>> + cpuidle_unregister_driver(&acpi_idle_driver);
>> +}
>>
>> int acpi_processor_power_init(struct acpi_processor *pr)
>> {
>> @@ -1375,22 +1416,7 @@ int acpi_processor_power_init(struct acpi_processor *pr)
>> if (!acpi_processor_get_power_info(pr))
>> pr->flags.power_setup_done = 1;
>>
>> - /*
>> - * Install the idle handler if processor power management is supported.
>> - * Note that we use previously set idle handler will be used on
>> - * platforms that only support C1.
>> - */
>> if (pr->flags.power) {
>> - /* Register acpi_idle_driver if not already registered */
>> - if (!acpi_processor_registered) {
>> - acpi_processor_setup_cpuidle_states(pr);
>> - retval = cpuidle_register_driver(&acpi_idle_driver);
>> - if (retval)
>> - return retval;
>> - pr_debug("%s registered with cpuidle\n",
>> - acpi_idle_driver.name);
>> - }
>> -
>> dev = kzalloc(sizeof(*dev), GFP_KERNEL);
>> if (!dev)
>> return -ENOMEM;
>> @@ -1403,13 +1429,10 @@ int acpi_processor_power_init(struct acpi_processor *pr)
>> */
>> retval = cpuidle_register_device(dev);
>> if (retval) {
>> - if (acpi_processor_registered == 0)
>> - cpuidle_unregister_driver(&acpi_idle_driver);
>> kfree(dev);
>> per_cpu(acpi_cpuidle_device, pr->id) = NULL;
>> return retval;
>> }
>> - acpi_processor_registered++;
>> }
>> return 0;
>> }
>> @@ -1423,10 +1446,6 @@ int acpi_processor_power_exit(struct acpi_processor *pr)
>>
>> if (pr->flags.power) {
>> cpuidle_unregister_device(dev);
>> - acpi_processor_registered--;
>> - if (acpi_processor_registered == 0)
>> - cpuidle_unregister_driver(&acpi_idle_driver);
>> -
>> kfree(dev);
>> }
>>
>> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
>> index d0eccbd920e5..1249f5e81d92 100644
>> --- a/include/acpi/processor.h
>> +++ b/include/acpi/processor.h
>> @@ -423,6 +423,8 @@ int acpi_processor_power_init(struct acpi_processor *pr);
>> int acpi_processor_power_exit(struct acpi_processor *pr);
>> int acpi_processor_power_state_has_changed(struct acpi_processor *pr);
>> int acpi_processor_hotplug(struct acpi_processor *pr);
>> +void acpi_processor_register_idle_driver(void);
>> +void acpi_processor_unregister_idle_driver(void);
>> #else
>> static inline int acpi_processor_power_init(struct acpi_processor *pr)
>> {
>> @@ -443,6 +445,12 @@ static inline int acpi_processor_hotplug(struct acpi_processor *pr)
>> {
>> return -ENODEV;
>> }
>> +static void acpi_processor_register_idle_driver(void)
>> +{
>> +}
>> +static void acpi_processor_unregister_idle_driver(void)
>> +{
>> +}
>> #endif /* CONFIG_ACPI_PROCESSOR_IDLE */
>>
>> /* in processor_thermal.c */
>> --
> Applied as 6.18 material, thanks!
>
> While at it, in the future, please always spell ACPI in capitals in
> patch subjects, changelogs and code comments.
Get it. Thanks.
>
Powered by blists - more mailing lists