Re: [PATCH V3 10/16] i3c: mipi-i3c-hci: Add DMA ring abort quirk for Intel controllers

[Frank Li]

On Mon, May 04, 2026 at 02:33:46PM +0300, Adrian Hunter wrote:
> DMA rings can be aborted either per-ring via RING_CONTROL or globally
> via HC_CONTROL_ABORT. The driver currently relies on the per-ring
> mechanism.
>
> Some Intel I3C HCI controllers require HC_CONTROL_ABORT to be asserted
> before a DMA ring abort is effective.  This behavior is non-standard.
> Introduce a controller quirk to select the required abort method and
> enable it for Intel LPSS I3C controllers.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> ---
>
>
> Changes in V2 and V3:
>
> 	None
>
>
>  drivers/i3c/master/mipi-i3c-hci/core.c | 18 +++++++++++++++--
>  drivers/i3c/master/mipi-i3c-hci/dma.c  | 27 +++++++++++++++++++++++---
>  drivers/i3c/master/mipi-i3c-hci/hci.h  |  2 ++
>  3 files changed, 42 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> index 770235ad6b25..8274c84b16be 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
> @@ -231,7 +231,20 @@ static void i3c_hci_bus_cleanup(struct i3c_master_controller *m)
>
>  void mipi_i3c_hci_resume(struct i3c_hci *hci)
>  {
> -	reg_set(HC_CONTROL, HC_CONTROL_RESUME);
> +	u32 reg = reg_read(HC_CONTROL);
> +
> +	reg |= HC_CONTROL_RESUME;
> +	reg &= ~HC_CONTROL_ABORT;
> +	reg_write(HC_CONTROL, reg);
> +}
> +
> +void mipi_i3c_hci_abort(struct i3c_hci *hci)
> +{
> +	u32 reg = reg_read(HC_CONTROL);
> +
> +	reg &= ~HC_CONTROL_RESUME; /* Do not set resume */
> +	reg |= HC_CONTROL_ABORT;
> +	reg_write(HC_CONTROL, reg);
>  }
>
>  /* located here rather than pio.c because needed bits are in core reg space */
> @@ -1053,7 +1066,8 @@ static const struct platform_device_id i3c_hci_driver_ids[] = {
>  	{ .name = "intel-lpss-i3c", HCI_QUIRK_RPM_ALLOWED |
>  				    HCI_QUIRK_RPM_IBI_ALLOWED |
>  				    HCI_QUIRK_RPM_PARENT_MANAGED |
> -				    HCI_QUIRK_DMA_ABORT_REQUIRES_PIO_RESET },
> +				    HCI_QUIRK_DMA_ABORT_REQUIRES_PIO_RESET |
> +				    HCI_QUIRK_DMA_REQUIRES_HC_ABORT },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(platform, i3c_hci_driver_ids);
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index 699c6d523eed..41bbd912df7f 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -597,6 +597,29 @@ static void hci_dma_xfer_done(struct i3c_hci *hci, struct hci_rh_data *rh)
>  	rh_reg_write(RING_OPERATION1, op1_val);
>  }
>
> +static bool hci_dma_requires_hc_abort_quirk(struct i3c_hci *hci, struct hci_rh_data *rh)
> +{
> +	if (!(hci->quirks & HCI_QUIRK_DMA_REQUIRES_HC_ABORT))
> +		return false;
> +
> +	reinit_completion(&rh->op_done);
> +	mipi_i3c_hci_abort(hci);
> +	wait_for_completion_timeout(&rh->op_done, HZ);
> +	rh_reg_write(RING_CONTROL, rh_reg_read(RING_CONTROL) | RING_CTRL_ABORT);
> +
> +	return true;
> +}
> +
> +static void hci_dma_abort(struct i3c_hci *hci, struct hci_rh_data *rh)
> +{
> +	if (hci_dma_requires_hc_abort_quirk(hci, rh))
> +		return;

Move check logic here to make overall logic simple

	if (hci->quirks & HCI_QUIRK_DMA_REQUIRES_HC_ABORT) {
		hci_dma_requires_hc_abort_quirk()
		return;
	}

> +
> +	reinit_completion(&rh->op_done);
> +	rh_reg_write(RING_CONTROL, rh_reg_read(RING_CONTROL) | RING_CTRL_ABORT);
> +	wait_for_completion_timeout(&rh->op_done, HZ);

move these codes reorg into new patch.

Frank
> +}
> +
>  static void hci_dma_abort_requires_pio_reset_quirk(struct i3c_hci *hci, struct hci_rh_data *rh)
>  {
>  	if ((hci->quirks & HCI_QUIRK_DMA_ABORT_REQUIRES_PIO_RESET) &&
> @@ -630,9 +653,7 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
>  		hci->enqueue_blocked = true;
>  		spin_unlock_irq(&hci->lock);
>  		/* stop the ring */
> -		reinit_completion(&rh->op_done);
> -		rh_reg_write(RING_CONTROL, rh_reg_read(RING_CONTROL) | RING_CTRL_ABORT);
> -		wait_for_completion_timeout(&rh->op_done, HZ);
> +		hci_dma_abort(hci, rh);
>  		spin_lock_irq(&hci->lock);
>  		ring_status = rh_reg_read(RING_STATUS);
>  		if (ring_status & RING_STATUS_RUNNING) {
> diff --git a/drivers/i3c/master/mipi-i3c-hci/hci.h b/drivers/i3c/master/mipi-i3c-hci/hci.h
> index 01237b12d32e..97c31a315a6e 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/hci.h
> +++ b/drivers/i3c/master/mipi-i3c-hci/hci.h
> @@ -157,9 +157,11 @@ struct i3c_hci_dev_data {
>  #define HCI_QUIRK_RPM_IBI_ALLOWED	BIT(6)  /* IBI and Hot-Join allowed while runtime suspended */
>  #define HCI_QUIRK_RPM_PARENT_MANAGED	BIT(7)  /* Runtime PM managed by parent device */
>  #define HCI_QUIRK_DMA_ABORT_REQUIRES_PIO_RESET	BIT(8)  /* Do PIO queue SW resets after DMA abort */
> +#define HCI_QUIRK_DMA_REQUIRES_HC_ABORT		BIT(9)  /* Use HC_CONTROL ABORT to abort DMA */
>
>  /* global functions */
>  void mipi_i3c_hci_resume(struct i3c_hci *hci);
> +void mipi_i3c_hci_abort(struct i3c_hci *hci);
>  void mipi_i3c_hci_pio_reset(struct i3c_hci *hci);
>  void mipi_i3c_hci_pio_reset_all_queues(struct i3c_hci *hci);
>  void mipi_i3c_hci_dct_index_reset(struct i3c_hci *hci);
> --
> 2.51.0
>