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.
Signed-off-by: Slawomir Rosek <srosek@google.com>
---
drivers/acpi/acpi_platform.c | 27 ++++++++++++++++++++
drivers/platform/x86/intel/hid.c | 41 +------------------------------
drivers/platform/x86/intel/vbtn.c | 30 +---------------------
include/linux/platform_device.h | 17 +++++++++++++
4 files changed, 46 insertions(+), 69 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);
+
+ 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..e2e0fc95e177 100644
--- a/drivers/platform/x86/intel/hid.c
+++ b/drivers/platform/x86/intel/hid.c
@@ -766,43 +766,4 @@ static struct platform_driver intel_hid_pl_driver = {
.remove = intel_hid_remove,
};
-/*
- * Unfortunately, some laptops provide a _HID="INT33D5" device with
- * _CID="PNP0C02". This causes the pnpacpi scan driver to claim the
- * ACPI node, so no platform device will be created. The pnpacpi
- * driver rejects this device in subsequent processing, so no physical
- * node is created at all.
- *
- * 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.
- */
-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);
-
- 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);
-
- 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 *);
+
/* 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
--
2.51.0.318.gd7df087d1a-googHi Slawomir,
On 30-Aug-25 7:34 AM, Slawomir Rosek 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.
>
> Signed-off-by: Slawomir Rosek <srosek@google.com>
Thank you for your interesting patch.
> ---
> drivers/acpi/acpi_platform.c | 27 ++++++++++++++++++++
> drivers/platform/x86/intel/hid.c | 41 +------------------------------
> drivers/platform/x86/intel/vbtn.c | 30 +---------------------
> include/linux/platform_device.h | 17 +++++++++++++
> 4 files changed, 46 insertions(+), 69 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);
> +
> + 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..e2e0fc95e177 100644
> --- a/drivers/platform/x86/intel/hid.c
> +++ b/drivers/platform/x86/intel/hid.c
> @@ -766,43 +766,4 @@ static struct platform_driver intel_hid_pl_driver = {
> .remove = intel_hid_remove,
> };
>
> -/*
> - * Unfortunately, some laptops provide a _HID="INT33D5" device with
> - * _CID="PNP0C02". This causes the pnpacpi scan driver to claim the
> - * ACPI node, so no platform device will be created. The pnpacpi
> - * driver rejects this device in subsequent processing, so no physical
> - * node is created at all.
> - *
> - * 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.
> - */
This comment contains useful info, please preserve the comment changing
the last paragraph to:
* 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 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);
> -
> - 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
...
> -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);
Too bad there is no comment here. I wonder if this is necessary
at all, or if this was just copy & pasted from the intel/hid.c
driver.
git blame is not really helpful here, the acpi_walk_namespace()
was added in 332e081225fc2 ("intel-vbtn: new driver for Intel Virtual
Button").
So it looks like this is just copy paste and maybe a regular
module_platform_driver() will be sufficient here. But changing
behavior like that is out of scope for this patch-set, so please
keep using module_acpi_platform_driver()
Otherwise this looks good to me.
Regards,
Hans
Hi Hans,
First of all I would like to apologize for the late reply and thank
you for your comments.
On Sat, Aug 30, 2025 at 5:13 PM Hans de Goede <hansg@kernel.org> wrote:
>
> Hi Slawomir,
>
> On 30-Aug-25 7:34 AM, Slawomir Rosek 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.
> >
> > Signed-off-by: Slawomir Rosek <srosek@google.com>
>
> Thank you for your interesting patch.
>
> > ---
> > drivers/acpi/acpi_platform.c | 27 ++++++++++++++++++++
> > drivers/platform/x86/intel/hid.c | 41 +------------------------------
> > drivers/platform/x86/intel/vbtn.c | 30 +---------------------
> > include/linux/platform_device.h | 17 +++++++++++++
> > 4 files changed, 46 insertions(+), 69 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);
> > +
> > + 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..e2e0fc95e177 100644
> > --- a/drivers/platform/x86/intel/hid.c
> > +++ b/drivers/platform/x86/intel/hid.c
> > @@ -766,43 +766,4 @@ static struct platform_driver intel_hid_pl_driver = {
> > .remove = intel_hid_remove,
> > };
> >
> > -/*
> > - * Unfortunately, some laptops provide a _HID="INT33D5" device with
> > - * _CID="PNP0C02". This causes the pnpacpi scan driver to claim the
> > - * ACPI node, so no platform device will be created. The pnpacpi
> > - * driver rejects this device in subsequent processing, so no physical
> > - * node is created at all.
> > - *
> > - * 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.
> > - */
>
> This comment contains useful info, please preserve the comment changing
> the last paragraph to:
>
> * 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 by using module_acpi_platform_driver()
> * instead of the regular module_platform_driver().
>
Good point. I shouldn't have deleted the original comment. I will restore and
rephrase it according to your suggestions.
> > -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);
> > -
> > - 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
>
> ...
>
> > -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);
>
> Too bad there is no comment here. I wonder if this is necessary
> at all, or if this was just copy & pasted from the intel/hid.c
> driver.
>
> git blame is not really helpful here, the acpi_walk_namespace()
> was added in 332e081225fc2 ("intel-vbtn: new driver for Intel Virtual
> Button").
>
> So it looks like this is just copy paste and maybe a regular
> module_platform_driver() will be sufficient here. But changing
> behavior like that is out of scope for this patch-set, so please
> keep using module_acpi_platform_driver()
>
Changing vbtn driver behaviour is definitely out of the scope for this
patch-set but you may be right about copy-paste. Should I update
the commit message and mention about it?
> Otherwise this looks good to me.
>
> Regards,
>
> Hans
>
>
Thanks, Regards,
Slawek
© 2016 - 2025 Red Hat, Inc.