[PATCH] dt-bindings: pwm: drop unneeded quotes

[PATCH] dt-bindings: pwm: drop unneeded quotes

[Krzysztof Kozlowski]

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

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 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 <claudiu.beznea@microchip.com>
 
 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

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

[Claudiu.Beznea]

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 <krzysztof.kozlowski@linaro.org>
Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>

> ---
>  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 <claudiu.beznea@microchip.com>
> 
>  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
> 

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

[Uwe Kleine-König]

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

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/ |

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

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

[Rob Herring]

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

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

[Rob Herring]

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#

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

[Rob Herring]

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 <krzysztof.kozlowski@linaro.org>
> ---
>  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!