From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrzej Hajda Subject: Re: [PATCH] i2c: exynos5: rework HSI2C_MASTER_ST_LOSE state handling Date: Mon, 12 Feb 2018 09:04:46 +0100 Message-ID: References: <20180126120950.9934-1-a.hajda@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Return-path: Received: from mailout2.w1.samsung.com ([210.118.77.12]:57542 "EHLO mailout2.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753986AbeBLIEx (ORCPT ); Mon, 12 Feb 2018 03:04:53 -0500 Received: from eucas1p2.samsung.com (unknown [182.198.249.207]) by mailout2.w1.samsung.com (KnoxPortal) with ESMTP id 20180212080451euoutp023fce366ad81774cafec4218af595e166~ShWSM9X0I2095220952euoutp02J for ; Mon, 12 Feb 2018 08:04:51 +0000 (GMT) In-Reply-To: <20180126120950.9934-1-a.hajda@samsung.com> Content-Language: en-US Sender: linux-i2c-owner@vger.kernel.org List-Id: linux-i2c@vger.kernel.org To: Wolfram Sang , Andi Shyti Cc: Bartlomiej Zolnierkiewicz , Marek Szyprowski , "open list:I2C SUBSYSTEM" , "moderated list:ARM/SAMSUNG EXYNOS ARM ARCHITECTURES" On 26.01.2018 13:09, Andrzej Hajda wrote: > HSI2C_MASTER_ST_LOSE state is not documented properly, extensive tests > show that hardware is usually able to recover from this state without > interrupting the transfer. Moreover documentation says that > such state can be caused by slave clock stretching, and should not be > treated as an error during transaction. The only place it indicates > an error is just before starting transaction. In such case bus recovery > procedure should be performed - master should pulse SCL line nine times > and then send STOP condition, it can be repeated until SDA goes high. > The procedure can be performed using manual commands HSI2C_CMD_READ_DATA > and HSI2C_CMD_SEND_STOP. > > Signed-off-by: Andrzej Hajda > --- > Hi Wolfram, > > This patch integrates and invalidates my previous patches [1][2]. > I hope inline comments are sufficient. Gentle ping. -- Regards Andrzej > > [1]: i2c: exynos5: implement bus recovery functionality > [2]: i2c: exynos5: do not check TRANS_STATUS in case of Exynos7 variant > > Regards > Andrzej > > drivers/i2c/busses/i2c-exynos5.c | 67 ++++++++++++++++++++++++++++++++++++---- > 1 file changed, 61 insertions(+), 6 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-exynos5.c b/drivers/i2c/busses/i2c-exynos5.c > index b02428498f6d..8dcc36835a1a 100644 > --- a/drivers/i2c/busses/i2c-exynos5.c > +++ b/drivers/i2c/busses/i2c-exynos5.c > @@ -128,6 +128,10 @@ > #define HSI2C_TIMEOUT_EN (1u << 31) > #define HSI2C_TIMEOUT_MASK 0xff > > +/* I2C_MANUAL_CMD register bits */ > +#define HSI2C_CMD_READ_DATA (1u << 4) > +#define HSI2C_CMD_SEND_STOP (1u << 2) > + > /* I2C_TRANS_STATUS register bits */ > #define HSI2C_MASTER_BUSY (1u << 17) > #define HSI2C_SLAVE_BUSY (1u << 16) > @@ -441,12 +445,6 @@ static irqreturn_t exynos5_i2c_irq(int irqno, void *dev_id) > i2c->state = -ETIMEDOUT; > 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) { > @@ -544,6 +542,61 @@ static int exynos5_i2c_wait_bus_idle(struct exynos5_i2c *i2c) > return -EBUSY; > } > > +static void exynos5_i2c_bus_recover(struct exynos5_i2c *i2c) > +{ > + u32 val; > + > + dev_dbg(i2c->dev, "%s: start\n", __func__); > + > + val = readl(i2c->regs + HSI2C_CTL) | HSI2C_RXCHON; > + writel(val, i2c->regs + HSI2C_CTL); > + val = readl(i2c->regs + HSI2C_CONF) & ~HSI2C_AUTO_MODE; > + writel(val, i2c->regs + HSI2C_CONF); > + > + /* > + * Specification says master should send nine clock pulses. It can be > + * emulated by sending manual read command (nine pulses for read eight > + * bits + one pulse for NACK). > + */ > + writel(HSI2C_CMD_READ_DATA, i2c->regs + HSI2C_MANUAL_CMD); > + exynos5_i2c_wait_bus_idle(i2c); > + writel(HSI2C_CMD_SEND_STOP, i2c->regs + HSI2C_MANUAL_CMD); > + exynos5_i2c_wait_bus_idle(i2c); > + > + val = readl(i2c->regs + HSI2C_CTL) & ~HSI2C_RXCHON; > + writel(val, i2c->regs + HSI2C_CTL); > + val = readl(i2c->regs + HSI2C_CONF) | HSI2C_AUTO_MODE; > + writel(val, i2c->regs + HSI2C_CONF); > + > + dev_dbg(i2c->dev, "%s: end\n", __func__); > +} > + > +static void exynos5_i2c_bus_check(struct exynos5_i2c *i2c) > +{ > + unsigned long timeout; > + > + if (i2c->variant->hw != HSI2C_EXYNOS7) > + return; > + > + /* > + * HSI2C_MASTER_ST_LOSE state in EXYNOS7 variant before transaction > + * indicates that bus is stuck (SDA is low). In such case bus recovery > + * can be performed. > + */ > + timeout = jiffies + msecs_to_jiffies(100); > + for (;;) { > + u32 st = readl(i2c->regs + HSI2C_TRANS_STATUS); > + > + if ((st & HSI2C_MASTER_ST_MASK) != HSI2C_MASTER_ST_LOSE) > + return; > + > + if (time_is_before_jiffies(timeout)) > + return; > + > + exynos5_i2c_bus_recover(i2c); > + } > +} > + > /* > * exynos5_i2c_message_start: Configures the bus and starts the xfer > * i2c: struct exynos5_i2c pointer for the current bus > @@ -598,6 +651,8 @@ static void exynos5_i2c_message_start(struct exynos5_i2c *i2c, int stop) > writel(fifo_ctl, i2c->regs + HSI2C_FIFO_CTL); > writel(i2c_ctl, i2c->regs + HSI2C_CTL); > > + exynos5_i2c_bus_check(i2c); > + > /* > * Enable interrupts before starting the transfer so that we don't > * miss any INT_I2C interrupts.