2023-11-01 14:25:59

by Dmitry Rokosov

[permalink] [raw]
Subject: [PATCH v3 11/11] dt-bindings: leds: aw200xx: fix led pattern and add reg constraints

AW200XX controllers have the capability to declare more than 0xf LEDs,
therefore, it is necessary to accept LED names using an appropriate
regex pattern.

The register offsets can be adjusted within the specified range, with
the maximum value corresponding to the highest number of LEDs that can
be connected to the controller.

Fixes: e338a05e76ca ("dt-bindings: leds: Add binding for AW200xx")
Signed-off-by: Dmitry Rokosov <[email protected]>
---
.../bindings/leds/awinic,aw200xx.yaml | 64 +++++++++++++++++--
1 file changed, 58 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
index 67c1d960db1d..ba4511664fb8 100644
--- a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
+++ b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
@@ -45,17 +45,12 @@ properties:
maxItems: 1

patternProperties:
- "^led@[0-9a-f]$":
+ "^led@[0-9a-f]+$":
type: object
$ref: common.yaml#
unevaluatedProperties: false

properties:
- reg:
- description:
- LED number
- maxItems: 1
-
led-max-microamp:
default: 9780
description: |
@@ -69,6 +64,63 @@ patternProperties:
where max-current-switch-number is determinated by led configuration
and depends on how leds are physically connected to the led driver.

+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: awinic,aw20036
+ then:
+ patternProperties:
+ "^led@[0-9a-f]+$":
+ properties:
+ reg:
+ items:
+ minimum: 0
+ maximum: 36
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: awinic,aw20054
+ then:
+ patternProperties:
+ "^led@[0-9a-f]+$":
+ properties:
+ reg:
+ items:
+ minimum: 0
+ maximum: 54
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: awinic,aw20072
+ then:
+ patternProperties:
+ "^led@[0-9a-f]+$":
+ properties:
+ reg:
+ items:
+ minimum: 0
+ maximum: 72
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: awinic,aw20108
+ then:
+ patternProperties:
+ "^led@[0-9a-f]+$":
+ properties:
+ reg:
+ items:
+ minimum: 0
+ maximum: 108
+
required:
- compatible
- reg
--
2.36.0


2023-11-01 15:32:04

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3 11/11] dt-bindings: leds: aw200xx: fix led pattern and add reg constraints

On Wed, Nov 01, 2023 at 05:24:45PM +0300, Dmitry Rokosov wrote:
> AW200XX controllers have the capability to declare more than 0xf LEDs,
> therefore, it is necessary to accept LED names using an appropriate
> regex pattern.
>
> The register offsets can be adjusted within the specified range, with
> the maximum value corresponding to the highest number of LEDs that can
> be connected to the controller.
>
> Fixes: e338a05e76ca ("dt-bindings: leds: Add binding for AW200xx")
> Signed-off-by: Dmitry Rokosov <[email protected]>

You did correctly guess what I was getting at on the previous version.
Apologies for not replying - I got sick and things probably fell a bit
through the cracks.

Reviewed-by: Conor Dooley <[email protected]>

Cheers,
Conor.

> ---
> .../bindings/leds/awinic,aw200xx.yaml | 64 +++++++++++++++++--
> 1 file changed, 58 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> index 67c1d960db1d..ba4511664fb8 100644
> --- a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> +++ b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> @@ -45,17 +45,12 @@ properties:
> maxItems: 1
>
> patternProperties:
> - "^led@[0-9a-f]$":
> + "^led@[0-9a-f]+$":
> type: object
> $ref: common.yaml#
> unevaluatedProperties: false
>
> properties:
> - reg:
> - description:
> - LED number
> - maxItems: 1
> -
> led-max-microamp:
> default: 9780
> description: |
> @@ -69,6 +64,63 @@ patternProperties:
> where max-current-switch-number is determinated by led configuration
> and depends on how leds are physically connected to the led driver.
>
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: awinic,aw20036
> + then:
> + patternProperties:
> + "^led@[0-9a-f]+$":
> + properties:
> + reg:
> + items:
> + minimum: 0
> + maximum: 36
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: awinic,aw20054
> + then:
> + patternProperties:
> + "^led@[0-9a-f]+$":
> + properties:
> + reg:
> + items:
> + minimum: 0
> + maximum: 54
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: awinic,aw20072
> + then:
> + patternProperties:
> + "^led@[0-9a-f]+$":
> + properties:
> + reg:
> + items:
> + minimum: 0
> + maximum: 72
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: awinic,aw20108
> + then:
> + patternProperties:
> + "^led@[0-9a-f]+$":
> + properties:
> + reg:
> + items:
> + minimum: 0
> + maximum: 108
> +
> required:
> - compatible
> - reg
> --
> 2.36.0
>


Attachments:
(No filename) (3.22 kB)
signature.asc (235.00 B)
Download all attachments

2023-11-01 16:04:52

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 11/11] dt-bindings: leds: aw200xx: fix led pattern and add reg constraints


On Wed, 01 Nov 2023 17:24:45 +0300, Dmitry Rokosov wrote:
> AW200XX controllers have the capability to declare more than 0xf LEDs,
> therefore, it is necessary to accept LED names using an appropriate
> regex pattern.
>
> The register offsets can be adjusted within the specified range, with
> the maximum value corresponding to the highest number of LEDs that can
> be connected to the controller.
>
> Fixes: e338a05e76ca ("dt-bindings: leds: Add binding for AW200xx")
> Signed-off-by: Dmitry Rokosov <[email protected]>
> ---
> .../bindings/leds/awinic,aw200xx.yaml | 64 +++++++++++++++++--
> 1 file changed, 58 insertions(+), 6 deletions(-)
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/leds/awinic,aw200xx.example.dtb: led-controller@3a: led@0: Unevaluated properties are not allowed ('reg' was unexpected)
from schema $id: http://devicetree.org/schemas/leds/awinic,aw200xx.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/leds/awinic,aw200xx.example.dtb: led-controller@3a: led@1: Unevaluated properties are not allowed ('reg' was unexpected)
from schema $id: http://devicetree.org/schemas/leds/awinic,aw200xx.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/leds/awinic,aw200xx.example.dtb: led-controller@3a: led@2: Unevaluated properties are not allowed ('reg' was unexpected)
from schema $id: http://devicetree.org/schemas/leds/awinic,aw200xx.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

2023-11-01 16:18:17

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3 11/11] dt-bindings: leds: aw200xx: fix led pattern and add reg constraints

On Wed, Nov 01, 2023 at 11:04:16AM -0500, Rob Herring wrote:
>
> On Wed, 01 Nov 2023 17:24:45 +0300, Dmitry Rokosov wrote:
> > AW200XX controllers have the capability to declare more than 0xf LEDs,
> > therefore, it is necessary to accept LED names using an appropriate
> > regex pattern.
> >
> > The register offsets can be adjusted within the specified range, with
> > the maximum value corresponding to the highest number of LEDs that can
> > be connected to the controller.
> >
> > Fixes: e338a05e76ca ("dt-bindings: leds: Add binding for AW200xx")
> > Signed-off-by: Dmitry Rokosov <[email protected]>
> > ---
> > .../bindings/leds/awinic,aw200xx.yaml | 64 +++++++++++++++++--
> > 1 file changed, 58 insertions(+), 6 deletions(-)
> >
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/leds/awinic,aw200xx.example.dtb: led-controller@3a: led@0: Unevaluated properties are not allowed ('reg' was unexpected)
> from schema $id: http://devicetree.org/schemas/leds/awinic,aw200xx.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/leds/awinic,aw200xx.example.dtb: led-controller@3a: led@1: Unevaluated properties are not allowed ('reg' was unexpected)
> from schema $id: http://devicetree.org/schemas/leds/awinic,aw200xx.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/leds/awinic,aw200xx.example.dtb: led-controller@3a: led@2: Unevaluated properties are not allowed ('reg' was unexpected)
> from schema $id: http://devicetree.org/schemas/leds/awinic,aw200xx.yaml#

Looks like you need to drop the second part of this hunk from the patch.
@@ -45,17 +45,12 @@ properties:
maxItems: 1

patternProperties:
- "^led@[0-9a-f]$":
+ "^led@[0-9a-f]+$":
type: object
$ref: common.yaml#
unevaluatedProperties: false

properties:
- reg:
- description:
- LED number
- maxItems: 1
-
led-max-microamp:
default: 9780
description: |

Each LED still only has one reg entry, right?

Cheers,
Conor.


Attachments:
(No filename) (2.27 kB)
signature.asc (235.00 B)
Download all attachments

2023-11-01 17:46:22

by Dmitry Rokosov

[permalink] [raw]
Subject: Re: [PATCH v3 11/11] dt-bindings: leds: aw200xx: fix led pattern and add reg constraints

Hello Conor,

On Wed, Nov 01, 2023 at 04:17:14PM +0000, Conor Dooley wrote:
> On Wed, Nov 01, 2023 at 11:04:16AM -0500, Rob Herring wrote:
> >
> > On Wed, 01 Nov 2023 17:24:45 +0300, Dmitry Rokosov wrote:
> > > AW200XX controllers have the capability to declare more than 0xf LEDs,
> > > therefore, it is necessary to accept LED names using an appropriate
> > > regex pattern.
> > >
> > > The register offsets can be adjusted within the specified range, with
> > > the maximum value corresponding to the highest number of LEDs that can
> > > be connected to the controller.
> > >
> > > Fixes: e338a05e76ca ("dt-bindings: leds: Add binding for AW200xx")
> > > Signed-off-by: Dmitry Rokosov <[email protected]>
> > > ---
> > > .../bindings/leds/awinic,aw200xx.yaml | 64 +++++++++++++++++--
> > > 1 file changed, 58 insertions(+), 6 deletions(-)
> > >
> >
> > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> > on your patch (DT_CHECKER_FLAGS is new in v5.13):
> >
> > yamllint warnings/errors:
> >
> > dtschema/dtc warnings/errors:
> > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/leds/awinic,aw200xx.example.dtb: led-controller@3a: led@0: Unevaluated properties are not allowed ('reg' was unexpected)
> > from schema $id: http://devicetree.org/schemas/leds/awinic,aw200xx.yaml#
> > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/leds/awinic,aw200xx.example.dtb: led-controller@3a: led@1: Unevaluated properties are not allowed ('reg' was unexpected)
> > from schema $id: http://devicetree.org/schemas/leds/awinic,aw200xx.yaml#
> > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/leds/awinic,aw200xx.example.dtb: led-controller@3a: led@2: Unevaluated properties are not allowed ('reg' was unexpected)
> > from schema $id: http://devicetree.org/schemas/leds/awinic,aw200xx.yaml#
>
> Looks like you need to drop the second part of this hunk from the patch.
> @@ -45,17 +45,12 @@ properties:
> maxItems: 1
>
> patternProperties:
> - "^led@[0-9a-f]$":
> + "^led@[0-9a-f]+$":
> type: object
> $ref: common.yaml#
> unevaluatedProperties: false
>
> properties:
> - reg:
> - description:
> - LED number
> - maxItems: 1
> -
> led-max-microamp:
> default: 9780
> description: |
>
> Each LED still only has one reg entry, right?

You're right... the maxItems for 'reg' is still needed. I'll back it in
the next version.
But I don't understand, why my dt_binding_check run doesn't show me this
problem... I don't specify DT_CHECKER_FLAGS, maybe this is a root cause.

--
Thank you,
Dmitry

2023-11-01 17:49:16

by Dmitry Rokosov

[permalink] [raw]
Subject: Re: [PATCH v3 11/11] dt-bindings: leds: aw200xx: fix led pattern and add reg constraints

Conor,

On Wed, Nov 01, 2023 at 03:31:28PM +0000, Conor Dooley wrote:
> On Wed, Nov 01, 2023 at 05:24:45PM +0300, Dmitry Rokosov wrote:
> > AW200XX controllers have the capability to declare more than 0xf LEDs,
> > therefore, it is necessary to accept LED names using an appropriate
> > regex pattern.
> >
> > The register offsets can be adjusted within the specified range, with
> > the maximum value corresponding to the highest number of LEDs that can
> > be connected to the controller.
> >
> > Fixes: e338a05e76ca ("dt-bindings: leds: Add binding for AW200xx")
> > Signed-off-by: Dmitry Rokosov <[email protected]>
>
> You did correctly guess what I was getting at on the previous version.
> Apologies for not replying - I got sick and things probably fell a bit
> through the cracks.

Don't worry! Take care and get well soon!

>
> Reviewed-by: Conor Dooley <[email protected]>
>

Should I include this tag in the next version with a fix for the 'reg'
maxItems, or would you review this patch again?

> Cheers,
> Conor.
>
> > ---
> > .../bindings/leds/awinic,aw200xx.yaml | 64 +++++++++++++++++--
> > 1 file changed, 58 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> > index 67c1d960db1d..ba4511664fb8 100644
> > --- a/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> > +++ b/Documentation/devicetree/bindings/leds/awinic,aw200xx.yaml
> > @@ -45,17 +45,12 @@ properties:
> > maxItems: 1
> >
> > patternProperties:
> > - "^led@[0-9a-f]$":
> > + "^led@[0-9a-f]+$":
> > type: object
> > $ref: common.yaml#
> > unevaluatedProperties: false
> >
> > properties:
> > - reg:
> > - description:
> > - LED number
> > - maxItems: 1
> > -
> > led-max-microamp:
> > default: 9780
> > description: |
> > @@ -69,6 +64,63 @@ patternProperties:
> > where max-current-switch-number is determinated by led configuration
> > and depends on how leds are physically connected to the led driver.
> >
> > +allOf:
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: awinic,aw20036
> > + then:
> > + patternProperties:
> > + "^led@[0-9a-f]+$":
> > + properties:
> > + reg:
> > + items:
> > + minimum: 0
> > + maximum: 36
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: awinic,aw20054
> > + then:
> > + patternProperties:
> > + "^led@[0-9a-f]+$":
> > + properties:
> > + reg:
> > + items:
> > + minimum: 0
> > + maximum: 54
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: awinic,aw20072
> > + then:
> > + patternProperties:
> > + "^led@[0-9a-f]+$":
> > + properties:
> > + reg:
> > + items:
> > + minimum: 0
> > + maximum: 72
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: awinic,aw20108
> > + then:
> > + patternProperties:
> > + "^led@[0-9a-f]+$":
> > + properties:
> > + reg:
> > + items:
> > + minimum: 0
> > + maximum: 108
> > +
> > required:
> > - compatible
> > - reg
> > --
> > 2.36.0
> >



--
Thank you,
Dmitry

2023-11-02 00:17:47

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3 11/11] dt-bindings: leds: aw200xx: fix led pattern and add reg constraints

On Wed, Nov 01, 2023 at 08:44:22PM +0300, Dmitry Rokosov wrote:
> Hello Conor,
>
> On Wed, Nov 01, 2023 at 04:17:14PM +0000, Conor Dooley wrote:
> > On Wed, Nov 01, 2023 at 11:04:16AM -0500, Rob Herring wrote:
> > >
> > > On Wed, 01 Nov 2023 17:24:45 +0300, Dmitry Rokosov wrote:
> > > > AW200XX controllers have the capability to declare more than 0xf LEDs,
> > > > therefore, it is necessary to accept LED names using an appropriate
> > > > regex pattern.
> > > >
> > > > The register offsets can be adjusted within the specified range, with
> > > > the maximum value corresponding to the highest number of LEDs that can
> > > > be connected to the controller.
> > > >
> > > > Fixes: e338a05e76ca ("dt-bindings: leds: Add binding for AW200xx")
> > > > Signed-off-by: Dmitry Rokosov <[email protected]>
> > > > ---
> > > > .../bindings/leds/awinic,aw200xx.yaml | 64 +++++++++++++++++--
> > > > 1 file changed, 58 insertions(+), 6 deletions(-)
> > > >
> > >
> > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> > > on your patch (DT_CHECKER_FLAGS is new in v5.13):
> > >
> > > yamllint warnings/errors:
> > >
> > > dtschema/dtc warnings/errors:
> > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/leds/awinic,aw200xx.example.dtb: led-controller@3a: led@0: Unevaluated properties are not allowed ('reg' was unexpected)
> > > from schema $id: http://devicetree.org/schemas/leds/awinic,aw200xx.yaml#
> > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/leds/awinic,aw200xx.example.dtb: led-controller@3a: led@1: Unevaluated properties are not allowed ('reg' was unexpected)
> > > from schema $id: http://devicetree.org/schemas/leds/awinic,aw200xx.yaml#
> > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/leds/awinic,aw200xx.example.dtb: led-controller@3a: led@2: Unevaluated properties are not allowed ('reg' was unexpected)
> > > from schema $id: http://devicetree.org/schemas/leds/awinic,aw200xx.yaml#
> >
> > Looks like you need to drop the second part of this hunk from the patch.
> > @@ -45,17 +45,12 @@ properties:
> > maxItems: 1
> >
> > patternProperties:
> > - "^led@[0-9a-f]$":
> > + "^led@[0-9a-f]+$":
> > type: object
> > $ref: common.yaml#
> > unevaluatedProperties: false
> >
> > properties:
> > - reg:
> > - description:
> > - LED number
> > - maxItems: 1
> > -
> > led-max-microamp:
> > default: 9780
> > description: |
> >
> > Each LED still only has one reg entry, right?
>
> You're right... the maxItems for 'reg' is still needed. I'll back it in
> the next version.
> But I don't understand, why my dt_binding_check run doesn't show me this
> problem... I don't specify DT_CHECKER_FLAGS, maybe this is a root cause.

I dunno! I do `make dt_binding_check W=1 DT_SCHEMA_FILES="$filename"` to
test stuff.

Also, you can keep the tag.


Attachments:
(No filename) (2.97 kB)
signature.asc (235.00 B)
Download all attachments