[PATCH v2 0/4] i3c: mipi-i3c-hci: Make able to work with IOMMU enabled

[PATCH v2 0/4] i3c: mipi-i3c-hci: Make able to work with IOMMU enabled

[Jarkko Nikula]

Hi

Here's the 2nd version of common I3C core DMA mapping and bounce buffering
handling and making the MIPI I3C HCI able to work with IOMMU enabled.

Changes to the v1:
- DMA mapping length is also cache_line_size() aligned in order to avoid
  possible double bouncing from the SWIOTLB.
- Using DEFINE_FREE and __free infra for cleanup and changes to the other
  patches accordingly.
- Added more description to the patch 1/4 commit log and slight corrections
  to others.

---
v1: http://lists.infradead.org/pipermail/linux-i3c/2025-July/002721.html

Jarkko Nikula (4):
  i3c: master: Add helpers for DMA mapping and bounce buffer handling
  i3c: mipi-i3c-hci: Use core helpers for DMA mapping and bounce
    buffering
  i3c: mipi-i3c-hci: Use physical device pointer with DMA API
  i3c: mipi-i3c-hci: Use own DMA bounce buffer management for I2C
    transfers

 drivers/i3c/master.c                   | 69 ++++++++++++++++++++++++
 drivers/i3c/master/mipi-i3c-hci/core.c | 40 +-------------
 drivers/i3c/master/mipi-i3c-hci/dma.c  | 74 +++++++++++++++-----------
 drivers/i3c/master/mipi-i3c-hci/hci.h  |  3 +-
 include/linux/i3c/master.h             | 26 +++++++++
 5 files changed, 140 insertions(+), 72 deletions(-)

-- 
2.47.2


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

[PATCH v2 1/4] i3c: master: Add helpers for DMA mapping and bounce buffer handling

[Jarkko Nikula]

Some I3C controllers such as MIPI I3C HCI may pad the last DWORD (32-bit)
with stale data from the RX FIFO in DMA transfers if the receive length
is not DWORD aligned and when the device DMA is IOMMU mapped.

In such a case, a properly sized bounce buffer is required in order to
avoid possible data corruption. In a review discussion, proposal was to
have a common helpers in I3C core for DMA mapping and bounce buffer
handling.

Drivers may use the helper i3c_master_dma_map_single() to map a buffer
for a DMA transfer. It internally allocates a bounce buffer if buffer is
not DMA'able or when the driver requires it for a transfer.

Helper i3c_master_dma_unmap_single() does the needed cleanups and
data copying from the bounce buffer.

Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
---
 drivers/i3c/master.c       | 69 ++++++++++++++++++++++++++++++++++++++
 include/linux/i3c/master.h | 26 ++++++++++++++
 2 files changed, 95 insertions(+)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 2ef898a8fd80..497bff57248a 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -8,6 +8,7 @@
 #include <linux/atomic.h>
 #include <linux/bug.h>
 #include <linux/device.h>
+#include <linux/dma-mapping.h>
 #include <linux/err.h>
 #include <linux/export.h>
 #include <linux/kernel.h>
@@ -1727,6 +1728,74 @@ int i3c_master_do_daa(struct i3c_master_controller *master)
 }
 EXPORT_SYMBOL_GPL(i3c_master_do_daa);
 
+/**
+ * i3c_master_dma_map_single() - Map buffer for single DMA transfer
+ * @dev: device object of a device doing DMA
+ * @buf: destination/source buffer for DMA
+ * @len: length of transfer
+ * @need_bounce: true if buffer is not DMA safe and need a bounce buffer
+ * @dir: DMA direction
+ *
+ * Map buffer for a DMA transfer and allocate a bounce buffer if required.
+ *
+ * Return: I3C DMA transfer descriptor or NULL in case of error.
+ */
+struct i3c_dma *i3c_master_dma_map_single(struct device *dev, void *buf,
+	size_t len, bool need_bounce, enum dma_data_direction dir)
+{
+	struct i3c_dma *dma_xfer __free(kfree) = NULL;
+	void *bounce __free(kfree) = NULL;
+	void *dma_buf = buf;
+
+	dma_xfer = kzalloc(sizeof(*dma_xfer), GFP_KERNEL);
+	if (!dma_xfer)
+		return NULL;
+	dma_xfer->dev = dev;
+	dma_xfer->buf = buf;
+	dma_xfer->dir = dir;
+	dma_xfer->len = len;
+	dma_xfer->map_len = len;
+
+	if (is_vmalloc_addr(buf))
+		need_bounce = true;
+
+	if (need_bounce) {
+		dma_xfer->map_len = ALIGN(len, cache_line_size());
+		if (dir == DMA_FROM_DEVICE)
+			bounce = kzalloc(dma_xfer->map_len, GFP_KERNEL);
+		else
+			bounce = kmemdup(buf, dma_xfer->map_len, GFP_KERNEL);
+		if (!bounce)
+			return NULL;
+		dma_buf = bounce;
+	}
+
+	dma_xfer->addr = dma_map_single(dev, dma_buf, dma_xfer->map_len, dir);
+	if (dma_mapping_error(dev, dma_xfer->addr))
+		return NULL;
+
+	dma_xfer->bounce_buf = no_free_ptr(bounce);
+	return no_free_ptr(dma_xfer);
+}
+EXPORT_SYMBOL_GPL(i3c_master_dma_map_single);
+
+/**
+ * i3c_master_dma_unmap_single() - Unmap buffer after DMA
+ * @dma_xfer: DMA transfer and mapping descriptor
+ *
+ * Unmap buffer and cleanup DMA transfer descriptor.
+ */
+void i3c_master_dma_unmap_single(struct i3c_dma *dma_xfer)
+{
+	struct i3c_dma *d __free(kfree) = dma_xfer;
+	void *bounce __free(kfree) = d->bounce_buf;
+
+	dma_unmap_single(d->dev, d->addr, d->map_len, d->dir);
+	if (bounce && d->dir == DMA_FROM_DEVICE)
+		memcpy(d->buf, bounce, d->len);
+}
+EXPORT_SYMBOL_GPL(i3c_master_dma_unmap_single);
+
 /**
  * i3c_master_set_info() - set master device information
  * @master: master used to send frames on the bus
diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
index 043f5c7ff398..80a621034ad0 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -558,6 +558,26 @@ struct i3c_master_controller {
 #define i3c_bus_for_each_i3cdev(bus, dev)				\
 	list_for_each_entry(dev, &(bus)->devs.i3c, common.node)
 
+/**
+ * struct i3c_dma - DMA transfer and mapping descriptor
+ * @dev: device object of a device doing DMA
+ * @buf: destination/source buffer for DMA
+ * @len: length of transfer
+ * @map_len: length of DMA mapping
+ * @addr: mapped DMA address for a Host Controller Driver
+ * @dir: DMA direction
+ * @bounce_buf: an allocated bounce buffer if transfer needs it or NULL
+ */
+struct i3c_dma {
+	struct device *dev;
+	void *buf;
+	size_t len;
+	size_t map_len;
+	dma_addr_t addr;
+	enum dma_data_direction dir;
+	void *bounce_buf;
+};
+
 int i3c_master_do_i2c_xfers(struct i3c_master_controller *master,
 			    const struct i2c_msg *xfers,
 			    int nxfers);
@@ -575,6 +595,12 @@ int i3c_master_get_free_addr(struct i3c_master_controller *master,
 int i3c_master_add_i3c_dev_locked(struct i3c_master_controller *master,
 				  u8 addr);
 int i3c_master_do_daa(struct i3c_master_controller *master);
+struct i3c_dma *i3c_master_dma_map_single(struct device *dev, void *ptr,
+					  size_t len, bool dma_safe,
+					  enum dma_data_direction dir);
+void i3c_master_dma_unmap_single(struct i3c_dma *dma_xfer);
+DEFINE_FREE(i3c_master_dma_unmap_single, void *,
+	    if (_T) i3c_master_dma_unmap_single(_T))
 
 int i3c_master_set_info(struct i3c_master_controller *master,
 			const struct i3c_device_info *info);
-- 
2.47.2


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

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

[Jarkko Nikula]

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@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;
 	}
 }
 
@@ -362,7 +357,6 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
 	struct hci_rh_data *rh;
 	unsigned int i, ring, enqueue_ptr;
 	u32 op1_val, op2_val;
-	void *buf;
 
 	/* For now we only use ring 0 */
 	ring = 0;
@@ -373,6 +367,8 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
 	for (i = 0; i < n; i++) {
 		struct hci_xfer *xfer = xfer_list + i;
 		u32 *ring_data = rh->xfer + rh->xfer_struct_sz * enqueue_ptr;
+		enum dma_data_direction dir = xfer->rnw ? DMA_FROM_DEVICE :
+							  DMA_TO_DEVICE;
 
 		/* store cmd descriptor */
 		*ring_data++ = xfer->cmd_desc[0];
@@ -391,21 +387,17 @@ 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;
-			xfer->data_dma =
-				dma_map_single(&hci->master.dev,
-					       buf,
-					       xfer->data_len,
-					       xfer->rnw ?
-						  DMA_FROM_DEVICE :
-						  DMA_TO_DEVICE);
-			if (dma_mapping_error(&hci->master.dev,
-					      xfer->data_dma)) {
+			xfer->dma = i3c_master_dma_map_single(&hci->master.dev,
+							      xfer->data,
+							      xfer->data_len,
+							      false,
+							      dir);
+			if (!xfer->dma) {
 				hci_dma_unmap_xfer(hci, xfer_list, i);
 				return -ENOMEM;
 			}
-			*ring_data++ = lower_32_bits(xfer->data_dma);
-			*ring_data++ = upper_32_bits(xfer->data_dma);
+			*ring_data++ = lower_32_bits(xfer->dma->addr);
+			*ring_data++ = upper_32_bits(xfer->dma->addr);
 		} else {
 			*ring_data++ = 0;
 			*ring_data++ = 0;
diff --git a/drivers/i3c/master/mipi-i3c-hci/hci.h b/drivers/i3c/master/mipi-i3c-hci/hci.h
index 69ea1d10414b..33bc4906df1f 100644
--- a/drivers/i3c/master/mipi-i3c-hci/hci.h
+++ b/drivers/i3c/master/mipi-i3c-hci/hci.h
@@ -94,8 +94,7 @@ struct hci_xfer {
 		};
 		struct {
 			/* DMA specific */
-			dma_addr_t data_dma;
-			void *bounce_buf;
+			struct i3c_dma *dma;
 			int ring_number;
 			int ring_entry;
 		};
-- 
2.47.2


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

[PATCH v2 3/4] i3c: mipi-i3c-hci: Use physical device pointer with DMA API

[Jarkko Nikula]

DMA transfer faults on Intel hardware when the IOMMU is enabled and
driver initialization will fail when attempting to do the first transfer:

	DMAR: DRHD: handling fault status reg 2
	DMAR: [DMA Read NO_PASID] Request device [00:11.0] fault addr 0x676e3000 [fault reason 0x71] SM: Present bit in first-level paging entry is clear
 	i3c mipi-i3c-hci.0: ring 0: Transfer Aborted
	mipi-i3c-hci mipi-i3c-hci.0: probe with driver mipi-i3c-hci failed with error -62

Reason for this is that the IOMMU setup is done for the physical devices
only and not for the virtual I3C Controller device object.

Therefore use the pointer to a physical device object with the DMA API.

Due to a data corruption observation when the device DMA is IOMMU
mapped, a properly sized receive bounce buffer is required if transfer
length is not a multiple of DWORDs.

Reported-by: Prabhakaran, Krishna <krishna.prabhakaran@intel.com>
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
---
 drivers/i3c/master/mipi-i3c-hci/dma.c | 46 +++++++++++++++++++--------
 1 file changed, 33 insertions(+), 13 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
index 295671daae09..8cea91d4d3bf 100644
--- a/drivers/i3c/master/mipi-i3c-hci/dma.c
+++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
@@ -14,6 +14,7 @@
 #include <linux/errno.h>
 #include <linux/i3c/master.h>
 #include <linux/io.h>
+#include <linux/pci.h>
 
 #include "hci.h"
 #include "cmd.h"
@@ -138,6 +139,7 @@ struct hci_rh_data {
 };
 
 struct hci_rings_data {
+	struct device *sysdev;
 	unsigned int total;
 	struct hci_rh_data headers[] __counted_by(total);
 };
@@ -165,20 +167,20 @@ static void hci_dma_cleanup(struct i3c_hci *hci)
 		rh_reg_write(IBI_SETUP, 0);
 
 		if (rh->xfer)
-			dma_free_coherent(&hci->master.dev,
+			dma_free_coherent(rings->sysdev,
 					  rh->xfer_struct_sz * rh->xfer_entries,
 					  rh->xfer, rh->xfer_dma);
 		if (rh->resp)
-			dma_free_coherent(&hci->master.dev,
+			dma_free_coherent(rings->sysdev,
 					  rh->resp_struct_sz * rh->xfer_entries,
 					  rh->resp, rh->resp_dma);
 		kfree(rh->src_xfers);
 		if (rh->ibi_status)
-			dma_free_coherent(&hci->master.dev,
+			dma_free_coherent(rings->sysdev,
 					  rh->ibi_status_sz * rh->ibi_status_entries,
 					  rh->ibi_status, rh->ibi_status_dma);
 		if (rh->ibi_data_dma)
-			dma_unmap_single(&hci->master.dev, rh->ibi_data_dma,
+			dma_unmap_single(rings->sysdev, rh->ibi_data_dma,
 					 rh->ibi_chunk_sz * rh->ibi_chunks_total,
 					 DMA_FROM_DEVICE);
 		kfree(rh->ibi_data);
@@ -194,11 +196,23 @@ static int hci_dma_init(struct i3c_hci *hci)
 {
 	struct hci_rings_data *rings;
 	struct hci_rh_data *rh;
+	struct device *sysdev;
 	u32 regval;
 	unsigned int i, nr_rings, xfers_sz, resps_sz;
 	unsigned int ibi_status_ring_sz, ibi_data_ring_sz;
 	int ret;
 
+	/*
+	 * Set pointer to a physical device that does DMA and has IOMMU setup
+	 * done for it in case of enabled IOMMU and use it with the DMA API.
+	 * Here such device is either
+	 * "mipi-i3c-hci" platform device (OF/ACPI enumeration) parent or
+	 * grandparent (PCI enumeration).
+	 */
+	sysdev = hci->master.dev.parent;
+	if (sysdev->parent && dev_is_pci(sysdev->parent))
+		sysdev = sysdev->parent;
+
 	regval = rhs_reg_read(CONTROL);
 	nr_rings = FIELD_GET(MAX_HEADER_COUNT_CAP, regval);
 	dev_info(&hci->master.dev, "%d DMA rings available\n", nr_rings);
@@ -213,6 +227,7 @@ static int hci_dma_init(struct i3c_hci *hci)
 		return -ENOMEM;
 	hci->io_data = rings;
 	rings->total = nr_rings;
+	rings->sysdev = sysdev;
 
 	regval = FIELD_PREP(MAX_HEADER_COUNT, rings->total);
 	rhs_reg_write(CONTROL, regval);
@@ -239,9 +254,9 @@ static int hci_dma_init(struct i3c_hci *hci)
 		xfers_sz = rh->xfer_struct_sz * rh->xfer_entries;
 		resps_sz = rh->resp_struct_sz * rh->xfer_entries;
 
-		rh->xfer = dma_alloc_coherent(&hci->master.dev, xfers_sz,
+		rh->xfer = dma_alloc_coherent(rings->sysdev, xfers_sz,
 					      &rh->xfer_dma, GFP_KERNEL);
-		rh->resp = dma_alloc_coherent(&hci->master.dev, resps_sz,
+		rh->resp = dma_alloc_coherent(rings->sysdev, resps_sz,
 					      &rh->resp_dma, GFP_KERNEL);
 		rh->src_xfers =
 			kmalloc_array(rh->xfer_entries, sizeof(*rh->src_xfers),
@@ -295,16 +310,16 @@ static int hci_dma_init(struct i3c_hci *hci)
 		ibi_data_ring_sz = rh->ibi_chunk_sz * rh->ibi_chunks_total;
 
 		rh->ibi_status =
-			dma_alloc_coherent(&hci->master.dev, ibi_status_ring_sz,
+			dma_alloc_coherent(rings->sysdev, ibi_status_ring_sz,
 					   &rh->ibi_status_dma, GFP_KERNEL);
 		rh->ibi_data = kmalloc(ibi_data_ring_sz, GFP_KERNEL);
 		ret = -ENOMEM;
 		if (!rh->ibi_status || !rh->ibi_data)
 			goto err_out;
 		rh->ibi_data_dma =
-			dma_map_single(&hci->master.dev, rh->ibi_data,
+			dma_map_single(rings->sysdev, rh->ibi_data,
 				       ibi_data_ring_sz, DMA_FROM_DEVICE);
-		if (dma_mapping_error(&hci->master.dev, rh->ibi_data_dma)) {
+		if (dma_mapping_error(rings->sysdev, rh->ibi_data_dma)) {
 			rh->ibi_data_dma = 0;
 			ret = -ENOMEM;
 			goto err_out;
@@ -369,6 +384,7 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
 		u32 *ring_data = rh->xfer + rh->xfer_struct_sz * enqueue_ptr;
 		enum dma_data_direction dir = xfer->rnw ? DMA_FROM_DEVICE :
 							  DMA_TO_DEVICE;
+		bool need_bounce;
 
 		/* store cmd descriptor */
 		*ring_data++ = xfer->cmd_desc[0];
@@ -387,10 +403,13 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
 
 		/* 2nd and 3rd words of Data Buffer Descriptor Structure */
 		if (xfer->data) {
-			xfer->dma = i3c_master_dma_map_single(&hci->master.dev,
+			need_bounce = device_iommu_mapped(rings->sysdev) &&
+				      xfer->rnw &&
+				      xfer->data_len != ALIGN(xfer->data_len, 4);
+			xfer->dma = i3c_master_dma_map_single(rings->sysdev,
 							      xfer->data,
 							      xfer->data_len,
-							      false,
+							      need_bounce,
 							      dir);
 			if (!xfer->dma) {
 				hci_dma_unmap_xfer(hci, xfer_list, i);
@@ -578,6 +597,7 @@ static void hci_dma_recycle_ibi_slot(struct i3c_hci *hci,
 
 static void hci_dma_process_ibi(struct i3c_hci *hci, struct hci_rh_data *rh)
 {
+	struct hci_rings_data *rings = hci->io_data;
 	struct i3c_dev_desc *dev;
 	struct i3c_hci_dev_data *dev_data;
 	struct hci_dma_dev_ibi_data *dev_ibi;
@@ -688,7 +708,7 @@ static void hci_dma_process_ibi(struct i3c_hci *hci, struct hci_rh_data *rh)
 			* rh->ibi_chunk_sz;
 	if (first_part > ibi_size)
 		first_part = ibi_size;
-	dma_sync_single_for_cpu(&hci->master.dev, ring_ibi_data_dma,
+	dma_sync_single_for_cpu(rings->sysdev, ring_ibi_data_dma,
 				first_part, DMA_FROM_DEVICE);
 	memcpy(slot->data, ring_ibi_data, first_part);
 
@@ -697,7 +717,7 @@ static void hci_dma_process_ibi(struct i3c_hci *hci, struct hci_rh_data *rh)
 		/* we wrap back to the start and copy remaining data */
 		ring_ibi_data = rh->ibi_data;
 		ring_ibi_data_dma = rh->ibi_data_dma;
-		dma_sync_single_for_cpu(&hci->master.dev, ring_ibi_data_dma,
+		dma_sync_single_for_cpu(rings->sysdev, ring_ibi_data_dma,
 					ibi_size - first_part, DMA_FROM_DEVICE);
 		memcpy(slot->data + first_part, ring_ibi_data,
 		       ibi_size - first_part);
-- 
2.47.2


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

[PATCH v2 4/4] i3c: mipi-i3c-hci: Use own DMA bounce buffer management for I2C transfers

[Jarkko Nikula]

Stop using I2C DMA-safe API for two reasons:
- Not needed if driver is using PIO mode.
- DMA transfers needs a DWORD align sized receive bounce buffer when the
  device DMA is IOMMU mapped, which is causing needless double bounce
  buffering in that case.

Cc: Billy Tsai <billy_tsai@aspeedtech.com>
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
---
 drivers/i3c/master/mipi-i3c-hci/core.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
index b2977b6ac9f7..7a467ef65787 100644
--- a/drivers/i3c/master/mipi-i3c-hci/core.c
+++ b/drivers/i3c/master/mipi-i3c-hci/core.c
@@ -348,7 +348,7 @@ static int i3c_hci_i2c_xfers(struct i2c_dev_desc *dev,
 		return -ENOMEM;
 
 	for (i = 0; i < nxfers; i++) {
-		xfer[i].data = i2c_get_dma_safe_msg_buf(&i2c_xfers[i], 1);
+		xfer[i].data = i2c_xfers[i].buf;
 		xfer[i].data_len = i2c_xfers[i].len;
 		xfer[i].rnw = i2c_xfers[i].flags & I2C_M_RD;
 		hci->cmd->prep_i2c_xfer(hci, dev, &xfer[i]);
@@ -374,10 +374,6 @@ static int i3c_hci_i2c_xfers(struct i2c_dev_desc *dev,
 	}
 
 out:
-	for (i = 0; i < nxfers; i++)
-		i2c_put_dma_safe_msg_buf(xfer[i].data, &i2c_xfers[i],
-					 ret ? false : true);
-
 	hci_free_xfer(xfer, nxfers);
 	return ret;
 }
-- 
2.47.2


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

Re: [PATCH v2 1/4] i3c: master: Add helpers for DMA mapping and bounce buffer handling

[Frank Li]

On Fri, Aug 15, 2025 at 05:12:39PM +0300, Jarkko Nikula wrote:
> Some I3C controllers such as MIPI I3C HCI may pad the last DWORD (32-bit)
> with stale data from the RX FIFO in DMA transfers if the receive length
> is not DWORD aligned and when the device DMA is IOMMU mapped.
>
> In such a case, a properly sized bounce buffer is required in order to
> avoid possible data corruption. In a review discussion, proposal was to
> have a common helpers in I3C core for DMA mapping and bounce buffer
> handling.
>
> Drivers may use the helper i3c_master_dma_map_single() to map a buffer
> for a DMA transfer. It internally allocates a bounce buffer if buffer is
> not DMA'able or when the driver requires it for a transfer.
>
> Helper i3c_master_dma_unmap_single() does the needed cleanups and
> data copying from the bounce buffer.
>
> Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> ---
>  drivers/i3c/master.c       | 69 ++++++++++++++++++++++++++++++++++++++
>  include/linux/i3c/master.h | 26 ++++++++++++++
>  2 files changed, 95 insertions(+)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 2ef898a8fd80..497bff57248a 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -8,6 +8,7 @@
>  #include <linux/atomic.h>
>  #include <linux/bug.h>
>  #include <linux/device.h>
> +#include <linux/dma-mapping.h>
>  #include <linux/err.h>
>  #include <linux/export.h>
>  #include <linux/kernel.h>
> @@ -1727,6 +1728,74 @@ int i3c_master_do_daa(struct i3c_master_controller *master)
>  }
>  EXPORT_SYMBOL_GPL(i3c_master_do_daa);
>
> +/**
> + * i3c_master_dma_map_single() - Map buffer for single DMA transfer
> + * @dev: device object of a device doing DMA
> + * @buf: destination/source buffer for DMA
> + * @len: length of transfer
> + * @need_bounce: true if buffer is not DMA safe and need a bounce buffer
> + * @dir: DMA direction
> + *
> + * Map buffer for a DMA transfer and allocate a bounce buffer if required.
> + *
> + * Return: I3C DMA transfer descriptor or NULL in case of error.
> + */
> +struct i3c_dma *i3c_master_dma_map_single(struct device *dev, void *buf,
> +	size_t len, bool need_bounce, enum dma_data_direction dir)
> +{
> +	struct i3c_dma *dma_xfer __free(kfree) = NULL;
> +	void *bounce __free(kfree) = NULL;
> +	void *dma_buf = buf;
> +
> +	dma_xfer = kzalloc(sizeof(*dma_xfer), GFP_KERNEL);
> +	if (!dma_xfer)
> +		return NULL;

nit: add empty line here

> +	dma_xfer->dev = dev;
> +	dma_xfer->buf = buf;
> +	dma_xfer->dir = dir;
> +	dma_xfer->len = len;
> +	dma_xfer->map_len = len;
> +
> +	if (is_vmalloc_addr(buf))
> +		need_bounce = true;
> +
> +	if (need_bounce) {
> +		dma_xfer->map_len = ALIGN(len, cache_line_size());
> +		if (dir == DMA_FROM_DEVICE)
> +			bounce = kzalloc(dma_xfer->map_len, GFP_KERNEL);
> +		else
> +			bounce = kmemdup(buf, dma_xfer->map_len, GFP_KERNEL);
> +		if (!bounce)
> +			return NULL;
> +		dma_buf = bounce;
> +	}
> +
> +	dma_xfer->addr = dma_map_single(dev, dma_buf, dma_xfer->map_len, dir);
> +	if (dma_mapping_error(dev, dma_xfer->addr))
> +		return NULL;
> +
> +	dma_xfer->bounce_buf = no_free_ptr(bounce);
> +	return no_free_ptr(dma_xfer);
> +}
> +EXPORT_SYMBOL_GPL(i3c_master_dma_map_single);
> +
> +/**
> + * i3c_master_dma_unmap_single() - Unmap buffer after DMA
> + * @dma_xfer: DMA transfer and mapping descriptor
> + *
> + * Unmap buffer and cleanup DMA transfer descriptor.
> + */
> +void i3c_master_dma_unmap_single(struct i3c_dma *dma_xfer)
> +{
> +	struct i3c_dma *d __free(kfree) = dma_xfer;
> +	void *bounce __free(kfree) = d->bounce_buf;

This simple free, call free(dma_xfer) after memcpy directly.

Frank
> +
> +	dma_unmap_single(d->dev, d->addr, d->map_len, d->dir);
> +	if (bounce && d->dir == DMA_FROM_DEVICE)
> +		memcpy(d->buf, bounce, d->len);
> +}
> +EXPORT_SYMBOL_GPL(i3c_master_dma_unmap_single);
> +
>  /**
>   * i3c_master_set_info() - set master device information
>   * @master: master used to send frames on the bus
> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
> index 043f5c7ff398..80a621034ad0 100644
> --- a/include/linux/i3c/master.h
> +++ b/include/linux/i3c/master.h
> @@ -558,6 +558,26 @@ struct i3c_master_controller {
>  #define i3c_bus_for_each_i3cdev(bus, dev)				\
>  	list_for_each_entry(dev, &(bus)->devs.i3c, common.node)
>
> +/**
> + * struct i3c_dma - DMA transfer and mapping descriptor
> + * @dev: device object of a device doing DMA
> + * @buf: destination/source buffer for DMA
> + * @len: length of transfer
> + * @map_len: length of DMA mapping
> + * @addr: mapped DMA address for a Host Controller Driver
> + * @dir: DMA direction
> + * @bounce_buf: an allocated bounce buffer if transfer needs it or NULL
> + */
> +struct i3c_dma {
> +	struct device *dev;
> +	void *buf;
> +	size_t len;
> +	size_t map_len;
> +	dma_addr_t addr;
> +	enum dma_data_direction dir;
> +	void *bounce_buf;
> +};
> +
>  int i3c_master_do_i2c_xfers(struct i3c_master_controller *master,
>  			    const struct i2c_msg *xfers,
>  			    int nxfers);
> @@ -575,6 +595,12 @@ int i3c_master_get_free_addr(struct i3c_master_controller *master,
>  int i3c_master_add_i3c_dev_locked(struct i3c_master_controller *master,
>  				  u8 addr);
>  int i3c_master_do_daa(struct i3c_master_controller *master);
> +struct i3c_dma *i3c_master_dma_map_single(struct device *dev, void *ptr,
> +					  size_t len, bool dma_safe,
> +					  enum dma_data_direction dir);
> +void i3c_master_dma_unmap_single(struct i3c_dma *dma_xfer);
> +DEFINE_FREE(i3c_master_dma_unmap_single, void *,
> +	    if (_T) i3c_master_dma_unmap_single(_T))
>
>  int i3c_master_set_info(struct i3c_master_controller *master,
>  			const struct i3c_device_info *info);
> --
> 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

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

[Frank Li]

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@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();

Frank
>  	}
>  }
>
> @@ -362,7 +357,6 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
>  	struct hci_rh_data *rh;
>  	unsigned int i, ring, enqueue_ptr;
>  	u32 op1_val, op2_val;
> -	void *buf;
>
>  	/* For now we only use ring 0 */
>  	ring = 0;
> @@ -373,6 +367,8 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
>  	for (i = 0; i < n; i++) {
>  		struct hci_xfer *xfer = xfer_list + i;
>  		u32 *ring_data = rh->xfer + rh->xfer_struct_sz * enqueue_ptr;
> +		enum dma_data_direction dir = xfer->rnw ? DMA_FROM_DEVICE :
> +							  DMA_TO_DEVICE;
>
>  		/* store cmd descriptor */
>  		*ring_data++ = xfer->cmd_desc[0];
> @@ -391,21 +387,17 @@ 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;
> -			xfer->data_dma =
> -				dma_map_single(&hci->master.dev,
> -					       buf,
> -					       xfer->data_len,
> -					       xfer->rnw ?
> -						  DMA_FROM_DEVICE :
> -						  DMA_TO_DEVICE);
> -			if (dma_mapping_error(&hci->master.dev,
> -					      xfer->data_dma)) {
> +			xfer->dma = i3c_master_dma_map_single(&hci->master.dev,
> +							      xfer->data,
> +							      xfer->data_len,
> +							      false,
> +							      dir);
> +			if (!xfer->dma) {
>  				hci_dma_unmap_xfer(hci, xfer_list, i);
>  				return -ENOMEM;
>  			}
> -			*ring_data++ = lower_32_bits(xfer->data_dma);
> -			*ring_data++ = upper_32_bits(xfer->data_dma);
> +			*ring_data++ = lower_32_bits(xfer->dma->addr);
> +			*ring_data++ = upper_32_bits(xfer->dma->addr);
>  		} else {
>  			*ring_data++ = 0;
>  			*ring_data++ = 0;
> diff --git a/drivers/i3c/master/mipi-i3c-hci/hci.h b/drivers/i3c/master/mipi-i3c-hci/hci.h
> index 69ea1d10414b..33bc4906df1f 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/hci.h
> +++ b/drivers/i3c/master/mipi-i3c-hci/hci.h
> @@ -94,8 +94,7 @@ struct hci_xfer {
>  		};
>  		struct {
>  			/* DMA specific */
> -			dma_addr_t data_dma;
> -			void *bounce_buf;
> +			struct i3c_dma *dma;
>  			int ring_number;
>  			int ring_entry;
>  		};
> --
> 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

Re: [PATCH v2 3/4] i3c: mipi-i3c-hci: Use physical device pointer with DMA API

[Frank Li]

On Fri, Aug 15, 2025 at 05:12:41PM +0300, Jarkko Nikula wrote:
> DMA transfer faults on Intel hardware when the IOMMU is enabled and
> driver initialization will fail when attempting to do the first transfer:
>
> 	DMAR: DRHD: handling fault status reg 2
> 	DMAR: [DMA Read NO_PASID] Request device [00:11.0] fault addr 0x676e3000 [fault reason 0x71] SM: Present bit in first-level paging entry is clear
>  	i3c mipi-i3c-hci.0: ring 0: Transfer Aborted
> 	mipi-i3c-hci mipi-i3c-hci.0: probe with driver mipi-i3c-hci failed with error -62
>
> Reason for this is that the IOMMU setup is done for the physical devices
> only and not for the virtual I3C Controller device object.
>
> Therefore use the pointer to a physical device object with the DMA API.
>
> Due to a data corruption observation when the device DMA is IOMMU
> mapped, a properly sized receive bounce buffer is required if transfer
> length is not a multiple of DWORDs.
>
> Reported-by: Prabhakaran, Krishna <krishna.prabhakaran@intel.com>
> Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> ---
>  drivers/i3c/master/mipi-i3c-hci/dma.c | 46 +++++++++++++++++++--------
>  1 file changed, 33 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index 295671daae09..8cea91d4d3bf 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -14,6 +14,7 @@
>  #include <linux/errno.h>
>  #include <linux/i3c/master.h>
>  #include <linux/io.h>
> +#include <linux/pci.h>
>
>  #include "hci.h"
>  #include "cmd.h"
> @@ -138,6 +139,7 @@ struct hci_rh_data {
>  };
>
>  struct hci_rings_data {
> +	struct device *sysdev;
>  	unsigned int total;
>  	struct hci_rh_data headers[] __counted_by(total);
>  };
> @@ -165,20 +167,20 @@ static void hci_dma_cleanup(struct i3c_hci *hci)
>  		rh_reg_write(IBI_SETUP, 0);
>
>  		if (rh->xfer)
> -			dma_free_coherent(&hci->master.dev,
> +			dma_free_coherent(rings->sysdev,
>  					  rh->xfer_struct_sz * rh->xfer_entries,
>  					  rh->xfer, rh->xfer_dma);
>  		if (rh->resp)
> -			dma_free_coherent(&hci->master.dev,
> +			dma_free_coherent(rings->sysdev,
>  					  rh->resp_struct_sz * rh->xfer_entries,
>  					  rh->resp, rh->resp_dma);
>  		kfree(rh->src_xfers);
>  		if (rh->ibi_status)
> -			dma_free_coherent(&hci->master.dev,
> +			dma_free_coherent(rings->sysdev,
>  					  rh->ibi_status_sz * rh->ibi_status_entries,
>  					  rh->ibi_status, rh->ibi_status_dma);
>  		if (rh->ibi_data_dma)
> -			dma_unmap_single(&hci->master.dev, rh->ibi_data_dma,
> +			dma_unmap_single(rings->sysdev, rh->ibi_data_dma,
>  					 rh->ibi_chunk_sz * rh->ibi_chunks_total,
>  					 DMA_FROM_DEVICE);
>  		kfree(rh->ibi_data);
> @@ -194,11 +196,23 @@ static int hci_dma_init(struct i3c_hci *hci)
>  {
>  	struct hci_rings_data *rings;
>  	struct hci_rh_data *rh;
> +	struct device *sysdev;
>  	u32 regval;
>  	unsigned int i, nr_rings, xfers_sz, resps_sz;
>  	unsigned int ibi_status_ring_sz, ibi_data_ring_sz;
>  	int ret;
>
> +	/*
> +	 * Set pointer to a physical device that does DMA and has IOMMU setup
> +	 * done for it in case of enabled IOMMU and use it with the DMA API.
> +	 * Here such device is either
> +	 * "mipi-i3c-hci" platform device (OF/ACPI enumeration) parent or
> +	 * grandparent (PCI enumeration).
> +	 */
> +	sysdev = hci->master.dev.parent;
> +	if (sysdev->parent && dev_is_pci(sysdev->parent))
> +		sysdev = sysdev->parent;
> +
>  	regval = rhs_reg_read(CONTROL);
>  	nr_rings = FIELD_GET(MAX_HEADER_COUNT_CAP, regval);
>  	dev_info(&hci->master.dev, "%d DMA rings available\n", nr_rings);
> @@ -213,6 +227,7 @@ static int hci_dma_init(struct i3c_hci *hci)
>  		return -ENOMEM;
>  	hci->io_data = rings;
>  	rings->total = nr_rings;
> +	rings->sysdev = sysdev;
>
>  	regval = FIELD_PREP(MAX_HEADER_COUNT, rings->total);
>  	rhs_reg_write(CONTROL, regval);
> @@ -239,9 +254,9 @@ static int hci_dma_init(struct i3c_hci *hci)
>  		xfers_sz = rh->xfer_struct_sz * rh->xfer_entries;
>  		resps_sz = rh->resp_struct_sz * rh->xfer_entries;
>
> -		rh->xfer = dma_alloc_coherent(&hci->master.dev, xfers_sz,
> +		rh->xfer = dma_alloc_coherent(rings->sysdev, xfers_sz,
>  					      &rh->xfer_dma, GFP_KERNEL);
> -		rh->resp = dma_alloc_coherent(&hci->master.dev, resps_sz,
> +		rh->resp = dma_alloc_coherent(rings->sysdev, resps_sz,
>  					      &rh->resp_dma, GFP_KERNEL);
>  		rh->src_xfers =
>  			kmalloc_array(rh->xfer_entries, sizeof(*rh->src_xfers),
> @@ -295,16 +310,16 @@ static int hci_dma_init(struct i3c_hci *hci)
>  		ibi_data_ring_sz = rh->ibi_chunk_sz * rh->ibi_chunks_total;
>
>  		rh->ibi_status =
> -			dma_alloc_coherent(&hci->master.dev, ibi_status_ring_sz,
> +			dma_alloc_coherent(rings->sysdev, ibi_status_ring_sz,
>  					   &rh->ibi_status_dma, GFP_KERNEL);
>  		rh->ibi_data = kmalloc(ibi_data_ring_sz, GFP_KERNEL);
>  		ret = -ENOMEM;
>  		if (!rh->ibi_status || !rh->ibi_data)
>  			goto err_out;
>  		rh->ibi_data_dma =
> -			dma_map_single(&hci->master.dev, rh->ibi_data,
> +			dma_map_single(rings->sysdev, rh->ibi_data,
>  				       ibi_data_ring_sz, DMA_FROM_DEVICE);
> -		if (dma_mapping_error(&hci->master.dev, rh->ibi_data_dma)) {
> +		if (dma_mapping_error(rings->sysdev, rh->ibi_data_dma)) {
>  			rh->ibi_data_dma = 0;
>  			ret = -ENOMEM;
>  			goto err_out;
> @@ -369,6 +384,7 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
>  		u32 *ring_data = rh->xfer + rh->xfer_struct_sz * enqueue_ptr;
>  		enum dma_data_direction dir = xfer->rnw ? DMA_FROM_DEVICE :
>  							  DMA_TO_DEVICE;
> +		bool need_bounce;
>
>  		/* store cmd descriptor */
>  		*ring_data++ = xfer->cmd_desc[0];
> @@ -387,10 +403,13 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
>
>  		/* 2nd and 3rd words of Data Buffer Descriptor Structure */
>  		if (xfer->data) {
> -			xfer->dma = i3c_master_dma_map_single(&hci->master.dev,
> +			need_bounce = device_iommu_mapped(rings->sysdev) &&
> +				      xfer->rnw &&

why only read need bounce? if iommu have not map enough size, still fault
for write.

such as you try to map IO space [0xffd,0x1000). when hardware try access
0x1000, iommu will capture this type error. Maybe it is hard to meet
allocate io space at close end of page, you have not found this problem.

Frank

> +				      xfer->data_len != ALIGN(xfer->data_len, 4);
> +			xfer->dma = i3c_master_dma_map_single(rings->sysdev,
>  							      xfer->data,
>  							      xfer->data_len,
> -							      false,
> +							      need_bounce,
>  							      dir);
>  			if (!xfer->dma) {
>  				hci_dma_unmap_xfer(hci, xfer_list, i);
> @@ -578,6 +597,7 @@ static void hci_dma_recycle_ibi_slot(struct i3c_hci *hci,
>
>  static void hci_dma_process_ibi(struct i3c_hci *hci, struct hci_rh_data *rh)
>  {
> +	struct hci_rings_data *rings = hci->io_data;
>  	struct i3c_dev_desc *dev;
>  	struct i3c_hci_dev_data *dev_data;
>  	struct hci_dma_dev_ibi_data *dev_ibi;
> @@ -688,7 +708,7 @@ static void hci_dma_process_ibi(struct i3c_hci *hci, struct hci_rh_data *rh)
>  			* rh->ibi_chunk_sz;
>  	if (first_part > ibi_size)
>  		first_part = ibi_size;
> -	dma_sync_single_for_cpu(&hci->master.dev, ring_ibi_data_dma,
> +	dma_sync_single_for_cpu(rings->sysdev, ring_ibi_data_dma,
>  				first_part, DMA_FROM_DEVICE);
>  	memcpy(slot->data, ring_ibi_data, first_part);
>
> @@ -697,7 +717,7 @@ static void hci_dma_process_ibi(struct i3c_hci *hci, struct hci_rh_data *rh)
>  		/* we wrap back to the start and copy remaining data */
>  		ring_ibi_data = rh->ibi_data;
>  		ring_ibi_data_dma = rh->ibi_data_dma;
> -		dma_sync_single_for_cpu(&hci->master.dev, ring_ibi_data_dma,
> +		dma_sync_single_for_cpu(rings->sysdev, ring_ibi_data_dma,
>  					ibi_size - first_part, DMA_FROM_DEVICE);
>  		memcpy(slot->data + first_part, ring_ibi_data,
>  		       ibi_size - first_part);
> --
> 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

Re: [PATCH v2 4/4] i3c: mipi-i3c-hci: Use own DMA bounce buffer management for I2C transfers

[Frank Li]

On Fri, Aug 15, 2025 at 05:12:42PM +0300, Jarkko Nikula wrote:
> Stop using I2C DMA-safe API for two reasons:
> - Not needed if driver is using PIO mode.
> - DMA transfers needs a DWORD align sized receive bounce buffer when the
>   device DMA is IOMMU mapped, which is causing needless double bounce
>   buffering in that case.
>
> Cc: Billy Tsai <billy_tsai@aspeedtech.com>
> Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
>  drivers/i3c/master/mipi-i3c-hci/core.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> index b2977b6ac9f7..7a467ef65787 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
> @@ -348,7 +348,7 @@ static int i3c_hci_i2c_xfers(struct i2c_dev_desc *dev,
>  		return -ENOMEM;
>
>  	for (i = 0; i < nxfers; i++) {
> -		xfer[i].data = i2c_get_dma_safe_msg_buf(&i2c_xfers[i], 1);
> +		xfer[i].data = i2c_xfers[i].buf;
>  		xfer[i].data_len = i2c_xfers[i].len;
>  		xfer[i].rnw = i2c_xfers[i].flags & I2C_M_RD;
>  		hci->cmd->prep_i2c_xfer(hci, dev, &xfer[i]);
> @@ -374,10 +374,6 @@ static int i3c_hci_i2c_xfers(struct i2c_dev_desc *dev,
>  	}
>
>  out:
> -	for (i = 0; i < nxfers; i++)
> -		i2c_put_dma_safe_msg_buf(xfer[i].data, &i2c_xfers[i],
> -					 ret ? false : true);
> -
>  	hci_free_xfer(xfer, nxfers);
>  	return ret;
>  }
> --
> 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

Re: [PATCH v2 1/4] i3c: master: Add helpers for DMA mapping and bounce buffer handling

[Jarkko Nikula]

On 8/18/25 7:07 PM, Frank Li wrote:
> On Fri, Aug 15, 2025 at 05:12:39PM +0300, Jarkko Nikula wrote:
>> Some I3C controllers such as MIPI I3C HCI may pad the last DWORD (32-bit)
>> with stale data from the RX FIFO in DMA transfers if the receive length
>> is not DWORD aligned and when the device DMA is IOMMU mapped.
>>
>> In such a case, a properly sized bounce buffer is required in order to
>> avoid possible data corruption. In a review discussion, proposal was to
>> have a common helpers in I3C core for DMA mapping and bounce buffer
>> handling.
>>
>> Drivers may use the helper i3c_master_dma_map_single() to map a buffer
>> for a DMA transfer. It internally allocates a bounce buffer if buffer is
>> not DMA'able or when the driver requires it for a transfer.
>>
>> Helper i3c_master_dma_unmap_single() does the needed cleanups and
>> data copying from the bounce buffer.
>>
>> Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
>> ---
>>   drivers/i3c/master.c       | 69 ++++++++++++++++++++++++++++++++++++++
>>   include/linux/i3c/master.h | 26 ++++++++++++++
>>   2 files changed, 95 insertions(+)
>>
>> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
>> index 2ef898a8fd80..497bff57248a 100644
>> --- a/drivers/i3c/master.c
>> +++ b/drivers/i3c/master.c
>> @@ -8,6 +8,7 @@
>>   #include <linux/atomic.h>
>>   #include <linux/bug.h>
>>   #include <linux/device.h>
>> +#include <linux/dma-mapping.h>
>>   #include <linux/err.h>
>>   #include <linux/export.h>
>>   #include <linux/kernel.h>
>> @@ -1727,6 +1728,74 @@ int i3c_master_do_daa(struct i3c_master_controller *master)
>>   }
>>   EXPORT_SYMBOL_GPL(i3c_master_do_daa);
>>
>> +/**
>> + * i3c_master_dma_map_single() - Map buffer for single DMA transfer
>> + * @dev: device object of a device doing DMA
>> + * @buf: destination/source buffer for DMA
>> + * @len: length of transfer
>> + * @need_bounce: true if buffer is not DMA safe and need a bounce buffer
>> + * @dir: DMA direction
>> + *
>> + * Map buffer for a DMA transfer and allocate a bounce buffer if required.
>> + *
>> + * Return: I3C DMA transfer descriptor or NULL in case of error.
>> + */
>> +struct i3c_dma *i3c_master_dma_map_single(struct device *dev, void *buf,
>> +	size_t len, bool need_bounce, enum dma_data_direction dir)
>> +{
>> +	struct i3c_dma *dma_xfer __free(kfree) = NULL;
>> +	void *bounce __free(kfree) = NULL;
>> +	void *dma_buf = buf;
>> +
>> +	dma_xfer = kzalloc(sizeof(*dma_xfer), GFP_KERNEL);
>> +	if (!dma_xfer)
>> +		return NULL;
> 
> nit: add empty line here
> 
>> +	dma_xfer->dev = dev;
>> +	dma_xfer->buf = buf;
>> +	dma_xfer->dir = dir;
>> +	dma_xfer->len = len;
>> +	dma_xfer->map_len = len;
>> +
>> +	if (is_vmalloc_addr(buf))
>> +		need_bounce = true;
>> +
>> +	if (need_bounce) {
>> +		dma_xfer->map_len = ALIGN(len, cache_line_size());
>> +		if (dir == DMA_FROM_DEVICE)
>> +			bounce = kzalloc(dma_xfer->map_len, GFP_KERNEL);
>> +		else
>> +			bounce = kmemdup(buf, dma_xfer->map_len, GFP_KERNEL);
>> +		if (!bounce)
>> +			return NULL;
>> +		dma_buf = bounce;
>> +	}
>> +
>> +	dma_xfer->addr = dma_map_single(dev, dma_buf, dma_xfer->map_len, dir);
>> +	if (dma_mapping_error(dev, dma_xfer->addr))
>> +		return NULL;
>> +
>> +	dma_xfer->bounce_buf = no_free_ptr(bounce);
>> +	return no_free_ptr(dma_xfer);
>> +}
>> +EXPORT_SYMBOL_GPL(i3c_master_dma_map_single);
>> +
>> +/**
>> + * i3c_master_dma_unmap_single() - Unmap buffer after DMA
>> + * @dma_xfer: DMA transfer and mapping descriptor
>> + *
>> + * Unmap buffer and cleanup DMA transfer descriptor.
>> + */
>> +void i3c_master_dma_unmap_single(struct i3c_dma *dma_xfer)
>> +{
>> +	struct i3c_dma *d __free(kfree) = dma_xfer;
>> +	void *bounce __free(kfree) = d->bounce_buf;
> 
> This simple free, call free(dma_xfer) after memcpy directly.
> 
I'm confused. So should I use __free() infra only in 
i3c_master_dma_map_single() and use explicit kfree() for dma_xfer and 
bounce_buf here?

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

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

[Jarkko Nikula]

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@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?

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

Re: [PATCH v2 3/4] i3c: mipi-i3c-hci: Use physical device pointer with DMA API

[Jarkko Nikula]

On 8/18/25 7:28 PM, Frank Li wrote:
> On Fri, Aug 15, 2025 at 05:12:41PM +0300, Jarkko Nikula wrote:
>> DMA transfer faults on Intel hardware when the IOMMU is enabled and
>> driver initialization will fail when attempting to do the first transfer:
>>
>> 	DMAR: DRHD: handling fault status reg 2
>> 	DMAR: [DMA Read NO_PASID] Request device [00:11.0] fault addr 0x676e3000 [fault reason 0x71] SM: Present bit in first-level paging entry is clear
>>   	i3c mipi-i3c-hci.0: ring 0: Transfer Aborted
>> 	mipi-i3c-hci mipi-i3c-hci.0: probe with driver mipi-i3c-hci failed with error -62
>>
>> Reason for this is that the IOMMU setup is done for the physical devices
>> only and not for the virtual I3C Controller device object.
>>
>> Therefore use the pointer to a physical device object with the DMA API.
>>
>> Due to a data corruption observation when the device DMA is IOMMU
>> mapped, a properly sized receive bounce buffer is required if transfer
>> length is not a multiple of DWORDs.
>>
>> Reported-by: Prabhakaran, Krishna <krishna.prabhakaran@intel.com>
>> Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
>> ---
>>   drivers/i3c/master/mipi-i3c-hci/dma.c | 46 +++++++++++++++++++--------
>>   1 file changed, 33 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
>> index 295671daae09..8cea91d4d3bf 100644
>> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
>> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
>> @@ -14,6 +14,7 @@
>>   #include <linux/errno.h>
>>   #include <linux/i3c/master.h>
>>   #include <linux/io.h>
>> +#include <linux/pci.h>
>>
>>   #include "hci.h"
>>   #include "cmd.h"
>> @@ -138,6 +139,7 @@ struct hci_rh_data {
>>   };
>>
>>   struct hci_rings_data {
>> +	struct device *sysdev;
>>   	unsigned int total;
>>   	struct hci_rh_data headers[] __counted_by(total);
>>   };
>> @@ -165,20 +167,20 @@ static void hci_dma_cleanup(struct i3c_hci *hci)
>>   		rh_reg_write(IBI_SETUP, 0);
>>
>>   		if (rh->xfer)
>> -			dma_free_coherent(&hci->master.dev,
>> +			dma_free_coherent(rings->sysdev,
>>   					  rh->xfer_struct_sz * rh->xfer_entries,
>>   					  rh->xfer, rh->xfer_dma);
>>   		if (rh->resp)
>> -			dma_free_coherent(&hci->master.dev,
>> +			dma_free_coherent(rings->sysdev,
>>   					  rh->resp_struct_sz * rh->xfer_entries,
>>   					  rh->resp, rh->resp_dma);
>>   		kfree(rh->src_xfers);
>>   		if (rh->ibi_status)
>> -			dma_free_coherent(&hci->master.dev,
>> +			dma_free_coherent(rings->sysdev,
>>   					  rh->ibi_status_sz * rh->ibi_status_entries,
>>   					  rh->ibi_status, rh->ibi_status_dma);
>>   		if (rh->ibi_data_dma)
>> -			dma_unmap_single(&hci->master.dev, rh->ibi_data_dma,
>> +			dma_unmap_single(rings->sysdev, rh->ibi_data_dma,
>>   					 rh->ibi_chunk_sz * rh->ibi_chunks_total,
>>   					 DMA_FROM_DEVICE);
>>   		kfree(rh->ibi_data);
>> @@ -194,11 +196,23 @@ static int hci_dma_init(struct i3c_hci *hci)
>>   {
>>   	struct hci_rings_data *rings;
>>   	struct hci_rh_data *rh;
>> +	struct device *sysdev;
>>   	u32 regval;
>>   	unsigned int i, nr_rings, xfers_sz, resps_sz;
>>   	unsigned int ibi_status_ring_sz, ibi_data_ring_sz;
>>   	int ret;
>>
>> +	/*
>> +	 * Set pointer to a physical device that does DMA and has IOMMU setup
>> +	 * done for it in case of enabled IOMMU and use it with the DMA API.
>> +	 * Here such device is either
>> +	 * "mipi-i3c-hci" platform device (OF/ACPI enumeration) parent or
>> +	 * grandparent (PCI enumeration).
>> +	 */
>> +	sysdev = hci->master.dev.parent;
>> +	if (sysdev->parent && dev_is_pci(sysdev->parent))
>> +		sysdev = sysdev->parent;
>> +
>>   	regval = rhs_reg_read(CONTROL);
>>   	nr_rings = FIELD_GET(MAX_HEADER_COUNT_CAP, regval);
>>   	dev_info(&hci->master.dev, "%d DMA rings available\n", nr_rings);
>> @@ -213,6 +227,7 @@ static int hci_dma_init(struct i3c_hci *hci)
>>   		return -ENOMEM;
>>   	hci->io_data = rings;
>>   	rings->total = nr_rings;
>> +	rings->sysdev = sysdev;
>>
>>   	regval = FIELD_PREP(MAX_HEADER_COUNT, rings->total);
>>   	rhs_reg_write(CONTROL, regval);
>> @@ -239,9 +254,9 @@ static int hci_dma_init(struct i3c_hci *hci)
>>   		xfers_sz = rh->xfer_struct_sz * rh->xfer_entries;
>>   		resps_sz = rh->resp_struct_sz * rh->xfer_entries;
>>
>> -		rh->xfer = dma_alloc_coherent(&hci->master.dev, xfers_sz,
>> +		rh->xfer = dma_alloc_coherent(rings->sysdev, xfers_sz,
>>   					      &rh->xfer_dma, GFP_KERNEL);
>> -		rh->resp = dma_alloc_coherent(&hci->master.dev, resps_sz,
>> +		rh->resp = dma_alloc_coherent(rings->sysdev, resps_sz,
>>   					      &rh->resp_dma, GFP_KERNEL);
>>   		rh->src_xfers =
>>   			kmalloc_array(rh->xfer_entries, sizeof(*rh->src_xfers),
>> @@ -295,16 +310,16 @@ static int hci_dma_init(struct i3c_hci *hci)
>>   		ibi_data_ring_sz = rh->ibi_chunk_sz * rh->ibi_chunks_total;
>>
>>   		rh->ibi_status =
>> -			dma_alloc_coherent(&hci->master.dev, ibi_status_ring_sz,
>> +			dma_alloc_coherent(rings->sysdev, ibi_status_ring_sz,
>>   					   &rh->ibi_status_dma, GFP_KERNEL);
>>   		rh->ibi_data = kmalloc(ibi_data_ring_sz, GFP_KERNEL);
>>   		ret = -ENOMEM;
>>   		if (!rh->ibi_status || !rh->ibi_data)
>>   			goto err_out;
>>   		rh->ibi_data_dma =
>> -			dma_map_single(&hci->master.dev, rh->ibi_data,
>> +			dma_map_single(rings->sysdev, rh->ibi_data,
>>   				       ibi_data_ring_sz, DMA_FROM_DEVICE);
>> -		if (dma_mapping_error(&hci->master.dev, rh->ibi_data_dma)) {
>> +		if (dma_mapping_error(rings->sysdev, rh->ibi_data_dma)) {
>>   			rh->ibi_data_dma = 0;
>>   			ret = -ENOMEM;
>>   			goto err_out;
>> @@ -369,6 +384,7 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
>>   		u32 *ring_data = rh->xfer + rh->xfer_struct_sz * enqueue_ptr;
>>   		enum dma_data_direction dir = xfer->rnw ? DMA_FROM_DEVICE :
>>   							  DMA_TO_DEVICE;
>> +		bool need_bounce;
>>
>>   		/* store cmd descriptor */
>>   		*ring_data++ = xfer->cmd_desc[0];
>> @@ -387,10 +403,13 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
>>
>>   		/* 2nd and 3rd words of Data Buffer Descriptor Structure */
>>   		if (xfer->data) {
>> -			xfer->dma = i3c_master_dma_map_single(&hci->master.dev,
>> +			need_bounce = device_iommu_mapped(rings->sysdev) &&
>> +				      xfer->rnw &&
> 
> why only read need bounce? if iommu have not map enough size, still fault
> for write.
> 
> such as you try to map IO space [0xffd,0x1000). when hardware try access
> 0x1000, iommu will capture this type error. Maybe it is hard to meet
> allocate io space at close end of page, you have not found this problem.
> 
Yes, only read has been problematic so far on our HW when the device is 
IOMMU mapped and write works fine with or without IOMMU. In my opinnion 
it's better to have another patch if such issue is seen on some other HW.

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

Re: [PATCH v2 3/4] i3c: mipi-i3c-hci: Use physical device pointer with DMA API

[Frank Li]

On Tue, Aug 19, 2025 at 09:27:04AM +0300, Jarkko Nikula wrote:
> On 8/18/25 7:28 PM, Frank Li wrote:
> > On Fri, Aug 15, 2025 at 05:12:41PM +0300, Jarkko Nikula wrote:
> > > DMA transfer faults on Intel hardware when the IOMMU is enabled and
> > > driver initialization will fail when attempting to do the first transfer:
> > >
> > > 	DMAR: DRHD: handling fault status reg 2
> > > 	DMAR: [DMA Read NO_PASID] Request device [00:11.0] fault addr 0x676e3000 [fault reason 0x71] SM: Present bit in first-level paging entry is clear
> > >   	i3c mipi-i3c-hci.0: ring 0: Transfer Aborted
> > > 	mipi-i3c-hci mipi-i3c-hci.0: probe with driver mipi-i3c-hci failed with error -62
> > >
> > > Reason for this is that the IOMMU setup is done for the physical devices
> > > only and not for the virtual I3C Controller device object.
> > >
> > > Therefore use the pointer to a physical device object with the DMA API.
> > >
> > > Due to a data corruption observation when the device DMA is IOMMU
> > > mapped, a properly sized receive bounce buffer is required if transfer
> > > length is not a multiple of DWORDs.
> > >
> > > Reported-by: Prabhakaran, Krishna <krishna.prabhakaran@intel.com>
> > > Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> > > ---
> > >   drivers/i3c/master/mipi-i3c-hci/dma.c | 46 +++++++++++++++++++--------
> > >   1 file changed, 33 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> > > index 295671daae09..8cea91d4d3bf 100644
> > > --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> > > +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> > > @@ -14,6 +14,7 @@
> > >   #include <linux/errno.h>
> > >   #include <linux/i3c/master.h>
> > >   #include <linux/io.h>
> > > +#include <linux/pci.h>
> > >
> > >   #include "hci.h"
> > >   #include "cmd.h"
> > > @@ -138,6 +139,7 @@ struct hci_rh_data {
> > >   };
> > >
> > >   struct hci_rings_data {
> > > +	struct device *sysdev;
> > >   	unsigned int total;
> > >   	struct hci_rh_data headers[] __counted_by(total);
> > >   };
> > > @@ -165,20 +167,20 @@ static void hci_dma_cleanup(struct i3c_hci *hci)
> > >   		rh_reg_write(IBI_SETUP, 0);
> > >
> > >   		if (rh->xfer)
> > > -			dma_free_coherent(&hci->master.dev,
> > > +			dma_free_coherent(rings->sysdev,
> > >   					  rh->xfer_struct_sz * rh->xfer_entries,
> > >   					  rh->xfer, rh->xfer_dma);
> > >   		if (rh->resp)
> > > -			dma_free_coherent(&hci->master.dev,
> > > +			dma_free_coherent(rings->sysdev,
> > >   					  rh->resp_struct_sz * rh->xfer_entries,
> > >   					  rh->resp, rh->resp_dma);
> > >   		kfree(rh->src_xfers);
> > >   		if (rh->ibi_status)
> > > -			dma_free_coherent(&hci->master.dev,
> > > +			dma_free_coherent(rings->sysdev,
> > >   					  rh->ibi_status_sz * rh->ibi_status_entries,
> > >   					  rh->ibi_status, rh->ibi_status_dma);
> > >   		if (rh->ibi_data_dma)
> > > -			dma_unmap_single(&hci->master.dev, rh->ibi_data_dma,
> > > +			dma_unmap_single(rings->sysdev, rh->ibi_data_dma,
> > >   					 rh->ibi_chunk_sz * rh->ibi_chunks_total,
> > >   					 DMA_FROM_DEVICE);
> > >   		kfree(rh->ibi_data);
> > > @@ -194,11 +196,23 @@ static int hci_dma_init(struct i3c_hci *hci)
> > >   {
> > >   	struct hci_rings_data *rings;
> > >   	struct hci_rh_data *rh;
> > > +	struct device *sysdev;
> > >   	u32 regval;
> > >   	unsigned int i, nr_rings, xfers_sz, resps_sz;
> > >   	unsigned int ibi_status_ring_sz, ibi_data_ring_sz;
> > >   	int ret;
> > >
> > > +	/*
> > > +	 * Set pointer to a physical device that does DMA and has IOMMU setup
> > > +	 * done for it in case of enabled IOMMU and use it with the DMA API.
> > > +	 * Here such device is either
> > > +	 * "mipi-i3c-hci" platform device (OF/ACPI enumeration) parent or
> > > +	 * grandparent (PCI enumeration).
> > > +	 */
> > > +	sysdev = hci->master.dev.parent;
> > > +	if (sysdev->parent && dev_is_pci(sysdev->parent))
> > > +		sysdev = sysdev->parent;
> > > +
> > >   	regval = rhs_reg_read(CONTROL);
> > >   	nr_rings = FIELD_GET(MAX_HEADER_COUNT_CAP, regval);
> > >   	dev_info(&hci->master.dev, "%d DMA rings available\n", nr_rings);
> > > @@ -213,6 +227,7 @@ static int hci_dma_init(struct i3c_hci *hci)
> > >   		return -ENOMEM;
> > >   	hci->io_data = rings;
> > >   	rings->total = nr_rings;
> > > +	rings->sysdev = sysdev;
> > >
> > >   	regval = FIELD_PREP(MAX_HEADER_COUNT, rings->total);
> > >   	rhs_reg_write(CONTROL, regval);
> > > @@ -239,9 +254,9 @@ static int hci_dma_init(struct i3c_hci *hci)
> > >   		xfers_sz = rh->xfer_struct_sz * rh->xfer_entries;
> > >   		resps_sz = rh->resp_struct_sz * rh->xfer_entries;
> > >
> > > -		rh->xfer = dma_alloc_coherent(&hci->master.dev, xfers_sz,
> > > +		rh->xfer = dma_alloc_coherent(rings->sysdev, xfers_sz,
> > >   					      &rh->xfer_dma, GFP_KERNEL);
> > > -		rh->resp = dma_alloc_coherent(&hci->master.dev, resps_sz,
> > > +		rh->resp = dma_alloc_coherent(rings->sysdev, resps_sz,
> > >   					      &rh->resp_dma, GFP_KERNEL);
> > >   		rh->src_xfers =
> > >   			kmalloc_array(rh->xfer_entries, sizeof(*rh->src_xfers),
> > > @@ -295,16 +310,16 @@ static int hci_dma_init(struct i3c_hci *hci)
> > >   		ibi_data_ring_sz = rh->ibi_chunk_sz * rh->ibi_chunks_total;
> > >
> > >   		rh->ibi_status =
> > > -			dma_alloc_coherent(&hci->master.dev, ibi_status_ring_sz,
> > > +			dma_alloc_coherent(rings->sysdev, ibi_status_ring_sz,
> > >   					   &rh->ibi_status_dma, GFP_KERNEL);
> > >   		rh->ibi_data = kmalloc(ibi_data_ring_sz, GFP_KERNEL);
> > >   		ret = -ENOMEM;
> > >   		if (!rh->ibi_status || !rh->ibi_data)
> > >   			goto err_out;
> > >   		rh->ibi_data_dma =
> > > -			dma_map_single(&hci->master.dev, rh->ibi_data,
> > > +			dma_map_single(rings->sysdev, rh->ibi_data,
> > >   				       ibi_data_ring_sz, DMA_FROM_DEVICE);
> > > -		if (dma_mapping_error(&hci->master.dev, rh->ibi_data_dma)) {
> > > +		if (dma_mapping_error(rings->sysdev, rh->ibi_data_dma)) {
> > >   			rh->ibi_data_dma = 0;
> > >   			ret = -ENOMEM;
> > >   			goto err_out;
> > > @@ -369,6 +384,7 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
> > >   		u32 *ring_data = rh->xfer + rh->xfer_struct_sz * enqueue_ptr;
> > >   		enum dma_data_direction dir = xfer->rnw ? DMA_FROM_DEVICE :
> > >   							  DMA_TO_DEVICE;
> > > +		bool need_bounce;
> > >
> > >   		/* store cmd descriptor */
> > >   		*ring_data++ = xfer->cmd_desc[0];
> > > @@ -387,10 +403,13 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
> > >
> > >   		/* 2nd and 3rd words of Data Buffer Descriptor Structure */
> > >   		if (xfer->data) {
> > > -			xfer->dma = i3c_master_dma_map_single(&hci->master.dev,
> > > +			need_bounce = device_iommu_mapped(rings->sysdev) &&
> > > +				      xfer->rnw &&
> >
> > why only read need bounce? if iommu have not map enough size, still fault
> > for write.
> >
> > such as you try to map IO space [0xffd,0x1000). when hardware try access
> > 0x1000, iommu will capture this type error. Maybe it is hard to meet
> > allocate io space at close end of page, you have not found this problem.
> >
> Yes, only read has been problematic so far on our HW when the device is
> IOMMU mapped and write works fine with or without IOMMU. In my opinnion it's
> better to have another patch if such issue is seen on some other HW.

Okay.

Frank
>
> --
> 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

Re: [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@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@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

Re: [PATCH v2 1/4] i3c: master: Add helpers for DMA mapping and bounce buffer handling

[Frank Li]

On Tue, Aug 19, 2025 at 09:08:36AM +0300, Jarkko Nikula wrote:
> On 8/18/25 7:07 PM, Frank Li wrote:
> > On Fri, Aug 15, 2025 at 05:12:39PM +0300, Jarkko Nikula wrote:
> > > Some I3C controllers such as MIPI I3C HCI may pad the last DWORD (32-bit)
> > > with stale data from the RX FIFO in DMA transfers if the receive length
> > > is not DWORD aligned and when the device DMA is IOMMU mapped.
> > >
> > > In such a case, a properly sized bounce buffer is required in order to
> > > avoid possible data corruption. In a review discussion, proposal was to
> > > have a common helpers in I3C core for DMA mapping and bounce buffer
> > > handling.
> > >
> > > Drivers may use the helper i3c_master_dma_map_single() to map a buffer
> > > for a DMA transfer. It internally allocates a bounce buffer if buffer is
> > > not DMA'able or when the driver requires it for a transfer.
> > >
> > > Helper i3c_master_dma_unmap_single() does the needed cleanups and
> > > data copying from the bounce buffer.
> > >
> > > Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> > > ---
> > >   drivers/i3c/master.c       | 69 ++++++++++++++++++++++++++++++++++++++
> > >   include/linux/i3c/master.h | 26 ++++++++++++++
> > >   2 files changed, 95 insertions(+)
> > >
> > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> > > index 2ef898a8fd80..497bff57248a 100644
> > > --- a/drivers/i3c/master.c
> > > +++ b/drivers/i3c/master.c
> > > @@ -8,6 +8,7 @@
> > >   #include <linux/atomic.h>
> > >   #include <linux/bug.h>
> > >   #include <linux/device.h>
> > > +#include <linux/dma-mapping.h>
> > >   #include <linux/err.h>
> > >   #include <linux/export.h>
> > >   #include <linux/kernel.h>
> > > @@ -1727,6 +1728,74 @@ int i3c_master_do_daa(struct i3c_master_controller *master)
> > >   }
> > >   EXPORT_SYMBOL_GPL(i3c_master_do_daa);
> > >
> > > +/**
> > > + * i3c_master_dma_map_single() - Map buffer for single DMA transfer
> > > + * @dev: device object of a device doing DMA
> > > + * @buf: destination/source buffer for DMA
> > > + * @len: length of transfer
> > > + * @need_bounce: true if buffer is not DMA safe and need a bounce buffer
> > > + * @dir: DMA direction
> > > + *
> > > + * Map buffer for a DMA transfer and allocate a bounce buffer if required.
> > > + *
> > > + * Return: I3C DMA transfer descriptor or NULL in case of error.
> > > + */
> > > +struct i3c_dma *i3c_master_dma_map_single(struct device *dev, void *buf,
> > > +	size_t len, bool need_bounce, enum dma_data_direction dir)
> > > +{
> > > +	struct i3c_dma *dma_xfer __free(kfree) = NULL;
> > > +	void *bounce __free(kfree) = NULL;
> > > +	void *dma_buf = buf;
> > > +
> > > +	dma_xfer = kzalloc(sizeof(*dma_xfer), GFP_KERNEL);
> > > +	if (!dma_xfer)
> > > +		return NULL;
> >
> > nit: add empty line here
> >
> > > +	dma_xfer->dev = dev;
> > > +	dma_xfer->buf = buf;
> > > +	dma_xfer->dir = dir;
> > > +	dma_xfer->len = len;
> > > +	dma_xfer->map_len = len;
> > > +
> > > +	if (is_vmalloc_addr(buf))
> > > +		need_bounce = true;
> > > +
> > > +	if (need_bounce) {
> > > +		dma_xfer->map_len = ALIGN(len, cache_line_size());
> > > +		if (dir == DMA_FROM_DEVICE)
> > > +			bounce = kzalloc(dma_xfer->map_len, GFP_KERNEL);
> > > +		else
> > > +			bounce = kmemdup(buf, dma_xfer->map_len, GFP_KERNEL);
> > > +		if (!bounce)
> > > +			return NULL;
> > > +		dma_buf = bounce;
> > > +	}
> > > +
> > > +	dma_xfer->addr = dma_map_single(dev, dma_buf, dma_xfer->map_len, dir);
> > > +	if (dma_mapping_error(dev, dma_xfer->addr))
> > > +		return NULL;
> > > +
> > > +	dma_xfer->bounce_buf = no_free_ptr(bounce);
> > > +	return no_free_ptr(dma_xfer);
> > > +}
> > > +EXPORT_SYMBOL_GPL(i3c_master_dma_map_single);
> > > +
> > > +/**
> > > + * i3c_master_dma_unmap_single() - Unmap buffer after DMA
> > > + * @dma_xfer: DMA transfer and mapping descriptor
> > > + *
> > > + * Unmap buffer and cleanup DMA transfer descriptor.
> > > + */
> > > +void i3c_master_dma_unmap_single(struct i3c_dma *dma_xfer)
> > > +{
> > > +	struct i3c_dma *d __free(kfree) = dma_xfer;
> > > +	void *bounce __free(kfree) = d->bounce_buf;
> >
> > This simple free, call free(dma_xfer) after memcpy directly.
> >
> I'm confused. So should I use __free() infra only in
> i3c_master_dma_map_single() and use explicit kfree() for dma_xfer and
> bounce_buf here?

See previous comments. __free() help reduce error handle. here just add
complex instead of simplify code.

Becuase mipi use build in dma, __free() have not help much, but it will
help much if use dmaegine because may dmaegine need check return value.

Frank

>
> --
> 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