[PATCH RFC 0/2] ASoC: dt-bindings: renesas,rsnd.yaml: adjust to R-Car Gen4

[PATCH RFC 0/2] ASoC: dt-bindings: renesas,rsnd.yaml: adjust to R-Car Gen4

[Kuninori Morimoto]

Hi

This is [RFC] patches.

This patch-set adjust to R-Car Gen4 on renesas,rsnd.yaml.
It works and no error reported.
But by this patch, non-Gen4 leaks from "ssi-[0-9]" checking.
I'm not sure why it happens.

	-----------------------
	rcar_sound,ssi:
	  ...
	  patternProperties:
	    "^ssi-[0-9]$":
	      ...
	      required:
(*)	        - interrupts
 ^	      allOf:
 |	        - if:
 |	            not:
 |	              properties:
 |	                compatible:
(X)	                  contains:
 |(4)	                    const: renesas,rcar_sound-gen4
 |	          then:
 |	            required:
 |(A)	              - dmas
 v(B)	              - dma-names
	-----------------------
	sound@ec500000 {
(3)		compatible = "renesas,rcar_sound-gen3";
		...
		rcar_sound,ssi {
			ssi0: ssi-0 {
(*)				interrupts = <GIC_SPI 370 IRQ_TYPE_LEVEL_HIGH>;
(A)				dmas = <&audma0 0x01>, <&audma1 0x02>;
(B)				dma-names = "rx", "tx";
			};
			...
		};
		...
	};
	sound@ec500000 {
(4)		compatible = "renesas,rcar_sound-gen4";
		...
		rcar_sound,ssi {
			ssi0: ssi-0 {
(*)				interrupts = <GIC_SPI 370 IRQ_TYPE_LEVEL_HIGH>;
			};
			...
		};
		...
	};
	-----------------------

I want to do here is that "interrupts" (*) is always requested,
and dmas (A) / dma-names (B) are required on non-Gen4 case only.

But, it say no error if I removed (A)(B) lines from "Gen3".
This is not my expect behavior, but I'm not sure why it happens.
It seems (X) part is not working.

I'm happy if someone checks it and pointing what was wrong.

Kuninori Morimoto (2):
  ASoC: dt-bindings: renesas,rsnd.yaml: tidyup reg/reg-name
  ASoC: dt-bindings: renesas,rsnd.yaml: adjust to R-Car Gen4

 .../bindings/sound/renesas,rsnd.yaml          | 89 ++++++++++++++-----
 1 file changed, 68 insertions(+), 21 deletions(-)

-- 
2.25.1

[PATCH RFC 1/2] ASoC: dt-bindings: renesas,rsnd.yaml: tidyup reg/reg-name

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current reg/reg-name is using "maxItems", but the convention is to
use "minItems".
And the core DT schemas already have a constraint that
requires reg-names to have the same number of elements as reg.

This patch switch to use "minItems" on reg, and remove it from
reg-names.

Link: https://lore.kernel.org/r/46974ae7-5f7f-8fc1-4ea8-fe77b58f5bfb@linaro.org
Reported-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 Documentation/devicetree/bindings/sound/renesas,rsnd.yaml | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
index d106de00c6b2..223f4859780f 100644
--- a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
+++ b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
@@ -298,9 +298,8 @@ allOf:
     then:
       properties:
         reg:
-          maxItems: 3
+          minItems: 3
         reg-names:
-          maxItems: 3
           items:
             enum:
               - scu
@@ -309,9 +308,8 @@ allOf:
     else:
       properties:
         reg:
-          maxItems: 5
+          minItems: 5
         reg-names:
-          maxItems: 5
           items:
             enum:
               - scu
-- 
2.25.1

[PATCH RFC 2/2] ASoC: dt-bindings: renesas,rsnd.yaml: adjust to R-Car Gen4

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

R-Car Gen4 is not compatible with Gen3, this patch adjusts
to R-Car Gen4.

Link: https://lore.kernel.org/r/CAMuHMdW_QHmODAKvn_GwHHUWw-=z4Tdq0NkhdK2u2piG_YgB-Q@mail.gmail.com
Link: https://lore.kernel.org/r/87zg9vk0ex.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 .../bindings/sound/renesas,rsnd.yaml          | 83 +++++++++++++++----
 1 file changed, 66 insertions(+), 17 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
index 223f4859780f..77b82bddd04a 100644
--- a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
+++ b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
@@ -99,20 +99,6 @@ properties:
     minItems: 1
     maxItems: 31
 
-  clock-names:
-    description: List of necessary clock names.
-    minItems: 1
-    maxItems: 31
-    items:
-      oneOf:
-        - const: ssi-all
-        - pattern: '^ssi\.[0-9]$'
-        - pattern: '^src\.[0-9]$'
-        - pattern: '^mix\.[0-1]$'
-        - pattern: '^ctu\.[0-1]$'
-        - pattern: '^dvc\.[0-1]$'
-        - pattern: '^clk_(a|b|c|i)$'
-
   ports:
     $ref: audio-graph-port.yaml#/definitions/port-base
     unevaluatedProperties: false
@@ -256,8 +242,17 @@ properties:
             $ref: /schemas/types.yaml#/definitions/flag
         required:
           - interrupts
-          - dmas
-          - dma-names
+        allOf:
+          - if:
+              not:
+                properties:
+                  compatible:
+                    contains:
+                      const: renesas,rcar_sound-gen4
+            then:
+              required:
+                - dmas
+                - dma-names
     additionalProperties: false
 
   # For DAI base
@@ -305,7 +300,14 @@ allOf:
               - scu
               - ssi
               - adg
-    else:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - renesas,rcar_sound-gen2
+              - renesas,rcar_sound-gen3
+    then:
       properties:
         reg:
           minItems: 5
@@ -317,6 +319,53 @@ allOf:
               - ssiu
               - ssi
               - audmapp
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,rcar_sound-gen4
+    then:
+      properties:
+        reg:
+          minItems: 4
+        reg-names:
+          items:
+            enum:
+              - adg
+              - ssiu
+              - ssi
+              - sdmc
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,rcar_sound-gen4
+    then:
+      properties:
+        clock-names:
+          description: List of necessary clock names.
+          minItems: 3
+          items:
+            enum:
+              - ssi.0
+              - ssiu.0
+              - clkin
+    else:
+      properties:
+        clock-names:
+          description: List of necessary clock names.
+          minItems: 1
+          maxItems: 31
+          items:
+            oneOf:
+              - const: ssi-all
+              - pattern: '^ssi\.[0-9]$'
+              - pattern: '^src\.[0-9]$'
+              - pattern: '^mix\.[0-1]$'
+              - pattern: '^ctu\.[0-1]$'
+              - pattern: '^dvc\.[0-1]$'
+              - pattern: '^clk_(a|b|c|i)$'
 
 unevaluatedProperties: false
 
-- 
2.25.1

Re: [PATCH RFC 0/2] ASoC: dt-bindings: renesas,rsnd.yaml: adjust to R-Car Gen4

[Geert Uytterhoeven]

Hi Morimoto-san,

On Tue, Feb 7, 2023 at 2:12 AM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> This is [RFC] patches.
>
> This patch-set adjust to R-Car Gen4 on renesas,rsnd.yaml.
> It works and no error reported.
> But by this patch, non-Gen4 leaks from "ssi-[0-9]" checking.
> I'm not sure why it happens.

If the logic becomes too complex, you can also split the binding
description in 4 files, one per R-Car generation.
That would lead to lots of duplication, though.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH RFC 1/2] ASoC: dt-bindings: renesas,rsnd.yaml: tidyup reg/reg-name

[Rob Herring]

On Tue, Feb 07, 2023 at 01:12:42AM +0000, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Current reg/reg-name is using "maxItems", but the convention is to
> use "minItems".

Yes, but that's not really true in if/then schemas. If that's your 
reason, then the changes are wrong.

> And the core DT schemas already have a constraint that
> requires reg-names to have the same number of elements as reg.

Actually, no it doesn't.

> 
> This patch switch to use "minItems" on reg, and remove it from
> reg-names.
> 
> Link: https://lore.kernel.org/r/46974ae7-5f7f-8fc1-4ea8-fe77b58f5bfb@linaro.org
> Reported-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  Documentation/devicetree/bindings/sound/renesas,rsnd.yaml | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> index d106de00c6b2..223f4859780f 100644
> --- a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> +++ b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> @@ -298,9 +298,8 @@ allOf:
>      then:
>        properties:
>          reg:
> -          maxItems: 3
> +          minItems: 3

This was probably correct before if the base 'reg' entry says more than 
3 entries are allowed and in this case 3 is most you can have.

>          reg-names:
> -          maxItems: 3
>            items:
>              enum:
>                - scu
> @@ -309,9 +308,8 @@ allOf:
>      else:
>        properties:
>          reg:
> -          maxItems: 5
> +          minItems: 5

This might be correct...

>          reg-names:
> -          maxItems: 5
>            items:
>              enum:
>                - scu
> -- 
> 2.25.1
> 

Re: [PATCH RFC 0/2] ASoC: dt-bindings: renesas,rsnd.yaml: adjust to R-Car Gen4

[Kuninori Morimoto]

Hi Geert

> > This patch-set adjust to R-Car Gen4 on renesas,rsnd.yaml.
> > It works and no error reported.
> > But by this patch, non-Gen4 leaks from "ssi-[0-9]" checking.
> > I'm not sure why it happens.
> 
> If the logic becomes too complex,

The logic itself is very simple IMO. The problem is that
all case hits to if-then for some reasons under certain conditions.
Under "patternProperties" or "xxx,yyy" style are the point ?
I'm not sure.

> you can also split the binding
> description in 4 files, one per R-Car generation.
> That would lead to lots of duplication, though.

It is nightmare for me...

Actually, to be very strict, dmas/dma-names are not mandatory,
because it supports PIO transfer mode.

I'm still trying to solve this issue by using many kind of schemas ways,
but all cases doesn't work for me...

Let's remove dmas/dma-names from required for now.
It is not a wrong schema.

Thank you for your help !!

Best regards
---
Kuninori Morimoto