[PATCH v2 0/2] ASoC: dt-bindings: renesas,rsnd.yaml: add R-Car Gen4 support

[PATCH v2 0/2] ASoC: dt-bindings: renesas,rsnd.yaml: add R-Car Gen4 support

[Kuninori Morimoto]

Hi Mark, Rob, Krzysztof
Cc Geert

These are v2 of R-Car Gen4 support for Renesas sound driver.

I have posted these patches as [RFC], because the schemas
doesn't work correctly for me under certain conditions.
"required: dmas/dma-names" always hits to "if-then" and
never hits to "else" for some reasons. I'm still not sure why...

But to be very strict, actually, this schemas is not mandatory,
because SSI has PIO transfer mode which doesn't need to have
DMA settings.

This patch-set drops "dmas/dma-names" from SSI for now,
and ignore the issue, but it is *not* a wrong schemas.

v1 -> v2
	- tidyup Subject
	- separate patches (drop "dmas/dma-names", add Gen4 support)
	- minItems -> maxItems
	- more git-logs.

Link: https://lore.kernel.org/all/87zg9vk0ex.wl-kuninori.morimoto.gx@renesas.com/#r
Link: https://lore.kernel.org/all/87r0v2uvm7.wl-kuninori.morimoto.gx@renesas.com/#r
Link: https://lore.kernel.org/all/87r0v1t02h.wl-kuninori.morimoto.gx@renesas.com/#r

Kuninori Morimoto (2):
  ASoC: dt-bindings: renesas,rsnd.yaml: drop "dmas/dma-names" from "rcar_sound,ssi"
  ASoC: dt-bindings: renesas,rsnd.yaml: add R-Car Gen4 support

 .../bindings/sound/renesas,rsnd.yaml          | 82 +++++++++++++++----
 1 file changed, 65 insertions(+), 17 deletions(-)

-- 
2.25.1

[PATCH v2 1/2] ASoC: dt-bindings: renesas,rsnd.yaml: drop "dmas/dma-names" from "rcar_sound,ssi"

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

SSI is supporting both "PIO mode" and "DMA mode", thus "dmas/dma-names"
are not mandatory property. Drop these from rcar_sound,ssi's required:.
See more details on Link

Link: https://lore.kernel.org/all/87zg9vk0ex.wl-kuninori.morimoto.gx@renesas.com/#r
Link: https://lore.kernel.org/all/87r0v2uvm7.wl-kuninori.morimoto.gx@renesas.com/#r
Link: https://lore.kernel.org/all/87r0v1t02h.wl-kuninori.morimoto.gx@renesas.com/#r
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 Documentation/devicetree/bindings/sound/renesas,rsnd.yaml | 2 --
 1 file changed, 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
index c3bea5b0ec40..12ccf29338d9 100644
--- a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
+++ b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
@@ -256,8 +256,6 @@ properties:
             $ref: /schemas/types.yaml#/definitions/flag
         required:
           - interrupts
-          - dmas
-          - dma-names
     additionalProperties: false
 
   # For DAI base
-- 
2.25.1

[PATCH v2 2/2] ASoC: dt-bindings: renesas,rsnd.yaml: add R-Car Gen4 support

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

There are no compatible for "reg/reg-names" and "clock-name"
between previous R-Car series and R-Car Gen4.

"reg/reg-names" needs 3 categorize (for Gen1, for Gen2/Gen3, for Gen4),
therefore, use 3 if-then to avoid nested if-then-else.

Move "clock-name" property to under allOf to use if-then-else.

Link: https://lore.kernel.org/all/87zg9vk0ex.wl-kuninori.morimoto.gx@renesas.com/#r
Link: https://lore.kernel.org/all/87r0v2uvm7.wl-kuninori.morimoto.gx@renesas.com/#r
Link: https://lore.kernel.org/all/87r0v1t02h.wl-kuninori.morimoto.gx@renesas.com/#r
Reported-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 .../bindings/sound/renesas,rsnd.yaml          | 80 +++++++++++++++----
 1 file changed, 65 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
index 12ccf29338d9..5fd6435f4873 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
@@ -288,6 +274,11 @@ required:
 
 allOf:
   - $ref: dai-common.yaml#
+
+  #--------------------
+  # reg/reg-names
+  #--------------------
+  # for Gen1
   - if:
       properties:
         compatible:
@@ -303,7 +294,15 @@ allOf:
               - scu
               - ssi
               - adg
-    else:
+  # for Gen2/Gen3
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - renesas,rcar_sound-gen2
+              - renesas,rcar_sound-gen3
+    then:
       properties:
         reg:
           minItems: 5
@@ -315,6 +314,57 @@ allOf:
               - ssiu
               - ssi
               - audmapp
+  # for Gen4
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,rcar_sound-gen4
+    then:
+      properties:
+        reg:
+          maxItems: 4
+        reg-names:
+          items:
+            enum:
+              - adg
+              - ssiu
+              - ssi
+              - sdmc
+
+  #--------------------
+  # clock-names
+  #--------------------
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,rcar_sound-gen4
+    then:
+      properties:
+        clock-names:
+          description: List of necessary clock names.
+          maxItems: 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
 
-- 
2.25.1

Re: [PATCH v2 2/2] ASoC: dt-bindings: renesas,rsnd.yaml: add R-Car Gen4 support

[Krzysztof Kozlowski]

On 09/02/2023 02:22, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> There are no compatible for "reg/reg-names" and "clock-name"
> between previous R-Car series and R-Car Gen4.
> 
> "reg/reg-names" needs 3 categorize (for Gen1, for Gen2/Gen3, for Gen4),
> therefore, use 3 if-then to avoid nested if-then-else.
> 
> Move "clock-name" property to under allOf to use if-then-else.
> 
> Link: https://lore.kernel.org/all/87zg9vk0ex.wl-kuninori.morimoto.gx@renesas.com/#r
> Link: https://lore.kernel.org/all/87r0v2uvm7.wl-kuninori.morimoto.gx@renesas.com/#r
> Link: https://lore.kernel.org/all/87r0v1t02h.wl-kuninori.morimoto.gx@renesas.com/#r
> Reported-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  .../bindings/sound/renesas,rsnd.yaml          | 80 +++++++++++++++----
>  1 file changed, 65 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> index 12ccf29338d9..5fd6435f4873 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

No improvements here. Your argument that you need to remove it to
customize is not correct.

https://elixir.bootlin.com/linux/v5.19-rc6/source/Documentation/devicetree/bindings/clock/samsung,exynos7-clock.yaml#L57

Best regards,
Krzysztof

Re: [PATCH v2 1/2] ASoC: dt-bindings: renesas,rsnd.yaml: drop "dmas/dma-names" from "rcar_sound,ssi"

[Krzysztof Kozlowski]

On 09/02/2023 02:21, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> SSI is supporting both "PIO mode" and "DMA mode", thus "dmas/dma-names"
> are not mandatory property. Drop these from rcar_sound,ssi's required:.
> See more details on Link

Still no improvements.

> 
> Link: https://lore.kernel.org/all/87zg9vk0ex.wl-kuninori.morimoto.gx@renesas.com/#r
> Link: https://lore.kernel.org/all/87r0v2uvm7.wl-kuninori.morimoto.gx@renesas.com/#r
> Link: https://lore.kernel.org/all/87r0v1t02h.wl-kuninori.morimoto.gx@renesas.com/#r
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Best regards,
Krzysztof

Re: [PATCH v2 2/2] ASoC: dt-bindings: renesas,rsnd.yaml: add R-Car Gen4 support

[Kuninori Morimoto]

Hi Krzysztof

> > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > 
> > There are no compatible for "reg/reg-names" and "clock-name"
> > between previous R-Car series and R-Car Gen4.
> > 
> > "reg/reg-names" needs 3 categorize (for Gen1, for Gen2/Gen3, for Gen4),
> > therefore, use 3 if-then to avoid nested if-then-else.
> > 
> > Move "clock-name" property to under allOf to use if-then-else.
(snip)
> > -  clock-names:
> > -    description: List of necessary clock names.
> > -    minItems: 1
> > -    maxItems: 31
> 
> No improvements here. Your argument that you need to remove it to
> customize is not correct.

???

	Move "clock-name" property to under allOf to use if-then-else.
	^^^^

"move", not "remove".

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH v2 2/2] ASoC: dt-bindings: renesas,rsnd.yaml: add R-Car Gen4 support

[Geert Uytterhoeven]

Hi Morimoto-san,

On Fri, Feb 10, 2023 at 1:22 AM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > >
> > > There are no compatible for "reg/reg-names" and "clock-name"
> > > between previous R-Car series and R-Car Gen4.
> > >
> > > "reg/reg-names" needs 3 categorize (for Gen1, for Gen2/Gen3, for Gen4),
> > > therefore, use 3 if-then to avoid nested if-then-else.
> > >
> > > Move "clock-name" property to under allOf to use if-then-else.
> (snip)
> > > -  clock-names:
> > > -    description: List of necessary clock names.
> > > -    minItems: 1
> > > -    maxItems: 31
> >
> > No improvements here. Your argument that you need to remove it to
> > customize is not correct.
>
> ???
>
>         Move "clock-name" property to under allOf to use if-then-else.
>         ^^^^
>
> "move", not "remove".

The point is that you did remove the common

      clock-names:
        description: List of necessary clock names.

from the top level, and added two copies of it under if/then/else.

Please keep common stuff as high up in the hierarchy as possible,
to avoid the need for duplication.
In this case, that means:
  - Keep the description at the top level,
  - Put only {min,max}Items and items under if/then/else.

I hope my explanation helps.

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 v2 2/2] ASoC: dt-bindings: renesas,rsnd.yaml: add R-Car Gen4 support

[Kuninori Morimoto]

Hi Geert

> > "move", not "remove".
> 
> The point is that you did remove the common
> 
>       clock-names:
>         description: List of necessary clock names.
> 
> from the top level, and added two copies of it under if/then/else.
> 
> Please keep common stuff as high up in the hierarchy as possible,
> to avoid the need for duplication.
> In this case, that means:
>   - Keep the description at the top level,
>   - Put only {min,max}Items and items under if/then/else.
> 
> I hope my explanation helps.

OK, I understand the point.
I will post v3 patch.

Thank you for your help !!

Best regards
---
Kuninori Morimoto