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

[Michal Simek <michal.simek@xilinx.com>]

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