[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cjuzht6hfzmog7xkwbydisl67iuttdo2jy5hndnruzeu3hp22g@dergmterq2an>
Date: Fri, 14 Nov 2025 14:15:57 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Kumari Pallavi <kumari.pallavi@....qualcomm.com>
Cc: kpallavi@....qualcomm.com, srini@...nel.org, amahesh@....qualcomm.com,
arnd@...db.de, gregkh@...uxfoundation.org, quic_bkumar@...cinc.com,
ekansh.gupta@....qualcomm.com, linux-kernel@...r.kernel.org,
quic_chennak@...cinc.com, dri-devel@...ts.freedesktop.org,
linux-arm-msm@...r.kernel.org, jingyi.wang@....qualcomm.com,
aiqun.yu@....qualcomm.com, ktadakam@....qualcomm.com
Subject: Re: [PATCH v3 2/4] misc: fastrpc: Rename phys to dma_addr for clarity
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@....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
Powered by blists - more mailing lists