[PATCH] ASoC: dt-bindings: renesas: adjust to R-Car Gen4

[PATCH] ASoC: dt-bindings: renesas: 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.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
The "required" with if - then - else on "rcar_sound,ssi" is
always match to "then" even though it is checking "renesas,rcar_sound-gen4" or not.
Why ?? Is it my fault ??

 .../bindings/sound/renesas,rsnd.yaml          | 62 ++++++++++++++-----
 1 file changed, 46 insertions(+), 16 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
index d106de00c6b2..9a88b1c34e72 100644
--- a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
+++ b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
@@ -106,7 +106,9 @@ properties:
     items:
       oneOf:
         - const: ssi-all
+        - const: clkin
         - pattern: '^ssi\.[0-9]$'
+        - pattern: '^ssiu\.[0-9]$'
         - pattern: '^src\.[0-9]$'
         - pattern: '^mix\.[0-1]$'
         - pattern: '^ctu\.[0-1]$'
@@ -254,10 +256,20 @@ properties:
           no-busif:
             description: BUSIF is not used when [mem -> SSI] via DMA case
             $ref: /schemas/types.yaml#/definitions/flag
-        required:
-          - interrupts
-          - dmas
-          - dma-names
+        allOf:
+          - if:
+              properties:
+                compatible:
+                  contains:
+                    const: renesas,rcar_sound-gen4
+            then:
+              required:
+                - interrupts
+            else:
+              required:
+                - interrupts
+                - dmas
+                - dma-names
     additionalProperties: false
 
   # For DAI base
@@ -307,18 +319,36 @@ allOf:
               - ssi
               - adg
     else:
-      properties:
-        reg:
-          maxItems: 5
-        reg-names:
-          maxItems: 5
-          items:
-            enum:
-              - scu
-              - adg
-              - ssiu
-              - ssi
-              - audmapp
+      if:
+        properties:
+          compatible:
+            contains:
+              const: renesas,rcar_sound-gen4
+      then:
+        properties:
+          reg:
+            maxItems: 4
+          reg-names:
+            maxItems: 4
+            items:
+              enum:
+                - adg
+                - ssiu
+                - ssi
+                - sdmc
+      else:
+        properties:
+          reg:
+            maxItems: 5
+          reg-names:
+            maxItems: 5
+            items:
+              enum:
+                - scu
+                - adg
+                - ssiu
+                - ssi
+                - audmapp
 
 unevaluatedProperties: false
 
-- 
2.25.1

Re: [PATCH] ASoC: dt-bindings: renesas: adjust to R-Car Gen4

[Krzysztof Kozlowski]

On 03/02/2023 02:22, Kuninori Morimoto wrote:
> 
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> R-Car Gen4 is not compatible with Gen3, this patch adjusts
> to R-Car Gen4.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
> The "required" with if - then - else on "rcar_sound,ssi" is
> always match to "then" even though it is checking "renesas,rcar_sound-gen4" or not.
> Why ?? Is it my fault ??
> 
>  .../bindings/sound/renesas,rsnd.yaml          | 62 ++++++++++++++-----
>  1 file changed, 46 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> index d106de00c6b2..9a88b1c34e72 100644
> --- a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> +++ b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> @@ -106,7 +106,9 @@ properties:
>      items:
>        oneOf:
>          - const: ssi-all
> +        - const: clkin
>          - pattern: '^ssi\.[0-9]$'
> +        - pattern: '^ssiu\.[0-9]$'
>          - pattern: '^src\.[0-9]$'
>          - pattern: '^mix\.[0-1]$'
>          - pattern: '^ctu\.[0-1]$'
> @@ -254,10 +256,20 @@ properties:
>            no-busif:
>              description: BUSIF is not used when [mem -> SSI] via DMA case
>              $ref: /schemas/types.yaml#/definitions/flag
> -        required:
> -          - interrupts
> -          - dmas
> -          - dma-names
> +        allOf:
> +          - if:
> +              properties:
> +                compatible:
> +                  contains:
> +                    const: renesas,rcar_sound-gen4
> +            then:
> +              required:
> +                - interrupts
> +            else:
> +              required:
> +                - interrupts

This does not make sense - you just require it always.



> +                - dmas
> +                - dma-names
>      additionalProperties: false
>  
>    # For DAI base
> @@ -307,18 +319,36 @@ allOf:
>                - ssi
>                - adg
>      else:
> -      properties:
> -        reg:
> -          maxItems: 5
> -        reg-names:
> -          maxItems: 5
> -          items:
> -            enum:
> -              - scu
> -              - adg
> -              - ssiu
> -              - ssi
> -              - audmapp
> +      if:

Please do not embed if within another if, unless strictly necessary. It
gets unmanageable.

> +        properties:
> +          compatible:
> +            contains:
> +              const: renesas,rcar_sound-gen4
> +      then:
> +        properties:
> +          reg:

minItems

> +            maxItems: 4
> +          reg-names:
> +            maxItems: 4

Drop

> +            items:
> +              enum:
> +                - adg
> +                - ssiu
> +                - ssi
> +                - sdmc
> +      else:
> +        properties:
> +          reg:

minItems

> +            maxItems: 5
> +          reg-names:
> +            maxItems: 5

Drop


Best regards,
Krzysztof

Re: [PATCH] ASoC: dt-bindings: renesas: adjust to R-Car Gen4

[Kuninori Morimoto]

Hi Krzysztof

Thank you for your review

> This does not make sense - you just require it always.
(snip)
> Please do not embed if within another if, unless strictly necessary. It
> gets unmanageable.
(snip)
> minItems
(snip)
> Drop

OK, thanks. Will fix in v2

> > The "required" with if - then - else on "rcar_sound,ssi" is
> > always match to "then" even though it is checking "renesas,rcar_sound-gen4" or not.
> > Why ?? Is it my fault ??

I'm not sure why but some "if - then - else" doesn't work correctly for me.
One concern is that it is under "patternProperties".
Non "patternProperties" case is works well.

This is just sample case.
In below case, only gen4 case requires "foo/bar" if my understanding was correct.
But I get error "foo/bar are required" on *all* compatible.

It is my fault ?

--- sample -----------
  rcar_sound,ssi:
    ...
    patternProperties:
      "^ssi-[0-9]$":
        ...
        allOf:
          - if:
              properties:
                compatible:
                  contains:
=>                  const: renesas,rcar_sound-gen4
            then:
              required:
=>              - foo
=>              - bar
-----------------------

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH] ASoC: dt-bindings: renesas: adjust to R-Car Gen4

[Geert Uytterhoeven]

Hi Morimoto-san,

On Mon, Feb 6, 2023 at 4:03 AM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> > > The "required" with if - then - else on "rcar_sound,ssi" is
> > > always match to "then" even though it is checking "renesas,rcar_sound-gen4" or not.
> > > Why ?? Is it my fault ??
>
> I'm not sure why but some "if - then - else" doesn't work correctly for me.
> One concern is that it is under "patternProperties".
> Non "patternProperties" case is works well.
>
> This is just sample case.
> In below case, only gen4 case requires "foo/bar" if my understanding was correct.
> But I get error "foo/bar are required" on *all* compatible.
>
> It is my fault ?
>
> --- sample -----------
>   rcar_sound,ssi:
>     ...
>     patternProperties:
>       "^ssi-[0-9]$":
>         ...
>         allOf:
>           - if:
>               properties:
>                 compatible:
>                   contains:
> =>                  const: renesas,rcar_sound-gen4
>             then:
>               required:
> =>              - foo
> =>              - bar

As it is under patternProperties, the "if: properties" applies to the
properties under the ssi node, where you do not have any compatible
value (and definitely not the "renesas,rcar_sound-gen4" value, which
belongs to the _parent_ of the ssi node).

So I think the only solution is to move the "if" up, and thus duplicate
the ssi node description:

    if:
        properties:
            compatible:
                contains:
                    const: renesas,rcar_sound-gen4
    then:
        patternProperties:
            "^ssi-[0-9]$":
                ...
    else:
        patternProperties:
            "^ssi-[0-9]$":
                ...

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] ASoC: dt-bindings: renesas: adjust to R-Car Gen4

[Kuninori Morimoto]

Hi Geert

Thank you for your help

> > --- sample -----------
> >   rcar_sound,ssi:
> >     ...
> >     patternProperties:
> >       "^ssi-[0-9]$":
> >         ...
> >         allOf:
> >           - if:
> >               properties:
> >                 compatible:
> >                   contains:
> > =>                  const: renesas,rcar_sound-gen4
> >             then:
> >               required:
> > =>              - foo
> > =>              - bar
> 
> As it is under patternProperties, the "if: properties" applies to the
> properties under the ssi node, where you do not have any compatible
> value (and definitely not the "renesas,rcar_sound-gen4" value, which
> belongs to the _parent_ of the ssi node).

Hmm...
I want to do on above sample case is "required foo/bar when only gen4",
but my concern is it *always* requests "foo/bar" even though it is *not* gen4.
May be it is opposite?

> So I think the only solution is to move the "if" up, and thus duplicate
> the ssi node description:
> 
>     if:
>         properties:
>             compatible:
>                 contains:
>                     const: renesas,rcar_sound-gen4
>     then:
>         patternProperties:
>             "^ssi-[0-9]$":
>                 ...
>     else:
>         patternProperties:
>             "^ssi-[0-9]$":
>                 ...

Hmm... I have tried this but it was same result...
I'm not sure why it doesn't match as I expected...

I will try to post my current patch as RFC.
I'm happy if someone try it, and confirm my issue.


Thank you for your help !!

Best regards
---
Kuninori Morimoto