diff mbox series

[v3,leds-next,2/2] dt-bindings: leds: Document Turris Omnia LED controller binding

Message ID 20190328193428.19273-3-marek.behun@nic.cz
State Not Applicable, archived
Headers show
Series None | expand

Checks

Context Check Description
robh/checkpatch warning "total: 0 errors, 1 warnings, 85 lines checked"

Commit Message

Marek Behún March 28, 2019, 7:34 p.m. UTC
This adds device tree binding documentation for the front panel LEDs
controller of CZ.NIC's Turris Omnia router.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
---
 .../bindings/leds/leds-turris-omnia.txt       | 85 +++++++++++++++++++
 1 file changed, 85 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-turris-omnia.txt

Comments

Marek Behún March 29, 2019, 7 p.m. UTC | #1
Hi Rob,

I have a question about how to design correctly the situation about
global brigthness on this controller in the device tree.

The thing is that these LEDs are, in hardware, controlled via PWM and
SPI, but this is done via microcontroller running a proprietary
firmware. So from the point of view of the CPU, should the LEDs be
described in the device tree as connected to the mircocontroller
direclty, or should pwm and spi nodes be created as well?

Next thing is that the values of the RGB channels for each LED is
controlled via SPI, but the global brightness controlled byt the single
PWM also controls the LED brightnesses globally.

How should this be specified?

Thank you.

> +Example:
> +
> +	led-controller@2b {
> +		compatible = "cznic,turris-omnia-leds";
> +		reg = <0x2b>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		led@0 {
> +			reg = <0x0>;
> +			label = "userB";
> +			linux,default-trigger = "heartbeat";
> +		};
> +
> +		led@1 {
> +			reg = <0x1>;
> +			label = "userA";
> +		};
> +
> +		led@2 {
> +			reg = <0x2>;
> +			label = "pci3";
> +		};
> +
> +		led@3 {
> +			reg = <0x3>;
> +			label = "pci2";
> +		};
> +
> +		led@4 {
> +			reg = <0x4>;
> +			label = "pci1";
> +		};
> +
> +		led@5 {
> +			reg = <0x5>;
> +			label = "wan";
> +		};
> +
> +		led@6 {
> +			reg = <0x6>;
> +			label = "lan4";
> +		};
> +
> +		led@7 {
> +			reg = <0x7>;
> +			label = "lan3";
> +		};
> +
> +		led@8 {
> +			reg = <0x8>;
> +			label = "lan2";
> +		};
> +
> +		led@9 {
> +			reg = <0x9>;
> +			label = "lan1";
> +		};
> +
> +		led@a {
> +			reg = <0xa>;
> +			label = "lan0";
> +		};
> +
> +		led@b {
> +			reg = <0xb>;
> +			label = "power";
> +		};
> +	};
Pavel Machek March 29, 2019, 8:40 p.m. UTC | #2
Hi!

> I have a question about how to design correctly the situation about
> global brigthness on this controller in the device tree.
> 
> The thing is that these LEDs are, in hardware, controlled via PWM and
> SPI, but this is done via microcontroller running a proprietary
> firmware. So from the point of view of the CPU, should the LEDs be
> described in the device tree as connected to the mircocontroller
> direclty, or should pwm and spi nodes be created as well?

I don't think you need to describe interfaces between 

> Next thing is that the values of the RGB channels for each LED is
> controlled via SPI, but the global brightness controlled byt the single
> PWM also controls the LED brightnesses globally.
> 
> How should this be specified?

I'd say it does not need to be specified in the device tree. Device
tree tells us about the microcontroller, and driver knows the
rest. This is not generic hardware, this is microcontroller with
special programming.

Strictly speaking, we'd need to know about the LEDs, either;
cznic,turris-omnia-leds means we have 12 LEDs and what they mean. But
let's keep the LEDs, it will be easier that way.

								Pave

IOW: this is okay:


> > +Example:
> > +
> > +	led-controller@2b {
> > +		compatible = "cznic,turris-omnia-leds";
> > +		reg = <0x2b>;
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		led@0 {
> > +			reg = <0x0>;
> > +			label = "userB";
> > +			linux,default-trigger = "heartbeat";
> > +		};
> > +
> > +		led@1 {
> > +			reg = <0x1>;
> > +			label = "userA";
> > +		};
> > +
> > +		led@2 {
> > +			reg = <0x2>;
> > +			label = "pci3";
> > +		};
> > +
> > +		led@3 {
> > +			reg = <0x3>;
> > +			label = "pci2";
> > +		};
> > +
> > +		led@4 {
> > +			reg = <0x4>;
> > +			label = "pci1";
> > +		};
> > +
> > +		led@5 {
> > +			reg = <0x5>;
> > +			label = "wan";
> > +		};
> > +
> > +		led@6 {
> > +			reg = <0x6>;
> > +			label = "lan4";
> > +		};
> > +
> > +		led@7 {
> > +			reg = <0x7>;
> > +			label = "lan3";
> > +		};
> > +
> > +		led@8 {
> > +			reg = <0x8>;
> > +			label = "lan2";
> > +		};
> > +
> > +		led@9 {
> > +			reg = <0x9>;
> > +			label = "lan1";
> > +		};
> > +
> > +		led@a {
> > +			reg = <0xa>;
> > +			label = "lan0";
> > +		};
> > +
> > +		led@b {
> > +			reg = <0xb>;
> > +			label = "power";
> > +		};
> > +	};
Rob Herring April 6, 2019, 6:05 a.m. UTC | #3
On Thu, 28 Mar 2019 20:34:28 +0100, =?UTF-8?q?Marek=20Beh=C3=BAn?= wrote:
> This adds device tree binding documentation for the front panel LEDs
> controller of CZ.NIC's Turris Omnia router.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> ---
>  .../bindings/leds/leds-turris-omnia.txt       | 85 +++++++++++++++++++
>  1 file changed, 85 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-turris-omnia.txt
> 

Reviewed-by: Rob Herring <robh@kernel.org>
Pavel Machek April 19, 2019, 6:10 p.m. UTC | #4
On Thu 2019-03-28 20:34:28, Marek Behún wrote:
> This adds device tree binding documentation for the front panel LEDs
> controller of CZ.NIC's Turris Omnia router.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org

Acked-by: Pavel Machek <pavel@ucw.cz>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/leds/leds-turris-omnia.txt b/Documentation/devicetree/bindings/leds/leds-turris-omnia.txt
new file mode 100644
index 000000000000..a9fd9a768251
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-turris-omnia.txt
@@ -0,0 +1,85 @@ 
+Bindings for LEDs on CZ.NIC's Turris Omnia
+
+Required properties:
+ - compatible :		Should be "cznic,turris-omnia-leds"
+ - reg :		I2C slave address
+ - #address-cells :	Should be 1
+ - #size-cells :	Should be 0
+
+LED sub-node properties:
+ - reg :		Must be from 0x0 to 0xb, since there are 12 LEDs on this
+			controller.
+ - label :		(optional)
+   see Documentation/devicetree/bindings/leds/common.txt
+ - linux,default-trigger : (optional)
+   see Documentation/devicetree/bindings/leds/common.txt
+
+Example:
+
+	led-controller@2b {
+		compatible = "cznic,turris-omnia-leds";
+		reg = <0x2b>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		led@0 {
+			reg = <0x0>;
+			label = "userB";
+			linux,default-trigger = "heartbeat";
+		};
+
+		led@1 {
+			reg = <0x1>;
+			label = "userA";
+		};
+
+		led@2 {
+			reg = <0x2>;
+			label = "pci3";
+		};
+
+		led@3 {
+			reg = <0x3>;
+			label = "pci2";
+		};
+
+		led@4 {
+			reg = <0x4>;
+			label = "pci1";
+		};
+
+		led@5 {
+			reg = <0x5>;
+			label = "wan";
+		};
+
+		led@6 {
+			reg = <0x6>;
+			label = "lan4";
+		};
+
+		led@7 {
+			reg = <0x7>;
+			label = "lan3";
+		};
+
+		led@8 {
+			reg = <0x8>;
+			label = "lan2";
+		};
+
+		led@9 {
+			reg = <0x9>;
+			label = "lan1";
+		};
+
+		led@a {
+			reg = <0xa>;
+			label = "lan0";
+		};
+
+		led@b {
+			reg = <0xb>;
+			label = "power";
+		};
+	};