Re: [PATCH v4 3/4] misc: fastrpc: Refactor domain ID to enforce strict mapping

[Srinivas Kandagatla <srini@kernel.org>]

On 6/27/25 1:23 PM, Konrad Dybcio wrote:
> On 6/27/25 12:33 PM, Ling Xu wrote:
>> Currently, domain ids are added for each instance, which is not a scalable.
> 
> 's/ a//g'
> 
> [...]
> 
>> -		/* Unsigned PD offloading is only supported on CDSP and CDSP1 */
>> +		/* Unsigned PD offloading is only supported on CDSP*/
> 
> missing space before comment end
> 
> [...]
> 
>> index f33d914d8f46..b890f8042e86 100644
>> --- a/include/uapi/misc/fastrpc.h
>> +++ b/include/uapi/misc/fastrpc.h
>> @@ -18,6 +18,13 @@
>>  #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)
> 
Pl move the defines back to driver, see below comments.

> No need to include parentheses, as you're not interacting with any
> variables
> 
>> +#define FASTRPC_DOMAIN_MAX    3
> 
> #define FASTRPC_DOMAIN_MAX CDSP_DOMAIN_ID?
> 
> What I meant in the previous revision is that you can not move
> data inside the fastrpc_ioctl_capability struct, but you can
> definitely add a comment like:
> 
> struct fastrpc_ioctl_capability {
>         __u32 domain; /* Retired in v6.whatever, now ignored by the kernel */

hmm, If the plan is to make this field deprecated then why are we adding
the defines to UAPI, it does not make sense.
Also like Konrad suggested
>         __u32 domain; /* deprecated, ignored by the kernel */


Also please move this change as a new patch incase you plan to add the
deprecation along with checks in the kernel to make sure no one is
actually passing data in this member.

--srini


>         __u32 attribute_id;
>         __u32 capability;   /* dsp capability */
>         __u32 reserved[4];
> };
> 
> Konrad