| Message ID | 20231025121725.46028-3-m.szyprowski@samsung.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add atomic transfers to s3c24xx i2c driver | expand |
> -----Original Message----- > From: Marek Szyprowski <m.szyprowski@samsung.com> > Sent: Wednesday, October 25, 2023 9:17 PM > To: linux-samsung-soc@vger.kernel.org; linux-i2c@vger.kernel.org > Cc: Marek Szyprowski <m.szyprowski@samsung.com>; Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org>; Alim Akhtar <alim.akhtar@samsung.com>; > Andi Shyti <andi.shyti@kernel.org>; Wolfram Sang <wsa@kernel.org> > Subject: [PATCH v2 2/3] i2c: s3c24xx: fix transferring more than one > message in polling mode > > To properly handle ACK on the bus when transferring more than one > message in polling mode, move the polling handling loop from > s3c24xx_i2c_message_start() to s3c24xx_i2c_doxfer(). This way > i2c_s3c_irq_nextbyte() is always executed till the end, properly > acknowledging the IRQ bits and no recursive calls to > i2c_s3c_irq_nextbyte() are made. > > While touching this, also fix finishing transfers in polling mode by > using common code path and always waiting for the bus to become idle > and disabled. > > Fixes: 117053f77a5a ("i2c: s3c2410: Add polling mode support") > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> Reviewed-by: Chanho Park <chanho61.park@samsung.com> > --- > drivers/i2c/busses/i2c-s3c2410.c | 25 ++++++++++--------------- > 1 file changed, 10 insertions(+), 15 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c- > s3c2410.c > index f9dcb1112a61..8da85cb42980 100644 > --- a/drivers/i2c/busses/i2c-s3c2410.c > +++ b/drivers/i2c/busses/i2c-s3c2410.c > @@ -275,16 +275,6 @@ static void s3c24xx_i2c_message_start(struct > s3c24xx_i2c *i2c, > > stat |= S3C2410_IICSTAT_START; > writel(stat, i2c->regs + S3C2410_IICSTAT); > - > - if (i2c->quirks & QUIRK_POLL) { > - while ((i2c->msg_num != 0) && is_ack(i2c)) { > - i2c_s3c_irq_nextbyte(i2c, stat); > - stat = readl(i2c->regs + S3C2410_IICSTAT); > - > - if (stat & S3C2410_IICSTAT_ARBITR) > - dev_err(i2c->dev, "deal with arbitration > loss\n"); > - } > - } > } > > static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) > @@ -691,7 +681,7 @@ static void s3c24xx_i2c_wait_idle(struct s3c24xx_i2c > *i2c) > static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, > struct i2c_msg *msgs, int num) > { > - unsigned long timeout; > + unsigned long timeout = 0; > int ret; > > ret = s3c24xx_i2c_set_master(i2c); > @@ -711,16 +701,21 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c > *i2c, > s3c24xx_i2c_message_start(i2c, msgs); > > if (i2c->quirks & QUIRK_POLL) { > - ret = i2c->msg_idx; > + while ((i2c->msg_num != 0) && is_ack(i2c)) { > + unsigned long stat = readl(i2c->regs + > S3C2410_IICSTAT); > > - if (ret != num) > - dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); > + i2c_s3c_irq_nextbyte(i2c, stat); > > - goto out; > + stat = readl(i2c->regs + S3C2410_IICSTAT); > + if (stat & S3C2410_IICSTAT_ARBITR) > + dev_err(i2c->dev, "deal with arbitration > loss\n"); > + } > + goto skip_waiting; > } > > timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5); > > + skip_waiting: > ret = i2c->msg_idx; > > /* > -- > 2.34.1
diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c index f9dcb1112a61..8da85cb42980 100644 --- a/drivers/i2c/busses/i2c-s3c2410.c +++ b/drivers/i2c/busses/i2c-s3c2410.c @@ -275,16 +275,6 @@ static void s3c24xx_i2c_message_start(struct s3c24xx_i2c *i2c, stat |= S3C2410_IICSTAT_START; writel(stat, i2c->regs + S3C2410_IICSTAT); - - if (i2c->quirks & QUIRK_POLL) { - while ((i2c->msg_num != 0) && is_ack(i2c)) { - i2c_s3c_irq_nextbyte(i2c, stat); - stat = readl(i2c->regs + S3C2410_IICSTAT); - - if (stat & S3C2410_IICSTAT_ARBITR) - dev_err(i2c->dev, "deal with arbitration loss\n"); - } - } } static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) @@ -691,7 +681,7 @@ static void s3c24xx_i2c_wait_idle(struct s3c24xx_i2c *i2c) static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, struct i2c_msg *msgs, int num) { - unsigned long timeout; + unsigned long timeout = 0; int ret; ret = s3c24xx_i2c_set_master(i2c); @@ -711,16 +701,21 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, s3c24xx_i2c_message_start(i2c, msgs); if (i2c->quirks & QUIRK_POLL) { - ret = i2c->msg_idx; + while ((i2c->msg_num != 0) && is_ack(i2c)) { + unsigned long stat = readl(i2c->regs + S3C2410_IICSTAT); - if (ret != num) - dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret); + i2c_s3c_irq_nextbyte(i2c, stat); - goto out; + stat = readl(i2c->regs + S3C2410_IICSTAT); + if (stat & S3C2410_IICSTAT_ARBITR) + dev_err(i2c->dev, "deal with arbitration loss\n"); + } + goto skip_waiting; } timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5); + skip_waiting: ret = i2c->msg_idx; /*
To properly handle ACK on the bus when transferring more than one message in polling mode, move the polling handling loop from s3c24xx_i2c_message_start() to s3c24xx_i2c_doxfer(). This way i2c_s3c_irq_nextbyte() is always executed till the end, properly acknowledging the IRQ bits and no recursive calls to i2c_s3c_irq_nextbyte() are made. While touching this, also fix finishing transfers in polling mode by using common code path and always waiting for the bus to become idle and disabled. Fixes: 117053f77a5a ("i2c: s3c2410: Add polling mode support") Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- drivers/i2c/busses/i2c-s3c2410.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-)