From a0e92d70f35b5fd7da8ec2160cda78b98e2113bc Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Wed, 16 Dec 2009 21:38:26 +0100 Subject: hwmon: (smsc47m1) Only request I/O ports we really use The I/O area of the SMSC LPC47M1xx chips which we use, gives access to a lot of registers, some of which are related to fan speed monitoring and control, but many are not. At the moment, the smsc47m1 driver requests the whole I/O port range. This could easily result in resource conflicts with either ACPI or other drivers. Request only the I/O ports we really use, to prevent such conflicts. Signed-off-by: Jean Delvare Tested-by: Sean Fidler --- drivers/hwmon/smsc47m1.c | 99 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 89 insertions(+), 10 deletions(-) (limited to 'drivers/hwmon/smsc47m1.c') diff --git a/drivers/hwmon/smsc47m1.c b/drivers/hwmon/smsc47m1.c index 8ad50fdba00..bfef2239577 100644 --- a/drivers/hwmon/smsc47m1.c +++ b/drivers/hwmon/smsc47m1.c @@ -481,13 +481,94 @@ static int __init smsc47m1_find(unsigned short *addr, return 0; } +#define CHECK 1 +#define REQUEST 2 +#define RELEASE 3 + +/* + * This function can be used to: + * - test for resource conflicts with ACPI + * - request the resources + * - release the resources + * We only allocate the I/O ports we really need, to minimize the risk of + * conflicts with ACPI or with other drivers. + */ +static int smsc47m1_handle_resources(unsigned short address, enum chips type, + int action, struct device *dev) +{ + static const u8 ports_m1[] = { + /* register, region length */ + 0x04, 1, + 0x33, 4, + 0x56, 7, + }; + + static const u8 ports_m2[] = { + /* register, region length */ + 0x04, 1, + 0x09, 1, + 0x2c, 2, + 0x35, 4, + 0x56, 7, + 0x69, 4, + }; + + int i, ports_size, err; + const u8 *ports; + + switch (type) { + case smsc47m1: + default: + ports = ports_m1; + ports_size = ARRAY_SIZE(ports_m1); + break; + case smsc47m2: + ports = ports_m2; + ports_size = ARRAY_SIZE(ports_m2); + break; + } + + for (i = 0; i + 1 < ports_size; i += 2) { + unsigned short start = address + ports[i]; + unsigned short len = ports[i + 1]; + + switch (action) { + case CHECK: + /* Only check for conflicts */ + err = acpi_check_region(start, len, DRVNAME); + if (err) + return err; + break; + case REQUEST: + /* Request the resources */ + if (!request_region(start, len, DRVNAME)) { + dev_err(dev, "Region 0x%hx-0x%hx already in " + "use!\n", start, start + len); + + /* Undo all requests */ + for (i -= 2; i >= 0; i -= 2) + release_region(address + ports[i], + ports[i + 1]); + return -EBUSY; + } + break; + case RELEASE: + /* Release the resources */ + release_region(start, len); + break; + } + } + + return 0; +} + static int __devinit smsc47m1_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct smsc47m1_sio_data *sio_data = dev->platform_data; struct smsc47m1_data *data; struct resource *res; - int err = 0; + int err; int fan1, fan2, fan3, pwm1, pwm2, pwm3; static const char *names[] = { @@ -496,12 +577,10 @@ static int __devinit smsc47m1_probe(struct platform_device *pdev) }; res = platform_get_resource(pdev, IORESOURCE_IO, 0); - if (!request_region(res->start, SMSC_EXTENT, DRVNAME)) { - dev_err(dev, "Region 0x%lx-0x%lx already in use!\n", - (unsigned long)res->start, - (unsigned long)res->end); - return -EBUSY; - } + err = smsc47m1_handle_resources(res->start, sio_data->type, + REQUEST, dev); + if (err < 0) + return err; if (!(data = kzalloc(sizeof(struct smsc47m1_data), GFP_KERNEL))) { err = -ENOMEM; @@ -637,7 +716,7 @@ error_free: platform_set_drvdata(pdev, NULL); kfree(data); error_release: - release_region(res->start, SMSC_EXTENT); + smsc47m1_handle_resources(res->start, sio_data->type, RELEASE, dev); return err; } @@ -650,7 +729,7 @@ static int __devexit smsc47m1_remove(struct platform_device *pdev) sysfs_remove_group(&pdev->dev.kobj, &smsc47m1_group); res = platform_get_resource(pdev, IORESOURCE_IO, 0); - release_region(res->start, SMSC_EXTENT); + smsc47m1_handle_resources(res->start, data->type, RELEASE, &pdev->dev); platform_set_drvdata(pdev, NULL); kfree(data); @@ -717,7 +796,7 @@ static int __init smsc47m1_device_add(unsigned short address, }; int err; - err = acpi_check_resource_conflict(&res); + err = smsc47m1_handle_resources(address, sio_data->type, CHECK, NULL); if (err) goto exit; -- cgit v1.2.3-70-g09d2 From 3ecf44b312758d10be20539b06b2df5d77d59cdb Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Wed, 16 Dec 2009 21:38:26 +0100 Subject: hwmon: (smsc47m1) Fail module loading on error If an error occurs during probing, there's no point in keeping the module in memory. Better fail the module loading early to make the problem more visible. Signed-off-by: Jean Delvare Tested-by: Sean Fidler --- drivers/hwmon/smsc47m1.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) (limited to 'drivers/hwmon/smsc47m1.c') diff --git a/drivers/hwmon/smsc47m1.c b/drivers/hwmon/smsc47m1.c index bfef2239577..92cca512b38 100644 --- a/drivers/hwmon/smsc47m1.c +++ b/drivers/hwmon/smsc47m1.c @@ -139,8 +139,7 @@ struct smsc47m1_sio_data { }; -static int smsc47m1_probe(struct platform_device *pdev); -static int __devexit smsc47m1_remove(struct platform_device *pdev); +static int __exit smsc47m1_remove(struct platform_device *pdev); static struct smsc47m1_data *smsc47m1_update_device(struct device *dev, int init); @@ -160,8 +159,7 @@ static struct platform_driver smsc47m1_driver = { .owner = THIS_MODULE, .name = DRVNAME, }, - .probe = smsc47m1_probe, - .remove = __devexit_p(smsc47m1_remove), + .remove = __exit_p(smsc47m1_remove), }; static ssize_t get_fan(struct device *dev, struct device_attribute @@ -562,7 +560,7 @@ static int smsc47m1_handle_resources(unsigned short address, enum chips type, return 0; } -static int __devinit smsc47m1_probe(struct platform_device *pdev) +static int __init smsc47m1_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct smsc47m1_sio_data *sio_data = dev->platform_data; @@ -720,7 +718,7 @@ error_release: return err; } -static int __devexit smsc47m1_remove(struct platform_device *pdev) +static int __exit smsc47m1_remove(struct platform_device *pdev) { struct smsc47m1_data *data = platform_get_drvdata(pdev); struct resource *res; @@ -845,27 +843,27 @@ static int __init sm_smsc47m1_init(void) if (smsc47m1_find(&address, &sio_data)) return -ENODEV; - err = platform_driver_register(&smsc47m1_driver); + /* Sets global pdev as a side effect */ + err = smsc47m1_device_add(address, &sio_data); if (err) goto exit; - /* Sets global pdev as a side effect */ - err = smsc47m1_device_add(address, &sio_data); + err = platform_driver_probe(&smsc47m1_driver, smsc47m1_probe); if (err) - goto exit_driver; + goto exit_device; return 0; -exit_driver: - platform_driver_unregister(&smsc47m1_driver); +exit_device: + platform_device_unregister(pdev); exit: return err; } static void __exit sm_smsc47m1_exit(void) { - platform_device_unregister(pdev); platform_driver_unregister(&smsc47m1_driver); + platform_device_unregister(pdev); } MODULE_AUTHOR("Mark D. Studebaker "); -- cgit v1.2.3-70-g09d2 From fa0bff02239abdad446effef22e5db281cf3d562 Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Wed, 16 Dec 2009 21:38:27 +0100 Subject: hwmon: (smsc47m1) Enable device if needed If the address is set but the device isn't enabled, attempt to enable it. If it won't work for any reason (resource conflict, no function enabled) the initial state is restored. The initial state is also restored on module unloading. Signed-off-by: Jean Delvare Tested-by: Sean Fidler --- drivers/hwmon/smsc47m1.c | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) (limited to 'drivers/hwmon/smsc47m1.c') diff --git a/drivers/hwmon/smsc47m1.c b/drivers/hwmon/smsc47m1.c index 92cca512b38..9ca97818bd4 100644 --- a/drivers/hwmon/smsc47m1.c +++ b/drivers/hwmon/smsc47m1.c @@ -136,6 +136,7 @@ struct smsc47m1_data { struct smsc47m1_sio_data { enum chips type; + u8 activate; /* Remember initial device state */ }; @@ -468,17 +469,38 @@ static int __init smsc47m1_find(unsigned short *addr, superio_select(); *addr = (superio_inb(SUPERIO_REG_BASE) << 8) | superio_inb(SUPERIO_REG_BASE + 1); - val = superio_inb(SUPERIO_REG_ACT); - if (*addr == 0 || (val & 0x01) == 0) { - pr_info(DRVNAME ": Device is disabled, will not use\n"); + if (*addr == 0) { + pr_info(DRVNAME ": Device address not set, will not use\n"); superio_exit(); return -ENODEV; } + /* Enable only if address is set (needed at least on the + * Compaq Presario S4000NX) */ + sio_data->activate = superio_inb(SUPERIO_REG_ACT); + if ((sio_data->activate & 0x01) == 0) { + pr_info(DRVNAME ": Enabling device\n"); + superio_outb(SUPERIO_REG_ACT, sio_data->activate | 0x01); + } + superio_exit(); return 0; } +/* Restore device to its initial state */ +static void __init smsc47m1_restore(const struct smsc47m1_sio_data *sio_data) +{ + if ((sio_data->activate & 0x01) == 0) { + superio_enter(); + superio_select(); + + pr_info(DRVNAME ": Disabling device\n"); + superio_outb(SUPERIO_REG_ACT, sio_data->activate); + + superio_exit(); + } +} + #define CHECK 1 #define REQUEST 2 #define RELEASE 3 @@ -856,6 +878,7 @@ static int __init sm_smsc47m1_init(void) exit_device: platform_device_unregister(pdev); + smsc47m1_restore(&sio_data); exit: return err; } @@ -863,6 +886,7 @@ exit: static void __exit sm_smsc47m1_exit(void) { platform_driver_unregister(&smsc47m1_driver); + smsc47m1_restore(pdev->dev.platform_data); platform_device_unregister(pdev); } -- cgit v1.2.3-70-g09d2