[PATCH] dt-bindings: xlnx,vcu-settings: fix dt_binding_check warnings

[PATCH] dt-bindings: xlnx,vcu-settings: fix dt_binding_check warnings

[Michael Tretter]

When running make dt_binding_check, the xlnx,vcu-settings binding
triggers the following two warnings:

	'additionalProperties' is a required property

	example-0: vcu@a0041000:reg:0: [0, 2684620800, 0, 4096] is too long

Fix the binding and make the checker happy.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---

Hi,

The xlnx,vcu-settings binding was reviewed [0] before the bot started to
run automated tests on the device tree bindings, but now produces some
warnings. The original patch that introduces the binding is still in
Michal's tree and I am not entirely sure how to handle it, but here is a
patch.

Michael

[0] https://lore.kernel.org/linux-arm-kernel/20200429213659.GA9051@bogus/
---
 .../bindings/soc/xilinx/xlnx,vcu-settings.yaml    | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml b/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml
index 378d0ced43c8..cb245f400287 100644
--- a/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml
+++ b/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml
@@ -26,9 +26,18 @@ required:
   - compatible
   - reg
 
+additionalProperties: false
+
 examples:
   - |
-    xlnx_vcu: vcu@a0041000 {
-          compatible = "xlnx,vcu-settings", "syscon";
-          reg = <0x0 0xa0041000 0x0 0x1000>;
+    fpga {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        xlnx_vcu: vcu@a0041000 {
+            compatible = "xlnx,vcu-settings", "syscon";
+            reg = <0x0 0xa0041000 0x0 0x1000>;
+        };
     };
+
+...
-- 
2.20.1

Re: [PATCH] dt-bindings: xlnx,vcu-settings: fix dt_binding_check warnings

[Michal Simek]

On 02. 12. 20 10:05, Michael Tretter wrote:
> When running make dt_binding_check, the xlnx,vcu-settings binding
> triggers the following two warnings:
> 
> 	'additionalProperties' is a required property
> 
> 	example-0: vcu@a0041000:reg:0: [0, 2684620800, 0, 4096] is too long
> 
> Fix the binding and make the checker happy.
> 
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> 
> Hi,
> 
> The xlnx,vcu-settings binding was reviewed [0] before the bot started to
> run automated tests on the device tree bindings, but now produces some
> warnings. The original patch that introduces the binding is still in
> Michal's tree and I am not entirely sure how to handle it, but here is a
> patch.
> 
> Michael
> 
> [0] https://lore.kernel.org/linux-arm-kernel/20200429213659.GA9051@bogus/
> ---
>  .../bindings/soc/xilinx/xlnx,vcu-settings.yaml    | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml b/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml
> index 378d0ced43c8..cb245f400287 100644
> --- a/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml
> +++ b/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml
> @@ -26,9 +26,18 @@ required:
>    - compatible
>    - reg
>  
> +additionalProperties: false
> +
>  examples:
>    - |
> -    xlnx_vcu: vcu@a0041000 {
> -          compatible = "xlnx,vcu-settings", "syscon";
> -          reg = <0x0 0xa0041000 0x0 0x1000>;
> +    fpga {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        xlnx_vcu: vcu@a0041000 {
> +            compatible = "xlnx,vcu-settings", "syscon";
> +            reg = <0x0 0xa0041000 0x0 0x1000>;
> +        };

IIRC we had been discussing this recently and Rob wanted to have just
1/1 mapping here.

Take a look at 0db958b689ca9.

Thanks,
Michal

Re: [PATCH] dt-bindings: xlnx,vcu-settings: fix dt_binding_check warnings

[Michael Tretter]

On Thu, 03 Dec 2020 08:49:01 +0100, Michal Simek wrote:
> On 02. 12. 20 10:05, Michael Tretter wrote:
> > When running make dt_binding_check, the xlnx,vcu-settings binding
> > triggers the following two warnings:
> > 
> > 	'additionalProperties' is a required property
> > 
> > 	example-0: vcu@a0041000:reg:0: [0, 2684620800, 0, 4096] is too long
> > 
> > Fix the binding and make the checker happy.
> > 
> > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > ---
> > 
> > Hi,
> > 
> > The xlnx,vcu-settings binding was reviewed [0] before the bot started to
> > run automated tests on the device tree bindings, but now produces some
> > warnings. The original patch that introduces the binding is still in
> > Michal's tree and I am not entirely sure how to handle it, but here is a
> > patch.
> > 
> > Michael
> > 
> > [0] https://lore.kernel.org/linux-arm-kernel/20200429213659.GA9051@bogus/
> > ---
> >  .../bindings/soc/xilinx/xlnx,vcu-settings.yaml    | 15 ++++++++++++---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml b/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml
> > index 378d0ced43c8..cb245f400287 100644
> > --- a/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml
> > +++ b/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml
> > @@ -26,9 +26,18 @@ required:
> >    - compatible
> >    - reg
> >  
> > +additionalProperties: false
> > +
> >  examples:
> >    - |
> > -    xlnx_vcu: vcu@a0041000 {
> > -          compatible = "xlnx,vcu-settings", "syscon";
> > -          reg = <0x0 0xa0041000 0x0 0x1000>;
> > +    fpga {
> > +        #address-cells = <2>;
> > +        #size-cells = <2>;
> > +
> > +        xlnx_vcu: vcu@a0041000 {
> > +            compatible = "xlnx,vcu-settings", "syscon";
> > +            reg = <0x0 0xa0041000 0x0 0x1000>;
> > +        };
> 
> IIRC we had been discussing this recently and Rob wanted to have just
> 1/1 mapping here.
> 
> Take a look at 0db958b689ca9.

Thanks for the pointer.

Rob: Is there some kind of rule, when to use a 1/1 mapping and when to add a
bus with more cells? I still see several examples that add a bus with 2 cells.
I assume that they more or less legacy, but I didn't find any discussion going
beyond the commit description of 0db958b689ca9, which "just" fixes the
warnings.

I will send a v2, but I'd like to understand the rationale for having the 1/1
mapping first.

Michael

Re: [PATCH] dt-bindings: xlnx,vcu-settings: fix dt_binding_check warnings

[Rob Herring]

On Thu, Dec 03, 2020 at 09:48:04AM +0100, Michael Tretter wrote:
> On Thu, 03 Dec 2020 08:49:01 +0100, Michal Simek wrote:
> > On 02. 12. 20 10:05, Michael Tretter wrote:
> > > When running make dt_binding_check, the xlnx,vcu-settings binding
> > > triggers the following two warnings:
> > > 
> > > 	'additionalProperties' is a required property
> > > 
> > > 	example-0: vcu@a0041000:reg:0: [0, 2684620800, 0, 4096] is too long
> > > 
> > > Fix the binding and make the checker happy.
> > > 
> > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > > ---
> > > 
> > > Hi,
> > > 
> > > The xlnx,vcu-settings binding was reviewed [0] before the bot started to
> > > run automated tests on the device tree bindings, but now produces some
> > > warnings. The original patch that introduces the binding is still in
> > > Michal's tree and I am not entirely sure how to handle it, but here is a
> > > patch.
> > > 
> > > Michael
> > > 
> > > [0] https://lore.kernel.org/linux-arm-kernel/20200429213659.GA9051@bogus/
> > > ---
> > >  .../bindings/soc/xilinx/xlnx,vcu-settings.yaml    | 15 ++++++++++++---
> > >  1 file changed, 12 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml b/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml
> > > index 378d0ced43c8..cb245f400287 100644
> > > --- a/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml
> > > +++ b/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml
> > > @@ -26,9 +26,18 @@ required:
> > >    - compatible
> > >    - reg
> > >  
> > > +additionalProperties: false
> > > +
> > >  examples:
> > >    - |
> > > -    xlnx_vcu: vcu@a0041000 {
> > > -          compatible = "xlnx,vcu-settings", "syscon";
> > > -          reg = <0x0 0xa0041000 0x0 0x1000>;
> > > +    fpga {
> > > +        #address-cells = <2>;
> > > +        #size-cells = <2>;
> > > +
> > > +        xlnx_vcu: vcu@a0041000 {
> > > +            compatible = "xlnx,vcu-settings", "syscon";
> > > +            reg = <0x0 0xa0041000 0x0 0x1000>;
> > > +        };
> > 
> > IIRC we had been discussing this recently and Rob wanted to have just
> > 1/1 mapping here.
> > 
> > Take a look at 0db958b689ca9.
> 
> Thanks for the pointer.
> 
> Rob: Is there some kind of rule, when to use a 1/1 mapping and when to add a
> bus with more cells? I still see several examples that add a bus with 2 cells.
> I assume that they more or less legacy, but I didn't find any discussion going
> beyond the commit description of 0db958b689ca9, which "just" fixes the
> warnings.
> 
> I will send a v2, but I'd like to understand the rationale for having the 1/1
> mapping first.

Simplifies the example is all.

This one is fine as-is.

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH] dt-bindings: xlnx,vcu-settings: fix dt_binding_check warnings

[Michal Simek]

Hi Rob,

On 09. 12. 20 19:34, Rob Herring wrote:
> On Thu, Dec 03, 2020 at 09:48:04AM +0100, Michael Tretter wrote:
>> On Thu, 03 Dec 2020 08:49:01 +0100, Michal Simek wrote:
>>> On 02. 12. 20 10:05, Michael Tretter wrote:
>>>> When running make dt_binding_check, the xlnx,vcu-settings binding
>>>> triggers the following two warnings:
>>>>
>>>> 	'additionalProperties' is a required property
>>>>
>>>> 	example-0: vcu@a0041000:reg:0: [0, 2684620800, 0, 4096] is too long
>>>>
>>>> Fix the binding and make the checker happy.
>>>>
>>>> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
>>>> ---
>>>>
>>>> Hi,
>>>>
>>>> The xlnx,vcu-settings binding was reviewed [0] before the bot started to
>>>> run automated tests on the device tree bindings, but now produces some
>>>> warnings. The original patch that introduces the binding is still in
>>>> Michal's tree and I am not entirely sure how to handle it, but here is a
>>>> patch.
>>>>
>>>> Michael
>>>>
>>>> [0] https://lore.kernel.org/linux-arm-kernel/20200429213659.GA9051@bogus/
>>>> ---
>>>>  .../bindings/soc/xilinx/xlnx,vcu-settings.yaml    | 15 ++++++++++++---
>>>>  1 file changed, 12 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml b/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml
>>>> index 378d0ced43c8..cb245f400287 100644
>>>> --- a/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml
>>>> +++ b/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml
>>>> @@ -26,9 +26,18 @@ required:
>>>>    - compatible
>>>>    - reg
>>>>  
>>>> +additionalProperties: false
>>>> +
>>>>  examples:
>>>>    - |
>>>> -    xlnx_vcu: vcu@a0041000 {
>>>> -          compatible = "xlnx,vcu-settings", "syscon";
>>>> -          reg = <0x0 0xa0041000 0x0 0x1000>;
>>>> +    fpga {
>>>> +        #address-cells = <2>;
>>>> +        #size-cells = <2>;
>>>> +
>>>> +        xlnx_vcu: vcu@a0041000 {
>>>> +            compatible = "xlnx,vcu-settings", "syscon";
>>>> +            reg = <0x0 0xa0041000 0x0 0x1000>;
>>>> +        };
>>>
>>> IIRC we had been discussing this recently and Rob wanted to have just
>>> 1/1 mapping here.
>>>
>>> Take a look at 0db958b689ca9.
>>
>> Thanks for the pointer.
>>
>> Rob: Is there some kind of rule, when to use a 1/1 mapping and when to add a
>> bus with more cells? I still see several examples that add a bus with 2 cells.
>> I assume that they more or less legacy, but I didn't find any discussion going
>> beyond the commit description of 0db958b689ca9, which "just" fixes the
>> warnings.
>>
>> I will send a v2, but I'd like to understand the rationale for having the 1/1
>> mapping first.
> 
> Simplifies the example is all.
> 
> This one is fine as-is.
> 
> Reviewed-by: Rob Herring <robh@kernel.org>

I remember that we have been fixing that 2:2 mapping to 1:1 in past.

And simplification in this case would be
- reg = <0x0 0xa0041000 0x0 0x1000>;
+ reg = <0xa0041000 0x1000>;

That's why I would like to know what we should be asking people to do.
Or is it fine because it is the part of fpga node?

Thanks,
Michal

Re: [PATCH] dt-bindings: xlnx,vcu-settings: fix dt_binding_check warnings

[Rob Herring]

On Wed, Dec 9, 2020 at 12:59 PM Michal Simek <michal.simek@xilinx.com> wrote:
>
> Hi Rob,
>
> On 09. 12. 20 19:34, Rob Herring wrote:
> > On Thu, Dec 03, 2020 at 09:48:04AM +0100, Michael Tretter wrote:
> >> On Thu, 03 Dec 2020 08:49:01 +0100, Michal Simek wrote:
> >>> On 02. 12. 20 10:05, Michael Tretter wrote:
> >>>> When running make dt_binding_check, the xlnx,vcu-settings binding
> >>>> triggers the following two warnings:
> >>>>
> >>>>    'additionalProperties' is a required property
> >>>>
> >>>>    example-0: vcu@a0041000:reg:0: [0, 2684620800, 0, 4096] is too long
> >>>>
> >>>> Fix the binding and make the checker happy.
> >>>>
> >>>> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> >>>> ---
> >>>>
> >>>> Hi,
> >>>>
> >>>> The xlnx,vcu-settings binding was reviewed [0] before the bot started to
> >>>> run automated tests on the device tree bindings, but now produces some
> >>>> warnings. The original patch that introduces the binding is still in
> >>>> Michal's tree and I am not entirely sure how to handle it, but here is a
> >>>> patch.
> >>>>
> >>>> Michael
> >>>>
> >>>> [0] https://lore.kernel.org/linux-arm-kernel/20200429213659.GA9051@bogus/
> >>>> ---
> >>>>  .../bindings/soc/xilinx/xlnx,vcu-settings.yaml    | 15 ++++++++++++---
> >>>>  1 file changed, 12 insertions(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml b/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml
> >>>> index 378d0ced43c8..cb245f400287 100644
> >>>> --- a/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml
> >>>> +++ b/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml
> >>>> @@ -26,9 +26,18 @@ required:
> >>>>    - compatible
> >>>>    - reg
> >>>>
> >>>> +additionalProperties: false
> >>>> +
> >>>>  examples:
> >>>>    - |
> >>>> -    xlnx_vcu: vcu@a0041000 {
> >>>> -          compatible = "xlnx,vcu-settings", "syscon";
> >>>> -          reg = <0x0 0xa0041000 0x0 0x1000>;
> >>>> +    fpga {
> >>>> +        #address-cells = <2>;
> >>>> +        #size-cells = <2>;
> >>>> +
> >>>> +        xlnx_vcu: vcu@a0041000 {
> >>>> +            compatible = "xlnx,vcu-settings", "syscon";
> >>>> +            reg = <0x0 0xa0041000 0x0 0x1000>;
> >>>> +        };
> >>>
> >>> IIRC we had been discussing this recently and Rob wanted to have just
> >>> 1/1 mapping here.
> >>>
> >>> Take a look at 0db958b689ca9.
> >>
> >> Thanks for the pointer.
> >>
> >> Rob: Is there some kind of rule, when to use a 1/1 mapping and when to add a
> >> bus with more cells? I still see several examples that add a bus with 2 cells.
> >> I assume that they more or less legacy, but I didn't find any discussion going
> >> beyond the commit description of 0db958b689ca9, which "just" fixes the
> >> warnings.
> >>
> >> I will send a v2, but I'd like to understand the rationale for having the 1/1
> >> mapping first.
> >
> > Simplifies the example is all.
> >
> > This one is fine as-is.
> >
> > Reviewed-by: Rob Herring <robh@kernel.org>
>
> I remember that we have been fixing that 2:2 mapping to 1:1 in past.
>
> And simplification in this case would be
> - reg = <0x0 0xa0041000 0x0 0x1000>;
> + reg = <0xa0041000 0x1000>;
>
> That's why I would like to know what we should be asking people to do.
> Or is it fine because it is the part of fpga node?

1:1 is my preference, but I'm not going to enforce either way.

Rob

Re: [PATCH] dt-bindings: xlnx, vcu-settings: fix dt_binding_check warnings

[Rob Herring]

On Wed, 02 Dec 2020 10:05:22 +0100, Michael Tretter wrote:
> When running make dt_binding_check, the xlnx,vcu-settings binding
> triggers the following two warnings:
> 
> 	'additionalProperties' is a required property
> 
> 	example-0: vcu@a0041000:reg:0: [0, 2684620800, 0, 4096] is too long
> 
> Fix the binding and make the checker happy.
> 
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> 
> Hi,
> 
> The xlnx,vcu-settings binding was reviewed [0] before the bot started to
> run automated tests on the device tree bindings, but now produces some
> warnings. The original patch that introduces the binding is still in
> Michal's tree and I am not entirely sure how to handle it, but here is a
> patch.
> 
> Michael
> 
> [0] https://lore.kernel.org/linux-arm-kernel/20200429213659.GA9051@bogus/
> ---
>  .../bindings/soc/xilinx/xlnx,vcu-settings.yaml    | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 

Applied, thanks!