[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 <ddrokosov@salutedevices.com>
---
 .../bindings/leds/awinic,aw200xx.yaml         | 64 +++++++++++++++++--
 1 file changed, 58 insertions(+), 6 deletions(-)
  

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 <ddrokosov@salutedevices.com>

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 <conor.dooley@microchip.com>

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
>
  

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 <ddrokosov@salutedevices.com>
> ---
>  .../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/20231101142445.8753-12-ddrokosov@salutedevices.com

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.
  

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 <ddrokosov@salutedevices.com>
> > ---
> >  .../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.
  

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 <ddrokosov@salutedevices.com>
> > > ---
> > >  .../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.
  

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 <ddrokosov@salutedevices.com>
> 
> 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 <conor.dooley@microchip.com>
> 

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

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 <ddrokosov@salutedevices.com>
> > > > ---
> > > >  .../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.