For some (unclear to me) reason, superio_enter/superio_exit, although calling request_muxed_region/release_region (respectively), are not sufficient on an multi-processor system to guarantee serialised access, leading to the following errors: [ 1210.586990] Trying to free nonexistent resource <000000000000002e-000000000000002f> [ 1211.227414] Trying to free nonexistent resource <000000000000002e-000000000000002f> [ 1211.867890] Trying to free nonexistent resource <000000000000002e-000000000000002f> [ 1212.508299] Trying to free nonexistent resource <000000000000002e-000000000000002f> [ 1213.148734] Trying to free nonexistent resource <000000000000002e-000000000000002f> [ 1213.789172] Trying to free nonexistent resource <000000000000002e-000000000000002f> [ 1214.429607] Trying to free nonexistent resource <000000000000002e-000000000000002f>
Disabling all but one core make the error disappear, and re-enabling it make it reappear. Loaded modules for this SuperIO chip on this system are: 8250_fintek (not actively used) fintek_cir (not actively used) f71882fg (polled every 2 seconds from userland) gpio_f7188x Loaded modules using GPIO functionality: gpio_keys_polled (20ms polling period) leds_gpio (usb-host and 500ms timer triggers) This change replaces frequent superio_enter/superio_exit and accesses via the common GPIO IO region (0x2e..0x2f or 0x4e..0x4f) with mutex locking and accesses via GPIO-dedicated IO region (pre-configured on chip). Code inspired from hwmon/f71882fg . Signed-off-by: Vincent Pelletier <[email protected]> --- drivers/gpio/gpio-f7188x.c | 126 ++++++++++++++++++++++++++++----------------- 1 file changed, 80 insertions(+), 46 deletions(-) diff --git a/drivers/gpio/gpio-f7188x.c b/drivers/gpio/gpio-f7188x.c index dbda843..2915f8d 100644 --- a/drivers/gpio/gpio-f7188x.c +++ b/drivers/gpio/gpio-f7188x.c @@ -16,6 +16,8 @@ #include <linux/platform_device.h> #include <linux/io.h> #include <linux/gpio.h> +#include <linux/mutex.h> +#include <linux/acpi.h> #define DRVNAME "gpio-f7188x" @@ -26,11 +28,17 @@ #define SIO_DEVID 0x20 /* Device ID (2 bytes) */ #define SIO_DEVREV 0x22 /* Device revision */ #define SIO_MANID 0x23 /* Fintek ID (2 bytes) */ +#define SIO_ENABLE 0x30 /* Logical device enable */ +#define SIO_ADDR 0x60 /* Logical device address (2 bytes) */ #define SIO_LD_GPIO 0x06 /* GPIO logical device */ #define SIO_UNLOCK_KEY 0x87 /* Key to enable Super-I/O */ #define SIO_LOCK_KEY 0xAA /* Key to disable Super-I/O */ +#define REGION_LENGTH 8 +#define ADDR_REG_OFFSET 5 +#define DATA_REG_OFFSET 6 + #define SIO_FINTEK_ID 0x1934 /* Manufacturer ID */ #define SIO_F71869_ID 0x0814 /* F71869 chipset ID */ #define SIO_F71869A_ID 0x1007 /* F71869A chipset ID */ @@ -47,8 +55,9 @@ static const char * const f7188x_names[] = { }; struct f7188x_sio { - int addr; + u16 addr; enum chips type; + struct mutex lock; }; struct f7188x_gpio_bank { @@ -118,6 +127,22 @@ static inline void superio_exit(int base) release_region(base, 2); } +static u8 f7188x_read8(struct f7188x_sio *data, u8 reg) +{ + u8 val; + + outb(reg, data->addr + ADDR_REG_OFFSET); + val = inb(data->addr + DATA_REG_OFFSET); + + return val; +} + +static void f7188x_write8(struct f7188x_sio *data, u8 reg, u8 val) +{ + outb(reg, data->addr + ADDR_REG_OFFSET); + outb(val, data->addr + DATA_REG_OFFSET); +} + /* * GPIO chip. */ @@ -192,47 +217,35 @@ static struct f7188x_gpio_bank f71889_gpio_bank[] = { static int f7188x_gpio_direction_in(struct gpio_chip *chip, unsigned offset) { - int err; struct f7188x_gpio_bank *bank = container_of(chip, struct f7188x_gpio_bank, chip); struct f7188x_sio *sio = bank->data->sio; u8 dir; - err = superio_enter(sio->addr); - if (err) - return err; - superio_select(sio->addr, SIO_LD_GPIO); - - dir = superio_inb(sio->addr, gpio_dir(bank->regbase)); + mutex_lock(&sio->lock); + dir = f7188x_read8(sio, gpio_dir(bank->regbase)); dir &= ~(1 << offset); - superio_outb(sio->addr, gpio_dir(bank->regbase), dir); - - superio_exit(sio->addr); + f7188x_write8(sio, gpio_dir(bank->regbase), dir); + mutex_unlock(&sio->lock); return 0; } static int f7188x_gpio_get(struct gpio_chip *chip, unsigned offset) { - int err; struct f7188x_gpio_bank *bank = container_of(chip, struct f7188x_gpio_bank, chip); struct f7188x_sio *sio = bank->data->sio; u8 dir, data; - err = superio_enter(sio->addr); - if (err) - return err; - superio_select(sio->addr, SIO_LD_GPIO); - - dir = superio_inb(sio->addr, gpio_dir(bank->regbase)); + mutex_lock(&sio->lock); + dir = f7188x_read8(sio, gpio_dir(bank->regbase)); dir = !!(dir & (1 << offset)); if (dir) - data = superio_inb(sio->addr, gpio_data_out(bank->regbase)); + data = f7188x_read8(sio, gpio_data_out(bank->regbase)); else - data = superio_inb(sio->addr, gpio_data_in(bank->regbase)); - - superio_exit(sio->addr); + data = f7188x_read8(sio, gpio_data_in(bank->regbase)); + mutex_unlock(&sio->lock); return !!(data & 1 << offset); } @@ -240,54 +253,42 @@ static int f7188x_gpio_get(struct gpio_chip *chip, unsigned offset) static int f7188x_gpio_direction_out(struct gpio_chip *chip, unsigned offset, int value) { - int err; struct f7188x_gpio_bank *bank = container_of(chip, struct f7188x_gpio_bank, chip); struct f7188x_sio *sio = bank->data->sio; u8 dir, data_out; - err = superio_enter(sio->addr); - if (err) - return err; - superio_select(sio->addr, SIO_LD_GPIO); - - data_out = superio_inb(sio->addr, gpio_data_out(bank->regbase)); + mutex_lock(&sio->lock); + data_out = f7188x_read8(sio, gpio_data_out(bank->regbase)); if (value) data_out |= (1 << offset); else data_out &= ~(1 << offset); - superio_outb(sio->addr, gpio_data_out(bank->regbase), data_out); + f7188x_write8(sio, gpio_data_out(bank->regbase), data_out); - dir = superio_inb(sio->addr, gpio_dir(bank->regbase)); + dir = f7188x_read8(sio, gpio_dir(bank->regbase)); dir |= (1 << offset); - superio_outb(sio->addr, gpio_dir(bank->regbase), dir); - - superio_exit(sio->addr); + f7188x_write8(sio, gpio_dir(bank->regbase), dir); + mutex_unlock(&sio->lock); return 0; } static void f7188x_gpio_set(struct gpio_chip *chip, unsigned offset, int value) { - int err; struct f7188x_gpio_bank *bank = container_of(chip, struct f7188x_gpio_bank, chip); struct f7188x_sio *sio = bank->data->sio; u8 data_out; - err = superio_enter(sio->addr); - if (err) - return; - superio_select(sio->addr, SIO_LD_GPIO); - - data_out = superio_inb(sio->addr, gpio_data_out(bank->regbase)); + mutex_lock(&sio->lock); + data_out = f7188x_read8(sio, gpio_data_out(bank->regbase)); if (value) data_out |= (1 << offset); else data_out &= ~(1 << offset); - superio_outb(sio->addr, gpio_data_out(bank->regbase), data_out); - - superio_exit(sio->addr); + f7188x_write8(sio, gpio_data_out(bank->regbase), data_out); + mutex_unlock(&sio->lock); } /* @@ -326,6 +327,7 @@ static int f7188x_gpio_probe(struct platform_device *pdev) return -ENODEV; } data->sio = sio; + mutex_init(&sio->lock); platform_set_drvdata(pdev, data); @@ -372,6 +374,7 @@ static int f7188x_gpio_remove(struct platform_device *pdev) static int __init f7188x_find(int addr, struct f7188x_sio *sio) { int err; + u16 logical_device_address; u16 devid; err = superio_enter(addr); @@ -403,12 +406,25 @@ static int __init f7188x_find(int addr, struct f7188x_sio *sio) pr_info(DRVNAME ": Unsupported Fintek device 0x%04x\n", devid); goto err; } - sio->addr = addr; + superio_select(addr, SIO_LD_GPIO); + + if (!(superio_inb(addr, SIO_ENABLE) & 0x01)) { + pr_warn("Device not activated\n"); + goto err; + } + + logical_device_address = superio_inw(addr, SIO_ADDR); + if (logical_device_address == 0) { + pr_warn("Base address not set\n"); + goto err; + } + logical_device_address &= ~(REGION_LENGTH - 1); /* Ignore 3 LSB */ + sio->addr = logical_device_address; err = 0; pr_info(DRVNAME ": Found %s at %#x, revision %d\n", f7188x_names[sio->type], - (unsigned int) addr, + (unsigned int) logical_device_address, (int) superio_inb(addr, SIO_DEVREV)); err: @@ -421,12 +437,28 @@ static struct platform_device *f7188x_gpio_pdev; static int __init f7188x_gpio_device_add(const struct f7188x_sio *sio) { + struct resource res = { + .start = sio->addr, + .end = sio->addr + REGION_LENGTH - 1, + .flags = IORESOURCE_IO, + }; int err; f7188x_gpio_pdev = platform_device_alloc(DRVNAME, -1); if (!f7188x_gpio_pdev) return -ENOMEM; + res.name = f7188x_gpio_pdev->name; + err = acpi_check_resource_conflict(&res); + if (err) + goto err; + + err = platform_device_add_resources(f7188x_gpio_pdev, &res, 1); + if (err) { + pr_err("Device resource addition failed\n"); + goto err; + } + err = platform_device_add_data(f7188x_gpio_pdev, sio, sizeof(*sio)); if (err) { @@ -467,6 +499,8 @@ static int __init f7188x_gpio_init(void) int err; struct f7188x_sio sio; + memset(&sio, 0, sizeof(sio)); + if (f7188x_find(0x2e, &sio) && f7188x_find(0x4e, &sio)) return -ENODEV; -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
