linux-kernel - Re: [PATCH v2] driver core: platform: set numa_node before platform_add

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2023091351-clarify-prototype-710c@gregkh>
Date:   Wed, 13 Sep 2023 18:37:03 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     guojinhui <guojinhui.liam@...edance.com>
Cc:     rafael@...nel.org, lenb@...nel.org, lizefan.x@...edance.com,
        linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel test robot <lkp@...el.com>
Subject: Re: [PATCH v2] driver core: platform: set numa_node before
 platform_add_device()

On Wed, Sep 13, 2023 at 04:38:25PM +0800, guojinhui wrote:
> From: "guojinhui" <guojinhui.liam@...edance.com>
> 
> platform_add_device creates numa_node attribute of sysfs according to
> whether dev_to_node(dev) is equal to NUMA_NO_NODE. So set the numa node
> of the device before creating numa_node attribute of sysfs.
> 
> Fixes: 4a60406d3592 ("driver core: platform: expose numa_node to users in sysfs")
> Reported-by: kernel test robot <lkp@...el.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202309122309.mbxAnAIe-lkp@intel.com/
> Signed-off-by: guojinhui <guojinhui.liam@...edance.com>
> ---
>  drivers/acpi/acpi_platform.c |  4 +---
>  drivers/base/platform.c      | 13 +++++++++++++
>  2 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c
> index 48d15dd785f6..adcbfbdc343f 100644
> --- a/drivers/acpi/acpi_platform.c
> +++ b/drivers/acpi/acpi_platform.c
> @@ -178,11 +178,9 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev,
>  	if (IS_ERR(pdev))
>  		dev_err(&adev->dev, "platform device creation failed: %ld\n",
>  			PTR_ERR(pdev));
> -	else {
> -		set_dev_node(&pdev->dev, acpi_get_node(adev->handle));
> +	else
>  		dev_dbg(&adev->dev, "created platform device %s\n",
>  			dev_name(&pdev->dev));
> -	}
>  
>  	kfree(resources);
>  
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 76bfcba25003..206dc7b020cd 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -795,6 +795,18 @@ void platform_device_unregister(struct platform_device *pdev)
>  }
>  EXPORT_SYMBOL_GPL(platform_device_unregister);
>  
> +#ifdef CONFIG_ACPI

#ifdef do not belong in .c files if at all possible.

Why can't this be an acpi call instead?  Why does this have to be in the
driver core?  Platform drivers shouldn't know anything about acpi, this
feels really odd.

> +static inline void platform_set_dev_node(struct platform_device *pdev)

Also, it's not "platform_set", it is acpi-specifc, right?  Again, the
ACPI core should handle this for its ACPI-platform devices, the driver
core shouldn't care at all.

thanks,

greg k-h