Re: [PATCH 1/3] i3c: mipi-i3c-hci: Make bounce buffer code generic to all DMA transfers

[Frank Li <Frank.li@nxp.com>]

On Wed, Jun 04, 2025 at 03:55:11PM +0300, Jarkko Nikula wrote:
> Move DMA bounce buffer code for I3C private transfers to be generic for
> all DMA transfers, and round up the receive bounce buffer size to a
> multiple of DWORDs.
>
> It was observed that when the device DMA is IOMMU mapped and the receive
> length is not a multiple of DWORDs, the last DWORD is padded with stale
> data from the RX FIFO, corrupting 1-3 bytes beyond the expected data.
>
> A similar issue, though less severe, occurs when an I3C target returns
> less data than requested. In this case, the padding does not exceed the
> requested number of bytes, assuming the device DMA is not IOMMU mapped.
>
> Therefore, all I3C private transfer, CCC command payload and I2C
> transfer receive buffers must be properly sized for the DMA being IOMMU
> mapped. Even if those buffers are already DMA safe, their size may not
> be, and I don't have a clear idea how to guarantee this other than
> using a local bounce buffer.
>
> To prepare for the device DMA being IOMMU mapped and to address the
> above issue, implement a local, properly sized bounce buffer for all
> DMA transfers. For now, allocate it only when the buffer is in the
> vmalloc() area to avoid unnecessary copying with CCC commands and
> DMA-safe I2C transfers.
>
> Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> ---
>  drivers/i3c/master/mipi-i3c-hci/core.c | 34 -------------------
>  drivers/i3c/master/mipi-i3c-hci/dma.c  | 47 +++++++++++++++++++++++++-
>  2 files changed, 46 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> index bc4538694540..24c5e7d5b439 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
> @@ -272,34 +272,6 @@ static int i3c_hci_daa(struct i3c_master_controller *m)
>  	return hci->cmd->perform_daa(hci);
>  }
>
...
>  }
>
> +static void *hci_dma_alloc_safe_xfer_buf(struct i3c_hci *hci,
> +					 struct hci_xfer *xfer)
> +{
> +	if (!is_vmalloc_addr(xfer->data))
> +		return xfer->data;
> +
> +	if (xfer->rnw)
> +		/*
> +		 * Round up the receive bounce buffer length to a multiple of
> +		 * DWORDs. Independently of buffer alignment, DMA_FROM_DEVICE
> +		 * transfers may corrupt the last DWORD when transfer length is
> +		 * not a multiple of DWORDs. This was observed when the device
> +		 * DMA is IOMMU mapped or when an I3C target device returns
> +		 * less data than requested. Latter case is less severe and
> +		 * does not exceed the requested number of bytes, assuming the
> +		 * device DMA is not IOMMU mapped.
> +		 */
> +		xfer->bounce_buf = kzalloc(ALIGN(xfer->data_len, 4),
> +					   GFP_KERNEL);
> +	else
> +		xfer->bounce_buf = kmemdup(xfer->data, xfer->data_len,
> +					   GFP_KERNEL);
> +
> +	return xfer->bounce_buf;

Why need use bounce_buf? why not use dma_map_single()?, which will check
alignment and size to decide if use swiotlb as bounce buffer.

Frank

> +}
> +
> +static void hci_dma_free_safe_xfer_buf(struct i3c_hci *hci,
> +				       struct hci_xfer *xfer)
> +{
> +	if (xfer->bounce_buf == NULL)
> +		return;
> +
> +	if (xfer->rnw)
> +		memcpy(xfer->data, xfer->bounce_buf, xfer->data_len);
> +
> +	kfree(xfer->bounce_buf);
> +}
> +
>  static void hci_dma_unmap_xfer(struct i3c_hci *hci,
>  			       struct hci_xfer *xfer_list, unsigned int n)
>  {
> @@ -352,6 +390,7 @@ static void hci_dma_unmap_xfer(struct i3c_hci *hci,
>  		dma_unmap_single(&hci->master.dev,
>  				 xfer->data_dma, xfer->data_len,
>  				 xfer->rnw ? DMA_FROM_DEVICE : DMA_TO_DEVICE);
> +		hci_dma_free_safe_xfer_buf(hci, xfer);
>  	}
>  }
>
> @@ -391,7 +430,12 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
>
>  		/* 2nd and 3rd words of Data Buffer Descriptor Structure */
>  		if (xfer->data) {
> -			buf = xfer->bounce_buf ? xfer->bounce_buf : xfer->data;
> +			buf = hci_dma_alloc_safe_xfer_buf(hci, xfer);
> +			if (buf == NULL) {
> +				hci_dma_unmap_xfer(hci, xfer_list, i);
> +				return -ENOMEM;
> +			}
> +
>  			xfer->data_dma =
>  				dma_map_single(&hci->master.dev,
>  					       buf,
> @@ -401,6 +445,7 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
>  						  DMA_TO_DEVICE);
>  			if (dma_mapping_error(&hci->master.dev,
>  					      xfer->data_dma)) {
> +				hci_dma_free_safe_xfer_buf(hci, xfer);
>  				hci_dma_unmap_xfer(hci, xfer_list, i);
>  				return -ENOMEM;
>  			}
> --
> 2.47.2
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c