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 <[email protected]> > > > --- > > > 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
