[PATCH] acer-wmi: add error checks in module_init

[PATCH] acer-wmi: add error checks in module_init

[Akinobu Mita]

There are sevaral bugs in module_init and module_exit for acer-wmi.

- No error handlings for platform_device_alloc() and platform_device_add()

- acer_platform_device is not freeed in module_exit

This patch fixes these bugs by using platform_device_register_simple()
and platform_device_unregister() respectively.

This patch also makes create_sysfs() take a argument struct
platform_device pointer so that it looks symmetrical to remove_sysfs().

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Carlos Corbacho <carlos@strangeworlds.co.uk>
---
 drivers/misc/acer-wmi.c |   48 +++++++++++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 21 deletions(-)

Index: 2.6-git/drivers/misc/acer-wmi.c
===================================================================
--- 2.6-git.orig/drivers/misc/acer-wmi.c
+++ 2.6-git/drivers/misc/acer-wmi.c
@@ -1129,40 +1129,36 @@ static int remove_sysfs(struct platform_
 	return 0;
 }
 
-static int create_sysfs(void)
+static int create_sysfs(struct platform_device *device)
 {
 	int retval = -ENOMEM;
 
 	if (has_cap(ACER_CAP_WIRELESS)) {
-		retval = device_create_file(&acer_platform_device->dev,
-			&dev_attr_wireless);
+		retval = device_create_file(&device->dev, &dev_attr_wireless);
 		if (retval)
 			goto error_sysfs;
 	}
 
 	if (has_cap(ACER_CAP_BLUETOOTH)) {
-		retval = device_create_file(&acer_platform_device->dev,
-			&dev_attr_bluetooth);
+		retval = device_create_file(&device->dev, &dev_attr_bluetooth);
 		if (retval)
 			goto error_sysfs;
 	}
 
 	if (has_cap(ACER_CAP_THREEG)) {
-		retval = device_create_file(&acer_platform_device->dev,
-			&dev_attr_threeg);
+		retval = device_create_file(&device->dev, &dev_attr_threeg);
 		if (retval)
 			goto error_sysfs;
 	}
 
-	retval = device_create_file(&acer_platform_device->dev,
-		&dev_attr_interface);
+	retval = device_create_file(&device->dev, &dev_attr_interface);
 	if (retval)
 		goto error_sysfs;
 
 	return 0;
 
 error_sysfs:
-		remove_sysfs(acer_platform_device);
+	remove_sysfs(device);
 	return retval;
 }
 
@@ -1242,22 +1238,27 @@ static int __init acer_wmi_init(void)
 
 	set_quirks();
 
-	if (platform_driver_register(&acer_platform_driver)) {
+	err = platform_driver_register(&acer_platform_driver);
+	if (err) {
 		printk(ACER_ERR "Unable to register platform driver.\n");
-		goto error_platform_register;
+		return err;
+	}
+	acer_platform_device = platform_device_register_simple("acer-wmi", -1,
+								NULL, 0);
+	if (IS_ERR(acer_platform_device)) {
+		err = PTR_ERR(acer_platform_device);
+		goto out1;
 	}
-	acer_platform_device = platform_device_alloc("acer-wmi", -1);
-	platform_device_add(acer_platform_device);
 
-	err = create_sysfs();
+	err = create_sysfs(acer_platform_device);
 	if (err)
-		return err;
+		goto out2;
 
 	if (wmi_has_guid(WMID_GUID2)) {
 		interface->debug.wmid_devices = get_wmid_devices();
 		err = create_debugfs();
 		if (err)
-			return err;
+			goto out3;
 	}
 
 	/* Override any initial settings with values from the commandline */
@@ -1265,19 +1266,24 @@ static int __init acer_wmi_init(void)
 
 	return 0;
 
-error_platform_register:
-	return -ENODEV;
+out3:
+	remove_sysfs(acer_platform_device);
+out2:
+	platform_device_unregister(acer_platform_device);
+out1:
+	platform_driver_unregister(&acer_platform_driver);
+
+	return err;
 }
 
 static void __exit acer_wmi_exit(void)
 {
 	remove_sysfs(acer_platform_device);
 	remove_debugfs();
-	platform_device_del(acer_platform_device);
+	platform_device_unregister(acer_platform_device);
 	platform_driver_unregister(&acer_platform_driver);
 
 	printk(ACER_INFO "Acer Laptop WMI Extras unloaded\n");
-	return;
 }
 
 module_init(acer_wmi_init);

Re: [PATCH] acer-wmi: add error checks in module_init

[Carlos Corbacho]

On Sunday 21 September 2008 15:29:59 Akinobu Mita wrote:
> This patch fixes these bugs by using platform_device_register_simple()

[Adding Greg KH to CC]

Last I heard, platform_device_register_simple() was in the process of being 
deprecated and then slated for future removal. Has this changed?

-Carlos
-- 
E-Mail: carlos@strangeworlds.co.uk
Web: strangeworlds.co.uk
GPG Key ID: 0x23EE722D

Re: [PATCH] acer-wmi: add error checks in module_init

[Carlos Corbacho]

On Sunday 21 September 2008 15:29:59 Akinobu Mita wrote:
> This patch fixes these bugs by using platform_device_register_simple()

[Adding Greg KH to CC - 2nd time lucky...]

Last I heard, platform_device_register_simple() was in the process of being 
deprecated and then slated for future removal. Has this changed?

-Carlos
-- 
E-Mail: carlos@strangeworlds.co.uk
Web: strangeworlds.co.uk
GPG Key ID: 0x23EE722D

Re: [PATCH] acer-wmi: add error checks in module_init

[Akinobu Mita]

On Sun, Sep 21, 2008 at 06:53:19PM +0100, Carlos Corbacho wrote:
> On Sunday 21 September 2008 15:29:59 Akinobu Mita wrote:
> > This patch fixes these bugs by using platform_device_register_simple()
> 
> [Adding Greg KH to CC - 2nd time lucky...]
> 
> Last I heard, platform_device_register_simple() was in the process of being 
> deprecated and then slated for future removal. Has this changed?

I didn't know about that plan. But if platform_device_alloc/add is
more preferable API than platform_device_register_simple().
I'll amend those patches and resend them.

Re: [PATCH] acer-wmi: add error checks in module_init

[Akinobu Mita]

(I forget to CC)

2008/9/22 Akinobu Mita <akinobu.mita@gmail.com>:
> On Sun, Sep 21, 2008 at 06:53:19PM +0100, Carlos Corbacho wrote:
>> On Sunday 21 September 2008 15:29:59 Akinobu Mita wrote:
>> > This patch fixes these bugs by using platform_device_register_simple()
>>
>> [Adding Greg KH to CC - 2nd time lucky...]
>>
>> Last I heard, platform_device_register_simple() was in the process of being
>> deprecated and then slated for future removal. Has this changed?
>
> I didn't know about that plan. But if platform_device_alloc/add is
> more preferable API than platform_device_register_simple().
> I'll amend those patches and resend them.

The number of platform_device_register_simple() is not decreasing.
So I think the removal plan for platform_device_register_simple is not
started (yet).

grep -r platform_device_register_simple 2.6.XX | wc -l

2.6.20: 64
2.6.21: 77
2.6.22: 66
2.6.23: 76
2.6.24: 80
2.6.25: 87
2.6.26: 83
2.6.27-rc7: 88

Re: [PATCH] acer-wmi: add error checks in module_init

[Greg KH]

On Sun, Sep 21, 2008 at 06:53:19PM +0100, Carlos Corbacho wrote:
> On Sunday 21 September 2008 15:29:59 Akinobu Mita wrote:
> > This patch fixes these bugs by using platform_device_register_simple()
> 
> [Adding Greg KH to CC - 2nd time lucky...]
> 
> Last I heard, platform_device_register_simple() was in the process of being 
> deprecated and then slated for future removal. Has this changed?

I don't recall this, but someone else might have stated it a while ago.

sorry,

greg k-h

Re: [PATCH] acer-wmi: add error checks in module_init

[Carlos Corbacho]

On Monday 22 September 2008 16:59:49 Greg KH wrote:
> On Sun, Sep 21, 2008 at 06:53:19PM +0100, Carlos Corbacho wrote:
> > On Sunday 21 September 2008 15:29:59 Akinobu Mita wrote:
> > > This patch fixes these bugs by using platform_device_register_simple()
> >
> > [Adding Greg KH to CC - 2nd time lucky...]
> >
> > Last I heard, platform_device_register_simple() was in the process of
> > being deprecated and then slated for future removal. Has this changed?
>
> I don't recall this, but someone else might have stated it a while ago.

http://linux.derkeiler.com/Mailing-Lists/Kernel/2006-04/msg02070.html

This was dated a while ago, but has anything changed since then?

-Carlos
-- 
E-Mail: carlos@strangeworlds.co.uk
Web: strangeworlds.co.uk
GPG Key ID: 0x23EE722D

Re: [PATCH] acer-wmi: add error checks in module_init

[Greg KH]

On Mon, Sep 22, 2008 at 06:54:44PM +0100, Carlos Corbacho wrote:
> On Monday 22 September 2008 16:59:49 Greg KH wrote:
> > On Sun, Sep 21, 2008 at 06:53:19PM +0100, Carlos Corbacho wrote:
> > > On Sunday 21 September 2008 15:29:59 Akinobu Mita wrote:
> > > > This patch fixes these bugs by using platform_device_register_simple()
> > >
> > > [Adding Greg KH to CC - 2nd time lucky...]
> > >
> > > Last I heard, platform_device_register_simple() was in the process of
> > > being deprecated and then slated for future removal. Has this changed?
> >
> > I don't recall this, but someone else might have stated it a while ago.
> 
> http://linux.derkeiler.com/Mailing-Lists/Kernel/2006-04/msg02070.html
> 
> This was dated a while ago, but has anything changed since then?

Not that I know of, I just forgot about it :)

thanks,

greg k-h