From patchwork Fri May 22 22:14:28 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ellen Wang X-Patchwork-Id: 475823 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 6C1C4140E45 for ; Sat, 23 May 2015 08:14:37 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=cumulusnetworks.com header.i=@cumulusnetworks.com header.b=KF1uVRB9; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757176AbbEVWOg (ORCPT ); Fri, 22 May 2015 18:14:36 -0400 Received: from mail-pd0-f171.google.com ([209.85.192.171]:35601 "EHLO mail-pd0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757125AbbEVWOf (ORCPT ); Fri, 22 May 2015 18:14:35 -0400 Received: by pdea3 with SMTP id a3so29123418pde.2 for ; Fri, 22 May 2015 15:14:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cumulusnetworks.com; s=google; h=from:to:cc:subject:date:message-id; bh=qZsLiI/zBAgdZt+esyh6U7KRbu9F+mf0dpc1X0AX26Y=; b=KF1uVRB9hFpeRiqpTbcSv3qgqT6q0g7/mtZVuvdy3OirbqxG5IBmhgPOBjfWDt2iOQ GSvPkbz04XiWobgEwwIn+TIX6IrkVM+lawjfiDm7+dk6zIvvkVCVagDK5Z2FtdQZB4YK Me10HBeI3hxLQtv+ENUz8SJ4ilVycCq/zmxUY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=qZsLiI/zBAgdZt+esyh6U7KRbu9F+mf0dpc1X0AX26Y=; b=aM0iZqU7jvd2eJEciGu+gXjzsFiRBottr67buQKxwlYRlr7TLyFU04lk+EId6tC+dY M+LuWL+zQLPFKsY/pXSyY/btG72W1MbFC93YRDinlpoWkMM1NWDv92mh3v4/PlzmVL8f QcZpzTcXoJr2cw4tMuciFrHptSSDribqamkw9mwumYUnBiAcLL7ZPCbA4H44MSR8pJfN f6s70Fo1B7Txetu94D9Y4Wgdpn8rH+vQIRfTYqnqr8oKQbAgv7huIIxyb5PQweFGHXNL 32KJ/3CBMjSLufVoIh3wTRC/cBdfHBBo5O7Ac0KI+9shpu2062Gmu6uvsi/8leCerroR dCaw== X-Gm-Message-State: ALoCoQnqpGl5zCI/OB2Bu9T7kXauTOJNYneMqg7peN5m6UWHLZ1thRTQSvWrJzNHas0jYWJ1oXFg X-Received: by 10.68.68.203 with SMTP id y11mr11134635pbt.34.1432332875306; Fri, 22 May 2015 15:14:35 -0700 (PDT) Received: from monster-01.cumulusnetworks.com ([216.129.126.126]) by mx.google.com with ESMTPSA id x4sm3067441pdl.55.2015.05.22.15.14.34 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 22 May 2015 15:14:34 -0700 (PDT) From: Ellen Wang To: jdelvare@suse.de, wsa@the-dreams.de, linux-i2c@vger.kernel.org Cc: ellen@cumulusnetworks.com Subject: [PATCH v3 i2c/for-next] i2c: i801: recover from hardware PEC errors Date: Fri, 22 May 2015 15:14:28 -0700 Message-Id: <1432332868-12705-1-git-send-email-ellen@cumulusnetworks.com> X-Mailer: git-send-email 1.7.10.4 Sender: linux-i2c-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-i2c@vger.kernel.org On a CRC error while using hardware-supported PEC, an additional error bit is set in the auxiliary status register. If this bit isn't cleared, all subsequent operations will fail, essentially hanging the controller. The fix is simple: check, report, and clear the bit in i801_check_post(). Also, in case the driver starts with the hardware in that state, clear it in i801_check_pre() as well. Signed-off-by: Ellen Wang Tested-by: Jean Delvare Reviewed-by: Jean Delvare --- Update: fix typos in commit message, otherwise the same as v2 This is essentially the patch from Jean Delvare, which handles the polling case while my original version didn't. (Thank you! Please add appropriate attribution if you wish.) I tested all the additional code paths by selectively commenting out code: with interrupts, without interrupts, relying on check_pre() to clear CRCE, no clearing of CRCE at all (baseline). --- drivers/i2c/busses/i2c-i801.c | 53 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c index 5ecbb3f..fa50df0 100644 --- a/drivers/i2c/busses/i2c-i801.c +++ b/drivers/i2c/busses/i2c-i801.c @@ -125,6 +125,10 @@ #define SMBHSTCFG_SMB_SMI_EN 2 #define SMBHSTCFG_I2C_EN 4 +/* Auxiliary status register bits, ICH4+ only */ +#define SMBAUXSTS_CRCE 1 +#define SMBAUXSTS_STCO 2 + /* Auxiliary control register bits, ICH4+ only */ #define SMBAUXCTL_CRC 1 #define SMBAUXCTL_E32B 2 @@ -273,6 +277,29 @@ static int i801_check_pre(struct i801_priv *priv) } } + /* + * Clear CRC status if needed. + * During normal operation, i801_check_post() takes care + * of it after every operation. We do it here only in case + * the hardware was already in this state when the driver + * started. + */ + if (priv->features & FEATURE_SMBUS_PEC) { + status = inb_p(SMBAUXSTS(priv)) & SMBAUXSTS_CRCE; + if (status) { + dev_dbg(&priv->pci_dev->dev, + "Clearing aux status flags (%02x)\n", status); + outb_p(status, SMBAUXSTS(priv)); + status = inb_p(SMBAUXSTS(priv)) & SMBAUXSTS_CRCE; + if (status) { + dev_err(&priv->pci_dev->dev, + "Failed clearing aux status flags (%02x)\n", + status); + return -EBUSY; + } + } + } + return 0; } @@ -316,8 +343,30 @@ static int i801_check_post(struct i801_priv *priv, int status) dev_err(&priv->pci_dev->dev, "Transaction failed\n"); } if (status & SMBHSTSTS_DEV_ERR) { - result = -ENXIO; - dev_dbg(&priv->pci_dev->dev, "No response\n"); + /* + * This may be a PEC error, check and clear it. + * + * AUXSTS is handled differently from HSTSTS. + * For HSTSTS, i801_isr() or i801_wait_intr() + * has already cleared the error bits in hardware, + * and we are passed a copy of the original value + * in "status". + * For AUXSTS, the hardware register is left + * for us to handle here. + * This is asymmetric, slightly iffy, but safe, + * since all this code is serialized and the CRCE + * bit is harmless as long as it's cleared before + * the next operation. + */ + if ((priv->features & FEATURE_SMBUS_PEC) && + (inb_p(SMBAUXSTS(priv)) & SMBAUXSTS_CRCE)) { + outb_p(SMBAUXSTS_CRCE, SMBAUXSTS(priv)); + result = -EBADMSG; + dev_dbg(&priv->pci_dev->dev, "PEC error\n"); + } else { + result = -ENXIO; + dev_dbg(&priv->pci_dev->dev, "No response\n"); + } } if (status & SMBHSTSTS_BUS_ERR) { result = -EAGAIN;