Re: [RESEND PATCH v1 1/2] dt-bindings: misc: fastrpc: add fastrpc group IDs property

[Ekansh Gupta <quic_ekangupt@quicinc.com>]

On 6/8/2023 12:14 AM, Krzysztof Kozlowski wrote:
> On 07/06/2023 18:30, Ekansh Gupta wrote:
>> Add "qcom,fastrpc-gids" property to the list of optional properties.
>> This property contains the list of privileged group IDs which is
>> used to offload process to remote subsystem with increased privileges.
> 
> Why or when anyone would use this property?
> 
This property can be used for the use cases which requires prioritized 
access to remote subsystem resources.
>>
>> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
>> ---
>>   Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml b/Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml
>> index 1ab9588..2a2124f 100644
>> --- a/Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml
>> +++ b/Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml
>> @@ -57,6 +57,11 @@ properties:
>>         Virtual machine IDs for remote processor.
>>       $ref: "/schemas/types.yaml#/definitions/uint32-array"
>>   
>> +  qcom,fastrpc-gids:
>> +    description:
>> +      Group IDs for fastrpc.
> 
> You just pasted here property name. It does not help me to understand
> what's this. Explain in description.
> 
I'll add more details here when I re-submit the patch.
>> +    $ref: "/schemas/types.yaml#/definitions/uint32-array"
> 
> Drop quotes.
> 
> missing min/maxItems.
> 
I'll update this in v2. Thanks for reviewing.
>> +
>>     "#address-cells":
>>       const: 1
>>   
>> @@ -120,6 +125,7 @@ examples:
>>               qcom,glink-channels = "fastrpcglink-apps-dsp";
>>               label = "sdsp";
>>               qcom,non-secure-domain;
>> +            qcom,fastrpc-gids = <2908>
> 
> Eh, what does 2908 stands for? Why not 3012 or 1842? How anyone can
> figure this out?
> 
There is no hard restriction for this value to be 2908, it can be 
anything. Just that the process which needs a privileged offload to any 
remote subsystem should have the same gid which is defined here, whether 
it is 2908 or anything else.

The intention of having 2908 in example is because this is used in 
Android world where any process running with 2908 gid can offload as 
privileged process to remote subsystem.
Ref: 
https://review.lineageos.org/c/LineageOS/android_device_motorola_sm7250-common/+/305612/4/config.fs

But, it can be set to anything just that the process also needs to adapt 
to this gid.

Thanks,
Ekansh
> Best regards,
> Krzysztof
>