[PATCH RFC 0/2] ASoC: dt-bindings: renesas, rsnd.yaml: adjust to R-Car Gen4
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(-)
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
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
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-... 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
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
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
participants (3)
-
Geert Uytterhoeven -
Kuninori Morimoto -
Rob Herring