| Message ID | 87zg9vk0ex.wl-kuninori.morimoto.gx@renesas.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | ASoC: dt-bindings: renesas: adjust to R-Car Gen4 | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
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 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