On 9/25/2025 11:54 AM, Arnd Bergmann wrote:
On Thu, Sep 25, 2025, at 01:46, Jingyi Wang wrote:
dma_free_coherent(buf->dev, buf->size, buf->virt,
- FASTRPC_PHYS(buf->phys));
+ IOVA_TO_PHYS(buf->phys, sid_pos));
kfree(buf);
}
I understand what you are doing, but the naming of the macros
seems a bit confusing: dma_free_coherent() and the related
functions are designed to take an IOVA argument, not a physical
address, so calling IOVA_TO_PHYS() before passing the
address sounds wrong. This is made worse by the naming
of 'buf->phys' that is not a physical address at all
but already transformed twice into a dma_addr_t and
from there into a dma_addr+sid tuple.
Ideally the SID handling would be abstracted behind a custom
dma_map_ops implementation that treats this as a custom
iommu, but if the fastrpc device is the only user of this
address format, I can understand you want to keep this as
a local hack in this driver.
Can you try to come up with some better naming here, and
replace the 'phys' bits with something that is more fitting
for an iova/dma_addr_t?
I will improve the naming for better clarity:
- Replace buf->phys with buf->dma_addr throughout the code.
- Rename the macro IOVA_TO_PHYS() to something more appropriate, like
IPA_TO_DMA_ADDR()
Arnd
Thanks,
Pallavi
