| Message ID | 87zg9vk0ex.wl-kuninori.morimoto.gx@renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: dt-bindings: renesas: adjust to R-Car Gen4 | expand |
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
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
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
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 --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