From patchwork Wed Mar 8 20:05:33 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Javier Martinez Canillas X-Patchwork-Id: 9612155 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 48ECE60414 for ; Thu, 9 Mar 2017 00:42:39 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2E369285C9 for ; Thu, 9 Mar 2017 00:42:39 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 112F72863E; Thu, 9 Mar 2017 00:42:39 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0618F285C9 for ; Thu, 9 Mar 2017 00:42:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754200AbdCIAmf (ORCPT ); Wed, 8 Mar 2017 19:42:35 -0500 Received: from ec2-52-27-115-49.us-west-2.compute.amazonaws.com ([52.27.115.49]:46374 "EHLO osg.samsung.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751929AbdCIAme (ORCPT ); Wed, 8 Mar 2017 19:42:34 -0500 Received: from localhost (localhost [127.0.0.1]) by osg.samsung.com (Postfix) with ESMTP id C8F26A0E75; Wed, 8 Mar 2017 20:06:04 +0000 (UTC) X-Virus-Scanned: amavisd-new at osg.samsung.com Received: from osg.samsung.com ([127.0.0.1]) by localhost (s-opensource.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id r-Q67EiIfN3K; Wed, 8 Mar 2017 20:06:03 +0000 (UTC) Received: from [192.168.0.37] (unknown [181.121.136.80]) by osg.samsung.com (Postfix) with ESMTPSA id 12D40A0C73; Wed, 8 Mar 2017 20:05:59 +0000 (UTC) Subject: Re: [PATCH v2] i2c: exynos5: fix arbitration lost handling To: Andrzej Hajda , Wolfram Sang , Krzysztof Kozlowski , linux-i2c@vger.kernel.org, linux-samsung-soc@vger.kernel.org References: <1487761474-11547-1-git-send-email-a.hajda@samsung.com> From: Javier Martinez Canillas Cc: Bartlomiej Zolnierkiewicz , Marek Szyprowski , Andi Shyti Message-ID: <2fb11816-85e7-14f6-7b61-5d3eba6096aa@osg.samsung.com> Date: Wed, 8 Mar 2017 17:05:33 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <1487761474-11547-1-git-send-email-a.hajda@samsung.com> Sender: linux-samsung-soc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-samsung-soc@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hello Andrzej, On 02/22/2017 08:04 AM, Andrzej Hajda wrote: > In case of arbitration lost adequate interrupt sometimes is not signaled. > As a result transfer timeouts and is not retried, as it should. To avoid > such cases code is added to check transaction status in case of every > interrupt. > > Signed-off-by: Andrzej Hajda > --- This patch causes regressions on Exynos5 boards (at least I noticed it in Exynos5800 Peach Pi board). I see transmission timeouts when accessing I2C device registers, i.e: $ cat /sys/class/rtc/rtc0/time [ 25.924594] exynos5-hsi2c 12e10000.i2c: rx timeout [ 65.028365] max77686-rtc max77802-rtc: Fail to read time reg(-22) cat: /sys/class/rtc/rtc0/time: Invalid argument Doing a partial revert of $SUBJECT (reading I2C_TRANS_STATUS register when it was before) "fixes" the problem [0]. I've read the Exynos5800 and Exynos5433 SoC manuals and couldn't figure out what's causing it. I first thought that the solution was to make I2C core to retry when the TRANSFER_DONE_AUTO field isn't set [1] but that only makes the issue less frequent, it still fails. I also tried to increment the retry count [2] but it also didn't help. Any ideas about what could be happening here? [0]: Best regards, diff --git a/drivers/i2c/busses/i2c-exynos5.c b/drivers/i2c/busses/i2c-exynos5.c index cbd93ce0661f..736a82472101 100644 --- a/drivers/i2c/busses/i2c-exynos5.c +++ b/drivers/i2c/busses/i2c-exynos5.c @@ -457,7 +457,6 @@ static irqreturn_t exynos5_i2c_irq(int irqno, void *dev_id) int_status = readl(i2c->regs + HSI2C_INT_STATUS); writel(int_status, i2c->regs + HSI2C_INT_STATUS); - trans_status = readl(i2c->regs + HSI2C_TRANS_STATUS); /* handle interrupt related to the transfer status */ if (i2c->variant->hw == HSI2C_EXYNOS7) { @@ -482,11 +481,13 @@ static irqreturn_t exynos5_i2c_irq(int irqno, void *dev_id) goto stop; } + trans_status = readl(i2c->regs + HSI2C_TRANS_STATUS); if ((trans_status & HSI2C_MASTER_ST_MASK) == HSI2C_MASTER_ST_LOSE) { i2c->state = -EAGAIN; goto stop; } } else if (int_status & HSI2C_INT_I2C) { + trans_status = readl(i2c->regs + HSI2C_TRANS_STATUS); if (trans_status & HSI2C_NO_DEV_ACK) { dev_dbg(i2c->dev, "No ACK from device\n"); i2c->state = -ENXIO; [1]: From 3c60d3a0356563b33b739d309a4f64c003d83053 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Mon, 6 Mar 2017 19:35:44 -0300 Subject: [PATCH] i2c: exynos5: Make transaction to retry if isn't successfully completed After commit 7999eecb7e56 ("i2c: exynos5: fix arbitration lost handling"), some I2C transactions are failing because the TRANSFER_DONE_AUTO field is not set in the I2C_TRANS_STATUS register so the i2c->status value is left to -EINVAL causing the i2c->msg_complete completion to never be signaled. For example, when reading the time of an I2C rtc on an Exynos5800 machine: $ cat /sys/class/rtc/rtc0/time [ 25.924594] exynos5-hsi2c 12e10000.i2c: rx timeout [ 65.028365] max77686-rtc max77802-rtc: Fail to read time reg(-22) cat: /sys/class/rtc/rtc0/time: Invalid argument To avoid this, set i2c->state to -EAGAIN if TRANSFER_DONE_AUTO is not set to allow the I2C core to retry if the transaction wasn't successfully completed. Signed-off-by: Javier Martinez Canillas --- drivers/i2c/busses/i2c-exynos5.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/i2c/busses/i2c-exynos5.c b/drivers/i2c/busses/i2c-exynos5.c index cbd93ce0661f..d294fb61520c 100644 --- a/drivers/i2c/busses/i2c-exynos5.c +++ b/drivers/i2c/busses/i2c-exynos5.c @@ -506,6 +506,10 @@ static irqreturn_t exynos5_i2c_irq(int irqno, void *dev_id) } else if (trans_status & HSI2C_TRANS_DONE) { i2c->trans_done = 1; i2c->state = 0; + } else { + dev_dbg(i2c->dev, "Transaction was not completed\n"); + i2c->state = -EAGAIN; + goto stop; } } -- 2.9.3 [2] diff --git a/drivers/i2c/busses/i2c-exynos5.c b/drivers/i2c/busses/i2c-exynos5.c index d294fb61520c..2b0ee9040188 100644 --- a/drivers/i2c/busses/i2c-exynos5.c +++ b/drivers/i2c/busses/i2c-exynos5.c @@ -774,7 +774,7 @@ static int exynos5_i2c_probe(struct platform_device *pdev) strlcpy(i2c->adap.name, "exynos5-i2c", sizeof(i2c->adap.name)); i2c->adap.owner = THIS_MODULE; i2c->adap.algo = &exynos5_i2c_algorithm; - i2c->adap.retries = 3; + i2c->adap.retries = 5; i2c->dev = &pdev->dev; i2c->clk = devm_clk_get(&pdev->dev, "hsi2c");