Re: [PATCH 2/2] dt-binding: leds: Document leds-multi-gpio bindings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

See below.

On 24.3.2021 9.56, Hermes Zhang wrote:
From: Hermes Zhang <chenhuiz@xxxxxxxx>

Document the device tree bindings of the multiple GPIOs LED driver
Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml.

Signed-off-by: Hermes Zhang <chenhuiz@xxxxxxxx>
---
  .../bindings/leds/leds-multi-gpio.yaml        | 50 +++++++++++++++++++
  1 file changed, 50 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml

diff --git a/Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml b/Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml
new file mode 100644
index 000000000000..6f2b47487b90
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/leds-multi-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Multiple GPIOs LED driver
+
+maintainers:
+  - Hermes Zhang <chenhuiz@xxxxxxxx>
+
+description:
+  This will support some LED made of multiple GPIOs and the brightness of the
+  LED could map to different states of the GPIOs.
+
+properties:
+  compatible:
+    const: multi-gpio-led
+
+  led-gpios:
+    description: Array of one or more GPIOs pins used to control the LED.
+    minItems: 1
+    maxItems: 8  # Should be enough

We also have a case with multi color LEDs (which is probably a more common than multi intensity LED. So I am wondering how these both could co-exist.

From: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/leds/leds-gpio.yaml?h=v5.12-rc4#n58

        led-0 {
            gpios = <&mcu_pio 0 GPIO_ACTIVE_LOW>;
            linux,default-trigger = "disk-activity";
            function = LED_FUNCTION_DISK;
        };

Now 'gpios' (and in LED context) and 'led-gpios' is very close to each other and could easily be confused.

Perhaps this could be something like:

intensity-gpios = ...

or even simplified then just to gpios = <...>

+
+  led-states:
+    description: |
+      The array list the supported states here which will map to brightness
+      from 0 to maximum. Each item in the array will present all the GPIOs
+      value by bit.
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    minItems: 1
+    maxItems: 16 # Should be enough
+
+required:
+  - compatible
+  - led-gpios
+  - led-states
+
+additionalProperties: false
+
+examples:
+  - |
+    gpios-led {
+      compatible = "multi-gpio-led";
+
+      led-gpios = <&gpio0 23 0x1>,
+                  <&gpio0 24 0x1>;
+      led-states = /bits/ 8 <0x00 0x01 0x02 0x03>;
+    };
+...


From: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml?h=v5.12-rc4#n196

There is example of multi color LED configuration. In example below I used two-color LED with red and green as an example (which what we seem to have most in use).

Then if try to combine these into something like:

# Multi color LED with single GPIO line per color
multi-led@2 {
  compatible = "gpio-leds";
  color = <LED_COLOR_ID_MULTICOLOR>;
  led@0 {
    color = <LED_COLOR_ID_GREEN>;
    gpios = <&mcu_pio 0 GPIO_ACTIVE_LOW>;
  };

  led@1 {
    color = <LED_COLOR_ID_RED>;
    gpios = <&mcu_pio 1 GPIO_ACTIVE_LOW>;
  };
};

# And with intensity GPIOs:
multi-led@2 {
  compatible = "gpio-leds";
  color = <LED_COLOR_ID_MULTICOLOR>;

  led@0 {
    color = <LED_COLOR_ID_GREEN>;
    gpios = <&gpio0 23 0x1>,
            <&gpio0 24 0x1>;
    ... see below
  };

  led@1 {
    color = <LED_COLOR_ID_RED>;
    gpios = <&gpio0 25 0x1>,
            <&gpio0 26 0x1>;
    ... see below
  };
};

# And then single GPIO with intensity GPIOs:
led@2 {
  compatible = "gpio-leds";
  gpios = <&gpio0 23 0x1>,
          <&gpio0 24 0x1>;
  gpios-brightness-levels = <0 1 2 3>
};

I changed 'led-states' to 'gpios-brightness-levels' as it describe more that this is about brightness and not some other state information.

How would this sound?

Thanks,
Vesa Jääskeläinen



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux