diff mbox series

ASoC: dt-bindings: renesas: adjust to R-Car Gen4

Message ID 87zg9vk0ex.wl-kuninori.morimoto.gx@renesas.com
State New
Headers show
Series ASoC: dt-bindings: renesas: adjust to R-Car Gen4 | expand

Commit Message

Kuninori Morimoto Feb. 3, 2023, 1:22 a.m. UTC
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(-)

Comments

Kuninori Morimoto Feb. 6, 2023, 2:41 a.m. UTC | #1
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
Geert Uytterhoeven Feb. 6, 2023, 6:49 a.m. UTC | #2
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
Kuninori Morimoto Feb. 7, 2023, 12:56 a.m. UTC | #3
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
diff mbox series

Patch

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