From 88fa2dfb075a20c3464e3d303c57dd8ced9e8309 Mon Sep 17 00:00:00 2001 From: Ricardo Ribalda Delgado Date: Tue, 10 Oct 2017 18:11:15 +0200 Subject: [PATCH] i2c: piix4: Disable completely the IMC during SMBUS_BLOCK_DATA SMBUS_BLOCK_DATA transactions might fail due to a race condition with the IMC (Integrated Micro Controller), even when the IMC semaphore is used. This bug has been reported and confirmed by AMD, who suggested as a solution an IMC firmware upgrade (obtained via BIOS update) and disabling the IMC during SMBUS_BLOCK_DATA transactions. Even without the IMC upgrade, the SMBUS is much more stable with this patch. Tested on a Bettong-alike board. Signed-off-by: Ricardo Ribalda Delgado Reviewed-by: Jean Delvare Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-piix4.c | 132 +++++++++++++++++++++++++++++++-- 1 file changed, 126 insertions(+), 6 deletions(-) diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c index 01f767ee4546..174579d32e5f 100644 --- a/drivers/i2c/busses/i2c-piix4.c +++ b/drivers/i2c/busses/i2c-piix4.c @@ -85,6 +85,9 @@ /* SB800 constants */ #define SB800_PIIX4_SMB_IDX 0xcd6 +#define KERNCZ_IMC_IDX 0x3e +#define KERNCZ_IMC_DATA 0x3f + /* * SB800 port is selected by bits 2:1 of the smb_en register (0x2c) * or the smb_sel register (0x2e), depending on bit 0 of register 0x2f. @@ -167,6 +170,7 @@ struct i2c_piix4_adapdata { /* SB800 */ bool sb800_main; + bool notify_imc; u8 port; /* Port number, shifted */ }; @@ -594,6 +598,67 @@ static s32 piix4_access(struct i2c_adapter * adap, u16 addr, return 0; } +static uint8_t piix4_imc_read(uint8_t idx) +{ + outb_p(idx, KERNCZ_IMC_IDX); + return inb_p(KERNCZ_IMC_DATA); +} + +static void piix4_imc_write(uint8_t idx, uint8_t value) +{ + outb_p(idx, KERNCZ_IMC_IDX); + outb_p(value, KERNCZ_IMC_DATA); +} + +static int piix4_imc_sleep(void) +{ + int timeout = MAX_TIMEOUT; + + if (!request_muxed_region(KERNCZ_IMC_IDX, 2, "smbus_kerncz_imc")) + return -EBUSY; + + /* clear response register */ + piix4_imc_write(0x82, 0x00); + /* request ownership flag */ + piix4_imc_write(0x83, 0xB4); + /* kick off IMC Mailbox command 96 */ + piix4_imc_write(0x80, 0x96); + + while (timeout--) { + if (piix4_imc_read(0x82) == 0xfa) { + release_region(KERNCZ_IMC_IDX, 2); + return 0; + } + usleep_range(1000, 2000); + } + + release_region(KERNCZ_IMC_IDX, 2); + return -ETIMEDOUT; +} + +static void piix4_imc_wakeup(void) +{ + int timeout = MAX_TIMEOUT; + + if (!request_muxed_region(KERNCZ_IMC_IDX, 2, "smbus_kerncz_imc")) + return; + + /* clear response register */ + piix4_imc_write(0x82, 0x00); + /* release ownership flag */ + piix4_imc_write(0x83, 0xB5); + /* kick off IMC Mailbox command 96 */ + piix4_imc_write(0x80, 0x96); + + while (timeout--) { + if (piix4_imc_read(0x82) == 0xfa) + break; + usleep_range(1000, 2000); + } + + release_region(KERNCZ_IMC_IDX, 2); +} + /* * Handles access to multiple SMBus ports on the SB800. * The port is selected by bits 2:1 of the smb_en register (0x2c). @@ -634,6 +699,41 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr, return -EBUSY; } + /* + * Notify the IMC (Integrated Micro Controller) if required. + * Among other responsibilities, the IMC is in charge of monitoring + * the System fans and temperature sensors, and act accordingly. + * All this is done through SMBus and can/will collide + * with our transactions if they are long (BLOCK_DATA). + * Therefore we need to request the ownership flag during those + * transactions. + */ + if ((size == I2C_SMBUS_BLOCK_DATA) && adapdata->notify_imc) { + int ret; + + ret = piix4_imc_sleep(); + switch (ret) { + case -EBUSY: + dev_warn(&adap->dev, + "IMC base address index region 0x%x already in use.\n", + KERNCZ_IMC_IDX); + break; + case -ETIMEDOUT: + dev_warn(&adap->dev, + "Failed to communicate with the IMC.\n"); + break; + default: + break; + } + + /* If IMC communication fails do not retry */ + if (ret) { + dev_warn(&adap->dev, + "Continuing without IMC notification.\n"); + adapdata->notify_imc = false; + } + } + outb_p(piix4_port_sel_sb800, SB800_PIIX4_SMB_IDX); smba_en_lo = inb_p(SB800_PIIX4_SMB_IDX + 1); @@ -650,6 +750,9 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr, /* Release the semaphore */ outb_p(smbslvcnt | 0x20, SMBSLVCNT); + if ((size == I2C_SMBUS_BLOCK_DATA) && adapdata->notify_imc) + piix4_imc_wakeup(); + mutex_unlock(&piix4_mutex_sb800); return retval; @@ -701,7 +804,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, u8 port, + bool sb800_main, u8 port, bool notify_imc, const char *name, struct i2c_adapter **padap) { struct i2c_adapter *adap; @@ -729,6 +832,7 @@ static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba, adapdata->smba = smba; adapdata->sb800_main = sb800_main; adapdata->port = port << piix4_port_shift_sb800; + adapdata->notify_imc = notify_imc; /* set up the sysfs linkage to our parent device */ adap->dev.parent = &dev->dev; @@ -750,14 +854,15 @@ static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba, return 0; } -static int piix4_add_adapters_sb800(struct pci_dev *dev, unsigned short smba) +static int piix4_add_adapters_sb800(struct pci_dev *dev, unsigned short smba, + bool notify_imc) { struct i2c_piix4_adapdata *adapdata; int port; int retval; for (port = 0; port < PIIX4_MAX_ADAPTERS; port++) { - retval = piix4_add_adapter(dev, smba, true, port, + retval = piix4_add_adapter(dev, smba, true, port, notify_imc, piix4_main_port_names_sb800[port], &piix4_main_adapters[port]); if (retval < 0) @@ -791,6 +896,7 @@ static int piix4_probe(struct pci_dev *dev, const struct pci_device_id *id) dev->device == PCI_DEVICE_ID_ATI_SBX00_SMBUS && dev->revision >= 0x40) || dev->vendor == PCI_VENDOR_ID_AMD) { + bool notify_imc = false; is_sb800 = true; if (!request_region(SB800_PIIX4_SMB_IDX, 2, "smba_idx")) { @@ -800,6 +906,20 @@ static int piix4_probe(struct pci_dev *dev, const struct pci_device_id *id) return -EBUSY; } + if (dev->vendor == PCI_VENDOR_ID_AMD && + dev->device == PCI_DEVICE_ID_AMD_KERNCZ_SMBUS) { + u8 imc; + + /* + * Detect if IMC is active or not, this method is + * described on coreboot's AMD IMC notes + */ + pci_bus_read_config_byte(dev->bus, PCI_DEVFN(0x14, 3), + 0x40, &imc); + if (imc & 0x80) + notify_imc = true; + } + /* base address location etc changed in SB800 */ retval = piix4_setup_sb800(dev, id, 0); if (retval < 0) { @@ -811,7 +931,7 @@ static int piix4_probe(struct pci_dev *dev, const struct pci_device_id *id) * Try to register multiplexed main SMBus adapter, * give up if we can't */ - retval = piix4_add_adapters_sb800(dev, retval); + retval = piix4_add_adapters_sb800(dev, retval, notify_imc); if (retval < 0) { release_region(SB800_PIIX4_SMB_IDX, 2); return retval; @@ -822,7 +942,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, "", + retval = piix4_add_adapter(dev, retval, false, 0, false, "", &piix4_main_adapters[0]); if (retval < 0) return retval; @@ -849,7 +969,7 @@ 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, false, 0, + piix4_add_adapter(dev, retval, false, 0, false, is_sb800 ? piix4_aux_port_name_sb800 : "", &piix4_aux_adapter); } -- 2.20.1