[PATCH 0/3] A few fixes to TC1100-WMI driver

[PATCH 0/3] A few fixes to TC1100-WMI driver

[Dmitry Torokhov]

Hi,

Here are a few small fixes to tc1100-wmi driver. Compile tested only since I
don't have the hardware in question. Please consider applying.

Thanks.

-- 
Dmitry

[PATCH 1/3] tc1100-wmi - switch to using attribute group

[Dmitry Torokhov]

Sysfs attribute group takes care of proper creation of a set of attributes
and implements proper error unwinding so the driver does not have to do it.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/platform/x86/tc1100-wmi.c |   39 ++++++++++---------------------------
 1 files changed, 11 insertions(+), 28 deletions(-)

diff --git a/drivers/platform/x86/tc1100-wmi.c b/drivers/platform/x86/tc1100-wmi.c
index 4416600..0d53d51 100644
--- a/drivers/platform/x86/tc1100-wmi.c
+++ b/drivers/platform/x86/tc1100-wmi.c
@@ -183,30 +183,15 @@ static DEVICE_ATTR(value, S_IWUGO | S_IRUGO | S_IWUSR, \
 show_set_bool(wireless, TC1100_INSTANCE_WIRELESS);
 show_set_bool(jogdial, TC1100_INSTANCE_JOGDIAL);
 
-static void remove_fs(void)
-{
-	device_remove_file(&tc1100_device->dev, &dev_attr_wireless);
-	device_remove_file(&tc1100_device->dev, &dev_attr_jogdial);
-}
-
-static int add_fs(void)
-{
-	int ret;
-
-	ret = device_create_file(&tc1100_device->dev, &dev_attr_wireless);
-	if (ret)
-		goto add_sysfs_error;
-
-	ret = device_create_file(&tc1100_device->dev, &dev_attr_jogdial);
-	if (ret)
-		goto add_sysfs_error;
-
-	return ret;
+static struct attribute *tc1100_attributes[] = {
+	&dev_attr_wireless.attr,
+	&dev_attr_jogdial.attr,
+	NULL
+};
 
-add_sysfs_error:
-	remove_fs();
-	return ret;
-}
+static struct attribute_group tc1100_attribute_group = {
+	.attrs	= tc1100_attributes,
+};
 
 /* --------------------------------------------------------------------------
 				Driver Model
@@ -214,16 +199,14 @@ add_sysfs_error:
 
 static int tc1100_probe(struct platform_device *device)
 {
-	int result = 0;
-
-	result = add_fs();
-	return result;
+	return sysfs_create_group(&device->dev.kobj, &tc1100_attribute_group);
 }
 
 
 static int tc1100_remove(struct platform_device *device)
 {
-	remove_fs();
+	sysfs_remove_group(&device->dev.kobj, &tc1100_attribute_group);
+
 	return 0;
 }
 

[PATCH 2/3] tc1100-wmi - add error handling for device registration

[Dmitry Torokhov]

Any of the platform API functions can fail; driver should be prepared
to handle such failures. Also:

 - changed to platform_driver_probe() since the device is created
   right there with the driver;
 - added __devexit annotation to remove method;
 - fixed memory leak on module unload - named platform_device_del() is not
   enough to free platform device, need platform_device_unregister().

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/platform/x86/tc1100-wmi.c |   58 +++++++++++++++++++------------------
 1 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/drivers/platform/x86/tc1100-wmi.c b/drivers/platform/x86/tc1100-wmi.c
index 0d53d51..fa2995b 100644
--- a/drivers/platform/x86/tc1100-wmi.c
+++ b/drivers/platform/x86/tc1100-wmi.c
@@ -47,22 +47,6 @@ MODULE_DESCRIPTION("HP Compaq TC1100 Tablet WMI Extras");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("wmi:C364AC71-36DB-495A-8494-B439D472A505");
 
-static int tc1100_probe(struct platform_device *device);
-static int tc1100_remove(struct platform_device *device);
-static int tc1100_suspend(struct platform_device *device, pm_message_t state);
-static int tc1100_resume(struct platform_device *device);
-
-static struct platform_driver tc1100_driver = {
-	.driver = {
-		.name = "tc1100-wmi",
-		.owner = THIS_MODULE,
-	},
-	.probe = tc1100_probe,
-	.remove = tc1100_remove,
-	.suspend = tc1100_suspend,
-	.resume = tc1100_resume,
-};
-
 static struct platform_device *tc1100_device;
 
 struct tc1100_data {
@@ -197,13 +181,13 @@ static struct attribute_group tc1100_attribute_group = {
 				Driver Model
    -------------------------------------------------------------------------- */
 
-static int tc1100_probe(struct platform_device *device)
+static int __init tc1100_probe(struct platform_device *device)
 {
 	return sysfs_create_group(&device->dev.kobj, &tc1100_attribute_group);
 }
 
 
-static int tc1100_remove(struct platform_device *device)
+static int __devexit tc1100_remove(struct platform_device *device)
 {
 	sysfs_remove_group(&device->dev.kobj, &tc1100_attribute_group);
 
@@ -240,31 +224,49 @@ static int tc1100_resume(struct platform_device *dev)
 	return ret;
 }
 
+static struct platform_driver tc1100_driver = {
+	.driver = {
+		.name = "tc1100-wmi",
+		.owner = THIS_MODULE,
+	},
+	.remove = __devexit_p(tc1100_remove),
+	.suspend = tc1100_suspend,
+	.resume = tc1100_resume,
+};
+
 static int __init tc1100_init(void)
 {
-	int result = 0;
+	int error;
 
 	if (!wmi_has_guid(GUID))
 		return -ENODEV;
 
-	result = platform_driver_register(&tc1100_driver);
-	if (result)
-		return result;
-
 	tc1100_device = platform_device_alloc("tc1100-wmi", -1);
-	platform_device_add(tc1100_device);
+	if (!tc1100_device)
+		return -ENOMEM;
+
+	error = platform_device_add(tc1100_device);
+	if (error)
+		goto err_device_put;
+
+	error = platform_driver_probe(&tc1100_driver, tc1100_probe);
+	if (error)
+		goto err_device_del;
 
 	printk(TC1100_INFO "HP Compaq TC1100 Tablet WMI Extras loaded\n");
+	return 0;
 
-	return result;
+ err_device_del:
+	platform_device_del(tc1100_device);
+ err_device_put:
+	platform_device_put(tc1100_device);
+	return error;
 }
 
 static void __exit tc1100_exit(void)
 {
-	platform_device_del(tc1100_device);
+	platform_device_unregister(tc1100_device);
 	platform_driver_unregister(&tc1100_driver);
-
-	printk(TC1100_INFO "HP Compaq TC1100 Tablet WMI Extras unloaded\n");
 }
 
 module_init(tc1100_init);

[PATCH 3/3] tc1100-wmi - switch to using dev_pm_ops

[Dmitry Torokhov]

Also guard PM operations with CONFIG_PM.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/platform/x86/tc1100-wmi.c |   22 ++++++++++++++++------
 1 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/tc1100-wmi.c b/drivers/platform/x86/tc1100-wmi.c
index fa2995b..dd33b51 100644
--- a/drivers/platform/x86/tc1100-wmi.c
+++ b/drivers/platform/x86/tc1100-wmi.c
@@ -194,7 +194,8 @@ static int __devexit tc1100_remove(struct platform_device *device)
 	return 0;
 }
 
-static int tc1100_suspend(struct platform_device *dev, pm_message_t state)
+#ifdef CONFIG_PM
+static int tc1100_suspend(struct device *dev)
 {
 	int ret;
 
@@ -206,10 +207,10 @@ static int tc1100_suspend(struct platform_device *dev, pm_message_t state)
 	if (ret)
 		return ret;
 
-	return ret;
+	return 0;
 }
 
-static int tc1100_resume(struct platform_device *dev)
+static int tc1100_resume(struct device *dev)
 {
 	int ret;
 
@@ -221,17 +222,26 @@ static int tc1100_resume(struct platform_device *dev)
 	if (ret)
 		return ret;
 
-	return ret;
+	return 0;
 }
 
+static const struct dev_pm_ops tc1100_pm_ops = {
+	.suspend	= tc1100_suspend,
+	.resume		= tc1100_resume,
+	.freeze		= tc1100_suspend,
+	.restore	= tc1100_resume,
+};
+#endif
+
 static struct platform_driver tc1100_driver = {
 	.driver = {
 		.name = "tc1100-wmi",
 		.owner = THIS_MODULE,
+#ifdef CONFIG_PM
+		.pm = &tc1100_pm_ops,
+#endif
 	},
 	.remove = __devexit_p(tc1100_remove),
-	.suspend = tc1100_suspend,
-	.resume = tc1100_resume,
 };
 
 static int __init tc1100_init(void)

Re: [PATCH 0/3] A few fixes to TC1100-WMI driver

[Len Brown]

> Here are a few small fixes to tc1100-wmi driver. Compile tested only since I
> don't have the hardware in question. Please consider applying.

Can anybody test this driver?
(Dmitry's patches are now in the acpi-test tree)

thanks,
-Len Brown, Intel Open Source Technology Center

Re: [PATCH 0/3] A few fixes to TC1100-WMI driver

[Carlos Corbacho]

On Wednesday 16 December 2009 17:55:28 Len Brown wrote:
> > Here are a few small fixes to tc1100-wmi driver. Compile tested only
> > since I don't have the hardware in question. Please consider applying.
> 
> Can anybody test this driver?
> (Dmitry's patches are now in the acpi-test tree)

I've never had the hardware, so can't test them. They do look fine to me 
though, so if they compile, that's about the best we can hope for.

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