在 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.
>> /**
>> * 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.
>
> Konrad
--
Thx and BRs,
Ling Xu
