[PATCH 1/4] dt-bindings: remoteproc: fsl,imx-rproc: add new compatible

Laurentiu Mihalcea posted 4 patches 1 month ago
There is a newer version of this series
[PATCH 1/4] dt-bindings: remoteproc: fsl,imx-rproc: add new compatible
Posted by Laurentiu Mihalcea 1 month ago
From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>

Add new compatible for imx95's CM7 with SOF.

Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
---
 .../bindings/remoteproc/fsl,imx-rproc.yaml    | 58 +++++++++++++++++--
 1 file changed, 53 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
index 57d75acb0b5e..ab0d8e017965 100644
--- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
@@ -28,6 +28,15 @@ properties:
       - fsl,imx8qxp-cm4
       - fsl,imx8ulp-cm33
       - fsl,imx93-cm33
+      - fsl,imx95-cm7-sof
+
+  reg:
+    maxItems: 2
+
+  reg-names:
+    items:
+      - const: dram
+      - const: mailbox
 
   clocks:
     maxItems: 1
@@ -38,10 +47,8 @@ properties:
       Phandle to syscon block which provide access to System Reset Controller
 
   mbox-names:
-    items:
-      - const: tx
-      - const: rx
-      - const: rxdb
+    minItems: 1
+    maxItems: 4
 
   mboxes:
     description:
@@ -49,7 +56,7 @@ properties:
       List of <&phandle type channel> - 1 channel for TX, 1 channel for RX, 1 channel for RXDB.
       (see mailbox/fsl,mu.yaml)
     minItems: 1
-    maxItems: 3
+    maxItems: 4
 
   memory-region:
     description:
@@ -84,6 +91,10 @@ properties:
       This property is to specify the resource id of the remote processor in SoC
       which supports SCFW
 
+  port:
+    $ref: /schemas/sound/audio-graph-port.yaml#
+    unevaluatedProperties: false
+
 required:
   - compatible
 
@@ -114,6 +125,43 @@ allOf:
       properties:
         power-domains: false
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: fsl,imx95-cm7-sof
+    then:
+      properties:
+        mboxes:
+          minItems: 4
+        mbox-names:
+          items:
+            - const: txdb0
+            - const: txdb1
+            - const: rxdb0
+            - const: rxdb1
+        memory-region:
+          maxItems: 1
+      required:
+        - reg
+        - reg-names
+        - mboxes
+        - mbox-names
+        - memory-region
+        - port
+    else:
+      properties:
+        reg: false
+        reg-names: false
+        mboxes:
+          maxItems: 3
+        mbox-names:
+          items:
+            - const: tx
+            - const: rx
+            - const: rxdb
+        port: false
+
 additionalProperties: false
 
 examples:
-- 
2.34.1
Re: [PATCH 1/4] dt-bindings: remoteproc: fsl,imx-rproc: add new compatible
Posted by Mathieu Poirier 1 month ago
Good day,

On Wed, Oct 23, 2024 at 12:21:11PM -0400, Laurentiu Mihalcea wrote:
> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> 
> Add new compatible for imx95's CM7 with SOF.
> 
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> ---
>  .../bindings/remoteproc/fsl,imx-rproc.yaml    | 58 +++++++++++++++++--
>  1 file changed, 53 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> index 57d75acb0b5e..ab0d8e017965 100644
> --- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> @@ -28,6 +28,15 @@ properties:
>        - fsl,imx8qxp-cm4
>        - fsl,imx8ulp-cm33
>        - fsl,imx93-cm33
> +      - fsl,imx95-cm7-sof

Why is this added in the remoteproc bindings when the driver is
sound/soc/sof/imx/imx95.c?

> +
> +  reg:
> +    maxItems: 2
> +
> +  reg-names:
> +    items:
> +      - const: dram
> +      - const: mailbox
>  
>    clocks:
>      maxItems: 1
> @@ -38,10 +47,8 @@ properties:
>        Phandle to syscon block which provide access to System Reset Controller
>  
>    mbox-names:
> -    items:
> -      - const: tx
> -      - const: rx
> -      - const: rxdb
> +    minItems: 1
> +    maxItems: 4
>  
>    mboxes:
>      description:
> @@ -49,7 +56,7 @@ properties:
>        List of <&phandle type channel> - 1 channel for TX, 1 channel for RX, 1 channel for RXDB.
>        (see mailbox/fsl,mu.yaml)
>      minItems: 1
> -    maxItems: 3
> +    maxItems: 4
>  
>    memory-region:
>      description:
> @@ -84,6 +91,10 @@ properties:
>        This property is to specify the resource id of the remote processor in SoC
>        which supports SCFW
>  
> +  port:
> +    $ref: /schemas/sound/audio-graph-port.yaml#
> +    unevaluatedProperties: false
> +
>  required:
>    - compatible
>  
> @@ -114,6 +125,43 @@ allOf:
>        properties:
>          power-domains: false
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: fsl,imx95-cm7-sof
> +    then:
> +      properties:
> +        mboxes:
> +          minItems: 4
> +        mbox-names:
> +          items:
> +            - const: txdb0
> +            - const: txdb1
> +            - const: rxdb0
> +            - const: rxdb1
> +        memory-region:
> +          maxItems: 1
> +      required:
> +        - reg
> +        - reg-names
> +        - mboxes
> +        - mbox-names
> +        - memory-region
> +        - port
> +    else:
> +      properties:
> +        reg: false
> +        reg-names: false
> +        mboxes:
> +          maxItems: 3
> +        mbox-names:
> +          items:
> +            - const: tx
> +            - const: rx
> +            - const: rxdb
> +        port: false
> +
>  additionalProperties: false
>  
>  examples:
> -- 
> 2.34.1
>
Re: [PATCH 1/4] dt-bindings: remoteproc: fsl,imx-rproc: add new compatible
Posted by Krzysztof Kozlowski 1 month ago
On Wed, Oct 23, 2024 at 12:21:11PM -0400, Laurentiu Mihalcea wrote:
> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> 
> Add new compatible for imx95's CM7 with SOF.
> 
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> ---
>  .../bindings/remoteproc/fsl,imx-rproc.yaml    | 58 +++++++++++++++++--
>  1 file changed, 53 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> index 57d75acb0b5e..ab0d8e017965 100644
> --- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> @@ -28,6 +28,15 @@ properties:
>        - fsl,imx8qxp-cm4
>        - fsl,imx8ulp-cm33
>        - fsl,imx93-cm33
> +      - fsl,imx95-cm7-sof
> +
> +  reg:
> +    maxItems: 2
> +
> +  reg-names:
> +    items:
> +      - const: dram
> +      - const: mailbox

That's quite different programming model. Are you sure these are devices
from similar class/type?

Your big if:then: block suggests this could be separate binding.

Best regards,
Krzysztof
Re: [PATCH 1/4] dt-bindings: remoteproc: fsl,imx-rproc: add new compatible
Posted by Laurentiu Mihalcea 1 month ago

On 10/24/2024 10:45 AM, Krzysztof Kozlowski wrote:
> On Wed, Oct 23, 2024 at 12:21:11PM -0400, Laurentiu Mihalcea wrote:
>> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>>
>> Add new compatible for imx95's CM7 with SOF.
>>
>> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>> ---
>>  .../bindings/remoteproc/fsl,imx-rproc.yaml    | 58 +++++++++++++++++--
>>  1 file changed, 53 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
>> index 57d75acb0b5e..ab0d8e017965 100644
>> --- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
>> +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
>> @@ -28,6 +28,15 @@ properties:
>>        - fsl,imx8qxp-cm4
>>        - fsl,imx8ulp-cm33
>>        - fsl,imx93-cm33
>> +      - fsl,imx95-cm7-sof
>> +
>> +  reg:
>> +    maxItems: 2
>> +
>> +  reg-names:
>> +    items:
>> +      - const: dram
>> +      - const: mailbox
> That's quite different programming model. Are you sure these are devices
> from similar class/type?
Yep, these are all Cortex-M cores. It's just that their usage differs quite a lot.
>
> Your big if:then: block suggests this could be separate binding.
Ideally I would have wanted to place the compatible inside dsp/fsl,dsp.yaml as the
programming model would have been more similar.

Unfortunately, these are different physical devices (HiFi DSP core vs CM core) even
though they're all used for DSP purposes so I'm not sure this is entirely appropriate.

Alternatively, if you think grouping these devices (i.e: those represented by the -dsp compatibles
from fsl,dsp and the one represented by the compatible introduced here) under the same binding
is alright we can just branch off from fsl,dsp and fsl,imx-rproc and create a new binding for
these devices. I'm expecting this to be relatively clean as they have the same programming
model.

Let me know your thoughts on this.
>
> Best regards,
> Krzysztof
>
Re: [PATCH 1/4] dt-bindings: remoteproc: fsl,imx-rproc: add new compatible
Posted by Rob Herring 1 month ago
On Thu, Oct 24, 2024 at 01:47:53PM +0300, Laurentiu Mihalcea wrote:
> 
> 
> On 10/24/2024 10:45 AM, Krzysztof Kozlowski wrote:
> > On Wed, Oct 23, 2024 at 12:21:11PM -0400, Laurentiu Mihalcea wrote:
> >> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> >>
> >> Add new compatible for imx95's CM7 with SOF.
> >>
> >> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> >> ---
> >>  .../bindings/remoteproc/fsl,imx-rproc.yaml    | 58 +++++++++++++++++--
> >>  1 file changed, 53 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> >> index 57d75acb0b5e..ab0d8e017965 100644
> >> --- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> >> +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> >> @@ -28,6 +28,15 @@ properties:
> >>        - fsl,imx8qxp-cm4
> >>        - fsl,imx8ulp-cm33
> >>        - fsl,imx93-cm33
> >> +      - fsl,imx95-cm7-sof
> >> +
> >> +  reg:
> >> +    maxItems: 2
> >> +
> >> +  reg-names:
> >> +    items:
> >> +      - const: dram
> >> +      - const: mailbox
> > That's quite different programming model. Are you sure these are devices
> > from similar class/type?
> Yep, these are all Cortex-M cores. It's just that their usage differs quite a lot.
> >
> > Your big if:then: block suggests this could be separate binding.
> Ideally I would have wanted to place the compatible inside dsp/fsl,dsp.yaml as the
> programming model would have been more similar.
> 
> Unfortunately, these are different physical devices (HiFi DSP core vs CM core) even
> though they're all used for DSP purposes so I'm not sure this is entirely appropriate.

That doesn't matter too much. trivial-devices.yaml is a bunch of 
completely unrelated devices which happen to have the same binding. We 
could probably take that farther with things like trivial clock 
providers for example. 

Having 'reg' vs not is pretty clearly something that should be different 
binding.

> 
> Alternatively, if you think grouping these devices (i.e: those represented by the -dsp compatibles
> from fsl,dsp and the one represented by the compatible introduced here) under the same binding
> is alright we can just branch off from fsl,dsp and fsl,imx-rproc and create a new binding for
> these devices. I'm expecting this to be relatively clean as they have the same programming
> model.

If it's just add a compatible and nothing else somewhere, I'd do that. 
If it's more than that, then I'd make a new binding doc.

Rob
Re: [PATCH 1/4] dt-bindings: remoteproc: fsl,imx-rproc: add new compatible
Posted by Frank Li 1 month ago
On Wed, Oct 23, 2024 at 12:21:11PM -0400, Laurentiu Mihalcea wrote:
> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>
> Add new compatible for imx95's CM7 with SOF.

It is not only add compatible string, but also change reg, reg-names ...

Please add descripts in commit message about these.

>
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> ---
>  .../bindings/remoteproc/fsl,imx-rproc.yaml    | 58 +++++++++++++++++--
>  1 file changed, 53 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> index 57d75acb0b5e..ab0d8e017965 100644
> --- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> @@ -28,6 +28,15 @@ properties:
>        - fsl,imx8qxp-cm4
>        - fsl,imx8ulp-cm33
>        - fsl,imx93-cm33
> +      - fsl,imx95-cm7-sof
> +
> +  reg:
> +    maxItems: 2
> +
> +  reg-names:
> +    items:
> +      - const: dram
> +      - const: mailbox
>
>    clocks:
>      maxItems: 1
> @@ -38,10 +47,8 @@ properties:
>        Phandle to syscon block which provide access to System Reset Controller
>
>    mbox-names:
> -    items:
> -      - const: tx
> -      - const: rx
> -      - const: rxdb
> +    minItems: 1
> +    maxItems: 4
>
>    mboxes:
>      description:
> @@ -49,7 +56,7 @@ properties:
>        List of <&phandle type channel> - 1 channel for TX, 1 channel for RX, 1 channel for RXDB.
>        (see mailbox/fsl,mu.yaml)
>      minItems: 1
> -    maxItems: 3
> +    maxItems: 4
>
>    memory-region:
>      description:
> @@ -84,6 +91,10 @@ properties:
>        This property is to specify the resource id of the remote processor in SoC
>        which supports SCFW
>
> +  port:
> +    $ref: /schemas/sound/audio-graph-port.yaml#
> +    unevaluatedProperties: false
> +
>  required:
>    - compatible
>
> @@ -114,6 +125,43 @@ allOf:
>        properties:
>          power-domains: false
>
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: fsl,imx95-cm7-sof
> +    then:
> +      properties:
> +        mboxes:
> +          minItems: 4
> +        mbox-names:
> +          items:
> +            - const: txdb0
> +            - const: txdb1
> +            - const: rxdb0
> +            - const: rxdb1
> +        memory-region:
> +          maxItems: 1
> +      required:
> +        - reg
> +        - reg-names
> +        - mboxes
> +        - mbox-names
> +        - memory-region
> +        - port
> +    else:
> +      properties:
> +        reg: false
> +        reg-names: false
> +        mboxes:
> +          maxItems: 3
> +        mbox-names:
> +          items:
> +            - const: tx
> +            - const: rx
> +            - const: rxdb
> +        port: false
> +
>  additionalProperties: false
>
>  examples:
> --
> 2.34.1
>