[v2,1/2] dt-bindings: leds: add cznic,turris-omnia-leds binding

Add device-tree bindings documentation for Turris Omnia RGB LEDs.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
---
 .../leds/cznic,turris-omnia-leds.yaml         | 113 ++++++++++++++++++
 1 file changed, 113 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml

On Thu, Apr 23, 2020 at 08:50:59AM +0200, Marek Behún wrote:
> Add device-tree bindings documentation for Turris Omnia RGB LEDs.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> ---
>  .../leds/cznic,turris-omnia-leds.yaml         | 113 ++++++++++++++++++
>  1 file changed, 113 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml
> 
> diff --git a/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml b/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml
> new file mode 100644
> index 000000000000..f3d13fa65df2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml
> @@ -0,0 +1,113 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/cznic,turris-omnia-leds.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: CZ.NIC's Turris Omnia LEDs driver
> +
> +maintainers:
> +  - Marek Behún <marek.behun@nic.cz>
> +
> +description:
> +  This module adds support for the RGB LEDs found on the fron panel of the
> +  Turris Omnia router. There are 12 RGB LEDs, they are controlled by device's
> +  microcontroller with which the system communicates via I2C. Each LED is
> +  described as a subnode of this I2C device.
> +
> +properties:
> +  compatible:
> +    const: cznic,turris-omnia-leds
> +
> +  reg:
> +    description: I2C slave address of the microcontroller.
> +    maxItems: 1
> +
> +patternProperties:
> +  "^led[0-9][0-9]?$":

'led-' is preferred. And numbering is usually hex.

What's the numbering here based on? Might be better to use 'reg' (and a 
unit-address) rather than led-sources. led-sources was for cases where 
the control interface (defined by reg) could have differing regulators 
connected to the LEDs.

> +    type: object
> +    allOf:
> +      - $ref: /schemas/leds/common.yaml#
> +    description:
> +      This node can either represent one channel of a RGB LED or a whole RGB
> +      LED.
> +
> +    properties:
> +      led-sources:
> +        description:
> +          List of device current outputs the LED connects to. If one number is
> +          given, the device described by this node will control one channel of a
> +          RGB LED. If three numbers are given (one for each channel of a RGB
> +          LED), the device described by this node will control one RGB LED. In
> +          this case the numbers must increase by one and the first must be a
> +          multiple of 3.
> +        allOf:
> +          - $ref: /schemas/leds/common.yaml#/properties/led-sources

No need for this. It is already done with the previous $ref.

> +          - oneOf:
> +              - items:
> +                  minimum: 0
> +                  maximum: 35
> +                minItems: 1
> +                maxItems: 1
> +              - items:
> +                  enum: [ [ 0, 1, 2 ], [ 3, 4, 5 ], [ 6, 7, 8 ], [ 9, 10, 11 ],
> +                          [ 12, 13, 14 ], [ 15, 16, 17 ], [ 18, 19, 20 ],
> +                          [ 21, 22, 23 ], [ 24, 25, 26 ], [ 27, 28, 29 ],
> +                          [ 30, 31, 32 ], [ 33, 34, 35] ]
> +
> +      color:
> +        description:
> +          Should be one of LED_COLOR_ID_*. Allowed values are red, green or blue
> +          if there is one item in led-sources, otherwise white.
> +        allOf:
> +          - $ref: /schemas/leds/common.yaml#/properties/color

Can be dropped.

> +          - enum: [ 0, 1, 2, 3 ]
> +
> +    required:
> +      - led-sources
> +      - color
> +
> +examples:
> +  - |
> +
> +    #include <dt-bindings/leds/common.h>
> +
> +    i2c0 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        led-controller@2b {
> +            compatible = "cznic,turris-omnia-leds";
> +            reg = <0x2b>;
> +
> +            led0 {
> +                led-sources = <33 34 35>;
> +                color = <LED_COLOR_ID_WHITE>;
> +                function = LED_FUNCTION_POWER;
> +                linux,default-trigger = "heartbeat";
> +            };
> +
> +            led1 {
> +                led-sources = <3>;
> +                color = <LED_COLOR_ID_RED>;
> +                function = LED_FUNCTION_INDICATOR;
> +                function-enumerator = <1>;
> +            };
> +
> +            led2 {
> +                led-sources = <4>;
> +                color = <LED_COLOR_ID_GREEN>;
> +                function = LED_FUNCTION_INDICATOR;
> +                function-enumerator = <1>;
> +            };
> +
> +            led3 {
> +                led-sources = <5>;
> +                color = <LED_COLOR_ID_BLUE>;
> +                function = LED_FUNCTION_INDICATOR;
> +                function-enumerator = <1>;
> +            };
> +        };
> +    };
> +
> +...
> -- 
> 2.24.1
>

On Mon, 11 May 2020 14:43:54 -0500
Rob Herring <robh@kernel.org> wrote:

> On Thu, Apr 23, 2020 at 08:50:59AM +0200, Marek Behún wrote:
> > Add device-tree bindings documentation for Turris Omnia RGB LEDs.
> > 
> > ...
> >
> > +properties:
> > +  compatible:
> > +    const: cznic,turris-omnia-leds
> > +
> > +  reg:
> > +    description: I2C slave address of the microcontroller.
> > +    maxItems: 1
> > +
> > +patternProperties:
> > +  "^led[0-9][0-9]?$":  
> 
> 'led-' is preferred. And numbering is usually hex.
>
> What's the numbering here based on? Might be better to use 'reg' (and a 
> unit-address) rather than led-sources. led-sources was for cases where 
> the control interface (defined by reg) could have differing regulators 
> connected to the LEDs.

Jacek, Pavel, it seems that it would be better if I just rewrote the
driver into Dan's multicolor API. It would solve Rob's complaint and
could be merged once multicolor framework is...

Marek

Hi!

> > What's the numbering here based on? Might be better to use 'reg' (and a 
> > unit-address) rather than led-sources. led-sources was for cases where 
> > the control interface (defined by reg) could have differing regulators 
> > connected to the LEDs.
> 
> Jacek, Pavel, it seems that it would be better if I just rewrote the
> driver into Dan's multicolor API. It would solve Rob's complaint and
> could be merged once multicolor framework is...

It would be great to try that now. Multicolor is now very close to
being merged, and having independend verification that it works would
be great.

Thank you,
									Pavel