Re: [PATCH 2/3] i3c: mipi-i3c-hci: Add PIO queue management support for HCI v1.2

On Thu, Jul 09, 2026 at 03:17:43PM +0800, Jian-Ming Liao wrote:
> - Support explicit enablement and starting of PIO queues as
>   required by HCI v1.2.
> - Handle alternate PIO queue sizes via ALT_QUEUE_SIZE register.
> - Implement explicit PIO queue stopping/disabling and restart
>   logic after errors.

Remove -

Support explicit enablement and starting of PIO queues as required by
HCI v1.2.

Handle alternate PIO queue sizes via ALT_QUEUE_SIZE register.

Implement explicit PIO queue stopping/disabling and restart logic after
errors.

if you prefer -, need

Some sentense ...
  - abc...

>
> Co-developed-by: Patrick Yen <[email protected]>
> Signed-off-by: Patrick Yen <[email protected]>
> Signed-off-by: Jian-Ming Liao <[email protected]>
> ---
>  drivers/i3c/master/mipi-i3c-hci/core.c |  1 +
>  drivers/i3c/master/mipi-i3c-hci/pio.c  | 72 ++++++++++++++++++++++----
>  2 files changed, 63 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> index e80aa1f5722e..dccc974ef15a 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
> @@ -1031,6 +1031,7 @@ static int i3c_hci_init(struct i3c_hci *hci)
>  	switch (regval & ~0xf) {
>  	case 0x100:	/* version 1.0 */
>  	case 0x110:	/* version 1.1 */
> +	case 0x120:	/* version 1.2 */
>  	case 0x200:	/* version 2.0 */
>  		break;
>  	default:
> diff --git a/drivers/i3c/master/mipi-i3c-hci/pio.c b/drivers/i3c/master/mipi-i3c-hci/pio.c
> index 7870b2c7888f..c05fb3666378 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/pio.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/pio.c
> @@ -45,6 +45,11 @@
>  #define IBI_STATUS_SIZE			GENMASK(15, 8)
>  #define CR_QUEUE_SIZE			GENMASK(7, 0)
>
> +#define PIO_ALT_QUEUE_SIZE      0x1C
> +#define EXT_IBI_QUEUE_EN        BIT(28)
> +#define ALT_RESP_QUEUE_EN       BIT(24)
> +#define ALT_RESP_QUEUE_SIZE     GENMASK(7, 0)
> +

Number start pos align to prevous GENMASK(7, 0) of CR_QUEUE_SIZE

>  #define PIO_INTR_STATUS			0x20
>  #define PIO_INTR_STATUS_ENABLE		0x24
>  #define PIO_INTR_SIGNAL_ENABLE		0x28
> @@ -72,6 +77,11 @@
>  #define STAT_RX_THLD			BIT(1)
>  #define STAT_TX_THLD			BIT(0)
>
> +#define PIO_CONTROL			0x30
> +#define PIO_CONTROL_ABORT		BIT(2)
> +#define PIO_CONTROL_RS			BIT(1)
> +#define PIO_CONTROL_ENABLE		BIT(0)
> +
>  #define PIO_QUEUE_CUR_STATUS		0x38
>  #define CUR_IBI_Q_LEVEL			GENMASK(28, 20)
>  #define CUR_RESP_Q_LEVEL		GENMASK(18, 10)
> @@ -138,11 +148,44 @@ static void __hci_pio_init(struct i3c_hci *hci, u32 *size_val_ptr)
>  {
>  	u32 val, size_val, rx_thresh, tx_thresh, ibi_val;
>  	struct hci_pio_data *pio = hci->io_data;
> +	u32 cmd_sz, resp_sz;
>
>  	size_val = pio_reg_read(QUEUE_SIZE);
>  	if (size_val_ptr)
>  		*size_val_ptr = size_val;
>
> +	cmd_sz = FIELD_GET(CR_QUEUE_SIZE, size_val);
> +	resp_sz = cmd_sz;
> +	ibi_val = FIELD_GET(IBI_STATUS_SIZE, size_val);
> +
> +	/* MIPI I3C HCI v1.2 requires explicitly enabling and starting PIO queues
> +	 * and supports alternate queue sizes.
> +	 */
> +	if (hci->version_major == 1 && hci->version_minor >= 2) {
> +		u32 ctl_val = pio_reg_read(CONTROL);
> +		u32 alt_val = pio_reg_read(ALT_QUEUE_SIZE);
> +
> +		if (!(ctl_val & PIO_CONTROL_ENABLE)) {
> +			ctl_val |= PIO_CONTROL_ENABLE;
> +			pio_reg_write(CONTROL, ctl_val);
> +		}
> +		pio_reg_write(CONTROL, ctl_val | PIO_CONTROL_RS);
> +
> +		/* Adjust actual FIFO sizes based on v1.2 ALT_QUEUE_SIZE */
> +		if (alt_val & ALT_RESP_QUEUE_EN)
> +			resp_sz = FIELD_GET(ALT_RESP_QUEUE_SIZE, alt_val);
> +		if (alt_val & EXT_IBI_QUEUE_EN)
> +			ibi_val *= 8;
> +	}
> +
> +	dev_dbg(&hci->master.dev, "CMD FIFO = %u, RESP FIFO = %u entries\n",
> +		cmd_sz, resp_sz);
> +	dev_dbg(&hci->master.dev, "IBI FIFO = %u bytes\n", 4 * ibi_val);
> +	dev_dbg(&hci->master.dev, "RX data FIFO = %d bytes\n",
> +		4 * (2 << FIELD_GET(RX_DATA_BUFFER_SIZE, size_val)));
> +	dev_dbg(&hci->master.dev, "TX data FIFO = %d bytes\n",
> +		4 * (2 << FIELD_GET(TX_DATA_BUFFER_SIZE, size_val)));
> +
>  	/*
>  	 * Let's initialize data thresholds to half of the actual FIFO size.
>  	 * The start thresholds aren't used (set to 0) as the FIFO is always
> @@ -172,7 +215,7 @@ static void __hci_pio_init(struct i3c_hci *hci, u32 *size_val_ptr)
>  	 * or one available response or IBI. For IBI data let's use half the
>  	 * IBI queue size within allowed bounds.
>  	 */
> -	ibi_val = FIELD_GET(IBI_STATUS_SIZE, size_val);
> +
>  	pio->max_ibi_thresh = clamp_val(ibi_val/2, 1, 63);
>  	val = FIELD_PREP(QUEUE_IBI_STATUS_THLD, 1) |
>  	      FIELD_PREP(QUEUE_IBI_DATA_THLD, pio->max_ibi_thresh) |
> @@ -219,15 +262,6 @@ static int hci_pio_init(struct i3c_hci *hci)
>
>  	__hci_pio_init(hci, &size_val);
>
> -	dev_dbg(&hci->master.dev, "CMD/RESP FIFO = %ld entries\n",
> -		FIELD_GET(CR_QUEUE_SIZE, size_val));
> -	dev_dbg(&hci->master.dev, "IBI FIFO = %ld bytes\n",
> -		4 * FIELD_GET(IBI_STATUS_SIZE, size_val));
> -	dev_dbg(&hci->master.dev, "RX data FIFO = %d bytes\n",
> -		4 * (2 << FIELD_GET(RX_DATA_BUFFER_SIZE, size_val)));
> -	dev_dbg(&hci->master.dev, "TX data FIFO = %d bytes\n",
> -		4 * (2 << FIELD_GET(TX_DATA_BUFFER_SIZE, size_val)));
> -

why need move this part?

>  	return 0;
>  }
>
> @@ -246,6 +280,12 @@ static void hci_pio_cleanup(struct i3c_hci *hci)
>  		BUG_ON(pio->curr_rx);
>  		BUG_ON(pio->curr_tx);
>  		BUG_ON(pio->curr_resp);
> +		/* MIPI I3C HCI v1.2 requires explicitly stopping and disabling PIO queues */
> +		if (hci->version_major == 1 && hci->version_minor >= 2)
> +			pio_reg_write(CONTROL, 0x0);
> +
> +		kfree(pio);

why need kfree() here? if fix existing problem, use seperate patch.

> +		hci->io_data = NULL;
>  	}
>  }
>
> @@ -766,6 +806,18 @@ static void hci_pio_err(struct i3c_hci *hci, struct hci_pio_data *pio,
>  		hci_pio_dequeue_xfer_common(hci, pio, pio->curr_tx, 1);
>  	/* then reset the hardware */
>  	mipi_i3c_hci_pio_reset(hci);
> +
> +	/* MIPI I3C HCI v1.2 requires explicitly restarting PIO queues after error/abort */
> +	if (hci->version_major == 1 && hci->version_minor >= 2) {

Can you use helper macro this check, many place use similar check

Frank

> +		u32 ctl_val = pio_reg_read(CONTROL);
> +
> +		if (!(ctl_val & PIO_CONTROL_ENABLE)) {
> +			ctl_val |= PIO_CONTROL_ENABLE;
> +			pio_reg_write(CONTROL, ctl_val);
> +		}
> +		pio_reg_write(CONTROL, ctl_val | PIO_CONTROL_RS);
> +	}
> +
>  	mipi_i3c_hci_resume(hci);
>
>  	dev_dbg(&hci->master.dev, "status=%#x/%#x",
> --
> 2.43.0
>
>
> --
> linux-i3c mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-i3c

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