[PATCH v2 2/4] i3c: mipi-i3c-hci: Use core helpers for DMA mapping and bounce buffering

[Frank Li]

On Tue, Aug 19, 2025 at 09:15:27AM +0300, Jarkko Nikula wrote:
> On 8/18/25 7:17 PM, Frank Li wrote:
> > On Fri, Aug 15, 2025 at 05:12:40PM +0300, Jarkko Nikula wrote:
> > > So far only I3C private and I2C transfers have required a bounce buffer
> > > for DMA transfers when buffer is not DMA'able.
> > >
> > > It was observed that when the device DMA is IOMMU mapped and the receive
> > > length is not a multiple of DWORDs (32-bit), 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 DWORD aligned.
> > >
> > > To prepare for the device DMA being IOMMU mapped and to address the
> > > above issue, use helpers from I3C core for DMA mapping and bounce
> > > buffering for all DMA transfers.
> > >
> > > For now, require bounce buffer 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 at linux.intel.com>
> > > ---
> > >   drivers/i3c/master/mipi-i3c-hci/core.c | 34 --------------------------
> > >   drivers/i3c/master/mipi-i3c-hci/dma.c  | 32 +++++++++---------------
> > >   drivers/i3c/master/mipi-i3c-hci/hci.h  |  3 +--
> > >   3 files changed, 13 insertions(+), 56 deletions(-)
> > >
> > > diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> > > index 60f1175f1f37..b2977b6ac9f7 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 int i3c_hci_alloc_safe_xfer_buf(struct i3c_hci *hci,
> > > -				       struct hci_xfer *xfer)
> > > -{
> > > -	if (hci->io != &mipi_i3c_hci_dma ||
> > > -	    xfer->data == NULL || !is_vmalloc_addr(xfer->data))
> > > -		return 0;
> > > -
> > > -	if (xfer->rnw)
> > > -		xfer->bounce_buf = kzalloc(xfer->data_len, GFP_KERNEL);
> > > -	else
> > > -		xfer->bounce_buf = kmemdup(xfer->data,
> > > -					   xfer->data_len, GFP_KERNEL);
> > > -
> > > -	return xfer->bounce_buf == NULL ? -ENOMEM : 0;
> > > -}
> > > -
> > > -static void i3c_hci_free_safe_xfer_buf(struct i3c_hci *hci,
> > > -				       struct hci_xfer *xfer)
> > > -{
> > > -	if (hci->io != &mipi_i3c_hci_dma || xfer->bounce_buf == NULL)
> > > -		return;
> > > -
> > > -	if (xfer->rnw)
> > > -		memcpy(xfer->data, xfer->bounce_buf, xfer->data_len);
> > > -
> > > -	kfree(xfer->bounce_buf);
> > > -}
> > > -
> > >   static int i3c_hci_priv_xfers(struct i3c_dev_desc *dev,
> > >   			      struct i3c_priv_xfer *i3c_xfers,
> > >   			      int nxfers)
> > > @@ -333,9 +305,6 @@ static int i3c_hci_priv_xfers(struct i3c_dev_desc *dev,
> > >   		}
> > >   		hci->cmd->prep_i3c_xfer(hci, dev, &xfer[i]);
> > >   		xfer[i].cmd_desc[0] |= CMD_0_ROC;
> > > -		ret = i3c_hci_alloc_safe_xfer_buf(hci, &xfer[i]);
> > > -		if (ret)
> > > -			goto out;
> > >   	}
> > >   	last = i - 1;
> > >   	xfer[last].cmd_desc[0] |= CMD_0_TOC;
> > > @@ -359,9 +328,6 @@ static int i3c_hci_priv_xfers(struct i3c_dev_desc *dev,
> > >   	}
> > >
> > >   out:
> > > -	for (i = 0; i < nxfers; i++)
> > > -		i3c_hci_free_safe_xfer_buf(hci, &xfer[i]);
> > > -
> > >   	hci_free_xfer(xfer, nxfers);
> > >   	return ret;
> > >   }
> > > diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> > > index 491dfe70b660..295671daae09 100644
> > > --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> > > +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> > > @@ -342,16 +342,11 @@ static int hci_dma_init(struct i3c_hci *hci)
> > >   static void hci_dma_unmap_xfer(struct i3c_hci *hci,
> > >   			       struct hci_xfer *xfer_list, unsigned int n)
> > >   {
> > > -	struct hci_xfer *xfer;
> > >   	unsigned int i;
> > >
> > >   	for (i = 0; i < n; i++) {
> > > -		xfer = xfer_list + i;
> > > -		if (!xfer->data)
> > > -			continue;
> > > -		dma_unmap_single(&hci->master.dev,
> > > -				 xfer->data_dma, xfer->data_len,
> > > -				 xfer->rnw ? DMA_FROM_DEVICE : DMA_TO_DEVICE);
> > > +		struct hci_xfer *xfer = xfer_list + i;
> > > +		struct i3c_dma *dma_xfer __free(i3c_master_dma_unmap_single) = xfer->dma;
> >
> > same here, directlly call i3c_master_dma_unmap_single();
> >
> Does that then make needless to define DEFINE_FREE() for the
> i3c_master_dma_unmap_single() in the patch 1/4 since then there won't be
> user for it?

mipi may not use it because build in dma. If use dma engine, it will be
useful.

__free() dma = i3c_master_dma_map_single()

if (dmaengine_prep_slave_single(dma...) == NULL)
	return -1;

We can define DEFINE_FREE() later when real use it, but i3c_master_dma_unmap_single()
API design just one argument, so easy to use DEFINE_FREE.

Leave DEFINE_FREE() here also okay.

Frank

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