[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <75f6bbad-49a0-4a15-b976-a0450f66a4dc@oss.qualcomm.com>
Date: Thu, 23 Oct 2025 12:50:13 +0530
From: Kumari Pallavi <kumari.pallavi@....qualcomm.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....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 v2 1/3] misc: fastrpc: Rename phys to dma_addr for clarity
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.pallavi@....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
Powered by blists - more mailing lists