Received-SPF: pass (google.com: domain of linux-kernel+bounces-138592-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Wed, 10 Apr 2024 14:23:59 +0100
From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: Miguel Luis <miguel.luis@oracle.com>
CC: "Rafael J. Wysocki" <rafael@kernel.org>, Len Brown <lenb@kernel.org>,
	<linux-acpi@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<rmk+kernel@armlinux.org.uk>
Subject: Re: [RFC PATCH 3/4] ACPI: processor: refactor
 acpi_processor_get_info: isolate acpi_{map|unmap}_cpu under
 CONFIG_ACPI_HOTPLUG_CPU
Message-ID: <20240410142359.00003dea@Huawei.com>
In-Reply-To: <20240409150536.9933-4-miguel.luis@oracle.com>
References: <20240409150536.9933-1-miguel.luis@oracle.com>
	<20240409150536.9933-4-miguel.luis@oracle.com>
Organization: Huawei Technologies Research and Development (UK) Ltd.
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="US-ASCII"
Content-Transfer-Encoding: 7bit

On Tue,  9 Apr 2024 15:05:32 +0000
Miguel Luis <miguel.luis@oracle.com> wrote:

> mapping and unmaping a cpu at the stage of extra cpu enumeration is
> architecture specific which depends on CONFIG_ACPI_HOTPLUG_CPU so let's
> isolate that functionality from architecture independent one.

Should we consider renaming acpi_map_cpu() to arch_acpi_map_cpu()
to make the arch specific nature of that call more obvious?
I think that has caused more confusion in the discussion than
whether it is hotplug specific or not.

As mentioned in patch 2, fairly sure this needs to go before that
patch.

Jonathan

> 
> Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
> ---
>  drivers/acpi/acpi_processor.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
> index 9ea58b61d741..c6e2f64a056b 100644
> --- a/drivers/acpi/acpi_processor.c
> +++ b/drivers/acpi/acpi_processor.c
> @@ -194,8 +194,21 @@ static void acpi_processor_hotplug_delay_init(struct acpi_processor *pr)
>  	pr_info("CPU%d has been hot-added\n", pr->id);
>  	pr->flags.need_hotplug_init = 1;
>  }
> +static int acpi_processor_hotplug_map_cpu(struct acpi_processor *pr)
> +{
> +	return acpi_map_cpu(pr->handle, pr->phys_id, pr->acpi_id, &pr->id);
> +}
> +static void acpi_processor_hotplug_unmap_cpu(struct acpi_processor *pr)
> +{
> +	acpi_unmap_cpu(pr->id);
> +}
>  #else
>  static void acpi_processor_hotplug_delay_init(struct acpi_processor *pr) {}
> +static int acpi_processor_hotplug_map_cpu(struct acpi_processor *pr)
> +{
> +	return 0;
> +}
> +static void acpi_processor_hotplug_unmap_cpu(struct acpi_processor *pr) {}
>  #endif /* CONFIG_ACPI_HOTPLUG_CPU */
>  
>  /* Enumerate extra CPUs */
> @@ -215,13 +228,13 @@ static int acpi_processor_enumerate_extra(struct acpi_processor *pr)
>  	cpu_maps_update_begin();
>  	cpus_write_lock();
>  
> -	ret = acpi_map_cpu(pr->handle, pr->phys_id, pr->acpi_id, &pr->id);
> +	ret = acpi_processor_hotplug_map_cpu(pr);
>  	if (ret)
>  		goto out;
>  
>  	ret = arch_register_cpu(pr->id);
>  	if (ret) {
> -		acpi_unmap_cpu(pr->id);
> +		acpi_processor_hotplug_unmap_cpu(pr);
>  		goto out;
>  	}
>