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

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

[Kuninori Morimoto]

Hi Mark

These are v3 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.

v2 -> v2
	- keep clock-name description on top
	- tidyup git-log

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
Link: https://lore.kernel.org/all/87y1p7bpma.wl-kuninori.morimoto.gx@renesas.com/#r


-- 
2.25.1

[PATCH v3 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" detail properties to under allOf to use if-then-else
for Gen4 or others.

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
Link: https://lore.kernel.org/all/87y1p7bpma.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          | 76 ++++++++++++++++---
 1 file changed, 64 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
index 12ccf29338d9..55e5213b90a1 100644
--- a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
+++ b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
@@ -101,17 +101,7 @@ 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)$'
+    # details are defined below
 
   ports:
     $ref: audio-graph-port.yaml#/definitions/port-base
@@ -288,6 +278,11 @@ required:
 
 allOf:
   - $ref: dai-common.yaml#
+
+  #--------------------
+  # reg/reg-names
+  #--------------------
+  # for Gen1
   - if:
       properties:
         compatible:
@@ -303,7 +298,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 +318,55 @@ 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:
+          maxItems: 3
+          items:
+            enum:
+              - ssi.0
+              - ssiu.0
+              - clkin
+    else:
+      properties:
+        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

[PATCH v3 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:.
This is prepare for Gen4 support. 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
Link: https://lore.kernel.org/all/87y1p7bpma.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

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

[Krzysztof Kozlowski]

On 13/02/2023 03:13, 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:.
> This is prepare for Gen4 support. See more details on Link

What did you change here? I still need to read 4 different
discussions/links to have understanding why?
> 
> Link: https://lore.kernel.org/all/87zg9vk0ex.wl-kuninori.morimoto.gx@renesas.com/#r

What details are here? There is a patch with gen4 but I don't understand
how does it answer anything here.

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

This is previous version, so it also does not bring more details.

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

This as well... so you say "more details under X, where X says more
details under Y, where Y links somewhere else"?

> Link: https://lore.kernel.org/all/87y1p7bpma.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

Best regards,
Krzysztof

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

[Geert Uytterhoeven]

Hi Morimoto-san,

Thanks for your patch!

On Mon, Feb 13, 2023 at 3:13 AM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> 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:.

BTW, do SSIU and DVC support PIO mode?

> This is prepare for Gen4 support. See more details on Link

"This is preparation" or "This prepares"?

> 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
> Link: https://lore.kernel.org/all/87y1p7bpma.wl-kuninori.morimoto.gx@renesas.com/#r
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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 v3 1/2] ASoC: dt-bindings: renesas,rsnd.yaml: drop "dmas/dma-names" from "rcar_sound,ssi"

[Krzysztof Kozlowski]

On 13/02/2023 09:56, Geert Uytterhoeven wrote:
> Hi Morimoto-san,
> 
> Thanks for your patch!
> 
> On Mon, Feb 13, 2023 at 3:13 AM Kuninori Morimoto
> <kuninori.morimoto.gx@renesas.com> 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:.
> 
> BTW, do SSIU and DVC support PIO mode?
> 
>> This is prepare for Gen4 support. See more details on Link
> 
> "This is preparation" or "This prepares"?

Use imperative.
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 13/02/2023 03:13, 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" detail properties to under allOf to use if-then-else
> for Gen4 or others.
> 
> 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
> Link: https://lore.kernel.org/all/87y1p7bpma.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          | 76 ++++++++++++++++---
>  1 file changed, 64 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> index 12ccf29338d9..55e5213b90a1 100644
> --- a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> +++ b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> @@ -101,17 +101,7 @@ properties:
>  
>    clock-names:
>      description: List of necessary clock names.
> -    minItems: 1
> -    maxItems: 31

Not much of an improvement here. We asked to keep the constraints here.
I gave you the reference how it should look like. Why did you decide to
do it differently than what I linked?

Best regards,
Krzysztof

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

[Kuninori Morimoto]

Hi Geert

> > 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:.
> 
> BTW, do SSIU and DVC support PIO mode?

Only SSI has PIO mode, others need DMA

Thank you for your help !!

Best regards
---
Kuninori Morimoto

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 985 bytes --]

On Mon, Feb 13, 2023 at 09:58:15AM +0100, Krzysztof Kozlowski wrote:
> On 13/02/2023 03:13, Kuninori Morimoto wrote:

> >    clock-names:
> >      description: List of necessary clock names.
> > -    minItems: 1
> > -    maxItems: 31

> Not much of an improvement here. We asked to keep the constraints here.
> I gave you the reference how it should look like. Why did you decide to
> do it differently than what I linked?

Krzysztof, please take more time to explain what issues you're
seeing, what you want people to do and why.  I'm having a hard
time identifying what the issue is here - AFAICT when you talk
about the example you linked you're referring to:

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

which as far as I can tell looks like what Morimoto-san is trying
to accomplish here.  I can't tell what the issue you're raising
is, let alone if it's something important or just a stylistic
nit.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: (subset) [PATCH v3 0/2] ASoC: dt-bindings: renesas,rsnd.yaml: add R-Car Gen4 support

[Mark Brown]

On Mon, 13 Feb 2023 02:12:41 +0000, Kuninori Morimoto wrote:
> These are v3 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...
> 
> [...]

Applied to

   broonie/sound.git for-next

Thanks!

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

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

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

[Krzysztof Kozlowski]

On 14/02/2023 18:52, Mark Brown wrote:
> On Mon, Feb 13, 2023 at 09:58:15AM +0100, Krzysztof Kozlowski wrote:
>> On 13/02/2023 03:13, Kuninori Morimoto wrote:
> 
>>>    clock-names:
>>>      description: List of necessary clock names.
>>> -    minItems: 1
>>> -    maxItems: 31
> 
>> Not much of an improvement here. We asked to keep the constraints here.
>> I gave you the reference how it should look like. Why did you decide to
>> do it differently than what I linked?
> 
> Krzysztof, please take more time to explain what issues you're
> seeing, what you want people to do and why.  I'm having a hard
> time identifying what the issue is here - AFAICT when you talk
> about the example you linked you're referring to:
> 
>   https://elixir.bootlin.com/linux/v5.19-rc6/source/Documentation/devicetree/bindings/clock/samsung,exynos7-clock.yaml#L57
> 
> which as far as I can tell looks like what Morimoto-san is trying
> to accomplish here.  I can't tell what the issue you're raising
> is, let alone if it's something important or just a stylistic
> nit.

If you leave the top-level definition without any constraints and you
forget to include all your compatibles in allOf:if:then, then you end up
without constraints. Consider:

-----
properties:
  compatible:
    enum:
      - foo
      - bar

clock-names:
  description: anything

if:
  prop:
    compat:
      enum:
        - foo
then:
  prop:
    clock-names:
      - ahb
      - sclk
-----

What clocks are valid for bar?

For simple cases this might be obvious, for more complex this is easy to
miss. So the recommended syntax is with constraints at the top.

BTW, if there is reason not to use it - sure, bring the reason, we talk
and maybe skip it, I don't mind. But there was no reason - just part was
skipped/missing.


Best regards,
Krzysztof

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

[Kuninori Morimoto]

Hi Krzysztof

> If you leave the top-level definition without any constraints and you
> forget to include all your compatibles in allOf:if:then, then you end up
> without constraints. Consider:
(snip)
> -----
> properties:
>   compatible:
>     enum:
>       - foo
>       - bar
> 
> clock-names:
>   description: anything
> 
> if:
>   prop:
>     compat:
>       enum:
>         - foo
> then:
>   prop:
>     clock-names:
>       - ahb
>       - sclk
> -----
> 
> What clocks are valid for bar?
> 
> For simple cases this might be obvious, for more complex this is easy to
> miss. So the recommended syntax is with constraints at the top.

I can understand we want to avoid the future miss.
But I did it on v2 patch and you NACKed it.
Thus people confused. That is the reason why above strange style was created.
And it is already using "else", your concern never happen ?

Thank you for your help !!

Best regards
---
Kuninori Morimoto

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

[Krzysztof Kozlowski]

On 16/02/2023 02:09, Kuninori Morimoto wrote:
> 
> Hi Krzysztof
> 
>> If you leave the top-level definition without any constraints and you
>> forget to include all your compatibles in allOf:if:then, then you end up
>> without constraints. Consider:
> (snip)
>> -----
>> properties:
>>   compatible:
>>     enum:
>>       - foo
>>       - bar
>>
>> clock-names:
>>   description: anything
>>
>> if:
>>   prop:
>>     compat:
>>       enum:
>>         - foo
>> then:
>>   prop:
>>     clock-names:
>>       - ahb
>>       - sclk
>> -----
>>
>> What clocks are valid for bar?
>>
>> For simple cases this might be obvious, for more complex this is easy to
>> miss. So the recommended syntax is with constraints at the top.
> 
> I can understand we want to avoid the future miss.
> But I did it on v2 patch and you NACKed it.

No, you did not do it in v2. The top-level property is a must, we talk
now about constraints.

> Thus people confused. That is the reason why above strange style was created.
> And it is already using "else", your concern never happen ?

Yes, with else my concern will never happen. However you have there
multiple ifs, thus finding the one related to clocks is not obvious now
and won't be anyhow easier later. What's more, clocks have constraints
in top-level, thus seeing clock-names without the constraints also
raises reviewers question. Someone might bring a new compatible with
another set of clocks (quite likely), thus else won't be valid anymore
and you will have to define constraints per *each* variant (each
if:then:). When this happens, please move the widest constraints to
top-level, just like I was asking since some time. Will you remember to
do this? I might not because I assume we follow same pattern everywhere.

With a promise of above:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

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

[Kuninori Morimoto]

Hi Krzysztof

> However you have there
> multiple ifs, thus finding the one related to clocks is not obvious now
> and won't be anyhow easier later. What's more, clocks have constraints
> in top-level, thus seeing clock-names without the constraints also
> raises reviewers question. Someone might bring a new compatible with
> another set of clocks (quite likely), thus else won't be valid anymore
> and you will have to define constraints per *each* variant (each
> if:then:).

Do you mean you want to tell was keeping minItems/maxItems on top ??

I think I could understand what you want to tell if your indicated
link was pointing to "clock-name" line, and/or if you indicated like
"please keep minItems/maxItems on top for constraints" or something.
But pointed link was to "allOf:" line with very unclear comment,
and no response to my question mail.
And your words of "constraints". I have been thoughting that you are
indicating was "if-then-else" need to catch all "compatible"
(but don't use "else if").

It is using "if-then-else", and "else" has minItems/maxItems,
I think it is there is no difference.
But if my above assumption was correct and Krzysztof agreed to it,
I will post v4 patch which keeps min/maxItems on top.
Otherwise I will do nothing to avoid endless pointless
ping-pong, becuase it already got Reviewed-by.

To avoid pointless ping-pong, I think v4 will be like this

----------
  clock-names:
    description: List of necessary clock names.
    minItems: 1
    maxItems: 31

  ...

  - if:
     properties:
       compatible:
         contains:
           const: renesas,rcar_sound-gen4
    then:
      properties:
        clock-names:
          maxItems: 3
         ...
    else
      properties:
        clock-names:
         ...
-------------

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: (subset) [PATCH v3 0/2] ASoC: dt-bindings: renesas,rsnd.yaml: add R-Car Gen4 support

[Mark Brown]

On Mon, 13 Feb 2023 02:12:41 +0000, Kuninori Morimoto wrote:
> These are v3 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...
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[2/2] ASoC: dt-bindings: renesas,rsnd.yaml: add R-Car Gen4 support
      commit: 7f8b5b24bbb463614dd6b797e6b2ee92b3e2a6a1

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

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

[Rafał Miłecki]

On 13.02.2023 03:13, 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" detail properties to under allOf to use if-then-else
> for Gen4 or others.

Hi, this patch seems to add errors for me. Are my tools outdated or is
it a real issue? See below.


> 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
> Link: https://lore.kernel.org/all/87y1p7bpma.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          | 76 ++++++++++++++++---
>   1 file changed, 64 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> index 12ccf29338d9..55e5213b90a1 100644
> --- a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> +++ b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> @@ -101,17 +101,7 @@ 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)$'
> +    # details are defined below
>   
>     ports:
>       $ref: audio-graph-port.yaml#/definitions/port-base
> @@ -288,6 +278,11 @@ required:
>   
>   allOf:
>     - $ref: dai-common.yaml#
> +
> +  #--------------------
> +  # reg/reg-names
> +  #--------------------
> +  # for Gen1

This seems to cause:

./Documentation/devicetree/bindings/sound/renesas,rsnd.yaml:282:4: [error] missing starting space in comment (comments)
./Documentation/devicetree/bindings/sound/renesas,rsnd.yaml:284:4: [error] missing starting space in comment (comments)
./Documentation/devicetree/bindings/sound/renesas,rsnd.yaml:339:4: [error] missing starting space in comment (comments)
./Documentation/devicetree/bindings/sound/renesas,rsnd.yaml:341:4: [error] missing starting space in comment (comments)


>     - if:
>         properties:
>           compatible:
> @@ -303,7 +298,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 +318,55 @@ 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:
> +          maxItems: 3
> +          items:
> +            enum:
> +              - ssi.0
> +              - ssiu.0
> +              - clkin
> +    else:
> +      properties:
> +        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
>   

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

[Kuninori Morimoto]

Hi Rafał

> Hi, this patch seems to add errors for me. Are my tools outdated or is
> it a real issue? See below.
(snip)
> > +  #--------------------
> > +  # reg/reg-names
> > +  #--------------------
> > +  # for Gen1
> 
> This seems to cause:
> 
> ./Documentation/devicetree/bindings/sound/renesas,rsnd.yaml:282:4: [error] missing starting space in comment (comments)
> ./Documentation/devicetree/bindings/sound/renesas,rsnd.yaml:284:4: [error] missing starting space in comment (comments)
> ./Documentation/devicetree/bindings/sound/renesas,rsnd.yaml:339:4: [error] missing starting space in comment (comments)
> ./Documentation/devicetree/bindings/sound/renesas,rsnd.yaml:341:4: [error] missing starting space in comment (comments)

Hmm... I couldn't reproduce this


Thank you for your help !!

Best regards
---
Kuninori Morimoto

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

[Krzysztof Kozlowski]

On 17/03/2023 00:44, Kuninori Morimoto wrote:
> 
> Hi Rafał
> 
>> Hi, this patch seems to add errors for me. Are my tools outdated or is
>> it a real issue? See below.
> (snip)
>>> +  #--------------------
>>> +  # reg/reg-names
>>> +  #--------------------
>>> +  # for Gen1
>>
>> This seems to cause:
>>
>> ./Documentation/devicetree/bindings/sound/renesas,rsnd.yaml:282:4: [error] missing starting space in comment (comments)
>> ./Documentation/devicetree/bindings/sound/renesas,rsnd.yaml:284:4: [error] missing starting space in comment (comments)
>> ./Documentation/devicetree/bindings/sound/renesas,rsnd.yaml:339:4: [error] missing starting space in comment (comments)
>> ./Documentation/devicetree/bindings/sound/renesas,rsnd.yaml:341:4: [error] missing starting space in comment (comments)
> 
> Hmm... I couldn't reproduce this
> 

It's visible on current next. I'll send a fix.

Best regards,
Krzysztof