[PATCH v2 0/2] leds: introduce generic LED level meter driver

[PATCH v2 0/2] leds: introduce generic LED level meter driver

[Akinobu Mita]

This introduces a new LED driver that enables us to create a virtual LED
level meter device that consists of multiple LED devices by different
drivers.

Previously I developed the level meter feature for leds-gpio ("leds: gpio:
support multi-level brightness") [1].  Then I got a feedback from
Bjorn Andersson and made more generic new driver.  This driver is also
inspired by led-backlight driver patchset [2] and actually requires
devm_of_led_get() function provided by the patchset.

[1] https://lore.kernel.org/linux-leds/1570203299-4270-1-git-send-email-akinobu.mita@gmail.com/
[2] https://lore.kernel.org/linux-leds/20191009085127.22843-1-jjhiblot@ti.com/

* v2
- Use proper subject line for dt-binding patch.
- Swap the patch order.
- Various fixes noticed by Rob and Dan.
- Update example usage for brightness-weights property
- Use unified device proerty interface as much as possible.
- Support linux,default-trigger and default-state properties.

Akinobu Mita (2):
  dt-bindings: leds: Add leds-meter binding
  leds: Add generic LED level meter driver

 .../devicetree/bindings/leds/leds-meter.yaml       |  67 +++++++++
 drivers/leds/Kconfig                               |  10 ++
 drivers/leds/Makefile                              |   1 +
 drivers/leds/leds-meter.c                          | 151 +++++++++++++++++++++
 4 files changed, 229 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-meter.yaml
 create mode 100644 drivers/leds/leds-meter.c

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Bjorn Andersson <bjorn@kryo.se>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Jean-Jacques Hiblot <jjhiblot@ti.com>
Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Dan Murphy <dmurphy@ti.com>
-- 
2.7.4

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

[Akinobu Mita]

Add DT binding for generic LED level meter which consists of multiple LED
devices by different drivers.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Bjorn Andersson <bjorn@kryo.se>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Jean-Jacques Hiblot <jjhiblot@ti.com>
Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Dan Murphy <dmurphy@ti.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 .../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@gmail.com>
+
+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>;
+    };
+
+...
-- 
2.7.4

[PATCH v2 2/2] leds: Add generic LED level meter driver

[Akinobu Mita]

This introduces a new LED driver that enables us to create a virtual LED
level meter device that consists of multiple LED devices by different
drivers.

The brightness of the LED level meter is proportional to the number of
actual LEDs that are turned on.

For example, the LED level meter with four LEDs can be described by the
following DT node.

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

When the brightness of meter-leds is LED_HALF, the brightness of the led0
and led1 is set to maximum brightness and the brightness of led2 and led3
is set to zero.

We can optionally control the proportionality within a level meter by
specifying the contribution ratio for each LED with the brightness-weights
proporty.

For example, the following brightness-weights proporty provides an
exponential relationship between the number of LEDs turned on and the
brightness of meter-leds.

    meter-leds {
        compatible = "meter-leds";
        leds = <&led0>, <&led1>, <&led2>, <&led3>;
        brightness-weights = <32 32 64 127>;
    };

- 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

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Bjorn Andersson <bjorn@kryo.se>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Jean-Jacques Hiblot <jjhiblot@ti.com>
Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Dan Murphy <dmurphy@ti.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 drivers/leds/Kconfig      |  10 +++
 drivers/leds/Makefile     |   1 +
 drivers/leds/leds-meter.c | 151 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 162 insertions(+)
 create mode 100644 drivers/leds/leds-meter.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 1988de1..feecd2e 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -823,6 +823,16 @@ config LEDS_LM36274
 	  Say Y to enable the LM36274 LED driver for TI LMU devices.
 	  This supports the LED device LM36274.
 
+config LEDS_METER
+	tristate "LED support for generic level meter driver"
+	depends on LEDS_CLASS
+	help
+	  This option enables support for generic LED level meter that
+	  consists of multiple LED devices.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called leds-meter.
+
 comment "LED Triggers"
 source "drivers/leds/trigger/Kconfig"
 
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 41fb073..0fe2a28 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -85,6 +85,7 @@ obj-$(CONFIG_LEDS_LM3601X)		+= leds-lm3601x.o
 obj-$(CONFIG_LEDS_TI_LMU_COMMON)	+= leds-ti-lmu-common.o
 obj-$(CONFIG_LEDS_LM3697)		+= leds-lm3697.o
 obj-$(CONFIG_LEDS_LM36274)		+= leds-lm36274.o
+obj-$(CONFIG_LEDS_METER)		+= leds-meter.o
 
 # LED SPI Drivers
 obj-$(CONFIG_LEDS_CR0014114)		+= leds-cr0014114.o
diff --git a/drivers/leds/leds-meter.c b/drivers/leds/leds-meter.c
new file mode 100644
index 0000000..3d1d616
--- /dev/null
+++ b/drivers/leds/leds-meter.c
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+struct meter_led_ref {
+	struct led_classdev *cdev;
+	unsigned int brightness_weight;
+};
+
+struct meter_led_data {
+	struct led_classdev cdev;
+	int num_leds;
+	int total_weight;
+	struct meter_led_ref leds[];
+};
+
+struct leds_fract {
+	unsigned int numerator;
+	unsigned int denominator;
+};
+
+/* Compare two leds_fract structs */
+#define LEDS_FRACT_COMPARE(a, OP, b)			\
+	((u64)(a).numerator * (b).denominator OP	\
+	(u64)(b).numerator * (a).denominator)
+
+static void meter_led_set(struct led_classdev *led_cdev,
+			  enum led_brightness value)
+{
+	struct meter_led_data *meter_led =
+		container_of(led_cdev, struct meter_led_data, cdev);
+	struct leds_fract level = { 0, meter_led->total_weight };
+	struct leds_fract br = { value, LED_FULL };
+	int i;
+
+	for (i = 0; i < meter_led->num_leds; i++) {
+		struct meter_led_ref *led = &meter_led->leds[i];
+
+		if (LEDS_FRACT_COMPARE(br, <=, level))
+			break;
+
+		level.numerator += led->brightness_weight;
+
+		led_set_brightness(led->cdev, led->cdev->max_brightness);
+	}
+
+	for (; i < meter_led->num_leds; i++) {
+		struct meter_led_ref *led = &meter_led->leds[i];
+
+		led_set_brightness(led->cdev, 0);
+	}
+}
+
+static int meter_led_probe(struct platform_device *pdev)
+{
+	int i;
+	int num_leds;
+	int num_weights;
+	struct meter_led_data *meter_led;
+	const char *state;
+	struct led_init_data init_data = {};
+
+	num_leds = of_count_phandle_with_args(pdev->dev.of_node, "leds", NULL);
+	if (num_leds < 0)
+		return num_leds;
+
+	if (num_leds < 1) {
+		dev_err(&pdev->dev, "At least one LED must be specified!\n");
+		return -EINVAL;
+	}
+
+	num_weights = device_property_count_u32(&pdev->dev,
+						"brightness-weights");
+	if (num_weights < 0) {
+		num_weights = 0;
+	} else if (num_weights != num_leds) {
+		dev_err(&pdev->dev,
+			"The number of brightness-weights must be equal to the number of LEDs\n");
+		return -EINVAL;
+	}
+
+	meter_led = devm_kzalloc(&pdev->dev,
+				 struct_size(meter_led, leds, num_leds),
+				 GFP_KERNEL);
+	if (!meter_led)
+		return -ENOMEM;
+
+	for (i = 0; i < num_leds; i++) {
+		meter_led->leds[i].cdev = devm_of_led_get(&pdev->dev, i);
+		if (IS_ERR(meter_led->leds[i].cdev))
+			return PTR_ERR(meter_led->leds[i].cdev);
+
+		if (num_weights) {
+			of_property_read_u32_index(pdev->dev.of_node,
+					"brightness-weights", i,
+					&meter_led->leds[i].brightness_weight);
+		} else {
+			meter_led->leds[i].brightness_weight = 1;
+		}
+
+		meter_led->total_weight += meter_led->leds[i].brightness_weight;
+	}
+
+	meter_led->num_leds = num_leds;
+	meter_led->cdev.brightness_set = meter_led_set;
+	meter_led->cdev.max_brightness = LED_FULL;
+	meter_led->cdev.brightness = LED_OFF;
+
+	device_property_read_string(&pdev->dev, "linux,default-trigger",
+				    &meter_led->cdev.default_trigger);
+
+	if (!device_property_read_string(&pdev->dev, "default-state", &state)) {
+		if (!strcmp(state, "keep"))
+			dev_err(&pdev->dev,
+				"The \"keep\" setting for the default-state property is not supported\n");
+		else if (!strcmp(state, "on"))
+			meter_led->cdev.brightness = LED_FULL;
+	}
+
+	meter_led->cdev.brightness_set(&meter_led->cdev,
+				       meter_led->cdev.brightness);
+
+	platform_set_drvdata(pdev, meter_led);
+	init_data.fwnode = of_fwnode_handle(pdev->dev.of_node);
+
+	return devm_led_classdev_register_ext(&pdev->dev, &meter_led->cdev,
+					      &init_data);
+}
+
+static const struct of_device_id of_meter_leds_match[] = {
+	{ .compatible = "meter-leds", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_meter_leds_match);
+
+static struct platform_driver meter_led_driver = {
+	.driver = {
+		.name = "meter-led",
+		.of_match_table = of_meter_leds_match,
+	},
+	.probe = meter_led_probe,
+};
+module_platform_driver(meter_led_driver);
+
+MODULE_AUTHOR("Akinobu Mita");
+MODULE_DESCRIPTION("LED level meter driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:leds-meter");
-- 
2.7.4

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

[Dan Murphy]

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@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Bjorn Andersson <bjorn@kryo.se>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Jean-Jacques Hiblot <jjhiblot@ti.com>
> Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Dan Murphy <dmurphy@ti.com>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> ---
>   .../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@gmail.com>
> +
> +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

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

[Rob Herring]

On Sun, Oct 27, 2019 at 11:09:38PM +0900, Akinobu Mita wrote:
> Add DT binding for generic LED level meter which consists of multiple LED
> devices by different drivers.

Do you have some pointers to actual h/w?
 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Bjorn Andersson <bjorn@kryo.se>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Jean-Jacques Hiblot <jjhiblot@ti.com>
> Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Dan Murphy <dmurphy@ti.com>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> ---
>  .../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@gmail.com>
> +
> +description:
> +  Generic LED level meter consists of multiple LED devices by different drivers.

Googling this, the only thing I see is audio level meters though those 
are all just a voltage level input, so it could be any source I guess.

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

This seems oddly named if this is a bar graph. Yes, more LEDs on is 
brighter, but it's really a piecewise linear graph you want. 

Each LED could have variable brightness, so you could also ramp the 
brightness for an individual LED.

Rob

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

[Akinobu Mita]

2019年10月29日(火) 21:06 Dan Murphy <dmurphy@ti.com>:
>
> 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@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Bjorn Andersson <bjorn@kryo.se>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Cc: Jean-Jacques Hiblot <jjhiblot@ti.com>
> > Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> > Cc: Pavel Machek <pavel@ucw.cz>
> > Cc: Dan Murphy <dmurphy@ti.com>
> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> > ---
> >   .../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@gmail.com>
> > +
> > +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>;
    };
  };

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

[Akinobu Mita]

2019年10月30日(水) 23:24 Rob Herring <robh@kernel.org>:
>
> On Sun, Oct 27, 2019 at 11:09:38PM +0900, Akinobu Mita wrote:
> > Add DT binding for generic LED level meter which consists of multiple LED
> > devices by different drivers.
>
> Do you have some pointers to actual h/w?

This doesn't require a specific hardware.
If there are more than two LEDs that can be described by devicetree,
it can be an LED level meter by bundling them up with this binding.

> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Bjorn Andersson <bjorn@kryo.se>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Cc: Jean-Jacques Hiblot <jjhiblot@ti.com>
> > Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> > Cc: Pavel Machek <pavel@ucw.cz>
> > Cc: Dan Murphy <dmurphy@ti.com>
> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> > ---
> >  .../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@gmail.com>
> > +
> > +description:
> > +  Generic LED level meter consists of multiple LED devices by different drivers.
>
> Googling this, the only thing I see is audio level meters though those
> are all just a voltage level input, so it could be any source I guess.
>
> > +
> > +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.
>
> This seems oddly named if this is a bar graph. Yes, more LEDs on is
> brighter, but it's really a piecewise linear graph you want.

OK.  I'll rewrite the description.

> Each LED could have variable brightness, so you could also ramp the
> brightness for an individual LED.

For now, each LED is set to 0 or max brightness.  Maybe we should
set middle brightness value for an LED on the border line.

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

[Rob Herring]

On Wed, Oct 30, 2019 at 10:27 AM Akinobu Mita <akinobu.mita@gmail.com> wrote:
>
> 2019年10月30日(水) 23:24 Rob Herring <robh@kernel.org>:
> >
> > On Sun, Oct 27, 2019 at 11:09:38PM +0900, Akinobu Mita wrote:
> > > Add DT binding for generic LED level meter which consists of multiple LED
> > > devices by different drivers.
> >
> > Do you have some pointers to actual h/w?
>
> This doesn't require a specific hardware.

But I do for my understanding. If I don't understand it, then my
default answer is: no, we don't do "generic" bindings because the h/w
is never generic.

> If there are more than two LEDs that can be described by devicetree,
> it can be an LED level meter by bundling them up with this binding.

Depends on the physical locations too? If you just want to group any
random LEDs, then do that from userspace because it's more flexible.

Rob

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

[Akinobu Mita]

2019年10月31日(木) 3:29 Rob Herring <robh@kernel.org>:
>
> On Wed, Oct 30, 2019 at 10:27 AM Akinobu Mita <akinobu.mita@gmail.com> wrote:
> >
> > 2019年10月30日(水) 23:24 Rob Herring <robh@kernel.org>:
> > >
> > > On Sun, Oct 27, 2019 at 11:09:38PM +0900, Akinobu Mita wrote:
> > > > Add DT binding for generic LED level meter which consists of multiple LED
> > > > devices by different drivers.
> > >
> > > Do you have some pointers to actual h/w?
> >
> > This doesn't require a specific hardware.
>
> But I do for my understanding. If I don't understand it, then my
> default answer is: no, we don't do "generic" bindings because the h/w
> is never generic.

For example, https://www.switch-science.com/catalog/2677/
The LEDs on this board are controlled by NXP PCA9956B.
The driver is under development by Bjorn Andersson.

But the LED level meter doesn't depend on the actual LED controller driver,
so I call it "generic" LED level meter driver.

> > If there are more than two LEDs that can be described by devicetree,
> > it can be an LED level meter by bundling them up with this binding.
>
> Depends on the physical locations too? If you just want to group any
> random LEDs, then do that from userspace because it's more flexible.

Yes.  I assume that the level meter's LEDs are mounted in a line, or matrix.
I plan to rename it to 'leds-array' because 'leds-meter' is unclear.