Re: [PATCH v2 1/2] dt-bindings: leds: Add leds-meter binding

[Akinobu Mita <akinobu.mita@xxxxxxxxx>]

2019年10月29日(火) 21:06 Dan Murphy <dmurphy@xxxxxx>:
>
> Akinobu
>
> On 10/27/19 9:09 AM, Akinobu Mita wrote:
> > Add DT binding for generic LED level meter which consists of multiple LED
> > devices by different drivers.
> >
> > Cc: Rob Herring <robh+dt@xxxxxxxxxx>
> > Cc: Mark Rutland <mark.rutland@xxxxxxx>
> > Cc: Bjorn Andersson <bjorn@xxxxxxx>
> > Cc: Tomi Valkeinen <tomi.valkeinen@xxxxxx>
> > Cc: Jean-Jacques Hiblot <jjhiblot@xxxxxx>
> > Cc: Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx>
> > Cc: Pavel Machek <pavel@xxxxxx>
> > Cc: Dan Murphy <dmurphy@xxxxxx>
> > Signed-off-by: Akinobu Mita <akinobu.mita@xxxxxxxxx>
> > ---
> >   .../devicetree/bindings/leds/leds-meter.yaml       | 67 ++++++++++++++++++++++
> >   1 file changed, 67 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/leds/leds-meter.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/leds/leds-meter.yaml b/Documentation/devicetree/bindings/leds/leds-meter.yaml
> > new file mode 100644
> > index 0000000..b5fcd98
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/leds/leds-meter.yaml
> > @@ -0,0 +1,67 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/leds/leds-meter.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Generic LED level meter
> > +
> > +maintainers:
> > +  - Akinobu Mita <akinobu.mita@xxxxxxxxx>
> > +
> > +description:
> > +  Generic LED level meter consists of multiple LED devices by different drivers.
> > +
> > +properties:
> > +  $nodename:
> > +    pattern: "^meter-leds(@.*)?"
> > +
> > +  compatible:
> > +    const: meter-leds
> > +
> > +  leds:
> > +    allOf:
> > +      - $ref: /schemas/types.yaml#/definitions/phandle-array
> > +    description: List of phandles to LED node that are members of a level meter.
> > +
> > +  brightness-weights:
> > +    allOf:
> > +      - $ref: /schemas/types.yaml#/definitions/uint32-array
> > +    description: |
> > +      By default, the brightness of the LED level meter is proportional to the
> > +      number of actual LEDs that are turned on.  We can optionally specify
> > +      the contribution ratio for each LED within a level meter by this
> > +      property.  The example below shows how to setup an exponential
> > +      relationship between the number of LEDs turned on and the brightness of
> > +      meter-leds.
> > +
> > +required:
> > + - compatible
> > + - leds
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    // Example 1: LED level meter with four LEDs
> > +    meter-leds {
> > +        compatible = "meter-leds";
> > +        leds = <&led0>, <&led1>, <&led2>, <&led3>;
> > +    };
> > +
> > +    // Example 2: Exponential relationship between the number of LEDs turned on
> > +    // and the brightness of meter-leds
> > +    //
> > +    // When the maximum brightness is 255
> > +    // - No LEDs are turned on if brightness == 0
> > +    // - led0 is turned on if 0 < brightness <= 32
> > +    // - led0 and led1 are turned on if 32 < brightness <= 64
> > +    // - led0, led1, and led2 are turned on if 64 < brightness <= 128
> > +    // - All LEDs are turned on if 128 < brightness <= 255
> > +    meter-leds {
> > +        compatible = "meter-leds";
> > +        leds = <&led0>, <&led1>, <&led2>, <&led3>;
> > +        brightness-weights = <32 32 64 127>;
> > +    };
> > +
> > +...
>
> Fails binding check
>
> Documentation/devicetree/bindings/leds/leds-meter.example.dts:32.20-36.11:
> ERROR (duplicate_node_names): /example-0/meter-leds: Duplicate node name
> ERROR: Input tree has errors, aborting (use -f to force output)
> scripts/Makefile.lib:314: recipe for target
> 'Documentation/devicetree/bindings/leds/leds-meter.example.dt.yaml' failed
> make[1]: ***
> [Documentation/devicetree/bindings/leds/leds-meter.example.dt.yaml] Error 2
> Makefile:1284: recipe for target 'dt_binding_check' failed
> make: *** [dt_binding_check] Error 2
>

Thanks for reporting.  This is because the two examples have the same
node name, so we need a '- |' line between the two examples.

And I noticed that current implementation doesn't follow the common leds
properties described in Documentation/devicetree/bindings/leds/common.txt.

i.e. when we define two level meters in the system, we need to describe
them like below.

  meter-leds-4seg {
    compatible = "meter-leds";
    leds = <&led0>, <&led1>, <&led2>, <&led3>;
  };

  meter-leds-2seg {
    compatible = "meter-leds";
    leds = <&led4>, <&led5>;
  };

But, the description should be like this:

  led-controller@0 {
    compatible = "meter-leds";

    meter-leds-4seg {
      leds = <&led0>, <&led1>, <&led2>, <&led3>;
    };

    meter-leds-2seg {
      leds = <&led4>, <&led5>;
    };
  };