| 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
| ||
|
Message-ID: <CAJZ5v0hGKHJKMXPLKKGUntDok8SSm0=TGYqHFkCxXBxkygfK7w@mail.gmail.com>
Date: Wed, 22 Oct 2025 20:58:49 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Slawomir Rosek <srosek@...gle.com>
Cc: "Rafael J . Wysocki" <rafael@...nel.org>, Alex Hung <alexhung@...il.com>,
Hans de Goede <hansg@...nel.org>, Ilpo Jarvinen <ilpo.jarvinen@...ux.intel.com>,
AceLan Kao <acelan.kao@...onical.com>, Daniel Lezcano <daniel.lezcano@...aro.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Zhang Rui <rui.zhang@...el.com>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>, Tomasz Nowicki <tnowicki@...gle.com>,
Stanislaw Kardach <skardach@...gle.com>, Michal Krawczyk <mikrawczyk@...gle.com>,
linux-kernel@...r.kernel.org, linux-acpi@...r.kernel.org,
platform-driver-x86@...r.kernel.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH v3 5/6] ACPI: platform: Add macro for acpi platform driver
On Thu, Oct 2, 2025 at 1:34 PM Slawomir Rosek <srosek@...gle.com> wrote:
>
> Introduce module_acpi_platform_driver() macro to simplify dynamic
> enumeration of ACPI device objects on the platform bus by loadable
> modules. Move common code from the intel-hid and intel-vbtn drivers
> to the ACPI platform core.
I think that this goes the other way around: common code is moved from
intel-hid and intel-vbtn to acpi_platform and the new macro is
introduced as a way for the drivers to invoke that code.
> Signed-off-by: Slawomir Rosek <srosek@...gle.com>
Overall, it looks like this patch does not depend on the [1-4/6] in this series.
> ---
> drivers/acpi/acpi_platform.c | 27 +++++++++++++++++++++++++
> drivers/platform/x86/intel/hid.c | 33 +++----------------------------
> drivers/platform/x86/intel/vbtn.c | 30 +---------------------------
> include/linux/platform_device.h | 17 ++++++++++++++++
> 4 files changed, 48 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c
> index 48d15dd785f6..adf32ffa6be6 100644
> --- a/drivers/acpi/acpi_platform.c
> +++ b/drivers/acpi/acpi_platform.c
> @@ -190,6 +190,33 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev,
> }
> EXPORT_SYMBOL_GPL(acpi_create_platform_device);
>
> +static acpi_status
> +__acpi_platform_driver_register_cb(acpi_handle handle, u32 lvl,
> + void *context, void **rv)
> +{
> + const struct acpi_device_id *ids = context;
> + struct acpi_device *dev = acpi_fetch_acpi_dev(handle);
> +
> + if (dev && acpi_match_device_ids(dev, ids) == 0)
> + if (!IS_ERR_OR_NULL(acpi_create_platform_device(dev, NULL))) {
> + dev_info(&dev->dev,
> + "created platform device\n");
> + }
> +
> + return AE_OK;
> +}
> +
> +int __acpi_platform_driver_register(struct platform_driver *drv,
> + struct module *owner)
> +{
> + acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, ACPI_UINT32_MAX,
> + __acpi_platform_driver_register_cb, NULL,
> + (void *)drv->driver.acpi_match_table, NULL);
Why is this needed here?
> +
> + return __platform_driver_register(drv, owner);
> +}
> +EXPORT_SYMBOL_GPL(__acpi_platform_driver_register);
> +
> void __init acpi_platform_init(void)
> {
> acpi_reconfig_notifier_register(&acpi_platform_notifier);
> diff --git a/drivers/platform/x86/intel/hid.c b/drivers/platform/x86/intel/hid.c
> index f25a427cccda..37f990686446 100644
> --- a/drivers/platform/x86/intel/hid.c
> +++ b/drivers/platform/x86/intel/hid.c
> @@ -775,34 +775,7 @@ static struct platform_driver intel_hid_pl_driver = {
> *
> * As a workaround until the ACPI core figures out how to handle
> * this corner case, manually ask the ACPI platform device code to
> - * claim the ACPI node.
> + * claim the ACPI node by using module_acpi_platform_driver()
> + * instead of the regular module_platform_driver().
> */
> -static acpi_status __init
> -check_acpi_dev(acpi_handle handle, u32 lvl, void *context, void **rv)
> -{
> - const struct acpi_device_id *ids = context;
> - struct acpi_device *dev = acpi_fetch_acpi_dev(handle);
> -
> - if (dev && acpi_match_device_ids(dev, ids) == 0)
> - if (!IS_ERR_OR_NULL(acpi_create_platform_device(dev, NULL)))
> - dev_info(&dev->dev,
> - "intel-hid: created platform device\n");
> -
> - return AE_OK;
> -}
> -
> -static int __init intel_hid_init(void)
> -{
> - acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
> - ACPI_UINT32_MAX, check_acpi_dev, NULL,
> - (void *)intel_hid_ids, NULL);
This actually isn't right and should not be perpetuated.
Platform devices are created for ACPI device objects with ACPI device
IDs by acpi_default_enumeration(), so it should not be necessary to do
that from a driver.
> -
> - return platform_driver_register(&intel_hid_pl_driver);
> -}
> -module_init(intel_hid_init);
> -
> -static void __exit intel_hid_exit(void)
> -{
> - platform_driver_unregister(&intel_hid_pl_driver);
> -}
> -module_exit(intel_hid_exit);
> +module_acpi_platform_driver(intel_hid_pl_driver);
> diff --git a/drivers/platform/x86/intel/vbtn.c b/drivers/platform/x86/intel/vbtn.c
> index 232cd12e3c9f..42932479de35 100644
> --- a/drivers/platform/x86/intel/vbtn.c
> +++ b/drivers/platform/x86/intel/vbtn.c
> @@ -390,32 +390,4 @@ static struct platform_driver intel_vbtn_pl_driver = {
> .remove = intel_vbtn_remove,
> };
>
> -static acpi_status __init
> -check_acpi_dev(acpi_handle handle, u32 lvl, void *context, void **rv)
> -{
> - const struct acpi_device_id *ids = context;
> - struct acpi_device *dev = acpi_fetch_acpi_dev(handle);
> -
> - if (dev && acpi_match_device_ids(dev, ids) == 0)
> - if (!IS_ERR_OR_NULL(acpi_create_platform_device(dev, NULL)))
> - dev_info(&dev->dev,
> - "intel-vbtn: created platform device\n");
> -
> - return AE_OK;
> -}
> -
> -static int __init intel_vbtn_init(void)
> -{
> - acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
> - ACPI_UINT32_MAX, check_acpi_dev, NULL,
> - (void *)intel_vbtn_ids, NULL);
Same thing here.
> -
> - return platform_driver_register(&intel_vbtn_pl_driver);
> -}
> -module_init(intel_vbtn_init);
> -
> -static void __exit intel_vbtn_exit(void)
> -{
> - platform_driver_unregister(&intel_vbtn_pl_driver);
> -}
> -module_exit(intel_vbtn_exit);
> +module_acpi_platform_driver(intel_vbtn_pl_driver);
> diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
> index 074754c23d33..3b70b054d8a5 100644
> --- a/include/linux/platform_device.h
> +++ b/include/linux/platform_device.h
> @@ -264,6 +264,14 @@ extern int __platform_driver_register(struct platform_driver *,
> struct module *);
> extern void platform_driver_unregister(struct platform_driver *);
>
> +/*
> + * use a macro to avoid include chaining to get THIS_MODULE
> + */
> +#define acpi_platform_driver_register(drv) \
> + __acpi_platform_driver_register(drv, THIS_MODULE)
> +extern int __acpi_platform_driver_register(struct platform_driver *,
> + struct module *);
If "extern" is not necessary above, please remove it.
> +
> /* non-hotpluggable platform devices may use this so that probe() and
> * its support may live in __init sections, conserving runtime memory.
> */
> @@ -292,6 +300,15 @@ static inline void platform_set_drvdata(struct platform_device *pdev,
> module_driver(__platform_driver, platform_driver_register, \
> platform_driver_unregister)
>
> +/* module_acpi_platform_driver() - Helper macro for drivers that don't do
> + * anything special in module init/exit. This eliminates a lot of
> + * boilerplate. Each module may only use this macro once, and
> + * calling it replaces module_init() and module_exit()
> + */
> +#define module_acpi_platform_driver(__platform_driver) \
> + module_driver(__platform_driver, acpi_platform_driver_register, \
> + platform_driver_unregister)
> +
> /* builtin_platform_driver() - Helper macro for builtin drivers that
> * don't do anything special in driver init. This eliminates some
> * boilerplate. Each driver may only use this macro once, and
> --
Powered by blists - more mailing lists