diff mbox series

[1/2] dt-bindings: firmware: arm,scmi: support parameter passing in smc/hvc

Message ID 20230409181918.29270-2-quic_nkela@quicinc.com (mailing list archive)
State New, archived
Headers show
Series Allow parameter in smc/hvc calls | expand

Commit Message

Nikunj Kela April 9, 2023, 6:19 p.m. UTC
Currently, smc/hvc calls are made with smc-id only. The parameters are
all set to zeros. This patch defines two optional device tree bindings,
that can be used to pass parameters in smc/hvc calls.

This is useful when multiple scmi instances are used with common smc-id.

Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
---
 .../devicetree/bindings/firmware/arm,scmi.yaml   | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Krzysztof Kozlowski April 10, 2023, 5:20 p.m. UTC | #1
On 09/04/2023 20:19, Nikunj Kela wrote:
> Currently, smc/hvc calls are made with smc-id only. The parameters are
> all set to zeros. This patch defines two optional device tree bindings,
> that can be used to pass parameters in smc/hvc calls.
> 
> This is useful when multiple scmi instances are used with common smc-id.
> 
> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
> ---
>  .../devicetree/bindings/firmware/arm,scmi.yaml   | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> index 5824c43e9893..08c331a79b80 100644
> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> @@ -115,6 +115,22 @@ properties:
>      description:
>        SMC id required when using smc or hvc transports
>  
> +  arm,smc32-params:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description:
> +      An optional parameter list passed in smc32 or hvc32 calls
> +    default: 0
> +    minItems: 1
> +    maxItems: 6
> +
> +  arm,smc64-params:
> +    $ref: /schemas/types.yaml#/definitions/uint64-array
> +    description:
> +      An optional parameter list passed in smc64 or hvc64 calls
> +    default: 0
> +    minItems: 1
> +    maxItems: 6

These do not look like hardware properties and the fact that you need
two properties for the same also points that you tied it to specific SW
interface.

Why this should be board-specific? Actually better question - why this
should be fixed per board? Doesn't my software want to have different
parameters, depending on some other condition?

You also did not provide any DTS user for this, so difficult to judge
usefulness.

Best regards,
Krzysztof
Nikunj Kela April 10, 2023, 5:33 p.m. UTC | #2
On 4/10/2023 10:20 AM, Krzysztof Kozlowski wrote:
> On 09/04/2023 20:19, Nikunj Kela wrote:
>> Currently, smc/hvc calls are made with smc-id only. The parameters are
>> all set to zeros. This patch defines two optional device tree bindings,
>> that can be used to pass parameters in smc/hvc calls.
>>
>> This is useful when multiple scmi instances are used with common smc-id.
>>
>> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
>> ---
>>   .../devicetree/bindings/firmware/arm,scmi.yaml   | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
>> index 5824c43e9893..08c331a79b80 100644
>> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
>> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
>> @@ -115,6 +115,22 @@ properties:
>>       description:
>>         SMC id required when using smc or hvc transports
>>   
>> +  arm,smc32-params:
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>> +    description:
>> +      An optional parameter list passed in smc32 or hvc32 calls
>> +    default: 0
>> +    minItems: 1
>> +    maxItems: 6
>> +
>> +  arm,smc64-params:
>> +    $ref: /schemas/types.yaml#/definitions/uint64-array
>> +    description:
>> +      An optional parameter list passed in smc64 or hvc64 calls
>> +    default: 0
>> +    minItems: 1
>> +    maxItems: 6
> These do not look like hardware properties and the fact that you need
> two properties for the same also points that you tied it to specific SW
> interface.

This is certainly not the H/W property but then smc-id is also not H/W 
property either

but that gets passed via DTB. I could use the same property for both 
however I wasn't sure

which datatype should be used, uint32-array/uint64-array. Moreover, I 
thought if users are

passing parameters, they better know which SMC convention they are using 
hence used two

explicit properties.

> Why this should be board-specific? Actually better question - why this
> should be fixed per board? Doesn't my software want to have different
> parameters, depending on some other condition?

Not sure I follow, I didn't say this is board specific. People can use 
the parameters to pass

whatever their S/W demands. SMC/HVC calls are made by passing parameters 
which is what this patch is enabling.

>
> You also did not provide any DTS user for this, so difficult to judge
> usefulness.

The work is still on going and we will push the dts in few months, 
however that shouldn't stop

making changes in advance.

> Best regards,
> Krzysztof
>
Krzysztof Kozlowski April 11, 2023, 5:17 p.m. UTC | #3
On 10/04/2023 19:33, Nikunj Kela wrote:
> 
> On 4/10/2023 10:20 AM, Krzysztof Kozlowski wrote:
>> On 09/04/2023 20:19, Nikunj Kela wrote:
>>> Currently, smc/hvc calls are made with smc-id only. The parameters are
>>> all set to zeros. This patch defines two optional device tree bindings,
>>> that can be used to pass parameters in smc/hvc calls.
>>>
>>> This is useful when multiple scmi instances are used with common smc-id.
>>>
>>> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
>>> ---
>>>   .../devicetree/bindings/firmware/arm,scmi.yaml   | 16 ++++++++++++++++
>>>   1 file changed, 16 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
>>> index 5824c43e9893..08c331a79b80 100644
>>> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
>>> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
>>> @@ -115,6 +115,22 @@ properties:
>>>       description:
>>>         SMC id required when using smc or hvc transports
>>>   
>>> +  arm,smc32-params:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>>> +    description:
>>> +      An optional parameter list passed in smc32 or hvc32 calls
>>> +    default: 0
>>> +    minItems: 1
>>> +    maxItems: 6
>>> +
>>> +  arm,smc64-params:
>>> +    $ref: /schemas/types.yaml#/definitions/uint64-array
>>> +    description:
>>> +      An optional parameter list passed in smc64 or hvc64 calls
>>> +    default: 0
>>> +    minItems: 1
>>> +    maxItems: 6
>> These do not look like hardware properties and the fact that you need
>> two properties for the same also points that you tied it to specific SW
>> interface.
> 
> This is certainly not the H/W property but then smc-id is also not H/W 
> property either

Yep, maybe it should be also removed?

> 
> but that gets passed via DTB. I could use the same property for both 
> however I wasn't sure
> 
> which datatype should be used, uint32-array/uint64-array. Moreover, I 
> thought if users are
> 
> passing parameters, they better know which SMC convention they are using 
> hence used two
> 
> explicit properties.

Does not solve my concern.

> 
>> Why this should be board-specific? Actually better question - why this
>> should be fixed per board? Doesn't my software want to have different
>> parameters, depending on some other condition?
> 
> Not sure I follow, I didn't say this is board specific.

But putting it in DTS which is customized per board is then board specific.

>  People can use 
> the parameters to pass
> 
> whatever their S/W demands. SMC/HVC calls are made by passing parameters 
> which is what this patch is enabling.

I cannot follow your paragraphs. You cut middle of sentence.

Anyway, if this is to match whatever their SW demands, it is not
Devicetree property.

> 
>>
>> You also did not provide any DTS user for this, so difficult to judge
>> usefulness.
> 
> The work is still on going and we will push the dts in few months, 
> however that shouldn't stop
> 
> making changes in advance.

With a proper DTS maybe it would be easier to convince us why SW
interface should be put to Devicetree... but sure, we can skip DTS and
answer is - this looks like SW property and not a DT.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index 5824c43e9893..08c331a79b80 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -115,6 +115,22 @@  properties:
     description:
       SMC id required when using smc or hvc transports
 
+  arm,smc32-params:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description:
+      An optional parameter list passed in smc32 or hvc32 calls
+    default: 0
+    minItems: 1
+    maxItems: 6
+
+  arm,smc64-params:
+    $ref: /schemas/types.yaml#/definitions/uint64-array
+    description:
+      An optional parameter list passed in smc64 or hvc64 calls
+    default: 0
+    minItems: 1
+    maxItems: 6
+
   linaro,optee-channel-id:
     $ref: /schemas/types.yaml#/definitions/uint32
     description: