* [PATCH v1] platform/x86: thinkpad_acpi: Drop ACPI driver registration
@ 2026-03-24 20:08 Rafael J. Wysocki
2026-03-28 1:29 ` Mark Pearson
2026-04-07 10:21 ` Ilpo Järvinen
0 siblings, 2 replies; 4+ messages in thread
From: Rafael J. Wysocki @ 2026-03-24 20:08 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Hans de Goede, LKML, Linux ACPI, platform-driver-x86,
Henrique de Moraes Holschuh, Mark Pearson, Derek J. Clark,
ibm-acpi-devel
From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
There is no point in registering an ACPI driver that only has an empty
.add() callback, which is done by the thinkpad_acpi driver, since
after binding to an ACPI device it only sits there and does nothing.
That binding only effectively causes the ACPI device's reference count
to increase, but that can be achieved by using acpi_get_acpi_dev()
instead of acpi_fetch_acpi_dev() in setup_acpi_notify(), and doing
the corresponding cleanup in ibm_exit().
Update the code accordingly and get rid of the non-functional ACPI
driver.
No intentional functional impact beyond altering sysfs content.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/platform/x86/lenovo/thinkpad_acpi.c | 62 ++-------------------
1 file changed, 4 insertions(+), 58 deletions(-)
diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
index 8982d92dfd97..9e1614754cd7 100644
--- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
+++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
@@ -299,7 +299,6 @@ struct ibm_struct;
struct tp_acpi_drv_struct {
const struct acpi_device_id *hid;
- struct acpi_driver *driver;
void (*notify) (struct ibm_struct *, u32);
acpi_handle *handle;
@@ -322,7 +321,6 @@ struct ibm_struct {
struct tp_acpi_drv_struct *acpi;
struct {
- u8 acpi_driver_registered:1;
u8 acpi_notify_installed:1;
u8 proc_created:1;
u8 init_called:1;
@@ -832,9 +830,9 @@ static int __init setup_acpi_notify(struct ibm_struct *ibm)
vdbg_printk(TPACPI_DBG_INIT,
"setting up ACPI notify for %s\n", ibm->name);
- ibm->acpi->device = acpi_fetch_acpi_dev(*ibm->acpi->handle);
+ ibm->acpi->device = acpi_get_acpi_dev(*ibm->acpi->handle);
if (!ibm->acpi->device) {
- pr_err("acpi_fetch_acpi_dev(%s) failed\n", ibm->name);
+ pr_err("acpi_get_acpi_dev(%s) failed\n", ibm->name);
return -ENODEV;
}
@@ -859,44 +857,6 @@ static int __init setup_acpi_notify(struct ibm_struct *ibm)
return 0;
}
-static int __init tpacpi_device_add(struct acpi_device *device)
-{
- return 0;
-}
-
-static int __init register_tpacpi_subdriver(struct ibm_struct *ibm)
-{
- int rc;
-
- dbg_printk(TPACPI_DBG_INIT,
- "registering %s as an ACPI driver\n", ibm->name);
-
- BUG_ON(!ibm->acpi);
-
- ibm->acpi->driver = kzalloc_obj(struct acpi_driver);
- if (!ibm->acpi->driver) {
- pr_err("failed to allocate memory for ibm->acpi->driver\n");
- return -ENOMEM;
- }
-
- sprintf(ibm->acpi->driver->name, "%s_%s", TPACPI_NAME, ibm->name);
- ibm->acpi->driver->ids = ibm->acpi->hid;
-
- ibm->acpi->driver->ops.add = &tpacpi_device_add;
-
- rc = acpi_bus_register_driver(ibm->acpi->driver);
- if (rc < 0) {
- pr_err("acpi_bus_register_driver(%s) failed: %d\n",
- ibm->name, rc);
- kfree(ibm->acpi->driver);
- ibm->acpi->driver = NULL;
- } else if (!rc)
- ibm->flags.acpi_driver_registered = 1;
-
- return rc;
-}
-
-
/****************************************************************************
****************************************************************************
*
@@ -11532,6 +11492,8 @@ static void ibm_exit(struct ibm_struct *ibm)
acpi_remove_notify_handler(*ibm->acpi->handle,
ibm->acpi->type,
dispatch_acpi_notify);
+ ibm->acpi->device->driver_data = NULL;
+ acpi_dev_put(ibm->acpi->device);
ibm->flags.acpi_notify_installed = 0;
}
@@ -11542,16 +11504,6 @@ static void ibm_exit(struct ibm_struct *ibm)
ibm->flags.proc_created = 0;
}
- if (ibm->flags.acpi_driver_registered) {
- dbg_printk(TPACPI_DBG_EXIT,
- "%s: acpi_bus_unregister_driver\n", ibm->name);
- BUG_ON(!ibm->acpi);
- acpi_bus_unregister_driver(ibm->acpi->driver);
- kfree(ibm->acpi->driver);
- ibm->acpi->driver = NULL;
- ibm->flags.acpi_driver_registered = 0;
- }
-
if (ibm->flags.init_called && ibm->exit) {
ibm->exit();
ibm->flags.init_called = 0;
@@ -11587,12 +11539,6 @@ static int __init ibm_init(struct ibm_init_struct *iibm)
}
if (ibm->acpi) {
- if (ibm->acpi->hid) {
- ret = register_tpacpi_subdriver(ibm);
- if (ret)
- goto err_out;
- }
-
if (ibm->acpi->notify) {
ret = setup_acpi_notify(ibm);
if (ret == -ENODEV) {
--
2.51.0
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v1] platform/x86: thinkpad_acpi: Drop ACPI driver registration
2026-03-24 20:08 [PATCH v1] platform/x86: thinkpad_acpi: Drop ACPI driver registration Rafael J. Wysocki
@ 2026-03-28 1:29 ` Mark Pearson
2026-03-28 11:59 ` Rafael J. Wysocki
2026-04-07 10:21 ` Ilpo Järvinen
1 sibling, 1 reply; 4+ messages in thread
From: Mark Pearson @ 2026-03-28 1:29 UTC (permalink / raw)
To: Rafael J. Wysocki, Ilpo Järvinen
Cc: Hans de Goede, LKML, linux-acpi, platform-driver-x86,
Henrique de Moraes Holschuh, Derek J . Clark, ibm-acpi-devel
Hi Rafael
On Tue, Mar 24, 2026, at 4:08 PM, Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
>
> There is no point in registering an ACPI driver that only has an empty
> .add() callback, which is done by the thinkpad_acpi driver, since
> after binding to an ACPI device it only sits there and does nothing.
>
> That binding only effectively causes the ACPI device's reference count
> to increase, but that can be achieved by using acpi_get_acpi_dev()
> instead of acpi_fetch_acpi_dev() in setup_acpi_notify(), and doing
> the corresponding cleanup in ibm_exit().
>
> Update the code accordingly and get rid of the non-functional ACPI
> driver.
>
> No intentional functional impact beyond altering sysfs content.
Just curious - where would I see changes?
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/platform/x86/lenovo/thinkpad_acpi.c | 62 ++-------------------
> 1 file changed, 4 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c
> b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> index 8982d92dfd97..9e1614754cd7 100644
> --- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
> +++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> @@ -299,7 +299,6 @@ struct ibm_struct;
>
> struct tp_acpi_drv_struct {
> const struct acpi_device_id *hid;
> - struct acpi_driver *driver;
>
> void (*notify) (struct ibm_struct *, u32);
> acpi_handle *handle;
> @@ -322,7 +321,6 @@ struct ibm_struct {
> struct tp_acpi_drv_struct *acpi;
>
> struct {
> - u8 acpi_driver_registered:1;
> u8 acpi_notify_installed:1;
> u8 proc_created:1;
> u8 init_called:1;
> @@ -832,9 +830,9 @@ static int __init setup_acpi_notify(struct ibm_struct *ibm)
> vdbg_printk(TPACPI_DBG_INIT,
> "setting up ACPI notify for %s\n", ibm->name);
>
> - ibm->acpi->device = acpi_fetch_acpi_dev(*ibm->acpi->handle);
> + ibm->acpi->device = acpi_get_acpi_dev(*ibm->acpi->handle);
> if (!ibm->acpi->device) {
> - pr_err("acpi_fetch_acpi_dev(%s) failed\n", ibm->name);
> + pr_err("acpi_get_acpi_dev(%s) failed\n", ibm->name);
> return -ENODEV;
> }
>
> @@ -859,44 +857,6 @@ static int __init setup_acpi_notify(struct ibm_struct *ibm)
> return 0;
> }
>
> -static int __init tpacpi_device_add(struct acpi_device *device)
> -{
> - return 0;
> -}
> -
> -static int __init register_tpacpi_subdriver(struct ibm_struct *ibm)
> -{
> - int rc;
> -
> - dbg_printk(TPACPI_DBG_INIT,
> - "registering %s as an ACPI driver\n", ibm->name);
> -
> - BUG_ON(!ibm->acpi);
> -
> - ibm->acpi->driver = kzalloc_obj(struct acpi_driver);
> - if (!ibm->acpi->driver) {
> - pr_err("failed to allocate memory for ibm->acpi->driver\n");
> - return -ENOMEM;
> - }
> -
> - sprintf(ibm->acpi->driver->name, "%s_%s", TPACPI_NAME, ibm->name);
> - ibm->acpi->driver->ids = ibm->acpi->hid;
> -
> - ibm->acpi->driver->ops.add = &tpacpi_device_add;
> -
> - rc = acpi_bus_register_driver(ibm->acpi->driver);
> - if (rc < 0) {
> - pr_err("acpi_bus_register_driver(%s) failed: %d\n",
> - ibm->name, rc);
> - kfree(ibm->acpi->driver);
> - ibm->acpi->driver = NULL;
> - } else if (!rc)
> - ibm->flags.acpi_driver_registered = 1;
> -
> - return rc;
> -}
> -
> -
> /****************************************************************************
> ****************************************************************************
> *
> @@ -11532,6 +11492,8 @@ static void ibm_exit(struct ibm_struct *ibm)
> acpi_remove_notify_handler(*ibm->acpi->handle,
> ibm->acpi->type,
> dispatch_acpi_notify);
> + ibm->acpi->device->driver_data = NULL;
> + acpi_dev_put(ibm->acpi->device);
> ibm->flags.acpi_notify_installed = 0;
> }
>
> @@ -11542,16 +11504,6 @@ static void ibm_exit(struct ibm_struct *ibm)
> ibm->flags.proc_created = 0;
> }
>
> - if (ibm->flags.acpi_driver_registered) {
> - dbg_printk(TPACPI_DBG_EXIT,
> - "%s: acpi_bus_unregister_driver\n", ibm->name);
> - BUG_ON(!ibm->acpi);
> - acpi_bus_unregister_driver(ibm->acpi->driver);
> - kfree(ibm->acpi->driver);
> - ibm->acpi->driver = NULL;
> - ibm->flags.acpi_driver_registered = 0;
> - }
> -
> if (ibm->flags.init_called && ibm->exit) {
> ibm->exit();
> ibm->flags.init_called = 0;
> @@ -11587,12 +11539,6 @@ static int __init ibm_init(struct
> ibm_init_struct *iibm)
> }
>
> if (ibm->acpi) {
> - if (ibm->acpi->hid) {
> - ret = register_tpacpi_subdriver(ibm);
> - if (ret)
> - goto err_out;
> - }
> -
> if (ibm->acpi->notify) {
> ret = setup_acpi_notify(ibm);
> if (ret == -ENODEV) {
> --
> 2.51.0
Changes seem good to me (but not an expert).
I did try them out on a system (X1 Carbon 13) and didn't find any problems. Let me know if there's anything in particular I should look out for or check and happy to do that.
Tested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Mark
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v1] platform/x86: thinkpad_acpi: Drop ACPI driver registration
2026-03-28 1:29 ` Mark Pearson
@ 2026-03-28 11:59 ` Rafael J. Wysocki
0 siblings, 0 replies; 4+ messages in thread
From: Rafael J. Wysocki @ 2026-03-28 11:59 UTC (permalink / raw)
To: Mark Pearson
Cc: Rafael J. Wysocki, Ilpo Järvinen, Hans de Goede, LKML,
linux-acpi, platform-driver-x86, Henrique de Moraes Holschuh,
Derek J . Clark, ibm-acpi-devel
On Sat, Mar 28, 2026 at 2:29 AM Mark Pearson <mpearson-lenovo@squebb.ca> wrote:
>
> Hi Rafael
>
> On Tue, Mar 24, 2026, at 4:08 PM, Rafael J. Wysocki wrote:
> > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> >
> > There is no point in registering an ACPI driver that only has an empty
> > .add() callback, which is done by the thinkpad_acpi driver, since
> > after binding to an ACPI device it only sits there and does nothing.
> >
> > That binding only effectively causes the ACPI device's reference count
> > to increase, but that can be achieved by using acpi_get_acpi_dev()
> > instead of acpi_fetch_acpi_dev() in setup_acpi_notify(), and doing
> > the corresponding cleanup in ibm_exit().
> >
> > Update the code accordingly and get rid of the non-functional ACPI
> > driver.
> >
> > No intentional functional impact beyond altering sysfs content.
>
> Just curious - where would I see changes?
In /sys/bus/acpi/drivers/ - no thinkpad_* any more (quite obviously)
and no "driver" link in the sysfs directory of the ACPI device the
driver used to bind to. That should be it IIRC.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> > drivers/platform/x86/lenovo/thinkpad_acpi.c | 62 ++-------------------
> > 1 file changed, 4 insertions(+), 58 deletions(-)
> >
> > diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c
> > b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> > index 8982d92dfd97..9e1614754cd7 100644
> > --- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
> > +++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> > @@ -299,7 +299,6 @@ struct ibm_struct;
> >
> > struct tp_acpi_drv_struct {
> > const struct acpi_device_id *hid;
> > - struct acpi_driver *driver;
> >
> > void (*notify) (struct ibm_struct *, u32);
> > acpi_handle *handle;
> > @@ -322,7 +321,6 @@ struct ibm_struct {
> > struct tp_acpi_drv_struct *acpi;
> >
> > struct {
> > - u8 acpi_driver_registered:1;
> > u8 acpi_notify_installed:1;
> > u8 proc_created:1;
> > u8 init_called:1;
> > @@ -832,9 +830,9 @@ static int __init setup_acpi_notify(struct ibm_struct *ibm)
> > vdbg_printk(TPACPI_DBG_INIT,
> > "setting up ACPI notify for %s\n", ibm->name);
> >
> > - ibm->acpi->device = acpi_fetch_acpi_dev(*ibm->acpi->handle);
> > + ibm->acpi->device = acpi_get_acpi_dev(*ibm->acpi->handle);
> > if (!ibm->acpi->device) {
> > - pr_err("acpi_fetch_acpi_dev(%s) failed\n", ibm->name);
> > + pr_err("acpi_get_acpi_dev(%s) failed\n", ibm->name);
> > return -ENODEV;
> > }
> >
> > @@ -859,44 +857,6 @@ static int __init setup_acpi_notify(struct ibm_struct *ibm)
> > return 0;
> > }
> >
> > -static int __init tpacpi_device_add(struct acpi_device *device)
> > -{
> > - return 0;
> > -}
> > -
> > -static int __init register_tpacpi_subdriver(struct ibm_struct *ibm)
> > -{
> > - int rc;
> > -
> > - dbg_printk(TPACPI_DBG_INIT,
> > - "registering %s as an ACPI driver\n", ibm->name);
> > -
> > - BUG_ON(!ibm->acpi);
> > -
> > - ibm->acpi->driver = kzalloc_obj(struct acpi_driver);
> > - if (!ibm->acpi->driver) {
> > - pr_err("failed to allocate memory for ibm->acpi->driver\n");
> > - return -ENOMEM;
> > - }
> > -
> > - sprintf(ibm->acpi->driver->name, "%s_%s", TPACPI_NAME, ibm->name);
> > - ibm->acpi->driver->ids = ibm->acpi->hid;
> > -
> > - ibm->acpi->driver->ops.add = &tpacpi_device_add;
> > -
> > - rc = acpi_bus_register_driver(ibm->acpi->driver);
> > - if (rc < 0) {
> > - pr_err("acpi_bus_register_driver(%s) failed: %d\n",
> > - ibm->name, rc);
> > - kfree(ibm->acpi->driver);
> > - ibm->acpi->driver = NULL;
> > - } else if (!rc)
> > - ibm->flags.acpi_driver_registered = 1;
> > -
> > - return rc;
> > -}
> > -
> > -
> > /****************************************************************************
> > ****************************************************************************
> > *
> > @@ -11532,6 +11492,8 @@ static void ibm_exit(struct ibm_struct *ibm)
> > acpi_remove_notify_handler(*ibm->acpi->handle,
> > ibm->acpi->type,
> > dispatch_acpi_notify);
> > + ibm->acpi->device->driver_data = NULL;
> > + acpi_dev_put(ibm->acpi->device);
> > ibm->flags.acpi_notify_installed = 0;
> > }
> >
> > @@ -11542,16 +11504,6 @@ static void ibm_exit(struct ibm_struct *ibm)
> > ibm->flags.proc_created = 0;
> > }
> >
> > - if (ibm->flags.acpi_driver_registered) {
> > - dbg_printk(TPACPI_DBG_EXIT,
> > - "%s: acpi_bus_unregister_driver\n", ibm->name);
> > - BUG_ON(!ibm->acpi);
> > - acpi_bus_unregister_driver(ibm->acpi->driver);
> > - kfree(ibm->acpi->driver);
> > - ibm->acpi->driver = NULL;
> > - ibm->flags.acpi_driver_registered = 0;
> > - }
> > -
> > if (ibm->flags.init_called && ibm->exit) {
> > ibm->exit();
> > ibm->flags.init_called = 0;
> > @@ -11587,12 +11539,6 @@ static int __init ibm_init(struct
> > ibm_init_struct *iibm)
> > }
> >
> > if (ibm->acpi) {
> > - if (ibm->acpi->hid) {
> > - ret = register_tpacpi_subdriver(ibm);
> > - if (ret)
> > - goto err_out;
> > - }
> > -
> > if (ibm->acpi->notify) {
> > ret = setup_acpi_notify(ibm);
> > if (ret == -ENODEV) {
> > --
> > 2.51.0
>
> Changes seem good to me (but not an expert).
> I did try them out on a system (X1 Carbon 13) and didn't find any problems. Let me know if there's anything in particular I should look out for or check and happy to do that.
>
> Tested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Thank you!
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1] platform/x86: thinkpad_acpi: Drop ACPI driver registration
2026-03-24 20:08 [PATCH v1] platform/x86: thinkpad_acpi: Drop ACPI driver registration Rafael J. Wysocki
2026-03-28 1:29 ` Mark Pearson
@ 2026-04-07 10:21 ` Ilpo Järvinen
1 sibling, 0 replies; 4+ messages in thread
From: Ilpo Järvinen @ 2026-04-07 10:21 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Hans de Goede, LKML, Linux ACPI, platform-driver-x86,
Henrique de Moraes Holschuh, Mark Pearson, Derek J. Clark,
ibm-acpi-devel
On Tue, 24 Mar 2026 21:08:01 +0100, Rafael J. Wysocki wrote:
> There is no point in registering an ACPI driver that only has an empty
> .add() callback, which is done by the thinkpad_acpi driver, since
> after binding to an ACPI device it only sits there and does nothing.
>
> That binding only effectively causes the ACPI device's reference count
> to increase, but that can be achieved by using acpi_get_acpi_dev()
> instead of acpi_fetch_acpi_dev() in setup_acpi_notify(), and doing
> the corresponding cleanup in ibm_exit().
>
> [...]
Thank you for your contribution, it has been applied to my local
review-ilpo-next branch. Note it will show up in the public
platform-drivers-x86/review-ilpo-next branch only once I've pushed my
local branch there, which might take a while.
The list of commits applied:
[1/1] platform/x86: thinkpad_acpi: Drop ACPI driver registration
commit: 955165c3e537668b9a5a6eb26397281b88143002
--
i.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-04-07 10:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-24 20:08 [PATCH v1] platform/x86: thinkpad_acpi: Drop ACPI driver registration Rafael J. Wysocki
2026-03-28 1:29 ` Mark Pearson
2026-03-28 11:59 ` Rafael J. Wysocki
2026-04-07 10:21 ` Ilpo Järvinen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®