[PATCH v2 14/15] ASoC: dt-bindings: renesas,rsnd.yaml: add common port-def

[PATCH v2 14/15] ASoC: dt-bindings: renesas,rsnd.yaml: add common port-def

[Kuninori Morimoto]

renesas,rsnd uses both "ports" has "port", and these are very similar.
To avoid duplicated definitions, this patch adds common port-def.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 .../bindings/sound/renesas,rsnd.yaml          | 38 ++++++++-----------
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
index 8a821dec9526..d9808b130e8d 100644
--- a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
+++ b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
@@ -9,6 +9,20 @@ title: Renesas R-Car Sound Driver
 maintainers:
   - Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
 
+definitions:
+  port-def:
+    $ref: audio-graph-port.yaml#/definitions/port-base
+    unevaluatedProperties: false
+    patternProperties:
+      "^endpoint(@[0-9a-f]+)?":
+        $ref: audio-graph-port.yaml#/definitions/endpoint-base
+        properties:
+          playback:
+            $ref: /schemas/types.yaml#/definitions/phandle-array
+          capture:
+            $ref: /schemas/types.yaml#/definitions/phandle-array
+        unevaluatedProperties: false
+
 properties:
 
   compatible:
@@ -108,30 +122,10 @@ properties:
     unevaluatedProperties: false
     patternProperties:
       '^port(@[0-9a-f]+)?$':
-        $ref: audio-graph-port.yaml#/definitions/port-base
-        unevaluatedProperties: false
-        patternProperties:
-          "^endpoint(@[0-9a-f]+)?":
-            $ref: audio-graph-port.yaml#/definitions/endpoint-base
-            properties:
-              playback:
-                $ref: /schemas/types.yaml#/definitions/phandle-array
-              capture:
-                $ref: /schemas/types.yaml#/definitions/phandle-array
-            unevaluatedProperties: false
+        $ref: "#/definitions/port-def"
 
   port:
-    $ref: audio-graph-port.yaml#/definitions/port-base
-    unevaluatedProperties: false
-    patternProperties:
-      "^endpoint(@[0-9a-f]+)?":
-        $ref: audio-graph-port.yaml#/definitions/endpoint-base
-        properties:
-          playback:
-            $ref: /schemas/types.yaml#/definitions/phandle-array
-          capture:
-            $ref: /schemas/types.yaml#/definitions/phandle-array
-        unevaluatedProperties: false
+    $ref: "#/definitions/port-def"
 
   rcar_sound,dvc:
     description: DVC subnode.
-- 
2.25.1

Re: [PATCH v2 14/15] ASoC: dt-bindings: renesas,rsnd.yaml: add common port-def

[Rob Herring]

On Mon, Jul 10, 2023 at 10:21:42AM +0900, Kuninori Morimoto wrote:
> renesas,rsnd uses both "ports" has "port", and these are very similar.
> To avoid duplicated definitions, this patch adds common port-def.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  .../bindings/sound/renesas,rsnd.yaml          | 38 ++++++++-----------
>  1 file changed, 16 insertions(+), 22 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> index 8a821dec9526..d9808b130e8d 100644
> --- a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> +++ b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> @@ -9,6 +9,20 @@ title: Renesas R-Car Sound Driver
>  maintainers:
>    - Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>  
> +definitions:

$defs

> +  port-def:
> +    $ref: audio-graph-port.yaml#/definitions/port-base
> +    unevaluatedProperties: false
> +    patternProperties:
> +      "^endpoint(@[0-9a-f]+)?":
> +        $ref: audio-graph-port.yaml#/definitions/endpoint-base
> +        properties:
> +          playback:
> +            $ref: /schemas/types.yaml#/definitions/phandle-array
> +          capture:
> +            $ref: /schemas/types.yaml#/definitions/phandle-array
> +        unevaluatedProperties: false
> +
>  properties:
>  
>    compatible:
> @@ -108,30 +122,10 @@ properties:
>      unevaluatedProperties: false
>      patternProperties:
>        '^port(@[0-9a-f]+)?$':
> -        $ref: audio-graph-port.yaml#/definitions/port-base
> -        unevaluatedProperties: false
> -        patternProperties:
> -          "^endpoint(@[0-9a-f]+)?":
> -            $ref: audio-graph-port.yaml#/definitions/endpoint-base
> -            properties:
> -              playback:
> -                $ref: /schemas/types.yaml#/definitions/phandle-array
> -              capture:
> -                $ref: /schemas/types.yaml#/definitions/phandle-array
> -            unevaluatedProperties: false
> +        $ref: "#/definitions/port-def"
>  
>    port:
> -    $ref: audio-graph-port.yaml#/definitions/port-base
> -    unevaluatedProperties: false
> -    patternProperties:
> -      "^endpoint(@[0-9a-f]+)?":
> -        $ref: audio-graph-port.yaml#/definitions/endpoint-base
> -        properties:
> -          playback:
> -            $ref: /schemas/types.yaml#/definitions/phandle-array
> -          capture:
> -            $ref: /schemas/types.yaml#/definitions/phandle-array
> -        unevaluatedProperties: false
> +    $ref: "#/definitions/port-def"

It would be better if users just always used "ports { port {}; };" even 
in the single port case.

>  
>    rcar_sound,dvc:
>      description: DVC subnode.
> -- 
> 2.25.1
> 

Re: [PATCH v2 14/15] ASoC: dt-bindings: renesas,rsnd.yaml: add common port-def

[Kuninori Morimoto]

Hi Rob

Thank you for reviewing


> > diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> > index 8a821dec9526..d9808b130e8d 100644
> > --- a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> > +++ b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> > @@ -9,6 +9,20 @@ title: Renesas R-Car Sound Driver
> >  maintainers:
> >    - Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> >  
> > +definitions:
> 
> $defs

Hmm... many drivers are using "definitions" ?
This kind of indication is very confusable...

> >    port:
> > -    $ref: audio-graph-port.yaml#/definitions/port-base
> > -    unevaluatedProperties: false
> > -    patternProperties:
> > -      "^endpoint(@[0-9a-f]+)?":
> > -        $ref: audio-graph-port.yaml#/definitions/endpoint-base
> > -        properties:
> > -          playback:
> > -            $ref: /schemas/types.yaml#/definitions/phandle-array
> > -          capture:
> > -            $ref: /schemas/types.yaml#/definitions/phandle-array
> > -        unevaluatedProperties: false
> > +    $ref: "#/definitions/port-def"
> 
> It would be better if users just always used "ports { port {}; };" even 
> in the single port case.

"ports" support is added, we can't modify past.



Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH v2 14/15] ASoC: dt-bindings: renesas,rsnd.yaml: add common port-def

[Rob Herring]

On Tue, Jul 11, 2023 at 09:00:18AM +0900, Kuninori Morimoto wrote:
> 
> Hi Rob
> 
> Thank you for reviewing
> 
> 
> > > diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> > > index 8a821dec9526..d9808b130e8d 100644
> > > --- a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> > > +++ b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> > > @@ -9,6 +9,20 @@ title: Renesas R-Car Sound Driver
> > >  maintainers:
> > >    - Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > >  
> > > +definitions:
> > 
> > $defs
> 
> Hmm... many drivers are using "definitions" ?

Patches welcome to change them. I don't think it's many (not counting 
types.yaml). Use of "definitions" or "$defs" is an exception.

> This kind of indication is very confusable...

"definitions" was common convention in early json-schema. "$defs" was 
added to the specification to standardize it.

> > >    port:
> > > -    $ref: audio-graph-port.yaml#/definitions/port-base
> > > -    unevaluatedProperties: false
> > > -    patternProperties:
> > > -      "^endpoint(@[0-9a-f]+)?":
> > > -        $ref: audio-graph-port.yaml#/definitions/endpoint-base
> > > -        properties:
> > > -          playback:
> > > -            $ref: /schemas/types.yaml#/definitions/phandle-array
> > > -          capture:
> > > -            $ref: /schemas/types.yaml#/definitions/phandle-array
> > > -        unevaluatedProperties: false
> > > +    $ref: "#/definitions/port-def"
> > 
> > It would be better if users just always used "ports { port {}; };" even 
> > in the single port case.
> 
> "ports" support is added, we can't modify past.

I don't understand. The graph APIs all work the same with or without 
'ports'. Changing the DT shouldn't affect the ABI.

Rob

Re: [PATCH v2 14/15] ASoC: dt-bindings: renesas,rsnd.yaml: add common port-def

[Kuninori Morimoto]

Hi Rob

> > > $defs
> > 
> > Hmm... many drivers are using "definitions" ?
> 
> Patches welcome to change them. I don't think it's many (not counting 
> types.yaml). Use of "definitions" or "$defs" is an exception.

OK, let's use "$defs" in v3


> > > It would be better if users just always used "ports { port {}; };" even 
> > > in the single port case.
> > 
> > "ports" support is added, we can't modify past.
> 
> I don't understand. The graph APIs all work the same with or without 
> 'ports'. Changing the DT shouldn't affect the ABI.

But, the driver didn't have "ports" handling code, "port" only.
From "DT point of view" changing port -> ports { port {}; }; is no problem,
but "driver code point of view", it is big problem.

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH v2 14/15] ASoC: dt-bindings: renesas,rsnd.yaml: add common port-def

[Kuninori Morimoto]

Hi Rob, Mark

> > > > diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> > > > index 8a821dec9526..d9808b130e8d 100644
> > > > --- a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> > > > +++ b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> > > > @@ -9,6 +9,20 @@ title: Renesas R-Car Sound Driver
> > > >  maintainers:
> > > >    - Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > > >  
> > > > +definitions:
> > > 
> > > $defs
> > 
> > Hmm... many drivers are using "definitions" ?
> 
> Patches welcome to change them. I don't think it's many (not counting 
> types.yaml). Use of "definitions" or "$defs" is an exception.

I will get error if I used "$defs" instead of "definitions", like below.
There is no error in "definitions" case.

Mark
Thus, I have no update for remainings (= [11/15] - [15/15]).


	> pip3 list | grep jsonschema
	jsonschema                     4.17.0
	> pip3 list | grep dtschema
	dtschema                       2023.6.1

	> make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
	  LINT    Documentation/devicetree/bindings
	  CHKDT   Documentation/devicetree/bindings/processed-schema.json
	/opt/disk_cd/morimoto/linux/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml:
	Unresolvable JSON pointer: 'definitions/port-def'
	  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
	Traceback (most recent call last):
	  File "/usr/local/bin/dt-mk-schema", line 38, in <module>
	    schemas = dtschema.DTValidator(args.schemas).schemas
	  File "/usr/local/lib/python3.8/dist-packages/dtschema/validator.py", line 355, in __init__
	    self.make_property_type_cache()
	  File "/usr/local/lib/python3.8/dist-packages/dtschema/validator.py", line 406, in make_property_type_cache
	    self.props, self.pat_props = get_prop_types(self.schemas)
	  File "/usr/local/lib/python3.8/dist-packages/dtschema/validator.py", line 173, in get_prop_types
	    props = extract_types(schemas)
	  File "/usr/local/lib/python3.8/dist-packages/dtschema/validator.py", line 165, in extract_types
	    _extract_subschema_types(props, sch, sch)
	  File "/usr/local/lib/python3.8/dist-packages/dtschema/validator.py", line 159, in _extract_subschema_types
	    _extract_prop_type(props, schema, p, v, k == 'patternProperties')
	  File "/usr/local/lib/python3.8/dist-packages/dtschema/validator.py", line 52, in _extract_prop_type
	    tmp_subschema = tmp_subschema[p]
	KeyError: 'definitions'
	make[2]: *** [/opt/disk_cd/morimoto/linux/Documentation/devicetree/bindings/Makefile:68: Documentation/devicetree/bindings/processed-schema.json] エ
	ラー 1
	make[2]: *** ファイル 'Documentation/devicetree/bindings/processed-schema.json'　を削除します
	make[1]: *** [/opt/disk_cd/morimoto/linux/Makefile:1500: dt_binding_check] エラー 2
	make: *** [/opt/disk_cd/morimoto/linux/Makefile:234: __sub-make] エラー 2

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH v2 14/15] ASoC: dt-bindings: renesas,rsnd.yaml: add common port-def

[Rob Herring]

On Wed, Jul 19, 2023 at 5:36 PM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
>
>
> Hi Rob, Mark
>
> > > > > diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> > > > > index 8a821dec9526..d9808b130e8d 100644
> > > > > --- a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> > > > > +++ b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> > > > > @@ -9,6 +9,20 @@ title: Renesas R-Car Sound Driver
> > > > >  maintainers:
> > > > >    - Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > > > >
> > > > > +definitions:
> > > >
> > > > $defs
> > >
> > > Hmm... many drivers are using "definitions" ?
> >
> > Patches welcome to change them. I don't think it's many (not counting
> > types.yaml). Use of "definitions" or "$defs" is an exception.
>
> I will get error if I used "$defs" instead of "definitions", like below.

Are you sure you used $defs?

> There is no error in "definitions" case.
>
> Mark
> Thus, I have no update for remainings (= [11/15] - [15/15]).
>
>
>         > pip3 list | grep jsonschema
>         jsonschema                     4.17.0
>         > pip3 list | grep dtschema
>         dtschema                       2023.6.1
>
>         > make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
>           LINT    Documentation/devicetree/bindings
>           CHKDT   Documentation/devicetree/bindings/processed-schema.json
>         /opt/disk_cd/morimoto/linux/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml:
>         Unresolvable JSON pointer: 'definitions/port-def'

Doesn't look like it. You have a ref to 'definitions/port-def'.

Rob

[PATCH v2 15/15] ASoC: dt-bindings: renesas,rsnd.yaml: enable multi ports for multi Component support

[Kuninori Morimoto]

To enable multi Component support, "multi ports" is needed for Audio Graph
Card/Card2, and "multi rcar_sound,dai" is needed for Simple Audio Card.
This patch enable these.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 .../bindings/sound/renesas,rsnd.yaml          | 24 ++++++++++++-------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
index d9808b130e8d..13a5a0a10fe6 100644
--- a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
+++ b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
@@ -91,6 +91,12 @@ properties:
       it must be 1 if your system has audio_clkout0/1/2/3
     enum: [0, 1]
 
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
   clock-frequency:
     description: for audio_clkout0/1/2/3
 
@@ -117,13 +123,7 @@ properties:
     description: List of necessary clock names.
     # details are defined below
 
-  ports:
-    $ref: audio-graph-port.yaml#/definitions/port-base
-    unevaluatedProperties: false
-    patternProperties:
-      '^port(@[0-9a-f]+)?$':
-        $ref: "#/definitions/port-def"
-
+  # ports is below
   port:
     $ref: "#/definitions/port-def"
 
@@ -242,8 +242,9 @@ properties:
           - interrupts
     additionalProperties: false
 
+patternProperties:
   # For DAI base
-  rcar_sound,dai:
+  'rcar_sound,dai(@[0-9a-f]+)?$':
     description: DAI subnode.
     type: object
     patternProperties:
@@ -263,6 +264,13 @@ properties:
               - capture
     additionalProperties: false
 
+  'ports(@[0-9a-f]+)?$':
+    $ref: audio-graph-port.yaml#/definitions/port-base
+    unevaluatedProperties: false
+    patternProperties:
+      '^port(@[0-9a-f]+)?$':
+        $ref: "#/definitions/port-def"
+
 required:
   - compatible
   - reg
-- 
2.25.1

Re: [PATCH v2 15/15] ASoC: dt-bindings: renesas,rsnd.yaml: enable multi ports for multi Component support

[Rob Herring]

On Mon, Jul 10, 2023 at 10:21:58AM +0900, Kuninori Morimoto wrote:
> To enable multi Component support, "multi ports" is needed for Audio Graph
> Card/Card2, and "multi rcar_sound,dai" is needed for Simple Audio Card.
> This patch enable these.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  .../bindings/sound/renesas,rsnd.yaml          | 24 ++++++++++++-------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> index d9808b130e8d..13a5a0a10fe6 100644
> --- a/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> +++ b/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
> @@ -91,6 +91,12 @@ properties:
>        it must be 1 if your system has audio_clkout0/1/2/3
>      enum: [0, 1]
>  
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
>    clock-frequency:
>      description: for audio_clkout0/1/2/3
>  
> @@ -117,13 +123,7 @@ properties:
>      description: List of necessary clock names.
>      # details are defined below
>  
> -  ports:
> -    $ref: audio-graph-port.yaml#/definitions/port-base
> -    unevaluatedProperties: false
> -    patternProperties:
> -      '^port(@[0-9a-f]+)?$':
> -        $ref: "#/definitions/port-def"
> -
> +  # ports is below
>    port:
>      $ref: "#/definitions/port-def"
>  
> @@ -242,8 +242,9 @@ properties:
>            - interrupts
>      additionalProperties: false
>  
> +patternProperties:
>    # For DAI base
> -  rcar_sound,dai:
> +  'rcar_sound,dai(@[0-9a-f]+)?$':

With a unit-address you need a 'reg' property.

>      description: DAI subnode.
>      type: object
>      patternProperties:
> @@ -263,6 +264,13 @@ properties:
>                - capture
>      additionalProperties: false
>  
> +  'ports(@[0-9a-f]+)?$':

Multiple 'ports' nodes is not valid.

Here too would need a 'reg' property. Did you test any of this because 
that would be a dtc warning.

> +    $ref: audio-graph-port.yaml#/definitions/port-base
> +    unevaluatedProperties: false
> +    patternProperties:
> +      '^port(@[0-9a-f]+)?$':
> +        $ref: "#/definitions/port-def"
> +
>  required:
>    - compatible
>    - reg
> -- 
> 2.25.1
> 

Re: [PATCH v2 15/15] ASoC: dt-bindings: renesas,rsnd.yaml: enable multi ports for multi Component support

[Kuninori Morimoto]

Hi Rob

Thank you for reviewing

> > +patternProperties:
> >    # For DAI base
> > -  rcar_sound,dai:
> > +  'rcar_sound,dai(@[0-9a-f]+)?$':
> 
> With a unit-address you need a 'reg' property.
(snip)
> Here too would need a 'reg' property. Did you test any of this because 
> that would be a dtc warning.

I don't remember detail, but some reviewer indicated that it is not needed,
because we have default "reg" property somewhere/somehow.


Thank you for your help !!

Best regards
---
Kuninori Morimoto