[PATCH v3 0/2] leds: Add support for CZ.NIC Turris 1.x LEDs

[PATCH v3 0/2] leds: Add support for CZ.NIC Turris 1.x LEDs

[Josef Schlehofer]

The CZ.NIC Turris 1.x routers have eight RGB LEDs on the front panel,
driven by the CZ.NIC CPLD firmware. arch/powerpc/boot/dts/turris1x.dts
already describes the LED controller, but there is no driver for it, so
Linux cannot control these LEDs.

Pali posted v1 and v2 in July 2022 [1][2] and a RESEND in December 2022
[3]. The RESEND did not address the review of v2, and Krzysztof NAKed
the binding [4] and objected to the driver [5] for that reason. Lee
asked Pavel whether he was happy with the user-space interface [6] and
later reviewed the driver [7]. Pali answered part of that review [8],
but no new version followed.

I am picking the series up. Most of the driver was reworked for v3;
authorship stays with Pali as the original author. The changelog of
each patch lists the changes and, for the review points that did not
lead to a change, the reason.

On the user-space interface: the global brightness follows the Turris
Omnia driver, which already has /sys/class/leds/<led>/device/brightness
[9]. The Turris 1.x uses the same attribute, documented in the same ABI
entry. Marek asked in the v1 review for the index of the selected level
[10], which is brightness_level, and Lee asked for one value per file
instead of the eight values in one brightness_values file [7], which
are now brightness_levels/<N>.

Marek, patch 2/2 extends the brightness entry in
Documentation/ABI/testing/sysfs-class-led-driver-turris-omnia and adds
that file to the new MAINTAINERS entry. Could you ack that part?

The series is based on leds/for-leds-next (05b4738b0078). The driver
implements hw_offloaded() for its private trigger and sets
hw_control_trigger, as the hardware control changes there require, so
it does not build on mainline yet.

Testing on a Turris 1.1:
- With OpenWrt's 6.18.44 kernel, the driver as it was before that
  adaptation, built as a module: colour and brightness, the brightness
  level attributes including invalid writes, the timer trigger, the
  turris1x-cpld hardware trigger, unbind and bind, and rmmod while
  triggers ran and a level file was held open, also watched at the
  panel. After a reboot, the CPLD registers read at the U-Boot prompt
  still had the WiFi LED's disable bit that the shutdown handler sets,
  and after sysrq-b, which skips the shutdown, the bit was clear. With
  linux,default-trigger set in the device tree, the driver took those
  LEDs over from the CPLD, and the turris1x-cpld trigger gave them back.
- With a kernel built from leds/for-leds-next and this series, the
  driver built in: the LEDs except the WiFi LED start under the
  turris1x-cpld trigger, which trigger_may_offload_to_hw reports as
  offloaded and under which reading brightness returns ENODATA. Writing
  a brightness drops the trigger, writing multi_intensity keeps it, the
  timer trigger works, and unbind and bind give the LEDs back to the
  CPLD. The WiFi LED has no trigger_may_offload_to_hw. The panel showed
  the same.
The lines were rewrapped at 100 columns after these runs, which changes
no object code (objdump -dr). Neither kernel had lock debugging.

dt_binding_check with dtschema 2026.9 is clean and rejects a bogus
property added to the example, dtbs_check reports no warning for the LED
controller in turris1x.dtb, and tools/docs/get_abi.py reports no new
warning. checkpatch --strict reports only the MAINTAINERS question on
1/2, answered in its changelog, and a macro argument reuse check on 2/2,
where the argument is always a literal. Built with W=1 and sparse on
leds/for-leds-next (05b4738b0078) for powerpc, as a module and built in
(vmlinux links), and with COMPILE_TEST for x86_64 and arm64 as a module.

[1] https://lore.kernel.org/r/20220705000448.14337-1-pali@kernel.org/
[2] https://lore.kernel.org/r/20220705155929.25565-1-pali@kernel.org/
[3] https://lore.kernel.org/r/20221226123630.6515-1-pali@kernel.org/
[4] https://lore.kernel.org/r/8b829332-5cd7-2910-88db-513716e0919a@kernel.org/
[5] https://lore.kernel.org/r/d8172c80-a3a0-07f6-97ea-9130c49fab18@kernel.org/
[6] https://lore.kernel.org/r/Y9Ozg2O41a2iijMc@google.com/
[7] https://lore.kernel.org/r/Y/iDVlodp9sBkX9D@google.com/
[8] https://lore.kernel.org/r/20230309203526.5hcfa2w47vqzmny6@pali/
[9] https://lore.kernel.org/r/20230202234653.ukwpjntws3roacty@pali/
[10] https://lore.kernel.org/r/20220705143001.7371a256@thinkpad/

Pali Rohár (2):
  dt-bindings: leds: Add CZ.NIC Turris 1.x LED controller
  leds: Add support for Turris 1.x LEDs

 .../sysfs-class-led-driver-turris-omnia       |  13 +-
 .../testing/sysfs-class-led-driver-turris1x   |  23 +
 .../bindings/leds/cznic,turris1x-leds.yaml    | 132 ++++
 MAINTAINERS                                   |  10 +
 drivers/leds/Kconfig                          |  13 +
 drivers/leds/Makefile                         |   1 +
 drivers/leds/leds-turris-1x.c                 | 609 ++++++++++++++++++
 7 files changed, 798 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-led-driver-turris1x
 create mode 100644 Documentation/devicetree/bindings/leds/cznic,turris1x-leds.yaml
 create mode 100644 drivers/leds/leds-turris-1x.c


base-commit: 05b4738b0078f7d6f154f68068a11c8a0635e9df
-- 
2.54.0 (Apple Git-157)

[PATCH v3 1/2] dt-bindings: leds: Add CZ.NIC Turris 1.x LED controller

[Josef Schlehofer]

From: Pali Rohár <pali@kernel.org>

Add a binding for the LED controller on the front panel of the CZ.NIC
Turris 1.x routers. The LEDs are controlled by the firmware of the
CZ.NIC CPLD, which is memory mapped on the local bus.

The turris1x.dts device tree, which is part of the mainline kernel since
v6.0, already references this compatible string.

Signed-off-by: Pali Rohár <pali@kernel.org>
Cc: Marek Behún <kabel@kernel.org>
Co-developed-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
---

Notes:
    Changes in v3, by Josef Schlehofer:
    - drop the file name and "binding" from the subject
    - describe the hardware rather than the driver
    - fix example indentation and use the merged turris1x.dts node
    - set Josef Schlehofer as the binding maintainer
    - constrain color to LED_COLOR_ID_RGB
    - list the required controller and LED properties
    - describe the register range and the LED order in the reg properties,
      and put the SPDX expression in parentheses
    - pass dt_binding_check with dtschema 2026.9
    
    checkpatch asks whether MAINTAINERS needs updating for the new file.
    The MAINTAINERS entry that covers it is added in 2/2, together with
    the driver.

 .../bindings/leds/cznic,turris1x-leds.yaml    | 132 ++++++++++++++++++
 1 file changed, 132 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/cznic,turris1x-leds.yaml

diff --git a/Documentation/devicetree/bindings/leds/cznic,turris1x-leds.yaml b/Documentation/devicetree/bindings/leds/cznic,turris1x-leds.yaml
new file mode 100644
index 000000000000..ad7ef7015f0b
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/cznic,turris1x-leds.yaml
@@ -0,0 +1,132 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/cznic,turris1x-leds.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: CZ.NIC Turris 1.x LED controller
+
+maintainers:
+  - Josef Schlehofer <pepe.schlehofer@gmail.com>
+
+description:
+  The front panel of the CZ.NIC Turris 1.x routers carries eight RGB LEDs
+  driven by the CPLD firmware running on a Lattice FPGA. The five LAN LEDs
+  share one set of colour registers, so they cannot be given different
+  colours or brightness from each other. The CPLD firmware is open source
+  and available at
+  https://gitlab.nic.cz/turris/hw/turris_cpld/-/blob/master/CZ_NIC_Router_CPLD.v
+
+properties:
+  compatible:
+    const: cznic,turris1x-leds
+
+  reg:
+    description:
+      CPLD address range of the LED registers. It starts at the first colour
+      register, offset 0x13 in the CPLD, and ends at 0x2f; the offsets of
+      the registers inside the range are fixed by the CPLD firmware.
+    maxItems: 1
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+patternProperties:
+  "^multi-led@[0-7]$":
+    type: object
+    $ref: leds-class-multicolor.yaml#
+    unevaluatedProperties: false
+
+    properties:
+      color:
+        const: 9  # LED_COLOR_ID_RGB
+
+      reg:
+        description:
+          Index of the LED, which is also its bit in the CPLD LED control
+          registers. 0 is WAN, 1 to 5 are LAN 5 to LAN 1, 6 is WiFi and 7 is
+          power. The CPLD source calls LEDs 1 to 5 lan1 to lan5 and the WiFi
+          LED status.
+        minimum: 0
+        maximum: 7
+
+    required:
+      - reg
+      - color
+
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/leds/common.h>
+
+    led-controller@13 {
+        compatible = "cznic,turris1x-leds";
+        reg = <0x13 0x1d>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        multi-led@0 {
+            reg = <0x0>;
+            color = <LED_COLOR_ID_RGB>;
+            function = LED_FUNCTION_WAN;
+        };
+
+        multi-led@1 {
+            reg = <0x1>;
+            color = <LED_COLOR_ID_RGB>;
+            function = LED_FUNCTION_LAN;
+            function-enumerator = <5>;
+        };
+
+        multi-led@2 {
+            reg = <0x2>;
+            color = <LED_COLOR_ID_RGB>;
+            function = LED_FUNCTION_LAN;
+            function-enumerator = <4>;
+        };
+
+        multi-led@3 {
+            reg = <0x3>;
+            color = <LED_COLOR_ID_RGB>;
+            function = LED_FUNCTION_LAN;
+            function-enumerator = <3>;
+        };
+
+        multi-led@4 {
+            reg = <0x4>;
+            color = <LED_COLOR_ID_RGB>;
+            function = LED_FUNCTION_LAN;
+            function-enumerator = <2>;
+        };
+
+        multi-led@5 {
+            reg = <0x5>;
+            color = <LED_COLOR_ID_RGB>;
+            function = LED_FUNCTION_LAN;
+            function-enumerator = <1>;
+        };
+
+        multi-led@6 {
+            reg = <0x6>;
+            color = <LED_COLOR_ID_RGB>;
+            function = LED_FUNCTION_WLAN;
+        };
+
+        multi-led@7 {
+            reg = <0x7>;
+            color = <LED_COLOR_ID_RGB>;
+            function = LED_FUNCTION_POWER;
+        };
+    };
+
+...
-- 
2.54.0 (Apple Git-157)

Re: [PATCH v3 1/2] dt-bindings: leds: Add CZ.NIC Turris 1.x LED controller

[sashiko-bot]

> From: Pali Rohár <pali@kernel.org>
> 
> Add a binding for the LED controller on the front panel of the CZ.NIC
> Turris 1.x routers. The LEDs are controlled by the firmware of the
> CZ.NIC CPLD, which is memory mapped on the local bus.
> 
> The turris1x.dts device tree, which is part of the mainline kernel since
> v6.0, already references this compatible string.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Cc: Marek Behún <kabel@kernel.org>
> Co-developed-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
> Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928111907.72592-1-pepe.schlehofer@gmail.com?part=1

Re: [PATCH v3 1/2] dt-bindings: leds: Add CZ.NIC Turris 1.x LED controller

[Rob Herring (Arm)]

On Mon, 28 Sep 2026 13:19:06 +0200, Josef Schlehofer wrote:
> From: Pali Rohár <pali@kernel.org>
> 
> Add a binding for the LED controller on the front panel of the CZ.NIC
> Turris 1.x routers. The LEDs are controlled by the firmware of the
> CZ.NIC CPLD, which is memory mapped on the local bus.
> 
> The turris1x.dts device tree, which is part of the mainline kernel since
> v6.0, already references this compatible string.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Cc: Marek Behún <kabel@kernel.org>
> Co-developed-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
> Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
> ---
> 
> Notes:
>     Changes in v3, by Josef Schlehofer:
>     - drop the file name and "binding" from the subject
>     - describe the hardware rather than the driver
>     - fix example indentation and use the merged turris1x.dts node
>     - set Josef Schlehofer as the binding maintainer
>     - constrain color to LED_COLOR_ID_RGB
>     - list the required controller and LED properties
>     - describe the register range and the LED order in the reg properties,
>       and put the SPDX expression in parentheses
>     - pass dt_binding_check with dtschema 2026.9
> 
>     checkpatch asks whether MAINTAINERS needs updating for the new file.
>     The MAINTAINERS entry that covers it is added in 2/2, together with
>     the driver.
> 
>  .../bindings/leds/cznic,turris1x-leds.yaml    | 132 ++++++++++++++++++
>  1 file changed, 132 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/cznic,turris1x-leds.yaml
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

[PATCH v3 2/2] leds: Add support for Turris 1.x LEDs

[Josef Schlehofer]

From: Pali Rohár <pali@kernel.org>

Add a driver for the eight RGB LEDs on the front panel of the CZ.NIC
Turris 1.x routers. They are driven by the CZ.NIC CPLD firmware, whose
source is available at
https://gitlab.nic.cz/turris/hw/turris_cpld/-/blob/master/CZ_NIC_Router_CPLD.v

The LEDs use the multicolor LED class. Every LED except the WiFi LED can
also be driven by the CPLD from hardware events, exposed as the private
turris1x-cpld trigger. The five LAN LEDs share one set of colour
registers, so the colour and brightness set last on any of them apply
to all five.

The controller device exposes the global brightness controlled by the
button on the back of the router through `brightness`,
`brightness_level`, and `brightness_levels/<N>`. The Turris Omnia
driver already has `brightness`, so its ABI entry is extended to cover
the Turris 1.x as well.

Signed-off-by: Pali Rohár <pali@kernel.org>
Cc: Marek Behún <kabel@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Co-developed-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
---

Notes:
    Changes in v3, by Josef Schlehofer:
    - use actual CPLD register addresses and common read/write helpers
    - replace literal LED counts with driver constants and simplify LED
      lookup
    - use property/fwnode helpers, sysfs_emit(), kstrtou8(), and set the
      Kconfig dependency to PPC_85xx || COMPILE_TEST
    - replace the mutex with a spinlock so that brightness_set() does not
      sleep
    - drop brightness_get(), which could only return 0 or 1
    - expose brightness levels as brightness_levels/<N> sysfs files and
      document brightness in the existing Turris Omnia ABI entry, with its
      own Date and KernelVersion for the Turris 1.x
    - scale RGB colour by max_brightness = 255, keep per-LED intensities,
      and clamp the colour components to the 8-bit registers
    - restore CPLD control on unbind and restore configured colours when
      enabling the hardware trigger
    - leave the software enable bit alone while the hardware trigger drives
      the LED
    - take an LED over from the CPLD when a DT default trigger replaces the
      hardware trigger
    - stop writing the LED registers once they are reset at shutdown
    - use .dev_groups instead of devm_device_add_groups() and
      MODULE_DEVICE_TABLE() instead of MODULE_ALIAS(), and add the
      MAINTAINERS entry, which also covers the Omnia ABI file
    - implement hw_offloaded() and set hw_control_trigger for the LEDs that
      have the turris1x-cpld trigger, as leds-turris-omnia does since the
      hardware control changes in leds-next
    - short error messages, e.g. "LED %u already registered", and lines
      wrapped at 100 columns (Lee)
    - drop the comma after the NULL terminators (Andy)
    - write U8_MAX instead of 0xff in the reset (Andy asked about
      GENMASK()): the value is the maximum of an 8-bit colour register,
      not a bit mask
    - mc_cdev.subled_info points to the per-LED subled_info array, it is
      not a second copy of it (Lee)
    - keep the static led_hw_trigger_type (Lee): it holds no data, the LED
      core only compares its address with cdev->trigger_type, and the
      static turris1x-cpld trigger refers to it in its initializer, as in
      leds-turris-omnia
    - keep the reset at shutdown rather than at boot (Lee): the LED
      registers survive a board reset, and the CPLD shows its reset pattern
      correctly only when they are back in the default state before the
      reset, which U-Boot would do too late (Pali's answer in [1])
    - drop Marek's Reviewed-by from v2, the driver was largely rewritten
    
    [1] https://lore.kernel.org/r/20230309203526.5hcfa2w47vqzmny6@pali/

 .../sysfs-class-led-driver-turris-omnia       |  13 +-
 .../testing/sysfs-class-led-driver-turris1x   |  23 +
 MAINTAINERS                                   |  10 +
 drivers/leds/Kconfig                          |  13 +
 drivers/leds/Makefile                         |   1 +
 drivers/leds/leds-turris-1x.c                 | 609 ++++++++++++++++++
 6 files changed, 666 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-led-driver-turris1x
 create mode 100644 drivers/leds/leds-turris-1x.c

diff --git a/Documentation/ABI/testing/sysfs-class-led-driver-turris-omnia b/Documentation/ABI/testing/sysfs-class-led-driver-turris-omnia
index 369b4ae8be5f..bc2eaf292b3f 100644
--- a/Documentation/ABI/testing/sysfs-class-led-driver-turris-omnia
+++ b/Documentation/ABI/testing/sysfs-class-led-driver-turris-omnia
@@ -1,7 +1,7 @@
 What:		/sys/class/leds/<led>/device/brightness
-Date:		July 2020
-KernelVersion:	5.9
-Contact:	Marek Behún <kabel@kernel.org>
+Date:		July 2020 (Turris Omnia), September 2026 (Turris 1.x)
+KernelVersion:	5.9 (Turris Omnia), 7.4 (Turris 1.x)
+Contact:	Marek Behún <kabel@kernel.org>, linux-leds@vger.kernel.org
 Description:	(RW) On the front panel of the Turris Omnia router there is also
 		a button which can be used to control the intensity of all the
 		LEDs at once, so that if they are too bright, user can dim them.
@@ -11,6 +11,13 @@ Description:	(RW) On the front panel of the Turris Omnia router there is also
 		integer value between 0 and 100. It is therefore convenient to be
 		able to change this setting from software.
 
+		The button on the back of Turris 1.x routers selects one of 8
+		brightness levels, whose values (0-255) are in
+		brightness_levels/<N>. Reading this file returns the value of
+		the level in use; writing it selects the level whose value is
+		closest to the written one. If two levels are equally close, the
+		lower level index is selected.
+
 		Format: %i
 
 What:		/sys/class/leds/<led>/device/gamma_correction
diff --git a/Documentation/ABI/testing/sysfs-class-led-driver-turris1x b/Documentation/ABI/testing/sysfs-class-led-driver-turris1x
new file mode 100644
index 000000000000..961d29dedafd
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-led-driver-turris1x
@@ -0,0 +1,23 @@
+What:		/sys/class/leds/<led>/device/brightness_level
+Date:		September 2026
+Contact:	Josef Schlehofer <pepe.schlehofer@gmail.com>
+Description:	(RW) Index (0-7) of the global brightness level in use on the
+		Turris 1.x routers. The button on the back side of the router
+		steps through the levels. Writing to this file selects a level
+		directly. The CPLD keeps the selection across driver unbind and
+		reboot.
+
+		Format: %u
+
+What:		/sys/class/leds/<led>/device/brightness_levels/<N>
+Date:		September 2026
+Contact:	Josef Schlehofer <pepe.schlehofer@gmail.com>
+Description:	(RW) Value of the global brightness level N (0-7) on the
+		Turris 1.x routers, one file per level. These are the values
+		the CPLD firmware steps through when the brightness button is
+		pressed. Reading returns the value, writing accepts an integer
+		between 0 and 255. The CPLD keeps the values across driver
+		unbind and reboot, and restores the defaults (255 64 32 16 8 4
+		2 0) at power-on and when the reset button is pressed.
+
+		Format: %u
diff --git a/MAINTAINERS b/MAINTAINERS
index a1eb0937238d..f3df66cbde05 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7163,6 +7163,16 @@ L:	linux-input@vger.kernel.org
 S:	Maintained
 F:	drivers/input/touchscreen/cyttsp*
 
+CZ.NIC TURRIS 1.X LED DRIVER
+M:	Josef Schlehofer <pepe.schlehofer@gmail.com>
+L:	linux-leds@vger.kernel.org
+S:	Maintained
+W:	https://www.turris.cz/
+F:	Documentation/ABI/testing/sysfs-class-led-driver-turris-omnia
+F:	Documentation/ABI/testing/sysfs-class-led-driver-turris1x
+F:	Documentation/devicetree/bindings/leds/cznic,turris1x-leds.yaml
+F:	drivers/leds/leds-turris-1x.c
+
 D-LINK DIR-685 TOUCHKEYS DRIVER
 M:	Linus Walleij <linusw@kernel.org>
 L:	linux-input@vger.kernel.org
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 7ec2c8d78542..bfcf19912900 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -227,6 +227,19 @@ config LEDS_EL15203000
 	  To compile this driver as a module, choose M here: the module
 	  will be called leds-el15203000.
 
+config LEDS_TURRIS_1X
+	tristate "LED support for CZ.NIC's Turris 1.x routers"
+	depends on LEDS_CLASS_MULTICOLOR
+	depends on PPC_85xx || COMPILE_TEST
+	select LEDS_TRIGGERS
+	help
+	  This option enables support for the eight RGB LEDs on the front
+	  panel of CZ.NIC's Turris 1.x routers. The LEDs are driven by the
+	  CPLD firmware, which can also drive them from hardware events.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called leds-turris-1x.
+
 config LEDS_TURRIS_OMNIA
 	tristate "LED support for CZ.NIC's Turris Omnia"
 	depends on LEDS_CLASS_MULTICOLOR
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 4d4b089156e2..1a5bddc3a3d6 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -96,6 +96,7 @@ obj-$(CONFIG_LEDS_TCA6507)		+= leds-tca6507.o
 obj-$(CONFIG_LEDS_TI_LMU_COMMON)	+= leds-ti-lmu-common.o
 obj-$(CONFIG_LEDS_TLC591XX)		+= leds-tlc591xx.o
 obj-$(CONFIG_LEDS_TPS6105X)		+= leds-tps6105x.o
+obj-$(CONFIG_LEDS_TURRIS_1X)		+= leds-turris-1x.o
 obj-$(CONFIG_LEDS_TURRIS_OMNIA)		+= leds-turris-omnia.o
 obj-$(CONFIG_LEDS_UPBOARD)		+= leds-upboard.o
 obj-$(CONFIG_LEDS_WM831X_STATUS)	+= leds-wm831x-status.o
diff --git a/drivers/leds/leds-turris-1x.c b/drivers/leds/leds-turris-1x.c
new file mode 100644
index 000000000000..491a0e70e4c0
--- /dev/null
+++ b/drivers/leds/leds-turris-1x.c
@@ -0,0 +1,609 @@
+// SPDX-License-Identifier: GPL-2.0
+// (C) 2022 Pali Rohár <pali@kernel.org>
+//
+// CZ.NIC's Turris 1.x LEDs driver, controlled by CPLD firmware:
+// https://gitlab.nic.cz/turris/hw/turris_cpld/-/blob/master/CZ_NIC_Router_CPLD.v
+
+#include <linux/bits.h>
+#include <linux/container_of.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kstrtox.h>
+#include <linux/led-class-multicolor.h>
+#include <linux/leds.h>
+#include <linux/limits.h>
+#include <linux/math.h>
+#include <linux/minmax.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/spinlock.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+
+/* Addresses in the CPLD memory map, the LED registers start at 0x13 */
+#define TURRIS1X_LED_REG_BASE			0x13
+#define TURRIS1X_LED_COLOR_REG			0x13
+#define TURRIS1X_LED_GLOBAL_LEVEL_REG		0x20
+#define TURRIS1X_LED_GLOBAL_BRIGHTNESS_REG	0x21
+#define TURRIS1X_LED_SW_OVERRIDE_REG		0x22
+/* Called led_sw_enable in the CPLD source, but a set bit turns the LED off */
+#define TURRIS1X_LED_SW_DISABLE_REG		0x23
+/* 0x28 + N holds the value of level N (level 7 - N in the CPLD source) */
+#define TURRIS1X_LED_LEVEL_VALUE_REG		0x28
+
+#define TURRIS1X_LED_NUM			8
+#define TURRIS1X_LED_NUM_COLORS			3
+#define TURRIS1X_LED_NUM_LEVELS			8
+#define TURRIS1X_LED_GLOBAL_LEVEL_MASK		GENMASK(2, 0)
+/* Blocks of colour registers: LED 0, LEDs 1-5 (shared), LED 6 and LED 7 */
+#define TURRIS1X_LED_NUM_COLOR_BLOCKS		4
+#define TURRIS1X_LED_WIFI			6
+
+struct turris1x_led {
+	struct led_classdev_mc mc_cdev;
+	struct mc_subled subled_info[TURRIS1X_LED_NUM_COLORS];
+	u32 reg;
+	bool registered;
+	/* Set when software has the LED on, i.e. its SW_DISABLE bit is clear */
+	bool on;
+	/* Set when the hardware trigger drives this LED */
+	bool hwtrig;
+};
+
+#define to_turris1x_led(cdev) \
+	container_of(lcdev_to_mccdev(cdev), struct turris1x_led, mc_cdev)
+
+struct turris1x_leds {
+	void __iomem *regs;
+	/* Protects the CPLD LED registers and @reset */
+	spinlock_t lock;
+	/* Set by turris1x_leds_reset(), after which the LEDs are left alone */
+	bool reset;
+	struct turris1x_led led[TURRIS1X_LED_NUM];
+};
+
+static struct led_hw_trigger_type turris1x_hw_trigger_type;
+
+static u8 turris1x_read(struct turris1x_leds *ddata, unsigned int reg)
+{
+	return readb(ddata->regs + reg - TURRIS1X_LED_REG_BASE);
+}
+
+static void turris1x_write(struct turris1x_leds *ddata, unsigned int reg, u8 val)
+{
+	writeb(val, ddata->regs + reg - TURRIS1X_LED_REG_BASE);
+}
+
+/* Block of three colour registers (red, green, blue) that each LED uses */
+static const u8 turris1x_color_block[TURRIS1X_LED_NUM] = {
+	0,		/* WAN */
+	1, 1, 1, 1, 1,	/* LAN 1-5 share one block */
+	2,		/* WiFi */
+	3,		/* power */
+};
+
+static unsigned int turris1x_color_reg(u32 led, unsigned int color)
+{
+	return TURRIS1X_LED_COLOR_REG + turris1x_color_block[led] * TURRIS1X_LED_NUM_COLORS + color;
+}
+
+/* Must be called with ddata->lock held */
+static void turris1x_led_set_colors(struct turris1x_leds *ddata, struct turris1x_led *led,
+				    enum led_brightness brightness)
+{
+	struct led_classdev_mc *mc_cdev = &led->mc_cdev;
+	unsigned int i;
+	u8 val;
+
+	led_mc_calc_color_components(mc_cdev, brightness);
+
+	/* The colour registers are 8 bits wide, do not let values wrap */
+	for (i = 0; i < TURRIS1X_LED_NUM_COLORS; i++) {
+		val = min_t(unsigned int, mc_cdev->subled_info[i].brightness, U8_MAX);
+		turris1x_write(ddata, turris1x_color_reg(led->reg, i), val);
+	}
+}
+
+static int turris1x_hwtrig_activate(struct led_classdev *cdev)
+{
+	struct turris1x_leds *ddata = dev_get_drvdata(cdev->dev->parent);
+	struct turris1x_led *led = to_turris1x_led(cdev);
+	unsigned long flags;
+	u8 val;
+
+	spin_lock_irqsave(&ddata->lock, flags);
+
+	if (ddata->reset)
+		goto unlock;
+
+	/*
+	 * If software turned the LED off, the last configured colour was not
+	 * necessarily written to the CPLD. Write it with max_brightness before
+	 * the hardware takes over.
+	 */
+	if (!led->on)
+		turris1x_led_set_colors(ddata, led, cdev->max_brightness);
+
+	/* Disable LED software control */
+	val = turris1x_read(ddata, TURRIS1X_LED_SW_OVERRIDE_REG);
+	turris1x_write(ddata, TURRIS1X_LED_SW_OVERRIDE_REG, val & ~BIT(led->reg));
+
+	led->hwtrig = true;
+
+unlock:
+	spin_unlock_irqrestore(&ddata->lock, flags);
+
+	return 0;
+}
+
+static void turris1x_hwtrig_deactivate(struct led_classdev *cdev)
+{
+	struct turris1x_leds *ddata = dev_get_drvdata(cdev->dev->parent);
+	struct turris1x_led *led = to_turris1x_led(cdev);
+	unsigned long flags;
+	u8 val;
+
+	spin_lock_irqsave(&ddata->lock, flags);
+
+	if (ddata->reset)
+		goto unlock;
+
+	led->hwtrig = false;
+
+	/*
+	 * Turn the LED off before software control takes over, as the LED core
+	 * does right after anyway.
+	 */
+	val = turris1x_read(ddata, TURRIS1X_LED_SW_DISABLE_REG);
+	turris1x_write(ddata, TURRIS1X_LED_SW_DISABLE_REG, val | BIT(led->reg));
+	led->on = false;
+
+	/* Enable LED software control */
+	val = turris1x_read(ddata, TURRIS1X_LED_SW_OVERRIDE_REG);
+	turris1x_write(ddata, TURRIS1X_LED_SW_OVERRIDE_REG, val | BIT(led->reg));
+
+unlock:
+	spin_unlock_irqrestore(&ddata->lock, flags);
+}
+
+static bool turris1x_hwtrig_hw_offloaded(struct led_classdev *cdev)
+{
+	return true;
+}
+
+static struct led_trigger turris1x_hw_trigger = {
+	.name		= "turris1x-cpld",
+	.activate	= turris1x_hwtrig_activate,
+	.deactivate	= turris1x_hwtrig_deactivate,
+	.hw_offloaded	= turris1x_hwtrig_hw_offloaded,
+	.trigger_type	= &turris1x_hw_trigger_type,
+};
+
+static void turris1x_led_brightness_set(struct led_classdev *cdev, enum led_brightness brightness)
+{
+	struct turris1x_leds *ddata = dev_get_drvdata(cdev->dev->parent);
+	struct turris1x_led *led = to_turris1x_led(cdev);
+	unsigned long flags;
+	u8 val;
+
+	spin_lock_irqsave(&ddata->lock, flags);
+
+	/* Software triggers run until the reboot, do not undo the reset */
+	if (ddata->reset)
+		goto unlock;
+
+	/*
+	 * Write the colours when the LED is on, and also when the hardware
+	 * trigger drives it, as the trigger uses the same registers.
+	 */
+	if (brightness || led->hwtrig)
+		turris1x_led_set_colors(ddata, led, brightness ?: cdev->max_brightness);
+
+	/*
+	 * Enable or disable the LED under software control. The CPLD ignores
+	 * this bit while the hardware trigger drives the LED, so leave it
+	 * alone.
+	 */
+	if (!led->hwtrig) {
+		val = turris1x_read(ddata, TURRIS1X_LED_SW_DISABLE_REG);
+		if (brightness)
+			val &= ~BIT(led->reg);
+		else
+			val |= BIT(led->reg);
+		turris1x_write(ddata, TURRIS1X_LED_SW_DISABLE_REG, val);
+		led->on = !!brightness;
+	}
+
+unlock:
+	spin_unlock_irqrestore(&ddata->lock, flags);
+}
+
+static int turris1x_led_register(struct device *dev, struct turris1x_leds *ddata,
+				 struct fwnode_handle *fwnode, u8 val_sw_override,
+				 u8 val_sw_disable)
+{
+	static const unsigned int colors[TURRIS1X_LED_NUM_COLORS] = {
+		LED_COLOR_ID_RED, LED_COLOR_ID_GREEN, LED_COLOR_ID_BLUE,
+	};
+	struct led_init_data init_data = {};
+	struct led_classdev *cdev;
+	struct turris1x_led *led;
+	unsigned long flags;
+	u8 val, dis;
+	u32 reg, color;
+	unsigned int i;
+	int ret;
+
+	ret = fwnode_property_read_u32(fwnode, "reg", &reg);
+	if (ret || reg >= TURRIS1X_LED_NUM)
+		return dev_err_probe(dev, -EINVAL, "Invalid or missing 'reg' property\n");
+
+	ret = fwnode_property_read_u32(fwnode, "color", &color);
+	if (ret || color != LED_COLOR_ID_RGB)
+		return dev_err_probe(dev, -EINVAL, "Invalid or missing 'color' property\n");
+
+	led = &ddata->led[reg];
+	if (led->registered)
+		return dev_err_probe(dev, -EINVAL, "LED %u already registered\n", reg);
+
+	led->reg = reg;
+
+	/* Set the initial colours to those currently in use */
+	for (i = 0; i < TURRIS1X_LED_NUM_COLORS; i++) {
+		led->subled_info[i].intensity = turris1x_read(ddata, turris1x_color_reg(reg, i));
+		led->subled_info[i].color_index = colors[i];
+		led->subled_info[i].channel = i;
+	}
+
+	/*
+	 * LEDs 1-5 (LAN) share one set of colour registers, and the brightness
+	 * of an LED is applied by scaling its colour, so all of them show the
+	 * colour and brightness written last. Each LED still keeps its own
+	 * intensities and brightness, so multi_intensity and brightness report
+	 * what the LED was last given, which is not necessarily what it shows.
+	 */
+	led->mc_cdev.subled_info = led->subled_info;
+	led->mc_cdev.num_colors = TURRIS1X_LED_NUM_COLORS;
+
+	init_data.fwnode = fwnode;
+
+	cdev = &led->mc_cdev.led_cdev;
+	cdev->max_brightness = 255;
+	cdev->brightness_set = turris1x_led_brightness_set;
+
+	/* All LEDs except the WiFi LED can be driven by the hardware trigger */
+	if (reg != TURRIS1X_LED_WIFI) {
+		cdev->trigger_type = &turris1x_hw_trigger_type;
+		cdev->hw_control_trigger = turris1x_hw_trigger.name;
+	}
+
+	if (!(val_sw_override & BIT(reg)))
+		cdev->default_trigger = turris1x_hw_trigger.name;
+
+	if (!(val_sw_override & BIT(reg)) || !(val_sw_disable & BIT(reg)))
+		cdev->brightness = cdev->max_brightness;
+
+	led->on = !(val_sw_disable & BIT(reg));
+
+	ret = devm_led_classdev_multicolor_register_ext(dev, &led->mc_cdev, &init_data);
+	if (ret)
+		return dev_err_probe(dev, ret, "Cannot register LED %u\n", reg);
+
+	/*
+	 * A linux,default-trigger property replaces the hardware trigger, and
+	 * the CPLD then keeps driving the LED and ignores software control.
+	 * Take such an LED over in the state the LED core reports.
+	 */
+	spin_lock_irqsave(&ddata->lock, flags);
+	val = turris1x_read(ddata, TURRIS1X_LED_SW_OVERRIDE_REG);
+	if (!led->hwtrig && !(val & BIT(reg))) {
+		dis = turris1x_read(ddata, TURRIS1X_LED_SW_DISABLE_REG);
+		if (cdev->brightness)
+			dis &= ~BIT(reg);
+		else
+			dis |= BIT(reg);
+		turris1x_write(ddata, TURRIS1X_LED_SW_DISABLE_REG, dis);
+		led->on = !!cdev->brightness;
+		turris1x_write(ddata, TURRIS1X_LED_SW_OVERRIDE_REG, val | BIT(reg));
+	}
+	spin_unlock_irqrestore(&ddata->lock, flags);
+
+	led->registered = true;
+
+	return 0;
+}
+
+static ssize_t brightness_show(struct device *dev, struct device_attribute *a, char *buf)
+{
+	struct turris1x_leds *ddata = dev_get_drvdata(dev);
+	u8 brightness;
+
+	/* The CPLD has the value of the level in use in a read-only register */
+	brightness = turris1x_read(ddata, TURRIS1X_LED_GLOBAL_BRIGHTNESS_REG);
+
+	return sysfs_emit(buf, "%u\n", brightness);
+}
+
+static ssize_t brightness_store(struct device *dev, struct device_attribute *a,
+				const char *buf, size_t count)
+{
+	struct turris1x_leds *ddata = dev_get_drvdata(dev);
+	int best_error, error, value;
+	unsigned int best_level, level;
+	unsigned long flags;
+	u8 brightness;
+	int ret;
+
+	ret = kstrtou8(buf, 10, &brightness);
+	if (ret)
+		return ret;
+
+	/*
+	 * The global brightness can only be one of the values of the levels.
+	 * Select the level whose value is nearest to the requested brightness.
+	 */
+	spin_lock_irqsave(&ddata->lock, flags);
+
+	best_level = 0;
+	best_error = INT_MAX;
+	for (level = 0; level < TURRIS1X_LED_NUM_LEVELS; level++) {
+		value = turris1x_read(ddata, TURRIS1X_LED_LEVEL_VALUE_REG + level);
+		error = abs(value - brightness);
+		if (error < best_error) {
+			best_error = error;
+			best_level = level;
+		}
+	}
+
+	turris1x_write(ddata, TURRIS1X_LED_GLOBAL_LEVEL_REG, best_level);
+
+	spin_unlock_irqrestore(&ddata->lock, flags);
+
+	return count;
+}
+static DEVICE_ATTR_RW(brightness);
+
+static ssize_t brightness_level_show(struct device *dev, struct device_attribute *a, char *buf)
+{
+	struct turris1x_leds *ddata = dev_get_drvdata(dev);
+	u8 level;
+
+	level = turris1x_read(ddata, TURRIS1X_LED_GLOBAL_LEVEL_REG);
+	level &= TURRIS1X_LED_GLOBAL_LEVEL_MASK;
+
+	return sysfs_emit(buf, "%u\n", level);
+}
+
+static ssize_t brightness_level_store(struct device *dev, struct device_attribute *a,
+				      const char *buf, size_t count)
+{
+	struct turris1x_leds *ddata = dev_get_drvdata(dev);
+	unsigned long flags;
+	u8 level;
+	int ret;
+
+	ret = kstrtou8(buf, 10, &level);
+	if (ret)
+		return ret;
+
+	if (level >= TURRIS1X_LED_NUM_LEVELS)
+		return -EINVAL;
+
+	spin_lock_irqsave(&ddata->lock, flags);
+	turris1x_write(ddata, TURRIS1X_LED_GLOBAL_LEVEL_REG, level);
+	spin_unlock_irqrestore(&ddata->lock, flags);
+
+	return count;
+}
+static DEVICE_ATTR_RW(brightness_level);
+
+/* One file per level under brightness_levels/, holding its value */
+struct turris1x_level_attr {
+	struct device_attribute attr;
+	u8 level;
+};
+
+#define to_turris1x_level_attr(a) \
+	container_of(a, struct turris1x_level_attr, attr)
+
+static ssize_t brightness_level_value_show(struct device *dev, struct device_attribute *a,
+					   char *buf)
+{
+	struct turris1x_leds *ddata = dev_get_drvdata(dev);
+	struct turris1x_level_attr *la = to_turris1x_level_attr(a);
+	u8 value;
+
+	value = turris1x_read(ddata, TURRIS1X_LED_LEVEL_VALUE_REG + la->level);
+
+	return sysfs_emit(buf, "%u\n", value);
+}
+
+static ssize_t brightness_level_value_store(struct device *dev, struct device_attribute *a,
+					    const char *buf, size_t count)
+{
+	struct turris1x_leds *ddata = dev_get_drvdata(dev);
+	struct turris1x_level_attr *la = to_turris1x_level_attr(a);
+	unsigned long flags;
+	u8 value;
+	int ret;
+
+	ret = kstrtou8(buf, 10, &value);
+	if (ret)
+		return ret;
+
+	spin_lock_irqsave(&ddata->lock, flags);
+	turris1x_write(ddata, TURRIS1X_LED_LEVEL_VALUE_REG + la->level, value);
+	spin_unlock_irqrestore(&ddata->lock, flags);
+
+	return count;
+}
+
+/* _level is always a literal 0..7, it is pasted, stringified and stored */
+#define TURRIS1X_LEVEL_ATTR(_level)						\
+	static struct turris1x_level_attr turris1x_level_attr_##_level = {	\
+		.attr = __ATTR(_level, 0644, brightness_level_value_show,	\
+			       brightness_level_value_store),			\
+		.level = _level,						\
+	}
+
+TURRIS1X_LEVEL_ATTR(0);
+TURRIS1X_LEVEL_ATTR(1);
+TURRIS1X_LEVEL_ATTR(2);
+TURRIS1X_LEVEL_ATTR(3);
+TURRIS1X_LEVEL_ATTR(4);
+TURRIS1X_LEVEL_ATTR(5);
+TURRIS1X_LEVEL_ATTR(6);
+TURRIS1X_LEVEL_ATTR(7);
+
+static struct attribute *turris1x_leds_levels_attrs[] = {
+	&turris1x_level_attr_0.attr.attr,
+	&turris1x_level_attr_1.attr.attr,
+	&turris1x_level_attr_2.attr.attr,
+	&turris1x_level_attr_3.attr.attr,
+	&turris1x_level_attr_4.attr.attr,
+	&turris1x_level_attr_5.attr.attr,
+	&turris1x_level_attr_6.attr.attr,
+	&turris1x_level_attr_7.attr.attr,
+	NULL
+};
+
+static const struct attribute_group turris1x_leds_levels_group = {
+	.name = "brightness_levels",
+	.attrs = turris1x_leds_levels_attrs,
+};
+
+static struct attribute *turris1x_leds_controller_attrs[] = {
+	&dev_attr_brightness.attr,
+	&dev_attr_brightness_level.attr,
+	NULL
+};
+
+static const struct attribute_group turris1x_leds_controller_group = {
+	.attrs = turris1x_leds_controller_attrs,
+};
+
+static const struct attribute_group *turris1x_leds_controller_groups[] = {
+	&turris1x_leds_controller_group,
+	&turris1x_leds_levels_group,
+	NULL
+};
+
+static void turris1x_leds_reset(void *data)
+{
+	struct turris1x_leds *ddata = data;
+	unsigned int reg, end;
+	unsigned long flags;
+	u8 val;
+
+	spin_lock_irqsave(&ddata->lock, flags);
+
+	ddata->reset = true;
+
+	/*
+	 * The LED registers persist across board resets and driver unbind, so
+	 * put the LED controller back into its default control state before
+	 * the kernel reboots and when the driver goes away.
+	 */
+
+	/* Disable software control of all LEDs except the WiFi LED */
+	turris1x_write(ddata, TURRIS1X_LED_SW_OVERRIDE_REG, BIT(TURRIS1X_LED_WIFI));
+
+	/* Turn off the WiFi LED, as there is no hardware trigger for it */
+	val = turris1x_read(ddata, TURRIS1X_LED_SW_DISABLE_REG);
+	turris1x_write(ddata, TURRIS1X_LED_SW_DISABLE_REG, val | BIT(TURRIS1X_LED_WIFI));
+
+	/* Reset the colours of all LEDs to full intensity */
+	end = TURRIS1X_LED_COLOR_REG + TURRIS1X_LED_NUM_COLOR_BLOCKS * TURRIS1X_LED_NUM_COLORS;
+	for (reg = TURRIS1X_LED_COLOR_REG; reg < end; reg++)
+		turris1x_write(ddata, reg, U8_MAX);
+
+	spin_unlock_irqrestore(&ddata->lock, flags);
+}
+
+static int turris1x_leds_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct turris1x_leds *ddata;
+	u8 val_sw_override, val_sw_disable;
+	unsigned int count = 0;
+	unsigned long flags;
+	int ret;
+
+	ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
+	if (!ddata)
+		return -ENOMEM;
+
+	ddata->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(ddata->regs))
+		return PTR_ERR(ddata->regs);
+
+	spin_lock_init(&ddata->lock);
+	platform_set_drvdata(pdev, ddata);
+
+	ret = devm_led_trigger_register(dev, &turris1x_hw_trigger);
+	if (ret)
+		return dev_err_probe(dev, ret, "Cannot register private LED trigger\n");
+
+	ret = devm_add_action_or_reset(dev, turris1x_leds_reset, ddata);
+	if (ret)
+		return ret;
+
+	spin_lock_irqsave(&ddata->lock, flags);
+
+	val_sw_override = turris1x_read(ddata, TURRIS1X_LED_SW_OVERRIDE_REG);
+	val_sw_disable = turris1x_read(ddata, TURRIS1X_LED_SW_DISABLE_REG);
+
+	/*
+	 * The WiFi LED has no hardware trigger, put it under software control
+	 * and turn it off.
+	 */
+	if (!(val_sw_override & BIT(TURRIS1X_LED_WIFI))) {
+		val_sw_disable |= BIT(TURRIS1X_LED_WIFI);
+		val_sw_override |= BIT(TURRIS1X_LED_WIFI);
+		turris1x_write(ddata, TURRIS1X_LED_SW_DISABLE_REG, val_sw_disable);
+		turris1x_write(ddata, TURRIS1X_LED_SW_OVERRIDE_REG, val_sw_override);
+	}
+
+	spin_unlock_irqrestore(&ddata->lock, flags);
+
+	device_for_each_child_node_scoped(dev, child) {
+		ret = turris1x_led_register(dev, ddata, child, val_sw_override, val_sw_disable);
+		if (ret)
+			return ret;
+		count++;
+	}
+
+	if (!count)
+		return dev_err_probe(dev, -ENODEV, "No LED devices found in device tree\n");
+
+	return 0;
+}
+
+static void turris1x_leds_shutdown(struct platform_device *pdev)
+{
+	turris1x_leds_reset(platform_get_drvdata(pdev));
+}
+
+static const struct of_device_id of_turris1x_leds_match[] = {
+	{ .compatible = "cznic,turris1x-leds" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, of_turris1x_leds_match);
+
+static struct platform_driver turris1x_leds_driver = {
+	.probe = turris1x_leds_probe,
+	.shutdown = turris1x_leds_shutdown,
+	.driver = {
+		.name = "turris1x_leds",
+		.of_match_table = of_turris1x_leds_match,
+		.dev_groups = turris1x_leds_controller_groups,
+	},
+};
+module_platform_driver(turris1x_leds_driver);
+
+MODULE_AUTHOR("Pali Rohár <pali@kernel.org>");
+MODULE_DESCRIPTION("CZ.NIC's Turris 1.x LEDs");
+MODULE_LICENSE("GPL");
-- 
2.54.0 (Apple Git-157)

Re: [PATCH v3 2/2] leds: Add support for Turris 1.x LEDs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] leds: commit message prefix omits driver name
--

commit 70e4f9934d8651d406ba12c71793199adbcac17c
Author: Pali Rohár <pali@kernel.org>

leds: Add support for Turris 1.x LEDs

This commit adds a driver for the eight RGB LEDs on the front panel of the
CZ.NIC Turris 1.x routers. The LEDs use the multicolor LED class and can be
driven by hardware events via a private CPLD trigger.

> leds: Add support for Turris 1.x LEDs

[Severity: Low]
Does this commit message prefix omit the required driver name? 

According to the LED subsystem naming conventions, the prefix format should
include the driver name before the capitalized description, for example:
"leds: <Driver>: <Capitalized description>".

Could this be updated to something like "leds: turris-1x: Add support for
Turris 1.x LEDs" to ensure consistent git history categorization?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928111907.72592-1-pepe.schlehofer@gmail.com?part=2

Re: [PATCH v3 2/2] leds: Add support for Turris 1.x LEDs

[Andy Shevchenko]

On Mon, Sep 28, 2026 at 2:19 PM Josef Schlehofer
<pepe.schlehofer@gmail.com> wrote:
>
> From: Pali Rohár <pali@kernel.org>
>
> Add a driver for the eight RGB LEDs on the front panel of the CZ.NIC
> Turris 1.x routers. They are driven by the CZ.NIC CPLD firmware, whose
> source is available at
> https://gitlab.nic.cz/turris/hw/turris_cpld/-/blob/master/CZ_NIC_Router_CPLD.v
>
> The LEDs use the multicolor LED class. Every LED except the WiFi LED can
> also be driven by the CPLD from hardware events, exposed as the private
> turris1x-cpld trigger. The five LAN LEDs share one set of colour
> registers, so the colour and brightness set last on any of them apply
> to all five.
>
> The controller device exposes the global brightness controlled by the
> button on the back of the router through `brightness`,
> `brightness_level`, and `brightness_levels/<N>`. The Turris Omnia
> driver already has `brightness`, so its ABI entry is extended to cover
> the Turris 1.x as well.

> Signed-off-by: Pali Rohár <pali@kernel.org>

> Cc: Marek Behún <kabel@kernel.org>
> Cc: Andy Shevchenko <andy@kernel.org>

Please, move these (Cc list) to the block under the cutter '---' line,
so it won't pollute the commit message.

> Co-developed-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
> Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
> ---

...

>  What:          /sys/class/leds/<led>/device/brightness
> -Date:          July 2020
> -KernelVersion: 5.9
> -Contact:       Marek Behún <kabel@kernel.org>

> +Date:          July 2020 (Turris Omnia), September 2026 (Turris 1.x)
> +KernelVersion: 5.9 (Turris Omnia), 7.4 (Turris 1.x)

This is quite unusual. If you want to refer to the kernel version, do
it in the description. Also note the version (see below more on it).

> +Contact:       Marek Behún <kabel@kernel.org>, linux-leds@vger.kernel.org

Why? The mailing list is kinda default, no?

...

> +What:          /sys/class/leds/<led>/device/brightness_level
> +Date:          September 2026

Impossible. Use https://hansen.beer/~dave/phb/ to predict the dates of
the next release, this is a huge driver that most likely may not make
v7.4, so the v7.5 is a plausible candidate.

> +Contact:       Josef Schlehofer <pepe.schlehofer@gmail.com>
> +Description:   (RW) Index (0-7) of the global brightness level in use on the
> +               Turris 1.x routers. The button on the back side of the router
> +               steps through the levels. Writing to this file selects a level
> +               directly. The CPLD keeps the selection across driver unbind and

the driver

> +               reboot.
> +
> +               Format: %u
> +
> +What:          /sys/class/leds/<led>/device/brightness_levels/<N>
> +Date:          September 2026

As per above

> +Contact:       Josef Schlehofer <pepe.schlehofer@gmail.com>
> +Description:   (RW) Value of the global brightness level N (0-7) on the
> +               Turris 1.x routers, one file per level. These are the values
> +               the CPLD firmware steps through when the brightness button is
> +               pressed. Reading returns the value, writing accepts an integer
> +               between 0 and 255. The CPLD keeps the values across driver

the driver

> +               unbind and reboot, and restores the defaults (255 64 32 16 8 4
> +               2 0) at power-on and when the reset button is pressed.

...

> +#include <linux/bits.h>
> +#include <linux/container_of.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kstrtox.h>
> +#include <linux/led-class-multicolor.h>
> +#include <linux/leds.h>
> +#include <linux/limits.h>
> +#include <linux/math.h>
> +#include <linux/minmax.h>

> +#include <linux/mod_devicetable.h>

Not anymore. Rely on what platform_device.h provides.

> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/spinlock.h>
> +#include <linux/sysfs.h>
> +#include <linux/types.h>

...

> +struct turris1x_leds {
> +       void __iomem *regs;
> +       /* Protects the CPLD LED registers and @reset */
> +       spinlock_t lock;
> +       /* Set by turris1x_leds_reset(), after which the LEDs are left alone */
> +       bool reset;
> +       struct turris1x_led led[TURRIS1X_LED_NUM];

Please, check the layout of all structures with `pahole`, it might
suggest a better one.

> +};

...

> +/* Must be called with ddata->lock held */

This is good, but having a lockdep annotation is even better.

> +static void turris1x_led_set_colors(struct turris1x_leds *ddata, struct turris1x_led *led,
> +                                   enum led_brightness brightness)
> +{
> +       struct led_classdev_mc *mc_cdev = &led->mc_cdev;
> +       unsigned int i;
> +       u8 val;
> +
> +       led_mc_calc_color_components(mc_cdev, brightness);
> +
> +       /* The colour registers are 8 bits wide, do not let values wrap */
> +       for (i = 0; i < TURRIS1X_LED_NUM_COLORS; i++) {

for (unsigned int i...) {

> +               val = min_t(unsigned int, mc_cdev->subled_info[i].brightness, U8_MAX);

No min_t(), it should be an exceptional use, and it was especially
proven to have issues with < INT_MAX comparisons in some cases. I
think you mean to have clamp() here.

> +               turris1x_write(ddata, turris1x_color_reg(led->reg, i), val);
> +       }
> +}

...

> +static int turris1x_hwtrig_activate(struct led_classdev *cdev)
> +{
> +       struct turris1x_leds *ddata = dev_get_drvdata(cdev->dev->parent);
> +       struct turris1x_led *led = to_turris1x_led(cdev);
> +       unsigned long flags;
> +       u8 val;
> +
> +       spin_lock_irqsave(&ddata->lock, flags);

Why not guard()()?

> +       if (ddata->reset)
> +               goto unlock;
> +
> +       /*
> +        * If software turned the LED off, the last configured colour was not
> +        * necessarily written to the CPLD. Write it with max_brightness before
> +        * the hardware takes over.
> +        */
> +       if (!led->on)
> +               turris1x_led_set_colors(ddata, led, cdev->max_brightness);
> +
> +       /* Disable LED software control */
> +       val = turris1x_read(ddata, TURRIS1X_LED_SW_OVERRIDE_REG);
> +       turris1x_write(ddata, TURRIS1X_LED_SW_OVERRIDE_REG, val & ~BIT(led->reg));
> +
> +       led->hwtrig = true;
> +
> +unlock:
> +       spin_unlock_irqrestore(&ddata->lock, flags);
> +
> +       return 0;
> +}

...

> +       /*
> +        * Enable or disable the LED under software control. The CPLD ignores
> +        * this bit while the hardware trigger drives the LED, so leave it
> +        * alone.
> +        */
> +       if (!led->hwtrig) {
> +               val = turris1x_read(ddata, TURRIS1X_LED_SW_DISABLE_REG);
> +               if (brightness)
> +                       val &= ~BIT(led->reg);
> +               else
> +                       val |= BIT(led->reg);

If led->reg is unsigned long, you can use __asign_bit() here.

> +               turris1x_write(ddata, TURRIS1X_LED_SW_DISABLE_REG, val);
> +               led->on = !!brightness;
> +       }

...

> +static int turris1x_led_register(struct device *dev, struct turris1x_leds *ddata,
> +                                struct fwnode_handle *fwnode, u8 val_sw_override,
> +                                u8 val_sw_disable)
> +{
> +       static const unsigned int colors[TURRIS1X_LED_NUM_COLORS] = {
> +               LED_COLOR_ID_RED, LED_COLOR_ID_GREEN, LED_COLOR_ID_BLUE,
> +       };
> +       struct led_init_data init_data = {};
> +       struct led_classdev *cdev;
> +       struct turris1x_led *led;
> +       unsigned long flags;
> +       u8 val, dis;
> +       u32 reg, color;
> +       unsigned int i;
> +       int ret;
> +
> +       ret = fwnode_property_read_u32(fwnode, "reg", &reg);
> +       if (ret || reg >= TURRIS1X_LED_NUM)
> +               return dev_err_probe(dev, -EINVAL, "Invalid or missing 'reg' property\n");
> +
> +       ret = fwnode_property_read_u32(fwnode, "color", &color);
> +       if (ret || color != LED_COLOR_ID_RGB)
> +               return dev_err_probe(dev, -EINVAL, "Invalid or missing 'color' property\n");

Do not shadow the error code.

> +       led = &ddata->led[reg];
> +       if (led->registered)
> +               return dev_err_probe(dev, -EINVAL, "LED %u already registered\n", reg);

EBUSY / EEXIST ?

> +       led->reg = reg;
> +
> +       /* Set the initial colours to those currently in use */
> +       for (i = 0; i < TURRIS1X_LED_NUM_COLORS; i++) {

for (unsigned int i...) {

> +               led->subled_info[i].intensity = turris1x_read(ddata, turris1x_color_reg(reg, i));
> +               led->subled_info[i].color_index = colors[i];
> +               led->subled_info[i].channel = i;
> +       }
> +
> +       /*
> +        * LEDs 1-5 (LAN) share one set of colour registers, and the brightness
> +        * of an LED is applied by scaling its colour, so all of them show the
> +        * colour and brightness written last. Each LED still keeps its own
> +        * intensities and brightness, so multi_intensity and brightness report
> +        * what the LED was last given, which is not necessarily what it shows.
> +        */
> +       led->mc_cdev.subled_info = led->subled_info;
> +       led->mc_cdev.num_colors = TURRIS1X_LED_NUM_COLORS;
> +
> +       init_data.fwnode = fwnode;
> +
> +       cdev = &led->mc_cdev.led_cdev;
> +       cdev->max_brightness = 255;
> +       cdev->brightness_set = turris1x_led_brightness_set;
> +
> +       /* All LEDs except the WiFi LED can be driven by the hardware trigger */
> +       if (reg != TURRIS1X_LED_WIFI) {
> +               cdev->trigger_type = &turris1x_hw_trigger_type;
> +               cdev->hw_control_trigger = turris1x_hw_trigger.name;
> +       }
> +
> +       if (!(val_sw_override & BIT(reg)))
> +               cdev->default_trigger = turris1x_hw_trigger.name;
> +
> +       if (!(val_sw_override & BIT(reg)) || !(val_sw_disable & BIT(reg)))
> +               cdev->brightness = cdev->max_brightness;
> +
> +       led->on = !(val_sw_disable & BIT(reg));
> +
> +       ret = devm_led_classdev_multicolor_register_ext(dev, &led->mc_cdev, &init_data);
> +       if (ret)
> +               return dev_err_probe(dev, ret, "Cannot register LED %u\n", reg);
> +
> +       /*
> +        * A linux,default-trigger property replaces the hardware trigger, and
> +        * the CPLD then keeps driving the LED and ignores software control.
> +        * Take such an LED over in the state the LED core reports.
> +        */
> +       spin_lock_irqsave(&ddata->lock, flags);
> +       val = turris1x_read(ddata, TURRIS1X_LED_SW_OVERRIDE_REG);
> +       if (!led->hwtrig && !(val & BIT(reg))) {
> +               dis = turris1x_read(ddata, TURRIS1X_LED_SW_DISABLE_REG);
> +               if (cdev->brightness)
> +                       dis &= ~BIT(reg);
> +               else
> +                       dis |= BIT(reg);
> +               turris1x_write(ddata, TURRIS1X_LED_SW_DISABLE_REG, dis);
> +               led->on = !!cdev->brightness;
> +               turris1x_write(ddata, TURRIS1X_LED_SW_OVERRIDE_REG, val | BIT(reg));
> +       }
> +       spin_unlock_irqrestore(&ddata->lock, flags);
> +
> +       led->registered = true;
> +
> +       return 0;
> +}

...

> +       best_level = 0;
> +       best_error = INT_MAX;
> +       for (level = 0; level < TURRIS1X_LED_NUM_LEVELS; level++) {

Ditto.

> +               value = turris1x_read(ddata, TURRIS1X_LED_LEVEL_VALUE_REG + level);
> +               error = abs(value - brightness);
> +               if (error < best_error) {
> +                       best_error = error;
> +                       best_level = level;
> +               }
> +       }

...

> +/* _level is always a literal 0..7, it is pasted, stringified and stored */
> +#define TURRIS1X_LEVEL_ATTR(_level)                                            \
> +       static struct turris1x_level_attr turris1x_level_attr_##_level = {      \
> +               .attr = __ATTR(_level, 0644, brightness_level_value_show,       \
> +                              brightness_level_value_store),                   \

Don't we have __ATTR_RW() ?

> +               .level = _level,                                                \
> +       }

...

> +       device_for_each_child_node_scoped(dev, child) {
> +               ret = turris1x_led_register(dev, ddata, child, val_sw_override, val_sw_disable);
> +               if (ret)
> +                       return ret;
> +               count++;
> +       }

> +       if (!count)
> +               return dev_err_probe(dev, -ENODEV, "No LED devices found in device tree\n");

We have counting API, so this can be done ahead. Yes, it will iterate
over the list twice, but I don't think it's an issue.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/2] leds: Add support for Turris 1.x LEDs

[Marek Behún]

On Mon, Sep 28, 2026 at 01:19:07PM +0200, Josef Schlehofer wrote:

> The five LAN LEDs share one set of colour
> registers, so the colour and brightness set last on any of them apply
> to all five.

...

> +static int turris1x_led_register(struct device *dev, struct turris1x_leds *ddata,
> +				 struct fwnode_handle *fwnode, u8 val_sw_override,

...

> +	cdev->max_brightness = 255;
> +	cdev->brightness_set = turris1x_led_brightness_set;

As described in the commit message, LAN LEDs form a group for which
the colors of individual LEDs cannot be changed.

Setting color for LAN3 LED to green will change colors for all
LAN1..5 LEDs to green.

This proposal does not reflect that in sysfs in any way.

My proposal is to register the first LAN LED as a true multicolor LED,
and the rest of LAN LEDs as simple LEDs, and then create sysfs
symlinks for the multi_intensity and multi_index attribute files:

  rgb:lan-1
    multi_intensity   (true attribute file)
    multi_index       (true attribute file)
  rgb:lan-2
    multi_intensity   (symlink to ../rgb:lan-1/multi_intensity)
    multi_index       (symlink to ../rgb:lan-1/multi_index)
  ...

This way the sysfs will somehow reflect this topology.

Also, the max_brigthness will need to be set to 1 instead of 255,
since we can set each LAN LED on/off state individually, but color
globally.

Marek

Re: [PATCH v3 2/2] leds: Add support for Turris 1.x LEDs

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1005 bytes --]

Hello,

On Mon, Sep 28, 2026 at 01:19:07PM +0200, Josef Schlehofer wrote:
> [...]
> +#include <linux/mod_devicetable.h>

Please don't include <linux/mod_devicetable.h>. This is a header that
pulls in a plethora of dependencies on subsystems you don't need in your
driver. So either rely on <linux/platform_device.h> to pull in the
definition of of_device_id (my preferred option) or include
<linux/device-id/of.h> if you want full iwyu.

> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> [...]
> +static void turris1x_leds_shutdown(struct platform_device *pdev)
> +{
> +	turris1x_leds_reset(platform_get_drvdata(pdev));

Is this needed to ensure a proper shutdown? If not I'd expect this
shouldn't be done.

> +}
> +
> +static const struct of_device_id of_turris1x_leds_match[] = {
> +	{ .compatible = "cznic,turris1x-leds" },
> +	{}

{ } please to match the most common style.

> +};
> +MODULE_DEVICE_TABLE(of, of_turris1x_leds_match);

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]