diff mbox series

[1/4] dt-bindings: remoteproc: qcom: adsp: document sm8550 adsp, cdsp & mpss compatible

Message ID 20221114-narmstrong-sm8550-upstream-remoteproc-v1-1-104c34cb3b91@linaro.org
State New
Headers
Series remoteproc: qcom_q6v5_pas: add support for SM8550 adsp, cdsp & mpss |

Commit Message

Neil Armstrong Nov. 16, 2022, 10:20 a.m. UTC 
  This documents the compatible for the component used to boot the
aDSP, cDSP and MPSS on the SM8550 SoC.

The SM8550 boot process on SM8550 now requires a secondary "Devicetree"
firmware to be passed along the main Firmware, and the cDSP a new power
domain named "NSP".

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 .../devicetree/bindings/remoteproc/qcom,adsp.yaml  | 60 +++++++++++++++++++++-
 1 file changed, 59 insertions(+), 1 deletion(-)

Comments

Krzysztof Kozlowski Nov. 16, 2022, 12:28 p.m. UTC | #1 
On 16/11/2022 11:20, Neil Armstrong wrote:
> This documents the compatible for the component used to boot the
> aDSP, cDSP and MPSS on the SM8550 SoC.
> 
> The SM8550 boot process on SM8550 now requires a secondary "Devicetree"
> firmware to be passed along the main Firmware, and the cDSP a new power
> domain named "NSP".
> 
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>  .../devicetree/bindings/remoteproc/qcom,adsp.yaml  | 60 +++++++++++++++++++++-
>  1 file changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
> index db9e0f0c2bea..678cb73f10de 100644
> --- a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
> @@ -55,6 +55,9 @@ properties:
>        - qcom,sm8450-cdsp-pas
>        - qcom,sm8450-mpss-pas
>        - qcom,sm8450-slpi-pas
> +      - qcom,sm8550-adsp-pas
> +      - qcom,sm8550-cdsp-pas
> +      - qcom,sm8550-mpss-pas
>  
>    reg:
>      maxItems: 1
> @@ -116,8 +119,13 @@ properties:
>      $ref: /schemas/types.yaml#/definitions/string
>      description: Firmware name for the Hexagon core
>  
> +  qcom,dtb-firmware-name:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description: Devicetree Firmware name for the Hexagon core

Not sure about this one.

Rob,
Don't we want rather to have multiple items in firmware-name?


> +
>    memory-region:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
>      description: Reference to the reserved-memory for the Hexagon core
>  
>    qcom,qmp:
> @@ -212,6 +220,9 @@ allOf:
>                - qcom,sm8450-cdsp-pas
>                - qcom,sm8450-slpi-pas
>                - qcom,sm8450-mpss-pas
> +              - qcom,sm8550-adsp-pas
> +              - qcom,sm8550-cdsp-pas
> +              - qcom,sm8550-mpss-pas
>      then:
>        properties:
>          clocks:
> @@ -327,6 +338,8 @@ allOf:
>                - qcom,sm8450-adsp-pas
>                - qcom,sm8450-cdsp-pas
>                - qcom,sm8450-slpi-pas
> +              - qcom,sm8550-adsp-pas
> +              - qcom,sm8550-cdsp-pas
>      then:
>        properties:
>          interrupts:
> @@ -347,6 +360,7 @@ allOf:
>                - qcom,sm8150-mpss-pas
>                - qcom,sm8350-mpss-pas
>                - qcom,sm8450-mpss-pas
> +              - qcom,sm8550-mpss-pas
>      then:
>        properties:
>          interrupts:
> @@ -448,6 +462,7 @@ allOf:
>                - qcom,sm8150-mpss-pas
>                - qcom,sm8350-mpss-pas
>                - qcom,sm8450-mpss-pas
> +              - qcom,sm8550-mpss-pas
>      then:
>        properties:
>          power-domains:
> @@ -475,6 +490,7 @@ allOf:
>                - qcom,sm8350-slpi-pas
>                - qcom,sm8450-adsp-pas
>                - qcom,sm8450-slpi-pas
> +              - qcom,sm8550-adsp-pas
>      then:
>        properties:
>          power-domains:
> @@ -504,6 +520,25 @@ allOf:
>              - const: cx
>              - const: mxc
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sm8550-cdsp-pas
> +    then:
> +      properties:
> +        power-domains:
> +          items:
> +            - description: CX power domain
> +            - description: MXC power domain
> +            - description: NSP power domain
> +        power-domain-names:
> +          items:
> +            - const: cx
> +            - const: mxc
> +            - const: nsp
> +

You also need to update entry for resets. I think it is missing.

>    - if:
>        properties:
>          compatible:
> @@ -573,6 +608,29 @@ allOf:
>        properties:
>          qcom,qmp: false
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sm8550-adsp-pas
> +              - qcom,sm8550-cdsp-pas
> +              - qcom,sm8550-mpss-pas
> +    then:
> +      properties:
> +        memory-region:
> +          minItems: 2
> +          description:
> +            First entry is a phandle for a reserved memory area that holds
> +            the main Firmware for authentication, and second entry a phandle for a
> +            reserved memory area that holds the Devicetree Firmware for authentication.

Instead of minItems and description:
  items:
    - description: Main Firmware for auth....
    - description: Devicetree Firmware....

> +    else:
> +      properties:
> +        qcom,dtb-firmware-name: false
> +
> +        memory-region:
> +          maxItems: 1
> +

Best regards,
Krzysztof
Rob Herring Nov. 16, 2022, 11:39 p.m. UTC | #2 
On Wed, Nov 16, 2022 at 01:28:11PM +0100, Krzysztof Kozlowski wrote:
> On 16/11/2022 11:20, Neil Armstrong wrote:
> > This documents the compatible for the component used to boot the
> > aDSP, cDSP and MPSS on the SM8550 SoC.
> > 
> > The SM8550 boot process on SM8550 now requires a secondary "Devicetree"
> > firmware to be passed along the main Firmware, and the cDSP a new power
> > domain named "NSP".
> > 
> > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> > ---
> >  .../devicetree/bindings/remoteproc/qcom,adsp.yaml  | 60 +++++++++++++++++++++-
> >  1 file changed, 59 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
> > index db9e0f0c2bea..678cb73f10de 100644
> > --- a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
> > +++ b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
> > @@ -55,6 +55,9 @@ properties:
> >        - qcom,sm8450-cdsp-pas
> >        - qcom,sm8450-mpss-pas
> >        - qcom,sm8450-slpi-pas
> > +      - qcom,sm8550-adsp-pas
> > +      - qcom,sm8550-cdsp-pas
> > +      - qcom,sm8550-mpss-pas
> >  
> >    reg:
> >      maxItems: 1
> > @@ -116,8 +119,13 @@ properties:
> >      $ref: /schemas/types.yaml#/definitions/string
> >      description: Firmware name for the Hexagon core
> >  
> > +  qcom,dtb-firmware-name:
> > +    $ref: /schemas/types.yaml#/definitions/string
> > +    description: Devicetree Firmware name for the Hexagon core
> 
> Not sure about this one.
> 
> Rob,
> Don't we want rather to have multiple items in firmware-name?

Yes, I think we already have that for some users. Should have been 
'firmware-names' I guess but I don't think it's worth dealing with 
another case of handling both (forever).

Rob
Neil Armstrong Nov. 17, 2022, 9:02 a.m. UTC | #3 
On 17/11/2022 00:39, Rob Herring wrote:
> On Wed, Nov 16, 2022 at 01:28:11PM +0100, Krzysztof Kozlowski wrote:
>> On 16/11/2022 11:20, Neil Armstrong wrote:
>>> This documents the compatible for the component used to boot the
>>> aDSP, cDSP and MPSS on the SM8550 SoC.
>>>
>>> The SM8550 boot process on SM8550 now requires a secondary "Devicetree"
>>> firmware to be passed along the main Firmware, and the cDSP a new power
>>> domain named "NSP".
>>>
>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>> ---
>>>   .../devicetree/bindings/remoteproc/qcom,adsp.yaml  | 60 +++++++++++++++++++++-
>>>   1 file changed, 59 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
>>> index db9e0f0c2bea..678cb73f10de 100644
>>> --- a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
>>> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
>>> @@ -55,6 +55,9 @@ properties:
>>>         - qcom,sm8450-cdsp-pas
>>>         - qcom,sm8450-mpss-pas
>>>         - qcom,sm8450-slpi-pas
>>> +      - qcom,sm8550-adsp-pas
>>> +      - qcom,sm8550-cdsp-pas
>>> +      - qcom,sm8550-mpss-pas
>>>   
>>>     reg:
>>>       maxItems: 1
>>> @@ -116,8 +119,13 @@ properties:
>>>       $ref: /schemas/types.yaml#/definitions/string
>>>       description: Firmware name for the Hexagon core
>>>   
>>> +  qcom,dtb-firmware-name:
>>> +    $ref: /schemas/types.yaml#/definitions/string
>>> +    description: Devicetree Firmware name for the Hexagon core
>>
>> Not sure about this one.
>>
>> Rob,
>> Don't we want rather to have multiple items in firmware-name?
> 
> Yes, I think we already have that for some users. Should have been
> 'firmware-names' I guess but I don't think it's worth dealing with
> another case of handling both (forever).

I'll be happy to switch to a single property but yeah, firmware-name isn't right
for multiple names...

Anyway, will follow qcom,sc7180-mss-pil.yaml since they already use 2 entries there.

Neil

> 
> Rob
Neil Armstrong Nov. 17, 2022, 9:22 a.m. UTC | #4 
On 16/11/2022 13:28, Krzysztof Kozlowski wrote:
> On 16/11/2022 11:20, Neil Armstrong wrote:
>> This documents the compatible for the component used to boot the
>> aDSP, cDSP and MPSS on the SM8550 SoC.
>>
>> The SM8550 boot process on SM8550 now requires a secondary "Devicetree"
>> firmware to be passed along the main Firmware, and the cDSP a new power
>> domain named "NSP".
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>>   .../devicetree/bindings/remoteproc/qcom,adsp.yaml  | 60 +++++++++++++++++++++-
>>   1 file changed, 59 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
>> index db9e0f0c2bea..678cb73f10de 100644
>> --- a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
>> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
>> @@ -55,6 +55,9 @@ properties:
>>         - qcom,sm8450-cdsp-pas
>>         - qcom,sm8450-mpss-pas
>>         - qcom,sm8450-slpi-pas
>> +      - qcom,sm8550-adsp-pas
>> +      - qcom,sm8550-cdsp-pas
>> +      - qcom,sm8550-mpss-pas
>>   
>>     reg:
>>       maxItems: 1
>> @@ -116,8 +119,13 @@ properties:
>>       $ref: /schemas/types.yaml#/definitions/string
>>       description: Firmware name for the Hexagon core
>>   
>> +  qcom,dtb-firmware-name:
>> +    $ref: /schemas/types.yaml#/definitions/string
>> +    description: Devicetree Firmware name for the Hexagon core
> 
> Not sure about this one.
> 
> Rob,
> Don't we want rather to have multiple items in firmware-name?
> 
> 
>> +
>>     memory-region:
>> -    maxItems: 1
>> +    minItems: 1
>> +    maxItems: 2
>>       description: Reference to the reserved-memory for the Hexagon core
>>   
>>     qcom,qmp:
>> @@ -212,6 +220,9 @@ allOf:
>>                 - qcom,sm8450-cdsp-pas
>>                 - qcom,sm8450-slpi-pas
>>                 - qcom,sm8450-mpss-pas
>> +              - qcom,sm8550-adsp-pas
>> +              - qcom,sm8550-cdsp-pas
>> +              - qcom,sm8550-mpss-pas
>>       then:
>>         properties:
>>           clocks:
>> @@ -327,6 +338,8 @@ allOf:
>>                 - qcom,sm8450-adsp-pas
>>                 - qcom,sm8450-cdsp-pas
>>                 - qcom,sm8450-slpi-pas
>> +              - qcom,sm8550-adsp-pas
>> +              - qcom,sm8550-cdsp-pas
>>       then:
>>         properties:
>>           interrupts:
>> @@ -347,6 +360,7 @@ allOf:
>>                 - qcom,sm8150-mpss-pas
>>                 - qcom,sm8350-mpss-pas
>>                 - qcom,sm8450-mpss-pas
>> +              - qcom,sm8550-mpss-pas
>>       then:
>>         properties:
>>           interrupts:
>> @@ -448,6 +462,7 @@ allOf:
>>                 - qcom,sm8150-mpss-pas
>>                 - qcom,sm8350-mpss-pas
>>                 - qcom,sm8450-mpss-pas
>> +              - qcom,sm8550-mpss-pas
>>       then:
>>         properties:
>>           power-domains:
>> @@ -475,6 +490,7 @@ allOf:
>>                 - qcom,sm8350-slpi-pas
>>                 - qcom,sm8450-adsp-pas
>>                 - qcom,sm8450-slpi-pas
>> +              - qcom,sm8550-adsp-pas
>>       then:
>>         properties:
>>           power-domains:
>> @@ -504,6 +520,25 @@ allOf:
>>               - const: cx
>>               - const: mxc
>>   
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - qcom,sm8550-cdsp-pas
>> +    then:
>> +      properties:
>> +        power-domains:
>> +          items:
>> +            - description: CX power domain
>> +            - description: MXC power domain
>> +            - description: NSP power domain
>> +        power-domain-names:
>> +          items:
>> +            - const: cx
>> +            - const: mxc
>> +            - const: nsp
>> +
> 
> You also need to update entry for resets. I think it is missing.

Hmm no, no resets needed for sm8550.

> 
>>     - if:
>>         properties:
>>           compatible:
>> @@ -573,6 +608,29 @@ allOf:
>>         properties:
>>           qcom,qmp: false
>>   
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - qcom,sm8550-adsp-pas
>> +              - qcom,sm8550-cdsp-pas
>> +              - qcom,sm8550-mpss-pas
>> +    then:
>> +      properties:
>> +        memory-region:
>> +          minItems: 2
>> +          description:
>> +            First entry is a phandle for a reserved memory area that holds
>> +            the main Firmware for authentication, and second entry a phandle for a
>> +            reserved memory area that holds the Devicetree Firmware for authentication.
> 
> Instead of minItems and description:
>    items:
>      - description: Main Firmware for auth....
>      - description: Devicetree Firmware....

Ack

> 
>> +    else:
>> +      properties:
>> +        qcom,dtb-firmware-name: false
>> +
>> +        memory-region:
>> +          maxItems: 1
>> +

I'll rebase on top of 20221116155416.164239-1-krzysztof.kozlowski@linaro.org.

Seems I should perhaps add a separate qcom,sm8550-pas.yaml right, or adding the qcom,sm6350-pas.yam would be ok ?

> 
> Best regards,
> Krzysztof
> 

Thanks,
Neil
Krzysztof Kozlowski Nov. 17, 2022, 9:32 a.m. UTC | #5 
On 17/11/2022 10:22, Neil Armstrong wrote:
>>>   
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - qcom,sm8550-cdsp-pas
>>> +    then:
>>> +      properties:
>>> +        power-domains:
>>> +          items:
>>> +            - description: CX power domain
>>> +            - description: MXC power domain
>>> +            - description: NSP power domain
>>> +        power-domain-names:
>>> +          items:
>>> +            - const: cx
>>> +            - const: mxc
>>> +            - const: nsp
>>> +
>>
>> You also need to update entry for resets. I think it is missing.
> 
> Hmm no, no resets needed for sm8550.

Indeed, only few variants update resets. I'll fix them in my cleanup
series. The series conflict with this one here.

https://lore.kernel.org/linux-arm-msm/20221116155416.164239-1-krzysztof.kozlowski@linaro.org/T/#t

> 
>>
>>>     - if:
>>>         properties:
>>>           compatible:
>>> @@ -573,6 +608,29 @@ allOf:
>>>         properties:
>>>           qcom,qmp: false
>>>   
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - qcom,sm8550-adsp-pas
>>> +              - qcom,sm8550-cdsp-pas
>>> +              - qcom,sm8550-mpss-pas
>>> +    then:
>>> +      properties:
>>> +        memory-region:
>>> +          minItems: 2
>>> +          description:
>>> +            First entry is a phandle for a reserved memory area that holds
>>> +            the main Firmware for authentication, and second entry a phandle for a
>>> +            reserved memory area that holds the Devicetree Firmware for authentication.
>>
>> Instead of minItems and description:
>>    items:
>>      - description: Main Firmware for auth....
>>      - description: Devicetree Firmware....
> 
> Ack
> 
>>
>>> +    else:
>>> +      properties:
>>> +        qcom,dtb-firmware-name: false
>>> +
>>> +        memory-region:
>>> +          maxItems: 1
>>> +
> 
> I'll rebase on top of 20221116155416.164239-1-krzysztof.kozlowski@linaro.org.
> 
> Seems I should perhaps add a separate qcom,sm8550-pas.yaml right, or adding the qcom,sm6350-pas.yam would be ok ?

The clocks and interrupts match qcom,sm8350-pas.yaml, but power domains
and memory region does not, so you need separate qcom,sm8550-pas.yaml file.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
index db9e0f0c2bea..678cb73f10de 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
@@ -55,6 +55,9 @@  properties:
       - qcom,sm8450-cdsp-pas
       - qcom,sm8450-mpss-pas
       - qcom,sm8450-slpi-pas
+      - qcom,sm8550-adsp-pas
+      - qcom,sm8550-cdsp-pas
+      - qcom,sm8550-mpss-pas
 
   reg:
     maxItems: 1
@@ -116,8 +119,13 @@  properties:
     $ref: /schemas/types.yaml#/definitions/string
     description: Firmware name for the Hexagon core
 
+  qcom,dtb-firmware-name:
+    $ref: /schemas/types.yaml#/definitions/string
+    description: Devicetree Firmware name for the Hexagon core
+
   memory-region:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
     description: Reference to the reserved-memory for the Hexagon core
 
   qcom,qmp:
@@ -212,6 +220,9 @@  allOf:
               - qcom,sm8450-cdsp-pas
               - qcom,sm8450-slpi-pas
               - qcom,sm8450-mpss-pas
+              - qcom,sm8550-adsp-pas
+              - qcom,sm8550-cdsp-pas
+              - qcom,sm8550-mpss-pas
     then:
       properties:
         clocks:
@@ -327,6 +338,8 @@  allOf:
               - qcom,sm8450-adsp-pas
               - qcom,sm8450-cdsp-pas
               - qcom,sm8450-slpi-pas
+              - qcom,sm8550-adsp-pas
+              - qcom,sm8550-cdsp-pas
     then:
       properties:
         interrupts:
@@ -347,6 +360,7 @@  allOf:
               - qcom,sm8150-mpss-pas
               - qcom,sm8350-mpss-pas
               - qcom,sm8450-mpss-pas
+              - qcom,sm8550-mpss-pas
     then:
       properties:
         interrupts:
@@ -448,6 +462,7 @@  allOf:
               - qcom,sm8150-mpss-pas
               - qcom,sm8350-mpss-pas
               - qcom,sm8450-mpss-pas
+              - qcom,sm8550-mpss-pas
     then:
       properties:
         power-domains:
@@ -475,6 +490,7 @@  allOf:
               - qcom,sm8350-slpi-pas
               - qcom,sm8450-adsp-pas
               - qcom,sm8450-slpi-pas
+              - qcom,sm8550-adsp-pas
     then:
       properties:
         power-domains:
@@ -504,6 +520,25 @@  allOf:
             - const: cx
             - const: mxc
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,sm8550-cdsp-pas
+    then:
+      properties:
+        power-domains:
+          items:
+            - description: CX power domain
+            - description: MXC power domain
+            - description: NSP power domain
+        power-domain-names:
+          items:
+            - const: cx
+            - const: mxc
+            - const: nsp
+
   - if:
       properties:
         compatible:
@@ -573,6 +608,29 @@  allOf:
       properties:
         qcom,qmp: false
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,sm8550-adsp-pas
+              - qcom,sm8550-cdsp-pas
+              - qcom,sm8550-mpss-pas
+    then:
+      properties:
+        memory-region:
+          minItems: 2
+          description:
+            First entry is a phandle for a reserved memory area that holds
+            the main Firmware for authentication, and second entry a phandle for a
+            reserved memory area that holds the Devicetree Firmware for authentication.
+    else:
+      properties:
+        qcom,dtb-firmware-name: false
+
+        memory-region:
+          maxItems: 1
+
 examples:
   - |
     #include <dt-bindings/clock/qcom,rpmcc.h>