2023-06-09 14:09:21

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH] dt-bindings: pwm: drop unneeded quotes

Cleanup bindings dropping unneeded quotes. Once all these are fixed,
checking for this can be enabled in yamllint.

Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
Documentation/devicetree/bindings/pwm/atmel,at91sam-pwm.yaml | 2 +-
Documentation/devicetree/bindings/pwm/mxs-pwm.yaml | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/atmel,at91sam-pwm.yaml b/Documentation/devicetree/bindings/pwm/atmel,at91sam-pwm.yaml
index ab45df80345d..d84268b59784 100644
--- a/Documentation/devicetree/bindings/pwm/atmel,at91sam-pwm.yaml
+++ b/Documentation/devicetree/bindings/pwm/atmel,at91sam-pwm.yaml
@@ -11,7 +11,7 @@ maintainers:
- Claudiu Beznea <[email protected]>

allOf:
- - $ref: "pwm.yaml#"
+ - $ref: pwm.yaml#

properties:
compatible:
diff --git a/Documentation/devicetree/bindings/pwm/mxs-pwm.yaml b/Documentation/devicetree/bindings/pwm/mxs-pwm.yaml
index a34cbc13f691..6ffbed204c25 100644
--- a/Documentation/devicetree/bindings/pwm/mxs-pwm.yaml
+++ b/Documentation/devicetree/bindings/pwm/mxs-pwm.yaml
@@ -25,7 +25,7 @@ properties:
const: 3

fsl,pwm-number:
- $ref: '/schemas/types.yaml#/definitions/uint32'
+ $ref: /schemas/types.yaml#/definitions/uint32
description: u32 value representing the number of PWM devices

required:
--
2.34.1



2023-06-12 08:32:26

by Claudiu Beznea

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: pwm: drop unneeded quotes

On 09.06.2023 17:07, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Cleanup bindings dropping unneeded quotes. Once all these are fixed,
> checking for this can be enabled in yamllint.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
Reviewed-by: Claudiu Beznea <[email protected]>

> ---
> Documentation/devicetree/bindings/pwm/atmel,at91sam-pwm.yaml | 2 +-
> Documentation/devicetree/bindings/pwm/mxs-pwm.yaml | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pwm/atmel,at91sam-pwm.yaml b/Documentation/devicetree/bindings/pwm/atmel,at91sam-pwm.yaml
> index ab45df80345d..d84268b59784 100644
> --- a/Documentation/devicetree/bindings/pwm/atmel,at91sam-pwm.yaml
> +++ b/Documentation/devicetree/bindings/pwm/atmel,at91sam-pwm.yaml
> @@ -11,7 +11,7 @@ maintainers:
> - Claudiu Beznea <[email protected]>
>
> allOf:
> - - $ref: "pwm.yaml#"
> + - $ref: pwm.yaml#
>
> properties:
> compatible:
> diff --git a/Documentation/devicetree/bindings/pwm/mxs-pwm.yaml b/Documentation/devicetree/bindings/pwm/mxs-pwm.yaml
> index a34cbc13f691..6ffbed204c25 100644
> --- a/Documentation/devicetree/bindings/pwm/mxs-pwm.yaml
> +++ b/Documentation/devicetree/bindings/pwm/mxs-pwm.yaml
> @@ -25,7 +25,7 @@ properties:
> const: 3
>
> fsl,pwm-number:
> - $ref: '/schemas/types.yaml#/definitions/uint32'
> + $ref: /schemas/types.yaml#/definitions/uint32
> description: u32 value representing the number of PWM devices
>
> required:
> --
> 2.34.1
>

2023-06-12 09:56:46

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: pwm: drop unneeded quotes

Hello,

On Fri, Jun 09, 2023 at 04:07:09PM +0200, Krzysztof Kozlowski wrote:
> Cleanup bindings dropping unneeded quotes. Once all these are fixed,
> checking for this can be enabled in yamllint.

in my book quoting everything instead of dropping quotes is the better
option. While that policy adds more quotes, it prevents surprises like:

$ yaml2json << EOF
> countrycodes:
> - de
> - fr
> - no
> - pl
> EOF
{
"countrycodes": [
"de",
"fr",
false,
"pl"
]
}

And if you use the "only-when-needed" rule of yamllint you have to write
the above list as:

countrycodes:
- de
- fr
- "no"
- pl

which is IMHO really ugly.

Another culprit is "on" (which is used e.g. in github action workflows),
so yamllint tells for example for
https://github.com/pengutronix/microcom/blob/main/.github/workflows/build.yml:

3:1 warning truthy value should be one of [false, true] (truthy)

and there are still more surprises (e.g. version numbers might be
subject to conversion to float). So at least in my bubble the general
hint is to *always* quote strings. Note that required: true is also the
default for yamllint's quoted-strings setting, proably for pitfalls like
these.

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |


Attachments:
(No filename) (1.41 kB)
signature.asc (499.00 B)
Download all attachments

2023-06-21 20:55:47

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: pwm: drop unneeded quotes

On Mon, Jun 12, 2023 at 11:33:15AM +0200, Uwe Kleine-K?nig wrote:
> Hello,
>
> On Fri, Jun 09, 2023 at 04:07:09PM +0200, Krzysztof Kozlowski wrote:
> > Cleanup bindings dropping unneeded quotes. Once all these are fixed,
> > checking for this can be enabled in yamllint.
>
> in my book quoting everything instead of dropping quotes is the better
> option. While that policy adds more quotes, it prevents surprises like:
>
> $ yaml2json << EOF
> > countrycodes:
> > - de
> > - fr
> > - no
> > - pl
> > EOF
> {
> "countrycodes": [
> "de",
> "fr",
> false,
> "pl"
> ]
> }
>
> And if you use the "only-when-needed" rule of yamllint you have to write
> the above list as:
>
> countrycodes:
> - de
> - fr
> - "no"
> - pl
>
> which is IMHO really ugly.

Agreed, but "no" and "yes" are unlikely values in DT.

>
> Another culprit is "on" (which is used e.g. in github action workflows),
> so yamllint tells for example for
> https://github.com/pengutronix/microcom/blob/main/.github/workflows/build.yml:
>
> 3:1 warning truthy value should be one of [false, true] (truthy)
>
> and there are still more surprises (e.g. version numbers might be
> subject to conversion to float).

I'll add a meta-schema check for this. 'const' is already limited to
string or integer. That's missing from 'enum'. I think we can also check
that all items are the same type as well.

> So at least in my bubble the general
> hint is to *always* quote strings. Note that required: true is also the
> default for yamllint's quoted-strings setting, proably for pitfalls like
> these.

We're so far gone the other direction from quoting everything, that's
not going to happen. Plus, if I liked everything quoted, I would have
used JSON.

My preference here is I don't want to care about this in reviews. I want
yamllint to check it and not have to think about it again.

Rob

2023-06-21 22:20:19

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: pwm: drop unneeded quotes

On Wed, Jun 21, 2023 at 02:53:17PM -0600, Rob Herring wrote:
> On Mon, Jun 12, 2023 at 11:33:15AM +0200, Uwe Kleine-K?nig wrote:
> > Hello,
> >
> > On Fri, Jun 09, 2023 at 04:07:09PM +0200, Krzysztof Kozlowski wrote:
> > > Cleanup bindings dropping unneeded quotes. Once all these are fixed,
> > > checking for this can be enabled in yamllint.
> >
> > in my book quoting everything instead of dropping quotes is the better
> > option. While that policy adds more quotes, it prevents surprises like:
> >
> > $ yaml2json << EOF
> > > countrycodes:
> > > - de
> > > - fr
> > > - no
> > > - pl
> > > EOF
> > {
> > "countrycodes": [
> > "de",
> > "fr",
> > false,
> > "pl"
> > ]
> > }
> >
> > And if you use the "only-when-needed" rule of yamllint you have to write
> > the above list as:
> >
> > countrycodes:
> > - de
> > - fr
> > - "no"
> > - pl
> >
> > which is IMHO really ugly.
>
> Agreed, but "no" and "yes" are unlikely values in DT.
>
> >
> > Another culprit is "on" (which is used e.g. in github action workflows),
> > so yamllint tells for example for
> > https://github.com/pengutronix/microcom/blob/main/.github/workflows/build.yml:
> >
> > 3:1 warning truthy value should be one of [false, true] (truthy)
> >
> > and there are still more surprises (e.g. version numbers might be
> > subject to conversion to float).
>
> I'll add a meta-schema check for this. 'const' is already limited to
> string or integer. That's missing from 'enum'. I think we can also check
> that all items are the same type as well.

And of course, like every meta-schema addition, we find new errors in
schemas:

/home/rob/proj/linux-dt/Documentation/devicetree/bindings/phy/brcm,brcmstb-usb-phy.yaml: allOf:0:if:properties:compatible:contains:enum: 'oneOf' conditional failed, one must be fixed:
{'const': 'brcm,bcm4908-usb-phy'} is not of type 'integer'
{'const': 'brcm,bcm4908-usb-phy'} is not of type 'string'
{'const': 'brcm,brcmstb-usb-phy'} is not of type 'integer'
{'const': 'brcm,brcmstb-usb-phy'} is not of type 'string'
hint: "enum" must be an array with the same type for all items
from schema $id: http://devicetree.org/meta-schemas/core.yaml#
/home/rob/proj/linux-dt/Documentation/devicetree/bindings/sound/microchip,sama7g5-pdmc.yaml: properties:microchip,mic-pos:items: 'oneOf' conditional failed, one must be fixed:
{'items': [{'description': 'value for DS line'}, {'description': 'value for sampling edge'}], 'anyOf': [{'enum': [[0, 0], [0, 1], [1, 0], [1, 1]]}]} is not of type 'array'
/home/rob/proj/linux-dt/Documentation/devicetree/bindings/sound/microchip,sama7g5-pdmc.yaml: properties:microchip,mic-pos:items:anyOf:0:enum: 'oneOf' conditional failed, one must be fixed:
[0, 0] is not of type 'integer'
[0, 0] is not of type 'string'
[0, 1] is not of type 'integer'
[0, 1] is not of type 'string'
[1, 0] is not of type 'integer'
[1, 0] is not of type 'string'
[1, 1] is not of type 'integer'
[1, 1] is not of type 'string'
hint: "enum" must be an array with the same type for all items
from schema $id: http://devicetree.org/meta-schemas/core.yaml#
from schema $id: http://devicetree.org/meta-schemas/core.yaml#
/home/rob/proj/linux-dt/Documentation/devicetree/bindings/net/altr,tse.yaml: allOf:1:if:properties:compatible:contains:enum: 'oneOf' conditional failed, one must be fixed:
{'const': 'altr,tse-1.0'} is not of type 'integer'
{'const': 'altr,tse-1.0'} is not of type 'string'
{'const': 'ALTR,tse-1.0'} is not of type 'integer'
{'const': 'ALTR,tse-1.0'} is not of type 'string'
hint: "enum" must be an array with the same type for all items
from schema $id: http://devicetree.org/meta-schemas/core.yaml#


2023-06-23 20:44:51

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: pwm: drop unneeded quotes


On Fri, 09 Jun 2023 16:07:09 +0200, Krzysztof Kozlowski wrote:
> Cleanup bindings dropping unneeded quotes. Once all these are fixed,
> checking for this can be enabled in yamllint.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
> Documentation/devicetree/bindings/pwm/atmel,at91sam-pwm.yaml | 2 +-
> Documentation/devicetree/bindings/pwm/mxs-pwm.yaml | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>

Applied, thanks!