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 <ricardo.ribalda@gmail.com>
Reviewed-by: Jean Delvare <jdelvare@suse.de>
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>

diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index 01f767e..174579d 100644 (file)
--- 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);
        }