From patchwork Tue Oct 10 12:48:58 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ricardo Ribalda Delgado X-Patchwork-Id: 823829 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=linux-i2c-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="EX3zs7zE"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3yBH431qDZz9tY3 for ; Tue, 10 Oct 2017 23:49:07 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932219AbdJJMtF (ORCPT ); Tue, 10 Oct 2017 08:49:05 -0400 Received: from mail-lf0-f68.google.com ([209.85.215.68]:48959 "EHLO mail-lf0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932177AbdJJMtE (ORCPT ); Tue, 10 Oct 2017 08:49:04 -0400 Received: by mail-lf0-f68.google.com with SMTP id a69so7874201lfe.5; Tue, 10 Oct 2017 05:49:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=Egxf4X2PryBsLuD3DS08zMC2UlgJsG0Lcu40ddC4ncA=; b=EX3zs7zEbNWLLCLp+fFR84Sp8xg8b8wUBOzQEP2P/nXmJYyb33Z73f68Xe6NxOYNsG lmAfRXAaFwI7xbeVEurUu/zZEEYyg/VijOyC1nQgRu+RL2ez9TReTn+3Ped570HnR5ic GBM8E/MAOirjluj2XcFA+D808j2RRHntq4cJI+p7SCxyYaCm+Rpsp6AzdscX6TFMhTrZ AMJyLX5aPgjlaDigtxlkiU3wzYobvpE9xyiolEbax0qxZyimwaFd+Vq5WpwRJ/ZLw/5T oFnSVqxpFc8WiaqBWOV5kUwkSVc+ZwFlJCaF3vkDdYHU4Kq8pDpALEqP8PuleckcOF6I LQHw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=Egxf4X2PryBsLuD3DS08zMC2UlgJsG0Lcu40ddC4ncA=; b=CjLtFfSwKXfTInbPnMZUOsEmOg4wsum7DYVCXlklwmOBQWEHTHWxtpKJo6WAW1XLQe v8ELj7mjQSLdqmbGMY3/1S48aYChVxfYbeKAEZ0tp+W7lrhNyvLAMoZ7KpB5FtP2G8lE R8Sbpv7pTx1xAXuxm2upUZ0wXik7BTy4DQ8b9G2nqat1mRANgKvEUT+LeY++eJLAkZde gbAADFYpLLjap7DJbHdvaoA4fZ3sZYjWWSpAnamH7eNj2wfCduPiHoYItt4vXROotddM l1Bl+dokTvRA0e/EMTb7Nrb1HvY2opMhofyp4pfYZkS+49vNquGsRPen4uM6WHrPr8ZR vwVw== X-Gm-Message-State: AMCzsaUTEheQ5f4LMTt0yJrBwnokxdSiMok6/VF/8OvY0bycSD5Fmbfi tLJu3XrmTnZ+eGPXL0QEnno= X-Google-Smtp-Source: AOwi7QBg9km88MtxY1L/XP73AWoyqeJWmrafcn4cNsH+zG0ZD8mHbGhkVGhqR5Rp883X1ADY7GgBHQ== X-Received: by 10.25.99.208 with SMTP id v77mr5118720lfi.108.1507639742217; Tue, 10 Oct 2017 05:49:02 -0700 (PDT) Received: from neopili.qtec.com (cpe.xe-3-0-1-778.vbrnqe10.dk.customer.tdc.net. [80.197.57.18]) by smtp.gmail.com with ESMTPSA id 66sm1819964lfs.60.2017.10.10.05.49.00 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 10 Oct 2017 05:49:00 -0700 (PDT) From: Ricardo Ribalda Delgado To: Jean Delvare , Wolfram Sang , linux-i2c@vger.kernel.org, LKML Cc: Ricardo Ribalda Delgado Subject: [PATCH v2] i2c: piix4: Disable completely the IMC during SMBUS_BLOCK_DATA Date: Tue, 10 Oct 2017 14:48:58 +0200 Message-Id: <20171010124858.2247-1-ricardo.ribalda@gmail.com> X-Mailer: git-send-email 2.14.2 Sender: linux-i2c-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-i2c@vger.kernel.org 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 --- v2: Suggestions by Jean Delvare -Fix function definitions (static) -Add register definition -Use muxed_io interface -Improve comments -Keep old timeout -Rebase 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..04ade552d426 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 IMC_SMB_IDX 0x3e +#define IMC_SMB_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. @@ -168,6 +171,7 @@ struct i2c_piix4_adapdata { /* SB800 */ bool sb800_main; u8 port; /* Port number, shifted */ + bool notify_imc; }; static int piix4_setup(struct pci_dev *PIIX4_dev, @@ -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, IMC_SMB_IDX); + return inb_p(IMC_SMB_DATA); +} + +static void piix4_imc_write(uint8_t idx, uint8_t value) +{ + outb_p(idx, IMC_SMB_IDX); + outb_p(value, IMC_SMB_DATA); +} + +static int piix4_imc_sleep(void) +{ + int timeout = MAX_TIMEOUT; + + if (!request_muxed_region(IMC_SMB_IDX, 2, "kerncz_imc_idx")) + 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(IMC_SMB_IDX, 2); + return 0; + } + usleep_range(1000, 2000); + } + + release_region(IMC_SMB_IDX, 2); + return -ETIMEDOUT; +} + +static void piix4_imc_wakeup(void) +{ + int timeout = MAX_TIMEOUT; + + if (!request_muxed_region(IMC_SMB_IDX, 2, "kerncz_imc_idx")) + 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(IMC_SMB_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", + IMC_SMB_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); }