Re: [PATCH v2 1/3] misc: fastrpc: Rename phys to dma_addr for clarity

[Kumari Pallavi]

On 10/15/2025 3:37 PM, Dmitry Baryshkov wrote:
On Wed, Oct 15, 2025 at 10:27:00AM +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.pall...@oss.qualcomm.com>
---
  drivers/misc/fastrpc.c | 68 +++++++++++++++++++++---------------------
  1 file changed, 34 insertions(+), 34 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 621bce7e101c..975be54a2491 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -194,7 +194,7 @@ struct fastrpc_buf {
        struct dma_buf *dmabuf;
        struct device *dev;
        void *virt;
-       u64 phys;
+       u64 dma_addr;

If it is dma_addr, why isn't it dma_addr_t?


While the field is named dma_addr, it is not strictly limited to holding 
a DMA address.
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


        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;

And this one.

        u64 size;
        void *va;
        u64 len;
@@ -406,12 +406,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 it was dma_addr_t, you wouldn't have had to typecast here.

        if (!buf->virt) {
                mutex_destroy(&buf->lock);
@@ -437,7 +437,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;
  }
@@ -682,7 +682,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);

FASTRPC_PHYS trunates addr to 32 bits. Is it expected? Is it a DMA
address on the  Linux or on the DSP side?


Yes, it is expected as the device can address up to 32 bit memory range. 
It is a DMA address on the Linux.

        if (ret < 0) {
                dev_err(buffer->dev, "failed to get scatterlist from DMA 
API\n");
                kfree(a);


Thanks,
Pallavi