From 0ad3dc3af8ba028368263b190a7a270f8d5cf5ae Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Fri, 4 Dec 2009 00:36:09 -0800 Subject: [PATCH 1/3] tc1100-wmi - switch to using attribute group 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 Signed-off-by: Len Brown --- drivers/platform/x86/tc1100-wmi.c | 39 +++++++++---------------------- 1 file changed, 11 insertions(+), 28 deletions(-) diff --git a/drivers/platform/x86/tc1100-wmi.c b/drivers/platform/x86/tc1100-wmi.c index 44166003d4e..0d53d516657 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 struct attribute *tc1100_attributes[] = { + &dev_attr_wireless.attr, + &dev_attr_jogdial.attr, + NULL +}; -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; - -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; } From 9634a627b330fcc7cdca25df4d7853ca9c7745de Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Fri, 4 Dec 2009 00:36:15 -0800 Subject: [PATCH 2/3] tc1100-wmi - add error handling for device registration 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 Signed-off-by: Len Brown --- drivers/platform/x86/tc1100-wmi.c | 58 ++++++++++++++++--------------- 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/drivers/platform/x86/tc1100-wmi.c b/drivers/platform/x86/tc1100-wmi.c index 0d53d516657..fa2995bfa1d 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); From 8e698a3c47887fe5aa5e2252c27bb6ff416a07e4 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Fri, 4 Dec 2009 00:36:20 -0800 Subject: [PATCH 3/3] tc1100-wmi - switch to using dev_pm_ops Also guard PM operations with CONFIG_PM. Signed-off-by: Dmitry Torokhov Signed-off-by: Len Brown --- drivers/platform/x86/tc1100-wmi.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/drivers/platform/x86/tc1100-wmi.c b/drivers/platform/x86/tc1100-wmi.c index fa2995bfa1d..dd33b51c348 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)