* [PATCH v3 1/2] dt-bindings: leds: add cznic,turris-omnia-leds binding
[not found] <20200712210601.5239-1-marek.behun@nic.cz>
@ 2020-07-12 21:06 ` Marek Behún
2020-07-12 21:27 ` Jacek Anaszewski
0 siblings, 1 reply; 6+ messages in thread
From: Marek Behún @ 2020-07-12 21:06 UTC (permalink / raw)
To: linux-leds
Cc: Pavel Machek, jacek.anaszewski, Dan Murphy, Marek Behún,
Rob Herring, devicetree
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 | 82 +++++++++++++++++++
1 file changed, 82 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..9817ea3ac69b
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml
@@ -0,0 +1,82 @@
+# 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
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+patternProperties:
+ "^multi-led[0-9a-f]$":
+ type: object
+ allOf:
+ - $ref: leds-class-multicolor.yaml#
+ description:
+ This node represents one of the RGB LED devices on Turris Omnia.
+
+ properties:
+ reg:
+ minimum: 0
+ maximum: 11
+ description:
+ This property identifies one of the LEDs on the front panel of the
+ Turris Omnia router.
+
+ required:
+ - reg
+
+examples:
+ - |
+
+ #include <dt-bindings/leds/common.h>
+
+ i2c0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ led-controller@2b {
+ compatible = "cznic,turris-omnia-leds";
+ reg = <0x2b>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ multi-led@0 {
+ reg = <0>;
+ color = <LED_COLOR_ID_MULTI>;
+ function = LED_FUNCTION_POWER;
+ linux,default-trigger = "heartbeat";
+ };
+
+ multi-led@a {
+ reg = <0xa>;
+ color = <LED_COLOR_ID_MULTI>;
+ function = LED_FUNCTION_INDICATOR;
+ function-enumerator = <1>;
+ };
+ };
+ };
+
+...
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: leds: add cznic,turris-omnia-leds binding
2020-07-12 21:06 ` [PATCH v3 1/2] dt-bindings: leds: add cznic,turris-omnia-leds binding Marek Behún
@ 2020-07-12 21:27 ` Jacek Anaszewski
2020-07-12 21:40 ` Marek Behun
0 siblings, 1 reply; 6+ messages in thread
From: Jacek Anaszewski @ 2020-07-12 21:27 UTC (permalink / raw)
To: Marek Behún, linux-leds
Cc: Pavel Machek, Dan Murphy, Rob Herring, devicetree
Hi Marek,
Thank you for the patch. One note below.
On 7/12/20 11:06 PM, 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 | 82 +++++++++++++++++++
> 1 file changed, 82 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..9817ea3ac69b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml
> @@ -0,0 +1,82 @@
> +# 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
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> +patternProperties:
> + "^multi-led[0-9a-f]$":
> + type: object
> + allOf:
> + - $ref: leds-class-multicolor.yaml#
> + description:
> + This node represents one of the RGB LED devices on Turris Omnia.
> +
> + properties:
> + reg:
> + minimum: 0
> + maximum: 11
> + description:
> + This property identifies one of the LEDs on the front panel of the
> + Turris Omnia router.
> +
> + required:
> + - reg
> +
> +examples:
> + - |
> +
> + #include <dt-bindings/leds/common.h>
> +
> + i2c0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + led-controller@2b {
> + compatible = "cznic,turris-omnia-leds";
> + reg = <0x2b>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + multi-led@0 {
> + reg = <0>;
> + color = <LED_COLOR_ID_MULTI>;
> + function = LED_FUNCTION_POWER;
Please provide child nodes for each color LED. Let's stick
to the bindings closely and not make any deviations from
the beginning.
> + linux,default-trigger = "heartbeat";
> + };
> +
> + multi-led@a {
> + reg = <0xa>;
> + color = <LED_COLOR_ID_MULTI>;
> + function = LED_FUNCTION_INDICATOR;
> + function-enumerator = <1>;
> + };
> + };
> + };
> +
> +...
>
--
Best regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: leds: add cznic,turris-omnia-leds binding
2020-07-12 21:27 ` Jacek Anaszewski
@ 2020-07-12 21:40 ` Marek Behun
2020-07-12 22:11 ` Jacek Anaszewski
0 siblings, 1 reply; 6+ messages in thread
From: Marek Behun @ 2020-07-12 21:40 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: linux-leds, Pavel Machek, Dan Murphy, Rob Herring, devicetree
On Sun, 12 Jul 2020 23:27:07 +0200
Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:
> > + multi-led@0 {
> > + reg = <0>;
> > + color = <LED_COLOR_ID_MULTI>;
> > + function = LED_FUNCTION_POWER;
>
> Please provide child nodes for each color LED. Let's stick
> to the bindings closely and not make any deviations from
> the beginning.
Why? It would make sense if there were devices using this controller
having other configuration, but on Omnia, all LEDs are RGB.
Also, if I do this, should I also make the driver check in the probe
function whether the per-channel child nodes are correct? Eg. if they
are always three: one for red, one for green and one for blue? Or
should the driver ignore this and only the device tree binding specify
it?
Because the way the driver is written now, it only registers
multi-color RGB LEDs.
Marek
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: leds: add cznic,turris-omnia-leds binding
2020-07-12 21:40 ` Marek Behun
@ 2020-07-12 22:11 ` Jacek Anaszewski
2020-07-13 15:06 ` Rob Herring
0 siblings, 1 reply; 6+ messages in thread
From: Jacek Anaszewski @ 2020-07-12 22:11 UTC (permalink / raw)
To: Marek Behun; +Cc: linux-leds, Pavel Machek, Dan Murphy, Rob Herring, devicetree
On 7/12/20 11:40 PM, Marek Behun wrote:
> On Sun, 12 Jul 2020 23:27:07 +0200
> Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:
>
>>> + multi-led@0 {
>>> + reg = <0>;
>>> + color = <LED_COLOR_ID_MULTI>;
>>> + function = LED_FUNCTION_POWER;
>>
>> Please provide child nodes for each color LED. Let's stick
>> to the bindings closely and not make any deviations from
>> the beginning.
>
> Why? It would make sense if there were devices using this controller
> having other configuration, but on Omnia, all LEDs are RGB.
>
> Also, if I do this, should I also make the driver check in the probe
> function whether the per-channel child nodes are correct? Eg. if they
> are always three: one for red, one for green and one for blue? Or
> should the driver ignore this and only the device tree binding specify
> it?
>
> Because the way the driver is written now, it only registers
> multi-color RGB LEDs.
This is not RGB framework, but multicolor framework. It is not justified
to pretend that RGB is default. Unless you would state that clearly in
the comment in DT, but that should be agreed upon with Rob.
--
Best regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: leds: add cznic,turris-omnia-leds binding
2020-07-12 22:11 ` Jacek Anaszewski
@ 2020-07-13 15:06 ` Rob Herring
2020-07-13 21:28 ` Jacek Anaszewski
0 siblings, 1 reply; 6+ messages in thread
From: Rob Herring @ 2020-07-13 15:06 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: Marek Behun, linux-leds, Pavel Machek, Dan Murphy, devicetree
On Mon, Jul 13, 2020 at 12:11:51AM +0200, Jacek Anaszewski wrote:
> On 7/12/20 11:40 PM, Marek Behun wrote:
> > On Sun, 12 Jul 2020 23:27:07 +0200
> > Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:
> >
> > > > + multi-led@0 {
> > > > + reg = <0>;
> > > > + color = <LED_COLOR_ID_MULTI>;
> > > > + function = LED_FUNCTION_POWER;
> > >
> > > Please provide child nodes for each color LED. Let's stick
> > > to the bindings closely and not make any deviations from
> > > the beginning.
> >
> > Why? It would make sense if there were devices using this controller
> > having other configuration, but on Omnia, all LEDs are RGB.
> >
> > Also, if I do this, should I also make the driver check in the probe
> > function whether the per-channel child nodes are correct? Eg. if they
> > are always three: one for red, one for green and one for blue? Or
> > should the driver ignore this and only the device tree binding specify
> > it?
> >
> > Because the way the driver is written now, it only registers
> > multi-color RGB LEDs.
>
> This is not RGB framework, but multicolor framework. It is not justified
> to pretend that RGB is default. Unless you would state that clearly in
> the comment in DT, but that should be agreed upon with Rob.
If the LEDs are fixed in h/w and never vary for this controller, then
they don't need to be in DT. However, is it really possible that a
channel only supports 1 color of LED? I don't think so.
Rob
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: leds: add cznic,turris-omnia-leds binding
2020-07-13 15:06 ` Rob Herring
@ 2020-07-13 21:28 ` Jacek Anaszewski
0 siblings, 0 replies; 6+ messages in thread
From: Jacek Anaszewski @ 2020-07-13 21:28 UTC (permalink / raw)
To: Rob Herring; +Cc: Marek Behun, linux-leds, Pavel Machek, Dan Murphy, devicetree
On 7/13/20 5:06 PM, Rob Herring wrote:
> On Mon, Jul 13, 2020 at 12:11:51AM +0200, Jacek Anaszewski wrote:
>> On 7/12/20 11:40 PM, Marek Behun wrote:
>>> On Sun, 12 Jul 2020 23:27:07 +0200
>>> Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:
>>>
>>>>> + multi-led@0 {
>>>>> + reg = <0>;
>>>>> + color = <LED_COLOR_ID_MULTI>;
>>>>> + function = LED_FUNCTION_POWER;
>>>>
>>>> Please provide child nodes for each color LED. Let's stick
>>>> to the bindings closely and not make any deviations from
>>>> the beginning.
>>>
>>> Why? It would make sense if there were devices using this controller
>>> having other configuration, but on Omnia, all LEDs are RGB.
>>>
>>> Also, if I do this, should I also make the driver check in the probe
>>> function whether the per-channel child nodes are correct? Eg. if they
>>> are always three: one for red, one for green and one for blue? Or
>>> should the driver ignore this and only the device tree binding specify
>>> it?
>>>
>>> Because the way the driver is written now, it only registers
>>> multi-color RGB LEDs.
>>
>> This is not RGB framework, but multicolor framework. It is not justified
>> to pretend that RGB is default. Unless you would state that clearly in
>> the comment in DT, but that should be agreed upon with Rob.
>
> If the LEDs are fixed in h/w and never vary for this controller, then
> they don't need to be in DT. However, is it really possible that a
> channel only supports 1 color of LED? I don't think so.
Theoretically we could allow for registering two LEDs as LED multi
color device and the other one as monochrome LED class device
but I admit that this would be a bit convoluted and entail unnecessary
complexity in the driver.
So I'm OK with no child nodes for these bindings, but let's add some
comment justifying that.
--
Best regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-13 21:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20200712210601.5239-1-marek.behun@nic.cz>
2020-07-12 21:06 ` [PATCH v3 1/2] dt-bindings: leds: add cznic,turris-omnia-leds binding Marek Behún
2020-07-12 21:27 ` Jacek Anaszewski
2020-07-12 21:40 ` Marek Behun
2020-07-12 22:11 ` Jacek Anaszewski
2020-07-13 15:06 ` Rob Herring
2020-07-13 21:28 ` Jacek Anaszewski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).