All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] ACPI: Remove unnecessary code in acpi drivers
@ 2009-09-14 16:09 Alan Jenkins
  2009-09-14 16:09 ` [PATCH 1/4] ACPI: Remember to clear acpi_dev->driver after calling ops.remove() Alan Jenkins
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Alan Jenkins @ 2009-09-14 16:09 UTC (permalink / raw)
  To: lenb; +Cc: bjorn.helgaas, linux-acpi

Bjorn critiqued some code in a new acpi driver as unnecesary. However,
the same code patterns are present in most acpi drivers.  Let's remove
them all.

The first patch ammends acpi/scan.c so that the following NULL
assignment pattern can be removed with complete confidence:

    acpi_driver_remove(acpi_device *device, int removal_type) 
    {
	...
	device->driverdata = NULL;
    }

Some failure paths in scan.c were omitting to clear device->driverdata,
and more seriously device->driver, so I fixed that before removing the
NULL assignments from drivers.

In total this removes ~150 lines.

 drivers/acpi/ac.c                       |   23 +++------------
 drivers/acpi/acpi_memhotplug.c          |   13 ++-------
 drivers/acpi/battery.c                  |   18 +++---------
 drivers/acpi/container.c                |   12 ++------
 drivers/acpi/ec.c                       |    9 +-----
 drivers/acpi/fan.c                      |   14 +---------
 drivers/acpi/pci_link.c                 |   11 ++++---
 drivers/acpi/power.c                    |   19 ++-----------
 drivers/acpi/processor_core.c           |    7 +----
 drivers/acpi/sbs.c                      |   11 +------
 drivers/acpi/sbshc.c                    |   17 +----------
 drivers/acpi/scan.c                     |   45 ++++++++++++++++++------------
 drivers/acpi/thermal.c                  |   21 +++-----------
 drivers/acpi/video.c                    |   17 ++---------
 drivers/hwmon/asus_atk0110.c            |    2 -
 drivers/hwmon/hp_accel.c                |    9 ------
 drivers/input/misc/atlas_btns.c         |    5 +---
 drivers/platform/x86/asus-laptop.c      |    9 ------
 drivers/platform/x86/asus_acpi.c        |    9 ------
 drivers/platform/x86/eeepc-laptop.c     |    7 -----
 drivers/platform/x86/fujitsu-laptop.c   |    6 ----
 drivers/platform/x86/intel_menlow.c     |    6 ----
 drivers/platform/x86/panasonic-laptop.c |   16 +---------
 drivers/platform/x86/wmi.c              |    3 --
24 files changed, 70 insertions(+), 239 deletions(-)

^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH 1/4] ACPI: Remember to clear acpi_dev->driver after calling ops.remove()
  2009-09-14 16:09 [PATCH 0/4] ACPI: Remove unnecessary code in acpi drivers Alan Jenkins
@ 2009-09-14 16:09 ` Alan Jenkins
  2009-09-14 16:09 ` [PATCH 2/4] ACPI: Remove uneccessary NULL assignments in acpi drivers Alan Jenkins
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Alan Jenkins @ 2009-09-14 16:09 UTC (permalink / raw)
  To: lenb; +Cc: bjorn.helgaas, linux-acpi, Alan Jenkins

If an error occurs while binding a driver to a device and we call
remove(), we should also clear acpi->driver and acpi_dev->driverdata.
Otherwise bad things will happen e.g. we will invoke the suspend()
driver callback when the system is suspended, even though the driver
thinks it has been unbound from the device.

Also check the return value of acpi_start_single_object().  I'm not
sure what will happen if we claim success despite seeing ops.start()
fail and then calling ops.remove(), but it can't be good.

Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
Reviewed-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
 drivers/acpi/scan.c |   45 +++++++++++++++++++++++++++------------------
 1 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 957620b..cde179e 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -423,26 +423,33 @@ static int acpi_device_probe(struct device * dev)
 	int ret;
 
 	ret = acpi_bus_driver_init(acpi_dev, acpi_drv);
-	if (!ret) {
-		if (acpi_dev->bus_ops.acpi_op_start)
-			acpi_start_single_object(acpi_dev);
-
-		if (acpi_drv->ops.notify) {
-			ret = acpi_device_install_notify_handler(acpi_dev);
-			if (ret) {
-				if (acpi_drv->ops.remove)
-					acpi_drv->ops.remove(acpi_dev,
-						     acpi_dev->removal_type);
-				return ret;
-			}
-		}
+	if (ret)
+		return ret;
 
-		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-			"Found driver [%s] for device [%s]\n",
-			acpi_drv->name, acpi_dev->pnp.bus_id));
-		get_device(dev);
+	if (acpi_dev->bus_ops.acpi_op_start) {
+		ret = acpi_start_single_object(acpi_dev);
+		if (ret)
+			return ret;
 	}
-	return ret;
+
+	if (acpi_drv->ops.notify) {
+		ret = acpi_device_install_notify_handler(acpi_dev);
+		if (ret) {
+			if (acpi_drv->ops.remove)
+				acpi_drv->ops.remove(acpi_dev,
+						acpi_dev->removal_type);
+			acpi_dev->driver = NULL;
+			acpi_dev->driver_data = NULL;
+			return ret;
+		}
+	}
+
+	ACPI_DEBUG_PRINT((ACPI_DB_INFO,
+		"Found driver [%s] for device [%s]\n",
+		acpi_drv->name, acpi_dev->pnp.bus_id));
+	get_device(dev);
+
+	return 0;
 }
 
 static int acpi_device_remove(struct device * dev)
@@ -617,6 +624,8 @@ static int acpi_start_single_object(struct acpi_device *device)
 		result = driver->ops.start(device);
 		if (result && driver->ops.remove)
 			driver->ops.remove(device, ACPI_BUS_REMOVAL_NORMAL);
+		device->driver = NULL;
+		device->driver_data = NULL;
 	}
 
 	return result;
-- 
1.6.3.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH 2/4] ACPI: Remove uneccessary NULL assignments in acpi drivers
  2009-09-14 16:09 [PATCH 0/4] ACPI: Remove unnecessary code in acpi drivers Alan Jenkins
  2009-09-14 16:09 ` [PATCH 1/4] ACPI: Remember to clear acpi_dev->driver after calling ops.remove() Alan Jenkins
@ 2009-09-14 16:09 ` Alan Jenkins
  2009-09-14 16:09 ` [PATCH 3/4] ACPI: Remove redundant NULL checks " Alan Jenkins
  2009-09-14 16:09 ` [PATCH 4/4] ACPI: Remove uneccesary acpi_disabled " Alan Jenkins
  3 siblings, 0 replies; 9+ messages in thread
From: Alan Jenkins @ 2009-09-14 16:09 UTC (permalink / raw)
  To: lenb; +Cc: bjorn.helgaas, linux-acpi, Alan Jenkins

The core is responsibile for clearing device->driverdata on removal.

Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
---
 drivers/acpi/ec.c            |    1 -
 drivers/acpi/sbshc.c         |    1 -
 drivers/acpi/video.c         |    1 -
 drivers/hwmon/asus_atk0110.c |    2 --
 4 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 64a20ef..add6621 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -852,7 +852,6 @@ static int acpi_ec_remove(struct acpi_device *device, int type)
 	}
 	mutex_unlock(&ec->lock);
 	acpi_ec_remove_fs(device);
-	device->driver_data = NULL;
 	if (ec == first_ec)
 		first_ec = NULL;
 	kfree(ec);
diff --git a/drivers/acpi/sbshc.c b/drivers/acpi/sbshc.c
index d933980..8d89337 100644
--- a/drivers/acpi/sbshc.c
+++ b/drivers/acpi/sbshc.c
@@ -304,7 +304,6 @@ static int acpi_smbus_hc_remove(struct acpi_device *device, int type)
 	hc = acpi_driver_data(device);
 	acpi_ec_remove_query_handler(hc->ec, hc->query_bit);
 	kfree(hc);
-	device->driver_data = NULL;
 	return 0;
 }
 
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index b51f1fe..f405807 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -2345,7 +2345,6 @@ static int acpi_video_bus_add(struct acpi_device *device)
 	acpi_video_bus_remove_fs(device);
  err_free_video:
 	kfree(video);
-	device->driver_data = NULL;
 
 	return error;
 }
diff --git a/drivers/hwmon/asus_atk0110.c b/drivers/hwmon/asus_atk0110.c
index fe4fa29..7fc7625 100644
--- a/drivers/hwmon/asus_atk0110.c
+++ b/drivers/hwmon/asus_atk0110.c
@@ -982,8 +982,6 @@ static int atk_remove(struct acpi_device *device, int type)
 	struct atk_data *data = device->driver_data;
 	dev_dbg(&device->dev, "removing...\n");
 
-	device->driver_data = NULL;
-
 	atk_remove_files(data);
 	atk_free_sensors(data);
 	hwmon_device_unregister(data->hwmon_dev);
-- 
1.6.3.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH 3/4] ACPI: Remove redundant NULL checks in acpi drivers
  2009-09-14 16:09 [PATCH 0/4] ACPI: Remove unnecessary code in acpi drivers Alan Jenkins
  2009-09-14 16:09 ` [PATCH 1/4] ACPI: Remember to clear acpi_dev->driver after calling ops.remove() Alan Jenkins
  2009-09-14 16:09 ` [PATCH 2/4] ACPI: Remove uneccessary NULL assignments in acpi drivers Alan Jenkins
@ 2009-09-14 16:09 ` Alan Jenkins
  2009-09-14 19:39   ` Bjorn Helgaas
  2009-09-14 16:09 ` [PATCH 4/4] ACPI: Remove uneccesary acpi_disabled " Alan Jenkins
  3 siblings, 1 reply; 9+ messages in thread
From: Alan Jenkins @ 2009-09-14 16:09 UTC (permalink / raw)
  To: lenb; +Cc: bjorn.helgaas, linux-acpi, Alan Jenkins

The acpi device callbacks add, start, remove, suspend and resume can
never be called with a NULL acpi_device. Each callsite in acpi/scan.c
has to dereference the device in order to get the ops structure, e.g.

    struct acpi_device *acpi_dev = to_acpi_device(dev);
    struct acpi_driver *acpi_drv = acpi_dev->driver;

    if (acpi_drv && acpi_drv->ops.suspend)
        return acpi_drv->ops.suspend(acpi_dev, state);

Remove all checks for acpi_dev == NULL within these callbacks.

Also remove the checks for acpi_driver_data(acpi_dev) == NULL. None of
these checks could fail unless the driver does something strange
(which none of them do), the acpi core did something terribly wrong,
or we have a memory corruption issue. If this does happen then it's
best to dereference the pointer and crash noisily.

Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
---
 drivers/acpi/ac.c                       |   18 ++++--------------
 drivers/acpi/acpi_memhotplug.c          |   11 ++---------
 drivers/acpi/battery.c                  |   16 +++++-----------
 drivers/acpi/container.c                |    8 +-------
 drivers/acpi/ec.c                       |    6 +-----
 drivers/acpi/fan.c                      |   12 ------------
 drivers/acpi/power.c                    |   17 ++---------------
 drivers/acpi/processor_core.c           |    7 +------
 drivers/acpi/sbs.c                      |    7 +------
 drivers/acpi/sbshc.c                    |    9 +--------
 drivers/acpi/thermal.c                  |   19 +++----------------
 drivers/acpi/video.c                    |   14 ++------------
 drivers/hwmon/hp_accel.c                |    6 ------
 drivers/platform/x86/asus-laptop.c      |    6 ------
 drivers/platform/x86/asus_acpi.c        |    6 ------
 drivers/platform/x86/eeepc-laptop.c     |    5 -----
 drivers/platform/x86/fujitsu-laptop.c   |    6 ------
 drivers/platform/x86/intel_menlow.c     |    6 ------
 drivers/platform/x86/panasonic-laptop.c |    9 ---------
 19 files changed, 23 insertions(+), 165 deletions(-)

diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index 98b9690..7725bda 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -256,11 +256,8 @@ static void acpi_ac_notify(struct acpi_device *device, u32 event)
 static int acpi_ac_add(struct acpi_device *device)
 {
 	int result = 0;
-	struct acpi_ac *ac = NULL;
-
+	struct acpi_ac *ac;
 
-	if (!device)
-		return -EINVAL;
 
 	ac = kzalloc(sizeof(struct acpi_ac), GFP_KERNEL);
 	if (!ac)
@@ -306,11 +303,9 @@ static int acpi_ac_add(struct acpi_device *device)
 
 static int acpi_ac_resume(struct acpi_device *device)
 {
-	struct acpi_ac *ac;
+	struct acpi_ac *ac = acpi_driver_data(device);
 	unsigned old_state;
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
-	ac = acpi_driver_data(device);
+
 	old_state = ac->state;
 	if (acpi_ac_get_state(ac))
 		return 0;
@@ -323,13 +318,8 @@ static int acpi_ac_resume(struct acpi_device *device)
 
 static int acpi_ac_remove(struct acpi_device *device, int type)
 {
-	struct acpi_ac *ac = NULL;
-
-
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
+	struct acpi_ac *ac = acpi_driver_data(device);
 
-	ac = acpi_driver_data(device);
 
 #ifdef CONFIG_ACPI_SYSFS_POWER
 	if (ac->charger.dev)
diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 28ccdbc..71085b9 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -401,11 +401,8 @@ static void acpi_memory_device_notify(acpi_handle handle, u32 event, void *data)
 static int acpi_memory_device_add(struct acpi_device *device)
 {
 	int result;
-	struct acpi_memory_device *mem_device = NULL;
-
+	struct acpi_memory_device *mem_device;
 
-	if (!device)
-		return -EINVAL;
 
 	mem_device = kzalloc(sizeof(struct acpi_memory_device), GFP_KERNEL);
 	if (!mem_device)
@@ -450,13 +447,9 @@ static int acpi_memory_device_add(struct acpi_device *device)
 
 static int acpi_memory_device_remove(struct acpi_device *device, int type)
 {
-	struct acpi_memory_device *mem_device = NULL;
+	struct acpi_memory_device *mem_device = acpi_driver_data(device);
 
 
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
-
-	mem_device = acpi_driver_data(device);
 	kfree(mem_device);
 
 	return 0;
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 3f4602b..90e39d9 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -840,9 +840,8 @@ static void acpi_battery_notify(struct acpi_device *device, u32 event)
 static int acpi_battery_add(struct acpi_device *device)
 {
 	int result = 0;
-	struct acpi_battery *battery = NULL;
-	if (!device)
-		return -EINVAL;
+	struct acpi_battery *battery;
+
 	battery = kzalloc(sizeof(struct acpi_battery), GFP_KERNEL);
 	if (!battery)
 		return -ENOMEM;
@@ -870,11 +869,8 @@ static int acpi_battery_add(struct acpi_device *device)
 
 static int acpi_battery_remove(struct acpi_device *device, int type)
 {
-	struct acpi_battery *battery = NULL;
+	struct acpi_battery *battery = acpi_driver_data(device);
 
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
-	battery = acpi_driver_data(device);
 #ifdef CONFIG_ACPI_PROCFS_POWER
 	acpi_battery_remove_fs(device);
 #endif
@@ -889,10 +885,8 @@ static int acpi_battery_remove(struct acpi_device *device, int type)
 /* this is needed to learn about changes made in suspended state */
 static int acpi_battery_resume(struct acpi_device *device)
 {
-	struct acpi_battery *battery;
-	if (!device)
-		return -EINVAL;
-	battery = acpi_driver_data(device);
+	struct acpi_battery *battery = acpi_driver_data(device);
+
 	battery->update_time = 0;
 	acpi_battery_update(battery);
 	return 0;
diff --git a/drivers/acpi/container.c b/drivers/acpi/container.c
index 642bb30..30c4700 100644
--- a/drivers/acpi/container.c
+++ b/drivers/acpi/container.c
@@ -97,11 +97,6 @@ static int acpi_container_add(struct acpi_device *device)
 	struct acpi_container *container;
 
 
-	if (!device) {
-		printk(KERN_ERR PREFIX "device is NULL\n");
-		return -EINVAL;
-	}
-
 	container = kzalloc(sizeof(struct acpi_container), GFP_KERNEL);
 	if (!container)
 		return -ENOMEM;
@@ -120,9 +115,8 @@ static int acpi_container_add(struct acpi_device *device)
 static int acpi_container_remove(struct acpi_device *device, int type)
 {
 	acpi_status status = AE_OK;
-	struct acpi_container *pc = NULL;
+	struct acpi_container *pc = acpi_driver_data(device);
 
-	pc = acpi_driver_data(device);
 	kfree(pc);
 	return status;
 }
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index add6621..5e93ae3 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -837,13 +837,9 @@ static int acpi_ec_add(struct acpi_device *device)
 
 static int acpi_ec_remove(struct acpi_device *device, int type)
 {
-	struct acpi_ec *ec;
+	struct acpi_ec *ec = acpi_driver_data(device);
 	struct acpi_ec_query_handler *handler, *tmp;
 
-	if (!device)
-		return -EINVAL;
-
-	ec = acpi_driver_data(device);
 	ec_remove_handlers(ec);
 	mutex_lock(&ec->lock);
 	list_for_each_entry_safe(handler, tmp, &ec->list, node) {
diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
index f419849..d40e214 100644
--- a/drivers/acpi/fan.c
+++ b/drivers/acpi/fan.c
@@ -244,9 +244,6 @@ static int acpi_fan_add(struct acpi_device *device)
 	int state = 0;
 	struct thermal_cooling_device *cdev;
 
-	if (!device)
-		return -EINVAL;
-
 	strcpy(acpi_device_name(device), "Fan");
 	strcpy(acpi_device_class(device), ACPI_FAN_CLASS);
 
@@ -300,9 +297,6 @@ static int acpi_fan_remove(struct acpi_device *device, int type)
 {
 	struct thermal_cooling_device *cdev = acpi_driver_data(device);
 
-	if (!device || !cdev)
-		return -EINVAL;
-
 	acpi_fan_remove_fs(device);
 	sysfs_remove_link(&device->dev.kobj, "thermal_cooling");
 	sysfs_remove_link(&cdev->device.kobj, "device");
@@ -313,9 +307,6 @@ static int acpi_fan_remove(struct acpi_device *device, int type)
 
 static int acpi_fan_suspend(struct acpi_device *device, pm_message_t state)
 {
-	if (!device)
-		return -EINVAL;
-
 	acpi_bus_set_power(device->handle, ACPI_STATE_D0);
 
 	return AE_OK;
@@ -326,9 +317,6 @@ static int acpi_fan_resume(struct acpi_device *device)
 	int result = 0;
 	int power_state = 0;
 
-	if (!device)
-		return -EINVAL;
-
 	result = acpi_bus_get_power(device->handle, &power_state);
 	if (result) {
 		printk(KERN_ERR PREFIX
diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
index e86603f..2c3d844 100644
--- a/drivers/acpi/power.c
+++ b/drivers/acpi/power.c
@@ -639,9 +639,6 @@ static int acpi_power_add(struct acpi_device *device)
 	struct acpi_buffer buffer = { sizeof(acpi_object), &acpi_object };
 
 
-	if (!device)
-		return -EINVAL;
-
 	resource = kzalloc(sizeof(struct acpi_power_resource), GFP_KERNEL);
 	if (!resource)
 		return -ENOMEM;
@@ -695,15 +692,10 @@ static int acpi_power_add(struct acpi_device *device)
 
 static int acpi_power_remove(struct acpi_device *device, int type)
 {
-	struct acpi_power_resource *resource = NULL;
+	struct acpi_power_resource *resource = acpi_driver_data(device);
 	struct list_head *node, *next;
 
 
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
-
-	resource = acpi_driver_data(device);
-
 	acpi_power_remove_fs(device);
 
 	mutex_lock(&resource->resource_lock);
@@ -722,14 +714,9 @@ static int acpi_power_remove(struct acpi_device *device, int type)
 static int acpi_power_resume(struct acpi_device *device)
 {
 	int result = 0, state;
-	struct acpi_power_resource *resource = NULL;
+	struct acpi_power_resource *resource = acpi_driver_data(device);
 	struct acpi_power_reference *ref;
 
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
-
-	resource = acpi_driver_data(device);
-
 	result = acpi_power_get_state(device->handle, &state);
 	if (result)
 		return result;
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index c2d4d6e..38ad5ef 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -888,13 +888,8 @@ err_free_cpumask:
 
 static int acpi_processor_remove(struct acpi_device *device, int type)
 {
-	struct acpi_processor *pr = NULL;
-
-
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
+	struct acpi_processor *pr = acpi_driver_data(device);
 
-	pr = acpi_driver_data(device);
 
 	if (pr->id >= nr_cpu_ids)
 		goto free;
diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c
index 52b9db8..4c7ab39 100644
--- a/drivers/acpi/sbs.c
+++ b/drivers/acpi/sbs.c
@@ -960,12 +960,9 @@ static int acpi_sbs_add(struct acpi_device *device)
 
 static int acpi_sbs_remove(struct acpi_device *device, int type)
 {
-	struct acpi_sbs *sbs;
+	struct acpi_sbs *sbs = acpi_driver_data(device);
 	int id;
 
-	if (!device)
-		return -EINVAL;
-	sbs = acpi_driver_data(device);
 	if (!sbs)
 		return -EINVAL;
 	mutex_lock(&sbs->lock);
@@ -996,8 +993,6 @@ static void acpi_sbs_rmdirs(void)
 static int acpi_sbs_resume(struct acpi_device *device)
 {
 	struct acpi_sbs *sbs;
-	if (!device)
-		return -EINVAL;
 	sbs = device->driver_data;
 	acpi_sbs_callback(sbs);
 	return 0;
diff --git a/drivers/acpi/sbshc.c b/drivers/acpi/sbshc.c
index 8d89337..9e06e9c 100644
--- a/drivers/acpi/sbshc.c
+++ b/drivers/acpi/sbshc.c
@@ -262,9 +262,6 @@ static int acpi_smbus_hc_add(struct acpi_device *device)
 	unsigned long long val;
 	struct acpi_smb_hc *hc;
 
-	if (!device)
-		return -EINVAL;
-
 	status = acpi_evaluate_integer(device->handle, "_EC", NULL, &val);
 	if (ACPI_FAILURE(status)) {
 		printk(KERN_ERR PREFIX "error obtaining _EC.\n");
@@ -296,12 +293,8 @@ extern void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit);
 
 static int acpi_smbus_hc_remove(struct acpi_device *device, int type)
 {
-	struct acpi_smb_hc *hc;
-
-	if (!device)
-		return -EINVAL;
+	struct acpi_smb_hc *hc = acpi_driver_data(device);
 
-	hc = acpi_driver_data(device);
 	acpi_ec_remove_query_handler(hc->ec, hc->query_bit);
 	kfree(hc);
 	return 0;
diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 65f6781..fca61a1 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -1361,12 +1361,9 @@ static void acpi_thermal_guess_offset(struct acpi_thermal *tz)
 static int acpi_thermal_add(struct acpi_device *device)
 {
 	int result = 0;
-	struct acpi_thermal *tz = NULL;
+	struct acpi_thermal *tz;
 
 
-	if (!device)
-		return -EINVAL;
-
 	tz = kzalloc(sizeof(struct acpi_thermal), GFP_KERNEL);
 	if (!tz)
 		return -ENOMEM;
@@ -1408,12 +1405,7 @@ end:
 
 static int acpi_thermal_remove(struct acpi_device *device, int type)
 {
-	struct acpi_thermal *tz = NULL;
-
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
-
-	tz = acpi_driver_data(device);
+	struct acpi_thermal *tz = acpi_driver_data(device);
 
 	acpi_thermal_remove_fs(device);
 	acpi_thermal_unregister_thermal_zone(tz);
@@ -1424,15 +1416,10 @@ static int acpi_thermal_remove(struct acpi_device *device, int type)
 
 static int acpi_thermal_resume(struct acpi_device *device)
 {
-	struct acpi_thermal *tz = NULL;
+	struct acpi_thermal *tz = acpi_driver_data(device);
 	int i, j, power_state, result;
 
 
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
-
-	tz = acpi_driver_data(device);
-
 	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
 		if (!(&tz->trips.active[i]))
 			break;
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index f405807..576a600 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -2207,15 +2207,10 @@ static void acpi_video_device_notify(acpi_handle handle, u32 event, void *data)
 static int instance;
 static int acpi_video_resume(struct acpi_device *device)
 {
-	struct acpi_video_bus *video;
+	struct acpi_video_bus *video = acpi_driver_data(device);
 	struct acpi_video_device *video_device;
 	int i;
 
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
-
-	video = acpi_driver_data(device);
-
 	for (i = 0; i < video->attached_count; i++) {
 		video_device = video->attached_array[i].bind_info;
 		if (video_device && video_device->backlight)
@@ -2351,13 +2346,8 @@ static int acpi_video_bus_add(struct acpi_device *device)
 
 static int acpi_video_bus_remove(struct acpi_device *device, int type)
 {
-	struct acpi_video_bus *video = NULL;
-
-
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
+	struct acpi_video_bus *video = acpi_driver_data(device);
 
-	video = acpi_driver_data(device);
 
 	acpi_video_bus_stop_devices(video);
 	acpi_video_bus_put_devices(video);
diff --git a/drivers/hwmon/hp_accel.c b/drivers/hwmon/hp_accel.c
index 6679854..c8d3c88 100644
--- a/drivers/hwmon/hp_accel.c
+++ b/drivers/hwmon/hp_accel.c
@@ -275,9 +275,6 @@ static int lis3lv02d_add(struct acpi_device *device)
 {
 	int ret;
 
-	if (!device)
-		return -EINVAL;
-
 	lis3_dev.bus_priv = device;
 	lis3_dev.init = lis3lv02d_acpi_init;
 	lis3_dev.read = lis3lv02d_acpi_read;
@@ -315,9 +312,6 @@ static int lis3lv02d_add(struct acpi_device *device)
 
 static int lis3lv02d_remove(struct acpi_device *device, int type)
 {
-	if (!device)
-		return -EINVAL;
-
 	lis3lv02d_joystick_disable();
 	lis3lv02d_poweroff(&lis3_dev);
 
diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
index b39d2bb..8af43e9 100644
--- a/drivers/platform/x86/asus-laptop.c
+++ b/drivers/platform/x86/asus-laptop.c
@@ -1240,9 +1240,6 @@ static int asus_hotk_add(struct acpi_device *device)
 {
 	int result;
 
-	if (!device)
-		return -EINVAL;
-
 	pr_notice("Asus Laptop Support version %s\n",
 	       ASUS_LAPTOP_VERSION);
 
@@ -1306,9 +1303,6 @@ end:
 
 static int asus_hotk_remove(struct acpi_device *device, int type)
 {
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
-
 	kfree(hotk->name);
 	kfree(hotk);
 
diff --git a/drivers/platform/x86/asus_acpi.c b/drivers/platform/x86/asus_acpi.c
index ddf5240..25a7d57 100644
--- a/drivers/platform/x86/asus_acpi.c
+++ b/drivers/platform/x86/asus_acpi.c
@@ -1334,9 +1334,6 @@ static int asus_hotk_add(struct acpi_device *device)
 	acpi_status status = AE_OK;
 	int result;
 
-	if (!device)
-		return -EINVAL;
-
 	printk(KERN_NOTICE "Asus Laptop ACPI Extras version %s\n",
 	       ASUS_ACPI_VERSION);
 
@@ -1392,9 +1389,6 @@ end:
 
 static int asus_hotk_remove(struct acpi_device *device, int type)
 {
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
-
 	asus_hotk_remove_fs(device);
 
 	kfree(hotk);
diff --git a/drivers/platform/x86/eeepc-laptop.c b/drivers/platform/x86/eeepc-laptop.c
index da3c08b..e7f14a4 100644
--- a/drivers/platform/x86/eeepc-laptop.c
+++ b/drivers/platform/x86/eeepc-laptop.c
@@ -1194,8 +1194,6 @@ static int eeepc_hotk_add(struct acpi_device *device)
 	struct device *dev;
 	int result;
 
-	if (!device)
-		return -EINVAL;
 	pr_notice(EEEPC_HOTK_NAME "\n");
 	ehotk = kzalloc(sizeof(struct eeepc_hotk), GFP_KERNEL);
 	if (!ehotk)
@@ -1276,9 +1274,6 @@ fail_platform_driver:
 
 static int eeepc_hotk_remove(struct acpi_device *device, int type)
 {
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
-
 	eeepc_backlight_exit();
 	eeepc_rfkill_exit();
 	eeepc_input_exit();
diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index f35aee5..2ab0318 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -657,9 +657,6 @@ static int acpi_fujitsu_add(struct acpi_device *device)
 	struct input_dev *input;
 	int error;
 
-	if (!device)
-		return -EINVAL;
-
 	fujitsu->acpi_handle = device->handle;
 	sprintf(acpi_device_name(device), "%s", ACPI_FUJITSU_DEVICE_NAME);
 	sprintf(acpi_device_class(device), "%s", ACPI_FUJITSU_CLASS);
@@ -813,9 +810,6 @@ static int acpi_fujitsu_hotkey_add(struct acpi_device *device)
 	int error;
 	int i;
 
-	if (!device)
-		return -EINVAL;
-
 	fujitsu_hotkey->acpi_handle = device->handle;
 	sprintf(acpi_device_name(device), "%s",
 		ACPI_FUJITSU_HOTKEY_DEVICE_NAME);
diff --git a/drivers/platform/x86/intel_menlow.c b/drivers/platform/x86/intel_menlow.c
index 29432a5..58de8fd 100644
--- a/drivers/platform/x86/intel_menlow.c
+++ b/drivers/platform/x86/intel_menlow.c
@@ -156,9 +156,6 @@ static int intel_menlow_memory_add(struct acpi_device *device)
 	acpi_handle dummy;
 	struct thermal_cooling_device *cdev;
 
-	if (!device)
-		return -EINVAL;
-
 	status = acpi_get_handle(device->handle, MEMORY_GET_BANDWIDTH, &dummy);
 	if (ACPI_FAILURE(status))
 		goto end;
@@ -200,9 +197,6 @@ static int intel_menlow_memory_remove(struct acpi_device *device, int type)
 {
 	struct thermal_cooling_device *cdev = acpi_driver_data(device);
 
-	if (!device || !cdev)
-		return -EINVAL;
-
 	sysfs_remove_link(&device->dev.kobj, "thermal_cooling");
 	sysfs_remove_link(&cdev->device.kobj, "device");
 	thermal_cooling_device_unregister(cdev);
diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
index fe7cf01..953b60c 100644
--- a/drivers/platform/x86/panasonic-laptop.c
+++ b/drivers/platform/x86/panasonic-laptop.c
@@ -588,9 +588,6 @@ static int acpi_pcc_hotkey_resume(struct acpi_device *device)
 	struct pcc_acpi *pcc = acpi_driver_data(device);
 	acpi_status status = AE_OK;
 
-	if (device == NULL || pcc == NULL)
-		return -EINVAL;
-
 	ACPI_DEBUG_PRINT((ACPI_DB_ERROR, "Sticky mode restore: %d\n",
 			  pcc->sticky_mode));
 
@@ -604,9 +601,6 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
 	struct pcc_acpi *pcc;
 	int num_sifr, result;
 
-	if (!device)
-		return -EINVAL;
-
 	num_sifr = acpi_pcc_get_sqty(device);
 
 	if (num_sifr > 255) {
@@ -703,9 +697,6 @@ static int acpi_pcc_hotkey_remove(struct acpi_device *device, int type)
 {
 	struct pcc_acpi *pcc = acpi_driver_data(device);
 
-	if (!device || !pcc)
-		return -EINVAL;
-
 	sysfs_remove_group(&device->dev.kobj, &pcc_attr_group);
 
 	backlight_device_unregister(pcc->backlight);
-- 
1.6.3.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH 4/4] ACPI: Remove uneccesary acpi_disabled checks in acpi drivers
  2009-09-14 16:09 [PATCH 0/4] ACPI: Remove unnecessary code in acpi drivers Alan Jenkins
                   ` (2 preceding siblings ...)
  2009-09-14 16:09 ` [PATCH 3/4] ACPI: Remove redundant NULL checks " Alan Jenkins
@ 2009-09-14 16:09 ` Alan Jenkins
  2009-09-14 19:46   ` Bjorn Helgaas
  3 siblings, 1 reply; 9+ messages in thread
From: Alan Jenkins @ 2009-09-14 16:09 UTC (permalink / raw)
  To: lenb; +Cc: bjorn.helgaas, linux-acpi, Alan Jenkins

acpi_bus_register_driver() already checks acpi_disabled, so acpi bus
drivers don't need to.  Also, pass on the return value when it fails
instead of assuming ENODEV.

Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
---
 drivers/acpi/ac.c                       |    5 +----
 drivers/acpi/acpi_memhotplug.c          |    2 +-
 drivers/acpi/battery.c                  |    2 --
 drivers/acpi/container.c                |    4 ++--
 drivers/acpi/ec.c                       |    2 +-
 drivers/acpi/fan.c                      |    2 +-
 drivers/acpi/pci_link.c                 |   11 ++++++-----
 drivers/acpi/power.c                    |    2 +-
 drivers/acpi/sbs.c                      |    4 +---
 drivers/acpi/sbshc.c                    |    7 +------
 drivers/acpi/thermal.c                  |    2 +-
 drivers/acpi/video.c                    |    2 +-
 drivers/hwmon/hp_accel.c                |    3 ---
 drivers/input/misc/atlas_btns.c         |    5 +----
 drivers/platform/x86/asus-laptop.c      |    3 ---
 drivers/platform/x86/asus_acpi.c        |    3 ---
 drivers/platform/x86/eeepc-laptop.c     |    2 --
 drivers/platform/x86/panasonic-laptop.c |    7 ++-----
 drivers/platform/x86/wmi.c              |    3 ---
 19 files changed, 20 insertions(+), 51 deletions(-)

diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index 7725bda..142744a 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -338,9 +338,6 @@ static int __init acpi_ac_init(void)
 {
 	int result;
 
-	if (acpi_disabled)
-		return -ENODEV;
-
 #ifdef CONFIG_ACPI_PROCFS_POWER
 	acpi_ac_dir = acpi_lock_ac_dir();
 	if (!acpi_ac_dir)
@@ -352,7 +349,7 @@ static int __init acpi_ac_init(void)
 #ifdef CONFIG_ACPI_PROCFS_POWER
 		acpi_unlock_ac_dir(acpi_ac_dir);
 #endif
-		return -ENODEV;
+		return result;
 	}
 
 	return 0;
diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 71085b9..30a4600 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -526,7 +526,7 @@ static int __init acpi_memory_device_init(void)
 	result = acpi_bus_register_driver(&acpi_memory_device_driver);
 
 	if (result < 0)
-		return -ENODEV;
+		return result;
 
 	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
 				     ACPI_UINT32_MAX,
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 90e39d9..f451dc0 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -907,8 +907,6 @@ static struct acpi_driver acpi_battery_driver = {
 
 static void __init acpi_battery_init_async(void *unused, async_cookie_t cookie)
 {
-	if (acpi_disabled)
-		return;
 #ifdef CONFIG_ACPI_PROCFS_POWER
 	acpi_battery_dir = acpi_lock_battery_dir();
 	if (!acpi_battery_dir)
diff --git a/drivers/acpi/container.c b/drivers/acpi/container.c
index 30c4700..c54b69b 100644
--- a/drivers/acpi/container.c
+++ b/drivers/acpi/container.c
@@ -245,7 +245,7 @@ static int __init acpi_container_init(void)
 
 	result = acpi_bus_register_driver(&acpi_container_driver);
 	if (result < 0) {
-		return (result);
+		return result;
 	}
 
 	/* register notify handler to every container device */
@@ -254,7 +254,7 @@ static int __init acpi_container_init(void)
 			    ACPI_UINT32_MAX,
 			    container_walk_namespace_cb, &action, NULL);
 
-	return (0);
+	return 0;
 }
 
 static void __exit acpi_container_exit(void)
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 5e93ae3..26c58a3 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -1010,7 +1010,7 @@ int __init acpi_ec_init(void)
 	result = acpi_bus_register_driver(&acpi_ec_driver);
 	if (result < 0) {
 		remove_proc_entry(ACPI_EC_CLASS, acpi_root_dir);
-		return -ENODEV;
+		return result;
 	}
 
 	return result;
diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
index d40e214..947556e 100644
--- a/drivers/acpi/fan.c
+++ b/drivers/acpi/fan.c
@@ -345,7 +345,7 @@ static int __init acpi_fan_init(void)
 	result = acpi_bus_register_driver(&acpi_fan_driver);
 	if (result < 0) {
 		remove_proc_entry(ACPI_FAN_CLASS, acpi_root_dir);
-		return -ENODEV;
+		return result;
 	}
 
 	return 0;
diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
index 394ae89..4a1e22a 100644
--- a/drivers/acpi/pci_link.c
+++ b/drivers/acpi/pci_link.c
@@ -769,9 +769,7 @@ static int irqrouter_resume(struct sys_device *dev)
 
 static int acpi_pci_link_remove(struct acpi_device *device, int type)
 {
-	struct acpi_pci_link *link;
-
-	link = acpi_driver_data(device);
+	struct acpi_pci_link *link = acpi_driver_data(device);
 
 	mutex_lock(&acpi_link_lock);
 	list_del(&link->list);
@@ -900,6 +898,8 @@ device_initcall(irqrouter_init_sysfs);
 
 static int __init acpi_pci_link_init(void)
 {
+	int result;
+
 	if (acpi_noirq)
 		return 0;
 
@@ -911,8 +911,9 @@ static int __init acpi_pci_link_init(void)
 			acpi_irq_balance = 0;
 	}
 
-	if (acpi_bus_register_driver(&acpi_pci_link_driver) < 0)
-		return -ENODEV;
+	result = acpi_bus_register_driver(&acpi_pci_link_driver);
+	if (result < 0)
+		return result;
 
 	return 0;
 }
diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
index 2c3d844..3cd8d45 100644
--- a/drivers/acpi/power.c
+++ b/drivers/acpi/power.c
@@ -747,7 +747,7 @@ int __init acpi_power_init(void)
 	result = acpi_bus_register_driver(&acpi_power_driver);
 	if (result < 0) {
 		remove_proc_entry(ACPI_POWER_CLASS, acpi_root_dir);
-		return -ENODEV;
+		return result;
 	}
 
 	return 0;
diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c
index 4c7ab39..192e613 100644
--- a/drivers/acpi/sbs.c
+++ b/drivers/acpi/sbs.c
@@ -1013,8 +1013,6 @@ static int __init acpi_sbs_init(void)
 {
 	int result = 0;
 
-	if (acpi_disabled)
-		return -ENODEV;
 #ifdef CONFIG_ACPI_PROCFS_POWER
 	acpi_ac_dir = acpi_lock_ac_dir();
 	if (!acpi_ac_dir)
@@ -1028,7 +1026,7 @@ static int __init acpi_sbs_init(void)
 	result = acpi_bus_register_driver(&acpi_sbs_driver);
 	if (result < 0) {
 		acpi_sbs_rmdirs();
-		return -ENODEV;
+		return result;
 	}
 	return 0;
 }
diff --git a/drivers/acpi/sbshc.c b/drivers/acpi/sbshc.c
index 9e06e9c..c07cabd 100644
--- a/drivers/acpi/sbshc.c
+++ b/drivers/acpi/sbshc.c
@@ -302,12 +302,7 @@ static int acpi_smbus_hc_remove(struct acpi_device *device, int type)
 
 static int __init acpi_smb_hc_init(void)
 {
-	int result;
-
-	result = acpi_bus_register_driver(&acpi_smb_hc_driver);
-	if (result < 0)
-		return -ENODEV;
-	return 0;
+	return acpi_bus_register_driver(&acpi_smb_hc_driver);
 }
 
 static void __exit acpi_smb_hc_exit(void)
diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index fca61a1..767e57d 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -1534,7 +1534,7 @@ static int __init acpi_thermal_init(void)
 	result = acpi_bus_register_driver(&acpi_thermal_driver);
 	if (result < 0) {
 		remove_proc_entry(ACPI_THERMAL_CLASS, acpi_root_dir);
-		return -ENODEV;
+		return result;
 	}
 
 	return 0;
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 576a600..0433df3 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -2398,7 +2398,7 @@ int acpi_video_register(void)
 	result = acpi_bus_register_driver(&acpi_video_bus);
 	if (result < 0) {
 		remove_proc_entry(ACPI_VIDEO_CLASS, acpi_root_dir);
-		return -ENODEV;
+		return result;
 	}
 
 	/*
diff --git a/drivers/hwmon/hp_accel.c b/drivers/hwmon/hp_accel.c
index c8d3c88..38f8959 100644
--- a/drivers/hwmon/hp_accel.c
+++ b/drivers/hwmon/hp_accel.c
@@ -357,9 +357,6 @@ static int __init lis3lv02d_init_module(void)
 {
 	int ret;
 
-	if (acpi_disabled)
-		return -ENODEV;
-
 	ret = acpi_bus_register_driver(&lis3lv02d_driver);
 	if (ret < 0)
 		return ret;
diff --git a/drivers/input/misc/atlas_btns.c b/drivers/input/misc/atlas_btns.c
index 1b87191..36740d1 100644
--- a/drivers/input/misc/atlas_btns.c
+++ b/drivers/input/misc/atlas_btns.c
@@ -156,13 +156,10 @@ static int __init atlas_acpi_init(void)
 {
 	int result;
 
-	if (acpi_disabled)
-		return -ENODEV;
-
 	result = acpi_bus_register_driver(&atlas_acpi_driver);
 	if (result < 0) {
 		printk(KERN_ERR "Atlas ACPI: Unable to register driver\n");
-		return -ENODEV;
+		return result;
 	}
 
 	return 0;
diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
index 8af43e9..4234edb 100644
--- a/drivers/platform/x86/asus-laptop.c
+++ b/drivers/platform/x86/asus-laptop.c
@@ -1438,9 +1438,6 @@ static int __init asus_laptop_init(void)
 {
 	int result;
 
-	if (acpi_disabled)
-		return -ENODEV;
-
 	result = acpi_bus_register_driver(&asus_hotk_driver);
 	if (result < 0)
 		return result;
diff --git a/drivers/platform/x86/asus_acpi.c b/drivers/platform/x86/asus_acpi.c
index 25a7d57..502692b 100644
--- a/drivers/platform/x86/asus_acpi.c
+++ b/drivers/platform/x86/asus_acpi.c
@@ -1416,9 +1416,6 @@ static int __init asus_acpi_init(void)
 {
 	int result;
 
-	if (acpi_disabled)
-		return -ENODEV;
-
 	asus_proc_dir = proc_mkdir(PROC_ASUS, acpi_root_dir);
 	if (!asus_proc_dir) {
 		printk(KERN_ERR "Asus ACPI: Unable to create /proc entry\n");
diff --git a/drivers/platform/x86/eeepc-laptop.c b/drivers/platform/x86/eeepc-laptop.c
index e7f14a4..eba3772 100644
--- a/drivers/platform/x86/eeepc-laptop.c
+++ b/drivers/platform/x86/eeepc-laptop.c
@@ -1291,8 +1291,6 @@ static int __init eeepc_laptop_init(void)
 {
 	int result;
 
-	if (acpi_disabled)
-		return -ENODEV;
 	result = acpi_bus_register_driver(&eeepc_hotk_driver);
 	if (result < 0)
 		return result;
diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
index 953b60c..ea28882 100644
--- a/drivers/platform/x86/panasonic-laptop.c
+++ b/drivers/platform/x86/panasonic-laptop.c
@@ -678,16 +678,13 @@ out_hotkey:
 
 static int __init acpi_pcc_init(void)
 {
-	int result = 0;
-
-	if (acpi_disabled)
-		return -ENODEV;
+	int result;
 
 	result = acpi_bus_register_driver(&acpi_pcc_driver);
 	if (result < 0) {
 		ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
 				  "Error registering hotkey driver\n"));
-		return -ENODEV;
+		return result;
 	}
 
 	return 0;
diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 177f8d7..d889f51 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -702,9 +702,6 @@ static int __init acpi_wmi_init(void)
 
 	INIT_LIST_HEAD(&wmi_blocks.list);
 
-	if (acpi_disabled)
-		return -ENODEV;
-
 	result = acpi_bus_register_driver(&acpi_wmi_driver);
 
 	if (result < 0) {
-- 
1.6.3.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH 3/4] ACPI: Remove redundant NULL checks in acpi drivers
  2009-09-14 16:09 ` [PATCH 3/4] ACPI: Remove redundant NULL checks " Alan Jenkins
@ 2009-09-14 19:39   ` Bjorn Helgaas
  2009-09-15 10:11     ` [PATCHv2 " Alan Jenkins
  0 siblings, 1 reply; 9+ messages in thread
From: Bjorn Helgaas @ 2009-09-14 19:39 UTC (permalink / raw)
  To: Alan Jenkins; +Cc: lenb, linux-acpi

On Monday 14 September 2009 10:09:28 am Alan Jenkins wrote:
> The acpi device callbacks add, start, remove, suspend and resume can
> never be called with a NULL acpi_device. Each callsite in acpi/scan.c
> has to dereference the device in order to get the ops structure, e.g.
> 
>     struct acpi_device *acpi_dev = to_acpi_device(dev);
>     struct acpi_driver *acpi_drv = acpi_dev->driver;
> 
>     if (acpi_drv && acpi_drv->ops.suspend)
>         return acpi_drv->ops.suspend(acpi_dev, state);
> 
> Remove all checks for acpi_dev == NULL within these callbacks.
> 
> Also remove the checks for acpi_driver_data(acpi_dev) == NULL. None of
> these checks could fail unless the driver does something strange
> (which none of them do), the acpi core did something terribly wrong,
> or we have a memory corruption issue. If this does happen then it's
> best to dereference the pointer and crash noisily.
> 
> Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>

Reviewed-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Minor comments below.

> ---
>  drivers/acpi/ac.c                       |   18 ++++--------------
>  drivers/acpi/acpi_memhotplug.c          |   11 ++---------
>  drivers/acpi/battery.c                  |   16 +++++-----------
>  drivers/acpi/container.c                |    8 +-------
>  drivers/acpi/ec.c                       |    6 +-----
>  drivers/acpi/fan.c                      |   12 ------------
>  drivers/acpi/power.c                    |   17 ++---------------
>  drivers/acpi/processor_core.c           |    7 +------
>  drivers/acpi/sbs.c                      |    7 +------
>  drivers/acpi/sbshc.c                    |    9 +--------
>  drivers/acpi/thermal.c                  |   19 +++----------------
>  drivers/acpi/video.c                    |   14 ++------------
>  drivers/hwmon/hp_accel.c                |    6 ------
>  drivers/platform/x86/asus-laptop.c      |    6 ------
>  drivers/platform/x86/asus_acpi.c        |    6 ------
>  drivers/platform/x86/eeepc-laptop.c     |    5 -----
>  drivers/platform/x86/fujitsu-laptop.c   |    6 ------
>  drivers/platform/x86/intel_menlow.c     |    6 ------
>  drivers/platform/x86/panasonic-laptop.c |    9 ---------
>  19 files changed, 23 insertions(+), 165 deletions(-)
> 
> diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
> index 98b9690..7725bda 100644
> --- a/drivers/acpi/ac.c
> +++ b/drivers/acpi/ac.c
> @@ -256,11 +256,8 @@ static void acpi_ac_notify(struct acpi_device *device, u32 event)
>  static int acpi_ac_add(struct acpi_device *device)
>  {
>  	int result = 0;
> -	struct acpi_ac *ac = NULL;
> -
> +	struct acpi_ac *ac;
>  
> -	if (!device)
> -		return -EINVAL;
>  
>  	ac = kzalloc(sizeof(struct acpi_ac), GFP_KERNEL);
>  	if (!ac)
> @@ -306,11 +303,9 @@ static int acpi_ac_add(struct acpi_device *device)
>  
>  static int acpi_ac_resume(struct acpi_device *device)
>  {
> -	struct acpi_ac *ac;
> +	struct acpi_ac *ac = acpi_driver_data(device);
>  	unsigned old_state;
> -	if (!device || !acpi_driver_data(device))
> -		return -EINVAL;
> -	ac = acpi_driver_data(device);
> +
>  	old_state = ac->state;
>  	if (acpi_ac_get_state(ac))
>  		return 0;
> @@ -323,13 +318,8 @@ static int acpi_ac_resume(struct acpi_device *device)
>  
>  static int acpi_ac_remove(struct acpi_device *device, int type)
>  {
> -	struct acpi_ac *ac = NULL;
> -
> -
> -	if (!device || !acpi_driver_data(device))
> -		return -EINVAL;
> +	struct acpi_ac *ac = acpi_driver_data(device);
>  
> -	ac = acpi_driver_data(device);
>  
>  #ifdef CONFIG_ACPI_SYSFS_POWER
>  	if (ac->charger.dev)
> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
> index 28ccdbc..71085b9 100644
> --- a/drivers/acpi/acpi_memhotplug.c
> +++ b/drivers/acpi/acpi_memhotplug.c
> @@ -401,11 +401,8 @@ static void acpi_memory_device_notify(acpi_handle handle, u32 event, void *data)
>  static int acpi_memory_device_add(struct acpi_device *device)
>  {
>  	int result;
> -	struct acpi_memory_device *mem_device = NULL;
> -
> +	struct acpi_memory_device *mem_device;
>  
> -	if (!device)
> -		return -EINVAL;
>  
>  	mem_device = kzalloc(sizeof(struct acpi_memory_device), GFP_KERNEL);
>  	if (!mem_device)
> @@ -450,13 +447,9 @@ static int acpi_memory_device_add(struct acpi_device *device)
>  
>  static int acpi_memory_device_remove(struct acpi_device *device, int type)
>  {
> -	struct acpi_memory_device *mem_device = NULL;
> +	struct acpi_memory_device *mem_device = acpi_driver_data(device);
>  
>  
> -	if (!device || !acpi_driver_data(device))
> -		return -EINVAL;
> -
> -	mem_device = acpi_driver_data(device);
>  	kfree(mem_device);
>  
>  	return 0;
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index 3f4602b..90e39d9 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -840,9 +840,8 @@ static void acpi_battery_notify(struct acpi_device *device, u32 event)
>  static int acpi_battery_add(struct acpi_device *device)
>  {
>  	int result = 0;
> -	struct acpi_battery *battery = NULL;
> -	if (!device)
> -		return -EINVAL;
> +	struct acpi_battery *battery;
> +
>  	battery = kzalloc(sizeof(struct acpi_battery), GFP_KERNEL);
>  	if (!battery)
>  		return -ENOMEM;
> @@ -870,11 +869,8 @@ static int acpi_battery_add(struct acpi_device *device)
>  
>  static int acpi_battery_remove(struct acpi_device *device, int type)
>  {
> -	struct acpi_battery *battery = NULL;
> +	struct acpi_battery *battery = acpi_driver_data(device);
>  
> -	if (!device || !acpi_driver_data(device))
> -		return -EINVAL;
> -	battery = acpi_driver_data(device);
>  #ifdef CONFIG_ACPI_PROCFS_POWER
>  	acpi_battery_remove_fs(device);
>  #endif
> @@ -889,10 +885,8 @@ static int acpi_battery_remove(struct acpi_device *device, int type)
>  /* this is needed to learn about changes made in suspended state */
>  static int acpi_battery_resume(struct acpi_device *device)
>  {
> -	struct acpi_battery *battery;
> -	if (!device)
> -		return -EINVAL;
> -	battery = acpi_driver_data(device);
> +	struct acpi_battery *battery = acpi_driver_data(device);
> +
>  	battery->update_time = 0;
>  	acpi_battery_update(battery);
>  	return 0;
> diff --git a/drivers/acpi/container.c b/drivers/acpi/container.c
> index 642bb30..30c4700 100644
> --- a/drivers/acpi/container.c
> +++ b/drivers/acpi/container.c
> @@ -97,11 +97,6 @@ static int acpi_container_add(struct acpi_device *device)
>  	struct acpi_container *container;
>  
>  
> -	if (!device) {
> -		printk(KERN_ERR PREFIX "device is NULL\n");
> -		return -EINVAL;
> -	}
> -
>  	container = kzalloc(sizeof(struct acpi_container), GFP_KERNEL);
>  	if (!container)
>  		return -ENOMEM;
> @@ -120,9 +115,8 @@ static int acpi_container_add(struct acpi_device *device)
>  static int acpi_container_remove(struct acpi_device *device, int type)
>  {
>  	acpi_status status = AE_OK;
> -	struct acpi_container *pc = NULL;
> +	struct acpi_container *pc = acpi_driver_data(device);
>  
> -	pc = acpi_driver_data(device);
>  	kfree(pc);
>  	return status;
>  }
> diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
> index add6621..5e93ae3 100644
> --- a/drivers/acpi/ec.c
> +++ b/drivers/acpi/ec.c
> @@ -837,13 +837,9 @@ static int acpi_ec_add(struct acpi_device *device)
>  
>  static int acpi_ec_remove(struct acpi_device *device, int type)
>  {
> -	struct acpi_ec *ec;
> +	struct acpi_ec *ec = acpi_driver_data(device);
>  	struct acpi_ec_query_handler *handler, *tmp;
>  
> -	if (!device)
> -		return -EINVAL;
> -
> -	ec = acpi_driver_data(device);
>  	ec_remove_handlers(ec);
>  	mutex_lock(&ec->lock);
>  	list_for_each_entry_safe(handler, tmp, &ec->list, node) {
> diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
> index f419849..d40e214 100644
> --- a/drivers/acpi/fan.c
> +++ b/drivers/acpi/fan.c
> @@ -244,9 +244,6 @@ static int acpi_fan_add(struct acpi_device *device)
>  	int state = 0;
>  	struct thermal_cooling_device *cdev;
>  
> -	if (!device)
> -		return -EINVAL;
> -
>  	strcpy(acpi_device_name(device), "Fan");
>  	strcpy(acpi_device_class(device), ACPI_FAN_CLASS);
>  
> @@ -300,9 +297,6 @@ static int acpi_fan_remove(struct acpi_device *device, int type)
>  {
>  	struct thermal_cooling_device *cdev = acpi_driver_data(device);
>  
> -	if (!device || !cdev)
> -		return -EINVAL;
> -
>  	acpi_fan_remove_fs(device);
>  	sysfs_remove_link(&device->dev.kobj, "thermal_cooling");
>  	sysfs_remove_link(&cdev->device.kobj, "device");
> @@ -313,9 +307,6 @@ static int acpi_fan_remove(struct acpi_device *device, int type)
>  
>  static int acpi_fan_suspend(struct acpi_device *device, pm_message_t state)
>  {
> -	if (!device)
> -		return -EINVAL;
> -
>  	acpi_bus_set_power(device->handle, ACPI_STATE_D0);
>  
>  	return AE_OK;
> @@ -326,9 +317,6 @@ static int acpi_fan_resume(struct acpi_device *device)
>  	int result = 0;
>  	int power_state = 0;
>  
> -	if (!device)
> -		return -EINVAL;
> -
>  	result = acpi_bus_get_power(device->handle, &power_state);
>  	if (result) {
>  		printk(KERN_ERR PREFIX
> diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
> index e86603f..2c3d844 100644
> --- a/drivers/acpi/power.c
> +++ b/drivers/acpi/power.c
> @@ -639,9 +639,6 @@ static int acpi_power_add(struct acpi_device *device)
>  	struct acpi_buffer buffer = { sizeof(acpi_object), &acpi_object };
>  
>  
> -	if (!device)
> -		return -EINVAL;
> -
>  	resource = kzalloc(sizeof(struct acpi_power_resource), GFP_KERNEL);
>  	if (!resource)
>  		return -ENOMEM;
> @@ -695,15 +692,10 @@ static int acpi_power_add(struct acpi_device *device)
>  
>  static int acpi_power_remove(struct acpi_device *device, int type)
>  {
> -	struct acpi_power_resource *resource = NULL;
> +	struct acpi_power_resource *resource = acpi_driver_data(device);
>  	struct list_head *node, *next;
>  
>  
> -	if (!device || !acpi_driver_data(device))
> -		return -EINVAL;
> -
> -	resource = acpi_driver_data(device);
> -
>  	acpi_power_remove_fs(device);
>  
>  	mutex_lock(&resource->resource_lock);
> @@ -722,14 +714,9 @@ static int acpi_power_remove(struct acpi_device *device, int type)
>  static int acpi_power_resume(struct acpi_device *device)
>  {
>  	int result = 0, state;
> -	struct acpi_power_resource *resource = NULL;
> +	struct acpi_power_resource *resource = acpi_driver_data(device);
>  	struct acpi_power_reference *ref;
>  
> -	if (!device || !acpi_driver_data(device))
> -		return -EINVAL;
> -
> -	resource = acpi_driver_data(device);
> -
>  	result = acpi_power_get_state(device->handle, &state);
>  	if (result)
>  		return result;
> diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
> index c2d4d6e..38ad5ef 100644
> --- a/drivers/acpi/processor_core.c
> +++ b/drivers/acpi/processor_core.c
> @@ -888,13 +888,8 @@ err_free_cpumask:
>  
>  static int acpi_processor_remove(struct acpi_device *device, int type)
>  {
> -	struct acpi_processor *pr = NULL;
> -
> -
> -	if (!device || !acpi_driver_data(device))
> -		return -EINVAL;
> +	struct acpi_processor *pr = acpi_driver_data(device);
>  
> -	pr = acpi_driver_data(device);
>  
>  	if (pr->id >= nr_cpu_ids)
>  		goto free;
> diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c
> index 52b9db8..4c7ab39 100644
> --- a/drivers/acpi/sbs.c
> +++ b/drivers/acpi/sbs.c
> @@ -960,12 +960,9 @@ static int acpi_sbs_add(struct acpi_device *device)
>  
>  static int acpi_sbs_remove(struct acpi_device *device, int type)
>  {
> -	struct acpi_sbs *sbs;
> +	struct acpi_sbs *sbs = acpi_driver_data(device);
>  	int id;
>  
> -	if (!device)
> -		return -EINVAL;
> -	sbs = acpi_driver_data(device);
>  	if (!sbs)
>  		return -EINVAL;
>  	mutex_lock(&sbs->lock);
> @@ -996,8 +993,6 @@ static void acpi_sbs_rmdirs(void)
>  static int acpi_sbs_resume(struct acpi_device *device)
>  {
>  	struct acpi_sbs *sbs;
> -	if (!device)
> -		return -EINVAL;

Nit: I'd pull the sbs initialization up and use acpi_driver_data()
as in acpi_sbs_remove().  And add a blank line.

>  	sbs = device->driver_data;
>  	acpi_sbs_callback(sbs);
>  	return 0;
> diff --git a/drivers/acpi/sbshc.c b/drivers/acpi/sbshc.c
> index 8d89337..9e06e9c 100644
> --- a/drivers/acpi/sbshc.c
> +++ b/drivers/acpi/sbshc.c
> @@ -262,9 +262,6 @@ static int acpi_smbus_hc_add(struct acpi_device *device)
>  	unsigned long long val;
>  	struct acpi_smb_hc *hc;
>  
> -	if (!device)
> -		return -EINVAL;
> -
>  	status = acpi_evaluate_integer(device->handle, "_EC", NULL, &val);
>  	if (ACPI_FAILURE(status)) {
>  		printk(KERN_ERR PREFIX "error obtaining _EC.\n");
> @@ -296,12 +293,8 @@ extern void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit);
>  
>  static int acpi_smbus_hc_remove(struct acpi_device *device, int type)
>  {
> -	struct acpi_smb_hc *hc;
> -
> -	if (!device)
> -		return -EINVAL;
> +	struct acpi_smb_hc *hc = acpi_driver_data(device);
>  
> -	hc = acpi_driver_data(device);
>  	acpi_ec_remove_query_handler(hc->ec, hc->query_bit);
>  	kfree(hc);
>  	return 0;
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index 65f6781..fca61a1 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -1361,12 +1361,9 @@ static void acpi_thermal_guess_offset(struct acpi_thermal *tz)
>  static int acpi_thermal_add(struct acpi_device *device)
>  {
>  	int result = 0;
> -	struct acpi_thermal *tz = NULL;
> +	struct acpi_thermal *tz;
>  
>  
> -	if (!device)
> -		return -EINVAL;
> -
>  	tz = kzalloc(sizeof(struct acpi_thermal), GFP_KERNEL);
>  	if (!tz)
>  		return -ENOMEM;
> @@ -1408,12 +1405,7 @@ end:
>  
>  static int acpi_thermal_remove(struct acpi_device *device, int type)
>  {
> -	struct acpi_thermal *tz = NULL;
> -
> -	if (!device || !acpi_driver_data(device))
> -		return -EINVAL;
> -
> -	tz = acpi_driver_data(device);
> +	struct acpi_thermal *tz = acpi_driver_data(device);
>  
>  	acpi_thermal_remove_fs(device);
>  	acpi_thermal_unregister_thermal_zone(tz);
> @@ -1424,15 +1416,10 @@ static int acpi_thermal_remove(struct acpi_device *device, int type)
>  
>  static int acpi_thermal_resume(struct acpi_device *device)
>  {
> -	struct acpi_thermal *tz = NULL;
> +	struct acpi_thermal *tz = acpi_driver_data(device);
>  	int i, j, power_state, result;
>  
>  
> -	if (!device || !acpi_driver_data(device))
> -		return -EINVAL;
> -
> -	tz = acpi_driver_data(device);
> -
>  	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
>  		if (!(&tz->trips.active[i]))
>  			break;
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index f405807..576a600 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -2207,15 +2207,10 @@ static void acpi_video_device_notify(acpi_handle handle, u32 event, void *data)
>  static int instance;
>  static int acpi_video_resume(struct acpi_device *device)
>  {
> -	struct acpi_video_bus *video;
> +	struct acpi_video_bus *video = acpi_driver_data(device);
>  	struct acpi_video_device *video_device;
>  	int i;
>  
> -	if (!device || !acpi_driver_data(device))
> -		return -EINVAL;
> -
> -	video = acpi_driver_data(device);
> -
>  	for (i = 0; i < video->attached_count; i++) {
>  		video_device = video->attached_array[i].bind_info;
>  		if (video_device && video_device->backlight)
> @@ -2351,13 +2346,8 @@ static int acpi_video_bus_add(struct acpi_device *device)
>  
>  static int acpi_video_bus_remove(struct acpi_device *device, int type)
>  {
> -	struct acpi_video_bus *video = NULL;
> -
> -
> -	if (!device || !acpi_driver_data(device))
> -		return -EINVAL;
> +	struct acpi_video_bus *video = acpi_driver_data(device);
>  
> -	video = acpi_driver_data(device);
>  
>  	acpi_video_bus_stop_devices(video);
>  	acpi_video_bus_put_devices(video);
> diff --git a/drivers/hwmon/hp_accel.c b/drivers/hwmon/hp_accel.c
> index 6679854..c8d3c88 100644
> --- a/drivers/hwmon/hp_accel.c
> +++ b/drivers/hwmon/hp_accel.c
> @@ -275,9 +275,6 @@ static int lis3lv02d_add(struct acpi_device *device)
>  {
>  	int ret;
>  
> -	if (!device)
> -		return -EINVAL;
> -
>  	lis3_dev.bus_priv = device;
>  	lis3_dev.init = lis3lv02d_acpi_init;
>  	lis3_dev.read = lis3lv02d_acpi_read;
> @@ -315,9 +312,6 @@ static int lis3lv02d_add(struct acpi_device *device)
>  
>  static int lis3lv02d_remove(struct acpi_device *device, int type)
>  {
> -	if (!device)
> -		return -EINVAL;
> -
>  	lis3lv02d_joystick_disable();
>  	lis3lv02d_poweroff(&lis3_dev);

Same basic issue as the kfree comments below; the fact that this
remove method doesn't reference "device" is a symptom of a driver
structure problem (beyond the scope of this patch, obviously).

> diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
> index b39d2bb..8af43e9 100644
> --- a/drivers/platform/x86/asus-laptop.c
> +++ b/drivers/platform/x86/asus-laptop.c
> @@ -1240,9 +1240,6 @@ static int asus_hotk_add(struct acpi_device *device)
>  {
>  	int result;
>  
> -	if (!device)
> -		return -EINVAL;
> -
>  	pr_notice("Asus Laptop Support version %s\n",
>  	       ASUS_LAPTOP_VERSION);
>  
> @@ -1306,9 +1303,6 @@ end:
>  
>  static int asus_hotk_remove(struct acpi_device *device, int type)
>  {
> -	if (!device || !acpi_driver_data(device))
> -		return -EINVAL;
> -
>  	kfree(hotk->name);
>  	kfree(hotk);

Separate issue: we should kfree acpi_driver_data(device) here,
not "hotk".  In general, the "remove" should deal with per-device
data, not globals.  This driver only deals with a single instance,
so they happen to be the same in this case, but you have to read
more of the driver to verify that.

>  
> diff --git a/drivers/platform/x86/asus_acpi.c b/drivers/platform/x86/asus_acpi.c
> index ddf5240..25a7d57 100644
> --- a/drivers/platform/x86/asus_acpi.c
> +++ b/drivers/platform/x86/asus_acpi.c
> @@ -1334,9 +1334,6 @@ static int asus_hotk_add(struct acpi_device *device)
>  	acpi_status status = AE_OK;
>  	int result;
>  
> -	if (!device)
> -		return -EINVAL;
> -
>  	printk(KERN_NOTICE "Asus Laptop ACPI Extras version %s\n",
>  	       ASUS_ACPI_VERSION);
>  
> @@ -1392,9 +1389,6 @@ end:
>  
>  static int asus_hotk_remove(struct acpi_device *device, int type)
>  {
> -	if (!device || !acpi_driver_data(device))
> -		return -EINVAL;
> -
>  	asus_hotk_remove_fs(device);
>  
>  	kfree(hotk);

The kfree comment above applies here, too.

> diff --git a/drivers/platform/x86/eeepc-laptop.c b/drivers/platform/x86/eeepc-laptop.c
> index da3c08b..e7f14a4 100644
> --- a/drivers/platform/x86/eeepc-laptop.c
> +++ b/drivers/platform/x86/eeepc-laptop.c
> @@ -1194,8 +1194,6 @@ static int eeepc_hotk_add(struct acpi_device *device)
>  	struct device *dev;
>  	int result;
>  
> -	if (!device)
> -		return -EINVAL;
>  	pr_notice(EEEPC_HOTK_NAME "\n");
>  	ehotk = kzalloc(sizeof(struct eeepc_hotk), GFP_KERNEL);
>  	if (!ehotk)
> @@ -1276,9 +1274,6 @@ fail_platform_driver:
>  
>  static int eeepc_hotk_remove(struct acpi_device *device, int type)
>  {
> -	if (!device || !acpi_driver_data(device))
> -		return -EINVAL;
> -
>  	eeepc_backlight_exit();
>  	eeepc_rfkill_exit();
>  	eeepc_input_exit();

The current upstream version of this function also has the kfree
issue.  You're patching a different version, so it might be fixed
there already.  But it seems like all these hotkey drivers (and the
accelerometer driver) use the same pattern of assuming they'll only
see a single device, and then they make assumptions like "hotk ==
acpi_driver_data(device)".  That's just a bad example for people
to follow, and might not even be true for things like accelerometers
where you could imagine having more than one.

> diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
> index f35aee5..2ab0318 100644
> --- a/drivers/platform/x86/fujitsu-laptop.c
> +++ b/drivers/platform/x86/fujitsu-laptop.c
> @@ -657,9 +657,6 @@ static int acpi_fujitsu_add(struct acpi_device *device)
>  	struct input_dev *input;
>  	int error;
>  
> -	if (!device)
> -		return -EINVAL;
> -
>  	fujitsu->acpi_handle = device->handle;
>  	sprintf(acpi_device_name(device), "%s", ACPI_FUJITSU_DEVICE_NAME);
>  	sprintf(acpi_device_class(device), "%s", ACPI_FUJITSU_CLASS);
> @@ -813,9 +810,6 @@ static int acpi_fujitsu_hotkey_add(struct acpi_device *device)
>  	int error;
>  	int i;
>  
> -	if (!device)
> -		return -EINVAL;
> -
>  	fujitsu_hotkey->acpi_handle = device->handle;
>  	sprintf(acpi_device_name(device), "%s",
>  		ACPI_FUJITSU_HOTKEY_DEVICE_NAME);
> diff --git a/drivers/platform/x86/intel_menlow.c b/drivers/platform/x86/intel_menlow.c
> index 29432a5..58de8fd 100644
> --- a/drivers/platform/x86/intel_menlow.c
> +++ b/drivers/platform/x86/intel_menlow.c
> @@ -156,9 +156,6 @@ static int intel_menlow_memory_add(struct acpi_device *device)
>  	acpi_handle dummy;
>  	struct thermal_cooling_device *cdev;
>  
> -	if (!device)
> -		return -EINVAL;
> -
>  	status = acpi_get_handle(device->handle, MEMORY_GET_BANDWIDTH, &dummy);
>  	if (ACPI_FAILURE(status))
>  		goto end;
> @@ -200,9 +197,6 @@ static int intel_menlow_memory_remove(struct acpi_device *device, int type)
>  {
>  	struct thermal_cooling_device *cdev = acpi_driver_data(device);
>  
> -	if (!device || !cdev)
> -		return -EINVAL;
> -
>  	sysfs_remove_link(&device->dev.kobj, "thermal_cooling");
>  	sysfs_remove_link(&cdev->device.kobj, "device");
>  	thermal_cooling_device_unregister(cdev);
> diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
> index fe7cf01..953b60c 100644
> --- a/drivers/platform/x86/panasonic-laptop.c
> +++ b/drivers/platform/x86/panasonic-laptop.c
> @@ -588,9 +588,6 @@ static int acpi_pcc_hotkey_resume(struct acpi_device *device)
>  	struct pcc_acpi *pcc = acpi_driver_data(device);
>  	acpi_status status = AE_OK;
>  
> -	if (device == NULL || pcc == NULL)
> -		return -EINVAL;
> -
>  	ACPI_DEBUG_PRINT((ACPI_DB_ERROR, "Sticky mode restore: %d\n",
>  			  pcc->sticky_mode));
>  
> @@ -604,9 +601,6 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
>  	struct pcc_acpi *pcc;
>  	int num_sifr, result;
>  
> -	if (!device)
> -		return -EINVAL;
> -
>  	num_sifr = acpi_pcc_get_sqty(device);
>  
>  	if (num_sifr > 255) {
> @@ -703,9 +697,6 @@ static int acpi_pcc_hotkey_remove(struct acpi_device *device, int type)
>  {
>  	struct pcc_acpi *pcc = acpi_driver_data(device);
>  
> -	if (!device || !pcc)
> -		return -EINVAL;
> -
>  	sysfs_remove_group(&device->dev.kobj, &pcc_attr_group);
>  
>  	backlight_device_unregister(pcc->backlight);



^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH 4/4] ACPI: Remove uneccesary acpi_disabled checks in acpi drivers
  2009-09-14 16:09 ` [PATCH 4/4] ACPI: Remove uneccesary acpi_disabled " Alan Jenkins
@ 2009-09-14 19:46   ` Bjorn Helgaas
  2009-09-15 10:11     ` [PATCHv2 4/4] ACPI: Remove unneccesary " Alan Jenkins
  0 siblings, 1 reply; 9+ messages in thread
From: Bjorn Helgaas @ 2009-09-14 19:46 UTC (permalink / raw)
  To: Alan Jenkins; +Cc: lenb, linux-acpi

On Monday 14 September 2009 10:09:29 am Alan Jenkins wrote:
> acpi_bus_register_driver() already checks acpi_disabled, so acpi bus
> drivers don't need to.  Also, pass on the return value when it fails
> instead of assuming ENODEV.
> 
> Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>

Reviewed-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Minor comments below.  Thanks for doing this.

> ---
>  drivers/acpi/ac.c                       |    5 +----
>  drivers/acpi/acpi_memhotplug.c          |    2 +-
>  drivers/acpi/battery.c                  |    2 --
>  drivers/acpi/container.c                |    4 ++--
>  drivers/acpi/ec.c                       |    2 +-
>  drivers/acpi/fan.c                      |    2 +-
>  drivers/acpi/pci_link.c                 |   11 ++++++-----
>  drivers/acpi/power.c                    |    2 +-
>  drivers/acpi/sbs.c                      |    4 +---
>  drivers/acpi/sbshc.c                    |    7 +------
>  drivers/acpi/thermal.c                  |    2 +-
>  drivers/acpi/video.c                    |    2 +-
>  drivers/hwmon/hp_accel.c                |    3 ---
>  drivers/input/misc/atlas_btns.c         |    5 +----
>  drivers/platform/x86/asus-laptop.c      |    3 ---
>  drivers/platform/x86/asus_acpi.c        |    3 ---
>  drivers/platform/x86/eeepc-laptop.c     |    2 --
>  drivers/platform/x86/panasonic-laptop.c |    7 ++-----
>  drivers/platform/x86/wmi.c              |    3 ---
>  19 files changed, 20 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
> index 7725bda..142744a 100644
> --- a/drivers/acpi/ac.c
> +++ b/drivers/acpi/ac.c
> @@ -338,9 +338,6 @@ static int __init acpi_ac_init(void)
>  {
>  	int result;
>  
> -	if (acpi_disabled)
> -		return -ENODEV;
> -
>  #ifdef CONFIG_ACPI_PROCFS_POWER
>  	acpi_ac_dir = acpi_lock_ac_dir();
>  	if (!acpi_ac_dir)
> @@ -352,7 +349,7 @@ static int __init acpi_ac_init(void)
>  #ifdef CONFIG_ACPI_PROCFS_POWER
>  		acpi_unlock_ac_dir(acpi_ac_dir);
>  #endif

Idle thought: I wonder if it'd be worth having a driver method
for procfs/sysfs management.  There are a bunch of drivers that
do this, and it's always a pain to figure out whether they do
the right thing and in the right order.

> -		return -ENODEV;
> +		return result;
>  	}
>  
>  	return 0;
> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
> index 71085b9..30a4600 100644
> --- a/drivers/acpi/acpi_memhotplug.c
> +++ b/drivers/acpi/acpi_memhotplug.c
> @@ -526,7 +526,7 @@ static int __init acpi_memory_device_init(void)
>  	result = acpi_bus_register_driver(&acpi_memory_device_driver);
>  
>  	if (result < 0)
> -		return -ENODEV;
> +		return result;
>  
>  	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
>  				     ACPI_UINT32_MAX,
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index 90e39d9..f451dc0 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -907,8 +907,6 @@ static struct acpi_driver acpi_battery_driver = {
>  
>  static void __init acpi_battery_init_async(void *unused, async_cookie_t cookie)
>  {
> -	if (acpi_disabled)
> -		return;
>  #ifdef CONFIG_ACPI_PROCFS_POWER
>  	acpi_battery_dir = acpi_lock_battery_dir();
>  	if (!acpi_battery_dir)
> diff --git a/drivers/acpi/container.c b/drivers/acpi/container.c
> index 30c4700..c54b69b 100644
> --- a/drivers/acpi/container.c
> +++ b/drivers/acpi/container.c
> @@ -245,7 +245,7 @@ static int __init acpi_container_init(void)
>  
>  	result = acpi_bus_register_driver(&acpi_container_driver);
>  	if (result < 0) {
> -		return (result);
> +		return result;
>  	}
>  
>  	/* register notify handler to every container device */
> @@ -254,7 +254,7 @@ static int __init acpi_container_init(void)
>  			    ACPI_UINT32_MAX,
>  			    container_walk_namespace_cb, &action, NULL);
>  
> -	return (0);
> +	return 0;
>  }
>  
>  static void __exit acpi_container_exit(void)
> diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
> index 5e93ae3..26c58a3 100644
> --- a/drivers/acpi/ec.c
> +++ b/drivers/acpi/ec.c
> @@ -1010,7 +1010,7 @@ int __init acpi_ec_init(void)
>  	result = acpi_bus_register_driver(&acpi_ec_driver);
>  	if (result < 0) {
>  		remove_proc_entry(ACPI_EC_CLASS, acpi_root_dir);
> -		return -ENODEV;
> +		return result;
>  	}
>  
>  	return result;
> diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
> index d40e214..947556e 100644
> --- a/drivers/acpi/fan.c
> +++ b/drivers/acpi/fan.c
> @@ -345,7 +345,7 @@ static int __init acpi_fan_init(void)
>  	result = acpi_bus_register_driver(&acpi_fan_driver);
>  	if (result < 0) {
>  		remove_proc_entry(ACPI_FAN_CLASS, acpi_root_dir);
> -		return -ENODEV;
> +		return result;
>  	}
>  
>  	return 0;
> diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
> index 394ae89..4a1e22a 100644
> --- a/drivers/acpi/pci_link.c
> +++ b/drivers/acpi/pci_link.c
> @@ -769,9 +769,7 @@ static int irqrouter_resume(struct sys_device *dev)
>  
>  static int acpi_pci_link_remove(struct acpi_device *device, int type)
>  {
> -	struct acpi_pci_link *link;
> -
> -	link = acpi_driver_data(device);
> +	struct acpi_pci_link *link = acpi_driver_data(device);
>  
>  	mutex_lock(&acpi_link_lock);
>  	list_del(&link->list);
> @@ -900,6 +898,8 @@ device_initcall(irqrouter_init_sysfs);
>  
>  static int __init acpi_pci_link_init(void)
>  {
> +	int result;
> +
>  	if (acpi_noirq)
>  		return 0;
>  
> @@ -911,8 +911,9 @@ static int __init acpi_pci_link_init(void)
>  			acpi_irq_balance = 0;
>  	}
>  
> -	if (acpi_bus_register_driver(&acpi_pci_link_driver) < 0)
> -		return -ENODEV;
> +	result = acpi_bus_register_driver(&acpi_pci_link_driver);
> +	if (result < 0)
> +		return result;
>  
>  	return 0;
>  }
> diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
> index 2c3d844..3cd8d45 100644
> --- a/drivers/acpi/power.c
> +++ b/drivers/acpi/power.c
> @@ -747,7 +747,7 @@ int __init acpi_power_init(void)
>  	result = acpi_bus_register_driver(&acpi_power_driver);
>  	if (result < 0) {
>  		remove_proc_entry(ACPI_POWER_CLASS, acpi_root_dir);
> -		return -ENODEV;
> +		return result;
>  	}
>  
>  	return 0;
> diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c
> index 4c7ab39..192e613 100644
> --- a/drivers/acpi/sbs.c
> +++ b/drivers/acpi/sbs.c
> @@ -1013,8 +1013,6 @@ static int __init acpi_sbs_init(void)
>  {
>  	int result = 0;
>  
> -	if (acpi_disabled)
> -		return -ENODEV;
>  #ifdef CONFIG_ACPI_PROCFS_POWER
>  	acpi_ac_dir = acpi_lock_ac_dir();
>  	if (!acpi_ac_dir)
> @@ -1028,7 +1026,7 @@ static int __init acpi_sbs_init(void)
>  	result = acpi_bus_register_driver(&acpi_sbs_driver);
>  	if (result < 0) {
>  		acpi_sbs_rmdirs();
> -		return -ENODEV;
> +		return result;
>  	}
>  	return 0;
>  }
> diff --git a/drivers/acpi/sbshc.c b/drivers/acpi/sbshc.c
> index 9e06e9c..c07cabd 100644
> --- a/drivers/acpi/sbshc.c
> +++ b/drivers/acpi/sbshc.c
> @@ -302,12 +302,7 @@ static int acpi_smbus_hc_remove(struct acpi_device *device, int type)
>  
>  static int __init acpi_smb_hc_init(void)
>  {
> -	int result;
> -
> -	result = acpi_bus_register_driver(&acpi_smb_hc_driver);
> -	if (result < 0)
> -		return -ENODEV;
> -	return 0;
> +	return acpi_bus_register_driver(&acpi_smb_hc_driver);
>  }
>  
>  static void __exit acpi_smb_hc_exit(void)
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index fca61a1..767e57d 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -1534,7 +1534,7 @@ static int __init acpi_thermal_init(void)
>  	result = acpi_bus_register_driver(&acpi_thermal_driver);
>  	if (result < 0) {
>  		remove_proc_entry(ACPI_THERMAL_CLASS, acpi_root_dir);
> -		return -ENODEV;
> +		return result;
>  	}
>  
>  	return 0;
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index 576a600..0433df3 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -2398,7 +2398,7 @@ int acpi_video_register(void)
>  	result = acpi_bus_register_driver(&acpi_video_bus);
>  	if (result < 0) {
>  		remove_proc_entry(ACPI_VIDEO_CLASS, acpi_root_dir);
> -		return -ENODEV;
> +		return result;
>  	}
>  
>  	/*
> diff --git a/drivers/hwmon/hp_accel.c b/drivers/hwmon/hp_accel.c
> index c8d3c88..38f8959 100644
> --- a/drivers/hwmon/hp_accel.c
> +++ b/drivers/hwmon/hp_accel.c
> @@ -357,9 +357,6 @@ static int __init lis3lv02d_init_module(void)
>  {
>  	int ret;
>  
> -	if (acpi_disabled)
> -		return -ENODEV;
> -
>  	ret = acpi_bus_register_driver(&lis3lv02d_driver);
>  	if (ret < 0)
>  		return ret;
> diff --git a/drivers/input/misc/atlas_btns.c b/drivers/input/misc/atlas_btns.c
> index 1b87191..36740d1 100644
> --- a/drivers/input/misc/atlas_btns.c
> +++ b/drivers/input/misc/atlas_btns.c
> @@ -156,13 +156,10 @@ static int __init atlas_acpi_init(void)
>  {
>  	int result;
>  
> -	if (acpi_disabled)
> -		return -ENODEV;
> -
>  	result = acpi_bus_register_driver(&atlas_acpi_driver);
>  	if (result < 0) {
>  		printk(KERN_ERR "Atlas ACPI: Unable to register driver\n");

I think this printk is pointless.

> -		return -ENODEV;
> +		return result;
>  	}
>  
>  	return 0;
> diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
> index 8af43e9..4234edb 100644
> --- a/drivers/platform/x86/asus-laptop.c
> +++ b/drivers/platform/x86/asus-laptop.c
> @@ -1438,9 +1438,6 @@ static int __init asus_laptop_init(void)
>  {
>  	int result;
>  
> -	if (acpi_disabled)
> -		return -ENODEV;
> -
>  	result = acpi_bus_register_driver(&asus_hotk_driver);
>  	if (result < 0)
>  		return result;
> diff --git a/drivers/platform/x86/asus_acpi.c b/drivers/platform/x86/asus_acpi.c
> index 25a7d57..502692b 100644
> --- a/drivers/platform/x86/asus_acpi.c
> +++ b/drivers/platform/x86/asus_acpi.c
> @@ -1416,9 +1416,6 @@ static int __init asus_acpi_init(void)
>  {
>  	int result;
>  
> -	if (acpi_disabled)
> -		return -ENODEV;
> -
>  	asus_proc_dir = proc_mkdir(PROC_ASUS, acpi_root_dir);
>  	if (!asus_proc_dir) {
>  		printk(KERN_ERR "Asus ACPI: Unable to create /proc entry\n");
> diff --git a/drivers/platform/x86/eeepc-laptop.c b/drivers/platform/x86/eeepc-laptop.c
> index e7f14a4..eba3772 100644
> --- a/drivers/platform/x86/eeepc-laptop.c
> +++ b/drivers/platform/x86/eeepc-laptop.c
> @@ -1291,8 +1291,6 @@ static int __init eeepc_laptop_init(void)
>  {
>  	int result;
>  
> -	if (acpi_disabled)
> -		return -ENODEV;
>  	result = acpi_bus_register_driver(&eeepc_hotk_driver);
>  	if (result < 0)
>  		return result;
> diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
> index 953b60c..ea28882 100644
> --- a/drivers/platform/x86/panasonic-laptop.c
> +++ b/drivers/platform/x86/panasonic-laptop.c
> @@ -678,16 +678,13 @@ out_hotkey:
>  
>  static int __init acpi_pcc_init(void)
>  {
> -	int result = 0;
> -
> -	if (acpi_disabled)
> -		return -ENODEV;
> +	int result;
>  
>  	result = acpi_bus_register_driver(&acpi_pcc_driver);
>  	if (result < 0) {
>  		ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
>  				  "Error registering hotkey driver\n"));

Another (IMHO) pointless printk.

> -		return -ENODEV;
> +		return result;
>  	}
>  
>  	return 0;
> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
> index 177f8d7..d889f51 100644
> --- a/drivers/platform/x86/wmi.c
> +++ b/drivers/platform/x86/wmi.c
> @@ -702,9 +702,6 @@ static int __init acpi_wmi_init(void)
>  
>  	INIT_LIST_HEAD(&wmi_blocks.list);
>  
> -	if (acpi_disabled)
> -		return -ENODEV;
> -
>  	result = acpi_bus_register_driver(&acpi_wmi_driver);
>  
>  	if (result < 0) {



^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCHv2 3/4] ACPI: Remove redundant NULL checks in acpi drivers
  2009-09-14 19:39   ` Bjorn Helgaas
@ 2009-09-15 10:11     ` Alan Jenkins
  0 siblings, 0 replies; 9+ messages in thread
From: Alan Jenkins @ 2009-09-15 10:11 UTC (permalink / raw)
  To: lenb; +Cc: linux-acpi, Bjorn Helgaas

The acpi device callbacks add, start, remove, suspend and resume can
never be called with a NULL acpi_device. Each callsite in acpi/scan.c
has to dereference the device in order to get the ops structure, e.g.

    struct acpi_device *acpi_dev = to_acpi_device(dev);
    struct acpi_driver *acpi_drv = acpi_dev->driver;

    if (acpi_drv && acpi_drv->ops.suspend)
        return acpi_drv->ops.suspend(acpi_dev, state);

Remove all checks for acpi_dev == NULL within these callbacks.

Also remove the checks for acpi_driver_data(acpi_dev) == NULL. None of
these checks could fail unless the driver does something strange
(which none of them do), the acpi core did something terribly wrong,
or we have a memory corruption issue. If this does happen then it's
best to dereference the pointer and crash noisily.

Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
Reviewed-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

---
v2: Tidy up acpi/sbs.c a little after removing the check

 drivers/acpi/ac.c                       |   18 ++++--------------
 drivers/acpi/acpi_memhotplug.c          |   11 ++---------
 drivers/acpi/battery.c                  |   16 +++++-----------
 drivers/acpi/container.c                |    8 +-------
 drivers/acpi/ec.c                       |    6 +-----
 drivers/acpi/fan.c                      |   12 ------------
 drivers/acpi/power.c                    |   17 ++---------------
 drivers/acpi/processor_core.c           |    7 +------
 drivers/acpi/sbs.c                      |   11 +++--------
 drivers/acpi/sbshc.c                    |    9 +--------
 drivers/acpi/thermal.c                  |   19 +++----------------
 drivers/acpi/video.c                    |   14 ++------------
 drivers/hwmon/hp_accel.c                |    6 ------
 drivers/platform/x86/asus-laptop.c      |    6 ------
 drivers/platform/x86/asus_acpi.c        |    6 ------
 drivers/platform/x86/eeepc-laptop.c     |    5 -----
 drivers/platform/x86/fujitsu-laptop.c   |    6 ------
 drivers/platform/x86/intel_menlow.c     |    6 ------
 drivers/platform/x86/panasonic-laptop.c |    9 ---------
 19 files changed, 25 insertions(+), 167 deletions(-)

diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index 98b9690..7725bda 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -256,11 +256,8 @@ static void acpi_ac_notify(struct acpi_device *device, u32 event)
 static int acpi_ac_add(struct acpi_device *device)
 {
 	int result = 0;
-	struct acpi_ac *ac = NULL;
-
+	struct acpi_ac *ac;
 
-	if (!device)
-		return -EINVAL;
 
 	ac = kzalloc(sizeof(struct acpi_ac), GFP_KERNEL);
 	if (!ac)
@@ -306,11 +303,9 @@ static int acpi_ac_add(struct acpi_device *device)
 
 static int acpi_ac_resume(struct acpi_device *device)
 {
-	struct acpi_ac *ac;
+	struct acpi_ac *ac = acpi_driver_data(device);
 	unsigned old_state;
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
-	ac = acpi_driver_data(device);
+
 	old_state = ac->state;
 	if (acpi_ac_get_state(ac))
 		return 0;
@@ -323,13 +318,8 @@ static int acpi_ac_resume(struct acpi_device *device)
 
 static int acpi_ac_remove(struct acpi_device *device, int type)
 {
-	struct acpi_ac *ac = NULL;
-
-
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
+	struct acpi_ac *ac = acpi_driver_data(device);
 
-	ac = acpi_driver_data(device);
 
 #ifdef CONFIG_ACPI_SYSFS_POWER
 	if (ac->charger.dev)
diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 28ccdbc..71085b9 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -401,11 +401,8 @@ static void acpi_memory_device_notify(acpi_handle handle, u32 event, void *data)
 static int acpi_memory_device_add(struct acpi_device *device)
 {
 	int result;
-	struct acpi_memory_device *mem_device = NULL;
-
+	struct acpi_memory_device *mem_device;
 
-	if (!device)
-		return -EINVAL;
 
 	mem_device = kzalloc(sizeof(struct acpi_memory_device), GFP_KERNEL);
 	if (!mem_device)
@@ -450,13 +447,9 @@ static int acpi_memory_device_add(struct acpi_device *device)
 
 static int acpi_memory_device_remove(struct acpi_device *device, int type)
 {
-	struct acpi_memory_device *mem_device = NULL;
+	struct acpi_memory_device *mem_device = acpi_driver_data(device);
 
 
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
-
-	mem_device = acpi_driver_data(device);
 	kfree(mem_device);
 
 	return 0;
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 3f4602b..90e39d9 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -840,9 +840,8 @@ static void acpi_battery_notify(struct acpi_device *device, u32 event)
 static int acpi_battery_add(struct acpi_device *device)
 {
 	int result = 0;
-	struct acpi_battery *battery = NULL;
-	if (!device)
-		return -EINVAL;
+	struct acpi_battery *battery;
+
 	battery = kzalloc(sizeof(struct acpi_battery), GFP_KERNEL);
 	if (!battery)
 		return -ENOMEM;
@@ -870,11 +869,8 @@ static int acpi_battery_add(struct acpi_device *device)
 
 static int acpi_battery_remove(struct acpi_device *device, int type)
 {
-	struct acpi_battery *battery = NULL;
+	struct acpi_battery *battery = acpi_driver_data(device);
 
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
-	battery = acpi_driver_data(device);
 #ifdef CONFIG_ACPI_PROCFS_POWER
 	acpi_battery_remove_fs(device);
 #endif
@@ -889,10 +885,8 @@ static int acpi_battery_remove(struct acpi_device *device, int type)
 /* this is needed to learn about changes made in suspended state */
 static int acpi_battery_resume(struct acpi_device *device)
 {
-	struct acpi_battery *battery;
-	if (!device)
-		return -EINVAL;
-	battery = acpi_driver_data(device);
+	struct acpi_battery *battery = acpi_driver_data(device);
+
 	battery->update_time = 0;
 	acpi_battery_update(battery);
 	return 0;
diff --git a/drivers/acpi/container.c b/drivers/acpi/container.c
index 642bb30..30c4700 100644
--- a/drivers/acpi/container.c
+++ b/drivers/acpi/container.c
@@ -97,11 +97,6 @@ static int acpi_container_add(struct acpi_device *device)
 	struct acpi_container *container;
 
 
-	if (!device) {
-		printk(KERN_ERR PREFIX "device is NULL\n");
-		return -EINVAL;
-	}
-
 	container = kzalloc(sizeof(struct acpi_container), GFP_KERNEL);
 	if (!container)
 		return -ENOMEM;
@@ -120,9 +115,8 @@ static int acpi_container_add(struct acpi_device *device)
 static int acpi_container_remove(struct acpi_device *device, int type)
 {
 	acpi_status status = AE_OK;
-	struct acpi_container *pc = NULL;
+	struct acpi_container *pc = acpi_driver_data(device);
 
-	pc = acpi_driver_data(device);
 	kfree(pc);
 	return status;
 }
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index add6621..5e93ae3 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -837,13 +837,9 @@ static int acpi_ec_add(struct acpi_device *device)
 
 static int acpi_ec_remove(struct acpi_device *device, int type)
 {
-	struct acpi_ec *ec;
+	struct acpi_ec *ec = acpi_driver_data(device);
 	struct acpi_ec_query_handler *handler, *tmp;
 
-	if (!device)
-		return -EINVAL;
-
-	ec = acpi_driver_data(device);
 	ec_remove_handlers(ec);
 	mutex_lock(&ec->lock);
 	list_for_each_entry_safe(handler, tmp, &ec->list, node) {
diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
index f419849..d40e214 100644
--- a/drivers/acpi/fan.c
+++ b/drivers/acpi/fan.c
@@ -244,9 +244,6 @@ static int acpi_fan_add(struct acpi_device *device)
 	int state = 0;
 	struct thermal_cooling_device *cdev;
 
-	if (!device)
-		return -EINVAL;
-
 	strcpy(acpi_device_name(device), "Fan");
 	strcpy(acpi_device_class(device), ACPI_FAN_CLASS);
 
@@ -300,9 +297,6 @@ static int acpi_fan_remove(struct acpi_device *device, int type)
 {
 	struct thermal_cooling_device *cdev = acpi_driver_data(device);
 
-	if (!device || !cdev)
-		return -EINVAL;
-
 	acpi_fan_remove_fs(device);
 	sysfs_remove_link(&device->dev.kobj, "thermal_cooling");
 	sysfs_remove_link(&cdev->device.kobj, "device");
@@ -313,9 +307,6 @@ static int acpi_fan_remove(struct acpi_device *device, int type)
 
 static int acpi_fan_suspend(struct acpi_device *device, pm_message_t state)
 {
-	if (!device)
-		return -EINVAL;
-
 	acpi_bus_set_power(device->handle, ACPI_STATE_D0);
 
 	return AE_OK;
@@ -326,9 +317,6 @@ static int acpi_fan_resume(struct acpi_device *device)
 	int result = 0;
 	int power_state = 0;
 
-	if (!device)
-		return -EINVAL;
-
 	result = acpi_bus_get_power(device->handle, &power_state);
 	if (result) {
 		printk(KERN_ERR PREFIX
diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
index e86603f..2c3d844 100644
--- a/drivers/acpi/power.c
+++ b/drivers/acpi/power.c
@@ -639,9 +639,6 @@ static int acpi_power_add(struct acpi_device *device)
 	struct acpi_buffer buffer = { sizeof(acpi_object), &acpi_object };
 
 
-	if (!device)
-		return -EINVAL;
-
 	resource = kzalloc(sizeof(struct acpi_power_resource), GFP_KERNEL);
 	if (!resource)
 		return -ENOMEM;
@@ -695,15 +692,10 @@ static int acpi_power_add(struct acpi_device *device)
 
 static int acpi_power_remove(struct acpi_device *device, int type)
 {
-	struct acpi_power_resource *resource = NULL;
+	struct acpi_power_resource *resource = acpi_driver_data(device);
 	struct list_head *node, *next;
 
 
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
-
-	resource = acpi_driver_data(device);
-
 	acpi_power_remove_fs(device);
 
 	mutex_lock(&resource->resource_lock);
@@ -722,14 +714,9 @@ static int acpi_power_remove(struct acpi_device *device, int type)
 static int acpi_power_resume(struct acpi_device *device)
 {
 	int result = 0, state;
-	struct acpi_power_resource *resource = NULL;
+	struct acpi_power_resource *resource = acpi_driver_data(device);
 	struct acpi_power_reference *ref;
 
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
-
-	resource = acpi_driver_data(device);
-
 	result = acpi_power_get_state(device->handle, &state);
 	if (result)
 		return result;
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index c2d4d6e..38ad5ef 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -888,13 +888,8 @@ err_free_cpumask:
 
 static int acpi_processor_remove(struct acpi_device *device, int type)
 {
-	struct acpi_processor *pr = NULL;
-
-
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
+	struct acpi_processor *pr = acpi_driver_data(device);
 
-	pr = acpi_driver_data(device);
 
 	if (pr->id >= nr_cpu_ids)
 		goto free;
diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c
index 52b9db8..25a2f6a 100644
--- a/drivers/acpi/sbs.c
+++ b/drivers/acpi/sbs.c
@@ -960,12 +960,9 @@ static int acpi_sbs_add(struct acpi_device *device)
 
 static int acpi_sbs_remove(struct acpi_device *device, int type)
 {
-	struct acpi_sbs *sbs;
+	struct acpi_sbs *sbs = acpi_driver_data(device);
 	int id;
 
-	if (!device)
-		return -EINVAL;
-	sbs = acpi_driver_data(device);
 	if (!sbs)
 		return -EINVAL;
 	mutex_lock(&sbs->lock);
@@ -995,10 +992,8 @@ static void acpi_sbs_rmdirs(void)
 
 static int acpi_sbs_resume(struct acpi_device *device)
 {
-	struct acpi_sbs *sbs;
-	if (!device)
-		return -EINVAL;
-	sbs = device->driver_data;
+	struct acpi_sbs *sbs = acpi_driver_data(device);
+
 	acpi_sbs_callback(sbs);
 	return 0;
 }
diff --git a/drivers/acpi/sbshc.c b/drivers/acpi/sbshc.c
index 8d89337..9e06e9c 100644
--- a/drivers/acpi/sbshc.c
+++ b/drivers/acpi/sbshc.c
@@ -262,9 +262,6 @@ static int acpi_smbus_hc_add(struct acpi_device *device)
 	unsigned long long val;
 	struct acpi_smb_hc *hc;
 
-	if (!device)
-		return -EINVAL;
-
 	status = acpi_evaluate_integer(device->handle, "_EC", NULL, &val);
 	if (ACPI_FAILURE(status)) {
 		printk(KERN_ERR PREFIX "error obtaining _EC.\n");
@@ -296,12 +293,8 @@ extern void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit);
 
 static int acpi_smbus_hc_remove(struct acpi_device *device, int type)
 {
-	struct acpi_smb_hc *hc;
-
-	if (!device)
-		return -EINVAL;
+	struct acpi_smb_hc *hc = acpi_driver_data(device);
 
-	hc = acpi_driver_data(device);
 	acpi_ec_remove_query_handler(hc->ec, hc->query_bit);
 	kfree(hc);
 	return 0;
diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 65f6781..fca61a1 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -1361,12 +1361,9 @@ static void acpi_thermal_guess_offset(struct acpi_thermal *tz)
 static int acpi_thermal_add(struct acpi_device *device)
 {
 	int result = 0;
-	struct acpi_thermal *tz = NULL;
+	struct acpi_thermal *tz;
 
 
-	if (!device)
-		return -EINVAL;
-
 	tz = kzalloc(sizeof(struct acpi_thermal), GFP_KERNEL);
 	if (!tz)
 		return -ENOMEM;
@@ -1408,12 +1405,7 @@ end:
 
 static int acpi_thermal_remove(struct acpi_device *device, int type)
 {
-	struct acpi_thermal *tz = NULL;
-
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
-
-	tz = acpi_driver_data(device);
+	struct acpi_thermal *tz = acpi_driver_data(device);
 
 	acpi_thermal_remove_fs(device);
 	acpi_thermal_unregister_thermal_zone(tz);
@@ -1424,15 +1416,10 @@ static int acpi_thermal_remove(struct acpi_device *device, int type)
 
 static int acpi_thermal_resume(struct acpi_device *device)
 {
-	struct acpi_thermal *tz = NULL;
+	struct acpi_thermal *tz = acpi_driver_data(device);
 	int i, j, power_state, result;
 
 
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
-
-	tz = acpi_driver_data(device);
-
 	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
 		if (!(&tz->trips.active[i]))
 			break;
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index f405807..576a600 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -2207,15 +2207,10 @@ static void acpi_video_device_notify(acpi_handle handle, u32 event, void *data)
 static int instance;
 static int acpi_video_resume(struct acpi_device *device)
 {
-	struct acpi_video_bus *video;
+	struct acpi_video_bus *video = acpi_driver_data(device);
 	struct acpi_video_device *video_device;
 	int i;
 
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
-
-	video = acpi_driver_data(device);
-
 	for (i = 0; i < video->attached_count; i++) {
 		video_device = video->attached_array[i].bind_info;
 		if (video_device && video_device->backlight)
@@ -2351,13 +2346,8 @@ static int acpi_video_bus_add(struct acpi_device *device)
 
 static int acpi_video_bus_remove(struct acpi_device *device, int type)
 {
-	struct acpi_video_bus *video = NULL;
-
-
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
+	struct acpi_video_bus *video = acpi_driver_data(device);
 
-	video = acpi_driver_data(device);
 
 	acpi_video_bus_stop_devices(video);
 	acpi_video_bus_put_devices(video);
diff --git a/drivers/hwmon/hp_accel.c b/drivers/hwmon/hp_accel.c
index 6679854..c8d3c88 100644
--- a/drivers/hwmon/hp_accel.c
+++ b/drivers/hwmon/hp_accel.c
@@ -275,9 +275,6 @@ static int lis3lv02d_add(struct acpi_device *device)
 {
 	int ret;
 
-	if (!device)
-		return -EINVAL;
-
 	lis3_dev.bus_priv = device;
 	lis3_dev.init = lis3lv02d_acpi_init;
 	lis3_dev.read = lis3lv02d_acpi_read;
@@ -315,9 +312,6 @@ static int lis3lv02d_add(struct acpi_device *device)
 
 static int lis3lv02d_remove(struct acpi_device *device, int type)
 {
-	if (!device)
-		return -EINVAL;
-
 	lis3lv02d_joystick_disable();
 	lis3lv02d_poweroff(&lis3_dev);
 
diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
index b39d2bb..8af43e9 100644
--- a/drivers/platform/x86/asus-laptop.c
+++ b/drivers/platform/x86/asus-laptop.c
@@ -1240,9 +1240,6 @@ static int asus_hotk_add(struct acpi_device *device)
 {
 	int result;
 
-	if (!device)
-		return -EINVAL;
-
 	pr_notice("Asus Laptop Support version %s\n",
 	       ASUS_LAPTOP_VERSION);
 
@@ -1306,9 +1303,6 @@ end:
 
 static int asus_hotk_remove(struct acpi_device *device, int type)
 {
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
-
 	kfree(hotk->name);
 	kfree(hotk);
 
diff --git a/drivers/platform/x86/asus_acpi.c b/drivers/platform/x86/asus_acpi.c
index ddf5240..25a7d57 100644
--- a/drivers/platform/x86/asus_acpi.c
+++ b/drivers/platform/x86/asus_acpi.c
@@ -1334,9 +1334,6 @@ static int asus_hotk_add(struct acpi_device *device)
 	acpi_status status = AE_OK;
 	int result;
 
-	if (!device)
-		return -EINVAL;
-
 	printk(KERN_NOTICE "Asus Laptop ACPI Extras version %s\n",
 	       ASUS_ACPI_VERSION);
 
@@ -1392,9 +1389,6 @@ end:
 
 static int asus_hotk_remove(struct acpi_device *device, int type)
 {
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
-
 	asus_hotk_remove_fs(device);
 
 	kfree(hotk);
diff --git a/drivers/platform/x86/eeepc-laptop.c b/drivers/platform/x86/eeepc-laptop.c
index da3c08b..e7f14a4 100644
--- a/drivers/platform/x86/eeepc-laptop.c
+++ b/drivers/platform/x86/eeepc-laptop.c
@@ -1194,8 +1194,6 @@ static int eeepc_hotk_add(struct acpi_device *device)
 	struct device *dev;
 	int result;
 
-	if (!device)
-		return -EINVAL;
 	pr_notice(EEEPC_HOTK_NAME "\n");
 	ehotk = kzalloc(sizeof(struct eeepc_hotk), GFP_KERNEL);
 	if (!ehotk)
@@ -1276,9 +1274,6 @@ fail_platform_driver:
 
 static int eeepc_hotk_remove(struct acpi_device *device, int type)
 {
-	if (!device || !acpi_driver_data(device))
-		return -EINVAL;
-
 	eeepc_backlight_exit();
 	eeepc_rfkill_exit();
 	eeepc_input_exit();
diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index f35aee5..2ab0318 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -657,9 +657,6 @@ static int acpi_fujitsu_add(struct acpi_device *device)
 	struct input_dev *input;
 	int error;
 
-	if (!device)
-		return -EINVAL;
-
 	fujitsu->acpi_handle = device->handle;
 	sprintf(acpi_device_name(device), "%s", ACPI_FUJITSU_DEVICE_NAME);
 	sprintf(acpi_device_class(device), "%s", ACPI_FUJITSU_CLASS);
@@ -813,9 +810,6 @@ static int acpi_fujitsu_hotkey_add(struct acpi_device *device)
 	int error;
 	int i;
 
-	if (!device)
-		return -EINVAL;
-
 	fujitsu_hotkey->acpi_handle = device->handle;
 	sprintf(acpi_device_name(device), "%s",
 		ACPI_FUJITSU_HOTKEY_DEVICE_NAME);
diff --git a/drivers/platform/x86/intel_menlow.c b/drivers/platform/x86/intel_menlow.c
index 29432a5..58de8fd 100644
--- a/drivers/platform/x86/intel_menlow.c
+++ b/drivers/platform/x86/intel_menlow.c
@@ -156,9 +156,6 @@ static int intel_menlow_memory_add(struct acpi_device *device)
 	acpi_handle dummy;
 	struct thermal_cooling_device *cdev;
 
-	if (!device)
-		return -EINVAL;
-
 	status = acpi_get_handle(device->handle, MEMORY_GET_BANDWIDTH, &dummy);
 	if (ACPI_FAILURE(status))
 		goto end;
@@ -200,9 +197,6 @@ static int intel_menlow_memory_remove(struct acpi_device *device, int type)
 {
 	struct thermal_cooling_device *cdev = acpi_driver_data(device);
 
-	if (!device || !cdev)
-		return -EINVAL;
-
 	sysfs_remove_link(&device->dev.kobj, "thermal_cooling");
 	sysfs_remove_link(&cdev->device.kobj, "device");
 	thermal_cooling_device_unregister(cdev);
diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
index fe7cf01..953b60c 100644
--- a/drivers/platform/x86/panasonic-laptop.c
+++ b/drivers/platform/x86/panasonic-laptop.c
@@ -588,9 +588,6 @@ static int acpi_pcc_hotkey_resume(struct acpi_device *device)
 	struct pcc_acpi *pcc = acpi_driver_data(device);
 	acpi_status status = AE_OK;
 
-	if (device == NULL || pcc == NULL)
-		return -EINVAL;
-
 	ACPI_DEBUG_PRINT((ACPI_DB_ERROR, "Sticky mode restore: %d\n",
 			  pcc->sticky_mode));
 
@@ -604,9 +601,6 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
 	struct pcc_acpi *pcc;
 	int num_sifr, result;
 
-	if (!device)
-		return -EINVAL;
-
 	num_sifr = acpi_pcc_get_sqty(device);
 
 	if (num_sifr > 255) {
@@ -703,9 +697,6 @@ static int acpi_pcc_hotkey_remove(struct acpi_device *device, int type)
 {
 	struct pcc_acpi *pcc = acpi_driver_data(device);
 
-	if (!device || !pcc)
-		return -EINVAL;
-
 	sysfs_remove_group(&device->dev.kobj, &pcc_attr_group);
 
 	backlight_device_unregister(pcc->backlight);
-- 
1.6.3.2




^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCHv2 4/4] ACPI: Remove unneccesary acpi_disabled checks in acpi drivers
  2009-09-14 19:46   ` Bjorn Helgaas
@ 2009-09-15 10:11     ` Alan Jenkins
  0 siblings, 0 replies; 9+ messages in thread
From: Alan Jenkins @ 2009-09-15 10:11 UTC (permalink / raw)
  To: lenb; +Cc: linux-acpi, Bjorn Helgaas

acpi_bus_register_driver() already checks acpi_disabled, so acpi bus
drivers don't need to.  Pass on the return value when it fails
instead of assuming ENODEV.

Also remove registration failure messages from a few drivers; they
serve no obvious purpose and most drivers do without them.

Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
Reviewed-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

---
v2: 
 - Remove printk("registration failed").

 - Some module_init functions can now be reduced to a single line:

	return acpi_bus_driver_register(&acpi_drv);


 drivers/acpi/ac.c                       |    5 +----
 drivers/acpi/acpi_memhotplug.c          |    2 +-
 drivers/acpi/battery.c                  |    2 --
 drivers/acpi/container.c                |    4 ++--
 drivers/acpi/ec.c                       |    2 +-
 drivers/acpi/fan.c                      |    2 +-
 drivers/acpi/pci_link.c                 |   11 ++++++-----
 drivers/acpi/power.c                    |    2 +-
 drivers/acpi/sbs.c                      |    4 +---
 drivers/acpi/sbshc.c                    |    7 +------
 drivers/acpi/thermal.c                  |    2 +-
 drivers/acpi/video.c                    |    2 +-
 drivers/hwmon/asus_atk0110.c            |    8 +-------
 drivers/hwmon/hp_accel.c                |    3 ---
 drivers/input/misc/atlas_btns.c         |   13 +------------
 drivers/platform/x86/asus-laptop.c      |    3 ---
 drivers/platform/x86/asus_acpi.c        |    3 ---
 drivers/platform/x86/eeepc-laptop.c     |    2 --
 drivers/platform/x86/panasonic-laptop.c |   14 +-------------
 drivers/platform/x86/wmi.c              |    3 ---
 20 files changed, 20 insertions(+), 74 deletions(-)

diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index 7725bda..142744a 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -338,9 +338,6 @@ static int __init acpi_ac_init(void)
 {
 	int result;
 
-	if (acpi_disabled)
-		return -ENODEV;
-
 #ifdef CONFIG_ACPI_PROCFS_POWER
 	acpi_ac_dir = acpi_lock_ac_dir();
 	if (!acpi_ac_dir)
@@ -352,7 +349,7 @@ static int __init acpi_ac_init(void)
 #ifdef CONFIG_ACPI_PROCFS_POWER
 		acpi_unlock_ac_dir(acpi_ac_dir);
 #endif
-		return -ENODEV;
+		return result;
 	}
 
 	return 0;
diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 71085b9..30a4600 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -526,7 +526,7 @@ static int __init acpi_memory_device_init(void)
 	result = acpi_bus_register_driver(&acpi_memory_device_driver);
 
 	if (result < 0)
-		return -ENODEV;
+		return result;
 
 	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
 				     ACPI_UINT32_MAX,
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 90e39d9..f451dc0 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -907,8 +907,6 @@ static struct acpi_driver acpi_battery_driver = {
 
 static void __init acpi_battery_init_async(void *unused, async_cookie_t cookie)
 {
-	if (acpi_disabled)
-		return;
 #ifdef CONFIG_ACPI_PROCFS_POWER
 	acpi_battery_dir = acpi_lock_battery_dir();
 	if (!acpi_battery_dir)
diff --git a/drivers/acpi/container.c b/drivers/acpi/container.c
index 30c4700..c54b69b 100644
--- a/drivers/acpi/container.c
+++ b/drivers/acpi/container.c
@@ -245,7 +245,7 @@ static int __init acpi_container_init(void)
 
 	result = acpi_bus_register_driver(&acpi_container_driver);
 	if (result < 0) {
-		return (result);
+		return result;
 	}
 
 	/* register notify handler to every container device */
@@ -254,7 +254,7 @@ static int __init acpi_container_init(void)
 			    ACPI_UINT32_MAX,
 			    container_walk_namespace_cb, &action, NULL);
 
-	return (0);
+	return 0;
 }
 
 static void __exit acpi_container_exit(void)
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 5e93ae3..26c58a3 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -1010,7 +1010,7 @@ int __init acpi_ec_init(void)
 	result = acpi_bus_register_driver(&acpi_ec_driver);
 	if (result < 0) {
 		remove_proc_entry(ACPI_EC_CLASS, acpi_root_dir);
-		return -ENODEV;
+		return result;
 	}
 
 	return result;
diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
index d40e214..947556e 100644
--- a/drivers/acpi/fan.c
+++ b/drivers/acpi/fan.c
@@ -345,7 +345,7 @@ static int __init acpi_fan_init(void)
 	result = acpi_bus_register_driver(&acpi_fan_driver);
 	if (result < 0) {
 		remove_proc_entry(ACPI_FAN_CLASS, acpi_root_dir);
-		return -ENODEV;
+		return result;
 	}
 
 	return 0;
diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
index 394ae89..4a1e22a 100644
--- a/drivers/acpi/pci_link.c
+++ b/drivers/acpi/pci_link.c
@@ -769,9 +769,7 @@ static int irqrouter_resume(struct sys_device *dev)
 
 static int acpi_pci_link_remove(struct acpi_device *device, int type)
 {
-	struct acpi_pci_link *link;
-
-	link = acpi_driver_data(device);
+	struct acpi_pci_link *link = acpi_driver_data(device);
 
 	mutex_lock(&acpi_link_lock);
 	list_del(&link->list);
@@ -900,6 +898,8 @@ device_initcall(irqrouter_init_sysfs);
 
 static int __init acpi_pci_link_init(void)
 {
+	int result;
+
 	if (acpi_noirq)
 		return 0;
 
@@ -911,8 +911,9 @@ static int __init acpi_pci_link_init(void)
 			acpi_irq_balance = 0;
 	}
 
-	if (acpi_bus_register_driver(&acpi_pci_link_driver) < 0)
-		return -ENODEV;
+	result = acpi_bus_register_driver(&acpi_pci_link_driver);
+	if (result < 0)
+		return result;
 
 	return 0;
 }
diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
index 2c3d844..3cd8d45 100644
--- a/drivers/acpi/power.c
+++ b/drivers/acpi/power.c
@@ -747,7 +747,7 @@ int __init acpi_power_init(void)
 	result = acpi_bus_register_driver(&acpi_power_driver);
 	if (result < 0) {
 		remove_proc_entry(ACPI_POWER_CLASS, acpi_root_dir);
-		return -ENODEV;
+		return result;
 	}
 
 	return 0;
diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c
index 25a2f6a..4f83688 100644
--- a/drivers/acpi/sbs.c
+++ b/drivers/acpi/sbs.c
@@ -1013,8 +1013,6 @@ static int __init acpi_sbs_init(void)
 {
 	int result = 0;
 
-	if (acpi_disabled)
-		return -ENODEV;
 #ifdef CONFIG_ACPI_PROCFS_POWER
 	acpi_ac_dir = acpi_lock_ac_dir();
 	if (!acpi_ac_dir)
@@ -1028,7 +1026,7 @@ static int __init acpi_sbs_init(void)
 	result = acpi_bus_register_driver(&acpi_sbs_driver);
 	if (result < 0) {
 		acpi_sbs_rmdirs();
-		return -ENODEV;
+		return result;
 	}
 	return 0;
 }
diff --git a/drivers/acpi/sbshc.c b/drivers/acpi/sbshc.c
index 9e06e9c..c07cabd 100644
--- a/drivers/acpi/sbshc.c
+++ b/drivers/acpi/sbshc.c
@@ -302,12 +302,7 @@ static int acpi_smbus_hc_remove(struct acpi_device *device, int type)
 
 static int __init acpi_smb_hc_init(void)
 {
-	int result;
-
-	result = acpi_bus_register_driver(&acpi_smb_hc_driver);
-	if (result < 0)
-		return -ENODEV;
-	return 0;
+	return acpi_bus_register_driver(&acpi_smb_hc_driver);
 }
 
 static void __exit acpi_smb_hc_exit(void)
diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index fca61a1..767e57d 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -1534,7 +1534,7 @@ static int __init acpi_thermal_init(void)
 	result = acpi_bus_register_driver(&acpi_thermal_driver);
 	if (result < 0) {
 		remove_proc_entry(ACPI_THERMAL_CLASS, acpi_root_dir);
-		return -ENODEV;
+		return result;
 	}
 
 	return 0;
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 576a600..0433df3 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -2398,7 +2398,7 @@ int acpi_video_register(void)
 	result = acpi_bus_register_driver(&acpi_video_bus);
 	if (result < 0) {
 		remove_proc_entry(ACPI_VIDEO_CLASS, acpi_root_dir);
-		return -ENODEV;
+		return result;
 	}
 
 	/*
diff --git a/drivers/hwmon/asus_atk0110.c b/drivers/hwmon/asus_atk0110.c
index 7fc7625..95c744d 100644
--- a/drivers/hwmon/asus_atk0110.c
+++ b/drivers/hwmon/asus_atk0110.c
@@ -993,13 +993,7 @@ static int atk_remove(struct acpi_device *device, int type)
 
 static int __init atk0110_init(void)
 {
-	int ret;
-
-	ret = acpi_bus_register_driver(&atk_driver);
-	if (ret)
-		pr_info("atk: acpi_bus_register_driver failed: %d\n", ret);
-
-	return ret;
+	return acpi_bus_register_driver(&atk_driver);
 }
 
 static void __exit atk0110_exit(void)
diff --git a/drivers/hwmon/hp_accel.c b/drivers/hwmon/hp_accel.c
index c8d3c88..38f8959 100644
--- a/drivers/hwmon/hp_accel.c
+++ b/drivers/hwmon/hp_accel.c
@@ -357,9 +357,6 @@ static int __init lis3lv02d_init_module(void)
 {
 	int ret;
 
-	if (acpi_disabled)
-		return -ENODEV;
-
 	ret = acpi_bus_register_driver(&lis3lv02d_driver);
 	if (ret < 0)
 		return ret;
diff --git a/drivers/input/misc/atlas_btns.c b/drivers/input/misc/atlas_btns.c
index 1b87191..0b7641a 100644
--- a/drivers/input/misc/atlas_btns.c
+++ b/drivers/input/misc/atlas_btns.c
@@ -154,18 +154,7 @@ static struct acpi_driver atlas_acpi_driver = {
 
 static int __init atlas_acpi_init(void)
 {
-	int result;
-
-	if (acpi_disabled)
-		return -ENODEV;
-
-	result = acpi_bus_register_driver(&atlas_acpi_driver);
-	if (result < 0) {
-		printk(KERN_ERR "Atlas ACPI: Unable to register driver\n");
-		return -ENODEV;
-	}
-
-	return 0;
+	return acpi_bus_register_driver(&atlas_acpi_driver);
 }
 
 static void __exit atlas_acpi_exit(void)
diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
index 8af43e9..4234edb 100644
--- a/drivers/platform/x86/asus-laptop.c
+++ b/drivers/platform/x86/asus-laptop.c
@@ -1438,9 +1438,6 @@ static int __init asus_laptop_init(void)
 {
 	int result;
 
-	if (acpi_disabled)
-		return -ENODEV;
-
 	result = acpi_bus_register_driver(&asus_hotk_driver);
 	if (result < 0)
 		return result;
diff --git a/drivers/platform/x86/asus_acpi.c b/drivers/platform/x86/asus_acpi.c
index 25a7d57..502692b 100644
--- a/drivers/platform/x86/asus_acpi.c
+++ b/drivers/platform/x86/asus_acpi.c
@@ -1416,9 +1416,6 @@ static int __init asus_acpi_init(void)
 {
 	int result;
 
-	if (acpi_disabled)
-		return -ENODEV;
-
 	asus_proc_dir = proc_mkdir(PROC_ASUS, acpi_root_dir);
 	if (!asus_proc_dir) {
 		printk(KERN_ERR "Asus ACPI: Unable to create /proc entry\n");
diff --git a/drivers/platform/x86/eeepc-laptop.c b/drivers/platform/x86/eeepc-laptop.c
index e7f14a4..eba3772 100644
--- a/drivers/platform/x86/eeepc-laptop.c
+++ b/drivers/platform/x86/eeepc-laptop.c
@@ -1291,8 +1291,6 @@ static int __init eeepc_laptop_init(void)
 {
 	int result;
 
-	if (acpi_disabled)
-		return -ENODEV;
 	result = acpi_bus_register_driver(&eeepc_hotk_driver);
 	if (result < 0)
 		return result;
diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
index 953b60c..cd6fbdb 100644
--- a/drivers/platform/x86/panasonic-laptop.c
+++ b/drivers/platform/x86/panasonic-laptop.c
@@ -678,19 +678,7 @@ out_hotkey:
 
 static int __init acpi_pcc_init(void)
 {
-	int result = 0;
-
-	if (acpi_disabled)
-		return -ENODEV;
-
-	result = acpi_bus_register_driver(&acpi_pcc_driver);
-	if (result < 0) {
-		ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
-				  "Error registering hotkey driver\n"));
-		return -ENODEV;
-	}
-
-	return 0;
+	return acpi_bus_register_driver(&acpi_pcc_driver);
 }
 
 static int acpi_pcc_hotkey_remove(struct acpi_device *device, int type)
diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 177f8d7..d889f51 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -702,9 +702,6 @@ static int __init acpi_wmi_init(void)
 
 	INIT_LIST_HEAD(&wmi_blocks.list);
 
-	if (acpi_disabled)
-		return -ENODEV;
-
 	result = acpi_bus_register_driver(&acpi_wmi_driver);
 
 	if (result < 0) {
-- 
1.6.3.2




^ permalink raw reply related	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2009-09-15 10:11 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-09-14 16:09 [PATCH 0/4] ACPI: Remove unnecessary code in acpi drivers Alan Jenkins
2009-09-14 16:09 ` [PATCH 1/4] ACPI: Remember to clear acpi_dev->driver after calling ops.remove() Alan Jenkins
2009-09-14 16:09 ` [PATCH 2/4] ACPI: Remove uneccessary NULL assignments in acpi drivers Alan Jenkins
2009-09-14 16:09 ` [PATCH 3/4] ACPI: Remove redundant NULL checks " Alan Jenkins
2009-09-14 19:39   ` Bjorn Helgaas
2009-09-15 10:11     ` [PATCHv2 " Alan Jenkins
2009-09-14 16:09 ` [PATCH 4/4] ACPI: Remove uneccesary acpi_disabled " Alan Jenkins
2009-09-14 19:46   ` Bjorn Helgaas
2009-09-15 10:11     ` [PATCHv2 4/4] ACPI: Remove unneccesary " Alan Jenkins

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.