RE: [PATCH v2 2/3] i2c: s3c24xx: fix transferring more than one message in polling mode

["Chanho Park" <chanho61.park@samsung.com>]

> -----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