[PATCH v3 0/4] Add ADSP and CDSP support on Kaanapali SoC

[PATCH v3 0/4] Add ADSP and CDSP support on Kaanapali SoC

[Kumari Pallavi]

Introduces support for new DSP IOVA formatting and hardware-specific
configuration required to enable ADSP and CDSP functionality on the
Kaanapali SoC.

Add support for a new IOVA formatting scheme by adding a sid_pos to the DSP
driver. Sid_pos standardizes the placement of the stream ID (SID) within the
physical address, which is required for DSPs to operate correctly on
Kaanapali. DSP currently supports 32-bit IOVA (32-bit PA + 4-bit SID) for
both Q6 and user DMA (uDMA) access.
This is being upgraded to 34-bit PA + 4-bit SID due to a hardware revision
in CDSP for Kaanapali SoC, which expands the DMA addressable range.
To support CDSP operation, this series updates the DMA mask configuration
to reflect the expanded DMA addressable range.

Patch [v2]:https://lore.kernel.org/all/20251015045702.3022060-1-kumari.pallavi@oss.qualcomm.com/

Changes in v3:
  - dt-bindings documentation update to support Kaanapali Soc
  - update comments to ensure clarity
  - Read SoC-specific data by matching the SoC’s .compatible field
    in the driver’s of_device_id match table instead of root node
  - Rename the dma_mask to the dma_bits for more clarity and set it's
    value based on the dsp_default_dma_bits instead of hardcode to 32

Kumari Pallavi (4):
  dt-bindings: misc: qcom,fastrpc: Add compatible for Kaanapali
  misc: fastrpc: Rename phys to dma_addr for clarity
  misc: fastrpc: Add support for new DSP IOVA formatting
  misc: fastrpc: Update dma_bits for CDSP support on Kaanapali SoC

 .../bindings/misc/qcom,fastrpc.yaml           |   5 +-
 drivers/misc/fastrpc.c                        | 127 ++++++++++++------
 2 files changed, 91 insertions(+), 41 deletions(-)

-- 
2.34.1

[PATCH v3 1/4] dt-bindings: misc: qcom,fastrpc: Add compatible for Kaanapali

[Kumari Pallavi]

Add a new compatible string "qcom,kaanapali-fastrpc" to support
for Kaanapali SoC.

Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml b/Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml
index 3f6199fc9ae6..13ba91fe1176 100644
--- a/Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml
+++ b/Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml
@@ -18,7 +18,10 @@ description: |
 
 properties:
   compatible:
-    const: qcom,fastrpc
+    items:
+    - enum:
+       - qcom,kaanapali-fastrpc
+       - qcom,fastrpc
 
   label:
     enum:
-- 
2.34.1

[PATCH v3 2/4] misc: fastrpc: Rename phys to dma_addr for clarity

[Kumari Pallavi]

Update all references of buf->phys and map->phys to buf->dma_addr and
map->dma_addr to accurately represent that these fields store DMA
addresses, not physical addresses. This change improves code clarity
and aligns with kernel conventions for dma_addr_t usage.

Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
---
 drivers/misc/fastrpc.c | 76 ++++++++++++++++++++++--------------------
 1 file changed, 40 insertions(+), 36 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index ee652ef01534..d6a7960fe716 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -106,7 +106,7 @@
 #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
 
 struct fastrpc_phy_page {
-	u64 addr;		/* physical address */
+	u64 addr;		/* physical or dma address */
 	u64 size;		/* size of contiguous region */
 };
 
@@ -171,7 +171,7 @@ struct fastrpc_msg {
 	u64 ctx;		/* invoke caller context */
 	u32 handle;	/* handle to invoke */
 	u32 sc;		/* scalars structure describing the data */
-	u64 addr;		/* physical address */
+	u64 addr;		/* physical or dma address */
 	u64 size;		/* size of contiguous region */
 };
 
@@ -194,7 +194,7 @@ struct fastrpc_buf {
 	struct dma_buf *dmabuf;
 	struct device *dev;
 	void *virt;
-	u64 phys;
+	u64 dma_addr;
 	u64 size;
 	/* Lock for dma buf attachments */
 	struct mutex lock;
@@ -217,7 +217,7 @@ struct fastrpc_map {
 	struct dma_buf *buf;
 	struct sg_table *table;
 	struct dma_buf_attachment *attach;
-	u64 phys;
+	u64 dma_addr;
 	u64 size;
 	void *va;
 	u64 len;
@@ -320,11 +320,12 @@ static void fastrpc_free_map(struct kref *ref)
 
 			perm.vmid = QCOM_SCM_VMID_HLOS;
 			perm.perm = QCOM_SCM_PERM_RWX;
-			err = qcom_scm_assign_mem(map->phys, map->len,
+			err = qcom_scm_assign_mem(map->dma_addr, map->len,
 				&src_perms, &perm, 1);
 			if (err) {
-				dev_err(map->fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d\n",
-						map->phys, map->len, err);
+				dev_err(map->fl->sctx->dev,
+					"Failed to assign memory dma_addr 0x%llx size 0x%llx err %d\n",
+					map->dma_addr, map->len, err);
 				return;
 			}
 		}
@@ -389,7 +390,7 @@ static int fastrpc_map_lookup(struct fastrpc_user *fl, int fd,
 static void fastrpc_buf_free(struct fastrpc_buf *buf)
 {
 	dma_free_coherent(buf->dev, buf->size, buf->virt,
-			  FASTRPC_PHYS(buf->phys));
+			  FASTRPC_PHYS(buf->dma_addr));
 	kfree(buf);
 }
 
@@ -408,12 +409,12 @@ static int __fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
 
 	buf->fl = fl;
 	buf->virt = NULL;
-	buf->phys = 0;
+	buf->dma_addr = 0;
 	buf->size = size;
 	buf->dev = dev;
 	buf->raddr = 0;
 
-	buf->virt = dma_alloc_coherent(dev, buf->size, (dma_addr_t *)&buf->phys,
+	buf->virt = dma_alloc_coherent(dev, buf->size, (dma_addr_t *)&buf->dma_addr,
 				       GFP_KERNEL);
 	if (!buf->virt) {
 		mutex_destroy(&buf->lock);
@@ -439,7 +440,7 @@ static int fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
 	buf = *obuf;
 
 	if (fl->sctx && fl->sctx->sid)
-		buf->phys += ((u64)fl->sctx->sid << 32);
+		buf->dma_addr += ((u64)fl->sctx->sid << 32);
 
 	return 0;
 }
@@ -684,7 +685,7 @@ static int fastrpc_dma_buf_attach(struct dma_buf *dmabuf,
 		return -ENOMEM;
 
 	ret = dma_get_sgtable(buffer->dev, &a->sgt, buffer->virt,
-			      FASTRPC_PHYS(buffer->phys), buffer->size);
+			      FASTRPC_PHYS(buffer->dma_addr), buffer->size);
 	if (ret < 0) {
 		dev_err(buffer->dev, "failed to get scatterlist from DMA API\n");
 		kfree(a);
@@ -733,7 +734,7 @@ static int fastrpc_mmap(struct dma_buf *dmabuf,
 	dma_resv_assert_held(dmabuf->resv);
 
 	return dma_mmap_coherent(buf->dev, vma, buf->virt,
-				 FASTRPC_PHYS(buf->phys), size);
+				 FASTRPC_PHYS(buf->dma_addr), size);
 }
 
 static const struct dma_buf_ops fastrpc_dma_buf_ops = {
@@ -785,10 +786,10 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
 	map->table = table;
 
 	if (attr & FASTRPC_ATTR_SECUREMAP) {
-		map->phys = sg_phys(map->table->sgl);
+		map->dma_addr = sg_phys(map->table->sgl);
 	} else {
-		map->phys = sg_dma_address(map->table->sgl);
-		map->phys += ((u64)fl->sctx->sid << 32);
+		map->dma_addr = sg_dma_address(map->table->sgl);
+		map->dma_addr += ((u64)fl->sctx->sid << 32);
 	}
 	for_each_sg(map->table->sgl, sgl, map->table->nents,
 		sgl_index)
@@ -815,10 +816,11 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
 		dst_perms[1].vmid = fl->cctx->vmperms[0].vmid;
 		dst_perms[1].perm = QCOM_SCM_PERM_RWX;
 		map->attr = attr;
-		err = qcom_scm_assign_mem(map->phys, (u64)map->len, &src_perms, dst_perms, 2);
+		err = qcom_scm_assign_mem(map->dma_addr, (u64)map->len, &src_perms, dst_perms, 2);
 		if (err) {
-			dev_err(sess->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d\n",
-					map->phys, map->len, err);
+			dev_err(sess->dev,
+				"Failed to assign memory with dma_addr 0x%llx size 0x%llx err %d\n",
+				map->dma_addr, map->len, err);
 			goto map_err;
 		}
 	}
@@ -1009,7 +1011,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
 			struct vm_area_struct *vma = NULL;
 
 			rpra[i].buf.pv = (u64) ctx->args[i].ptr;
-			pages[i].addr = ctx->maps[i]->phys;
+			pages[i].addr = ctx->maps[i]->dma_addr;
 
 			mmap_read_lock(current->mm);
 			vma = find_vma(current->mm, ctx->args[i].ptr);
@@ -1036,7 +1038,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
 				goto bail;
 
 			rpra[i].buf.pv = args - ctx->olaps[oix].offset;
-			pages[i].addr = ctx->buf->phys -
+			pages[i].addr = ctx->buf->dma_addr -
 					ctx->olaps[oix].offset +
 					(pkt_size - rlen);
 			pages[i].addr = pages[i].addr &	PAGE_MASK;
@@ -1068,7 +1070,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
 		list[i].num = ctx->args[i].length ? 1 : 0;
 		list[i].pgidx = i;
 		if (ctx->maps[i]) {
-			pages[i].addr = ctx->maps[i]->phys;
+			pages[i].addr = ctx->maps[i]->dma_addr;
 			pages[i].size = ctx->maps[i]->size;
 		}
 		rpra[i].dma.fd = ctx->args[i].fd;
@@ -1150,7 +1152,7 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx *sctx,
 	msg->ctx = ctx->ctxid | fl->pd;
 	msg->handle = handle;
 	msg->sc = ctx->sc;
-	msg->addr = ctx->buf ? ctx->buf->phys : 0;
+	msg->addr = ctx->buf ? ctx->buf->dma_addr : 0;
 	msg->size = roundup(ctx->msg_sz, PAGE_SIZE);
 	fastrpc_context_get(ctx);
 
@@ -1306,13 +1308,14 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
 		if (fl->cctx->vmcount) {
 			u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
 
-			err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
+			err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
 							(u64)fl->cctx->remote_heap->size,
 							&src_perms,
 							fl->cctx->vmperms, fl->cctx->vmcount);
 			if (err) {
-				dev_err(fl->sctx->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d\n",
-					fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
+				dev_err(fl->sctx->dev,
+					"Failed to assign memory with dma_addr 0x%llx size 0x%llx err %d\n",
+					fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
 				goto err_map;
 			}
 			scm_done = true;
@@ -1332,7 +1335,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
 	args[1].length = inbuf.namelen;
 	args[1].fd = -1;
 
-	pages[0].addr = fl->cctx->remote_heap->phys;
+	pages[0].addr = fl->cctx->remote_heap->dma_addr;
 	pages[0].size = fl->cctx->remote_heap->size;
 
 	args[2].ptr = (u64)(uintptr_t) pages;
@@ -1361,12 +1364,12 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
 
 		dst_perms.vmid = QCOM_SCM_VMID_HLOS;
 		dst_perms.perm = QCOM_SCM_PERM_RWX;
-		err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
+		err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
 						(u64)fl->cctx->remote_heap->size,
 						&src_perms, &dst_perms, 1);
 		if (err)
-			dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d\n",
-				fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
+			dev_err(fl->sctx->dev, "Failed to assign memory dma_addr 0x%llx size 0x%llx err %d\n",
+				fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
 	}
 err_map:
 	fastrpc_buf_free(fl->cctx->remote_heap);
@@ -1455,7 +1458,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl,
 	args[2].length = inbuf.filelen;
 	args[2].fd = init.filefd;
 
-	pages[0].addr = imem->phys;
+	pages[0].addr = imem->dma_addr;
 	pages[0].size = imem->size;
 
 	args[3].ptr = (u64)(uintptr_t) pages;
@@ -1913,7 +1916,7 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
 	args[0].ptr = (u64) (uintptr_t) &req_msg;
 	args[0].length = sizeof(req_msg);
 
-	pages.addr = buf->phys;
+	pages.addr = buf->dma_addr;
 	pages.size = buf->size;
 
 	args[1].ptr = (u64) (uintptr_t) &pages;
@@ -1941,11 +1944,12 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
 	if (req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR && fl->cctx->vmcount) {
 		u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
 
-		err = qcom_scm_assign_mem(buf->phys, (u64)buf->size,
+		err = qcom_scm_assign_mem(buf->dma_addr, (u64)buf->size,
 			&src_perms, fl->cctx->vmperms, fl->cctx->vmcount);
 		if (err) {
-			dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d",
-					buf->phys, buf->size, err);
+			dev_err(fl->sctx->dev,
+				"Failed to assign memory dma_addr 0x%llx size 0x%llx err %d",
+				buf->dma_addr, buf->size, err);
 			goto err_assign;
 		}
 	}
@@ -2059,7 +2063,7 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp)
 	args[0].ptr = (u64) (uintptr_t) &req_msg;
 	args[0].length = sizeof(req_msg);
 
-	pages.addr = map->phys;
+	pages.addr = map->dma_addr;
 	pages.size = map->len;
 
 	args[1].ptr = (u64) (uintptr_t) &pages;
-- 
2.34.1

[PATCH v3 3/4] misc: fastrpc: Add support for new DSP IOVA formatting

[Kumari Pallavi]

Implement the new IOVA formatting required by the DSP architecture change
on Kaanapali SoC. Place the SID for DSP DMA transactions at bit 56 in the
physical address. This placement is necessary for the DSPs to correctly
identify streams and operate as intended.
To address this, set SID position to bit 56 via OF matching on the fastrpc
node; otherwise, default to legacy 32-bit placement.
This change ensures consistent SID placement across DSPs.

Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
---
 drivers/misc/fastrpc.c | 46 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 39 insertions(+), 7 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index d6a7960fe716..bcf3c7f8d3e9 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -33,7 +33,6 @@
 #define FASTRPC_ALIGN		128
 #define FASTRPC_MAX_FDLIST	16
 #define FASTRPC_MAX_CRCLIST	64
-#define FASTRPC_PHYS(p)	((p) & 0xffffffff)
 #define FASTRPC_CTX_MAX (256)
 #define FASTRPC_INIT_HANDLE	1
 #define FASTRPC_DSP_UTILITIES_HANDLE	2
@@ -105,6 +104,15 @@
 
 #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
 
+/* Extract smmu pa from consolidated iova */
+#define IPA_TO_DMA_ADDR(iova, sid_pos) (iova & ((1ULL << sid_pos) - 1ULL))
+/*
+ * Prepare the consolidated iova to send to dsp by prepending the sid
+ * to smmu pa at the appropriate position
+ */
+#define IOVA_FROM_SID_PA(sid, phys, sid_pos) \
+       (phys += sid << sid_pos)
+
 struct fastrpc_phy_page {
 	u64 addr;		/* physical or dma address */
 	u64 size;		/* size of contiguous region */
@@ -257,6 +265,10 @@ struct fastrpc_session_ctx {
 	bool valid;
 };
 
+struct fastrpc_soc_data {
+	u32 sid_pos;
+};
+
 struct fastrpc_channel_ctx {
 	int domain_id;
 	int sesscount;
@@ -278,6 +290,7 @@ struct fastrpc_channel_ctx {
 	bool secure;
 	bool unsigned_support;
 	u64 dma_mask;
+	const struct fastrpc_soc_data *soc_data;
 };
 
 struct fastrpc_device {
@@ -390,7 +403,7 @@ static int fastrpc_map_lookup(struct fastrpc_user *fl, int fd,
 static void fastrpc_buf_free(struct fastrpc_buf *buf)
 {
 	dma_free_coherent(buf->dev, buf->size, buf->virt,
-			  FASTRPC_PHYS(buf->dma_addr));
+			  IPA_TO_DMA_ADDR(buf->dma_addr, buf->fl->cctx->soc_data->sid_pos));
 	kfree(buf);
 }
 
@@ -440,7 +453,8 @@ static int fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
 	buf = *obuf;
 
 	if (fl->sctx && fl->sctx->sid)
-		buf->dma_addr += ((u64)fl->sctx->sid << 32);
+		IOVA_FROM_SID_PA((u64)fl->sctx->sid, buf->dma_addr,
+				 fl->cctx->soc_data->sid_pos);
 
 	return 0;
 }
@@ -685,7 +699,8 @@ static int fastrpc_dma_buf_attach(struct dma_buf *dmabuf,
 		return -ENOMEM;
 
 	ret = dma_get_sgtable(buffer->dev, &a->sgt, buffer->virt,
-			      FASTRPC_PHYS(buffer->dma_addr), buffer->size);
+			      IPA_TO_DMA_ADDR(buffer->dma_addr,
+			      buffer->fl->cctx->soc_data->sid_pos), buffer->size);
 	if (ret < 0) {
 		dev_err(buffer->dev, "failed to get scatterlist from DMA API\n");
 		kfree(a);
@@ -734,7 +749,8 @@ static int fastrpc_mmap(struct dma_buf *dmabuf,
 	dma_resv_assert_held(dmabuf->resv);
 
 	return dma_mmap_coherent(buf->dev, vma, buf->virt,
-				 FASTRPC_PHYS(buf->dma_addr), size);
+				 IPA_TO_DMA_ADDR(buf->dma_addr,
+				 buf->fl->cctx->soc_data->sid_pos), size);
 }
 
 static const struct dma_buf_ops fastrpc_dma_buf_ops = {
@@ -789,7 +805,8 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
 		map->dma_addr = sg_phys(map->table->sgl);
 	} else {
 		map->dma_addr = sg_dma_address(map->table->sgl);
-		map->dma_addr += ((u64)fl->sctx->sid << 32);
+		IOVA_FROM_SID_PA((u64)fl->sctx->sid,
+				 map->dma_addr, fl->cctx->soc_data->sid_pos);
 	}
 	for_each_sg(map->table->sgl, sgl, map->table->nents,
 		sgl_index)
@@ -2289,6 +2306,14 @@ static int fastrpc_get_domain_id(const char *domain)
 	return -EINVAL;
 }
 
+static const struct fastrpc_soc_data kaanapali_soc_data = {
+	.sid_pos = 56,
+};
+
+static const struct fastrpc_soc_data default_soc_data = {
+	.sid_pos = 32,
+};
+
 static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
 {
 	struct device *rdev = &rpdev->dev;
@@ -2297,6 +2322,11 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
 	const char *domain;
 	bool secure_dsp;
 	unsigned int vmids[FASTRPC_MAX_VMIDS];
+	const struct fastrpc_soc_data *soc_data = NULL;
+
+	soc_data = device_get_match_data(rdev);
+	if (!soc_data)
+		soc_data = &default_soc_data;
 
 	err = of_property_read_string(rdev->of_node, "label", &domain);
 	if (err) {
@@ -2349,6 +2379,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
 
 	secure_dsp = !(of_property_read_bool(rdev->of_node, "qcom,non-secure-domain"));
 	data->secure = secure_dsp;
+	data->soc_data = soc_data;
 
 	switch (domain_id) {
 	case ADSP_DOMAIN_ID:
@@ -2486,7 +2517,8 @@ static int fastrpc_rpmsg_callback(struct rpmsg_device *rpdev, void *data,
 }
 
 static const struct of_device_id fastrpc_rpmsg_of_match[] = {
-	{ .compatible = "qcom,fastrpc" },
+	{ .compatible = "qcom,kaanapali-fastrpc", .data = &kaanapali_soc_data },
+	{ .compatible = "qcom,fastrpc", .data = &default_soc_data },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, fastrpc_rpmsg_of_match);
-- 
2.34.1

[PATCH v3 4/4] misc: fastrpc: Update dma_bits for CDSP support on Kaanapali SoC

[Kumari Pallavi]

DSP currently supports 32-bit IOVA (32-bit PA + 4-bit SID) for
both Q6 and user DMA (uDMA) access. This is being upgraded to
34-bit PA + 4-bit SID due to a hardware revision in CDSP for
Kaanapali SoC, which expands the DMA addressable range.
Update DMA bits configuration in the driver to support CDSP on
Kaanapali SoC. Set the default `dma_bits` to 32-bit and update
it to 34-bit based on CDSP and OF matching on the fastrpc node.

Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
---
 drivers/misc/fastrpc.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index bcf3c7f8d3e9..2eb8d37cd9b4 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -267,6 +267,8 @@ struct fastrpc_session_ctx {
 
 struct fastrpc_soc_data {
 	u32 sid_pos;
+	u32 cdsp_dma_bits;
+	u32 dsp_default_dma_bits;
 };
 
 struct fastrpc_channel_ctx {
@@ -2186,6 +2188,7 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
 	int i, sessions = 0;
 	unsigned long flags;
 	int rc;
+	u32 dma_bits;
 
 	cctx = dev_get_drvdata(dev->parent);
 	if (!cctx)
@@ -2199,12 +2202,16 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
 		spin_unlock_irqrestore(&cctx->lock, flags);
 		return -ENOSPC;
 	}
+	dma_bits = cctx->soc_data->dsp_default_dma_bits;
 	sess = &cctx->session[cctx->sesscount++];
 	sess->used = false;
 	sess->valid = true;
 	sess->dev = dev;
 	dev_set_drvdata(dev, sess);
 
+	if (cctx->domain_id == CDSP_DOMAIN_ID)
+		dma_bits = cctx->soc_data->cdsp_dma_bits;
+
 	if (of_property_read_u32(dev->of_node, "reg", &sess->sid))
 		dev_info(dev, "FastRPC Session ID not specified in DT\n");
 
@@ -2219,9 +2226,9 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
 		}
 	}
 	spin_unlock_irqrestore(&cctx->lock, flags);
-	rc = dma_set_mask(dev, DMA_BIT_MASK(32));
+	rc = dma_set_mask(dev, DMA_BIT_MASK(dma_bits));
 	if (rc) {
-		dev_err(dev, "32-bit DMA enable failed\n");
+		dev_err(dev, "%u-bit DMA enable failed\n", dma_bits);
 		return rc;
 	}
 
@@ -2308,10 +2315,14 @@ static int fastrpc_get_domain_id(const char *domain)
 
 static const struct fastrpc_soc_data kaanapali_soc_data = {
 	.sid_pos = 56,
+	.cdsp_dma_bits = 34,
+	.dsp_default_dma_bits = 32,
 };
 
 static const struct fastrpc_soc_data default_soc_data = {
 	.sid_pos = 32,
+	.cdsp_dma_bits = 32,
+	.dsp_default_dma_bits = 32,
 };
 
 static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
-- 
2.34.1

Re: [PATCH v3 2/4] misc: fastrpc: Rename phys to dma_addr for clarity

[Dmitry Baryshkov]

On Fri, Nov 14, 2025 at 02:11:40PM +0530, Kumari Pallavi wrote:
> Update all references of buf->phys and map->phys to buf->dma_addr and
> map->dma_addr to accurately represent that these fields store DMA
> addresses, not physical addresses. This change improves code clarity
> and aligns with kernel conventions for dma_addr_t usage.

Why do you mention dma_addr_t here?

> 
> Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
> ---
>  drivers/misc/fastrpc.c | 76 ++++++++++++++++++++++--------------------
>  1 file changed, 40 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index ee652ef01534..d6a7960fe716 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -106,7 +106,7 @@
>  #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
>  
>  struct fastrpc_phy_page {
> -	u64 addr;		/* physical address */
> +	u64 addr;		/* physical or dma address */

What is the difference here? Aren't all of them DMA addresses?

>  	u64 size;		/* size of contiguous region */
>  };
>  
> @@ -171,7 +171,7 @@ struct fastrpc_msg {
>  	u64 ctx;		/* invoke caller context */
>  	u32 handle;	/* handle to invoke */
>  	u32 sc;		/* scalars structure describing the data */
> -	u64 addr;		/* physical address */
> +	u64 addr;		/* physical or dma address */
>  	u64 size;		/* size of contiguous region */
>  };
>  
> @@ -194,7 +194,7 @@ struct fastrpc_buf {
>  	struct dma_buf *dmabuf;
>  	struct device *dev;
>  	void *virt;
> -	u64 phys;
> +	u64 dma_addr;
>  	u64 size;
>  	/* Lock for dma buf attachments */
>  	struct mutex lock;
> @@ -217,7 +217,7 @@ struct fastrpc_map {
>  	struct dma_buf *buf;
>  	struct sg_table *table;
>  	struct dma_buf_attachment *attach;
> -	u64 phys;
> +	u64 dma_addr;
>  	u64 size;
>  	void *va;
>  	u64 len;
> @@ -320,11 +320,12 @@ static void fastrpc_free_map(struct kref *ref)
>  
>  			perm.vmid = QCOM_SCM_VMID_HLOS;
>  			perm.perm = QCOM_SCM_PERM_RWX;
> -			err = qcom_scm_assign_mem(map->phys, map->len,
> +			err = qcom_scm_assign_mem(map->dma_addr, map->len,
>  				&src_perms, &perm, 1);
>  			if (err) {
> -				dev_err(map->fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d\n",
> -						map->phys, map->len, err);
> +				dev_err(map->fl->sctx->dev,
> +					"Failed to assign memory dma_addr 0x%llx size 0x%llx err %d\n",
> +					map->dma_addr, map->len, err);
>  				return;
>  			}
>  		}
> @@ -389,7 +390,7 @@ static int fastrpc_map_lookup(struct fastrpc_user *fl, int fd,
>  static void fastrpc_buf_free(struct fastrpc_buf *buf)
>  {
>  	dma_free_coherent(buf->dev, buf->size, buf->virt,
> -			  FASTRPC_PHYS(buf->phys));
> +			  FASTRPC_PHYS(buf->dma_addr));
>  	kfree(buf);
>  }
>  
> @@ -408,12 +409,12 @@ static int __fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
>  
>  	buf->fl = fl;
>  	buf->virt = NULL;
> -	buf->phys = 0;
> +	buf->dma_addr = 0;
>  	buf->size = size;
>  	buf->dev = dev;
>  	buf->raddr = 0;
>  
> -	buf->virt = dma_alloc_coherent(dev, buf->size, (dma_addr_t *)&buf->phys,
> +	buf->virt = dma_alloc_coherent(dev, buf->size, (dma_addr_t *)&buf->dma_addr,
>  				       GFP_KERNEL);
>  	if (!buf->virt) {
>  		mutex_destroy(&buf->lock);
> @@ -439,7 +440,7 @@ static int fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
>  	buf = *obuf;
>  
>  	if (fl->sctx && fl->sctx->sid)
> -		buf->phys += ((u64)fl->sctx->sid << 32);
> +		buf->dma_addr += ((u64)fl->sctx->sid << 32);
>  
>  	return 0;
>  }
> @@ -684,7 +685,7 @@ static int fastrpc_dma_buf_attach(struct dma_buf *dmabuf,
>  		return -ENOMEM;
>  
>  	ret = dma_get_sgtable(buffer->dev, &a->sgt, buffer->virt,
> -			      FASTRPC_PHYS(buffer->phys), buffer->size);
> +			      FASTRPC_PHYS(buffer->dma_addr), buffer->size);
>  	if (ret < 0) {
>  		dev_err(buffer->dev, "failed to get scatterlist from DMA API\n");
>  		kfree(a);
> @@ -733,7 +734,7 @@ static int fastrpc_mmap(struct dma_buf *dmabuf,
>  	dma_resv_assert_held(dmabuf->resv);
>  
>  	return dma_mmap_coherent(buf->dev, vma, buf->virt,
> -				 FASTRPC_PHYS(buf->phys), size);
> +				 FASTRPC_PHYS(buf->dma_addr), size);
>  }
>  
>  static const struct dma_buf_ops fastrpc_dma_buf_ops = {
> @@ -785,10 +786,10 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
>  	map->table = table;
>  
>  	if (attr & FASTRPC_ATTR_SECUREMAP) {
> -		map->phys = sg_phys(map->table->sgl);
> +		map->dma_addr = sg_phys(map->table->sgl);
>  	} else {
> -		map->phys = sg_dma_address(map->table->sgl);
> -		map->phys += ((u64)fl->sctx->sid << 32);
> +		map->dma_addr = sg_dma_address(map->table->sgl);
> +		map->dma_addr += ((u64)fl->sctx->sid << 32);
>  	}
>  	for_each_sg(map->table->sgl, sgl, map->table->nents,
>  		sgl_index)
> @@ -815,10 +816,11 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
>  		dst_perms[1].vmid = fl->cctx->vmperms[0].vmid;
>  		dst_perms[1].perm = QCOM_SCM_PERM_RWX;
>  		map->attr = attr;
> -		err = qcom_scm_assign_mem(map->phys, (u64)map->len, &src_perms, dst_perms, 2);
> +		err = qcom_scm_assign_mem(map->dma_addr, (u64)map->len, &src_perms, dst_perms, 2);
>  		if (err) {
> -			dev_err(sess->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d\n",
> -					map->phys, map->len, err);
> +			dev_err(sess->dev,
> +				"Failed to assign memory with dma_addr 0x%llx size 0x%llx err %d\n",
> +				map->dma_addr, map->len, err);
>  			goto map_err;
>  		}
>  	}
> @@ -1009,7 +1011,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
>  			struct vm_area_struct *vma = NULL;
>  
>  			rpra[i].buf.pv = (u64) ctx->args[i].ptr;
> -			pages[i].addr = ctx->maps[i]->phys;
> +			pages[i].addr = ctx->maps[i]->dma_addr;
>  
>  			mmap_read_lock(current->mm);
>  			vma = find_vma(current->mm, ctx->args[i].ptr);
> @@ -1036,7 +1038,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
>  				goto bail;
>  
>  			rpra[i].buf.pv = args - ctx->olaps[oix].offset;
> -			pages[i].addr = ctx->buf->phys -
> +			pages[i].addr = ctx->buf->dma_addr -
>  					ctx->olaps[oix].offset +
>  					(pkt_size - rlen);
>  			pages[i].addr = pages[i].addr &	PAGE_MASK;
> @@ -1068,7 +1070,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
>  		list[i].num = ctx->args[i].length ? 1 : 0;
>  		list[i].pgidx = i;
>  		if (ctx->maps[i]) {
> -			pages[i].addr = ctx->maps[i]->phys;
> +			pages[i].addr = ctx->maps[i]->dma_addr;
>  			pages[i].size = ctx->maps[i]->size;
>  		}
>  		rpra[i].dma.fd = ctx->args[i].fd;
> @@ -1150,7 +1152,7 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx *sctx,
>  	msg->ctx = ctx->ctxid | fl->pd;
>  	msg->handle = handle;
>  	msg->sc = ctx->sc;
> -	msg->addr = ctx->buf ? ctx->buf->phys : 0;
> +	msg->addr = ctx->buf ? ctx->buf->dma_addr : 0;
>  	msg->size = roundup(ctx->msg_sz, PAGE_SIZE);
>  	fastrpc_context_get(ctx);
>  
> @@ -1306,13 +1308,14 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>  		if (fl->cctx->vmcount) {
>  			u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
>  
> -			err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
> +			err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
>  							(u64)fl->cctx->remote_heap->size,
>  							&src_perms,
>  							fl->cctx->vmperms, fl->cctx->vmcount);
>  			if (err) {
> -				dev_err(fl->sctx->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d\n",
> -					fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
> +				dev_err(fl->sctx->dev,
> +					"Failed to assign memory with dma_addr 0x%llx size 0x%llx err %d\n",
> +					fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
>  				goto err_map;
>  			}
>  			scm_done = true;
> @@ -1332,7 +1335,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>  	args[1].length = inbuf.namelen;
>  	args[1].fd = -1;
>  
> -	pages[0].addr = fl->cctx->remote_heap->phys;
> +	pages[0].addr = fl->cctx->remote_heap->dma_addr;
>  	pages[0].size = fl->cctx->remote_heap->size;
>  
>  	args[2].ptr = (u64)(uintptr_t) pages;
> @@ -1361,12 +1364,12 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>  
>  		dst_perms.vmid = QCOM_SCM_VMID_HLOS;
>  		dst_perms.perm = QCOM_SCM_PERM_RWX;
> -		err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
> +		err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
>  						(u64)fl->cctx->remote_heap->size,
>  						&src_perms, &dst_perms, 1);
>  		if (err)
> -			dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d\n",
> -				fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
> +			dev_err(fl->sctx->dev, "Failed to assign memory dma_addr 0x%llx size 0x%llx err %d\n",
> +				fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
>  	}
>  err_map:
>  	fastrpc_buf_free(fl->cctx->remote_heap);
> @@ -1455,7 +1458,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl,
>  	args[2].length = inbuf.filelen;
>  	args[2].fd = init.filefd;
>  
> -	pages[0].addr = imem->phys;
> +	pages[0].addr = imem->dma_addr;
>  	pages[0].size = imem->size;
>  
>  	args[3].ptr = (u64)(uintptr_t) pages;
> @@ -1913,7 +1916,7 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
>  	args[0].ptr = (u64) (uintptr_t) &req_msg;
>  	args[0].length = sizeof(req_msg);
>  
> -	pages.addr = buf->phys;
> +	pages.addr = buf->dma_addr;
>  	pages.size = buf->size;
>  
>  	args[1].ptr = (u64) (uintptr_t) &pages;
> @@ -1941,11 +1944,12 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
>  	if (req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR && fl->cctx->vmcount) {
>  		u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
>  
> -		err = qcom_scm_assign_mem(buf->phys, (u64)buf->size,
> +		err = qcom_scm_assign_mem(buf->dma_addr, (u64)buf->size,
>  			&src_perms, fl->cctx->vmperms, fl->cctx->vmcount);
>  		if (err) {
> -			dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d",
> -					buf->phys, buf->size, err);
> +			dev_err(fl->sctx->dev,
> +				"Failed to assign memory dma_addr 0x%llx size 0x%llx err %d",
> +				buf->dma_addr, buf->size, err);
>  			goto err_assign;
>  		}
>  	}
> @@ -2059,7 +2063,7 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp)
>  	args[0].ptr = (u64) (uintptr_t) &req_msg;
>  	args[0].length = sizeof(req_msg);
>  
> -	pages.addr = map->phys;
> +	pages.addr = map->dma_addr;
>  	pages.size = map->len;
>  
>  	args[1].ptr = (u64) (uintptr_t) &pages;
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v3 2/4] misc: fastrpc: Rename phys to dma_addr for clarity

[Bjorn Andersson]

On Fri, Nov 14, 2025 at 02:11:40PM +0530, Kumari Pallavi wrote:
> Update all references of buf->phys and map->phys to buf->dma_addr and
> map->dma_addr to accurately represent that these fields store DMA
> addresses, not physical addresses. This change improves code clarity
> and aligns with kernel conventions for dma_addr_t usage.
> 
> Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
> ---
>  drivers/misc/fastrpc.c | 76 ++++++++++++++++++++++--------------------
>  1 file changed, 40 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index ee652ef01534..d6a7960fe716 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -106,7 +106,7 @@
>  #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
>  
>  struct fastrpc_phy_page {
> -	u64 addr;		/* physical address */
> +	u64 addr;		/* physical or dma address */
>  	u64 size;		/* size of contiguous region */
>  };
>  
> @@ -171,7 +171,7 @@ struct fastrpc_msg {
>  	u64 ctx;		/* invoke caller context */
>  	u32 handle;	/* handle to invoke */
>  	u32 sc;		/* scalars structure describing the data */
> -	u64 addr;		/* physical address */
> +	u64 addr;		/* physical or dma address */

Can you go all the way and make the type dma_addr_t? That way you don't
need to typecast the dma_alloc_coherent() and dma_free_coherent().

I believe it might complicate the places where you do math on it, but
that is a good thing, as it highlights those places where you do
something unexpected.

>  	u64 size;		/* size of contiguous region */
>  };
>  
> @@ -194,7 +194,7 @@ struct fastrpc_buf {
>  	struct dma_buf *dmabuf;
>  	struct device *dev;
>  	void *virt;
> -	u64 phys;
> +	u64 dma_addr;
>  	u64 size;
>  	/* Lock for dma buf attachments */
>  	struct mutex lock;
> @@ -217,7 +217,7 @@ struct fastrpc_map {
>  	struct dma_buf *buf;
>  	struct sg_table *table;
>  	struct dma_buf_attachment *attach;
> -	u64 phys;
> +	u64 dma_addr;
>  	u64 size;
>  	void *va;
>  	u64 len;
> @@ -320,11 +320,12 @@ static void fastrpc_free_map(struct kref *ref)
>  
>  			perm.vmid = QCOM_SCM_VMID_HLOS;
>  			perm.perm = QCOM_SCM_PERM_RWX;
> -			err = qcom_scm_assign_mem(map->phys, map->len,
> +			err = qcom_scm_assign_mem(map->dma_addr, map->len,
>  				&src_perms, &perm, 1);
>  			if (err) {
> -				dev_err(map->fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d\n",
> -						map->phys, map->len, err);
> +				dev_err(map->fl->sctx->dev,
> +					"Failed to assign memory dma_addr 0x%llx size 0x%llx err %d\n",
> +					map->dma_addr, map->len, err);
>  				return;
>  			}
>  		}
> @@ -389,7 +390,7 @@ static int fastrpc_map_lookup(struct fastrpc_user *fl, int fd,
>  static void fastrpc_buf_free(struct fastrpc_buf *buf)
>  {
>  	dma_free_coherent(buf->dev, buf->size, buf->virt,
> -			  FASTRPC_PHYS(buf->phys));
> +			  FASTRPC_PHYS(buf->dma_addr));
>  	kfree(buf);
>  }
>  
> @@ -408,12 +409,12 @@ static int __fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
>  
>  	buf->fl = fl;
>  	buf->virt = NULL;
> -	buf->phys = 0;
> +	buf->dma_addr = 0;
>  	buf->size = size;
>  	buf->dev = dev;
>  	buf->raddr = 0;
>  
> -	buf->virt = dma_alloc_coherent(dev, buf->size, (dma_addr_t *)&buf->phys,
> +	buf->virt = dma_alloc_coherent(dev, buf->size, (dma_addr_t *)&buf->dma_addr,
>  				       GFP_KERNEL);
>  	if (!buf->virt) {
>  		mutex_destroy(&buf->lock);
> @@ -439,7 +440,7 @@ static int fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
>  	buf = *obuf;
>  
>  	if (fl->sctx && fl->sctx->sid)
> -		buf->phys += ((u64)fl->sctx->sid << 32);
> +		buf->dma_addr += ((u64)fl->sctx->sid << 32);
>  
>  	return 0;
>  }
> @@ -684,7 +685,7 @@ static int fastrpc_dma_buf_attach(struct dma_buf *dmabuf,
>  		return -ENOMEM;
>  
>  	ret = dma_get_sgtable(buffer->dev, &a->sgt, buffer->virt,
> -			      FASTRPC_PHYS(buffer->phys), buffer->size);
> +			      FASTRPC_PHYS(buffer->dma_addr), buffer->size);
>  	if (ret < 0) {
>  		dev_err(buffer->dev, "failed to get scatterlist from DMA API\n");
>  		kfree(a);
> @@ -733,7 +734,7 @@ static int fastrpc_mmap(struct dma_buf *dmabuf,
>  	dma_resv_assert_held(dmabuf->resv);
>  
>  	return dma_mmap_coherent(buf->dev, vma, buf->virt,
> -				 FASTRPC_PHYS(buf->phys), size);
> +				 FASTRPC_PHYS(buf->dma_addr), size);

In fact, we invoke dma_alloc_coherent() above to get a dma_addr_t, and
then we call map, unmap, and free on the lower 32 bits of that
address...

In other words, each time we reference dma_addr we have that implicit
thing that it's a composit of a dma_addr_t as seen from Linux's point of
view (which is matching the addresses in the SMMU page tables) and the
adjusted address that we use in communication with the firmware to
direct the accesses to the right SID + iova.

I think it would be quite nice to make this more explicit throughout the
code, rather than juggling the two perspectives in the same variable.

Regards,
Bjorn

>  }
>  
>  static const struct dma_buf_ops fastrpc_dma_buf_ops = {
> @@ -785,10 +786,10 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
>  	map->table = table;
>  
>  	if (attr & FASTRPC_ATTR_SECUREMAP) {
> -		map->phys = sg_phys(map->table->sgl);
> +		map->dma_addr = sg_phys(map->table->sgl);
>  	} else {
> -		map->phys = sg_dma_address(map->table->sgl);
> -		map->phys += ((u64)fl->sctx->sid << 32);
> +		map->dma_addr = sg_dma_address(map->table->sgl);
> +		map->dma_addr += ((u64)fl->sctx->sid << 32);
>  	}
>  	for_each_sg(map->table->sgl, sgl, map->table->nents,
>  		sgl_index)
> @@ -815,10 +816,11 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
>  		dst_perms[1].vmid = fl->cctx->vmperms[0].vmid;
>  		dst_perms[1].perm = QCOM_SCM_PERM_RWX;
>  		map->attr = attr;
> -		err = qcom_scm_assign_mem(map->phys, (u64)map->len, &src_perms, dst_perms, 2);
> +		err = qcom_scm_assign_mem(map->dma_addr, (u64)map->len, &src_perms, dst_perms, 2);
>  		if (err) {
> -			dev_err(sess->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d\n",
> -					map->phys, map->len, err);
> +			dev_err(sess->dev,
> +				"Failed to assign memory with dma_addr 0x%llx size 0x%llx err %d\n",
> +				map->dma_addr, map->len, err);
>  			goto map_err;
>  		}
>  	}
> @@ -1009,7 +1011,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
>  			struct vm_area_struct *vma = NULL;
>  
>  			rpra[i].buf.pv = (u64) ctx->args[i].ptr;
> -			pages[i].addr = ctx->maps[i]->phys;
> +			pages[i].addr = ctx->maps[i]->dma_addr;
>  
>  			mmap_read_lock(current->mm);
>  			vma = find_vma(current->mm, ctx->args[i].ptr);
> @@ -1036,7 +1038,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
>  				goto bail;
>  
>  			rpra[i].buf.pv = args - ctx->olaps[oix].offset;
> -			pages[i].addr = ctx->buf->phys -
> +			pages[i].addr = ctx->buf->dma_addr -
>  					ctx->olaps[oix].offset +
>  					(pkt_size - rlen);
>  			pages[i].addr = pages[i].addr &	PAGE_MASK;
> @@ -1068,7 +1070,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
>  		list[i].num = ctx->args[i].length ? 1 : 0;
>  		list[i].pgidx = i;
>  		if (ctx->maps[i]) {
> -			pages[i].addr = ctx->maps[i]->phys;
> +			pages[i].addr = ctx->maps[i]->dma_addr;
>  			pages[i].size = ctx->maps[i]->size;
>  		}
>  		rpra[i].dma.fd = ctx->args[i].fd;
> @@ -1150,7 +1152,7 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx *sctx,
>  	msg->ctx = ctx->ctxid | fl->pd;
>  	msg->handle = handle;
>  	msg->sc = ctx->sc;
> -	msg->addr = ctx->buf ? ctx->buf->phys : 0;
> +	msg->addr = ctx->buf ? ctx->buf->dma_addr : 0;
>  	msg->size = roundup(ctx->msg_sz, PAGE_SIZE);
>  	fastrpc_context_get(ctx);
>  
> @@ -1306,13 +1308,14 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>  		if (fl->cctx->vmcount) {
>  			u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
>  
> -			err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
> +			err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
>  							(u64)fl->cctx->remote_heap->size,
>  							&src_perms,
>  							fl->cctx->vmperms, fl->cctx->vmcount);
>  			if (err) {
> -				dev_err(fl->sctx->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d\n",
> -					fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
> +				dev_err(fl->sctx->dev,
> +					"Failed to assign memory with dma_addr 0x%llx size 0x%llx err %d\n",
> +					fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
>  				goto err_map;
>  			}
>  			scm_done = true;
> @@ -1332,7 +1335,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>  	args[1].length = inbuf.namelen;
>  	args[1].fd = -1;
>  
> -	pages[0].addr = fl->cctx->remote_heap->phys;
> +	pages[0].addr = fl->cctx->remote_heap->dma_addr;
>  	pages[0].size = fl->cctx->remote_heap->size;
>  
>  	args[2].ptr = (u64)(uintptr_t) pages;
> @@ -1361,12 +1364,12 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>  
>  		dst_perms.vmid = QCOM_SCM_VMID_HLOS;
>  		dst_perms.perm = QCOM_SCM_PERM_RWX;
> -		err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
> +		err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
>  						(u64)fl->cctx->remote_heap->size,
>  						&src_perms, &dst_perms, 1);
>  		if (err)
> -			dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d\n",
> -				fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
> +			dev_err(fl->sctx->dev, "Failed to assign memory dma_addr 0x%llx size 0x%llx err %d\n",
> +				fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
>  	}
>  err_map:
>  	fastrpc_buf_free(fl->cctx->remote_heap);
> @@ -1455,7 +1458,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl,
>  	args[2].length = inbuf.filelen;
>  	args[2].fd = init.filefd;
>  
> -	pages[0].addr = imem->phys;
> +	pages[0].addr = imem->dma_addr;
>  	pages[0].size = imem->size;
>  
>  	args[3].ptr = (u64)(uintptr_t) pages;
> @@ -1913,7 +1916,7 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
>  	args[0].ptr = (u64) (uintptr_t) &req_msg;
>  	args[0].length = sizeof(req_msg);
>  
> -	pages.addr = buf->phys;
> +	pages.addr = buf->dma_addr;
>  	pages.size = buf->size;
>  
>  	args[1].ptr = (u64) (uintptr_t) &pages;
> @@ -1941,11 +1944,12 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
>  	if (req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR && fl->cctx->vmcount) {
>  		u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
>  
> -		err = qcom_scm_assign_mem(buf->phys, (u64)buf->size,
> +		err = qcom_scm_assign_mem(buf->dma_addr, (u64)buf->size,
>  			&src_perms, fl->cctx->vmperms, fl->cctx->vmcount);
>  		if (err) {
> -			dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d",
> -					buf->phys, buf->size, err);
> +			dev_err(fl->sctx->dev,
> +				"Failed to assign memory dma_addr 0x%llx size 0x%llx err %d",
> +				buf->dma_addr, buf->size, err);
>  			goto err_assign;
>  		}
>  	}
> @@ -2059,7 +2063,7 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp)
>  	args[0].ptr = (u64) (uintptr_t) &req_msg;
>  	args[0].length = sizeof(req_msg);
>  
> -	pages.addr = map->phys;
> +	pages.addr = map->dma_addr;
>  	pages.size = map->len;
>  
>  	args[1].ptr = (u64) (uintptr_t) &pages;
> -- 
> 2.34.1
> 
> 

Re: [PATCH v3 1/4] dt-bindings: misc: qcom,fastrpc: Add compatible for Kaanapali

[Krzysztof Kozlowski]

On 14/11/2025 09:41, Kumari Pallavi wrote:
> Add a new compatible string "qcom,kaanapali-fastrpc" to support
> for Kaanapali SoC.
> 
> Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
> ---

You not only did not bother to test it - there are obvious bugs here -
but you also did not send it to mailing lists for actual testing by
Rob's bot.

NAK

Best regards,
Krzysztof

Re: [PATCH v3 3/4] misc: fastrpc: Add support for new DSP IOVA formatting

[Bjorn Andersson]

On Fri, Nov 14, 2025 at 02:11:41PM +0530, Kumari Pallavi wrote:
> Implement the new IOVA formatting required by the DSP architecture change
> on Kaanapali SoC. Place the SID for DSP DMA transactions at bit 56 in the
> physical address. This placement is necessary for the DSPs to correctly
> identify streams and operate as intended.
> To address this, set SID position to bit 56 via OF matching on the fastrpc
> node; otherwise, default to legacy 32-bit placement.
> This change ensures consistent SID placement across DSPs.
> 

In patch 2 I said I think it would be a good idea to separate the two
perspectives (Linux/SMMU vs remote addresses).

Looking ta this patch I'm completely convinced that it's the right thing
to do!

> Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
> ---
>  drivers/misc/fastrpc.c | 46 +++++++++++++++++++++++++++++++++++-------
>  1 file changed, 39 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index d6a7960fe716..bcf3c7f8d3e9 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -33,7 +33,6 @@
>  #define FASTRPC_ALIGN		128
>  #define FASTRPC_MAX_FDLIST	16
>  #define FASTRPC_MAX_CRCLIST	64
> -#define FASTRPC_PHYS(p)	((p) & 0xffffffff)
>  #define FASTRPC_CTX_MAX (256)
>  #define FASTRPC_INIT_HANDLE	1
>  #define FASTRPC_DSP_UTILITIES_HANDLE	2
> @@ -105,6 +104,15 @@
>  
>  #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
>  
> +/* Extract smmu pa from consolidated iova */
> +#define IPA_TO_DMA_ADDR(iova, sid_pos) (iova & ((1ULL << sid_pos) - 1ULL))
> +/*
> + * Prepare the consolidated iova to send to dsp by prepending the sid
> + * to smmu pa at the appropriate position
> + */
> +#define IOVA_FROM_SID_PA(sid, phys, sid_pos) \
> +       (phys += sid << sid_pos)

This is a horrible macro. It looks just like a function taking values,
it's named to sound like it takes a sid and pa and return an iova, but
it has side effects.

And what's up with the ordering? Take argument 1 and 3, and put the
result in argument 2?!

> +
>  struct fastrpc_phy_page {
>  	u64 addr;		/* physical or dma address */
>  	u64 size;		/* size of contiguous region */
> @@ -257,6 +265,10 @@ struct fastrpc_session_ctx {
>  	bool valid;
>  };
>  
> +struct fastrpc_soc_data {
> +	u32 sid_pos;
> +};
> +
>  struct fastrpc_channel_ctx {
>  	int domain_id;
>  	int sesscount;
> @@ -278,6 +290,7 @@ struct fastrpc_channel_ctx {
>  	bool secure;
>  	bool unsigned_support;
>  	u64 dma_mask;
> +	const struct fastrpc_soc_data *soc_data;
>  };
>  
>  struct fastrpc_device {
> @@ -390,7 +403,7 @@ static int fastrpc_map_lookup(struct fastrpc_user *fl, int fd,
>  static void fastrpc_buf_free(struct fastrpc_buf *buf)
>  {
>  	dma_free_coherent(buf->dev, buf->size, buf->virt,
> -			  FASTRPC_PHYS(buf->dma_addr));
> +			  IPA_TO_DMA_ADDR(buf->dma_addr, buf->fl->cctx->soc_data->sid_pos));
>  	kfree(buf);
>  }
>  
> @@ -440,7 +453,8 @@ static int fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
>  	buf = *obuf;
>  
>  	if (fl->sctx && fl->sctx->sid)
> -		buf->dma_addr += ((u64)fl->sctx->sid << 32);
> +		IOVA_FROM_SID_PA((u64)fl->sctx->sid, buf->dma_addr,
> +				 fl->cctx->soc_data->sid_pos);

There's no way _you_ can look at this statement and feel that it's going
to add the first argument shifted by the third argument, to the second
argument.

Please write that is easy to read, follow and possible to maintain!

Regards,
Bjorn

>  
>  	return 0;
>  }
> @@ -685,7 +699,8 @@ static int fastrpc_dma_buf_attach(struct dma_buf *dmabuf,
>  		return -ENOMEM;
>  
>  	ret = dma_get_sgtable(buffer->dev, &a->sgt, buffer->virt,
> -			      FASTRPC_PHYS(buffer->dma_addr), buffer->size);
> +			      IPA_TO_DMA_ADDR(buffer->dma_addr,
> +			      buffer->fl->cctx->soc_data->sid_pos), buffer->size);
>  	if (ret < 0) {
>  		dev_err(buffer->dev, "failed to get scatterlist from DMA API\n");
>  		kfree(a);
> @@ -734,7 +749,8 @@ static int fastrpc_mmap(struct dma_buf *dmabuf,
>  	dma_resv_assert_held(dmabuf->resv);
>  
>  	return dma_mmap_coherent(buf->dev, vma, buf->virt,
> -				 FASTRPC_PHYS(buf->dma_addr), size);
> +				 IPA_TO_DMA_ADDR(buf->dma_addr,
> +				 buf->fl->cctx->soc_data->sid_pos), size);
>  }
>  
>  static const struct dma_buf_ops fastrpc_dma_buf_ops = {
> @@ -789,7 +805,8 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
>  		map->dma_addr = sg_phys(map->table->sgl);
>  	} else {
>  		map->dma_addr = sg_dma_address(map->table->sgl);
> -		map->dma_addr += ((u64)fl->sctx->sid << 32);
> +		IOVA_FROM_SID_PA((u64)fl->sctx->sid,
> +				 map->dma_addr, fl->cctx->soc_data->sid_pos);
>  	}
>  	for_each_sg(map->table->sgl, sgl, map->table->nents,
>  		sgl_index)
> @@ -2289,6 +2306,14 @@ static int fastrpc_get_domain_id(const char *domain)
>  	return -EINVAL;
>  }
>  
> +static const struct fastrpc_soc_data kaanapali_soc_data = {
> +	.sid_pos = 56,
> +};
> +
> +static const struct fastrpc_soc_data default_soc_data = {
> +	.sid_pos = 32,
> +};
> +
>  static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>  {
>  	struct device *rdev = &rpdev->dev;
> @@ -2297,6 +2322,11 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>  	const char *domain;
>  	bool secure_dsp;
>  	unsigned int vmids[FASTRPC_MAX_VMIDS];
> +	const struct fastrpc_soc_data *soc_data = NULL;
> +
> +	soc_data = device_get_match_data(rdev);
> +	if (!soc_data)
> +		soc_data = &default_soc_data;
>  
>  	err = of_property_read_string(rdev->of_node, "label", &domain);
>  	if (err) {
> @@ -2349,6 +2379,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>  
>  	secure_dsp = !(of_property_read_bool(rdev->of_node, "qcom,non-secure-domain"));
>  	data->secure = secure_dsp;
> +	data->soc_data = soc_data;
>  
>  	switch (domain_id) {
>  	case ADSP_DOMAIN_ID:
> @@ -2486,7 +2517,8 @@ static int fastrpc_rpmsg_callback(struct rpmsg_device *rpdev, void *data,
>  }
>  
>  static const struct of_device_id fastrpc_rpmsg_of_match[] = {
> -	{ .compatible = "qcom,fastrpc" },
> +	{ .compatible = "qcom,kaanapali-fastrpc", .data = &kaanapali_soc_data },
> +	{ .compatible = "qcom,fastrpc", .data = &default_soc_data },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, fastrpc_rpmsg_of_match);
> -- 
> 2.34.1
> 
> 

Re: [PATCH v3 4/4] misc: fastrpc: Update dma_bits for CDSP support on Kaanapali SoC

[Bjorn Andersson]

On Fri, Nov 14, 2025 at 02:11:42PM +0530, Kumari Pallavi wrote:
> DSP currently supports 32-bit IOVA (32-bit PA + 4-bit SID) for
> both Q6 and user DMA (uDMA) access. This is being upgraded to
> 34-bit PA + 4-bit SID due to a hardware revision in CDSP for
> Kaanapali SoC, which expands the DMA addressable range.
> Update DMA bits configuration in the driver to support CDSP on
> Kaanapali SoC. Set the default `dma_bits` to 32-bit and update
> it to 34-bit based on CDSP and OF matching on the fastrpc node.
> 
> Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
> ---
>  drivers/misc/fastrpc.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index bcf3c7f8d3e9..2eb8d37cd9b4 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -267,6 +267,8 @@ struct fastrpc_session_ctx {
>  
>  struct fastrpc_soc_data {
>  	u32 sid_pos;
> +	u32 cdsp_dma_bits;
> +	u32 dsp_default_dma_bits;
>  };
>  
>  struct fastrpc_channel_ctx {
> @@ -2186,6 +2188,7 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
>  	int i, sessions = 0;
>  	unsigned long flags;
>  	int rc;
> +	u32 dma_bits;
>  
>  	cctx = dev_get_drvdata(dev->parent);
>  	if (!cctx)
> @@ -2199,12 +2202,16 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
>  		spin_unlock_irqrestore(&cctx->lock, flags);
>  		return -ENOSPC;
>  	}
> +	dma_bits = cctx->soc_data->dsp_default_dma_bits;
>  	sess = &cctx->session[cctx->sesscount++];
>  	sess->used = false;
>  	sess->valid = true;
>  	sess->dev = dev;
>  	dev_set_drvdata(dev, sess);
>  
> +	if (cctx->domain_id == CDSP_DOMAIN_ID)
> +		dma_bits = cctx->soc_data->cdsp_dma_bits;
> +
>  	if (of_property_read_u32(dev->of_node, "reg", &sess->sid))
>  		dev_info(dev, "FastRPC Session ID not specified in DT\n");
>  
> @@ -2219,9 +2226,9 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
>  		}
>  	}
>  	spin_unlock_irqrestore(&cctx->lock, flags);
> -	rc = dma_set_mask(dev, DMA_BIT_MASK(32));
> +	rc = dma_set_mask(dev, DMA_BIT_MASK(dma_bits));
>  	if (rc) {
> -		dev_err(dev, "32-bit DMA enable failed\n");
> +		dev_err(dev, "%u-bit DMA enable failed\n", dma_bits);
>  		return rc;
>  	}
>  
> @@ -2308,10 +2315,14 @@ static int fastrpc_get_domain_id(const char *domain)
>  
>  static const struct fastrpc_soc_data kaanapali_soc_data = {
>  	.sid_pos = 56,
> +	.cdsp_dma_bits = 34,
> +	.dsp_default_dma_bits = 32,
>  };
>  
>  static const struct fastrpc_soc_data default_soc_data = {
>  	.sid_pos = 32,
> +	.cdsp_dma_bits = 32,
> +	.dsp_default_dma_bits = 32,

So, "dsp_default_dma_bits" specified "what is the dma_mask for the
non-CDSP fastrpc instances"? I don't find "dsp_default" to naturally
mean "not the cdsp".


Wouldn't it be better to introduce two different compatibles, one being
the "qcom,kaanapali-fastrpc" and one being the
"qcom,kaanapali-cdsp-fastrpc" and then use that to select things here?


PS. You store "dma_bits" just for the sake of turning it into a
dma_mask, just store the DMA_BIT_MASK() directly here instead.

Regards,
Bjorn

>  };
>  
>  static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
> -- 
> 2.34.1
> 
> 

Re: [PATCH v3 3/4] misc: fastrpc: Add support for new DSP IOVA formatting

[Kumari Pallavi]

On 11/14/2025 9:21 PM, Bjorn Andersson wrote:
> On Fri, Nov 14, 2025 at 02:11:41PM +0530, Kumari Pallavi wrote:
>> Implement the new IOVA formatting required by the DSP architecture change
>> on Kaanapali SoC. Place the SID for DSP DMA transactions at bit 56 in the
>> physical address. This placement is necessary for the DSPs to correctly
>> identify streams and operate as intended.
>> To address this, set SID position to bit 56 via OF matching on the fastrpc
>> node; otherwise, default to legacy 32-bit placement.
>> This change ensures consistent SID placement across DSPs.
>>
> 
> In patch 2 I said I think it would be a good idea to separate the two
> perspectives (Linux/SMMU vs remote addresses).
> 
> Looking ta this patch I'm completely convinced that it's the right thing
> to do!
> 
>> Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
>> ---
>>   drivers/misc/fastrpc.c | 46 +++++++++++++++++++++++++++++++++++-------
>>   1 file changed, 39 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index d6a7960fe716..bcf3c7f8d3e9 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -33,7 +33,6 @@
>>   #define FASTRPC_ALIGN		128
>>   #define FASTRPC_MAX_FDLIST	16
>>   #define FASTRPC_MAX_CRCLIST	64
>> -#define FASTRPC_PHYS(p)	((p) & 0xffffffff)
>>   #define FASTRPC_CTX_MAX (256)
>>   #define FASTRPC_INIT_HANDLE	1
>>   #define FASTRPC_DSP_UTILITIES_HANDLE	2
>> @@ -105,6 +104,15 @@
>>   
>>   #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
>>   
>> +/* Extract smmu pa from consolidated iova */
>> +#define IPA_TO_DMA_ADDR(iova, sid_pos) (iova & ((1ULL << sid_pos) - 1ULL))
>> +/*
>> + * Prepare the consolidated iova to send to dsp by prepending the sid
>> + * to smmu pa at the appropriate position
>> + */
>> +#define IOVA_FROM_SID_PA(sid, phys, sid_pos) \
>> +       (phys += sid << sid_pos)
> 
> This is a horrible macro. It looks just like a function taking values,
> it's named to sound like it takes a sid and pa and return an iova, but
> it has side effects.
> 
> And what's up with the ordering? Take argument 1 and 3, and put the
> result in argument 2?!
> 

Thank you for the feedback regarding the macro implementation. I 
understand your concern about readability and hidden side effects.
To address this, I’ve replaced the macro with an inline function


static inline u64 fastrpc_compute_sid_offset(u64 sid, u32 sid_pos)
{
     return sid << sid_pos;
}


buf->dma_addr += fastrpc_compute_sid_offset(sid, sid_pos);

Could you confirm if this is in line with what you suggested?

Thanks,
Pallavi

>> +
>>   struct fastrpc_phy_page {
>>   	u64 addr;		/* physical or dma address */
>>   	u64 size;		/* size of contiguous region */
>> @@ -257,6 +265,10 @@ struct fastrpc_session_ctx {
>>   	bool valid;
>>   };
>>   
>> +struct fastrpc_soc_data {
>> +	u32 sid_pos;
>> +};
>> +
>>   struct fastrpc_channel_ctx {
>>   	int domain_id;
>>   	int sesscount;
>> @@ -278,6 +290,7 @@ struct fastrpc_channel_ctx {
>>   	bool secure;
>>   	bool unsigned_support;
>>   	u64 dma_mask;
>> +	const struct fastrpc_soc_data *soc_data;
>>   };
>>   
>>   struct fastrpc_device {
>> @@ -390,7 +403,7 @@ static int fastrpc_map_lookup(struct fastrpc_user *fl, int fd,
>>   static void fastrpc_buf_free(struct fastrpc_buf *buf)
>>   {
>>   	dma_free_coherent(buf->dev, buf->size, buf->virt,
>> -			  FASTRPC_PHYS(buf->dma_addr));
>> +			  IPA_TO_DMA_ADDR(buf->dma_addr, buf->fl->cctx->soc_data->sid_pos));
>>   	kfree(buf);
>>   }
>>   
>> @@ -440,7 +453,8 @@ static int fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
>>   	buf = *obuf;
>>   
>>   	if (fl->sctx && fl->sctx->sid)
>> -		buf->dma_addr += ((u64)fl->sctx->sid << 32);
>> +		IOVA_FROM_SID_PA((u64)fl->sctx->sid, buf->dma_addr,
>> +				 fl->cctx->soc_data->sid_pos);
> 
> There's no way _you_ can look at this statement and feel that it's going
> to add the first argument shifted by the third argument, to the second
> argument.
> 
> Please write that is easy to read, follow and possible to maintain!
> 
> Regards,
> Bjorn
> 
>>   
>>   	return 0;
>>   }
>> @@ -685,7 +699,8 @@ static int fastrpc_dma_buf_attach(struct dma_buf *dmabuf,
>>   		return -ENOMEM;
>>   
>>   	ret = dma_get_sgtable(buffer->dev, &a->sgt, buffer->virt,
>> -			      FASTRPC_PHYS(buffer->dma_addr), buffer->size);
>> +			      IPA_TO_DMA_ADDR(buffer->dma_addr,
>> +			      buffer->fl->cctx->soc_data->sid_pos), buffer->size);
>>   	if (ret < 0) {
>>   		dev_err(buffer->dev, "failed to get scatterlist from DMA API\n");
>>   		kfree(a);
>> @@ -734,7 +749,8 @@ static int fastrpc_mmap(struct dma_buf *dmabuf,
>>   	dma_resv_assert_held(dmabuf->resv);
>>   
>>   	return dma_mmap_coherent(buf->dev, vma, buf->virt,
>> -				 FASTRPC_PHYS(buf->dma_addr), size);
>> +				 IPA_TO_DMA_ADDR(buf->dma_addr,
>> +				 buf->fl->cctx->soc_data->sid_pos), size);
>>   }
>>   
>>   static const struct dma_buf_ops fastrpc_dma_buf_ops = {
>> @@ -789,7 +805,8 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
>>   		map->dma_addr = sg_phys(map->table->sgl);
>>   	} else {
>>   		map->dma_addr = sg_dma_address(map->table->sgl);
>> -		map->dma_addr += ((u64)fl->sctx->sid << 32);
>> +		IOVA_FROM_SID_PA((u64)fl->sctx->sid,
>> +				 map->dma_addr, fl->cctx->soc_data->sid_pos);
>>   	}
>>   	for_each_sg(map->table->sgl, sgl, map->table->nents,
>>   		sgl_index)
>> @@ -2289,6 +2306,14 @@ static int fastrpc_get_domain_id(const char *domain)
>>   	return -EINVAL;
>>   }
>>   
>> +static const struct fastrpc_soc_data kaanapali_soc_data = {
>> +	.sid_pos = 56,
>> +};
>> +
>> +static const struct fastrpc_soc_data default_soc_data = {
>> +	.sid_pos = 32,
>> +};
>> +
>>   static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>>   {
>>   	struct device *rdev = &rpdev->dev;
>> @@ -2297,6 +2322,11 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>>   	const char *domain;
>>   	bool secure_dsp;
>>   	unsigned int vmids[FASTRPC_MAX_VMIDS];
>> +	const struct fastrpc_soc_data *soc_data = NULL;
>> +
>> +	soc_data = device_get_match_data(rdev);
>> +	if (!soc_data)
>> +		soc_data = &default_soc_data;
>>   
>>   	err = of_property_read_string(rdev->of_node, "label", &domain);
>>   	if (err) {
>> @@ -2349,6 +2379,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>>   
>>   	secure_dsp = !(of_property_read_bool(rdev->of_node, "qcom,non-secure-domain"));
>>   	data->secure = secure_dsp;
>> +	data->soc_data = soc_data;
>>   
>>   	switch (domain_id) {
>>   	case ADSP_DOMAIN_ID:
>> @@ -2486,7 +2517,8 @@ static int fastrpc_rpmsg_callback(struct rpmsg_device *rpdev, void *data,
>>   }
>>   
>>   static const struct of_device_id fastrpc_rpmsg_of_match[] = {
>> -	{ .compatible = "qcom,fastrpc" },
>> +	{ .compatible = "qcom,kaanapali-fastrpc", .data = &kaanapali_soc_data },
>> +	{ .compatible = "qcom,fastrpc", .data = &default_soc_data },
>>   	{ },
>>   };
>>   MODULE_DEVICE_TABLE(of, fastrpc_rpmsg_of_match);
>> -- 
>> 2.34.1
>>
>>

Re: [PATCH v3 2/4] misc: fastrpc: Rename phys to dma_addr for clarity

[Kumari Pallavi]

On 11/14/2025 5:45 PM, Dmitry Baryshkov wrote:
>> Update all references of buf->phys and map->phys to buf->dma_addr and
>> map->dma_addr to accurately represent that these fields store DMA
>> addresses, not physical addresses. This change improves code clarity
>> and aligns with kernel conventions for dma_addr_t usage.
> Why do you mention dma_addr_t here?
> 

I mentioned dma_addr_t in the commit message because of the earlier 
feedback highlighted about the confusing use of phys for fields that 
actually store an IOVA-like address ('phys' with something more fitting 
for an IOVA or DMA address).

https://lore.kernel.org/all/969bdb49-0682-4345-98f7-523404bb4213@app.fastmail.com/

>> Signed-off-by: Kumari Pallavi<kumari.pallavi@oss.qualcomm.com>
>> ---
>>   drivers/misc/fastrpc.c | 76 ++++++++++++++++++++++--------------------
>>   1 file changed, 40 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index ee652ef01534..d6a7960fe716 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -106,7 +106,7 @@
>>   #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
>>   
>>   struct fastrpc_phy_page {
>> -	u64 addr;		/* physical address */
>> +	u64 addr;		/* physical or dma address */
> What is the difference here? Aren't all of them DMA addresses?

Yes, correct—both represent DMA addresses, just typed differently 
depending on whether it originate from a physical or DMA mapping context.

ACK, I will update this in the next patch series.

Thanks,
Pallavi

Re: [PATCH v3 2/4] misc: fastrpc: Rename phys to dma_addr for clarity

[Kumari Pallavi]

On 11/14/2025 9:14 PM, Bjorn Andersson wrote:
> On Fri, Nov 14, 2025 at 02:11:40PM +0530, Kumari Pallavi wrote:
>> Update all references of buf->phys and map->phys to buf->dma_addr and
>> map->dma_addr to accurately represent that these fields store DMA
>> addresses, not physical addresses. This change improves code clarity
>> and aligns with kernel conventions for dma_addr_t usage.
>>
>> Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
>> ---
>>   drivers/misc/fastrpc.c | 76 ++++++++++++++++++++++--------------------
>>   1 file changed, 40 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index ee652ef01534..d6a7960fe716 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -106,7 +106,7 @@
>>   #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
>>   
>>   struct fastrpc_phy_page {
>> -	u64 addr;		/* physical address */
>> +	u64 addr;		/* physical or dma address */
>>   	u64 size;		/* size of contiguous region */
>>   };
>>   
>> @@ -171,7 +171,7 @@ struct fastrpc_msg {
>>   	u64 ctx;		/* invoke caller context */
>>   	u32 handle;	/* handle to invoke */
>>   	u32 sc;		/* scalars structure describing the data */
>> -	u64 addr;		/* physical address */
>> +	u64 addr;		/* physical or dma address */
> 
> Can you go all the way and make the type dma_addr_t? That way you don't
> need to typecast the dma_alloc_coherent() and dma_free_coherent().
> 
> I believe it might complicate the places where you do math on it, but
> that is a good thing, as it highlights those places where you do
> something unexpected.
> 

While this not strictly limited to holding a dma_addr_t.
Based on historical behavior, when the FASTRPC_ATTR_SECUREMAP flag is
set, S2 mapping expects a physical address to be passed to the
qcom_scm_assign_mem() API.
reference-
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/misc/fastrpc.c?id=e90d911906196bf987492c94e38f10ca611dfd7b

If you suggest, I can define it as dma_addr_t and perform typecasting to 
u64 wherever required.

Thanks,
Pallavi


>>   	u64 size;		/* size of contiguous region */
>>   };
>>   
>> @@ -194,7 +194,7 @@ struct fastrpc_buf {
>>   	struct dma_buf *dmabuf;
>>   	struct device *dev;
>>   	void *virt;
>> -	u64 phys;
>> +	u64 dma_addr;
>>   	u64 size;
>>   	/* Lock for dma buf attachments */
>>   	struct mutex lock;
>> @@ -217,7 +217,7 @@ struct fastrpc_map {
>>   	struct dma_buf *buf;
>>   	struct sg_table *table;
>>   	struct dma_buf_attachment *attach;
>> -	u64 phys;
>> +	u64 dma_addr;
>>   	u64 size;
>>   	void *va;
>>   	u64 len;
>> @@ -320,11 +320,12 @@ static void fastrpc_free_map(struct kref *ref)
>>   
>>   			perm.vmid = QCOM_SCM_VMID_HLOS;
>>   			perm.perm = QCOM_SCM_PERM_RWX;
>> -			err = qcom_scm_assign_mem(map->phys, map->len,
>> +			err = qcom_scm_assign_mem(map->dma_addr, map->len,
>>   				&src_perms, &perm, 1);
>>   			if (err) {
>> -				dev_err(map->fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d\n",
>> -						map->phys, map->len, err);
>> +				dev_err(map->fl->sctx->dev,
>> +					"Failed to assign memory dma_addr 0x%llx size 0x%llx err %d\n",
>> +					map->dma_addr, map->len, err);
>>   				return;
>>   			}
>>   		}
>> @@ -389,7 +390,7 @@ static int fastrpc_map_lookup(struct fastrpc_user *fl, int fd,
>>   static void fastrpc_buf_free(struct fastrpc_buf *buf)
>>   {
>>   	dma_free_coherent(buf->dev, buf->size, buf->virt,
>> -			  FASTRPC_PHYS(buf->phys));
>> +			  FASTRPC_PHYS(buf->dma_addr));
>>   	kfree(buf);
>>   }
>>   
>> @@ -408,12 +409,12 @@ static int __fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
>>   
>>   	buf->fl = fl;
>>   	buf->virt = NULL;
>> -	buf->phys = 0;
>> +	buf->dma_addr = 0;
>>   	buf->size = size;
>>   	buf->dev = dev;
>>   	buf->raddr = 0;
>>   
>> -	buf->virt = dma_alloc_coherent(dev, buf->size, (dma_addr_t *)&buf->phys,
>> +	buf->virt = dma_alloc_coherent(dev, buf->size, (dma_addr_t *)&buf->dma_addr,
>>   				       GFP_KERNEL);
>>   	if (!buf->virt) {
>>   		mutex_destroy(&buf->lock);
>> @@ -439,7 +440,7 @@ static int fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
>>   	buf = *obuf;
>>   
>>   	if (fl->sctx && fl->sctx->sid)
>> -		buf->phys += ((u64)fl->sctx->sid << 32);
>> +		buf->dma_addr += ((u64)fl->sctx->sid << 32);
>>   
>>   	return 0;
>>   }
>> @@ -684,7 +685,7 @@ static int fastrpc_dma_buf_attach(struct dma_buf *dmabuf,
>>   		return -ENOMEM;
>>   
>>   	ret = dma_get_sgtable(buffer->dev, &a->sgt, buffer->virt,
>> -			      FASTRPC_PHYS(buffer->phys), buffer->size);
>> +			      FASTRPC_PHYS(buffer->dma_addr), buffer->size);
>>   	if (ret < 0) {
>>   		dev_err(buffer->dev, "failed to get scatterlist from DMA API\n");
>>   		kfree(a);
>> @@ -733,7 +734,7 @@ static int fastrpc_mmap(struct dma_buf *dmabuf,
>>   	dma_resv_assert_held(dmabuf->resv);
>>   
>>   	return dma_mmap_coherent(buf->dev, vma, buf->virt,
>> -				 FASTRPC_PHYS(buf->phys), size);
>> +				 FASTRPC_PHYS(buf->dma_addr), size);
> 
> In fact, we invoke dma_alloc_coherent() above to get a dma_addr_t, and
> then we call map, unmap, and free on the lower 32 bits of that
> address...
> 
> In other words, each time we reference dma_addr we have that implicit
> thing that it's a composit of a dma_addr_t as seen from Linux's point of
> view (which is matching the addresses in the SMMU page tables) and the
> adjusted address that we use in communication with the firmware to
> direct the accesses to the right SID + iova.
> 
> I think it would be quite nice to make this more explicit throughout the
> code, rather than juggling the two perspectives in the same variable.
> 
> Regards,
> Bjorn
> 
>>   }
>>   
>>   static const struct dma_buf_ops fastrpc_dma_buf_ops = {
>> @@ -785,10 +786,10 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
>>   	map->table = table;
>>   
>>   	if (attr & FASTRPC_ATTR_SECUREMAP) {
>> -		map->phys = sg_phys(map->table->sgl);
>> +		map->dma_addr = sg_phys(map->table->sgl);
>>   	} else {
>> -		map->phys = sg_dma_address(map->table->sgl);
>> -		map->phys += ((u64)fl->sctx->sid << 32);
>> +		map->dma_addr = sg_dma_address(map->table->sgl);
>> +		map->dma_addr += ((u64)fl->sctx->sid << 32);
>>   	}
>>   	for_each_sg(map->table->sgl, sgl, map->table->nents,
>>   		sgl_index)
>> @@ -815,10 +816,11 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
>>   		dst_perms[1].vmid = fl->cctx->vmperms[0].vmid;
>>   		dst_perms[1].perm = QCOM_SCM_PERM_RWX;
>>   		map->attr = attr;
>> -		err = qcom_scm_assign_mem(map->phys, (u64)map->len, &src_perms, dst_perms, 2);
>> +		err = qcom_scm_assign_mem(map->dma_addr, (u64)map->len, &src_perms, dst_perms, 2);
>>   		if (err) {
>> -			dev_err(sess->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d\n",
>> -					map->phys, map->len, err);
>> +			dev_err(sess->dev,
>> +				"Failed to assign memory with dma_addr 0x%llx size 0x%llx err %d\n",
>> +				map->dma_addr, map->len, err);
>>   			goto map_err;
>>   		}
>>   	}
>> @@ -1009,7 +1011,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
>>   			struct vm_area_struct *vma = NULL;
>>   
>>   			rpra[i].buf.pv = (u64) ctx->args[i].ptr;
>> -			pages[i].addr = ctx->maps[i]->phys;
>> +			pages[i].addr = ctx->maps[i]->dma_addr;
>>   
>>   			mmap_read_lock(current->mm);
>>   			vma = find_vma(current->mm, ctx->args[i].ptr);
>> @@ -1036,7 +1038,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
>>   				goto bail;
>>   
>>   			rpra[i].buf.pv = args - ctx->olaps[oix].offset;
>> -			pages[i].addr = ctx->buf->phys -
>> +			pages[i].addr = ctx->buf->dma_addr -
>>   					ctx->olaps[oix].offset +
>>   					(pkt_size - rlen);
>>   			pages[i].addr = pages[i].addr &	PAGE_MASK;
>> @@ -1068,7 +1070,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
>>   		list[i].num = ctx->args[i].length ? 1 : 0;
>>   		list[i].pgidx = i;
>>   		if (ctx->maps[i]) {
>> -			pages[i].addr = ctx->maps[i]->phys;
>> +			pages[i].addr = ctx->maps[i]->dma_addr;
>>   			pages[i].size = ctx->maps[i]->size;
>>   		}
>>   		rpra[i].dma.fd = ctx->args[i].fd;
>> @@ -1150,7 +1152,7 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx *sctx,
>>   	msg->ctx = ctx->ctxid | fl->pd;
>>   	msg->handle = handle;
>>   	msg->sc = ctx->sc;
>> -	msg->addr = ctx->buf ? ctx->buf->phys : 0;
>> +	msg->addr = ctx->buf ? ctx->buf->dma_addr : 0;
>>   	msg->size = roundup(ctx->msg_sz, PAGE_SIZE);
>>   	fastrpc_context_get(ctx);
>>   
>> @@ -1306,13 +1308,14 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>>   		if (fl->cctx->vmcount) {
>>   			u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
>>   
>> -			err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
>> +			err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
>>   							(u64)fl->cctx->remote_heap->size,
>>   							&src_perms,
>>   							fl->cctx->vmperms, fl->cctx->vmcount);
>>   			if (err) {
>> -				dev_err(fl->sctx->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d\n",
>> -					fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
>> +				dev_err(fl->sctx->dev,
>> +					"Failed to assign memory with dma_addr 0x%llx size 0x%llx err %d\n",
>> +					fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
>>   				goto err_map;
>>   			}
>>   			scm_done = true;
>> @@ -1332,7 +1335,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>>   	args[1].length = inbuf.namelen;
>>   	args[1].fd = -1;
>>   
>> -	pages[0].addr = fl->cctx->remote_heap->phys;
>> +	pages[0].addr = fl->cctx->remote_heap->dma_addr;
>>   	pages[0].size = fl->cctx->remote_heap->size;
>>   
>>   	args[2].ptr = (u64)(uintptr_t) pages;
>> @@ -1361,12 +1364,12 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>>   
>>   		dst_perms.vmid = QCOM_SCM_VMID_HLOS;
>>   		dst_perms.perm = QCOM_SCM_PERM_RWX;
>> -		err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
>> +		err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
>>   						(u64)fl->cctx->remote_heap->size,
>>   						&src_perms, &dst_perms, 1);
>>   		if (err)
>> -			dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d\n",
>> -				fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
>> +			dev_err(fl->sctx->dev, "Failed to assign memory dma_addr 0x%llx size 0x%llx err %d\n",
>> +				fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
>>   	}
>>   err_map:
>>   	fastrpc_buf_free(fl->cctx->remote_heap);
>> @@ -1455,7 +1458,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl,
>>   	args[2].length = inbuf.filelen;
>>   	args[2].fd = init.filefd;
>>   
>> -	pages[0].addr = imem->phys;
>> +	pages[0].addr = imem->dma_addr;
>>   	pages[0].size = imem->size;
>>   
>>   	args[3].ptr = (u64)(uintptr_t) pages;
>> @@ -1913,7 +1916,7 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
>>   	args[0].ptr = (u64) (uintptr_t) &req_msg;
>>   	args[0].length = sizeof(req_msg);
>>   
>> -	pages.addr = buf->phys;
>> +	pages.addr = buf->dma_addr;
>>   	pages.size = buf->size;
>>   
>>   	args[1].ptr = (u64) (uintptr_t) &pages;
>> @@ -1941,11 +1944,12 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
>>   	if (req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR && fl->cctx->vmcount) {
>>   		u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
>>   
>> -		err = qcom_scm_assign_mem(buf->phys, (u64)buf->size,
>> +		err = qcom_scm_assign_mem(buf->dma_addr, (u64)buf->size,
>>   			&src_perms, fl->cctx->vmperms, fl->cctx->vmcount);
>>   		if (err) {
>> -			dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d",
>> -					buf->phys, buf->size, err);
>> +			dev_err(fl->sctx->dev,
>> +				"Failed to assign memory dma_addr 0x%llx size 0x%llx err %d",
>> +				buf->dma_addr, buf->size, err);
>>   			goto err_assign;
>>   		}
>>   	}
>> @@ -2059,7 +2063,7 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp)
>>   	args[0].ptr = (u64) (uintptr_t) &req_msg;
>>   	args[0].length = sizeof(req_msg);
>>   
>> -	pages.addr = map->phys;
>> +	pages.addr = map->dma_addr;
>>   	pages.size = map->len;
>>   
>>   	args[1].ptr = (u64) (uintptr_t) &pages;
>> -- 
>> 2.34.1
>>
>>

Re: [PATCH v3 1/4] dt-bindings: misc: qcom,fastrpc: Add compatible for Kaanapali

[Kumari Pallavi]

On 11/14/2025 9:17 PM, Krzysztof Kozlowski wrote:
> On 14/11/2025 09:41, Kumari Pallavi wrote:
>> Add a new compatible string "qcom,kaanapali-fastrpc" to support
>> for Kaanapali SoC.
>>
>> Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
>> ---
> 
> You not only did not bother to test it - there are obvious bugs here -
> but you also did not send it to mailing lists for actual testing by
> Rob's bot.
> 
> NAK
> 
> Best regards,
> Krzysztof

Thank you for the feedback. I did run the schema validation locally using:
make dt_binding_check
(as per 
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/), 
and the build completed successfully without any errors.
Ack that I missed adding the mailing lists in the initial submission due 
to an issue with the script I used for retrieving mailing addresses 
locally.
I’ll make sure to include the correct mailing lists in the next revision.

Thanks,
Pallavi

Re: [PATCH v3 1/4] dt-bindings: misc: qcom,fastrpc: Add compatible for Kaanapali

[Krzysztof Kozlowski]

On 17/11/2025 08:17, Kumari Pallavi wrote:
> 
> 
> On 11/14/2025 9:17 PM, Krzysztof Kozlowski wrote:
>> On 14/11/2025 09:41, Kumari Pallavi wrote:
>>> Add a new compatible string "qcom,kaanapali-fastrpc" to support
>>> for Kaanapali SoC.
>>>
>>> Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
>>> ---
>>
>> You not only did not bother to test it - there are obvious bugs here -
>> but you also did not send it to mailing lists for actual testing by
>> Rob's bot.
>>
>> NAK
>>
>> Best regards,
>> Krzysztof
> 
> Thank you for the feedback. I did run the schema validation locally using:
> make dt_binding_check
> (as per 
> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/), 
> and the build completed successfully without any errors.

But you did not read warnings and messages shown to you...

That's still obviously NAK.

Best regards,
Krzysztof

Re: [PATCH v3 4/4] misc: fastrpc: Update dma_bits for CDSP support on Kaanapali SoC

[Kumari Pallavi]

On 11/14/2025 9:30 PM, Bjorn Andersson wrote:
> On Fri, Nov 14, 2025 at 02:11:42PM +0530, Kumari Pallavi wrote:
>> DSP currently supports 32-bit IOVA (32-bit PA + 4-bit SID) for
>> both Q6 and user DMA (uDMA) access. This is being upgraded to
>> 34-bit PA + 4-bit SID due to a hardware revision in CDSP for
>> Kaanapali SoC, which expands the DMA addressable range.
>> Update DMA bits configuration in the driver to support CDSP on
>> Kaanapali SoC. Set the default `dma_bits` to 32-bit and update
>> it to 34-bit based on CDSP and OF matching on the fastrpc node.
>>
>> Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
>> ---
>>   drivers/misc/fastrpc.c | 15 +++++++++++++--
>>   1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index bcf3c7f8d3e9..2eb8d37cd9b4 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -267,6 +267,8 @@ struct fastrpc_session_ctx {
>>   
>>   struct fastrpc_soc_data {
>>   	u32 sid_pos;
>> +	u32 cdsp_dma_bits;
>> +	u32 dsp_default_dma_bits;
>>   };
>>   
>>   struct fastrpc_channel_ctx {
>> @@ -2186,6 +2188,7 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
>>   	int i, sessions = 0;
>>   	unsigned long flags;
>>   	int rc;
>> +	u32 dma_bits;
>>   
>>   	cctx = dev_get_drvdata(dev->parent);
>>   	if (!cctx)
>> @@ -2199,12 +2202,16 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
>>   		spin_unlock_irqrestore(&cctx->lock, flags);
>>   		return -ENOSPC;
>>   	}
>> +	dma_bits = cctx->soc_data->dsp_default_dma_bits;
>>   	sess = &cctx->session[cctx->sesscount++];
>>   	sess->used = false;
>>   	sess->valid = true;
>>   	sess->dev = dev;
>>   	dev_set_drvdata(dev, sess);
>>   
>> +	if (cctx->domain_id == CDSP_DOMAIN_ID)
>> +		dma_bits = cctx->soc_data->cdsp_dma_bits;
>> +
>>   	if (of_property_read_u32(dev->of_node, "reg", &sess->sid))
>>   		dev_info(dev, "FastRPC Session ID not specified in DT\n");
>>   
>> @@ -2219,9 +2226,9 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
>>   		}
>>   	}
>>   	spin_unlock_irqrestore(&cctx->lock, flags);
>> -	rc = dma_set_mask(dev, DMA_BIT_MASK(32));
>> +	rc = dma_set_mask(dev, DMA_BIT_MASK(dma_bits));
>>   	if (rc) {
>> -		dev_err(dev, "32-bit DMA enable failed\n");
>> +		dev_err(dev, "%u-bit DMA enable failed\n", dma_bits);
>>   		return rc;
>>   	}
>>   
>> @@ -2308,10 +2315,14 @@ static int fastrpc_get_domain_id(const char *domain)
>>   
>>   static const struct fastrpc_soc_data kaanapali_soc_data = {
>>   	.sid_pos = 56,
>> +	.cdsp_dma_bits = 34,
>> +	.dsp_default_dma_bits = 32,
>>   };
>>   
>>   static const struct fastrpc_soc_data default_soc_data = {
>>   	.sid_pos = 32,
>> +	.cdsp_dma_bits = 32,
>> +	.dsp_default_dma_bits = 32,
> 
> So, "dsp_default_dma_bits" specified "what is the dma_mask for the
> non-CDSP fastrpc instances"? I don't find "dsp_default" to naturally
> mean "not the cdsp".
> 
> 
> Wouldn't it be better to introduce two different compatibles, one being
> the "qcom,kaanapali-fastrpc" and one being the
> "qcom,kaanapali-cdsp-fastrpc" and then use that to select things here?
> 

Thank you for the suggestion. In this case, sid_pos is common across all
DSP domains on kaanapali Soc. Splitting into two compatibles would lead 
to duplication of these shared property in the DT schema and driver logic.
The only difference here is the DMA address width for CDSP (34-bit) 
versus other DSPs (32-bit).

To address the concern about naming, I can provide:

dma_bits_cdsp → clearly indicates this applies to the CDSP domain.
dma_bits_non_cdsp (or dma_bits_other_dsp) → for ADSP and other DSP domains.
Please let me know if this aligns with your suggestion ?

> 
> PS. You store "dma_bits" just for the sake of turning it into a
> dma_mask, just store the DMA_BIT_MASK() directly here instead.
> 

The current approach of assigning a value to cdsp_dma_mask allows for 
adaptable logging behavior, making it easier to trace.


> Regards,
> Bjorn
> 
>>   };
>>   
>>   static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>> -- 
>> 2.34.1
>>
>>

Thanks,
Pallavi

Re: [PATCH v3 2/4] misc: fastrpc: Rename phys to dma_addr for clarity

[Dmitry Baryshkov]

On Mon, Nov 17, 2025 at 12:37:33PM +0530, Kumari Pallavi wrote:
> 
> 
> On 11/14/2025 9:14 PM, Bjorn Andersson wrote:
> > On Fri, Nov 14, 2025 at 02:11:40PM +0530, Kumari Pallavi wrote:
> > > Update all references of buf->phys and map->phys to buf->dma_addr and
> > > map->dma_addr to accurately represent that these fields store DMA
> > > addresses, not physical addresses. This change improves code clarity
> > > and aligns with kernel conventions for dma_addr_t usage.
> > > 
> > > Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
> > > ---
> > >   drivers/misc/fastrpc.c | 76 ++++++++++++++++++++++--------------------
> > >   1 file changed, 40 insertions(+), 36 deletions(-)
> > > 
> > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> > > index ee652ef01534..d6a7960fe716 100644
> > > --- a/drivers/misc/fastrpc.c
> > > +++ b/drivers/misc/fastrpc.c
> > > @@ -106,7 +106,7 @@
> > >   #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
> > >   struct fastrpc_phy_page {
> > > -	u64 addr;		/* physical address */
> > > +	u64 addr;		/* physical or dma address */
> > >   	u64 size;		/* size of contiguous region */
> > >   };
> > > @@ -171,7 +171,7 @@ struct fastrpc_msg {
> > >   	u64 ctx;		/* invoke caller context */
> > >   	u32 handle;	/* handle to invoke */
> > >   	u32 sc;		/* scalars structure describing the data */
> > > -	u64 addr;		/* physical address */
> > > +	u64 addr;		/* physical or dma address */
> > 
> > Can you go all the way and make the type dma_addr_t? That way you don't
> > need to typecast the dma_alloc_coherent() and dma_free_coherent().
> > 
> > I believe it might complicate the places where you do math on it, but
> > that is a good thing, as it highlights those places where you do
> > something unexpected.
> > 
> 
> While this not strictly limited to holding a dma_addr_t.
> Based on historical behavior, when the FASTRPC_ATTR_SECUREMAP flag is
> set, S2 mapping expects a physical address to be passed to the
> qcom_scm_assign_mem() API.
> reference-
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/misc/fastrpc.c?id=e90d911906196bf987492c94e38f10ca611dfd7b
> 
> If you suggest, I can define it as dma_addr_t and perform typecasting to u64
> wherever required.

You don't need to typecase dma_addr_t when passing u64.


-- 
With best wishes
Dmitry

Re: [PATCH v3 2/4] misc: fastrpc: Rename phys to dma_addr for clarity

[Konrad Dybcio]

On 11/17/25 12:22 PM, Dmitry Baryshkov wrote:
> On Mon, Nov 17, 2025 at 12:37:33PM +0530, Kumari Pallavi wrote:
>>
>>
>> On 11/14/2025 9:14 PM, Bjorn Andersson wrote:
>>> On Fri, Nov 14, 2025 at 02:11:40PM +0530, Kumari Pallavi wrote:
>>>> Update all references of buf->phys and map->phys to buf->dma_addr and
>>>> map->dma_addr to accurately represent that these fields store DMA
>>>> addresses, not physical addresses. This change improves code clarity
>>>> and aligns with kernel conventions for dma_addr_t usage.
>>>>
>>>> Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
>>>> ---
>>>>   drivers/misc/fastrpc.c | 76 ++++++++++++++++++++++--------------------
>>>>   1 file changed, 40 insertions(+), 36 deletions(-)
>>>>
>>>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>>>> index ee652ef01534..d6a7960fe716 100644
>>>> --- a/drivers/misc/fastrpc.c
>>>> +++ b/drivers/misc/fastrpc.c
>>>> @@ -106,7 +106,7 @@
>>>>   #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
>>>>   struct fastrpc_phy_page {
>>>> -	u64 addr;		/* physical address */
>>>> +	u64 addr;		/* physical or dma address */
>>>>   	u64 size;		/* size of contiguous region */
>>>>   };
>>>> @@ -171,7 +171,7 @@ struct fastrpc_msg {
>>>>   	u64 ctx;		/* invoke caller context */
>>>>   	u32 handle;	/* handle to invoke */
>>>>   	u32 sc;		/* scalars structure describing the data */
>>>> -	u64 addr;		/* physical address */
>>>> +	u64 addr;		/* physical or dma address */
>>>
>>> Can you go all the way and make the type dma_addr_t? That way you don't
>>> need to typecast the dma_alloc_coherent() and dma_free_coherent().
>>>
>>> I believe it might complicate the places where you do math on it, but
>>> that is a good thing, as it highlights those places where you do
>>> something unexpected.
>>>
>>
>> While this not strictly limited to holding a dma_addr_t.
>> Based on historical behavior, when the FASTRPC_ATTR_SECUREMAP flag is
>> set, S2 mapping expects a physical address to be passed to the
>> qcom_scm_assign_mem() API.
>> reference-
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/misc/fastrpc.c?id=e90d911906196bf987492c94e38f10ca611dfd7b
>>
>> If you suggest, I can define it as dma_addr_t and perform typecasting to u64
>> wherever required.
> 
> You don't need to typecase dma_addr_t when passing u64.

*on arm anyway

Konrad

Re: [PATCH v3 3/4] misc: fastrpc: Add support for new DSP IOVA formatting

[Bjorn Andersson]

On Mon, Nov 17, 2025 at 12:32:59PM +0530, Kumari Pallavi wrote:
> 
> 
> On 11/14/2025 9:21 PM, Bjorn Andersson wrote:
> > On Fri, Nov 14, 2025 at 02:11:41PM +0530, Kumari Pallavi wrote:
> > > Implement the new IOVA formatting required by the DSP architecture change
> > > on Kaanapali SoC. Place the SID for DSP DMA transactions at bit 56 in the
> > > physical address. This placement is necessary for the DSPs to correctly
> > > identify streams and operate as intended.
> > > To address this, set SID position to bit 56 via OF matching on the fastrpc
> > > node; otherwise, default to legacy 32-bit placement.
> > > This change ensures consistent SID placement across DSPs.
> > > 
> > 
> > In patch 2 I said I think it would be a good idea to separate the two
> > perspectives (Linux/SMMU vs remote addresses).
> > 
> > Looking ta this patch I'm completely convinced that it's the right thing
> > to do!
> > 
> > > Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
> > > ---
> > >   drivers/misc/fastrpc.c | 46 +++++++++++++++++++++++++++++++++++-------
> > >   1 file changed, 39 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> > > index d6a7960fe716..bcf3c7f8d3e9 100644
> > > --- a/drivers/misc/fastrpc.c
> > > +++ b/drivers/misc/fastrpc.c
> > > @@ -33,7 +33,6 @@
> > >   #define FASTRPC_ALIGN		128
> > >   #define FASTRPC_MAX_FDLIST	16
> > >   #define FASTRPC_MAX_CRCLIST	64
> > > -#define FASTRPC_PHYS(p)	((p) & 0xffffffff)
> > >   #define FASTRPC_CTX_MAX (256)
> > >   #define FASTRPC_INIT_HANDLE	1
> > >   #define FASTRPC_DSP_UTILITIES_HANDLE	2
> > > @@ -105,6 +104,15 @@
> > >   #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
> > > +/* Extract smmu pa from consolidated iova */
> > > +#define IPA_TO_DMA_ADDR(iova, sid_pos) (iova & ((1ULL << sid_pos) - 1ULL))
> > > +/*
> > > + * Prepare the consolidated iova to send to dsp by prepending the sid
> > > + * to smmu pa at the appropriate position
> > > + */
> > > +#define IOVA_FROM_SID_PA(sid, phys, sid_pos) \
> > > +       (phys += sid << sid_pos)
> > 
> > This is a horrible macro. It looks just like a function taking values,
> > it's named to sound like it takes a sid and pa and return an iova, but
> > it has side effects.
> > 
> > And what's up with the ordering? Take argument 1 and 3, and put the
> > result in argument 2?!
> > 
> 
> Thank you for the feedback regarding the macro implementation. I understand
> your concern about readability and hidden side effects.
> To address this, I’ve replaced the macro with an inline function
> 
> 
> static inline u64 fastrpc_compute_sid_offset(u64 sid, u32 sid_pos)
> {
>     return sid << sid_pos;
> }
> 
> 
> buf->dma_addr += fastrpc_compute_sid_offset(sid, sid_pos);
> 
> Could you confirm if this is in line with what you suggested?
> 

That is possible to read, so yes that would be much better. 

Regards,
Bjorn

Re: [PATCH v3 2/4] misc: fastrpc: Rename phys to dma_addr for clarity

[Bjorn Andersson]

On Mon, Nov 17, 2025 at 12:37:33PM +0530, Kumari Pallavi wrote:
> 
> 
> On 11/14/2025 9:14 PM, Bjorn Andersson wrote:
> > On Fri, Nov 14, 2025 at 02:11:40PM +0530, Kumari Pallavi wrote:
> > > Update all references of buf->phys and map->phys to buf->dma_addr and
> > > map->dma_addr to accurately represent that these fields store DMA
> > > addresses, not physical addresses. This change improves code clarity
> > > and aligns with kernel conventions for dma_addr_t usage.
> > > 
> > > Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
> > > ---
> > >   drivers/misc/fastrpc.c | 76 ++++++++++++++++++++++--------------------
> > >   1 file changed, 40 insertions(+), 36 deletions(-)
> > > 
> > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> > > index ee652ef01534..d6a7960fe716 100644
> > > --- a/drivers/misc/fastrpc.c
> > > +++ b/drivers/misc/fastrpc.c
> > > @@ -106,7 +106,7 @@
> > >   #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
> > >   struct fastrpc_phy_page {
> > > -	u64 addr;		/* physical address */
> > > +	u64 addr;		/* physical or dma address */
> > >   	u64 size;		/* size of contiguous region */
> > >   };
> > > @@ -171,7 +171,7 @@ struct fastrpc_msg {
> > >   	u64 ctx;		/* invoke caller context */
> > >   	u32 handle;	/* handle to invoke */
> > >   	u32 sc;		/* scalars structure describing the data */
> > > -	u64 addr;		/* physical address */
> > > +	u64 addr;		/* physical or dma address */
> > 
> > Can you go all the way and make the type dma_addr_t? That way you don't
> > need to typecast the dma_alloc_coherent() and dma_free_coherent().
> > 
> > I believe it might complicate the places where you do math on it, but
> > that is a good thing, as it highlights those places where you do
> > something unexpected.
> > 
> 
> While this not strictly limited to holding a dma_addr_t.
> Based on historical behavior, when the FASTRPC_ATTR_SECUREMAP flag is
> set, S2 mapping expects a physical address to be passed to the
> qcom_scm_assign_mem() API.
> reference-
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/misc/fastrpc.c?id=e90d911906196bf987492c94e38f10ca611dfd7b

If I understand correctly, when FASTRPC_ATTR_SECUREMAP is set that
implies that the mapping happens in the firmware SID, i.e. outside of
Linux's view. So it's still a dma_addr_t, but it's mapped 1:1 with the
physical address.

I think this is another good reason to make the changes I suggested
below (which you didn't comment on?). Sometimes this "addr" is the
actual address and sometimes it contains the annotated address.

> 
> If you suggest, I can define it as dma_addr_t and perform typecasting to u64
> wherever required.

Yes, maintain the mapping in a dma_addr_t, then you have the two cases:

1) Contexts with SMMU, you cast the dma_addr_t and annotate it with the
sid information before communicate the addresses to the firmware.

2) Contexts without SMMU, you make it clear that your dma_addr_t is 1:1
with physical addresses and you then pass it to qcom_scm_assign_mem() et
al.


That way the "addr" is always the actual iova (with or without mapping)
and the places where it's treated as a physical address or an annotated
"address" becomes very explicit.

Regards,
Bjorn

> 
> Thanks,
> Pallavi
> 
> 
> > >   	u64 size;		/* size of contiguous region */
> > >   };
> > > @@ -194,7 +194,7 @@ struct fastrpc_buf {
> > >   	struct dma_buf *dmabuf;
> > >   	struct device *dev;
> > >   	void *virt;
> > > -	u64 phys;
> > > +	u64 dma_addr;
> > >   	u64 size;
> > >   	/* Lock for dma buf attachments */
> > >   	struct mutex lock;
> > > @@ -217,7 +217,7 @@ struct fastrpc_map {
> > >   	struct dma_buf *buf;
> > >   	struct sg_table *table;
> > >   	struct dma_buf_attachment *attach;
> > > -	u64 phys;
> > > +	u64 dma_addr;
> > >   	u64 size;
> > >   	void *va;
> > >   	u64 len;
> > > @@ -320,11 +320,12 @@ static void fastrpc_free_map(struct kref *ref)
> > >   			perm.vmid = QCOM_SCM_VMID_HLOS;
> > >   			perm.perm = QCOM_SCM_PERM_RWX;
> > > -			err = qcom_scm_assign_mem(map->phys, map->len,
> > > +			err = qcom_scm_assign_mem(map->dma_addr, map->len,
> > >   				&src_perms, &perm, 1);
> > >   			if (err) {
> > > -				dev_err(map->fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d\n",
> > > -						map->phys, map->len, err);
> > > +				dev_err(map->fl->sctx->dev,
> > > +					"Failed to assign memory dma_addr 0x%llx size 0x%llx err %d\n",
> > > +					map->dma_addr, map->len, err);
> > >   				return;
> > >   			}
> > >   		}
> > > @@ -389,7 +390,7 @@ static int fastrpc_map_lookup(struct fastrpc_user *fl, int fd,
> > >   static void fastrpc_buf_free(struct fastrpc_buf *buf)
> > >   {
> > >   	dma_free_coherent(buf->dev, buf->size, buf->virt,
> > > -			  FASTRPC_PHYS(buf->phys));
> > > +			  FASTRPC_PHYS(buf->dma_addr));
> > >   	kfree(buf);
> > >   }
> > > @@ -408,12 +409,12 @@ static int __fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
> > >   	buf->fl = fl;
> > >   	buf->virt = NULL;
> > > -	buf->phys = 0;
> > > +	buf->dma_addr = 0;
> > >   	buf->size = size;
> > >   	buf->dev = dev;
> > >   	buf->raddr = 0;
> > > -	buf->virt = dma_alloc_coherent(dev, buf->size, (dma_addr_t *)&buf->phys,
> > > +	buf->virt = dma_alloc_coherent(dev, buf->size, (dma_addr_t *)&buf->dma_addr,
> > >   				       GFP_KERNEL);
> > >   	if (!buf->virt) {
> > >   		mutex_destroy(&buf->lock);
> > > @@ -439,7 +440,7 @@ static int fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
> > >   	buf = *obuf;
> > >   	if (fl->sctx && fl->sctx->sid)
> > > -		buf->phys += ((u64)fl->sctx->sid << 32);
> > > +		buf->dma_addr += ((u64)fl->sctx->sid << 32);
> > >   	return 0;
> > >   }
> > > @@ -684,7 +685,7 @@ static int fastrpc_dma_buf_attach(struct dma_buf *dmabuf,
> > >   		return -ENOMEM;
> > >   	ret = dma_get_sgtable(buffer->dev, &a->sgt, buffer->virt,
> > > -			      FASTRPC_PHYS(buffer->phys), buffer->size);
> > > +			      FASTRPC_PHYS(buffer->dma_addr), buffer->size);
> > >   	if (ret < 0) {
> > >   		dev_err(buffer->dev, "failed to get scatterlist from DMA API\n");
> > >   		kfree(a);
> > > @@ -733,7 +734,7 @@ static int fastrpc_mmap(struct dma_buf *dmabuf,
> > >   	dma_resv_assert_held(dmabuf->resv);
> > >   	return dma_mmap_coherent(buf->dev, vma, buf->virt,
> > > -				 FASTRPC_PHYS(buf->phys), size);
> > > +				 FASTRPC_PHYS(buf->dma_addr), size);
> > 
> > In fact, we invoke dma_alloc_coherent() above to get a dma_addr_t, and
> > then we call map, unmap, and free on the lower 32 bits of that
> > address...
> > 
> > In other words, each time we reference dma_addr we have that implicit
> > thing that it's a composit of a dma_addr_t as seen from Linux's point of
> > view (which is matching the addresses in the SMMU page tables) and the
> > adjusted address that we use in communication with the firmware to
> > direct the accesses to the right SID + iova.
> > 
> > I think it would be quite nice to make this more explicit throughout the
> > code, rather than juggling the two perspectives in the same variable.
> > 
> > Regards,
> > Bjorn
> > 
> > >   }
> > >   static const struct dma_buf_ops fastrpc_dma_buf_ops = {
> > > @@ -785,10 +786,10 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
> > >   	map->table = table;
> > >   	if (attr & FASTRPC_ATTR_SECUREMAP) {
> > > -		map->phys = sg_phys(map->table->sgl);
> > > +		map->dma_addr = sg_phys(map->table->sgl);
> > >   	} else {
> > > -		map->phys = sg_dma_address(map->table->sgl);
> > > -		map->phys += ((u64)fl->sctx->sid << 32);
> > > +		map->dma_addr = sg_dma_address(map->table->sgl);
> > > +		map->dma_addr += ((u64)fl->sctx->sid << 32);
> > >   	}
> > >   	for_each_sg(map->table->sgl, sgl, map->table->nents,
> > >   		sgl_index)
> > > @@ -815,10 +816,11 @@ static int fastrpc_map_attach(struct fastrpc_user *fl, int fd,
> > >   		dst_perms[1].vmid = fl->cctx->vmperms[0].vmid;
> > >   		dst_perms[1].perm = QCOM_SCM_PERM_RWX;
> > >   		map->attr = attr;
> > > -		err = qcom_scm_assign_mem(map->phys, (u64)map->len, &src_perms, dst_perms, 2);
> > > +		err = qcom_scm_assign_mem(map->dma_addr, (u64)map->len, &src_perms, dst_perms, 2);
> > >   		if (err) {
> > > -			dev_err(sess->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d\n",
> > > -					map->phys, map->len, err);
> > > +			dev_err(sess->dev,
> > > +				"Failed to assign memory with dma_addr 0x%llx size 0x%llx err %d\n",
> > > +				map->dma_addr, map->len, err);
> > >   			goto map_err;
> > >   		}
> > >   	}
> > > @@ -1009,7 +1011,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
> > >   			struct vm_area_struct *vma = NULL;
> > >   			rpra[i].buf.pv = (u64) ctx->args[i].ptr;
> > > -			pages[i].addr = ctx->maps[i]->phys;
> > > +			pages[i].addr = ctx->maps[i]->dma_addr;
> > >   			mmap_read_lock(current->mm);
> > >   			vma = find_vma(current->mm, ctx->args[i].ptr);
> > > @@ -1036,7 +1038,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
> > >   				goto bail;
> > >   			rpra[i].buf.pv = args - ctx->olaps[oix].offset;
> > > -			pages[i].addr = ctx->buf->phys -
> > > +			pages[i].addr = ctx->buf->dma_addr -
> > >   					ctx->olaps[oix].offset +
> > >   					(pkt_size - rlen);
> > >   			pages[i].addr = pages[i].addr &	PAGE_MASK;
> > > @@ -1068,7 +1070,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
> > >   		list[i].num = ctx->args[i].length ? 1 : 0;
> > >   		list[i].pgidx = i;
> > >   		if (ctx->maps[i]) {
> > > -			pages[i].addr = ctx->maps[i]->phys;
> > > +			pages[i].addr = ctx->maps[i]->dma_addr;
> > >   			pages[i].size = ctx->maps[i]->size;
> > >   		}
> > >   		rpra[i].dma.fd = ctx->args[i].fd;
> > > @@ -1150,7 +1152,7 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx *sctx,
> > >   	msg->ctx = ctx->ctxid | fl->pd;
> > >   	msg->handle = handle;
> > >   	msg->sc = ctx->sc;
> > > -	msg->addr = ctx->buf ? ctx->buf->phys : 0;
> > > +	msg->addr = ctx->buf ? ctx->buf->dma_addr : 0;
> > >   	msg->size = roundup(ctx->msg_sz, PAGE_SIZE);
> > >   	fastrpc_context_get(ctx);
> > > @@ -1306,13 +1308,14 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
> > >   		if (fl->cctx->vmcount) {
> > >   			u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
> > > -			err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
> > > +			err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
> > >   							(u64)fl->cctx->remote_heap->size,
> > >   							&src_perms,
> > >   							fl->cctx->vmperms, fl->cctx->vmcount);
> > >   			if (err) {
> > > -				dev_err(fl->sctx->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d\n",
> > > -					fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
> > > +				dev_err(fl->sctx->dev,
> > > +					"Failed to assign memory with dma_addr 0x%llx size 0x%llx err %d\n",
> > > +					fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
> > >   				goto err_map;
> > >   			}
> > >   			scm_done = true;
> > > @@ -1332,7 +1335,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
> > >   	args[1].length = inbuf.namelen;
> > >   	args[1].fd = -1;
> > > -	pages[0].addr = fl->cctx->remote_heap->phys;
> > > +	pages[0].addr = fl->cctx->remote_heap->dma_addr;
> > >   	pages[0].size = fl->cctx->remote_heap->size;
> > >   	args[2].ptr = (u64)(uintptr_t) pages;
> > > @@ -1361,12 +1364,12 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
> > >   		dst_perms.vmid = QCOM_SCM_VMID_HLOS;
> > >   		dst_perms.perm = QCOM_SCM_PERM_RWX;
> > > -		err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
> > > +		err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
> > >   						(u64)fl->cctx->remote_heap->size,
> > >   						&src_perms, &dst_perms, 1);
> > >   		if (err)
> > > -			dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d\n",
> > > -				fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
> > > +			dev_err(fl->sctx->dev, "Failed to assign memory dma_addr 0x%llx size 0x%llx err %d\n",
> > > +				fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
> > >   	}
> > >   err_map:
> > >   	fastrpc_buf_free(fl->cctx->remote_heap);
> > > @@ -1455,7 +1458,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl,
> > >   	args[2].length = inbuf.filelen;
> > >   	args[2].fd = init.filefd;
> > > -	pages[0].addr = imem->phys;
> > > +	pages[0].addr = imem->dma_addr;
> > >   	pages[0].size = imem->size;
> > >   	args[3].ptr = (u64)(uintptr_t) pages;
> > > @@ -1913,7 +1916,7 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
> > >   	args[0].ptr = (u64) (uintptr_t) &req_msg;
> > >   	args[0].length = sizeof(req_msg);
> > > -	pages.addr = buf->phys;
> > > +	pages.addr = buf->dma_addr;
> > >   	pages.size = buf->size;
> > >   	args[1].ptr = (u64) (uintptr_t) &pages;
> > > @@ -1941,11 +1944,12 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
> > >   	if (req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR && fl->cctx->vmcount) {
> > >   		u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
> > > -		err = qcom_scm_assign_mem(buf->phys, (u64)buf->size,
> > > +		err = qcom_scm_assign_mem(buf->dma_addr, (u64)buf->size,
> > >   			&src_perms, fl->cctx->vmperms, fl->cctx->vmcount);
> > >   		if (err) {
> > > -			dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d",
> > > -					buf->phys, buf->size, err);
> > > +			dev_err(fl->sctx->dev,
> > > +				"Failed to assign memory dma_addr 0x%llx size 0x%llx err %d",
> > > +				buf->dma_addr, buf->size, err);
> > >   			goto err_assign;
> > >   		}
> > >   	}
> > > @@ -2059,7 +2063,7 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp)
> > >   	args[0].ptr = (u64) (uintptr_t) &req_msg;
> > >   	args[0].length = sizeof(req_msg);
> > > -	pages.addr = map->phys;
> > > +	pages.addr = map->dma_addr;
> > >   	pages.size = map->len;
> > >   	args[1].ptr = (u64) (uintptr_t) &pages;
> > > -- 
> > > 2.34.1
> > > 
> > > 
> 

Re: [PATCH v3 4/4] misc: fastrpc: Update dma_bits for CDSP support on Kaanapali SoC

[Bjorn Andersson]

On Mon, Nov 17, 2025 at 02:42:23PM +0530, Kumari Pallavi wrote:
> 
> 
> On 11/14/2025 9:30 PM, Bjorn Andersson wrote:
> > On Fri, Nov 14, 2025 at 02:11:42PM +0530, Kumari Pallavi wrote:
> > > DSP currently supports 32-bit IOVA (32-bit PA + 4-bit SID) for
> > > both Q6 and user DMA (uDMA) access. This is being upgraded to
> > > 34-bit PA + 4-bit SID due to a hardware revision in CDSP for
> > > Kaanapali SoC, which expands the DMA addressable range.
> > > Update DMA bits configuration in the driver to support CDSP on
> > > Kaanapali SoC. Set the default `dma_bits` to 32-bit and update
> > > it to 34-bit based on CDSP and OF matching on the fastrpc node.
> > > 
> > > Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
> > > ---
> > >   drivers/misc/fastrpc.c | 15 +++++++++++++--
> > >   1 file changed, 13 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> > > index bcf3c7f8d3e9..2eb8d37cd9b4 100644
> > > --- a/drivers/misc/fastrpc.c
> > > +++ b/drivers/misc/fastrpc.c
> > > @@ -267,6 +267,8 @@ struct fastrpc_session_ctx {
> > >   struct fastrpc_soc_data {
> > >   	u32 sid_pos;
> > > +	u32 cdsp_dma_bits;
> > > +	u32 dsp_default_dma_bits;
> > >   };
> > >   struct fastrpc_channel_ctx {
> > > @@ -2186,6 +2188,7 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
> > >   	int i, sessions = 0;
> > >   	unsigned long flags;
> > >   	int rc;
> > > +	u32 dma_bits;
> > >   	cctx = dev_get_drvdata(dev->parent);
> > >   	if (!cctx)
> > > @@ -2199,12 +2202,16 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
> > >   		spin_unlock_irqrestore(&cctx->lock, flags);
> > >   		return -ENOSPC;
> > >   	}
> > > +	dma_bits = cctx->soc_data->dsp_default_dma_bits;
> > >   	sess = &cctx->session[cctx->sesscount++];
> > >   	sess->used = false;
> > >   	sess->valid = true;
> > >   	sess->dev = dev;
> > >   	dev_set_drvdata(dev, sess);
> > > +	if (cctx->domain_id == CDSP_DOMAIN_ID)
> > > +		dma_bits = cctx->soc_data->cdsp_dma_bits;
> > > +
> > >   	if (of_property_read_u32(dev->of_node, "reg", &sess->sid))
> > >   		dev_info(dev, "FastRPC Session ID not specified in DT\n");
> > > @@ -2219,9 +2226,9 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
> > >   		}
> > >   	}
> > >   	spin_unlock_irqrestore(&cctx->lock, flags);
> > > -	rc = dma_set_mask(dev, DMA_BIT_MASK(32));
> > > +	rc = dma_set_mask(dev, DMA_BIT_MASK(dma_bits));
> > >   	if (rc) {
> > > -		dev_err(dev, "32-bit DMA enable failed\n");
> > > +		dev_err(dev, "%u-bit DMA enable failed\n", dma_bits);
> > >   		return rc;
> > >   	}
> > > @@ -2308,10 +2315,14 @@ static int fastrpc_get_domain_id(const char *domain)
> > >   static const struct fastrpc_soc_data kaanapali_soc_data = {
> > >   	.sid_pos = 56,
> > > +	.cdsp_dma_bits = 34,
> > > +	.dsp_default_dma_bits = 32,
> > >   };
> > >   static const struct fastrpc_soc_data default_soc_data = {
> > >   	.sid_pos = 32,
> > > +	.cdsp_dma_bits = 32,
> > > +	.dsp_default_dma_bits = 32,
> > 
> > So, "dsp_default_dma_bits" specified "what is the dma_mask for the
> > non-CDSP fastrpc instances"? I don't find "dsp_default" to naturally
> > mean "not the cdsp".
> > 
> > 
> > Wouldn't it be better to introduce two different compatibles, one being
> > the "qcom,kaanapali-fastrpc" and one being the
> > "qcom,kaanapali-cdsp-fastrpc" and then use that to select things here?
> > 
> 
> Thank you for the suggestion. In this case, sid_pos is common across all
> DSP domains on kaanapali Soc. Splitting into two compatibles would lead to
> duplication of these shared property in the DT schema and driver logic.
> The only difference here is the DMA address width for CDSP (34-bit) versus
> other DSPs (32-bit).
> 
> To address the concern about naming, I can provide:
> 
> dma_bits_cdsp → clearly indicates this applies to the CDSP domain.
> dma_bits_non_cdsp (or dma_bits_other_dsp) → for ADSP and other DSP domains.
> Please let me know if this aligns with your suggestion ?
> 

This naming is much better.

I'm not entirely sure about the compatibility part though. The Kaanapali
CSDP and Kaanapali ADSP doesn't have the same DMA address width, so are
they then compatible/the same?

The fact that the two compatibles would refer to something with the same
sid_pos isn't a concern to me. De-duplicating a single constant at the
expense of more complicated logic, that is a concern however.

> > 
> > PS. You store "dma_bits" just for the sake of turning it into a
> > dma_mask, just store the DMA_BIT_MASK() directly here instead.
> > 
> 
> The current approach of assigning a value to cdsp_dma_mask allows for
> adaptable logging behavior, making it easier to trace.
> 

I presume you mean it allows you to do "%u-bit DMA enable failed"?

There are only two options here (32 and 34), and the only reason why
it's not directly obvious which case you're looking at is because you're
"dynamically" deriving that number from something else.

Regards,
Bjorn

> 
> > Regards,
> > Bjorn
> > 
> > >   };
> > >   static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
> > > -- 
> > > 2.34.1
> > > 
> > > 
> 
> Thanks,
> Pallavi

Re: [PATCH v3 4/4] misc: fastrpc: Update dma_bits for CDSP support on Kaanapali SoC

[Kumari Pallavi]

On 11/18/2025 10:27 PM, Bjorn Andersson wrote:
> On Mon, Nov 17, 2025 at 02:42:23PM +0530, Kumari Pallavi wrote:
>>
>>
>> On 11/14/2025 9:30 PM, Bjorn Andersson wrote:
>>> On Fri, Nov 14, 2025 at 02:11:42PM +0530, Kumari Pallavi wrote:
>>>> DSP currently supports 32-bit IOVA (32-bit PA + 4-bit SID) for
>>>> both Q6 and user DMA (uDMA) access. This is being upgraded to
>>>> 34-bit PA + 4-bit SID due to a hardware revision in CDSP for
>>>> Kaanapali SoC, which expands the DMA addressable range.
>>>> Update DMA bits configuration in the driver to support CDSP on
>>>> Kaanapali SoC. Set the default `dma_bits` to 32-bit and update
>>>> it to 34-bit based on CDSP and OF matching on the fastrpc node.
>>>>
>>>> Signed-off-by: Kumari Pallavi <kumari.pallavi@oss.qualcomm.com>
>>>> ---
>>>>    drivers/misc/fastrpc.c | 15 +++++++++++++--
>>>>    1 file changed, 13 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>>>> index bcf3c7f8d3e9..2eb8d37cd9b4 100644
>>>> --- a/drivers/misc/fastrpc.c
>>>> +++ b/drivers/misc/fastrpc.c
>>>> @@ -267,6 +267,8 @@ struct fastrpc_session_ctx {
>>>>    struct fastrpc_soc_data {
>>>>    	u32 sid_pos;
>>>> +	u32 cdsp_dma_bits;
>>>> +	u32 dsp_default_dma_bits;
>>>>    };
>>>>    struct fastrpc_channel_ctx {
>>>> @@ -2186,6 +2188,7 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
>>>>    	int i, sessions = 0;
>>>>    	unsigned long flags;
>>>>    	int rc;
>>>> +	u32 dma_bits;
>>>>    	cctx = dev_get_drvdata(dev->parent);
>>>>    	if (!cctx)
>>>> @@ -2199,12 +2202,16 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
>>>>    		spin_unlock_irqrestore(&cctx->lock, flags);
>>>>    		return -ENOSPC;
>>>>    	}
>>>> +	dma_bits = cctx->soc_data->dsp_default_dma_bits;
>>>>    	sess = &cctx->session[cctx->sesscount++];
>>>>    	sess->used = false;
>>>>    	sess->valid = true;
>>>>    	sess->dev = dev;
>>>>    	dev_set_drvdata(dev, sess);
>>>> +	if (cctx->domain_id == CDSP_DOMAIN_ID)
>>>> +		dma_bits = cctx->soc_data->cdsp_dma_bits;
>>>> +
>>>>    	if (of_property_read_u32(dev->of_node, "reg", &sess->sid))
>>>>    		dev_info(dev, "FastRPC Session ID not specified in DT\n");
>>>> @@ -2219,9 +2226,9 @@ static int fastrpc_cb_probe(struct platform_device *pdev)
>>>>    		}
>>>>    	}
>>>>    	spin_unlock_irqrestore(&cctx->lock, flags);
>>>> -	rc = dma_set_mask(dev, DMA_BIT_MASK(32));
>>>> +	rc = dma_set_mask(dev, DMA_BIT_MASK(dma_bits));
>>>>    	if (rc) {
>>>> -		dev_err(dev, "32-bit DMA enable failed\n");
>>>> +		dev_err(dev, "%u-bit DMA enable failed\n", dma_bits);
>>>>    		return rc;
>>>>    	}
>>>> @@ -2308,10 +2315,14 @@ static int fastrpc_get_domain_id(const char *domain)
>>>>    static const struct fastrpc_soc_data kaanapali_soc_data = {
>>>>    	.sid_pos = 56,
>>>> +	.cdsp_dma_bits = 34,
>>>> +	.dsp_default_dma_bits = 32,
>>>>    };
>>>>    static const struct fastrpc_soc_data default_soc_data = {
>>>>    	.sid_pos = 32,
>>>> +	.cdsp_dma_bits = 32,
>>>> +	.dsp_default_dma_bits = 32,
>>>
>>> So, "dsp_default_dma_bits" specified "what is the dma_mask for the
>>> non-CDSP fastrpc instances"? I don't find "dsp_default" to naturally
>>> mean "not the cdsp".
>>>
>>>
>>> Wouldn't it be better to introduce two different compatibles, one being
>>> the "qcom,kaanapali-fastrpc" and one being the
>>> "qcom,kaanapali-cdsp-fastrpc" and then use that to select things here?
>>>
>>
>> Thank you for the suggestion. In this case, sid_pos is common across all
>> DSP domains on kaanapali Soc. Splitting into two compatibles would lead to
>> duplication of these shared property in the DT schema and driver logic.
>> The only difference here is the DMA address width for CDSP (34-bit) versus
>> other DSPs (32-bit).
>>
>> To address the concern about naming, I can provide:
>>
>> dma_bits_cdsp → clearly indicates this applies to the CDSP domain.
>> dma_bits_non_cdsp (or dma_bits_other_dsp) → for ADSP and other DSP domains.
>> Please let me know if this aligns with your suggestion ?
>>
> 
> This naming is much better.
> 
> I'm not entirely sure about the compatibility part though. The Kaanapali
> CSDP and Kaanapali ADSP doesn't have the same DMA address width, so are
> they then compatible/the same?
> 
> The fact that the two compatibles would refer to something with the same
> sid_pos isn't a concern to me. De-duplicating a single constant at the
> expense of more complicated logic, that is a concern however.
> 

It might be a good idea to replace the domain-based naming with 
functional naming. Currently, CDSP supports an extended DMA address 
width of 34 bits due to specific use cases, but this could change in the 
future for other DSPs as well.
Using names like "dma_addr_bits_extended" and "dma_addr_bits_default" 
would keep the design flexible and independent of domain.

>>>
>>> PS. You store "dma_bits" just for the sake of turning it into a
>>> dma_mask, just store the DMA_BIT_MASK() directly here instead.
>>>
>>
>> The current approach of assigning a value to cdsp_dma_mask allows for
>> adaptable logging behavior, making it easier to trace.
>>
> 
> I presume you mean it allows you to do "%u-bit DMA enable failed"?
> 

Yes, now I can directly use the dma_bits otherwise I have to extract the
dma_bits from DMA_BIT_MASK() just for logging.


> There are only two options here (32 and 34), and the only reason why
> it's not directly obvious which case you're looking at is because you're
> "dynamically" deriving that number from something else.
> 
> Regards,
> Bjorn
> 
>>
>>> Regards,
>>> Bjorn
>>>
>>>>    };
>>>>    static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>>>> -- 
>>>> 2.34.1
>>>>
>>>>
>>
>> Thanks,
>> Pallavi

Thanks
Pallavi