| Message ID | 20230609140709.64655-1-krzysztof.kozlowski@linaro.org |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | dt-bindings: pwm: drop unneeded quotes | expand |
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 >
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
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
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#
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!
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:
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(-)