On Tue, Jun 24, 2025 at 01:58:47PM +0800, Ling Xu wrote:
> 在 6/23/2025 6:28 PM, Konrad Dybcio 写道:
> > On 6/22/25 3:38 PM, Ling Xu wrote:
> >> The fastrpc driver has support for 5 types of remoteprocs. There are
> >> some products which support GDSP remoteprocs. Add changes to support
> >> GDSP remoteprocs.
> >
> > Commit messages saying "add changes to support xyz" often indicate
> > the problem or the non-obvious solution is not properly described
> > (which is the case here as well)
> >
> > [...]
> >
>
> Okay, I will change to
> "Add related domain IDS to support GDSP remoteprocs."
>
> >> +static int fastrpc_get_domain_id(const char *domain)
> >> +{
> >> + if (strncmp(domain, "adsp", 4) == 0)
> >
> > if (!strncmp(...)) is the common syntax, although it's obviously
> > not functionally different
> >
> >> + return ADSP_DOMAIN_ID;
> >> + else if (strncmp(domain, "cdsp", 4) == 0)
> >> + return CDSP_DOMAIN_ID;
> >> + else if (strncmp(domain, "mdsp", 4) == 0)
> >> + return MDSP_DOMAIN_ID;
> >> + else if (strncmp(domain, "sdsp", 4) == 0)
> >> + return SDSP_DOMAIN_ID;
> >> + else if (strncmp(domain, "gdsp", 4) == 0)
> >> + return GDSP_DOMAIN_ID;
> >
> > FWIW, other places call it G*P*DSP
> >
> In fastrpc, we call it GDSP to match dsp side.
> because in device,the related path for gdsp images are gdsp and gdsp0.
> > [...]
> >
> >> --- a/include/uapi/misc/fastrpc.h
> >> +++ b/include/uapi/misc/fastrpc.h
> >> @@ -18,6 +18,14 @@
> >> #define FASTRPC_IOCTL_MEM_UNMAP _IOWR('R', 11, struct
> >> fastrpc_mem_unmap)
> >> #define FASTRPC_IOCTL_GET_DSP_INFO _IOWR('R', 13, struct
> >> fastrpc_ioctl_capability)
> >>
> >> +#define ADSP_DOMAIN_ID (0)
> >> +#define MDSP_DOMAIN_ID (1)
> >> +#define SDSP_DOMAIN_ID (2)
> >> +#define CDSP_DOMAIN_ID (3)
> >> +#define GDSP_DOMAIN_ID (4)
> >> +
> >> +#define FASTRPC_DOMAIN_MAX 4
> >
> > What are these used for now?
> >
> To get proper domain IDs for fastrpc_rpmsg_probe etc.
These seem to be driver-internal, so they don't need to be exposed to
userspace.
> >> /**
> >> * enum fastrpc_map_flags - control flags for mapping memory on DSP user
> >> process
> >> * @FASTRPC_MAP_STATIC: Map memory pages with RW- permission and CACHE
> >> WRITEBACK.
> >> @@ -134,10 +142,9 @@ struct fastrpc_mem_unmap {
> >> };
> >>
> >> struct fastrpc_ioctl_capability {
> >> - __u32 domain;
> >> __u32 attribute_id;
> >> __u32 capability; /* dsp capability */
> >> - __u32 reserved[4];
> >> + __u32 reserved[5];
> >
> > This is an ABI break, as the data within structs is well, structured
>
> this is suggested by Dmitry, I will have a discussion internally.
No, I didn't suggest to break the ABI. I suggested making the domain
field reserved.
> >
> > Konrad
>
> --
> Thx and BRs,
> Ling Xu
>
--
With best wishes
Dmitry
