diff mbox series

[v4,3/4] dt-bindings: arm: Add new compatible for smc/hvc transport for SCMI

Message ID 20230911194359.27547-4-quic_nkela@quicinc.com (mailing list archive)
State Not Applicable
Headers show
Series [v4,1/4] firmware: arm_scmi: Add polling support for completion in smc | expand

Commit Message

Nikunj Kela Sept. 11, 2023, 7:43 p.m. UTC 
Introduce compatible "qcom,scmi-hvc-shmem" for SCMI smc/hvc
transport channel for Qualcomm virtual platforms.
The compatible mandates a shared memory channel.

Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../devicetree/bindings/firmware/arm,scmi.yaml       | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Sudeep Holla Oct. 3, 2023, 10:44 a.m. UTC | #1 
On Mon, Sep 11, 2023 at 12:43:58PM -0700, Nikunj Kela wrote:
> Introduce compatible "qcom,scmi-hvc-shmem" for SCMI smc/hvc
> transport channel for Qualcomm virtual platforms.
> The compatible mandates a shared memory channel.
> 
> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../devicetree/bindings/firmware/arm,scmi.yaml       | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> index 8d54ea768d38..4090240f45b1 100644
> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> @@ -45,6 +45,9 @@ properties:
>        - description: SCMI compliant firmware with OP-TEE transport
>          items:
>            - const: linaro,scmi-optee
> +      - description: SCMI compliant firmware with Qualcomm hvc/shmem transport
> +        items:
> +          - const: qcom,scmi-hvc-shmem

Can it be simply "qcom,scmi-smc" for 2 reasons ?
1. We don't support SMC/HVC without shmem, so what is your argument to add
   '-shmem' in the compatible here ?
2. The exact conduit(SMC/HVC) used is detected runtime, so I prefer to keep
  '-smc' instead of '-hvc' in the compatible just to avoid giving an illusion
  that HVC is the conduit chosen here based on the compatible. It can be true
  for other reason but I don't want to mislead here by using HVC.
>
>    interrupts:
>      description:
> @@ -320,6 +323,15 @@ allOf:
>        required:
>          - linaro,optee-channel-id
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: qcom,scmi-hvc-shmem
> +    then:
> +      required:
> +        - shmem
> +
>  examples:
>    - |
>      firmware {
> -- 
> 2.17.1
>
Nikunj Kela Oct. 3, 2023, 3:59 p.m. UTC | #2 
On 10/3/2023 3:44 AM, Sudeep Holla wrote:
> On Mon, Sep 11, 2023 at 12:43:58PM -0700, Nikunj Kela wrote:
>> Introduce compatible "qcom,scmi-hvc-shmem" for SCMI smc/hvc
>> transport channel for Qualcomm virtual platforms.
>> The compatible mandates a shared memory channel.
>>
>> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>   .../devicetree/bindings/firmware/arm,scmi.yaml       | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
>> index 8d54ea768d38..4090240f45b1 100644
>> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
>> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
>> @@ -45,6 +45,9 @@ properties:
>>         - description: SCMI compliant firmware with OP-TEE transport
>>           items:
>>             - const: linaro,scmi-optee
>> +      - description: SCMI compliant firmware with Qualcomm hvc/shmem transport
>> +        items:
>> +          - const: qcom,scmi-hvc-shmem
> Can it be simply "qcom,scmi-smc" for 2 reasons ?
> 1. We don't support SMC/HVC without shmem, so what is your argument to add
>     '-shmem' in the compatible here ?

In our platforms, there are multiple ways to allocate memory. One is 
preallocated shmem as used here, another is dynamically by hypervisor 
APIs. shmem was to just to indicate it is preallocated.


> 2. The exact conduit(SMC/HVC) used is detected runtime, so I prefer to keep
>    '-smc' instead of '-hvc' in the compatible just to avoid giving an illusion
>    that HVC is the conduit chosen here based on the compatible. It can be true
>    for other reason but I don't want to mislead here by using HVC.

IUUC, currently, conduit comes from PSCI dt node. We have been using smc 
for PSCI but want to use hvc here. That being said, I am fine to explore 
if we can change PSCI to use hvc too.


>>     interrupts:
>>       description:
>> @@ -320,6 +323,15 @@ allOf:
>>         required:
>>           - linaro,optee-channel-id
>>   
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: qcom,scmi-hvc-shmem
>> +    then:
>> +      required:
>> +        - shmem
>> +
>>   examples:
>>     - |
>>       firmware {
>> -- 
>> 2.17.1
>>
Sudeep Holla Oct. 4, 2023, 3:53 p.m. UTC | #3 
On Tue, Oct 03, 2023 at 08:59:45AM -0700, Nikunj Kela wrote:
> 
> On 10/3/2023 3:44 AM, Sudeep Holla wrote:
> > On Mon, Sep 11, 2023 at 12:43:58PM -0700, Nikunj Kela wrote:
> > > Introduce compatible "qcom,scmi-hvc-shmem" for SCMI smc/hvc
> > > transport channel for Qualcomm virtual platforms.
> > > The compatible mandates a shared memory channel.
> > > 
> > > Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
> > > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > ---
> > >   .../devicetree/bindings/firmware/arm,scmi.yaml       | 12 ++++++++++++
> > >   1 file changed, 12 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > index 8d54ea768d38..4090240f45b1 100644
> > > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > @@ -45,6 +45,9 @@ properties:
> > >         - description: SCMI compliant firmware with OP-TEE transport
> > >           items:
> > >             - const: linaro,scmi-optee
> > > +      - description: SCMI compliant firmware with Qualcomm hvc/shmem transport
> > > +        items:
> > > +          - const: qcom,scmi-hvc-shmem
> > Can it be simply "qcom,scmi-smc" for 2 reasons ?
> > 1. We don't support SMC/HVC without shmem, so what is your argument to add
> >     '-shmem' in the compatible here ?
> 
> In our platforms, there are multiple ways to allocate memory. One is
> preallocated shmem as used here, another is dynamically by hypervisor APIs.
> shmem was to just to indicate it is preallocated.
>

Let us keep it without shmem. If it is dynamically allocated, you must not
need another compatible as you can check it at the runtime.

> 
> > 2. The exact conduit(SMC/HVC) used is detected runtime, so I prefer to keep
> >    '-smc' instead of '-hvc' in the compatible just to avoid giving an illusion
> >    that HVC is the conduit chosen here based on the compatible. It can be true
> >    for other reason but I don't want to mislead here by using HVC.
> 
> IUUC, currently, conduit comes from PSCI dt node. We have been using smc for
> PSCI but want to use hvc here. That being said, I am fine to explore if we
> can change PSCI to use hvc too.
>

I think only OPTEE has explicit conduit other than PSCI and it is continued
for legacy/compatibility reasons IIUC and IIRC. Anything else depends on
the conduit used by PSCI to be consistent. So yes you need to use what the
PSCI conduit is and you don't need the extra information from the DT either
as new property or in the compatible.
Nikunj Kela Oct. 5, 2023, 9:51 p.m. UTC | #4 
On 10/4/2023 8:53 AM, Sudeep Holla wrote:
> On Tue, Oct 03, 2023 at 08:59:45AM -0700, Nikunj Kela wrote:
>> On 10/3/2023 3:44 AM, Sudeep Holla wrote:
>>> On Mon, Sep 11, 2023 at 12:43:58PM -0700, Nikunj Kela wrote:
>>>> Introduce compatible "qcom,scmi-hvc-shmem" for SCMI smc/hvc
>>>> transport channel for Qualcomm virtual platforms.
>>>> The compatible mandates a shared memory channel.
>>>>
>>>> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
>>>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> ---
>>>>    .../devicetree/bindings/firmware/arm,scmi.yaml       | 12 ++++++++++++
>>>>    1 file changed, 12 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
>>>> index 8d54ea768d38..4090240f45b1 100644
>>>> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
>>>> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
>>>> @@ -45,6 +45,9 @@ properties:
>>>>          - description: SCMI compliant firmware with OP-TEE transport
>>>>            items:
>>>>              - const: linaro,scmi-optee
>>>> +      - description: SCMI compliant firmware with Qualcomm hvc/shmem transport
>>>> +        items:
>>>> +          - const: qcom,scmi-hvc-shmem
>>> Can it be simply "qcom,scmi-smc" for 2 reasons ?
>>> 1. We don't support SMC/HVC without shmem, so what is your argument to add
>>>      '-shmem' in the compatible here ?
>> In our platforms, there are multiple ways to allocate memory. One is
>> preallocated shmem as used here, another is dynamically by hypervisor APIs.
>> shmem was to just to indicate it is preallocated.
>>
> Let us keep it without shmem. If it is dynamically allocated, you must not
> need another compatible as you can check it at the runtime.
>
>>> 2. The exact conduit(SMC/HVC) used is detected runtime, so I prefer to keep
>>>     '-smc' instead of '-hvc' in the compatible just to avoid giving an illusion
>>>     that HVC is the conduit chosen here based on the compatible. It can be true
>>>     for other reason but I don't want to mislead here by using HVC.
>> IUUC, currently, conduit comes from PSCI dt node. We have been using smc for
>> PSCI but want to use hvc here. That being said, I am fine to explore if we
>> can change PSCI to use hvc too.
>>
> I think only OPTEE has explicit conduit other than PSCI and it is continued
> for legacy/compatibility reasons IIUC and IIRC. Anything else depends on
> the conduit used by PSCI to be consistent. So yes you need to use what the
> PSCI conduit is and you don't need the extra information from the DT either
> as new property or in the compatible.

Ok, will use conduit then. Thanks!


>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index 8d54ea768d38..4090240f45b1 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -45,6 +45,9 @@  properties:
       - description: SCMI compliant firmware with OP-TEE transport
         items:
           - const: linaro,scmi-optee
+      - description: SCMI compliant firmware with Qualcomm hvc/shmem transport
+        items:
+          - const: qcom,scmi-hvc-shmem
 
   interrupts:
     description:
@@ -320,6 +323,15 @@  allOf:
       required:
         - linaro,optee-channel-id
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: qcom,scmi-hvc-shmem
+    then:
+      required:
+        - shmem
+
 examples:
   - |
     firmware {