Re: [PATCH V4 15/17] i3c: mipi-i3c-hci: Base timeouts on actual transfer start time

[Frank Li]

On Fri, May 15, 2026 at 07:26:19PM +0300, Adrian Hunter wrote:
> Transfer timeouts are currently measured from the point where a transfer
> list is queued to the controller.  This can cause transfers to time out
> before they have actually started, if earlier queued transfers consume
> the timeout interval.
>
> Fix this by recording when a transfer reaches the head of the queue and
> adjusting the timeout calculation to start from that point.  The existing
> low-overhead completion-based timeout mechanism is preserved, but care is
> taken to ensure the transfer start time is consistently recorded for both
> PIO and DMA paths.
>
> This prevents premature timeouts while retaining efficient timeout
> handling.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> ---

Reviewed-by: Frank Li <Frank.Li@xxxxxxx>

>
>
> Changes in V4:
>
> 	Rename start_time to start_jiffies
>
> Changes in V3:
>
> 	None
>
> Changes in V2:
> 	Do not flag the next transfer as started when there is an error
> 	which halts the controller
> 	Instead flag it started at the end of hci_dma_dequeue_xfer()
> 	Use hci_start_xfer() in pio.c
>
>
>  drivers/i3c/master/mipi-i3c-hci/core.c | 19 ++++++++++++++++++-
>  drivers/i3c/master/mipi-i3c-hci/dma.c  | 19 ++++++++++++++++++-
>  drivers/i3c/master/mipi-i3c-hci/hci.h  | 11 +++++++++++
>  drivers/i3c/master/mipi-i3c-hci/pio.c  |  1 +
>  4 files changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> index 69dcf5dad3a5..c6edbbedfdd7 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
> @@ -275,13 +275,30 @@ int i3c_hci_process_xfer(struct i3c_hci *hci, struct hci_xfer *xfer, int n)
>  {
>  	struct completion *done = xfer[n - 1].completion;
>  	unsigned long timeout = xfer[n - 1].timeout;
> +	unsigned long remaining_timeout = timeout;
> +	long time_taken;
> +	bool started;
>  	int ret;
>
> +	xfer[0].started = false;
> +
>  	ret = hci->io->queue_xfer(hci, xfer, n);
>  	if (ret)
>  		return ret;
>
> -	if (!wait_for_completion_timeout(done, timeout)) {
> +	while (!wait_for_completion_timeout(done, remaining_timeout)) {
> +		scoped_guard(spinlock_irqsave, &hci->lock) {
> +			started = xfer[0].started;
> +			time_taken = jiffies - xfer[0].start_jiffies;
> +		}
> +		/* Keep waiting if xfer has not started */
> +		if (!started)
> +			continue;
> +		/* Recalculate timeout based on actual start time */
> +		if (time_taken < timeout) {
> +			remaining_timeout = timeout - time_taken;
> +			continue;
> +		}
>  		if (hci->io->dequeue_xfer(hci, xfer, n)) {
>  			dev_err(&hci->master.dev, "%s: timeout error\n", __func__);
>  			return -ETIMEDOUT;
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index 0fd56bbb84ef..9a01c740760f 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -543,6 +543,9 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
>  		enqueue_ptr = (enqueue_ptr + 1) % rh->xfer_entries;
>  	}
>
> +	if (rh->xfer_space == rh->xfer_entries)
> +		hci_start_xfer(xfer_list);
> +
>  	rh->xfer_space -= n;
>
>  	op1_val &= ~RING_OP1_CR_ENQ_PTR;
> @@ -558,6 +561,7 @@ static void hci_dma_xfer_done(struct i3c_hci *hci, struct hci_rh_data *rh)
>  	u32 op1_val, op2_val, resp, *ring_resp;
>  	unsigned int tid, done_ptr = rh->done_ptr;
>  	unsigned int done_cnt = 0;
> +	bool start_next = false;
>  	struct hci_xfer *xfer;
>
>  	for (;;) {
> @@ -588,8 +592,14 @@ static void hci_dma_xfer_done(struct i3c_hci *hci, struct hci_rh_data *rh)
>  			xfer->response = resp;
>  			if (xfer == xfer->final_xfer || RESP_STATUS(resp))
>  				complete(xfer->final_xfer->completion);
> -			if (RESP_STATUS(resp))
> +			else
> +				hci_start_xfer(xfer);
> +			if (RESP_STATUS(resp)) {
>  				hci->enqueue_blocked = true;
> +				start_next = false;
> +			} else {
> +				start_next = true;
> +			}
>  		}
>
>  		done_ptr = (done_ptr + 1) % rh->xfer_entries;
> @@ -598,6 +608,10 @@ static void hci_dma_xfer_done(struct i3c_hci *hci, struct hci_rh_data *rh)
>  	}
>
>  	rh->xfer_space += done_cnt;
> +	if (start_next && rh->xfer_space < rh->xfer_entries) {
> +		xfer = rh->src_xfers[done_ptr];
> +		hci_start_xfer(xfer);
> +	}
>  	op1_val = rh_reg_read(RING_OPERATION1);
>  	op1_val &= ~RING_OP1_CR_SW_DEQ_PTR;
>  	op1_val |= FIELD_PREP(RING_OP1_CR_SW_DEQ_PTR, done_ptr);
> @@ -810,6 +824,9 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
>
>  	hci_dma_unblock_enqueue(hci);
>
> +	if (rh->xfer_space < rh->xfer_entries)
> +		hci_start_xfer(rh->src_xfers[rh->done_ptr]);
> +
>  	spin_unlock_irq(&hci->lock);
>
>  	wait_for_completion_timeout(&rh->op_done, HZ);
> diff --git a/drivers/i3c/master/mipi-i3c-hci/hci.h b/drivers/i3c/master/mipi-i3c-hci/hci.h
> index 4bf2c66c97b4..30297823ca85 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/hci.h
> +++ b/drivers/i3c/master/mipi-i3c-hci/hci.h
> @@ -11,6 +11,7 @@
>  #define HCI_H
>
>  #include <linux/io.h>
> +#include <linux/jiffies.h>
>
>  /* 32-bit word aware bit and mask macros */
>  #define W0_MASK(h, l)  GENMASK((h) - 0,  (l) - 0)
> @@ -88,11 +89,13 @@ struct hci_xfer {
>  	u32 cmd_desc[4];
>  	u32 response;
>  	bool rnw;
> +	bool started;
>  	void *data;
>  	unsigned int data_len;
>  	unsigned int cmd_tid;
>  	struct completion *completion;
>  	unsigned long timeout;
> +	unsigned long start_jiffies;
>  	union {
>  		struct {
>  			/* PIO specific */
> @@ -123,6 +126,14 @@ static inline void hci_free_xfer(struct hci_xfer *xfer, unsigned int n)
>  	kfree(xfer);
>  }
>
> +static inline void hci_start_xfer(struct hci_xfer *xfer)
> +{
> +	if (!xfer->started) {
> +		xfer->started = true;
> +		xfer->start_jiffies = jiffies;
> +	}
> +}
> +
>  /* This abstracts PIO vs DMA operations */
>  struct hci_io_ops {
>  	bool (*irq_handler)(struct i3c_hci *hci);
> diff --git a/drivers/i3c/master/mipi-i3c-hci/pio.c b/drivers/i3c/master/mipi-i3c-hci/pio.c
> index 8f48a81e65ab..6b8cc5f2b4d2 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/pio.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/pio.c
> @@ -605,6 +605,7 @@ static bool hci_pio_process_cmd(struct i3c_hci *hci, struct hci_pio_data *pio)
>  		 * Finally send the command.
>  		 */
>  		hci_pio_write_cmd(hci, pio->curr_xfer);
> +		hci_start_xfer(pio->curr_xfer);
>  		/*
>  		 * And move on.
>  		 */
> --
> 2.51.0
>