From a28e35171cb1ff84197e8d271b65aaeb8c404827 Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Fri, 22 Jan 2016 14:12:02 +0100 Subject: i2c: piix4: Fix SB800 locking We need a single mutex for all 4 shared SMBus ports on the SB800. A per-port mutex doesn't protect us from concurrent access. In theory the mutex should be per PCI device, however in practice we know that there's only ever a single instance of the device in a given system so we can use a global. Also take the mutex during initialization, as first port may be already in use when second port is initialized. Signed-off-by: Jean Delvare Tested-by: Christian Fetzer Reviewed-by: Mika Westerberg [wsa: made mutex static] Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-piix4.c | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c index e04598595073..5fd7505b9018 100644 --- a/drivers/i2c/busses/i2c-piix4.c +++ b/drivers/i2c/busses/i2c-piix4.c @@ -137,6 +137,7 @@ static const struct dmi_system_id piix4_dmi_ibm[] = { }; /* SB800 globals */ +static DEFINE_MUTEX(piix4_mutex_sb800); static const char *piix4_main_port_names_sb800[PIIX4_MAX_ADAPTERS] = { "SDA0", "SDA2", "SDA3", "SDA4" }; @@ -148,7 +149,6 @@ struct i2c_piix4_adapdata { /* SB800 */ bool sb800_main; unsigned short port; - struct mutex *mutex; }; static int piix4_setup(struct pci_dev *PIIX4_dev, @@ -275,10 +275,12 @@ static int piix4_setup_sb800(struct pci_dev *PIIX4_dev, else smb_en = (aux) ? 0x28 : 0x2c; + mutex_lock(&piix4_mutex_sb800); outb_p(smb_en, SB800_PIIX4_SMB_IDX); smba_en_lo = inb_p(SB800_PIIX4_SMB_IDX + 1); outb_p(smb_en + 1, SB800_PIIX4_SMB_IDX); smba_en_hi = inb_p(SB800_PIIX4_SMB_IDX + 1); + mutex_unlock(&piix4_mutex_sb800); if (!smb_en) { smb_en_status = smba_en_lo & 0x10; @@ -559,7 +561,7 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr, u8 port; int retval; - mutex_lock(adapdata->mutex); + mutex_lock(&piix4_mutex_sb800); outb_p(SB800_PIIX4_PORT_IDX, SB800_PIIX4_SMB_IDX); smba_en_lo = inb_p(SB800_PIIX4_SMB_IDX + 1); @@ -574,7 +576,7 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr, outb_p(smba_en_lo, SB800_PIIX4_SMB_IDX + 1); - mutex_unlock(adapdata->mutex); + mutex_unlock(&piix4_mutex_sb800); return retval; } @@ -673,17 +675,10 @@ static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba, static int piix4_add_adapters_sb800(struct pci_dev *dev, unsigned short smba) { - struct mutex *mutex; struct i2c_piix4_adapdata *adapdata; int port; int retval; - mutex = kzalloc(sizeof(*mutex), GFP_KERNEL); - if (mutex == NULL) - return -ENOMEM; - - mutex_init(mutex); - for (port = 0; port < PIIX4_MAX_ADAPTERS; port++) { retval = piix4_add_adapter(dev, smba, piix4_main_port_names_sb800[port], @@ -696,7 +691,6 @@ static int piix4_add_adapters_sb800(struct pci_dev *dev, unsigned short smba) adapdata = i2c_get_adapdata(piix4_main_adapters[port]); adapdata->sb800_main = true; adapdata->port = port; - adapdata->mutex = mutex; } return retval; @@ -714,8 +708,6 @@ error: } } - kfree(mutex); - return retval; } @@ -798,10 +790,8 @@ static void piix4_adap_remove(struct i2c_adapter *adap) i2c_del_adapter(adap); if (adapdata->port == 0) { release_region(adapdata->smba, SMBIOSIZE); - if (adapdata->sb800_main) { - kfree(adapdata->mutex); + if (adapdata->sb800_main) release_region(SB800_PIIX4_SMB_IDX, 2); - } } kfree(adapdata); kfree(adap); -- cgit v1.2.3-59-g8ed1b From 83c60158ebf16417af28b338bc0380cf17f2b9f9 Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Mon, 25 Jan 2016 12:17:07 +0100 Subject: i2c: piix4: Fully initialize SB800 before it is registered This closes a race window where I2C device drivers attempt to access I2C buses which aren't fully initialized yet. Signed-off-by: Jean Delvare Tested-by: Christian Fetzer Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-piix4.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c index 5fd7505b9018..f79a84ef1aa4 100644 --- a/drivers/i2c/busses/i2c-piix4.c +++ b/drivers/i2c/busses/i2c-piix4.c @@ -627,6 +627,7 @@ static struct i2c_adapter *piix4_main_adapters[PIIX4_MAX_ADAPTERS]; static struct i2c_adapter *piix4_aux_adapter; static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba, + bool sb800_main, unsigned short port, const char *name, struct i2c_adapter **padap) { struct i2c_adapter *adap; @@ -641,7 +642,8 @@ static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba, adap->owner = THIS_MODULE; adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD; - adap->algo = &smbus_algorithm; + adap->algo = sb800_main ? &piix4_smbus_algorithm_sb800 + : &smbus_algorithm; adapdata = kzalloc(sizeof(*adapdata), GFP_KERNEL); if (adapdata == NULL) { @@ -651,6 +653,8 @@ static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba, } adapdata->smba = smba; + adapdata->sb800_main = sb800_main; + adapdata->port = port; /* set up the sysfs linkage to our parent device */ adap->dev.parent = &dev->dev; @@ -680,17 +684,11 @@ static int piix4_add_adapters_sb800(struct pci_dev *dev, unsigned short smba) int retval; for (port = 0; port < PIIX4_MAX_ADAPTERS; port++) { - retval = piix4_add_adapter(dev, smba, + retval = piix4_add_adapter(dev, smba, true, port, piix4_main_port_names_sb800[port], &piix4_main_adapters[port]); if (retval < 0) goto error; - - piix4_main_adapters[port]->algo = &piix4_smbus_algorithm_sb800; - - adapdata = i2c_get_adapdata(piix4_main_adapters[port]); - adapdata->sb800_main = true; - adapdata->port = port; } return retval; @@ -748,7 +746,7 @@ static int piix4_probe(struct pci_dev *dev, const struct pci_device_id *id) return retval; /* Try to register main SMBus adapter, give up if we can't */ - retval = piix4_add_adapter(dev, retval, "main", + retval = piix4_add_adapter(dev, retval, false, 0, "main", &piix4_main_adapters[0]); if (retval < 0) return retval; @@ -775,7 +773,8 @@ static int piix4_probe(struct pci_dev *dev, const struct pci_device_id *id) if (retval > 0) { /* Try to add the aux adapter if it exists, * piix4_add_adapter will clean up if this fails */ - piix4_add_adapter(dev, retval, piix4_aux_port_name_sb800, + piix4_add_adapter(dev, retval, false, 0, + piix4_aux_port_name_sb800, &piix4_aux_adapter); } -- cgit v1.2.3-59-g8ed1b From 08c6e8cc66282a082484480c1a5641bc27d26c55 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Fri, 15 Jan 2016 22:02:12 +0200 Subject: i2c: designware-pci: use IRQF_COND_SUSPEND flag This is effectively reapplies the commit b0898fdaffb2 ("i2c: designware-pci: use IRQF_COND_SUSPEND flag") after the commit d80d134182ba ("i2c: designware: Move common probe code into i2c_dw_probe()"). Original message as follows. The mentioned flag fixes a warning on Intel Edison board since one of the I2C controller shares IRQ line with watchdog timer. Fixes: d80d134182ba (i2c: designware: Move common probe code into i2c_dw_probe()) Signed-off-by: Andy Shevchenko Acked-by: Jarkko Nikula Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-designware-core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c index ba9732c236c5..10fbd6d841e0 100644 --- a/drivers/i2c/busses/i2c-designware-core.c +++ b/drivers/i2c/busses/i2c-designware-core.c @@ -874,7 +874,8 @@ int i2c_dw_probe(struct dw_i2c_dev *dev) i2c_set_adapdata(adap, dev); i2c_dw_disable_int(dev); - r = devm_request_irq(dev->dev, dev->irq, i2c_dw_isr, IRQF_SHARED, + r = devm_request_irq(dev->dev, dev->irq, i2c_dw_isr, + IRQF_SHARED | IRQF_COND_SUSPEND, dev_name(dev->dev), dev); if (r) { dev_err(dev->dev, "failure requesting irq %i: %d\n", -- cgit v1.2.3-59-g8ed1b From 52795f6fdeb8a2b98373108ac2838c674bb2cbc4 Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Wed, 27 Jan 2016 14:40:33 +0100 Subject: i2c: piix4: don't regress on bus names The I2C bus names are supposed to be stable as they can be used by userspace to uniquely identify a specific I2C bus. So restore the original names for all legacy (pre-SB800) devices. For SB800 devices and later, improve the names. "SDA" refers to the serial data pin of each SMBus port, it's an implementation detail the user doesn't need to know. Use "port" instead, which is easier to understand. Signed-off-by: Jean Delvare Reviewed-by: Mika Westerberg Tested-by: Christian Fetzer Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-piix4.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c index f79a84ef1aa4..93f2895383ee 100644 --- a/drivers/i2c/busses/i2c-piix4.c +++ b/drivers/i2c/busses/i2c-piix4.c @@ -139,9 +139,9 @@ static const struct dmi_system_id piix4_dmi_ibm[] = { /* SB800 globals */ static DEFINE_MUTEX(piix4_mutex_sb800); static const char *piix4_main_port_names_sb800[PIIX4_MAX_ADAPTERS] = { - "SDA0", "SDA2", "SDA3", "SDA4" + " port 0", " port 2", " port 3", " port 4" }; -static const char *piix4_aux_port_name_sb800 = "SDA1"; +static const char *piix4_aux_port_name_sb800 = " port 1"; struct i2c_piix4_adapdata { unsigned short smba; @@ -660,7 +660,7 @@ static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba, adap->dev.parent = &dev->dev; snprintf(adap->name, sizeof(adap->name), - "SMBus PIIX4 adapter %s at %04x", name, smba); + "SMBus PIIX4 adapter%s at %04x", name, smba); i2c_set_adapdata(adap, adapdata); @@ -712,11 +712,14 @@ error: static int piix4_probe(struct pci_dev *dev, const struct pci_device_id *id) { int retval; + bool is_sb800 = false; if ((dev->vendor == PCI_VENDOR_ID_ATI && dev->device == PCI_DEVICE_ID_ATI_SBX00_SMBUS && dev->revision >= 0x40) || dev->vendor == PCI_VENDOR_ID_AMD) { + is_sb800 = true; + if (!request_region(SB800_PIIX4_SMB_IDX, 2, "smba_idx")) { dev_err(&dev->dev, "SMBus base address index region 0x%x already in use!\n", @@ -746,7 +749,7 @@ static int piix4_probe(struct pci_dev *dev, const struct pci_device_id *id) return retval; /* Try to register main SMBus adapter, give up if we can't */ - retval = piix4_add_adapter(dev, retval, false, 0, "main", + retval = piix4_add_adapter(dev, retval, false, 0, "", &piix4_main_adapters[0]); if (retval < 0) return retval; @@ -774,7 +777,7 @@ static int piix4_probe(struct pci_dev *dev, const struct pci_device_id *id) /* Try to add the aux adapter if it exists, * piix4_add_adapter will clean up if this fails */ piix4_add_adapter(dev, retval, false, 0, - piix4_aux_port_name_sb800, + is_sb800 ? piix4_aux_port_name_sb800 : "", &piix4_aux_adapter); } -- cgit v1.2.3-59-g8ed1b