diff mbox series

ASoC: dt-bindings: simple-card: care missing address #address-cells

Message ID 87pnay3ptb.wl-kuninori.morimoto.gx@renesas.com
State Changes Requested
Headers show
Series ASoC: dt-bindings: simple-card: care missing address #address-cells | expand

Checks

Context Check Description
robh/checkpatch warning total: 0 errors, 1 warnings, 83 lines checked

Commit Message

Kuninori Morimoto May 21, 2020, 3:54 a.m. UTC
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current simple-card will get below error,
because it doesn't care about #address-cells at some part.

	DTC     Documentation/devicetree/bindings/sound/simple-card.example.dt.yaml
	Documentation/devicetree/bindings/sound/simple-card.example.dts:171.46-173.15: \
		Warning (unit_address_vs_reg): /example-4/sound/simple-audio-card,cpu@0: \
		node has a unit name, but no reg or ranges property
	Documentation/devicetree/bindings/sound/simple-card.example.dts:175.37-177.15: \
		Warning (unit_address_vs_reg): /example-4/sound/simple-audio-card,cpu@1: \
		node has a unit name, but no reg or ranges property
	...

This patch fixup this issue.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 .../bindings/sound/simple-card.yaml           | 25 ++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

Comments

Rob Herring May 28, 2020, 10:39 p.m. UTC | #1
On Thu, May 21, 2020 at 12:54:56PM +0900, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Current simple-card will get below error,
> because it doesn't care about #address-cells at some part.
> 
> 	DTC     Documentation/devicetree/bindings/sound/simple-card.example.dt.yaml
> 	Documentation/devicetree/bindings/sound/simple-card.example.dts:171.46-173.15: \
> 		Warning (unit_address_vs_reg): /example-4/sound/simple-audio-card,cpu@0: \
> 		node has a unit name, but no reg or ranges property
> 	Documentation/devicetree/bindings/sound/simple-card.example.dts:175.37-177.15: \
> 		Warning (unit_address_vs_reg): /example-4/sound/simple-audio-card,cpu@1: \
> 		node has a unit name, but no reg or ranges property
> 	...
> 
> This patch fixup this issue.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  .../bindings/sound/simple-card.yaml           | 25 ++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/simple-card.yaml b/Documentation/devicetree/bindings/sound/simple-card.yaml
> index cb2bb5fac0e1..6c4c2c6d6d3c 100644
> --- a/Documentation/devicetree/bindings/sound/simple-card.yaml
> +++ b/Documentation/devicetree/bindings/sound/simple-card.yaml
> @@ -208,6 +208,11 @@ patternProperties:
>        reg:
>          maxItems: 1
>  
> +      "#address-cells":
> +        const: 1
> +      "#size-cells":
> +        const: 0
> +
>        # common properties
>        frame-master:
>          $ref: "#/definitions/frame-master"
> @@ -288,7 +293,6 @@ examples:
>  
>          #address-cells = <1>;
>          #size-cells = <0>;
> -
>          simple-audio-card,dai-link@0 {		/* I2S - HDMI */
>              reg = <0>;
>              format = "i2s";
> @@ -392,11 +396,15 @@ examples:
>          simple-audio-card,routing = "ak4642 Playback", "DAI0 Playback",
>                                      "ak4642 Playback", "DAI1 Playback";
>  
> +        #address-cells = <1>;
> +        #size-cells = <0>;
>          dpcmcpu: simple-audio-card,cpu@0 {
> +            reg = <0>;

You need to add 'reg' to the schema. This isn't flagged because 
'additionalProperties: false' is missing there too. 

>              sound-dai = <&rcar_sound 0>;
>          };
>  
>          simple-audio-card,cpu@1 {
> +            reg = <1>;
>              sound-dai = <&rcar_sound 1>;
>          };
>  
> @@ -427,7 +435,12 @@ examples:
>              "pcm3168a Playback", "DAI3 Playback",
>              "pcm3168a Playback", "DAI4 Playback";
>  
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
>          simple-audio-card,dai-link@0 {
> +            reg = <0>;
> +
>              format = "left_j";
>              bitclock-master = <&sndcpu0>;
>              frame-master = <&sndcpu0>;
> @@ -441,22 +454,30 @@ examples:
>          };
>  
>          simple-audio-card,dai-link@1 {
> +            reg = <1>;
> +
>              format = "i2s";
>              bitclock-master = <&sndcpu1>;
>              frame-master = <&sndcpu1>;
>  
>              convert-channels = <8>; /* TDM Split */
>  
> +            #address-cells = <1>;
> +            #size-cells = <0>;
>              sndcpu1: cpu@0 {
> +                reg = <0>;
>                  sound-dai = <&rcar_sound 1>;
>              };
>              cpu@1 {
> +                reg = <1>;
>                  sound-dai = <&rcar_sound 2>;
>              };
>              cpu@2 {
> +                reg = <2>;
>                  sound-dai = <&rcar_sound 3>;
>              };
>              cpu@3 {
> +                reg = <3>;
>                  sound-dai = <&rcar_sound 4>;
>              };
>              codec {
> @@ -468,6 +489,8 @@ examples:
>          };
>  
>          simple-audio-card,dai-link@2 {
> +            reg = <2>;
> +
>              format = "i2s";
>              bitclock-master = <&sndcpu2>;
>              frame-master = <&sndcpu2>;
> -- 
> 2.17.1
>
Kuninori Morimoto May 29, 2020, 2:41 a.m. UTC | #2
The Subject was "Re: [PATCH] ASoC: dt-bindings: simple-card: care missing address #address-cells"

Hi Rob

I'm trying to create v2 of simple-card patch,
And got issue which I can't solve by myself.

I think "xxx,yyy" (= which has "," at the property name)
needs special care, but it is very un-understandable...
Now, I'm give up.
So, can I ask you 2 things about Yaml Doc "xxx,yyy" type property ?

========================
1) reference own definitions from "xxx,yyy"
========================

I guess "xxx,yyy" naming property needs to has "description", right ?

But, it is OK if it references "/schemas/xxxx"

	--- OK ------
	xxx,yyy:
	  description: xxx
	  $ref: /schemas/types.yaml#/definitions/phandle-array
	-------------

but, will be error if it references own definitions

	--- NG ------
	xxx,yyy:
	  description: xxx
	  $ref: "#/definitions/mydef"
	-------------

This is the related error

	-- error(?) --
	xxx.yaml: properties:xxx,yyy:\
	  $ref: '#/definitions/mydef' does not match 'types.yaml#[/]{0,1}definitions/.*'
	--------------

# but, there is no problem if it was defined as "patternProperties"

Q. The "xxx,yyy" property can't references own definitions,
   or needs some magical extra settings ??

========================
2) phandle for "xxx,yyy"
========================

I noticed that it seems "xxx,yyy" property can't be referenced.
Here, "xxx,yyy" has "type: object" and "additionalProperties: false"
(below didn't happen if it doesn't have "additionalProperties: false")

If "xxx,yyy" has phandle, but not referenced,
This is not a problem.

	--- OK ---
	...
	foo = <&bar>;
	...
	xxx_yyy: xxx,yyy {
	  ...
	};
	--------------

But will be error if it is referenced.

	--- NG ---
	foo = <&xxx_yyy>;
	...
	xxx_yyy: xxx,yyy {
	  ...
	};
	------------

The error is

	-- error ---
	xxx.yaml: xxx.yyy: \
	Additional properties are not allowed ('phandle' was unexpected)
	------------

Q. The "xxx,yyy" needs magical settings to be referenced, or can't be ?
Rob Herring July 2, 2020, 1:54 p.m. UTC | #3
On Thu, May 28, 2020 at 8:41 PM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
>
>
> The Subject was "Re: [PATCH] ASoC: dt-bindings: simple-card: care missing address #address-cells"
>
> Hi Rob
>
> I'm trying to create v2 of simple-card patch,
> And got issue which I can't solve by myself.
>
> I think "xxx,yyy" (= which has "," at the property name)
> needs special care, but it is very un-understandable...
> Now, I'm give up.
> So, can I ask you 2 things about Yaml Doc "xxx,yyy" type property ?
>
> ========================
> 1) reference own definitions from "xxx,yyy"
> ========================
>
> I guess "xxx,yyy" naming property needs to has "description", right ?
>
> But, it is OK if it references "/schemas/xxxx"
>
>         --- OK ------
>         xxx,yyy:
>           description: xxx
>           $ref: /schemas/types.yaml#/definitions/phandle-array
>         -------------
>
> but, will be error if it references own definitions
>
>         --- NG ------
>         xxx,yyy:
>           description: xxx
>           $ref: "#/definitions/mydef"
>         -------------
>
> This is the related error
>
>         -- error(?) --
>         xxx.yaml: properties:xxx,yyy:\
>           $ref: '#/definitions/mydef' does not match 'types.yaml#[/]{0,1}definitions/.*'
>         --------------
>
> # but, there is no problem if it was defined as "patternProperties"
>
> Q. The "xxx,yyy" property can't references own definitions,
>    or needs some magical extra settings ??

No, it can't. The problem with definitions is we can't really check
and do fixups on the definitions with the meta-schema.

> ========================
> 2) phandle for "xxx,yyy"
> ========================
>
> I noticed that it seems "xxx,yyy" property can't be referenced.
> Here, "xxx,yyy" has "type: object" and "additionalProperties: false"
> (below didn't happen if it doesn't have "additionalProperties: false")
>
> If "xxx,yyy" has phandle, but not referenced,
> This is not a problem.
>
>         --- OK ---
>         ...
>         foo = <&bar>;
>         ...
>         xxx_yyy: xxx,yyy {
>           ...
>         };
>         --------------
>
> But will be error if it is referenced.
>
>         --- NG ---
>         foo = <&xxx_yyy>;
>         ...
>         xxx_yyy: xxx,yyy {
>           ...
>         };
>         ------------
>
> The error is
>
>         -- error ---
>         xxx.yaml: xxx.yyy: \
>         Additional properties are not allowed ('phandle' was unexpected)
>         ------------
>
> Q. The "xxx,yyy" needs magical settings to be referenced, or can't be ?

'phandle' (among other things) is automatically added by the tools. If
'xxx,yyy' is defined thru a 'definitions' then that fix-up is not
going to happen.

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/sound/simple-card.yaml b/Documentation/devicetree/bindings/sound/simple-card.yaml
index cb2bb5fac0e1..6c4c2c6d6d3c 100644
--- a/Documentation/devicetree/bindings/sound/simple-card.yaml
+++ b/Documentation/devicetree/bindings/sound/simple-card.yaml
@@ -208,6 +208,11 @@  patternProperties:
       reg:
         maxItems: 1
 
+      "#address-cells":
+        const: 1
+      "#size-cells":
+        const: 0
+
       # common properties
       frame-master:
         $ref: "#/definitions/frame-master"
@@ -288,7 +293,6 @@  examples:
 
         #address-cells = <1>;
         #size-cells = <0>;
-
         simple-audio-card,dai-link@0 {		/* I2S - HDMI */
             reg = <0>;
             format = "i2s";
@@ -392,11 +396,15 @@  examples:
         simple-audio-card,routing = "ak4642 Playback", "DAI0 Playback",
                                     "ak4642 Playback", "DAI1 Playback";
 
+        #address-cells = <1>;
+        #size-cells = <0>;
         dpcmcpu: simple-audio-card,cpu@0 {
+            reg = <0>;
             sound-dai = <&rcar_sound 0>;
         };
 
         simple-audio-card,cpu@1 {
+            reg = <1>;
             sound-dai = <&rcar_sound 1>;
         };
 
@@ -427,7 +435,12 @@  examples:
             "pcm3168a Playback", "DAI3 Playback",
             "pcm3168a Playback", "DAI4 Playback";
 
+        #address-cells = <1>;
+        #size-cells = <0>;
+
         simple-audio-card,dai-link@0 {
+            reg = <0>;
+
             format = "left_j";
             bitclock-master = <&sndcpu0>;
             frame-master = <&sndcpu0>;
@@ -441,22 +454,30 @@  examples:
         };
 
         simple-audio-card,dai-link@1 {
+            reg = <1>;
+
             format = "i2s";
             bitclock-master = <&sndcpu1>;
             frame-master = <&sndcpu1>;
 
             convert-channels = <8>; /* TDM Split */
 
+            #address-cells = <1>;
+            #size-cells = <0>;
             sndcpu1: cpu@0 {
+                reg = <0>;
                 sound-dai = <&rcar_sound 1>;
             };
             cpu@1 {
+                reg = <1>;
                 sound-dai = <&rcar_sound 2>;
             };
             cpu@2 {
+                reg = <2>;
                 sound-dai = <&rcar_sound 3>;
             };
             cpu@3 {
+                reg = <3>;
                 sound-dai = <&rcar_sound 4>;
             };
             codec {
@@ -468,6 +489,8 @@  examples:
         };
 
         simple-audio-card,dai-link@2 {
+            reg = <2>;
+
             format = "i2s";
             bitclock-master = <&sndcpu2>;
             frame-master = <&sndcpu2>;