Re: [PATCH] i3c: master: renesas: reset xfer status before each wait

Hi, Pengpeng,

On 6/25/26 06:03, Pengpeng Hou wrote:
> renesas_i3c_wait_xfer() reinitializes the completion before queuing a
> transfer, but it leaves xfer->ret unchanged. 

In the current code the xfer->ret is initialized with -ETIMEDOUT when it is 
allocated in renesas_i3c_alloc_xfer().

> After one transfer completes
> successfully, a later timeout can therefore reuse the old zero status.

The patch here took this into account: 
https://lore.kernel.org/all/[email protected]/

Please check the places where renesas_i3c_irqs_mask_and_clear() is called.

> 
> Reset xfer->ret to -ETIMEDOUT before each wait, return the transfer
> status from the wait helper, and propagate it from DAA, CCC and private
> I3C transfers.
> 
> Signed-off-by: Pengpeng Hou <[email protected]>
> ---
>   drivers/i3c/master/renesas-i3c.c | 19 ++++++++++++-------
>   1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> index f39c449922ca..7f123cd72cdf 100644
> --- a/drivers/i3c/master/renesas-i3c.c
> +++ b/drivers/i3c/master/renesas-i3c.c
> @@ -433,15 +433,18 @@ static void renesas_i3c_enqueue_xfer(struct renesas_i3c *i3c, struct renesas_i3c
>   	}
>   }
>   
> -static void renesas_i3c_wait_xfer(struct renesas_i3c *i3c, struct renesas_i3c_xfer *xfer)
> +static int renesas_i3c_wait_xfer(struct renesas_i3c *i3c, struct renesas_i3c_xfer *xfer)
>   {
>   	unsigned long time_left;
>   
> +	xfer->ret = -ETIMEDOUT;
>   	renesas_i3c_enqueue_xfer(i3c, xfer);
>   
>   	time_left = wait_for_completion_timeout(&xfer->comp, msecs_to_jiffies(1000));
>   	if (!time_left)
>   		renesas_i3c_dequeue_xfer(i3c, xfer);
> +
> +	return xfer->ret;

In some DAA cases this might be a negative number but we could still consider it OK.

>   }
>   
>   static void renesas_i3c_set_prts(struct renesas_i3c *i3c, u32 val)
> @@ -687,7 +690,9 @@ static int renesas_i3c_daa(struct i3c_master_controller *m)
>   		    NCMDQP_CMD(I3C_CCC_ENTDAA) | NCMDQP_DEV_INDEX(ret) |
>   		    NCMDQP_DEV_COUNT(i3c->maxdevs - ret) | NCMDQP_TOC;
>   
> -	renesas_i3c_wait_xfer(i3c, xfer);
> +	ret = renesas_i3c_wait_xfer(i3c, xfer);
> +	if (ret)
> +		return ret;

Have you tested this? My previous experiments around this code tell me this 
might not be OK for all the cases.

Thank you,
Claudiu

>   
>   	newdevs = GENMASK(i3c->maxdevs - cmd->rx_count - 1, 0);
>   	newdevs &= ~olddevs;
> @@ -800,9 +805,7 @@ static int renesas_i3c_send_ccc_cmd(struct i3c_master_controller *m,
>   		}
>   	}
>   
> -	renesas_i3c_wait_xfer(i3c, xfer);
> -
> -	ret = xfer->ret;
> +	ret = renesas_i3c_wait_xfer(i3c, xfer);
>   	if (ret)
>   		ccc->err = I3C_ERROR_M2;
>   
> @@ -815,7 +818,7 @@ static int renesas_i3c_i3c_xfers(struct i3c_dev_desc *dev, struct i3c_xfer *i3c_
>   	struct i3c_master_controller *m = i3c_dev_get_master(dev);
>   	struct renesas_i3c *i3c = to_renesas_i3c(m);
>   	struct renesas_i3c_i2c_dev_data *data = i3c_dev_get_master_data(dev);
> -	int i;
> +	int i, ret;
>   
>   	/* Enable I3C bus. */
>   	renesas_i3c_bus_enable(m, true);
> @@ -854,7 +857,9 @@ static int renesas_i3c_i3c_xfers(struct i3c_dev_desc *dev, struct i3c_xfer *i3c_
>   				renesas_set_bit(i3c->regs, NTIE, NTIE_TDBEIE0);
>   		}
>   
> -		renesas_i3c_wait_xfer(i3c, xfer);
> +		ret = renesas_i3c_wait_xfer(i3c, xfer);
> +		if (ret)
> +			return ret;
>   	}
>   
>   	return 0;


-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c