From: Hermes Zhang <[email protected]>
*** BLURB HERE ***
New multiple GPIOs LED driver
Hermes Zhang (2):
leds: leds-multi-gpio: Add multiple GPIOs LED driver
dt-binding: leds: Document leds-multi-gpio bindings
.../bindings/leds/leds-multi-gpio.yaml | 50 +++++++
drivers/leds/Kconfig | 12 ++
drivers/leds/Makefile | 1 +
drivers/leds/leds-multi-gpio.c | 140 ++++++++++++++++++
4 files changed, 203 insertions(+)
create mode 100644 Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml
create mode 100644 drivers/leds/leds-multi-gpio.c
--
2.20.1
From: Hermes Zhang <[email protected]>
Document the device tree bindings of the multiple GPIOs LED driver
Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml.
Signed-off-by: Hermes Zhang <[email protected]>
---
.../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 <[email protected]>
+
+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
+
+ 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>;
+ };
+...
--
2.20.1
On Wed, 24 Mar 2021 15:56:31 +0800
Hermes Zhang <[email protected]> wrote:
> From: Hermes Zhang <[email protected]>
>
> Document the device tree bindings of the multiple GPIOs LED driver
> Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml.
The dt-binding should come before the actual driver. Otherwise there
will be a commit with the driver existing but no documentation for its
bindings.
> +description:
> + This will support some LED made of multiple GPIOs and the brightness of the
> + LED could map to different states of the GPIOs.
Don't use future tense. Don't mention drivers in device tree
binding documentation, if it can be helped. The device tree binding
documentation should describe devices and their nodes... (and I see
that I did a similar mistake for the cznic,turris-omnia-leds.yaml
binding, I will have to fix that).
Instead the description should be something like this:
This binding represents LED devices which are controller with
multiple GPIO lines in order to achieve more than two brightness
states.
> +
> +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
> +
> + 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.
Again future tense...
> + $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>;
> + };
> +...
On Wed, 24 Mar 2021 15:56:29 +0800
Hermes Zhang <[email protected]> wrote:
> From: Hermes Zhang <[email protected]>
>
> *** BLURB HERE ***
"*** BLURB HERE ***" is meant to be substituted with your text :)
All in all I think you are adding functionality which can be
incorporated simply into the existing leds-gpio driver instead of new
driver.
Marek
Hi,
See below.
On 24.3.2021 9.56, Hermes Zhang wrote:
> From: Hermes Zhang <[email protected]>
>
> Document the device tree bindings of the multiple GPIOs LED driver
> Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml.
>
> Signed-off-by: Hermes Zhang <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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
Hi!
> See below.
Please trim.
> > + 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 = <...>
...
> How would this sound?
Well, not too bad on a quick look.
Are you willing to implement such multi-color-multi-bit-multi-gpio
driver?
Best regards,
Pavel
--
http://www.livejournal.com/~pavelmachek
Hi,
On 25.3.2021 20.41, Pavel Machek wrote:
>>> + 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 = <...>
>
> ...
>> How would this sound?
>
> Well, not too bad on a quick look.
>
> Are you willing to implement such multi-color-multi-bit-multi-gpio
> driver?
We have a need for multi color GPIO LED support so I can work on that if
no one else gets there before me -- I do not have hardware with multiple
GPIO lines controlling the brightness so that needs a bit more effort in
order to test that out.
At some point of time I could also revive the PWM stuff if no one else
beats me to it -- but probably the GPIO variant is easier to get done as
binary states are easier.
Thanks,
Vesa Jääskeläinen