Re: [PATCH] dt-bindings: Whitespace clean-ups in schema files

Re: [PATCH] dt-bindings: Whitespace clean-ups in schema files

[Sam Ravnborg]

Hi Rob.

On Wed, Aug 12, 2020 at 02:36:18PM -0600, Rob Herring wrote:
> Clean-up incorrect indentation, extra spaces, long lines, and missing
> EOF newline in schema files. Most of the clean-ups are for list
> indentation which should always be 2 spaces more than the preceding
> keyword.
> 
> Found with yamllint (which I plan to integrate into the checks).

I have browsed through the patch - and there was only a few things
that jumped at me.

With these points considered:
Acked-by: Sam Ravnborg <sam@ravnborg.org>

I expect only some (few) of my points to actually results in any updates.

I look forward to have the lint functionality as part of the built-in
tools so we catch these things early.

	Sam

> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> index f63895c8ce2d..88814a2a14a5 100644
> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> @@ -273,8 +273,8 @@ properties:
>                - fsl,imx6ull-14x14-evk     # i.MX6 UltraLiteLite 14x14 EVK Board
>                - kontron,imx6ull-n6411-som # Kontron N6411 SOM
>                - myir,imx6ull-mys-6ulx-eval # MYiR Tech iMX6ULL Evaluation Board
> -              - toradex,colibri-imx6ull-eval            # Colibri iMX6ULL Module on Colibri Evaluation Board
> -              - toradex,colibri-imx6ull-wifi-eval       # Colibri iMX6ULL Wi-Fi / Bluetooth Module on Colibri Evaluation Board
> +              - toradex,colibri-imx6ull-eval      # Colibri iMX6ULL Module on Colibri Eval Board
> +              - toradex,colibri-imx6ull-wifi-eval # Colibri iMX6ULL Wi-Fi / BT Module on Colibri Eval Board
>            - const: fsl,imx6ull

This change looks bad as it drops the alignment with the comments below.
See following patch chunck:

>
>        - description: Kontron N6411 S Board
> @@ -312,9 +312,12 @@ properties:
>                - toradex,colibri-imx7d                   # Colibri iMX7 Dual Module
>                - toradex,colibri-imx7d-aster             # Colibri iMX7 Dual Module on Aster Carrier Board
>                - toradex,colibri-imx7d-emmc              # Colibri iMX7 Dual 1GB (eMMC) Module
> -              - toradex,colibri-imx7d-emmc-aster        # Colibri iMX7 Dual 1GB (eMMC) Module on Aster Carrier Board
> -              - toradex,colibri-imx7d-emmc-eval-v3      # Colibri iMX7 Dual 1GB (eMMC) Module on Colibri Evaluation Board V3
> -              - toradex,colibri-imx7d-eval-v3           # Colibri iMX7 Dual Module on Colibri Evaluation Board V3
> +              - toradex,colibri-imx7d-emmc-aster        # Colibri iMX7 Dual 1GB (eMMC) Module on
> +                                                        #  Aster Carrier Board



> diff --git a/Documentation/devicetree/bindings/display/panel/ilitek,ili9322.yaml b/Documentation/devicetree/bindings/display/panel/ilitek,ili9322.yaml
> index 177d48c5bd97..e89c1ea62ffa 100644
> --- a/Documentation/devicetree/bindings/display/panel/ilitek,ili9322.yaml
> +++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9322.yaml
> @@ -25,8 +25,7 @@ properties:
>    compatible:
>      items:
>        - enum:
> -        - dlink,dir-685-panel
> -
> +          - dlink,dir-685-panel
>        - const: ilitek,ili9322
>
>    reset-gpios: true
> diff --git a/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml b/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml
> index a39332276bab..76a9068a85dd 100644
> --- a/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml
> +++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml
> @@ -13,8 +13,7 @@ properties:
>    compatible:
>      items:
>        - enum:
> -        - bananapi,lhr050h41
> -
> +          - bananapi,lhr050h41
>        - const: ilitek,ili9881c
>

The extra lines is a simple way to indicate that here shall be added
more in the future. So I like the empty line.


> diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
> index 32e0896c6bc1..47938e372987 100644
> --- a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
> +++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
> @@ -79,7 +79,8 @@ properties:
>      description: |
>        kHz; switching frequency.
>      $ref: /schemas/types.yaml#/definitions/uint32
> -    enum: [ 600, 640, 685, 738, 800, 872, 960, 1066, 1200, 1371, 1600, 1920, 2400, 3200, 4800, 9600 ]
> +    enum: [ 600, 640, 685, 738, 800, 872, 960, 1066, 1200, 1371, 1600, 1920,
> +            2400, 3200, 4800, 9600 ]
>
>    qcom,ovp:
>      description: |

In the modern world we are living in now line length of 100 chars are
OK. checkpatch and coding_style is updated to reflected this.

> diff --git a/Documentation/devicetree/bindings/spi/mikrotik,rb4xx-spi.yaml b/Documentation/devicetree/bindings/spi/mikrotik,rb4xx-spi.yaml
> index 4ddb42a4ae05..9102feae90a2 100644
> --- a/Documentation/devicetree/bindings/spi/mikrotik,rb4xx-spi.yaml
> +++ b/Documentation/devicetree/bindings/spi/mikrotik,rb4xx-spi.yaml
> @@ -33,4 +33,5 @@ examples:
>          reg = <0x1f000000 0x10>;
>      };
>
> -...
> \ No newline at end of file
> +...
> +

Added one line too much?

 diff --git a/Documentation/devicetree/bindings/spi/spi-mux.yaml b/Documentation/devicetree/bindings/spi/spi-mux.yaml
> index 0ae692dc28b5..3d3fed63409b 100644
> --- a/Documentation/devicetree/bindings/spi/spi-mux.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-mux.yaml
> @@ -43,47 +43,47 @@ properties:
>      maxItems: 1
>
>  required:
> -   - compatible
> -   - reg
> -   - spi-max-frequency
> -   - mux-controls
> +  - compatible
> +  - reg
> +  - spi-max-frequency
> +  - mux-controls
>
>  examples:
> -   - |
> -     #include <dt-bindings/gpio/gpio.h>
> -     mux: mux-controller {
> -       compatible = "gpio-mux";
> -       #mux-control-cells = <0>;
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    mux: mux-controller {
> +        compatible = "gpio-mux";
> +        #mux-control-cells = <0>;
>
> -       mux-gpios = <&gpio0 3 GPIO_ACTIVE_HIGH>;
> -     };
> +        mux-gpios = <&gpio0 3 GPIO_ACTIVE_HIGH>;
> +    };

Example is updated to use 4-space indent. I like.

But many other examples are left untouched.

So I wonder if updating all examples to the same indent should
be left for another mega-patch?

> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index f3d847832fdc..2baee2c817c1 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -993,7 +993,8 @@ patternProperties:
>    "^sst,.*":
>      description: Silicon Storage Technology, Inc.
>    "^sstar,.*":
> -    description: Xiamen Xingchen(SigmaStar) Technology Co., Ltd. (formerly part of MStar Semiconductor, Inc.)
> +    description: Xiamen Xingchen(SigmaStar) Technology Co., Ltd.
> +      (formerly part of MStar Semiconductor, Inc.)
>    "^st,.*":
>      description: STMicroelectronics
>    "^starry,.*":

Did you check that they are all in alphabetical order?
I would be suprised if this is the only issue in this file.

Re: [PATCH] dt-bindings: Whitespace clean-ups in schema files

[Joe Perches]

On Wed, 2020-08-12 at 14:36 -0600, Rob Herring wrote:
> Clean-up incorrect indentation, extra spaces, long lines, and missing
> EOF newline in schema files. Most of the clean-ups are for list
> indentation which should always be 2 spaces more than the preceding
                                     ^
> keyword.
[]
> diff --git a/Documentation/devicetree/bindings/arm/arm,integrator.yaml b/Documentation/devicetree/bindings/arm/arm,integrator.yaml
> index 192ded470e32..f0daf990e077 100644
> --- a/Documentation/devicetree/bindings/arm/arm,integrator.yaml
> +++ b/Documentation/devicetree/bindings/arm/arm,integrator.yaml
> @@ -67,9 +67,9 @@ patternProperties:
>        compatible:
>          items:
>            - enum:
> -            - arm,integrator-ap-syscon
> -            - arm,integrator-cp-syscon
> -            - arm,integrator-sp-syscon
> +              - arm,integrator-ap-syscon
> +              - arm,integrator-cp-syscon
> +              - arm,integrator-sp-syscon

Confused a bit here.
          - enum:
	10 spaces to dash
old line:
            - arm,integrator-ap-syscon
	12 spaces to dash
new line:
              - arm,integrator-ap-syscon
	14 spaces to dash

Is it supposed to be 2 spaces more than the preceding line
or 4 more?

Re: [PATCH] dt-bindings: Whitespace clean-ups in schema files

[Rob Herring]

On Wed, Aug 12, 2020 at 3:34 PM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Rob.
>
> On Wed, Aug 12, 2020 at 02:36:18PM -0600, Rob Herring wrote:
> > Clean-up incorrect indentation, extra spaces, long lines, and missing
> > EOF newline in schema files. Most of the clean-ups are for list
> > indentation which should always be 2 spaces more than the preceding
> > keyword.
> >
> > Found with yamllint (which I plan to integrate into the checks).
>
> I have browsed through the patch - and there was only a few things
> that jumped at me.
>
> With these points considered:
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
>
> I expect only some (few) of my points to actually results in any updates.
>
> I look forward to have the lint functionality as part of the built-in
> tools so we catch these things early.
>
>         Sam
>
> > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> > index f63895c8ce2d..88814a2a14a5 100644
> > --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> > @@ -273,8 +273,8 @@ properties:
> >                - fsl,imx6ull-14x14-evk     # i.MX6 UltraLiteLite 14x14 EVK Board
> >                - kontron,imx6ull-n6411-som # Kontron N6411 SOM
> >                - myir,imx6ull-mys-6ulx-eval # MYiR Tech iMX6ULL Evaluation Board
> > -              - toradex,colibri-imx6ull-eval            # Colibri iMX6ULL Module on Colibri Evaluation Board
> > -              - toradex,colibri-imx6ull-wifi-eval       # Colibri iMX6ULL Wi-Fi / Bluetooth Module on Colibri Evaluation Board
> > +              - toradex,colibri-imx6ull-eval      # Colibri iMX6ULL Module on Colibri Eval Board
> > +              - toradex,colibri-imx6ull-wifi-eval # Colibri iMX6ULL Wi-Fi / BT Module on Colibri Eval Board
> >            - const: fsl,imx6ull
>
> This change looks bad as it drops the alignment with the comments below.
> See following patch chunck:

Yes, but as a whole there's no alignment in this file. Even the rest
of the entries for the hunk below aren't aligned.

Perhaps this form would be better:

    # Colibri iMX6ULL Wi-Fi / BT Module on Colibri Eval Board
  - toradex,colibri-imx6ull-wifi-eval

But I really don't want to go fix this in the whole file...

> >        - description: Kontron N6411 S Board
> > @@ -312,9 +312,12 @@ properties:
> >                - toradex,colibri-imx7d                   # Colibri iMX7 Dual Module
> >                - toradex,colibri-imx7d-aster             # Colibri iMX7 Dual Module on Aster Carrier Board
> >                - toradex,colibri-imx7d-emmc              # Colibri iMX7 Dual 1GB (eMMC) Module
> > -              - toradex,colibri-imx7d-emmc-aster        # Colibri iMX7 Dual 1GB (eMMC) Module on Aster Carrier Board
> > -              - toradex,colibri-imx7d-emmc-eval-v3      # Colibri iMX7 Dual 1GB (eMMC) Module on Colibri Evaluation Board V3
> > -              - toradex,colibri-imx7d-eval-v3           # Colibri iMX7 Dual Module on Colibri Evaluation Board V3
> > +              - toradex,colibri-imx7d-emmc-aster        # Colibri iMX7 Dual 1GB (eMMC) Module on
> > +                                                        #  Aster Carrier Board
>
>
>
> > diff --git a/Documentation/devicetree/bindings/display/panel/ilitek,ili9322.yaml b/Documentation/devicetree/bindings/display/panel/ilitek,ili9322.yaml
> > index 177d48c5bd97..e89c1ea62ffa 100644
> > --- a/Documentation/devicetree/bindings/display/panel/ilitek,ili9322.yaml
> > +++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9322.yaml
> > @@ -25,8 +25,7 @@ properties:
> >    compatible:
> >      items:
> >        - enum:
> > -        - dlink,dir-685-panel
> > -
> > +          - dlink,dir-685-panel
> >        - const: ilitek,ili9322
> >
> >    reset-gpios: true
> > diff --git a/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml b/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml
> > index a39332276bab..76a9068a85dd 100644
> > --- a/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml
> > +++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml
> > @@ -13,8 +13,7 @@ properties:
> >    compatible:
> >      items:
> >        - enum:
> > -        - bananapi,lhr050h41
> > -
> > +          - bananapi,lhr050h41
> >        - const: ilitek,ili9881c
> >
>
> The extra lines is a simple way to indicate that here shall be added
> more in the future. So I like the empty line.

News to me. I thought 'enum' indicates that. My preference here is a
blank line just between DT properties.

> > diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
> > index 32e0896c6bc1..47938e372987 100644
> > --- a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
> > +++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
> > @@ -79,7 +79,8 @@ properties:
> >      description: |
> >        kHz; switching frequency.
> >      $ref: /schemas/types.yaml#/definitions/uint32
> > -    enum: [ 600, 640, 685, 738, 800, 872, 960, 1066, 1200, 1371, 1600, 1920, 2400, 3200, 4800, 9600 ]
> > +    enum: [ 600, 640, 685, 738, 800, 872, 960, 1066, 1200, 1371, 1600, 1920,
> > +            2400, 3200, 4800, 9600 ]
> >
> >    qcom,ovp:
> >      description: |
>
> In the modern world we are living in now line length of 100 chars are
> OK. checkpatch and coding_style is updated to reflected this.

Yes, and it was 102. For yamllint I actually put it at 110 just to get
to a reasonable number that I wanted to fix and warning free. I think
I fixed all non comment cases to be less than 100 and comments to be
up to 110.

> > diff --git a/Documentation/devicetree/bindings/spi/mikrotik,rb4xx-spi.yaml b/Documentation/devicetree/bindings/spi/mikrotik,rb4xx-spi.yaml
> > index 4ddb42a4ae05..9102feae90a2 100644
> > --- a/Documentation/devicetree/bindings/spi/mikrotik,rb4xx-spi.yaml
> > +++ b/Documentation/devicetree/bindings/spi/mikrotik,rb4xx-spi.yaml
> > @@ -33,4 +33,5 @@ examples:
> >          reg = <0x1f000000 0x10>;
> >      };
> >
> > -...
> > \ No newline at end of file
> > +...
> > +
>
> Added one line too much?

Indeed.

>  diff --git a/Documentation/devicetree/bindings/spi/spi-mux.yaml b/Documentation/devicetree/bindings/spi/spi-mux.yaml
> > index 0ae692dc28b5..3d3fed63409b 100644
> > --- a/Documentation/devicetree/bindings/spi/spi-mux.yaml
> > +++ b/Documentation/devicetree/bindings/spi/spi-mux.yaml
> > @@ -43,47 +43,47 @@ properties:
> >      maxItems: 1
> >
> >  required:
> > -   - compatible
> > -   - reg
> > -   - spi-max-frequency
> > -   - mux-controls
> > +  - compatible
> > +  - reg
> > +  - spi-max-frequency
> > +  - mux-controls
> >
> >  examples:
> > -   - |
> > -     #include <dt-bindings/gpio/gpio.h>
> > -     mux: mux-controller {
> > -       compatible = "gpio-mux";
> > -       #mux-control-cells = <0>;
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +    mux: mux-controller {
> > +        compatible = "gpio-mux";
> > +        #mux-control-cells = <0>;
> >
> > -       mux-gpios = <&gpio0 3 GPIO_ACTIVE_HIGH>;
> > -     };
> > +        mux-gpios = <&gpio0 3 GPIO_ACTIVE_HIGH>;
> > +    };
>
> Example is updated to use 4-space indent. I like.
>
> But many other examples are left untouched.

That was not the purpose here. The '- |' line was indented 1 too many.
IIRC, the parser is not happy if you only change that line, so at
least the 1st line of the example had to be updated anyways.

> So I wonder if updating all examples to the same indent should
> be left for another mega-patch?

I've said this before, but until example indentation is automatically
checked I'm not going to care.

> > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > index f3d847832fdc..2baee2c817c1 100644
> > --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > @@ -993,7 +993,8 @@ patternProperties:
> >    "^sst,.*":
> >      description: Silicon Storage Technology, Inc.
> >    "^sstar,.*":
> > -    description: Xiamen Xingchen(SigmaStar) Technology Co., Ltd. (formerly part of MStar Semiconductor, Inc.)
> > +    description: Xiamen Xingchen(SigmaStar) Technology Co., Ltd.
> > +      (formerly part of MStar Semiconductor, Inc.)
> >    "^st,.*":
> >      description: STMicroelectronics
> >    "^starry,.*":
>
> Did you check that they are all in alphabetical order?
> I would be suprised if this is the only issue in this file.

Nope, as that's not a WS or linter thing. Alphabetical order is about
the only thing I look at reviewing additions to this, but I'm sure
some errors have slipped in.

Thanks for taking a look.


Rob

Re: [PATCH] dt-bindings: Whitespace clean-ups in schema files

[Rob Herring]

On Wed, Aug 12, 2020 at 4:32 PM Joe Perches <joe@perches.com> wrote:
>
> On Wed, 2020-08-12 at 14:36 -0600, Rob Herring wrote:
> > Clean-up incorrect indentation, extra spaces, long lines, and missing
> > EOF newline in schema files. Most of the clean-ups are for list
> > indentation which should always be 2 spaces more than the preceding
>                                      ^
> > keyword.

keyword is the key part...

> []
> > diff --git a/Documentation/devicetree/bindings/arm/arm,integrator.yaml b/Documentation/devicetree/bindings/arm/arm,integrator.yaml
> > index 192ded470e32..f0daf990e077 100644
> > --- a/Documentation/devicetree/bindings/arm/arm,integrator.yaml
> > +++ b/Documentation/devicetree/bindings/arm/arm,integrator.yaml
> > @@ -67,9 +67,9 @@ patternProperties:
> >        compatible:
> >          items:
> >            - enum:
> > -            - arm,integrator-ap-syscon
> > -            - arm,integrator-cp-syscon
> > -            - arm,integrator-sp-syscon
> > +              - arm,integrator-ap-syscon
> > +              - arm,integrator-cp-syscon
> > +              - arm,integrator-sp-syscon
>
> Confused a bit here.
>           - enum:
>         10 spaces to dash
> old line:
>             - arm,integrator-ap-syscon
>         12 spaces to dash
> new line:
>               - arm,integrator-ap-syscon
>         14 spaces to dash
>
> Is it supposed to be 2 spaces more than the preceding line
> or 4 more?

If the preceding line is a list entry (i.e. starts with '-'), then
it's 4 more spaces. It's always 2 more spaces than the preceding
keyword start (aka json-schema vocabulary).

Arguably, this style is a bit inconsistent in that the '-' counts
toward as indentation of the current line, but not the preceding line.
However, I think this style is a bit less error prone and easier to
review. With the other style (always N more spaces) it's harder to
distinguish lists vs. dicts. For example, you can have something like
this:

- key:
  - foo
  - bar

- key:
    foo
    bar

- key:
  - foo
    bar

All 3 of these could be valid. Which one was intended? (Can't really
tell here, but you can with actual DT schema.)

Rob

Re: [PATCH] dt-bindings: Whitespace clean-ups in schema files

[Geert Uytterhoeven]

Hi Rob,

On Wed, Aug 12, 2020 at 10:36 PM Rob Herring <robh@kernel.org> wrote:
> Clean-up incorrect indentation, extra spaces, long lines, and missing
> EOF newline in schema files. Most of the clean-ups are for list
> indentation which should always be 2 spaces more than the preceding
> keyword.
>
> Found with yamllint (which I plan to integrate into the checks).

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

Thanks for your patch!

> --- a/Documentation/devicetree/bindings/clock/renesas,cpg-clocks.yaml
> +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-clocks.yaml
> @@ -24,9 +24,9 @@ properties:
>        - const: renesas,r8a7778-cpg-clocks # R-Car M1
>        - const: renesas,r8a7779-cpg-clocks # R-Car H1
>        - items:
> -        - enum:
> -            - renesas,r7s72100-cpg-clocks # RZ/A1H
> -        - const: renesas,rz-cpg-clocks    # RZ/A1
> +          - enum:
> +              - renesas,r7s72100-cpg-clocks # RZ/A1H
> +          - const: renesas,rz-cpg-clocks    # RZ/A1

This change breaks alignment of the comments at the end of each line.

>        - const: renesas,sh73a0-cpg-clocks  # SH-Mobile AG5

(I only checked the files I care about)

If you don't update commit  e0fe7fc6f2ca0781 ("dt-bindings: Whitespace
clean-ups in schema files"), I can send a patch after v5.9-rc1.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] dt-bindings: Whitespace clean-ups in schema files

[Luca Ceresoli]

Hi Rob,

On 12/08/20 22:36, Rob Herring wrote:
> Clean-up incorrect indentation, extra spaces, long lines, and missing
> EOF newline in schema files. Most of the clean-ups are for list
> indentation which should always be 2 spaces more than the preceding
> keyword.
> 
> Found with yamllint (which I plan to integrate into the checks).

[...]

> diff --git a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
> index 3d4e1685cc55..28c6461b9a9a 100644
> --- a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
> +++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
> @@ -95,10 +95,10 @@ allOf:
>        # Devices without builtin crystal
>        properties:
>          clock-names:
> -            minItems: 1
> -            maxItems: 2
> -            items:
> -              enum: [ xin, clkin ]
> +          minItems: 1
> +          maxItems: 2
> +          items:
> +            enum: [ xin, clkin ]
>          clocks:
>            minItems: 1
>            maxItems: 2

Thanks for noticing, LGTM.

[...]

> diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> index d7dac16a3960..36dc7b56a453 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> +++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> @@ -33,8 +33,8 @@ properties:
>      $ref: /schemas/types.yaml#/definitions/uint32
>  
>    touchscreen-min-pressure:
> -    description: minimum pressure on the touchscreen to be achieved in order for the
> -                 touchscreen driver to report a touch event.
> +    description: minimum pressure on the touchscreen to be achieved in order
> +      for the touchscreen driver to report a touch event.

Out of personal taste, I find the original layout more pleasant and
readable. This third option is also good, especially for long descriptions:

  description:
    minimum pressure on the touchscreen to be achieved in order for the
    touchscreen driver to report a touch event.

At first glance yamllint seems to support exactly these two by default:

> With indentation: {spaces: 4, check-multi-line-strings: true}
> 
> the following code snippet would PASS:
> 
> Blaise Pascal:
>     Je vous écris une longue lettre parce que
>     je n'ai pas le temps d'en écrire une courte.
> 
> the following code snippet would PASS:
> 
> Blaise Pascal: Je vous écris une longue lettre parce que
>                je n'ai pas le temps d'en écrire une courte.
> 
> the following code snippet would FAIL:
> 
> Blaise Pascal: Je vous écris une longue lettre parce que
>   je n'ai pas le temps d'en écrire une courte.
> 
(https://yamllint.readthedocs.io/en/stable/rules.html#module-yamllint.rules.indentation)


-- 
Luca

Re: [PATCH] dt-bindings: Whitespace clean-ups in schema files

[Rob Herring]

On Thu, Aug 13, 2020 at 4:31 AM Luca Ceresoli <luca@lucaceresoli.net> wrote:
>
> Hi Rob,
>
> On 12/08/20 22:36, Rob Herring wrote:
> > Clean-up incorrect indentation, extra spaces, long lines, and missing
> > EOF newline in schema files. Most of the clean-ups are for list
> > indentation which should always be 2 spaces more than the preceding
> > keyword.
> >
> > Found with yamllint (which I plan to integrate into the checks).
>
> [...]
>
> > diff --git a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
> > index 3d4e1685cc55..28c6461b9a9a 100644
> > --- a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
> > +++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
> > @@ -95,10 +95,10 @@ allOf:
> >        # Devices without builtin crystal
> >        properties:
> >          clock-names:
> > -            minItems: 1
> > -            maxItems: 2
> > -            items:
> > -              enum: [ xin, clkin ]
> > +          minItems: 1
> > +          maxItems: 2
> > +          items:
> > +            enum: [ xin, clkin ]
> >          clocks:
> >            minItems: 1
> >            maxItems: 2
>
> Thanks for noticing, LGTM.
>
> [...]
>
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> > index d7dac16a3960..36dc7b56a453 100644
> > --- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> > @@ -33,8 +33,8 @@ properties:
> >      $ref: /schemas/types.yaml#/definitions/uint32
> >
> >    touchscreen-min-pressure:
> > -    description: minimum pressure on the touchscreen to be achieved in order for the
> > -                 touchscreen driver to report a touch event.
> > +    description: minimum pressure on the touchscreen to be achieved in order
> > +      for the touchscreen driver to report a touch event.
>
> Out of personal taste, I find the original layout more pleasant and
> readable. This third option is also good, especially for long descriptions:
>
>   description:
>     minimum pressure on the touchscreen to be achieved in order for the
>     touchscreen driver to report a touch event.
>
> At first glance yamllint seems to support exactly these two by default:
>
> > With indentation: {spaces: 4, check-multi-line-strings: true}

Turning on check-multi-line-strings results in 10K+ warnings, so no.

The other issue is the style ruamel.yaml wants to write out is as the
patch does above. This matters when doing some scripted
transformations where we read in the files and write them back out. I
can somewhat work around that by first doing a pass with no changes
and then another pass with the actual changes, but that's completely
scriptable. Hopefully, ruamel learns to preserve the style better.

Rob

Re: [PATCH] dt-bindings: Whitespace clean-ups in schema files

[Luca Ceresoli]

Hi,

On 14/08/20 16:51, Rob Herring wrote:
> On Thu, Aug 13, 2020 at 4:31 AM Luca Ceresoli <luca@lucaceresoli.net> wrote:
>>
>> Hi Rob,
>>
>> On 12/08/20 22:36, Rob Herring wrote:
>>> Clean-up incorrect indentation, extra spaces, long lines, and missing
>>> EOF newline in schema files. Most of the clean-ups are for list
>>> indentation which should always be 2 spaces more than the preceding
>>> keyword.
>>>
>>> Found with yamllint (which I plan to integrate into the checks).
>>
>> [...]
>>
>>> diff --git a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
>>> index 3d4e1685cc55..28c6461b9a9a 100644
>>> --- a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
>>> +++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
>>> @@ -95,10 +95,10 @@ allOf:
>>>        # Devices without builtin crystal
>>>        properties:
>>>          clock-names:
>>> -            minItems: 1
>>> -            maxItems: 2
>>> -            items:
>>> -              enum: [ xin, clkin ]
>>> +          minItems: 1
>>> +          maxItems: 2
>>> +          items:
>>> +            enum: [ xin, clkin ]
>>>          clocks:
>>>            minItems: 1
>>>            maxItems: 2
>>
>> Thanks for noticing, LGTM.
>>
>> [...]
>>
>>> diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
>>> index d7dac16a3960..36dc7b56a453 100644
>>> --- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
>>> +++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
>>> @@ -33,8 +33,8 @@ properties:
>>>      $ref: /schemas/types.yaml#/definitions/uint32
>>>
>>>    touchscreen-min-pressure:
>>> -    description: minimum pressure on the touchscreen to be achieved in order for the
>>> -                 touchscreen driver to report a touch event.
>>> +    description: minimum pressure on the touchscreen to be achieved in order
>>> +      for the touchscreen driver to report a touch event.
>>
>> Out of personal taste, I find the original layout more pleasant and
>> readable. This third option is also good, especially for long descriptions:
>>
>>   description:
>>     minimum pressure on the touchscreen to be achieved in order for the
>>     touchscreen driver to report a touch event.
>>
>> At first glance yamllint seems to support exactly these two by default:
>>
>>> With indentation: {spaces: 4, check-multi-line-strings: true}
> 
> Turning on check-multi-line-strings results in 10K+ warnings, so no.
> 
> The other issue is the style ruamel.yaml wants to write out is as the
> patch does above. This matters when doing some scripted
> transformations where we read in the files and write them back out. I
> can somewhat work around that by first doing a pass with no changes
> and then another pass with the actual changes, but that's completely
> scriptable. Hopefully, ruamel learns to preserve the style better.

Kind of sad, but I understand the reason as far as my understanding of
the yaml world allows. Thanks for the explanation.

[For idt,versaclock5.yaml, plus an overview of whole patch]
Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>

-- 
Luca

Re: [PATCH] dt-bindings: Whitespace clean-ups in schema files

[Jonathan Cameron]

On Wed, 12 Aug 2020 23:34:53 +0200
Sam Ravnborg <sam@ravnborg.org> wrote:

> Hi Rob.
> 
> On Wed, Aug 12, 2020 at 02:36:18PM -0600, Rob Herring wrote:
> > Clean-up incorrect indentation, extra spaces, long lines, and missing
> > EOF newline in schema files. Most of the clean-ups are for list
> > indentation which should always be 2 spaces more than the preceding
> > keyword.
> > 
> > Found with yamllint (which I plan to integrate into the checks).  
> 
> I have browsed through the patch - and there was only a few things
> that jumped at me.
> 
> With these points considered:
> Acked-by: Sam Ravnborg <sam@ravnborg.org>

Replying here as the patch doesn't seem to have made it to linux-iio
at least. I'm not sure why...

Anyhow, found it in an lkml archive so for the iio changes
Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> 
> I expect only some (few) of my points to actually results in any updates.
> 
> I look forward to have the lint functionality as part of the built-in
> tools so we catch these things early.
> 
> 	Sam
> 
> > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> > index f63895c8ce2d..88814a2a14a5 100644
> > --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> > @@ -273,8 +273,8 @@ properties:
> >                - fsl,imx6ull-14x14-evk     # i.MX6 UltraLiteLite 14x14 EVK Board
> >                - kontron,imx6ull-n6411-som # Kontron N6411 SOM
> >                - myir,imx6ull-mys-6ulx-eval # MYiR Tech iMX6ULL Evaluation Board
> > -              - toradex,colibri-imx6ull-eval            # Colibri iMX6ULL Module on Colibri Evaluation Board
> > -              - toradex,colibri-imx6ull-wifi-eval       # Colibri iMX6ULL Wi-Fi / Bluetooth Module on Colibri Evaluation Board
> > +              - toradex,colibri-imx6ull-eval      # Colibri iMX6ULL Module on Colibri Eval Board
> > +              - toradex,colibri-imx6ull-wifi-eval # Colibri iMX6ULL Wi-Fi / BT Module on Colibri Eval Board
> >            - const: fsl,imx6ull  
> 
> This change looks bad as it drops the alignment with the comments below.
> See following patch chunck:
> 
> >
> >        - description: Kontron N6411 S Board
> > @@ -312,9 +312,12 @@ properties:
> >                - toradex,colibri-imx7d                   # Colibri iMX7 Dual Module
> >                - toradex,colibri-imx7d-aster             # Colibri iMX7 Dual Module on Aster Carrier Board
> >                - toradex,colibri-imx7d-emmc              # Colibri iMX7 Dual 1GB (eMMC) Module
> > -              - toradex,colibri-imx7d-emmc-aster        # Colibri iMX7 Dual 1GB (eMMC) Module on Aster Carrier Board
> > -              - toradex,colibri-imx7d-emmc-eval-v3      # Colibri iMX7 Dual 1GB (eMMC) Module on Colibri Evaluation Board V3
> > -              - toradex,colibri-imx7d-eval-v3           # Colibri iMX7 Dual Module on Colibri Evaluation Board V3
> > +              - toradex,colibri-imx7d-emmc-aster        # Colibri iMX7 Dual 1GB (eMMC) Module on
> > +                                                        #  Aster Carrier Board  
> 
> 
> 
> > diff --git a/Documentation/devicetree/bindings/display/panel/ilitek,ili9322.yaml b/Documentation/devicetree/bindings/display/panel/ilitek,ili9322.yaml
> > index 177d48c5bd97..e89c1ea62ffa 100644
> > --- a/Documentation/devicetree/bindings/display/panel/ilitek,ili9322.yaml
> > +++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9322.yaml
> > @@ -25,8 +25,7 @@ properties:
> >    compatible:
> >      items:
> >        - enum:
> > -        - dlink,dir-685-panel
> > -
> > +          - dlink,dir-685-panel
> >        - const: ilitek,ili9322
> >
> >    reset-gpios: true
> > diff --git a/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml b/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml
> > index a39332276bab..76a9068a85dd 100644
> > --- a/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml
> > +++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml
> > @@ -13,8 +13,7 @@ properties:
> >    compatible:
> >      items:
> >        - enum:
> > -        - bananapi,lhr050h41
> > -
> > +          - bananapi,lhr050h41
> >        - const: ilitek,ili9881c
> >  
> 
> The extra lines is a simple way to indicate that here shall be added
> more in the future. So I like the empty line.
> 
> 
> > diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
> > index 32e0896c6bc1..47938e372987 100644
> > --- a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
> > +++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml
> > @@ -79,7 +79,8 @@ properties:
> >      description: |
> >        kHz; switching frequency.
> >      $ref: /schemas/types.yaml#/definitions/uint32
> > -    enum: [ 600, 640, 685, 738, 800, 872, 960, 1066, 1200, 1371, 1600, 1920, 2400, 3200, 4800, 9600 ]
> > +    enum: [ 600, 640, 685, 738, 800, 872, 960, 1066, 1200, 1371, 1600, 1920,
> > +            2400, 3200, 4800, 9600 ]
> >
> >    qcom,ovp:
> >      description: |  
> 
> In the modern world we are living in now line length of 100 chars are
> OK. checkpatch and coding_style is updated to reflected this.
> 
> > diff --git a/Documentation/devicetree/bindings/spi/mikrotik,rb4xx-spi.yaml b/Documentation/devicetree/bindings/spi/mikrotik,rb4xx-spi.yaml
> > index 4ddb42a4ae05..9102feae90a2 100644
> > --- a/Documentation/devicetree/bindings/spi/mikrotik,rb4xx-spi.yaml
> > +++ b/Documentation/devicetree/bindings/spi/mikrotik,rb4xx-spi.yaml
> > @@ -33,4 +33,5 @@ examples:
> >          reg = <0x1f000000 0x10>;
> >      };
> >
> > -...
> > \ No newline at end of file
> > +...
> > +  
> 
> Added one line too much?
> 
>  diff --git a/Documentation/devicetree/bindings/spi/spi-mux.yaml b/Documentation/devicetree/bindings/spi/spi-mux.yaml
> > index 0ae692dc28b5..3d3fed63409b 100644
> > --- a/Documentation/devicetree/bindings/spi/spi-mux.yaml
> > +++ b/Documentation/devicetree/bindings/spi/spi-mux.yaml
> > @@ -43,47 +43,47 @@ properties:
> >      maxItems: 1
> >
> >  required:
> > -   - compatible
> > -   - reg
> > -   - spi-max-frequency
> > -   - mux-controls
> > +  - compatible
> > +  - reg
> > +  - spi-max-frequency
> > +  - mux-controls
> >
> >  examples:
> > -   - |
> > -     #include <dt-bindings/gpio/gpio.h>
> > -     mux: mux-controller {
> > -       compatible = "gpio-mux";
> > -       #mux-control-cells = <0>;
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +    mux: mux-controller {
> > +        compatible = "gpio-mux";
> > +        #mux-control-cells = <0>;
> >
> > -       mux-gpios = <&gpio0 3 GPIO_ACTIVE_HIGH>;
> > -     };
> > +        mux-gpios = <&gpio0 3 GPIO_ACTIVE_HIGH>;
> > +    };  
> 
> Example is updated to use 4-space indent. I like.
> 
> But many other examples are left untouched.
> 
> So I wonder if updating all examples to the same indent should
> be left for another mega-patch?
> 
> > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > index f3d847832fdc..2baee2c817c1 100644
> > --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > @@ -993,7 +993,8 @@ patternProperties:
> >    "^sst,.*":
> >      description: Silicon Storage Technology, Inc.
> >    "^sstar,.*":
> > -    description: Xiamen Xingchen(SigmaStar) Technology Co., Ltd. (formerly part of MStar Semiconductor, Inc.)
> > +    description: Xiamen Xingchen(SigmaStar) Technology Co., Ltd.
> > +      (formerly part of MStar Semiconductor, Inc.)
> >    "^st,.*":
> >      description: STMicroelectronics
> >    "^starry,.*":  
> 
> Did you check that they are all in alphabetical order?
> I would be suprised if this is the only issue in this file.
> 
> 

Re: [PATCH] dt-bindings: Whitespace clean-ups in schema files

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1222 bytes --]

On Wed, Aug 12, 2020 at 02:36:18PM -0600, Rob Herring wrote:
> Clean-up incorrect indentation, extra spaces, long lines, and missing
> EOF newline in schema files. Most of the clean-ups are for list
> indentation which should always be 2 spaces more than the preceding
> keyword.
> 
> Found with yamllint (which I plan to integrate into the checks).
> 
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-clk@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-spi@vger.kernel.org
> Cc: linux-gpio@vger.kernel.org
> Cc: linux-remoteproc@vger.kernel.org
> Cc: linux-hwmon@vger.kernel.org
> Cc: linux-i2c@vger.kernel.org
> Cc: linux-fbdev@vger.kernel.org
> Cc: linux-iio@vger.kernel.org
> Cc: linux-input@vger.kernel.org
> Cc: linux-pm@vger.kernel.org
> Cc: linux-media@vger.kernel.org
> Cc: alsa-devel@alsa-project.org
> Cc: linux-mmc@vger.kernel.org
> Cc: linux-mtd@lists.infradead.org
> Cc: netdev@vger.kernel.org
> Cc: linux-rtc@vger.kernel.org
> Cc: linux-serial@vger.kernel.org
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: Rob Herring <robh@kernel.org>

I trust you guys in figuring out the details, so for touching I2C:

Acked-by: Wolfram Sang <wsa@kernel.org>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]