Re: [PATCH 1/1] dt-bindings: timer: Convert ingenic,tcu.txt to YAML

[Paul Cercueil <paul@crapouillou.net>]

Le lun., mars 2, 2020 at 13:07, Rob Herring <robh+dt@kernel.org> a 
écrit :
> On Mon, Mar 2, 2020 at 12:25 PM Paul Cercueil <paul@crapouillou.net> 
> wrote:
>> 
>>  Hi Rob,
>> 
>> 
>>  Le lun., mars 2, 2020 at 11:06, Rob Herring <robh+dt@kernel.org> a
>>  écrit :
>>  > On Sun, Mar 1, 2020 at 11:47 AM Paul Cercueil 
>> <paul@crapouillou.net>
>>  > wrote:
>>  >>
>>  >
>>  > Well, this flew into linux-next quickly and breaks 'make
>>  > dt_binding_check'... Please drop, revert or fix quickly.
>> 
>>  For my defense I said to merge "provided Rob acks it" ;)
>> 
>>  >>  Convert the ingenic,tcu.txt file to YAML.
>>  >>
>>  >>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  >>  ---
>>  >>   .../devicetree/bindings/timer/ingenic,tcu.txt | 138 ----------
>>  >>   .../bindings/timer/ingenic,tcu.yaml           | 235
>>  >> ++++++++++++++++++
>>  >>   2 files changed, 235 insertions(+), 138 deletions(-)
>>  >>   delete mode 100644
>>  >> Documentation/devicetree/bindings/timer/ingenic,tcu.txt
>>  >>   create mode 100644
>>  >> Documentation/devicetree/bindings/timer/ingenic,tcu.yaml
>>  >
>>  >
>>  >>  diff --git
>>  >> a/Documentation/devicetree/bindings/timer/ingenic,tcu.yaml
>>  >> b/Documentation/devicetree/bindings/timer/ingenic,tcu.yaml
>>  >>  new file mode 100644
>>  >>  index 000000000000..1ded3b4762bb
>>  >>  --- /dev/null
>>  >>  +++ b/Documentation/devicetree/bindings/timer/ingenic,tcu.yaml
>>  >>  @@ -0,0 +1,235 @@
>>  >>  +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>  >>  +%YAML 1.2
>>  >>  +---
>>  >>  +$id: http://devicetree.org/schemas/timer/ingenic,tcu.yaml#
>>  >>  +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>  >>  +
>>  >>  +title: Ingenic SoCs Timer/Counter Unit (TCU) devicetree 
>> bindings
>>  >>  +
>>  >>  +description: |
>>  >>  +  For a description of the TCU hardware and drivers, have a 
>> look at
>>  >>  +  Documentation/mips/ingenic-tcu.rst.
>>  >>  +
>>  >>  +maintainers:
>>  >>  +  - Paul Cercueil <paul@crapouillou.net>
>>  >>  +
>>  >>  +properties:
>>  >>  +  $nodename:
>>  >>  +    pattern: "^timer@.*"
>>  >
>>  > '.*' is redundant.
>>  >
>>  >>  +
>>  >>  +  "#address-cells":
>>  >>  +    const: 1
>>  >>  +
>>  >>  +  "#size-cells":
>>  >>  +    const: 1
>>  >>  +
>>  >>  +  "#clock-cells":
>>  >>  +    const: 1
>>  >>  +
>>  >>  +  "#interrupt-cells":
>>  >>  +    const: 1
>>  >>  +
>>  >>  +  interrupt-controller: true
>>  >>  +
>>  >>  +  ranges: true
>>  >>  +
>>  >>  +  compatible:
>>  >>  +    items:
>>  >>  +      - enum:
>>  >>  +        - ingenic,jz4740-tcu
>>  >>  +        - ingenic,jz4725b-tcu
>>  >>  +        - ingenic,jz4770-tcu
>>  >>  +        - ingenic,x1000-tcu
>>  >>  +      - const: simple-mfd
>>  >
>>  > This breaks several examples in dt_binding_check because this 
>> schema
>>  > will be applied to every 'simple-mfd' node. You need a custom 
>> select
>>  > entry that excludes 'simple-mfd'. There should be several 
>> examples in
>>  > tree to copy.
>> 
>>  Why would it be applied to all 'single-mfd' nodes?
> 
> single-mfd?

simple-mfd* of course, sorry.

> The way the tool decides to apply a schema or not is my matching on
> any of the compatible strings (or node name if no compatible
> specified). You can override this with 'select'.
> 
>>  Doesn't what I wrote
>>  specify that it needs one of ingenic,*-tcu _and_ simple-mfd?
> 
> Yes, but matching is on any of them. You need to add:

Alright, will do. Is there a reason why it's done that way? It sounds a 
bit counter-intuitive.

> select:
>   properties:
>     compatible:
>       contains:
>         enum:
>           - ingenic,jz4740-tcu
>           - ingenic,jz4725b-tcu
>           - ingenic,jz4770-tcu
>           - ingenic,x1000-tcu
>   required:
>     - compatible
> 
>>  I'm not sure I understand what you mean.
>> 
>>  I did grep for 'single-mfd' in all YAML files in Documentation/ and
>>  nothing really stands out.
> 
> I guess even without the typo it was harder to find an example than I 
> thought.
> 
> Note that I think I'll make the tool exclude 'simple-mfd', but it will
> take some time for users to update so you still need to fix this.

Alright, thanks for the help.

-Paul