From 9d9f749b316ac21cb59ad3e595cbce469b409e1a Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Tue, 28 Mar 2006 17:04:00 -0500 Subject: ACPI: make acpi_bus_register_driver() return success/failure, not device count acpi_bus_register_driver() should not return the number of devices claimed. We're not asking to find devices, we're making a driver available to devices, including hot-pluggable devices that may appear in the future. I audited all callers of acpi_bus_register_driver(), and except asus_acpi.c and sonypi.c (fixed in previous patches), all either ignore the return value or test only for failure (<0). Signed-off-by: Bjorn Helgaas Signed-off-by: Andrew Morton Signed-off-by: Len Brown --- drivers/acpi/scan.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index a0ab828b2cc..669553553fb 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -548,10 +548,9 @@ static int acpi_start_single_object(struct acpi_device *device) return_VALUE(result); } -static int acpi_driver_attach(struct acpi_driver *drv) +static void acpi_driver_attach(struct acpi_driver *drv) { struct list_head *node, *next; - int count = 0; ACPI_FUNCTION_TRACE("acpi_driver_attach"); @@ -568,7 +567,6 @@ static int acpi_driver_attach(struct acpi_driver *drv) if (!acpi_bus_driver_init(dev, drv)) { acpi_start_single_object(dev); atomic_inc(&drv->references); - count++; ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found driver [%s] for device [%s]\n", drv->name, dev->pnp.bus_id)); @@ -577,7 +575,6 @@ static int acpi_driver_attach(struct acpi_driver *drv) spin_lock(&acpi_device_lock); } spin_unlock(&acpi_device_lock); - return_VALUE(count); } static int acpi_driver_detach(struct acpi_driver *drv) @@ -610,14 +607,11 @@ static int acpi_driver_detach(struct acpi_driver *drv) * @driver: driver being registered * * Registers a driver with the ACPI bus. Searches the namespace for all - * devices that match the driver's criteria and binds. Returns the - * number of devices that were claimed by the driver, or a negative - * error status for failure. + * devices that match the driver's criteria and binds. Returns zero for + * success or a negative error status for failure. */ int acpi_bus_register_driver(struct acpi_driver *driver) { - int count; - ACPI_FUNCTION_TRACE("acpi_bus_register_driver"); if (acpi_disabled) @@ -629,9 +623,9 @@ int acpi_bus_register_driver(struct acpi_driver *driver) spin_lock(&acpi_device_lock); list_add_tail(&driver->node, &acpi_bus_drivers); spin_unlock(&acpi_device_lock); - count = acpi_driver_attach(driver); + acpi_driver_attach(driver); - return_VALUE(count); + return_VALUE(0); } EXPORT_SYMBOL(acpi_bus_register_driver); -- cgit v1.2.3 From 578b333bfe8eb1360207a08a53c321822a8f40f3 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Tue, 28 Mar 2006 17:04:00 -0500 Subject: ACPI: update asus_acpi driver registration to unload on failure Remove the assumption that acpi_bus_register_driver() returns the number of devices claimed. Returning the count is unreliable because devices may be hot-plugged in the future (admittedly not applicable for this driver). Since the hardware for this driver is not hot-pluggable, determine whether the hardware is present by noticing calls to the .add() method. It would be better to probe the ACPI namespace for the ASUS HIDs, and load the driver only when we find one, but ACPI doesn't support that yet. I don't have an ASUS laptop to test on, but on my HP dl360, it does report the appropriate error when attempting to load the module: $ sudo insmod drivers/acpi/asus_acpi.ko insmod: error inserting 'drivers/acpi/asus_acpi.ko': -1 No such device Signed-off-by: Bjorn Helgaas Signed-off-by: Andrew Morton Signed-off-by: Len Brown --- drivers/acpi/asus_acpi.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/drivers/acpi/asus_acpi.c b/drivers/acpi/asus_acpi.c index f4c87750dbf..523fff9cf14 100644 --- a/drivers/acpi/asus_acpi.c +++ b/drivers/acpi/asus_acpi.c @@ -1119,6 +1119,8 @@ static int __init asus_hotk_check(void) return result; } +static int asus_hotk_found; + static int __init asus_hotk_add(struct acpi_device *device) { acpi_status status = AE_OK; @@ -1180,6 +1182,8 @@ static int __init asus_hotk_add(struct acpi_device *device) } } + asus_hotk_found = 1; + end: if (result) { kfree(hotk); @@ -1226,7 +1230,19 @@ static int __init asus_acpi_init(void) asus_proc_dir->owner = THIS_MODULE; result = acpi_bus_register_driver(&asus_hotk_driver); - if (result < 1) { + if (result < 0) { + remove_proc_entry(PROC_ASUS, acpi_root_dir); + return -ENODEV; + } + + /* + * This is a bit of a kludge. We only want this module loaded + * for ASUS systems, but there's currently no way to probe the + * ACPI namespace for ASUS HIDs. So we just return failure if + * we didn't find one, which will cause the module to be + * unloaded. + */ + if (!asus_hotk_found) { acpi_bus_unregister_driver(&asus_hotk_driver); remove_proc_entry(PROC_ASUS, acpi_root_dir); return -ENODEV; -- cgit v1.2.3 From e4513a57ef719d3d6d1cee0ca4d9f4016aa452bb Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Tue, 28 Mar 2006 17:04:00 -0500 Subject: ACPI: fix sonypi ACPI driver registration to unregister on failure Remove the assumption that acpi_bus_register_driver() returns the number of devices claimed. Returning the count is unreliable because devices may be hot-plugged in the future (admittedly not applicable for this driver). This also fixes a bug: if sonypi_acpi_driver was registered but found no devices, sonypi_exit() did not unregister it. Signed-off-by: Bjorn Helgaas Signed-off-by: Andrew Morton Signed-off-by: Len Brown --- drivers/char/sonypi.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/char/sonypi.c b/drivers/char/sonypi.c index f8dd8527c6a..81ce5515b41 100644 --- a/drivers/char/sonypi.c +++ b/drivers/char/sonypi.c @@ -512,7 +512,7 @@ static struct sonypi_device { #ifdef CONFIG_ACPI static struct acpi_device *sonypi_acpi_device; -static int acpi_enabled; +static int acpi_driver_registered; #endif static int sonypi_ec_write(u8 addr, u8 value) @@ -869,7 +869,7 @@ found: sonypi_report_input_event(event); #ifdef CONFIG_ACPI - if (acpi_enabled) + if (sonypi_acpi_device) acpi_bus_generate_event(sonypi_acpi_device, 1, event); #endif @@ -1548,8 +1548,8 @@ static int __init sonypi_init(void) goto err_free_device; #ifdef CONFIG_ACPI - if (acpi_bus_register_driver(&sonypi_acpi_driver) > 0) - acpi_enabled = 1; + if (acpi_bus_register_driver(&sonypi_acpi_driver) >= 0) + acpi_driver_registered = 1; #endif return 0; @@ -1564,7 +1564,7 @@ static int __init sonypi_init(void) static void __exit sonypi_exit(void) { #ifdef CONFIG_ACPI - if (acpi_enabled) + if (acpi_driver_registered) acpi_bus_unregister_driver(&sonypi_acpi_driver); #endif platform_device_unregister(sonypi_platform_device); -- cgit v1.2.3 From 06ea8e08ae7e7e450b6a78e7ce5e10b3c5f954ea Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Thu, 27 Apr 2006 05:25:00 -0400 Subject: ACPI: acpi_bus_unregister_driver() returns void Nobody looks at the return value, and this brings it into line with pci_unregister_driver(), etc. Also removed validation of the driver pointer passed in to register and unregister. More consistent, and we'll find bugs faster if we fault rather than returning an error that's ignored. Also makes internal functions acpi_device_unregister() and acpi_driver_detach() void, since nobody uses their returns either. Signed-off-by: Bjorn Helgaas Signed-off-by: Andrew Morton Signed-off-by: Len Brown --- drivers/acpi/scan.c | 32 ++++++++++---------------------- include/acpi/acpi_bus.h | 2 +- 2 files changed, 11 insertions(+), 23 deletions(-) diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 669553553fb..e8efaac71e7 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -142,7 +142,7 @@ static void acpi_device_register(struct acpi_device *device, create_sysfs_device_files(device); } -static int acpi_device_unregister(struct acpi_device *device, int type) +static void acpi_device_unregister(struct acpi_device *device, int type) { spin_lock(&acpi_device_lock); if (device->parent) { @@ -158,7 +158,6 @@ static int acpi_device_unregister(struct acpi_device *device, int type) acpi_detach_data(device->handle, acpi_bus_data_handler); remove_sysfs_device_files(device); kobject_unregister(&device->kobj); - return 0; } void acpi_bus_data_handler(acpi_handle handle, u32 function, void *context) @@ -577,7 +576,7 @@ static void acpi_driver_attach(struct acpi_driver *drv) spin_unlock(&acpi_device_lock); } -static int acpi_driver_detach(struct acpi_driver *drv) +static void acpi_driver_detach(struct acpi_driver *drv) { struct list_head *node, *next; @@ -599,7 +598,6 @@ static int acpi_driver_detach(struct acpi_driver *drv) } } spin_unlock(&acpi_device_lock); - return_VALUE(0); } /** @@ -617,9 +615,6 @@ int acpi_bus_register_driver(struct acpi_driver *driver) if (acpi_disabled) return_VALUE(-ENODEV); - if (!driver) - return_VALUE(-EINVAL); - spin_lock(&acpi_device_lock); list_add_tail(&driver->node, &acpi_bus_drivers); spin_unlock(&acpi_device_lock); @@ -637,23 +632,16 @@ EXPORT_SYMBOL(acpi_bus_register_driver); * Unregisters a driver with the ACPI bus. Searches the namespace for all * devices that match the driver's criteria and unbinds. */ -int acpi_bus_unregister_driver(struct acpi_driver *driver) +void acpi_bus_unregister_driver(struct acpi_driver *driver) { - int error = 0; - - ACPI_FUNCTION_TRACE("acpi_bus_unregister_driver"); - - if (driver) { - acpi_driver_detach(driver); + acpi_driver_detach(driver); - if (!atomic_read(&driver->references)) { - spin_lock(&acpi_device_lock); - list_del_init(&driver->node); - spin_unlock(&acpi_device_lock); - } - } else - error = -EINVAL; - return_VALUE(error); + if (!atomic_read(&driver->references)) { + spin_lock(&acpi_device_lock); + list_del_init(&driver->node); + spin_unlock(&acpi_device_lock); + } + return; } EXPORT_SYMBOL(acpi_bus_unregister_driver); diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 6dca3d54208..809eb501a22 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -327,7 +327,7 @@ int acpi_bus_set_power(acpi_handle handle, int state); int acpi_bus_generate_event(struct acpi_device *device, u8 type, int data); int acpi_bus_receive_event(struct acpi_bus_event *event); int acpi_bus_register_driver(struct acpi_driver *driver); -int acpi_bus_unregister_driver(struct acpi_driver *driver); +void acpi_bus_unregister_driver(struct acpi_driver *driver); int acpi_bus_add(struct acpi_device **child, struct acpi_device *parent, acpi_handle handle, int type); int acpi_bus_trim(struct acpi_device *start, int rmdevice); -- cgit v1.2.3