On Wed, Oct 25, 2023 at 02:17:24PM +0200, Marek Szyprowski wrote:
> 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@xxxxxxxxxxx>
> ---
> 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;
please use else.
if (i2c->quirks & QUIRK_POLL)
while ((i2c->msg_num != 0) && is_ack(i2c)) {
...
}
else
timeout = wait_even...
ret = i2c->msg_idx;
> }
>
> timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5);
>
> + skip_waiting:
> ret = i2c->msg_idx;
>
> /*
> --
> 2.34.1
>
