From: "guojinhui" <[email protected]>
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 <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
Signed-off-by: guojinhui <[email protected]>
---
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
+static inline void platform_set_dev_node(struct platform_device *pdev)
+{
+ struct acpi_device *adev = to_acpi_device_node(pdev->dev.fwnode);
+
+ if (adev && adev->handle)
+ set_dev_node(&pdev->dev, acpi_get_node(adev->handle));
+}
+#else
+static inline void platform_set_dev_node(struct platform_device *pdev) {}
+#endif
+
/**
* platform_device_register_full - add a platform-level device with
* resources and platform-specific data
@@ -841,6 +853,7 @@ struct platform_device *platform_device_register_full(
goto err;
}
+ platform_set_dev_node(pdev);
ret = platform_device_add(pdev);
if (ret) {
err:
--
2.20.1
On Wed, Sep 13, 2023 at 04:38:25PM +0800, guojinhui wrote:
> From: "guojinhui" <[email protected]>
>
> 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 <[email protected]>
> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> Signed-off-by: guojinhui <[email protected]>
> ---
> 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
On Wed, 13 Sep 2023 06:37:03PM +0200, Greg KH wrote:
> On Wed, Sep 13, 2023 at 04:38:25PM +0800, guojinhui wrote:
> > From: "guojinhui" <[email protected]>
> >
> > 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 <[email protected]>
> > Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> > Signed-off-by: guojinhui <[email protected]>
> > ---
> > 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
>
I agree with that. I will try to fix it in the ACPI code which call the platform function
soon.
thanks,
Jinhui Guo