[PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Nam Tran]

This patch series adds support for the TI/National Semiconductor LP5812
4x3 matrix RGB LED driver. The driver supports features such as autonomous
animation and time-cross-multiplexing (TCM) for dynamic LED effects.

Signed-off-by: Nam Tran <trannamatk@gmail.com>
---
Changes in v8:
- Move driver to drivers/auxdisplay/ instead of drivers/leds/.
- Rename files from leds-lp5812.c/.h to lp5812.c/.h.
- Move ti,lp5812.yaml binding to auxdisplay/ directory,
  and update the title and $id to match new path.
- No functional changes to the binding itself (keep Reviewed-by).
- Update commit messages and patch titles to reflect the move.
- Link to v7: https://lore.kernel.org/linux-leds/20250422190121.46839-1-trannamatk@gmail.com/

Changes in v7:
- Mark `chip_leds_map` as const.
- Use consistent `ret` initialization.
- Simplify the function `set_mix_sel_led()`.
- Refactor `dev_config_show()` and `led_auto_animation_show()` to avoid temp buffer, malloc/free.
- Simplify the code and ensure consistent use of mutex lock/unlock in show/store functions.
- Remove `total_leds` and `total_aeu`.
- Link to v6: https://lore.kernel.org/linux-leds/20250419184333.56617-1-trannamatk@gmail.com/

Changes in v6:
- Add `vcc-supply` property to describe the LP5812 power supply.
- Remove `chan-name` property and entire LED subnodes, as they are not needed.
- Update LP5812 LED driver node to Raspberry Pi 4 B Device Tree, based on updated binding.
- Link to v5: https://lore.kernel.org/linux-leds/20250414145742.35713-1-trannamatk@gmail.com/

Changes in v5:
- Rebase on v6.15-rc2
- Removed unused functions (lp5812_dump_regs, lp5812_update_bit).
- Address Krzysztof's review comments
- Link to v4: https://lore.kernel.org/linux-leds/20250405183246.198568-1-trannamatk@gmail.com/
---

Nam Tran (5):
  dt-bindings: auxdisplay: add TI LP5812 4×3 Matrix RGB LED Driver
  auxdisplay: add TI LP5812 4×3 Matrix RGB LED Driver
  docs: ABI: Document LP5812 sysfs interfaces
  docs: auxdisplay: document TI LP5812 RGB LED driver
  arm64: dts: Add LP5812 node for Raspberry Pi 4 Model B

 .../ABI/testing/sysfs-bus-i2c-devices-lp5812  |  144 +
 .../admin-guide/auxdisplay/index.rst          |    3 +-
 .../admin-guide/auxdisplay/lp5812.rst         |   79 +
 .../bindings/auxdisplay/ti,lp5812.yaml        |   46 +
 MAINTAINERS                                   |   12 +
 .../arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts |   10 +
 drivers/auxdisplay/Kconfig                    |   17 +
 drivers/auxdisplay/Makefile                   |    1 +
 drivers/auxdisplay/lp5812.c                   | 2736 +++++++++++++++++
 drivers/auxdisplay/lp5812.h                   |  348 +++
 10 files changed, 3395 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c-devices-lp5812
 create mode 100644 Documentation/admin-guide/auxdisplay/lp5812.rst
 create mode 100644 Documentation/devicetree/bindings/auxdisplay/ti,lp5812.yaml
 create mode 100644 drivers/auxdisplay/lp5812.c
 create mode 100644 drivers/auxdisplay/lp5812.h


base-commit: f1a3944c860b0615d0513110d8cf62bb94adbb41
-- 
2.25.1

[PATCH v8 1/5] dt-bindings: auxdisplay: add TI LP5812 4×3 Matrix RGB LED Driver

[Nam Tran]

The LP5812 is a 4×3 RGB LED driver with an autonomous animation engine
and time-cross-multiplexing (TCM) support for up to 12 LEDs.
It supports both analog (256 levels) and PWM (8-bit) dimming,
including exponential PWM for smooth brightness control.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Nam Tran <trannamatk@gmail.com>
---
 .../bindings/auxdisplay/ti,lp5812.yaml        | 46 +++++++++++++++++++
 MAINTAINERS                                   |  6 +++
 2 files changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/auxdisplay/ti,lp5812.yaml

diff --git a/Documentation/devicetree/bindings/auxdisplay/ti,lp5812.yaml b/Documentation/devicetree/bindings/auxdisplay/ti,lp5812.yaml
new file mode 100644
index 000000000000..fb94a6336e16
--- /dev/null
+++ b/Documentation/devicetree/bindings/auxdisplay/ti,lp5812.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/auxdisplay/ti,lp5812.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI LP5812 4×3 Matrix RGB LED Driver with Autonomous Control
+
+maintainers:
+  - Nam Tran <trannamatk@gmail.com>
+
+description: |
+  The LP5812 is an I2C LED Driver that can support LED matrix 4x3.
+  For more product information please see the link below:
+  https://www.ti.com/product/LP5812#tech-docs
+
+properties:
+  compatible:
+    const: ti,lp5812
+
+  reg:
+    maxItems: 1
+
+  vcc-supply:
+    description: Regulator providing power to the 'VCC' pin.
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        led-controller@1b {
+            compatible = "ti,lp5812";
+            reg = <0x1b>;
+            vcc-supply = <&vdd_3v3_reg>;
+        };
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 7940ddd91196..241c5441e239 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -24016,6 +24016,12 @@ S:	Supported
 F:	Documentation/devicetree/bindings/iio/dac/ti,dac7612.yaml
 F:	drivers/iio/dac/ti-dac7612.c
 
+TEXAS INSTRUMENTS' LP5812 4X3 MATRIX LED DRIVER
+M:	Nam Tran <trannamatk@gmail.com>
+L:	linux-kernel@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/auxdisplay/ti,lp5812.yaml
+
 TEXAS INSTRUMENTS' LB8864 LED BACKLIGHT DRIVER
 M:	Alexander Sverdlin <alexander.sverdlin@siemens.com>
 L:	linux-leds@vger.kernel.org
-- 
2.25.1

[PATCH v8 2/5] auxdisplay: add TI LP5812 4×3 Matrix RGB LED Driver

[Nam Tran]

The LP5812 is a 4×3 matrix RGB LED driver
with an autonomous animation engine
and time-cross-multiplexing (TCM) support for up to 12 LEDs.
Each LED can be configured through the related registers
to realize vivid and fancy lighting effects.

Signed-off-by: Nam Tran <trannamatk@gmail.com>
---
 MAINTAINERS                 |    4 +
 drivers/auxdisplay/Kconfig  |   17 +
 drivers/auxdisplay/Makefile |    1 +
 drivers/auxdisplay/lp5812.c | 2736 +++++++++++++++++++++++++++++++++++
 drivers/auxdisplay/lp5812.h |  348 +++++
 5 files changed, 3106 insertions(+)
 create mode 100644 drivers/auxdisplay/lp5812.c
 create mode 100644 drivers/auxdisplay/lp5812.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 241c5441e239..091373b01dc2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -24021,6 +24021,10 @@ M:	Nam Tran <trannamatk@gmail.com>
 L:	linux-kernel@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/auxdisplay/ti,lp5812.yaml
+F:	drivers/auxdisplay/Kconfig
+F:	drivers/auxdisplay/Makefile
+F:	drivers/auxdisplay/lp5812.c
+F:	drivers/auxdisplay/lp5812.h
 
 TEXAS INSTRUMENTS' LB8864 LED BACKLIGHT DRIVER
 M:	Alexander Sverdlin <alexander.sverdlin@siemens.com>
diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
index bedc6133f970..896f02b9a06b 100644
--- a/drivers/auxdisplay/Kconfig
+++ b/drivers/auxdisplay/Kconfig
@@ -539,6 +539,23 @@ config ARM_CHARLCD
 	  line and the Linux version on the second line, but that's
 	  still useful.
 
+#
+# TI LP5812 matrix RGB LED driver section
+#
+config LP5812
+tristate "Enable LP5812 support LED matrix 4x3"
+depends on I2C
+help
+  If you say Y here you get support for TI LP5812 LED driver.
+
+  The LP5812 is a 4 × 3 matrix RGB LED driver with autonomous
+  animation engine control.
+
+  To compile this driver as a module, choose M here: the
+  module will be called lp5812.
+
+  If unsure, say N.
+
 endif # AUXDISPLAY
 
 #
diff --git a/drivers/auxdisplay/Makefile b/drivers/auxdisplay/Makefile
index f5c13ed1cd4f..36b9bb5cb1cf 100644
--- a/drivers/auxdisplay/Makefile
+++ b/drivers/auxdisplay/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_IMG_ASCII_LCD)	+= img-ascii-lcd.o
 obj-$(CONFIG_KS0108)		+= ks0108.o
 obj-$(CONFIG_LCD2S)		+= lcd2s.o
 obj-$(CONFIG_LINEDISP)		+= line-display.o
+obj-$(CONFIG_LP5812)		+= lp5812.o
 obj-$(CONFIG_MAX6959)		+= max6959.o
 obj-$(CONFIG_PARPORT_PANEL)	+= panel.o
 obj-$(CONFIG_SEG_LED_GPIO)	+= seg-led-gpio.o
diff --git a/drivers/auxdisplay/lp5812.c b/drivers/auxdisplay/lp5812.c
new file mode 100644
index 000000000000..8491939cf395
--- /dev/null
+++ b/drivers/auxdisplay/lp5812.c
@@ -0,0 +1,2736 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * LP5812 LED driver
+ *
+ * Copyright (C) 2025 Texas Instruments
+ *
+ * Author: Jared Zhou <jared-zhou@ti.com>
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/mod_devicetable.h>
+#include <linux/string.h>
+
+#include "lp5812.h"
+
+#define to_lp5812_led(x) container_of(x, struct lp5812_led, kobj)
+#define to_anim_engine_unit(x) container_of(x, struct anim_engine_unit, kobj)
+
+static int lp5812_init_dev_config(struct lp5812_chip *chip,
+		const char *drive_mode, int rm_led_sysfs);
+
+static const struct drive_mode_led_map chip_leds_map[] = {
+	{
+		"direct_mode",
+		(const char *[]){LED0, LED1, LED2, LED3, NULL}
+	},
+	{
+		"tcmscan:1:0", /* tcm 1 scan; scan order 0 out0 */
+		(const char *[]){LED_A0, LED_A1, LED_A2, NULL}
+	},
+	{
+		"tcmscan:1:1", /* tcm 1 scan; scan order 0 out1 */
+		(const char *[]){LED_B0, LED_B1, LED_B2, NULL}
+	},
+	{
+		"tcmscan:1:2", /* tcm 1 scan; scan order 0 out2 */
+		(const char *[]){LED_C0, LED_C1, LED_C2, NULL}
+	},
+	{
+		"tcmscan:1:3", /* tcm 1 scan; scan order 0 out3 */
+		(const char *[]){LED_D0, LED_D1, LED_D2, NULL}
+	},
+	{ /* tcm 2 scan, scan order 0 out0; scan order 1 out1 */
+		"tcmscan:2:0:1",
+		(const char *[]){LED_A0, LED_A1, LED_A2, LED_B0, LED_B1, LED_B2, NULL}
+	},
+	{ /* tcm 2 scan, scan order 0 out0; scan order 1 out2 */
+		"tcmscan:2:0:2",
+		(const char *[]){LED_A0, LED_A1, LED_A2, LED_C0, LED_C1, LED_C2, NULL}
+	},
+	{ /* tcm 2 scan, scan order 0 out0; scan order 1 out3 */
+		"tcmscan:2:0:3",
+		(const char *[]){LED_A0, LED_A1, LED_A2, LED_D0, LED_D1, LED_D2, NULL}
+	},
+	{ /* tcm 2 scan, scan order 0 out1; scan order 1 out2 */
+		"tcmscan:2:1:2",
+		(const char *[]){LED_B0, LED_B1, LED_B2, LED_C0, LED_C1, LED_C2, NULL}
+	},
+	{ /* tcm 2 scan, scan order 0 out1; scan order 1 out3 */
+		"tcmscan:2:1:3",
+		(const char *[]){LED_B0, LED_B1, LED_B2, LED_D0, LED_D1, LED_D2, NULL}
+	},
+	{ /* tcm 2 scan, scan order 0 out2; scan order 1 out3 */
+		"tcmscan:2:2:3",
+		(const char *[]){LED_C0, LED_C1, LED_C2, LED_D0, LED_D1, LED_D2, NULL}
+	},
+	{ /* tcm 3 scan, scan order 0 out0; scan order 1 out1;
+	   * scan order 2 out2
+	   */
+		"tcmscan:3:0:1:2",
+		(const char *[]){LED_A0, LED_A1, LED_A2, LED_B0, LED_B1, LED_B2,
+		LED_C0, LED_C1, LED_C2, NULL}
+	},
+	{ /* tcm 3 scan, scan order 0 out0; scan order 1 out1;
+	   * scan order 2 out3
+	   */
+		"tcmscan:3:0:1:3",
+		(const char *[]){LED_A0, LED_A1, LED_A2, LED_B0, LED_B1, LED_B2,
+		LED_D0, LED_D1, LED_D2, NULL}
+	},
+	{ /* tcm 3 scan, scan order 0 out0; scan order 1 out2;
+	   * scan order 2 out3
+	   */
+		"tcmscan:3:0:2:3",
+		(const char *[]){LED_A0, LED_A1, LED_A2, LED_C0, LED_C1, LED_C2,
+		LED_D0, LED_D1, LED_D2, NULL}
+	},
+	{ /* tcm 4 scan, scan order 0 out0; scan order 1 out1;
+	   * scan order 2 out2; scan order 3 out3
+	   */
+		"tcmscan:4:0:1:2:3",
+		(const char *[]){LED_A0, LED_A1, LED_A2, LED_B0, LED_B1, LED_B2,
+		LED_C0, LED_C1, LED_C2, LED_D0, LED_D1, LED_D2, NULL}
+	},
+	{ /* mix 1 scan, direct connect out0; scan order 0 out1 */
+		"mixscan:1:0:1",
+		(const char *[]){LED0, LED_B0, LED_B1, NULL}
+	},
+	{ /* mix 1 scan, direct connect out0; scan order 0 out2 */
+		"mixscan:1:0:2",
+		(const char *[]){LED0, LED_C0, LED_C2, NULL}
+	},
+	{ /* mix 1 scan, direct connect out0; scan order 0 out3 */
+		"mixscan:1:0:3",
+		(const char *[]){LED0, LED_D1, LED_D2, NULL}
+	},
+	{ /* mix 1 scan, direct connect out1; scan order 0 out0 */
+		"mixscan:1:1:0",
+		(const char *[]){LED1, LED_A1, LED_A2, NULL}
+	},
+	{ /* mix 1 scan, direct connect out1; scan order 0 out2 */
+		"mixscan:1:1:2",
+		(const char *[]){LED1, LED_C0, LED_C1, NULL}
+	},
+	{ /* mix 1 scan, direct connect out1; scan order 0 out3 */
+		"mixscan:1:1:3",
+		(const char *[]){LED1, LED_D0, LED_D2, NULL}
+	},
+	{ /* mix 1 scan, direct connect out2; scan order 0 out0 */
+		"mixscan:1:2:0",
+		(const char *[]){LED2, LED_A0, LED_A2, NULL}
+	},
+	{ /* mix 1 scan, direct connect out2; scan order 0 out1 */
+		"mixscan:1:2:1",
+		(const char *[]){LED2, LED_B1, LED_B2, NULL}
+	},
+	{ /* mix 1 scan, direct connect out2; scan order 0 out3 */
+		"mixscan:1:2:3",
+		(const char *[]){LED2, LED_D0, LED_D1, NULL}
+	},
+	{ /* mix 1 scan, direct connect out3; scan order 0 out0 */
+		"mixscan:1:3:0",
+		(const char *[]){LED3, LED_A0, LED_A1, NULL}
+	},
+	{ /* mix 1 scan, direct connect out3; scan order 0 out1 */
+		"mixscan:1:3:1",
+		(const char *[]){LED3, LED_B0, LED_B2, NULL}
+	},
+	{ /* mix 1 scan, direct connect out3; scan order 0 out2 */
+		"mixscan:1:3:2",
+		(const char *[]){LED3, LED_C1, LED_C2, NULL}
+	},
+	{ /* mix 2 scan, direct connect out0; scan order 0 out1;
+	   * scan order 1 out2
+	   */
+		"mixscan:2:0:1:2",
+		(const char *[]){LED0, LED_B0, LED_B1, LED_C0, LED_C2, NULL}
+	},
+	{ /* mix 2 scan, direct connect out0; scan order 0 out1;
+	   * scan order 1 out3
+	   */
+		"mixscan:2:0:1:3",
+		(const char *[]){LED0, LED_B0, LED_B1, LED_D1, LED_D2, NULL}
+	},
+	{ /* mix 2 scan, direct connect out0; scan order 0 out2;
+	   * scan order 1 out3
+	   */
+		"mixscan:2:0:2:3",
+		(const char *[]){LED0, LED_C0, LED_C2, LED_D1, LED_D2, NULL}
+	},
+	{ /* mix 2 scan, direct connect out1; scan order 0 out0;
+	   * scan order 1 out2
+	   */
+		"mixscan:2:1:0:2",
+		(const char *[]){LED1, LED_A1, LED_A2, LED_C0, LED_C1, NULL}
+	},
+	{ /* mix 2 scan, direct connect out1; scan order 0 out0;
+	   * scan order 1 out3
+	   */
+		"mixscan:2:1:0:3",
+		(const char *[]){LED1, LED_A1, LED_A2, LED_D0, LED_D2, NULL}
+	},
+	{ /* mix 2 scan, direct connect out1; scan order 0 out2;
+	   * scan order 1 out3
+	   */
+		"mixscan:2:1:2:3",
+		(const char *[]){LED1, LED_C0, LED_C1, LED_D0, LED_D2, NULL}
+	},
+	{ /* mix 2 scan, direct connect out2; scan order 0 out0;
+	   * scan order 1 out1
+	   */
+		"mixscan:2:2:0:1",
+		(const char *[]){LED2, LED_A0, LED_A2, LED_B1, LED_B2, NULL}
+	},
+	{ /* mix 2 scan, direct connect out2; scan order 0 out0;
+	   * scan order 1 out3
+	   */
+		"mixscan:2:2:0:3",
+		(const char *[]){LED2, LED_A0, LED_A2, LED_D0, LED_D1, NULL}
+	},
+	{ /* mix 2 scan, direct connect out2; scan order 0 out1;
+	   * scan order 1 out3
+	   */
+		"mixscan:2:2:1:3",
+		(const char *[]){LED2, LED_B1, LED_B2, LED_D0, LED_D1, NULL}
+	},
+	{ /* mix 2 scan, direct connect out3; scan order 0 out0;
+	   * scan order 1 out1
+	   */
+		"mixscan:2:3:0:1",
+		(const char *[]){LED3, LED_A0, LED_A1, LED_B0, LED_B2, NULL}
+	},
+	{ /* mix 2 scan, direct connect out3; scan order 0 out0;
+	   * scan order 1 out2
+	   */
+		"mixscan:2:3:0:2",
+		(const char *[]){LED3, LED_A0, LED_A1, LED_C1, LED_C2, NULL}
+	},
+	{ /* mix 2 scan, direct connect out3; scan order 0 out1;
+	   * scan order 1 out2
+	   */
+		"mixscan:2:3:1:2",
+		(const char *[]){LED3, LED_B0, LED_B2, LED_C1, LED_C2, NULL}
+	},
+	{ /* mix 3 scan, direct connect out0; scan order 0 out1;
+	   * scan order 1 out2; scan order 2 out3
+	   */
+		"mixscan:3:0:1:2:3",
+		(const char *[]){LED0, LED_B0, LED_B1, LED_C0, LED_C2, LED_D1,
+		LED_D2, NULL}
+	},
+	{ /* mix 3 scan, direct connect out1; scan order 0 out0;
+	   * scan order 1 out2; scan order 2 out3
+	   */
+		"mixscan:3:1:0:2:3",
+		(const char *[]){LED1, LED_A1, LED_A2, LED_C0, LED_C1, LED_D0,
+		LED_D2, NULL}
+	},
+	{ /* mix 3 scan, direct connect out2; scan order 0 out0;
+	   * scan order 1 out1; scan order 2 out3
+	   */
+		"mixscan:3:2:0:1:3",
+		(const char *[]){LED2, LED_A0, LED_A2, LED_B1, LED_B2, LED_D0,
+		LED_D1, NULL}
+	},
+	{ /* mix 3 scan, direct connect out3; scan order 0 out0;
+	   * scan order 1 out1; scan order 2 out2
+	   */
+		"mixscan:3:3:0:1:2",
+		(const char *[]){LED3, LED_A0, LED_A1, LED_B0, LED_B2, LED_C1,
+		LED_C2, NULL}
+	}
+};
+
+static const char *led_name_array[MAX_LEDS] = {
+	LED0, LED1, LED2, LED3, LED_A0, LED_A1, LED_A2, LED_B0, LED_B1,
+	LED_B2, LED_C0, LED_C1, LED_C2, LED_D0, LED_D1, LED_D2
+};
+
+static u16 anim_base_addr_array[MAX_LEDS] = {
+	LED0_AUTO_BASE_ADRR, LED1_AUTO_BASE_ADRR, LED2_AUTO_BASE_ADRR,
+	LED3_AUTO_BASE_ADRR, LED_A0_AUTO_BASE_ADRR, LED_A1_AUTO_BASE_ADRR,
+	LED_A2_AUTO_BASE_ADRR, LED_B0_AUTO_BASE_ADRR, LED_B1_AUTO_BASE_ADRR,
+	LED_B2_AUTO_BASE_ADRR, LED_C0_AUTO_BASE_ADRR, LED_C1_AUTO_BASE_ADRR,
+	LED_C2_AUTO_BASE_ADRR, LED_D0_AUTO_BASE_ADRR, LED_D1_AUTO_BASE_ADRR,
+	LED_D2_AUTO_BASE_ADRR
+};
+
+static const char *time_name_array[MAX_TIME] = {
+	TIME0, TIME1, TIME2, TIME3, TIME4, TIME5, TIME6, TIME7, TIME8,
+	TIME9, TIME10, TIME11, TIME12, TIME13, TIME14, TIME15
+};
+
+static const char *led_playback_time_arr[MAX_TIME] = {
+	"0 time", "1 time", "2 times", "3 times", "4 times", "5 times",
+	"6 times", "7 times", "8 times", "9 times", "10 times", "11 times",
+	"12 times", "13 times", "14 times", "infinite times"
+};
+
+static const char *aeu_name_array[MAX_AEU] = {AEU1, AEU2, AEU3};
+
+static const struct lp5812_specific_regs regs = {
+	.enable_reg            = CHIP_EN_REG,
+	.reset_reg             = RESET_REG,
+	.update_cmd_reg        = CMD_UPDATE_REG,
+	.start_cmd_reg         = CMD_START_REG,
+	.stop_cmd_reg          = CMD_STOP_REG,
+	.pause_cmd_reg         = CMD_PAUSE_REG,
+	.continue_cmd_reg      = CMD_CONTINUE_REG,
+	.fault_clear_reg       = FAULT_CLEAR_REG,
+	.tsd_config_status_reg = TSD_CONFIG_STAT_REG,
+};
+
+static void led_kobj_release(struct kobject *kobj)
+{
+	kfree(kobj);
+}
+
+static void aeu_kobj_release(struct kobject *kobj)
+{
+	kfree(kobj);
+}
+
+static const struct kobj_type led_ktype = {
+	.release = led_kobj_release,
+	.sysfs_ops = &kobj_sysfs_ops,
+};
+
+static const struct kobj_type aeu_ktype = {
+	.release = aeu_kobj_release,
+	.sysfs_ops = &kobj_sysfs_ops,
+};
+
+static int lp5812_write(struct lp5812_chip *chip, u16 reg, u8 val)
+{
+	int ret;
+	u8 extracted_bits; /* save 9th and 8th bit of reg address */
+	struct i2c_msg msg;
+	u8 buf[2] = {(u8)(reg & 0xFF), val};
+
+	extracted_bits = (reg >> 8) & 0x03;
+	msg.addr = (chip->i2c_cl->addr << 2) | extracted_bits;
+	msg.flags = 0;
+	msg.len = sizeof(buf);
+	msg.buf = buf;
+
+	ret = i2c_transfer(chip->i2c_cl->adapter, &msg, 1);
+	if (ret != 1) {
+		dev_err(chip->dev, "i2c write error, register 0x%02X, ret=%d\n", reg, ret);
+		ret = ret < 0 ? ret : -EIO;
+	} else {
+		ret = 0;
+	}
+
+	return ret;
+}
+
+static int lp5812_read(struct lp5812_chip *chip, u16 reg, u8 *val)
+{
+	int ret;
+	u8 ret_val;  /* lp5812_chip return value */
+	u8 extracted_bits; /* save 9th and 8th bit of reg address */
+	u8 converted_reg;  /* extracted 8bit from reg */
+	struct i2c_msg msgs[2];
+
+	extracted_bits = (reg >> 8) & 0x03;
+	converted_reg = (u8)(reg & 0xFF);
+
+	msgs[0].addr = (chip->i2c_cl->addr << 2) | extracted_bits;
+	msgs[0].flags = 0;
+	msgs[0].len = 1;
+	msgs[0].buf = &converted_reg;
+
+	msgs[1].addr = (chip->i2c_cl->addr << 2) | extracted_bits;
+	msgs[1].flags = I2C_M_RD;
+	msgs[1].len = 1;
+	msgs[1].buf = &ret_val;
+
+	ret = i2c_transfer(chip->i2c_cl->adapter, msgs, 2);
+	if (ret != 2) {
+		dev_err(chip->dev, "Read register 0x%02X error, ret=%d\n", reg, ret);
+		*val = 0;
+		ret = ret < 0 ? ret : -EIO;
+	} else {
+		*val = ret_val;
+		ret = 0;
+	}
+
+	return ret;
+}
+
+static int lp5812_read_tsd_config_status(struct lp5812_chip *chip, u8 *reg_val)
+{
+	int ret;
+
+	if (!reg_val)
+		return -1;
+
+	ret = lp5812_read(chip, chip->regs->tsd_config_status_reg, reg_val);
+
+	return ret;
+}
+
+static int lp5812_update_regs_config(struct lp5812_chip *chip)
+{
+	int ret;
+	u8 reg_val; /* save register value */
+
+	/* Send update command to update config setting */
+	ret = lp5812_write(chip, chip->regs->update_cmd_reg, UPDATE_CMD_VAL);
+	if (ret)
+		return ret;
+	/* check if the configuration is proper */
+	ret = lp5812_read_tsd_config_status(chip, &reg_val);
+	if (ret == 0)
+		return (int)(reg_val & 0x01);
+
+	return ret;
+}
+
+static int lp5812_read_lod_status(struct lp5812_chip *chip, int led_number, u8 *val)
+{
+	int ret;
+	u16 reg;
+	u8 reg_val;
+
+	if (!val)
+		return -1;
+
+	if (led_number < 0x8)
+		reg = LOD_STAT_1_REG;
+	else
+		reg = LOD_STAT_2_REG;
+
+	ret = lp5812_read(chip, reg, &reg_val);
+	if (ret)
+		return ret;
+
+	*val = (reg_val & (1 << (led_number % 8))) ? 1 : 0;
+
+	return ret;
+}
+
+static int lp5812_read_lsd_status(struct lp5812_chip *chip, int led_number, u8 *val)
+{
+	int ret;
+	u16 reg;
+	u8 reg_val;
+
+	if (!val)
+		return -1;
+
+	if (led_number < 0x8)
+		reg = LSD_STAT_1_REG;
+	else
+		reg = LSD_STAT_2_REG;
+
+	ret = lp5812_read(chip, reg, &reg_val);
+	if (ret)
+		return ret;
+
+	*val = (reg_val & (1 << (led_number % 8))) ? 1 : 0;
+
+	return ret;
+}
+
+static int lp5812_read_auto_pwm_value(struct lp5812_chip *chip, int led_number,
+		u8 *val)
+{
+	int ret;
+	u16 reg;
+	u8 reg_val;
+
+	reg = AUTO_PWM_BASE_ADDR + led_number;
+
+	ret = lp5812_read(chip, reg, &reg_val);
+	if (ret)
+		return ret;
+
+	*val = reg_val;
+
+	return ret;
+}
+
+static int lp5812_read_aep_status(struct lp5812_chip *chip, int led_number, u8 *val)
+{
+	int ret;
+	u16 reg;
+	u8 reg_val;
+
+	switch (led_number / 2) {
+	case 0:
+		reg = AEP_STATUS_0_REG; // LED_0 and LED_1
+		break;
+	case 1:
+		reg = AEP_STATUS_1_REG; // LED_2 and LED_3
+		break;
+	case 2:
+		reg = AEP_STATUS_2_REG; // LED_A0 and LED_A1
+		break;
+	case 3:
+		reg = AEP_STATUS_3_REG; // LED_A2 and LED_B0
+		break;
+	case 4:
+		reg = AEP_STATUS_4_REG; // LED_B1 and LED_B2
+		break;
+	case 5:
+		reg = AEP_STATUS_5_REG; // LED_C0 and LED_C1
+		break;
+	case 6:
+		reg = AEP_STATUS_6_REG; // LED_C2 and LED_D0
+		break;
+	case 7:
+		reg = AEP_STATUS_7_REG; // LED_D1 and LED_D2
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = lp5812_read(chip, reg, &reg_val);
+	if (ret)
+		return ret;
+
+	*val = (led_number % 2) ? ((reg_val >> 3) & 0x07) : (reg_val & 0x07);
+
+	return ret;
+}
+
+static int lp5812_enable_disable(struct lp5812_chip *chip, int enable)
+{
+	return lp5812_write(chip, chip->regs->enable_reg, (u8)enable);
+}
+
+static int lp5812_reset(struct lp5812_chip *chip)
+{
+	return lp5812_write(chip, chip->regs->reset_reg, RESET_REG_VAL);
+}
+
+static int lp5812_fault_clear(struct lp5812_chip *chip, u8 value)
+{
+	u8 reg_val;
+
+	if (value == 0)
+		reg_val = LOD_CLEAR_VAL;
+	else if (value == 1)
+		reg_val = LSD_CLEAR_VAL;
+	else if (value == 2)
+		reg_val = TSD_CLEAR_VAL;
+	else if (value == 3)
+		reg_val = FAULT_CLEAR_ALL;
+	else
+		return -EINVAL;
+
+	return lp5812_write(chip, chip->regs->fault_clear_reg, reg_val);
+}
+
+static int lp5812_device_command(struct lp5812_chip *chip, enum device_command command)
+{
+	switch (command) {
+	case UPDATE:
+		return lp5812_write(chip, chip->regs->update_cmd_reg,
+				UPDATE_CMD_VAL);
+	case START:
+		return lp5812_write(chip, chip->regs->start_cmd_reg,
+				START_CMD_VAL);
+	case STOP:
+		return lp5812_write(chip, chip->regs->stop_cmd_reg,
+				STOP_CMD_VAL);
+	case PAUSE:
+		return lp5812_write(chip, chip->regs->pause_cmd_reg,
+				PAUSE_CMD_VAL);
+	case CONTINUE:
+		return lp5812_write(chip, chip->regs->continue_cmd_reg,
+				CONTINUE_CMD_VAL);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int lp5812_set_pwm_dimming_scale(struct lp5812_chip *chip, int led_number,
+		enum pwm_dimming_scale scale)
+{
+	int ret;
+	u16 reg;
+	u8 reg_val;
+
+	if (led_number <= 7)
+		reg = (u16)DEV_CONFIG5;
+	else
+		reg = (u16)DEV_CONFIG6;
+
+	ret = lp5812_read(chip, reg, &reg_val);
+	if (ret)
+		return ret;
+	if (scale == LINEAR) {
+		reg_val &= ~(1 << (led_number % 8));
+		ret = lp5812_write(chip, reg, reg_val);
+		if (ret)
+			return ret;
+	} else {
+		reg_val |= (1 << (led_number % 8));
+		ret = lp5812_write(chip, reg, reg_val);
+		if (ret)
+			return ret;
+	}
+
+	ret = lp5812_update_regs_config(chip);
+
+	return ret;
+}
+
+static int lp5812_get_pwm_dimming_scale(struct lp5812_chip *chip,
+		int led_number, enum pwm_dimming_scale *scale)
+{
+	int ret;
+	u16 reg;
+	u8 reg_val;
+
+	if (led_number < 0x8)
+		reg = DEV_CONFIG5;
+	else
+		reg = DEV_CONFIG6;
+
+	ret = lp5812_read(chip, reg, &reg_val);
+	if (ret)
+		return ret;
+
+	*scale = (reg_val & (1 << (led_number % 8))) ? EXPONENTIAL : LINEAR;
+
+	return 0;
+}
+
+static int lp5812_set_phase_align(struct lp5812_chip *chip, int led_number,
+		int phase_align_val)
+{
+	int ret;
+	int bit_pos;
+	u16 reg;
+	u8 reg_val;
+
+	reg = DEV_CONFIG7 + (led_number / 4);
+	bit_pos = (led_number % 4) * 2;
+
+	ret = lp5812_read(chip, reg, &reg_val);
+	if (ret)
+		return ret;
+	reg_val |= (phase_align_val << bit_pos);
+	ret = lp5812_write(chip, reg, reg_val);
+	if (ret)
+		return ret;
+	ret = lp5812_update_regs_config(chip);
+
+	return ret;
+}
+
+static int lp5812_get_phase_align(struct lp5812_chip *chip, int led_number,
+		int *phase_align_val)
+{
+	int ret;
+	int bit_pos;
+	u16 reg;
+	u8 reg_val;
+
+	reg = DEV_CONFIG7 + (led_number / 4);
+	bit_pos = (led_number % 4) * 2;
+
+	ret = lp5812_read(chip, reg, &reg_val);
+	if (ret)
+		return ret;
+
+	*phase_align_val = (reg_val >> bit_pos) & 0x03;
+
+	return ret;
+}
+
+static int lp5812_get_led_mode(struct lp5812_chip *chip,
+		int led_number, enum control_mode *mode)
+{
+	int ret;
+	u16 reg;
+	u8 reg_val;
+
+	if (led_number < 0x8)
+		reg = DEV_CONFIG3;
+	else
+		reg = DEV_CONFIG4;
+
+	ret = lp5812_read(chip, reg, &reg_val);
+	if (ret)
+		return ret;
+
+	*mode = (reg_val & (1 << (led_number % 8))) ? AUTONOMOUS : MANUAL;
+
+	return 0;
+}
+
+static int lp5812_set_led_mode(struct lp5812_chip *chip, int led_number,
+		enum control_mode mode)
+{
+	int ret;
+	u16 reg;
+	u8 reg_val;
+
+	if (led_number <= 7)
+		reg = (u16)DEV_CONFIG3;
+	else
+		reg = (u16)DEV_CONFIG4;
+	ret = lp5812_read(chip, reg, &reg_val);
+	if (ret)
+		return ret;
+	if (mode == MANUAL) {
+		reg_val &= ~(1 << (led_number % 8));
+		ret = lp5812_write(chip, reg, reg_val);
+		if (ret)
+			return ret;
+	} else {
+		reg_val |= (1 << (led_number % 8));
+		ret = lp5812_write(chip, reg, reg_val);
+		if (ret)
+			return ret;
+	}
+
+	ret = lp5812_update_regs_config(chip);
+
+	return ret;
+}
+
+static int lp5812_manual_dc_pwm_control(struct lp5812_chip *chip,
+		int led_number, u8 val, enum dimming_type dimming_type)
+{
+	int ret;
+	u16 led_base_reg;
+
+	if (dimming_type == ANALOG)
+		led_base_reg = (u16)MANUAL_DC_LED_0_REG;
+	else
+		led_base_reg = (u16)MANUAL_PWM_LED_0_REG;
+	ret = lp5812_write(chip, led_base_reg + led_number, val);
+
+	return ret;
+}
+
+static int lp5812_manual_dc_pwm_read(struct lp5812_chip *chip,
+		int led_number, u8 *val, enum dimming_type dimming_type)
+{
+	int ret;
+	u16 led_base_reg;
+
+	if (dimming_type == ANALOG)
+		led_base_reg = (u16)MANUAL_DC_LED_0_REG;
+	else
+		led_base_reg = (u16)MANUAL_PWM_LED_0_REG;
+	ret = lp5812_read(chip, led_base_reg + led_number, val);
+
+	return ret;
+}
+
+static int lp5812_autonomous_dc_pwm_control(struct lp5812_chip *chip,
+		int led_number, u8 val, enum dimming_type dimming_type)
+{
+	int ret;
+	u16 led_base_reg;
+
+	led_base_reg = (u16)AUTO_DC_LED_0_REG;
+	ret = lp5812_write(chip, led_base_reg + led_number, val);
+
+	return ret;
+}
+
+static int lp5812_autonomous_dc_pwm_read(struct lp5812_chip *chip,
+		int led_number, u8 *val, enum dimming_type dimming_type)
+{
+	int ret;
+	u16 led_base_reg;
+
+	led_base_reg = (u16)AUTO_DC_LED_0_REG;
+	ret = lp5812_read(chip, led_base_reg + led_number, val);
+
+	return ret;
+}
+
+static int lp5812_disable_all_leds(struct lp5812_chip *chip)
+{
+	int ret;
+
+	ret = lp5812_write(chip, (u16)LED_ENABLE_1_REG, 0x00);
+	if (ret)
+		return ret;
+	ret = lp5812_write(chip, (u16)LED_ENABLE_2_REG, 0x00);
+	if (ret)
+		return ret;
+
+	return ret;
+}
+
+static int lp5812_get_drive_mode_scan_order(struct lp5812_chip *chip)
+{
+	u8 val;
+	int ret;
+
+	/* get led mode */
+	ret = lp5812_read(chip, (u16)DEV_CONFIG1, &val);
+	if (ret)
+		return ret;
+	chip->u_drive_mode.drive_mode_val = val;
+
+	/* get scan order */
+	ret = lp5812_read(chip, (u16)DEV_CONFIG2, &val);
+	if (ret)
+		return ret;
+	chip->u_scan_order.scan_order_val = val;
+
+	return ret;
+}
+
+static int lp5812_set_drive_mode_scan_order(struct lp5812_chip *chip)
+{
+	u8 val;
+	int ret;
+
+	/* Set led mode */
+	val = chip->u_drive_mode.drive_mode_val;
+	ret = lp5812_write(chip, (u16)DEV_CONFIG1, val);
+	if (ret)
+		return ret;
+
+	/* Setup scan order */
+	val = chip->u_scan_order.scan_order_val;
+	ret = lp5812_write(chip, (u16)DEV_CONFIG2, val);
+	if (ret)
+		return ret;
+
+	return ret;
+}
+
+static int lp5812_initialize(struct lp5812_chip *chip)
+{
+	int ret;
+
+	/* wait for 1 ms */
+	usleep_range(1000, 1100);
+
+	/* enable the lp5812 */
+	ret = lp5812_enable_disable(chip, 1);
+	if (ret) {
+		dev_err(chip->dev, "lp5812_enable_disable failed\n");
+		return ret;
+	}
+	ret = lp5812_set_drive_mode_scan_order(chip);
+	if (ret) {
+		dev_err(chip->dev, "lp5812_set_drive_mode_scan_order failed\n");
+		return ret;
+	}
+
+	/* Set lsd_threshold = 3h to avoid incorrect LSD detection */
+	ret = lp5812_write(chip, (u16)DEV_CONFIG12, 0x0B);
+	if (ret) {
+		dev_err(chip->dev, "write 0x0B to DEV_CONFIG12 failed\n");
+		return ret;
+	}
+
+	/* Send update command to complete configuration settings */
+	ret = lp5812_update_regs_config(chip);
+	if (ret) {
+		dev_err(chip->dev, "lp5812_update_regs_config failed\n");
+		return ret;
+	}
+
+	/* Enable LED_A0 for testing */
+	ret = lp5812_write(chip, (u16)LED_ENABLE_1_REG, 0x20);
+	if (ret) {
+		dev_err(chip->dev, "write 0x10 to LED_ENABLE_1_REG failed\n");
+		return ret;
+	}
+	/* set max DC current for LED_A0 */
+	ret = lp5812_write(chip, (u16)0x35, 0x80);
+	if (ret)
+		dev_err(chip->dev, "set max DC current for LED_A0 failed\n");
+
+	/* set 100% pwm cycle for LED_A0 */
+	ret = lp5812_write(chip, (u16)0x45, 0x80);
+	if (ret)
+		dev_err(chip->dev, "set 100 percent pwm cycle for LED_A0 failed\n");
+
+	return ret;
+}
+
+static int led_set_autonomous_animation_config(struct lp5812_led *led)
+{
+	int ret;
+	u16 reg;
+	struct lp5812_chip *chip = led->priv;
+
+	/* Set start/end pause */
+	reg = led->anim_base_addr + AUTO_PAUSE;
+	ret = lp5812_write(chip, reg, led->start_stop_pause_time.time_val);
+	if (ret)
+		return ret;
+
+	/* Set led playback and AEU selection */
+	reg = led->anim_base_addr + AUTO_PLAYBACK;
+	ret = lp5812_write(chip, reg, led->led_playback.led_playback_val);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int led_get_autonomous_animation_config(struct lp5812_led *led)
+{
+	int ret;
+	u16 reg;
+	struct lp5812_chip *chip = led->priv;
+
+	/* Get start/end pause value */
+	reg = led->anim_base_addr + AUTO_PAUSE;
+	ret = lp5812_read(chip, reg, &led->start_stop_pause_time.time_val);
+	if (ret)
+		return ret;
+
+	/* Get led playback and AEU selection values */
+	reg = led->anim_base_addr + AUTO_PLAYBACK;
+	ret = lp5812_read(chip, reg, &led->led_playback.led_playback_val);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static u16 get_aeu_pwm_register(struct anim_engine_unit *aeu,
+		enum pwm_slope_time_num pwm_num)
+{
+	u16 reg;
+	struct lp5812_led *led = aeu->led;
+
+	switch (pwm_num) {
+	case PWM1:
+		if (aeu->aeu_number == 1)
+			reg = led->anim_base_addr + AEU1_PWM_1;
+		else if (aeu->aeu_number == 2)
+			reg = led->anim_base_addr + AEU2_PWM_1;
+		else
+			reg = led->anim_base_addr + AEU3_PWM_1;
+		break;
+	case PWM2:
+		if (aeu->aeu_number == 1)
+			reg = led->anim_base_addr + AEU1_PWM_2;
+		else if (aeu->aeu_number == 2)
+			reg = led->anim_base_addr + AEU2_PWM_2;
+		else
+			reg = led->anim_base_addr + AEU3_PWM_2;
+		break;
+	case PWM3:
+		if (aeu->aeu_number == 1)
+			reg = led->anim_base_addr + AEU1_PWM_3;
+		else if (aeu->aeu_number == 2)
+			reg = led->anim_base_addr + AEU2_PWM_3;
+		else
+			reg = led->anim_base_addr + AEU3_PWM_3;
+		break;
+	case PWM4:
+		if (aeu->aeu_number == 1)
+			reg = led->anim_base_addr + AEU1_PWM_4;
+		else if (aeu->aeu_number == 2)
+			reg = led->anim_base_addr + AEU2_PWM_4;
+		else
+			reg = led->anim_base_addr + AEU3_PWM_4;
+		break;
+	case PWM5:
+		if (aeu->aeu_number == 1)
+			reg = led->anim_base_addr + AEU1_PWM_5;
+		else if (aeu->aeu_number == 2)
+			reg = led->anim_base_addr + AEU2_PWM_5;
+		else
+			reg = led->anim_base_addr + AEU3_PWM_5;
+		break;
+	default:
+		reg = led->anim_base_addr;
+		break;
+	}
+
+	return reg;
+}
+
+static u16 get_aeu_slope_time_register(struct anim_engine_unit *aeu,
+		enum pwm_slope_time_num slope_time_num)
+{
+	u16 reg;
+	struct lp5812_led *led = aeu->led;
+
+	switch (slope_time_num) {
+	case SLOPE_T1:
+	case SLOPE_T2:
+		if (aeu->aeu_number == 1)
+			reg = led->anim_base_addr + AEU1_T12;
+		else if (aeu->aeu_number == 2)
+			reg = led->anim_base_addr + AEU2_T12;
+		else
+			reg = led->anim_base_addr + AEU3_T12;
+		break;
+	case SLOPE_T3:
+	case SLOPE_T4:
+		if (aeu->aeu_number == 1)
+			reg = led->anim_base_addr + AEU1_T34;
+		else if (aeu->aeu_number == 2)
+			reg = led->anim_base_addr + AEU2_T34;
+		else
+			reg = led->anim_base_addr + AEU3_T34;
+		break;
+	default:
+		reg = led->anim_base_addr;
+		break;
+	}
+
+	return reg;
+}
+
+static u16 get_aeu_playback_time_register(struct anim_engine_unit *aeu)
+{
+	u16 reg;
+	struct lp5812_led *led = aeu->led;
+
+	if (aeu->aeu_number == 1)
+		reg = led->anim_base_addr + AEU1_PLAYBACK;
+	else if (aeu->aeu_number == 2)
+		reg = led->anim_base_addr + AEU2_PLAYBACK;
+	else
+		reg = led->anim_base_addr + AEU3_PLAYBACK;
+
+	return reg;
+}
+
+static int led_aeu_pwm_set_val(struct anim_engine_unit *aeu, u8 val,
+		enum pwm_slope_time_num pwm_num)
+{
+	int ret;
+	u16 reg;
+	struct lp5812_led *led = aeu->led;
+	struct lp5812_chip *chip = led->priv;
+
+	reg = get_aeu_pwm_register(aeu, pwm_num);
+	ret = lp5812_write(chip, reg, val);
+
+	return ret;
+}
+
+static int led_aeu_pwm_get_val(struct anim_engine_unit *aeu, u8 *val,
+		enum pwm_slope_time_num pwm_num)
+{
+	int ret;
+	u16 reg;
+	struct lp5812_led *led = aeu->led;
+	struct lp5812_chip *chip = led->priv;
+
+	reg = get_aeu_pwm_register(aeu, pwm_num);
+	ret = lp5812_read(chip, reg, val);
+
+	return ret;
+}
+
+static int led_aeu_slope_time_set_val(struct anim_engine_unit *aeu, u8 val,
+		enum pwm_slope_time_num slope_time_num)
+{
+	int ret;
+	u16 reg;
+	union time slope_time_val;
+	struct lp5812_led *led = aeu->led;
+	struct lp5812_chip *chip = led->priv;
+
+	reg = get_aeu_slope_time_register(aeu, slope_time_num);
+
+	/* get original value of slope time */
+	ret = lp5812_read(chip, reg, &slope_time_val.time_val);
+	if (ret)
+		return ret;
+
+	/* Update new value for slope time*/
+	if (slope_time_num == SLOPE_T1 || slope_time_num == SLOPE_T3)
+		slope_time_val.s_time.first = val;
+	if (slope_time_num == SLOPE_T2 || slope_time_num == SLOPE_T4)
+		slope_time_val.s_time.second = val;
+
+	/* Save updated value to hardware */
+	ret = lp5812_write(chip, reg, slope_time_val.time_val);
+
+	return ret;
+}
+
+static int led_aeu_slope_time_get_val(struct anim_engine_unit *aeu, u8 *val,
+		enum pwm_slope_time_num slope_time_num)
+{
+	int ret;
+	u16 reg;
+	union time slope_time_val;
+	struct lp5812_led *led = aeu->led;
+	struct lp5812_chip *chip = led->priv;
+
+	reg = get_aeu_slope_time_register(aeu, slope_time_num);
+	/* get slope time value */
+	ret = lp5812_read(chip, reg, &slope_time_val.time_val);
+	if (ret)
+		return ret;
+
+	if (slope_time_num == SLOPE_T1 || slope_time_num == SLOPE_T3)
+		*val = slope_time_val.s_time.first;
+
+	if (slope_time_num == SLOPE_T2 || slope_time_num == SLOPE_T4)
+		*val = slope_time_val.s_time.second;
+
+	return ret;
+}
+
+static int led_aeu_playback_time_set_val(struct anim_engine_unit *aeu, u8 val)
+{
+	int ret;
+	u16 reg;
+	struct lp5812_led *led = aeu->led;
+	struct lp5812_chip *chip = led->priv;
+
+	reg = get_aeu_playback_time_register(aeu);
+	ret = lp5812_write(chip, reg, val);
+
+	return ret;
+}
+
+static int led_aeu_playback_time_get_val(struct anim_engine_unit *aeu, u8 *val)
+{
+	int ret;
+	u16 reg;
+	struct lp5812_led *led = aeu->led;
+	struct lp5812_chip *chip = led->priv;
+
+	reg = get_aeu_playback_time_register(aeu);
+	ret = lp5812_read(chip, reg, val);
+
+	return ret;
+}
+
+static int aeu_create_sysfs_group(struct anim_engine_unit *aeu)
+{
+	int ret;
+
+	ret = kobject_add(&aeu->kobj, &aeu->led->kobj, "%s",
+			aeu->aeu_name);
+	if (ret)
+		return ret;
+	ret = sysfs_create_group(&aeu->kobj, &aeu->attr_group);
+	aeu->enabled = 1;
+
+	return ret;
+}
+
+static void aeu_remove_sysfs_group(struct anim_engine_unit *aeu)
+{
+	aeu->enabled = 0;
+	sysfs_remove_group(&aeu->kobj, &aeu->attr_group);
+	kobject_del(&aeu->kobj);
+}
+
+/* Function to remove multiple sysfs group of AEU in one led */
+static void aeu_remove_multi_sysfs_groups(struct lp5812_led *led)
+{
+	int i;
+
+	for (i = 0; i < MAX_AEU; i++) {
+		if (led->aeu[i].enabled)
+			aeu_remove_sysfs_group(&led->aeu[i]);
+	}
+}
+
+/*
+ * Function to create multi sysfs group for specific led. it will
+ * check the mode of led and AEU number selection to create
+ * corresponding AEU interfaces
+ */
+static int aeu_create_multi_sysfs_groups(struct lp5812_led *led)
+{
+	int i;
+	int aeu_select;
+	enum control_mode mode;
+	struct lp5812_chip *chip = led->priv;
+
+	if (lp5812_get_led_mode(chip, led->led_number, &mode))
+		return -EIO;
+	if (mode == AUTONOMOUS) {
+		if (led_get_autonomous_animation_config(led))
+			return -EIO;
+		aeu_select = led->led_playback.s_led_playback.aeu_selection;
+		if (aeu_select == 3)
+			aeu_select = 2;
+		for (i = 0; i < aeu_select + 1; i++)
+			aeu_create_sysfs_group(&led->aeu[i]);
+	}
+
+	return 0;
+}
+
+static int lp5812_create_sysfs_group(struct lp5812_led *led)
+{
+	int ret;
+
+	ret = kobject_add(&led->kobj, &led->priv->dev->kobj, "%s",
+			led->led_name);
+	if (ret)
+		return ret;
+	ret = sysfs_create_group(&led->kobj, &led->attr_group);
+	if (ret)
+		return ret;
+	led->is_sysfs_created = 1;
+
+	return ret;
+}
+
+static void lp5812_remove_sysfs_group(struct lp5812_led *led)
+{
+	int i;
+
+	/* remove sysfs group of AEU */
+	for (i = 0; i < MAX_AEU; i++) {
+		if (led->aeu[i].enabled)
+			aeu_remove_sysfs_group(&led->aeu[i]);
+	}
+	led->is_sysfs_created = 0;
+	sysfs_remove_group(&led->kobj, &led->attr_group);
+	kobject_del(&led->kobj);
+}
+
+static ssize_t device_enable_store(struct device *dev,
+		struct device_attribute *attr,
+		const char *buf, size_t len)
+{
+	int enable;
+	int ret;
+	struct lp5812_chip *chip = i2c_get_clientdata(to_i2c_client(dev));
+
+	ret = kstrtoint(buf, 0, &enable);
+	if (ret)
+		return ret;
+
+	if (enable != 0 && enable != 1)
+		return -EINVAL;
+
+	/* set to hardware */
+	mutex_lock(&chip->lock);
+	ret = lp5812_enable_disable(chip, enable);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	return len;
+}
+
+static ssize_t device_enable_show(struct device *dev,
+		struct device_attribute *attr,
+		char *buf)
+{
+	u8 enable;
+	int ret;
+	struct lp5812_chip *chip = i2c_get_clientdata(to_i2c_client(dev));
+
+	mutex_lock(&chip->lock);
+	ret = lp5812_read(chip, chip->regs->enable_reg, &enable);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	return sysfs_emit(buf, "%d\n", enable);
+}
+
+static ssize_t device_command_store(struct device *dev,
+		struct device_attribute *attr,
+		const char *buf, size_t len)
+{
+	struct lp5812_chip *chip = i2c_get_clientdata(to_i2c_client(dev));
+	enum device_command cmd;
+
+	if (sysfs_streq(buf, "update"))
+		cmd = UPDATE;
+	else if (sysfs_streq(buf, "start"))
+		cmd = START;
+	else if (sysfs_streq(buf, "stop"))
+		cmd = STOP;
+	else if (sysfs_streq(buf, "pause"))
+		cmd = PAUSE;
+	else if (sysfs_streq(buf, "continue"))
+		cmd = CONTINUE;
+	else
+		return -EINVAL;
+
+	mutex_lock(&chip->lock);
+	lp5812_device_command(chip, cmd);
+	mutex_unlock(&chip->lock);
+
+	return len;
+}
+
+static ssize_t sw_reset_store(struct device *dev,
+		struct device_attribute *attr,
+		const char *buf, size_t len)
+{
+	int reset;
+	int ret;
+	struct lp5812_chip *chip = i2c_get_clientdata(to_i2c_client(dev));
+
+	ret = kstrtoint(buf, 0, &reset);
+	if (ret)
+		return ret;
+
+	if (reset != 1)
+		return -EINVAL;
+
+	/* reset hardware */
+	mutex_lock(&chip->lock);
+	lp5812_reset(chip);
+	msleep(1000);
+	/* set back manual mode as default for all LEDs */
+	lp5812_write(chip, (u16)DEV_CONFIG3, 0x00);
+	lp5812_write(chip, (u16)DEV_CONFIG4, 0x00);
+	lp5812_update_regs_config(chip);
+	mutex_unlock(&chip->lock);
+
+	/* Update sysfs based on default mode when hardware reseted*/
+	ret = lp5812_init_dev_config(chip, "direct_mode", 1);
+	if (ret) {
+		dev_err(dev, "%s: lp5812_init_dev_config failed\n",
+			__func__);
+		return ret;
+	}
+
+	return len;
+}
+
+static ssize_t fault_clear_store(struct device *dev,
+		struct device_attribute *attr,
+		const char *buf, size_t len)
+{
+	int fault_clear;
+	int ret;
+	struct lp5812_chip *chip = i2c_get_clientdata(to_i2c_client(dev));
+
+	ret = kstrtoint(buf, 0, &fault_clear);
+	if (ret)
+		return ret;
+
+	if (fault_clear < 0 || fault_clear > 3)
+		return -EINVAL;
+
+	mutex_lock(&chip->lock);
+	ret = lp5812_fault_clear(chip, fault_clear);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	return len;
+}
+
+static void lp5812_create_sysfs_via_led_name(struct lp5812_chip *chip,
+		const char *led_name)
+{
+	int i;
+
+	for (i = 0; i < MAX_LEDS; i++) {
+		if (sysfs_streq(led_name, chip->leds[i].led_name)) {
+			lp5812_create_sysfs_group(&chip->leds[i]);
+
+			/*
+			 * create AEU interface if current led mode is
+			 * autonomous
+			 */
+			aeu_create_multi_sysfs_groups(&chip->leds[i]);
+		}
+	}
+}
+
+static void set_mix_sel_led(struct lp5812_chip *chip, int mix_sel_led)
+{
+	chip->u_drive_mode.s_drive_mode.mix_sel_led_0 = 0;
+	chip->u_drive_mode.s_drive_mode.mix_sel_led_1 = 0;
+	chip->u_drive_mode.s_drive_mode.mix_sel_led_2 = 0;
+	chip->u_drive_mode.s_drive_mode.mix_sel_led_3 = 0;
+
+	if (mix_sel_led == 0)
+		chip->u_drive_mode.s_drive_mode.mix_sel_led_0 = 1;
+	else if (mix_sel_led == 1)
+		chip->u_drive_mode.s_drive_mode.mix_sel_led_1 = 1;
+	else if (mix_sel_led == 2)
+		chip->u_drive_mode.s_drive_mode.mix_sel_led_2 = 1;
+	else if (mix_sel_led == 3)
+		chip->u_drive_mode.s_drive_mode.mix_sel_led_3 = 1;
+}
+
+static void parse_drive_mode(struct lp5812_chip *chip, char *str)
+{
+	int ret;
+	char *sub_str;
+	int tcm_scan_num, mix_scan_num, mix_sel_led;
+	int scan_oder0, scan_oder1, scan_oder2, scan_oder3;
+
+	sub_str = strsep(&str, ":");
+	if (sysfs_streq(sub_str, "direct_mode")) {
+		chip->u_drive_mode.s_drive_mode.led_mode = 0;
+		chip->u_drive_mode.s_drive_mode.mix_sel_led_0 = 0;
+		chip->u_drive_mode.s_drive_mode.mix_sel_led_1 = 0;
+		chip->u_drive_mode.s_drive_mode.mix_sel_led_2 = 0;
+		chip->u_drive_mode.s_drive_mode.mix_sel_led_3 = 0;
+	}
+	if (sysfs_streq(sub_str, "tcmscan")) {
+		chip->u_drive_mode.s_drive_mode.mix_sel_led_0 = 0;
+		chip->u_drive_mode.s_drive_mode.mix_sel_led_1 = 0;
+		chip->u_drive_mode.s_drive_mode.mix_sel_led_2 = 0;
+		chip->u_drive_mode.s_drive_mode.mix_sel_led_3 = 0;
+
+		/* Get tcm scan number */
+		sub_str = strsep(&str, ":");
+		ret = kstrtoint(sub_str, 0, &tcm_scan_num);
+		if (ret)
+			return;
+		chip->u_drive_mode.s_drive_mode.led_mode = tcm_scan_num;
+		switch (tcm_scan_num) {
+		case TCM_1_SCAN:
+			/* Get scan_oder0 */
+			sub_str = strsep(&str, ":");
+			ret = kstrtoint(sub_str, 0, &scan_oder0);
+			if (ret)
+				return;
+			chip->u_scan_order.s_scan_order.scan_order_0 =
+				scan_oder0;
+			break;
+		case TCM_2_SCAN:
+			/* Get scan_order0 */
+			sub_str = strsep(&str, ":");
+			ret = kstrtoint(sub_str, 0, &scan_oder0);
+			if (ret)
+				return;
+			/* Get scan_order1 */
+			sub_str = strsep(&str, ":");
+			ret = kstrtoint(sub_str, 0, &scan_oder1);
+			if (ret)
+				return;
+			chip->u_scan_order.s_scan_order.scan_order_0 =
+				scan_oder0;
+			chip->u_scan_order.s_scan_order.scan_order_1 =
+				scan_oder1;
+			break;
+		case TCM_3_SCAN:
+			/* Get scan_order0 */
+			sub_str = strsep(&str, ":");
+			ret = kstrtoint(sub_str, 0, &scan_oder0);
+			if (ret)
+				return;
+			/* Get scan_order1 */
+			sub_str = strsep(&str, ":");
+			ret = kstrtoint(sub_str, 0, &scan_oder1);
+			if (ret)
+				return;
+			/* Get scan_order2 */
+			sub_str = strsep(&str, ":");
+			ret = kstrtoint(sub_str, 0, &scan_oder2);
+			if (ret)
+				return;
+			chip->u_scan_order.s_scan_order.scan_order_0 =
+				scan_oder0;
+			chip->u_scan_order.s_scan_order.scan_order_1 =
+				scan_oder1;
+			chip->u_scan_order.s_scan_order.scan_order_2 =
+				scan_oder2;
+			break;
+		case TCM_4_SCAN:
+			/* Get scan_order0 */
+			sub_str = strsep(&str, ":");
+			ret = kstrtoint(sub_str, 0, &scan_oder0);
+			if (ret)
+				return;
+			/* Get scan_order1 */
+			sub_str = strsep(&str, ":");
+			ret = kstrtoint(sub_str, 0, &scan_oder1);
+			if (ret)
+				return;
+			/* Get scan_order2 */
+			sub_str = strsep(&str, ":");
+			ret = kstrtoint(sub_str, 0, &scan_oder2);
+			if (ret)
+				return;
+			/* Get scan_order3 */
+			sub_str = strsep(&str, ":");
+			ret = kstrtoint(sub_str, 0, &scan_oder3);
+			if (ret)
+				return;
+			chip->u_scan_order.s_scan_order.scan_order_0 =
+				scan_oder0;
+			chip->u_scan_order.s_scan_order.scan_order_1 =
+				scan_oder1;
+			chip->u_scan_order.s_scan_order.scan_order_2 =
+				scan_oder2;
+			chip->u_scan_order.s_scan_order.scan_order_3 =
+				scan_oder3;
+			break;
+		default:
+			break;
+		}
+	}
+	if (sysfs_streq(sub_str, "mixscan")) {
+		/* Get mix scan number */
+		sub_str = strsep(&str, ":");
+		ret = kstrtoint(sub_str, 0, &mix_scan_num);
+		if (ret)
+			return;
+		chip->u_drive_mode.s_drive_mode.led_mode = mix_scan_num + 4;
+		/* Get mix_sel_led */
+		sub_str = strsep(&str, ":");
+		ret = kstrtoint(sub_str, 0, &mix_sel_led);
+		if (ret)
+			return;
+		set_mix_sel_led(chip, mix_sel_led);
+		if (mix_scan_num == 1) {
+			/* Get scan_order0 */
+			sub_str = strsep(&str, ":");
+			ret = kstrtoint(sub_str, 0, &scan_oder0);
+			if (ret)
+				return;
+			chip->u_scan_order.s_scan_order.scan_order_0 =
+				scan_oder0;
+		}
+		if (mix_scan_num == 2) {
+			/* Get scan_order0 */
+			sub_str = strsep(&str, ":");
+			ret = kstrtoint(sub_str, 0, &scan_oder0);
+			if (ret)
+				return;
+			/* Get scan_order1 */
+			sub_str = strsep(&str, ":");
+			ret = kstrtoint(sub_str, 0, &scan_oder1);
+			if (ret)
+				return;
+			chip->u_scan_order.s_scan_order.scan_order_0 =
+				scan_oder0;
+			chip->u_scan_order.s_scan_order.scan_order_1 =
+				scan_oder1;
+		}
+		if (mix_scan_num == 3) {
+			/* Get scan_order0 */
+			sub_str = strsep(&str, ":");
+			ret = kstrtoint(sub_str, 0, &scan_oder0);
+			if (ret)
+				return;
+			/* Get scan_order1 */
+			sub_str = strsep(&str, ":");
+			ret = kstrtoint(sub_str, 0, &scan_oder1);
+			if (ret)
+				return;
+			/* Get scan_order2 */
+			sub_str = strsep(&str, ":");
+			ret = kstrtoint(sub_str, 0, &scan_oder2);
+			if (ret)
+				return;
+			chip->u_scan_order.s_scan_order.scan_order_0 =
+				scan_oder0;
+			chip->u_scan_order.s_scan_order.scan_order_1 =
+				scan_oder1;
+			chip->u_scan_order.s_scan_order.scan_order_2 =
+				scan_oder2;
+		}
+	}
+}
+
+static void leds_remove_existed_sysfs(struct lp5812_chip *chip)
+{
+	int i;
+
+	for (i = 0; i < MAX_LEDS; i++) {
+		if (chip->leds[i].is_sysfs_created)
+			lp5812_remove_sysfs_group(&chip->leds[i]);
+	}
+}
+
+static int lp5812_init_dev_config(struct lp5812_chip *chip,
+		const char *drive_mode, int rm_led_sysfs)
+{
+	int i;
+	int num_drive_mode;
+	int is_valid_arg = 0;
+	char *str;
+	const char **led_ptr;
+
+	str = kstrdup(drive_mode, GFP_KERNEL);
+	if (!str)
+		return -ENOMEM;
+
+	num_drive_mode = ARRAY_SIZE(chip_leds_map);
+	for (i = 0; i < num_drive_mode; ++i) {
+		if (sysfs_streq(str, chip_leds_map[i].drive_mode)) {
+			if (rm_led_sysfs)
+				leds_remove_existed_sysfs(chip);
+			parse_drive_mode(chip, str);
+			led_ptr = chip_leds_map[i].led_arr;
+			while (*led_ptr) {
+				lp5812_create_sysfs_via_led_name(chip,
+						*led_ptr);
+				++led_ptr;
+			}
+			is_valid_arg = 1;
+			break;
+		}
+	}
+	kfree(str);
+	if (!is_valid_arg)
+		return -EINVAL;
+
+	return 0;
+}
+
+static ssize_t dev_config_store(struct device *dev,
+		struct device_attribute *attr,
+		const char *buf, size_t len)
+{
+	int ret;
+	struct lp5812_chip *chip = i2c_get_clientdata(to_i2c_client(dev));
+
+	mutex_lock(&chip->lock);
+	ret = lp5812_init_dev_config(chip, buf, 1);
+	if (ret) {
+		dev_err(chip->dev, "%s: lp5812_init_dev_config failed\n",
+			__func__);
+		goto out;
+	}
+
+	/* set drive mode and scan order to hardware */
+	ret = lp5812_set_drive_mode_scan_order(chip);
+	if (ret) {
+		ret = -EIO;
+		goto out;
+	}
+	ret = lp5812_update_regs_config(chip);
+	if (ret) {
+		ret = -EIO;
+		goto out;
+	}
+
+	/* disable all LEDs that were previously enabled */
+	ret = lp5812_disable_all_leds(chip);
+	if (ret) {
+		ret = -EIO;
+		goto out;
+	}
+	mutex_unlock(&chip->lock);
+
+	return len;
+out:
+	mutex_unlock(&chip->lock);
+	return ret;
+}
+
+static char *parse_dev_config_info(struct lp5812_chip *chip)
+{
+	char *tmp;
+	int order_0, order_1, order_2, order_3;
+	char *scan_order_0 = " scan_order_0: ";
+	char *scan_order_1 = " scan_order_1: ";
+	char *scan_order_2 = " scan_order_2: ";
+	char *scan_order_3 = " scan_order_3: ";
+
+	tmp = kmalloc(128, GFP_KERNEL);
+	if (!tmp)
+		return NULL;
+
+	order_0 = chip->u_scan_order.s_scan_order.scan_order_0;
+	order_1 = chip->u_scan_order.s_scan_order.scan_order_1;
+	order_2 = chip->u_scan_order.s_scan_order.scan_order_2;
+	order_3 = chip->u_scan_order.s_scan_order.scan_order_3;
+
+	switch (chip->u_drive_mode.s_drive_mode.led_mode) {
+	case DIRECT_MODE:
+		sprintf(tmp, "%s", "Mode: direct mode;");
+		break;
+	case TCM_1_SCAN:
+		sprintf(tmp, "%s%s%d%s", "Mode: tcm 1 scan;", scan_order_0,
+			order_0, ";");
+		break;
+	case TCM_2_SCAN:
+		sprintf(tmp, "%s%s%d%s%s%d%s", "Mode: tcm 2 scan;",
+		scan_order_0, order_0, ";", scan_order_1, order_1, ";");
+		break;
+	case TCM_3_SCAN:
+		sprintf(tmp, "%s%s%d%s%s%d%s%s%d%s", "Mode: tcm 3 scan;",
+			scan_order_0, order_0, ";", scan_order_1, order_1,
+			";", scan_order_2, order_2, ";");
+		break;
+	case TCM_4_SCAN:
+		sprintf(tmp, "%s%s%d%s%s%d%s%s%d%s%s%d%s", "Mode: tcm 4 scan;",
+			scan_order_0, order_0, ";", scan_order_1, order_1, ";",
+			scan_order_2, order_2, ";", scan_order_3, order_3, ";");
+		break;
+	case MIX_1_SCAN:
+		sprintf(tmp, "%s%s%d%s", "Mode: mix 1 scan;", scan_order_0,
+			order_0, ";");
+		break;
+	case MIX_2_SCAN:
+		sprintf(tmp, "%s%s%d%s%s%d%s", "Mode: mix 2 scan;",
+		scan_order_0, order_0, ";", scan_order_1, order_1, ";");
+		break;
+	case MIX_3_SCAN:
+		sprintf(tmp, "%s%s%d%s%s%d%s%s%d%s", "Mode: mix 3 scan;",
+			scan_order_0, order_0, ";", scan_order_1, order_1,
+			";", scan_order_2, order_2, ";");
+		break;
+	}
+
+	if (chip->u_drive_mode.s_drive_mode.mix_sel_led_0)
+		strcat(tmp, " Direct output: OUT0;");
+	else if (chip->u_drive_mode.s_drive_mode.mix_sel_led_1)
+		strcat(tmp, " Direct output: OUT1;");
+	else if (chip->u_drive_mode.s_drive_mode.mix_sel_led_2)
+		strcat(tmp, " Direct output: OUT2;");
+	else if (chip->u_drive_mode.s_drive_mode.mix_sel_led_3)
+		strcat(tmp, " Direct output: OUT3;");
+	else
+		strcat(tmp, " Direct output: None;");
+	return tmp;
+}
+
+static ssize_t dev_config_show(struct device *dev,
+		struct device_attribute *attr,
+		char *buf)
+{
+	int i;
+	char *mode_info;
+	int ret;
+	int pos = 0;
+	struct lp5812_chip *chip = i2c_get_clientdata(to_i2c_client(dev));
+
+	/* get drive mode and scan order */
+	mutex_lock(&chip->lock);
+	ret = lp5812_get_drive_mode_scan_order(chip);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	mode_info = parse_dev_config_info(chip);
+	if (!mode_info)
+		return -ENOMEM;
+
+	pos += sysfs_emit_at(buf, pos, "%s", mode_info);
+	pos += sysfs_emit_at(buf, pos, "%s", "\nPlease select below valid drive mode:\n");
+	pos += sysfs_emit_at(buf, pos, "%s", "For Ex: echo tcmscan:1:0 > dev_config\n");
+	for (i = 0; i < ARRAY_SIZE(chip_leds_map); ++i)
+		pos += sysfs_emit_at(buf, pos, "%s\n", chip_leds_map[i].drive_mode);
+	kfree(mode_info);
+
+	return pos;
+}
+
+static ssize_t tsd_config_status_show(struct device *dev,
+		struct device_attribute *attr,
+		char *buf)
+{
+	int ret;
+	u8 reg_val;
+	int tsd_stat;
+	int config_stat;
+	struct lp5812_chip *chip = i2c_get_clientdata(to_i2c_client(dev));
+
+	mutex_lock(&chip->lock);
+	ret = lp5812_read(chip, (u16)TSD_CONFIG_STAT_REG, &reg_val);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+	tsd_stat = (reg_val >> 1) & 0x01;
+	config_stat = reg_val & 0x01;
+
+	return sysfs_emit(buf, "%d %d\n", tsd_stat, config_stat);
+}
+
+static LP5812_DEV_ATTR_RW(device_enable);
+static LP5812_DEV_ATTR_RW(dev_config);
+static LP5812_DEV_ATTR_WO(device_command);
+static LP5812_DEV_ATTR_WO(sw_reset);
+static LP5812_DEV_ATTR_WO(fault_clear);
+static LP5812_DEV_ATTR_RO(tsd_config_status);
+
+static struct attribute *lp5812_chip_attributes[] = {
+	&dev_attr_device_enable.attr,
+	&dev_attr_device_command.attr,
+	&dev_attr_sw_reset.attr,
+	&dev_attr_fault_clear.attr,
+	&dev_attr_dev_config.attr,
+	&dev_attr_tsd_config_status.attr,
+	NULL
+};
+
+static ssize_t led_activate_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	u8 val;
+	u16 reg;
+	u8 reg_val;
+	int ret;
+	struct lp5812_led *led = to_lp5812_led(kobj);
+	struct lp5812_chip *chip = led->priv;
+
+	if (led->led_number < 0x8)
+		reg = LED_ENABLE_1_REG;
+	else
+		reg = LED_ENABLE_2_REG;
+
+	mutex_lock(&chip->lock);
+	ret = lp5812_read(chip, reg, &reg_val);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	val = (reg_val & (1 << (led->led_number % 8))) ? 1 : 0;
+	return sysfs_emit(buf, "%d\n", val);
+}
+
+static ssize_t led_activate_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	int val;
+	u16 reg;
+	u8 reg_val;
+	int ret;
+	struct lp5812_led *led = to_lp5812_led(kobj);
+	struct lp5812_chip *chip = led->priv;
+
+	ret = kstrtoint(buf, 0, &val);
+	if (ret)
+		return -EINVAL;
+
+	if (val != 0 && val != 1)
+		return -EINVAL;
+
+	if (led->led_number < 0x8)
+		reg = LED_ENABLE_1_REG;
+	else
+		reg = LED_ENABLE_2_REG;
+
+	mutex_lock(&chip->lock);
+	ret = lp5812_read(chip, reg, &reg_val);
+	if (ret == 0) {
+		if (val == 0) {
+			ret = lp5812_write(chip, reg,
+				reg_val & (~(1 << (led->led_number % 8))));
+		} else {
+			ret = lp5812_write(chip, reg,
+				reg_val | (1 << (led->led_number % 8)));
+		}
+	}
+	mutex_unlock(&chip->lock);
+
+	if (ret)
+		return -EIO;
+
+	return count;
+}
+
+static ssize_t led_mode_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	int ret;
+	enum control_mode mode;
+	struct lp5812_led *led = to_lp5812_led(kobj);
+	struct lp5812_chip *chip = led->priv;
+
+	mutex_lock(&chip->lock);
+	ret = lp5812_get_led_mode(chip, led->led_number, &mode);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	return sysfs_emit(buf, "%s\n", AUTONOMOUS == mode ? "autonomous" : "manual");
+}
+
+static ssize_t led_mode_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	int ret;
+	enum control_mode val;
+	struct lp5812_led *led = to_lp5812_led(kobj);
+	struct lp5812_chip *chip = led->priv;
+
+	if (sysfs_streq(buf, "manual")) {
+		/* Remove AEU sysfs interface for current led in manual mode */
+		aeu_remove_multi_sysfs_groups(led);
+		val = MANUAL;
+	} else if (sysfs_streq(buf, "autonomous")) {
+		val = AUTONOMOUS;
+	} else {
+		return -EINVAL;
+	}
+
+	mutex_lock(&chip->lock);
+	ret = lp5812_set_led_mode(chip, led->led_number, val);
+	if (ret) {
+		ret = -EIO;
+		goto out;
+	}
+
+	ret = aeu_create_multi_sysfs_groups(led);
+	if (ret) {
+		dev_err(chip->dev, "aeu_create_multi_sysfs_groups() failed\n");
+		goto out;
+	}
+
+	mutex_unlock(&chip->lock);
+	return count;
+out:
+	mutex_unlock(&chip->lock);
+	return ret;
+}
+
+static ssize_t led_manual_dc_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	int manual_dc;
+	int ret;
+	struct lp5812_led *led = to_lp5812_led(kobj);
+	struct lp5812_chip *chip = led->priv;
+
+	ret = kstrtoint(buf, 0, &manual_dc);
+	if (ret)
+		return ret;
+
+	if (manual_dc < 0 || manual_dc > 255)
+		return -EINVAL;
+
+	mutex_lock(&chip->lock);
+	ret = lp5812_manual_dc_pwm_control(chip, led->led_number, manual_dc, ANALOG);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	return count;
+}
+
+static ssize_t led_manual_dc_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	u8 val;
+	int ret;
+	struct lp5812_led *led = to_lp5812_led(kobj);
+	struct lp5812_chip *chip = led->priv;
+
+	mutex_lock(&chip->lock);
+	ret = lp5812_manual_dc_pwm_read(chip, led->led_number, &val, ANALOG);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	return sysfs_emit(buf, "%d\n", val);
+}
+
+static ssize_t led_manual_pwm_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	int manual_pwm;
+	int ret;
+	struct lp5812_led *led = to_lp5812_led(kobj);
+	struct lp5812_chip *chip = led->priv;
+
+	ret = kstrtoint(buf, 0, &manual_pwm);
+	if (ret)
+		return ret;
+
+	if (manual_pwm < 0 || manual_pwm > 255)
+		return -EINVAL;
+
+	mutex_lock(&chip->lock);
+	ret = lp5812_manual_dc_pwm_control(chip, led->led_number, manual_pwm, PWM);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	return count;
+}
+
+static ssize_t led_manual_pwm_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	u8 val;
+	int ret;
+	struct lp5812_led *led = to_lp5812_led(kobj);
+	struct lp5812_chip *chip = led->priv;
+
+	mutex_lock(&chip->lock);
+	ret = lp5812_manual_dc_pwm_read(chip, led->led_number, &val, PWM);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	return sysfs_emit(buf, "%d\n", val);
+}
+
+static ssize_t led_auto_dc_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	int auto_dc;
+	int ret;
+	struct lp5812_led *led = to_lp5812_led(kobj);
+	struct lp5812_chip *chip = led->priv;
+
+	ret = kstrtoint(buf, 0, &auto_dc);
+	if (ret)
+		return ret;
+
+	if (auto_dc < 0 || auto_dc > 255)
+		return -EINVAL;
+
+	mutex_lock(&chip->lock);
+	ret = lp5812_autonomous_dc_pwm_control(chip, led->led_number, auto_dc, ANALOG);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	return count;
+}
+
+static ssize_t led_auto_dc_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	u8 val;
+	int ret;
+	struct lp5812_led *led = to_lp5812_led(kobj);
+	struct lp5812_chip *chip = led->priv;
+
+	mutex_lock(&chip->lock);
+	ret = lp5812_autonomous_dc_pwm_read(chip, led->led_number, &val, ANALOG);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	return sysfs_emit(buf, "%d\n", val);
+}
+
+static int parse_autonomous_animation_config(struct lp5812_led *led,
+		const char *user_buf)
+{
+	int ret;
+	int i;
+	char *str;
+	char *sub_str;
+	int aeu_select, start_pause_time, stop_pause_time, led_playback_time;
+
+	str = kstrdup(user_buf, GFP_KERNEL);
+	if (!str)
+		return -ENOMEM;
+
+	/* parse aeu_select */
+	sub_str = strsep(&str, ":");
+	ret = kstrtoint(sub_str, 0, &aeu_select);
+	if (ret)
+		goto free_str;
+	if (aeu_select < 1 || aeu_select > 3) {
+		ret = -EINVAL;
+		goto free_str;
+	}
+
+	aeu_remove_multi_sysfs_groups(led);
+
+	for (i = 0; i < aeu_select; i++)
+		aeu_create_sysfs_group(&led->aeu[i]);
+	led->led_playback.s_led_playback.aeu_selection = aeu_select - 1;
+
+	/* parse start_pause_time */
+	sub_str = strsep(&str, ":");
+	if (sub_str) {
+		ret = kstrtoint(sub_str, 0, &start_pause_time);
+		if (ret)
+			goto free_str;
+		if (start_pause_time < 0 || start_pause_time > 15) {
+			ret = -EINVAL;
+			goto free_str;
+		}
+		led->start_stop_pause_time.s_time.second = start_pause_time;
+	} else {
+		led->start_stop_pause_time.s_time.second = 15;
+	}
+
+	/* parse stop_pause_time */
+	sub_str = strsep(&str, ":");
+	if (sub_str) {
+		ret = kstrtoint(sub_str, 0, &stop_pause_time);
+		if (ret)
+			goto free_str;
+		if (stop_pause_time < 0 || stop_pause_time > 15) {
+			ret = -EINVAL;
+			goto free_str;
+		}
+		led->start_stop_pause_time.s_time.first = stop_pause_time;
+	} else {
+		led->start_stop_pause_time.s_time.first = 15;
+	}
+
+	/* parse led_playback_time */
+	sub_str = strsep(&str, ":");
+	if (sub_str) {
+		ret = kstrtoint(sub_str, 0, &led_playback_time);
+		if (ret)
+			goto free_str;
+		if (led_playback_time < 0 || led_playback_time > 15) {
+			ret = -EINVAL;
+			goto free_str;
+		}
+		led->led_playback.s_led_playback.led_playback_time =
+			led_playback_time;
+	} else {
+		led->led_playback.s_led_playback.led_playback_time = 15;
+	}
+
+	kfree(str);
+	return 0;
+
+free_str:
+	kfree(str);
+	return ret;
+}
+
+static ssize_t led_auto_animation_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	int ret;
+	enum control_mode led_mode;
+	struct lp5812_led *led = to_lp5812_led(kobj);
+	struct lp5812_chip *chip = led->priv;
+
+	mutex_lock(&chip->lock);
+	/* First check the mode of led is manual or autonomous */
+	ret = lp5812_get_led_mode(chip, led->led_number, &led_mode);
+	if (ret) {
+		ret = -EIO;
+		goto out;
+	}
+
+	if (led_mode == AUTONOMOUS) {
+		ret = parse_autonomous_animation_config(led, buf);
+		if (ret) {
+			dev_err(chip->dev, "parse_autonomous_animation_config failed\n");
+			goto out;
+		}
+		/* Write data to hardware */
+		ret = led_set_autonomous_animation_config(led);
+		if (ret) {
+			ret = -EIO;
+			goto out;
+		}
+	}
+	mutex_unlock(&chip->lock);
+	return count;
+
+out:
+	mutex_unlock(&chip->lock);
+	return ret;
+}
+
+static ssize_t led_auto_animation_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	int aeu_selection, playback_time, start_pause, stop_pause;
+	struct lp5812_led *led = to_lp5812_led(kobj);
+	struct lp5812_chip *chip = led->priv;
+	int pos = 0;
+	int ret;
+
+	mutex_lock(&chip->lock);
+	ret = led_get_autonomous_animation_config(led);
+	if (ret) {
+		ret = -EIO;
+		goto out;
+	}
+
+	/* parse config and feedback to userspace */
+	aeu_selection = led->led_playback.s_led_playback.aeu_selection;
+	playback_time = led->led_playback.s_led_playback.led_playback_time;
+	start_pause = led->start_stop_pause_time.s_time.second;
+	stop_pause = led->start_stop_pause_time.s_time.first;
+	mutex_unlock(&chip->lock);
+
+	pos += sysfs_emit_at(buf, pos, "AEU Select: ");
+	if (aeu_selection == ONLY_AEU1)
+		pos += sysfs_emit_at(buf, pos, "Only use AEU1");
+	else if (aeu_selection == AEU1_AEU2)
+		pos += sysfs_emit_at(buf, pos, "Use AEU1 and AEU2");
+	else
+		pos += sysfs_emit_at(buf, pos, "Use AEU1, AEU2 and AEU3");
+
+	pos += sysfs_emit_at(buf, pos, "; Start pause time: %s",
+						time_name_array[start_pause]);
+	pos += sysfs_emit_at(buf, pos, "; Stop pause time: %s",
+						time_name_array[stop_pause]);
+	pos += sysfs_emit_at(buf, pos, "; LED Playback time: %s",
+						led_playback_time_arr[playback_time]);
+
+	pos += sysfs_emit_at(buf, pos, "\n");
+	pos += sysfs_emit_at(buf, pos, "Command usage: echo (aeu number):(start pause time):");
+	pos += sysfs_emit_at(buf, pos, "(stop pause time):(playback time) > auto_animation\n");
+
+	return pos;
+
+out:
+	mutex_unlock(&chip->lock);
+	return ret;
+}
+
+static ssize_t led_lod_lsd_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	int ret;
+	u8 lsd_status, lod_status;
+	struct lp5812_led *led = to_lp5812_led(kobj);
+	struct lp5812_chip *chip = led->priv;
+
+	mutex_lock(&chip->lock);
+	ret = lp5812_read_lsd_status(chip, led->led_number, &lsd_status);
+	if (!ret)
+		ret = lp5812_read_lod_status(chip, led->led_number, &lod_status);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	return sysfs_emit(buf, "%d %d\n", lod_status, lsd_status);
+}
+
+static ssize_t led_auto_pwm_val_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	int ret;
+	u8 auto_pwm_val;
+	struct lp5812_led *led = to_lp5812_led(kobj);
+	struct lp5812_chip *chip = led->priv;
+
+	mutex_lock(&chip->lock);
+	ret = lp5812_read_auto_pwm_value(chip, led->led_number, &auto_pwm_val);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	return sysfs_emit(buf, "%d\n", auto_pwm_val);
+}
+
+static ssize_t led_aep_status_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	int ret;
+	u8 aep_status;
+	struct lp5812_led *led = to_lp5812_led(kobj);
+	struct lp5812_chip *chip = led->priv;
+
+	mutex_lock(&chip->lock);
+	ret = lp5812_read_aep_status(chip, led->led_number, &aep_status);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	return sysfs_emit(buf, "%d\n", aep_status);
+}
+
+static ssize_t led_pwm_phase_align_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	int val, ret;
+	struct lp5812_led *led = to_lp5812_led(kobj);
+	struct lp5812_chip *chip = led->priv;
+
+	if (sysfs_streq(buf, "forward"))
+		val = 0;
+	else if (sysfs_streq(buf, "middle"))
+		val = 2;
+	else if (sysfs_streq(buf, "backward"))
+		val = 3;
+	else
+		return -EINVAL;
+	mutex_lock(&chip->lock);
+	ret = lp5812_set_phase_align(chip, led->led_number, val);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	return count;
+}
+
+static ssize_t led_pwm_phase_align_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	int val, ret;
+	char *str;
+	struct lp5812_led *led = to_lp5812_led(kobj);
+	struct lp5812_chip *chip = led->priv;
+
+	mutex_lock(&chip->lock);
+	ret = lp5812_get_phase_align(chip, led->led_number, &val);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+	if (val == 0 || val == 1)
+		str = "forward";
+	else if (val == 2)
+		str = "middle";
+	else
+		str = "backward";
+
+	return sysfs_emit(buf, "%s\n", str);
+}
+
+static ssize_t led_pwm_dimming_scale_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	int ret;
+	enum pwm_dimming_scale val;
+	struct lp5812_led *led = to_lp5812_led(kobj);
+	struct lp5812_chip *chip = led->priv;
+
+	if (sysfs_streq(buf, "linear"))
+		val = LINEAR;
+	else if (sysfs_streq(buf, "exponential"))
+		val = EXPONENTIAL;
+	else
+		return -EINVAL;
+
+	mutex_lock(&chip->lock);
+	ret = lp5812_set_pwm_dimming_scale(chip, led->led_number, val);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	return count;
+}
+
+static ssize_t led_pwm_dimming_scale_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	int ret;
+	enum pwm_dimming_scale scale;
+	struct lp5812_led *led = to_lp5812_led(kobj);
+	struct lp5812_chip *chip = led->priv;
+
+	mutex_lock(&chip->lock);
+	ret = lp5812_get_pwm_dimming_scale(chip, led->led_number, &scale);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	return sysfs_emit(buf, "%s\n",
+		scale == EXPONENTIAL ? "exponential" : "linear");
+}
+
+static LP5812_KOBJ_ATTR_RW(activate, led_activate_show, led_activate_store);
+static LP5812_KOBJ_ATTR_RW(mode, led_mode_show, led_mode_store);
+static LP5812_KOBJ_ATTR_RW(manual_dc, led_manual_dc_show,
+		led_manual_dc_store);
+static LP5812_KOBJ_ATTR_RW(manual_pwm, led_manual_pwm_show,
+		led_manual_pwm_store);
+static LP5812_KOBJ_ATTR_RW(autonomous_dc, led_auto_dc_show,
+		led_auto_dc_store);
+static LP5812_KOBJ_ATTR_RW(autonomous_animation, led_auto_animation_show,
+		led_auto_animation_store);
+static LP5812_KOBJ_ATTR_RW(pwm_phase_align, led_pwm_phase_align_show,
+		led_pwm_phase_align_store);
+static LP5812_KOBJ_ATTR_RW(pwm_dimming_scale, led_pwm_dimming_scale_show,
+		led_pwm_dimming_scale_store);
+static LP5812_KOBJ_ATTR_RO(lod_lsd, led_lod_lsd_show);
+static LP5812_KOBJ_ATTR_RO(auto_pwm_val, led_auto_pwm_val_show);
+static LP5812_KOBJ_ATTR_RO(aep_status, led_aep_status_show);
+
+static struct attribute *led_kobj_attributes[] = {
+	&kobj_attr_activate.attr,
+	&kobj_attr_mode.attr,
+	&kobj_attr_manual_dc.attr,
+	&kobj_attr_manual_pwm.attr,
+	&kobj_attr_autonomous_dc.attr,
+	&kobj_attr_autonomous_animation.attr,
+	&kobj_attr_lod_lsd.attr,
+	&kobj_attr_auto_pwm_val.attr,
+	&kobj_attr_aep_status.attr,
+	&kobj_attr_pwm_phase_align.attr,
+	&kobj_attr_pwm_dimming_scale.attr,
+	NULL
+};
+
+static ssize_t aeu_pwm_store(struct kobject *kobj, struct kobj_attribute *attr,
+		const char *buf, size_t count, enum pwm_slope_time_num pwm_channel)
+{
+	int val;
+	int ret;
+	struct anim_engine_unit *aeu = to_anim_engine_unit(kobj);
+	struct lp5812_chip *chip = aeu->led->priv;
+
+	ret = kstrtoint(buf, 0, &val);
+	if (ret)
+		return -EINVAL;
+	if (val < 0 || val > 255)
+		return -EINVAL;
+
+	mutex_lock(&chip->lock);
+	ret = led_aeu_pwm_set_val(aeu, val, pwm_channel);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	return count;
+}
+
+static ssize_t aeu_pwm_show(struct kobject *kobj, struct kobj_attribute *attr,
+		char *buf, enum pwm_slope_time_num pwm_channel)
+{
+	int ret;
+	u8 val;
+	struct anim_engine_unit *aeu = to_anim_engine_unit(kobj);
+	struct lp5812_chip *chip = aeu->led->priv;
+
+	mutex_lock(&chip->lock);
+	ret = led_aeu_pwm_get_val(aeu, &val, pwm_channel);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	return sysfs_emit(buf, "%d\n", val);
+}
+
+static ssize_t aeu_pwm1_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	return aeu_pwm_store(kobj, attr, buf, count, PWM1);
+}
+
+static ssize_t aeu_pwm1_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	return aeu_pwm_show(kobj, attr, buf, PWM1);
+}
+
+static ssize_t aeu_pwm2_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	return aeu_pwm_store(kobj, attr, buf, count, PWM2);
+}
+
+static ssize_t aeu_pwm2_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	return aeu_pwm_show(kobj, attr, buf, PWM2);
+}
+
+static ssize_t aeu_pwm3_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	return aeu_pwm_store(kobj, attr, buf, count, PWM3);
+}
+
+static ssize_t aeu_pwm3_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	return aeu_pwm_show(kobj, attr, buf, PWM3);
+}
+
+static ssize_t aeu_pwm4_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	return aeu_pwm_store(kobj, attr, buf, count, PWM4);
+}
+
+static ssize_t aeu_pwm4_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	return aeu_pwm_show(kobj, attr, buf, PWM4);
+}
+
+static ssize_t aeu_pwm5_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	return aeu_pwm_store(kobj, attr, buf, count, PWM5);
+}
+
+static ssize_t aeu_pwm5_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	return aeu_pwm_show(kobj, attr, buf, PWM5);
+}
+
+static ssize_t aeu_slope_time_store(struct kobject *kobj, struct kobj_attribute *attr,
+		const char *buf, size_t count, enum pwm_slope_time_num slope_time_num)
+{
+	int val;
+	int ret;
+	struct anim_engine_unit *aeu = to_anim_engine_unit(kobj);
+	struct lp5812_chip *chip = aeu->led->priv;
+
+	ret = kstrtoint(buf, 0, &val);
+	if (ret)
+		return -EINVAL;
+
+	if (val < 0x00 || val > 0x0F)
+		return -EINVAL;
+
+	mutex_lock(&chip->lock);
+	ret = led_aeu_slope_time_set_val(aeu, val, slope_time_num);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	return count;
+}
+
+static ssize_t aeu_slope_time_show(struct kobject *kobj, struct kobj_attribute *attr,
+		char *buf, enum pwm_slope_time_num slope_time_num)
+{
+	int ret;
+	u8 val;
+	struct anim_engine_unit *aeu = to_anim_engine_unit(kobj);
+	struct lp5812_chip *chip = aeu->led->priv;
+
+	mutex_lock(&chip->lock);
+	ret = led_aeu_slope_time_get_val(aeu, &val, slope_time_num);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	return sysfs_emit(buf, "%d\n", val);
+}
+
+static ssize_t aeu_slope_time_t1_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	return aeu_slope_time_store(kobj, attr, buf, count, SLOPE_T1);
+}
+
+static ssize_t aeu_slope_time_t1_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	return aeu_slope_time_show(kobj, attr, buf, SLOPE_T1);
+}
+
+static ssize_t aeu_slope_time_t2_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	return aeu_slope_time_store(kobj, attr, buf, count, SLOPE_T2);
+}
+
+static ssize_t aeu_slope_time_t2_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	return aeu_slope_time_show(kobj, attr, buf, SLOPE_T2);
+}
+
+static ssize_t aeu_slope_time_t3_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	return aeu_slope_time_store(kobj, attr, buf, count, SLOPE_T3);
+}
+
+static ssize_t aeu_slope_time_t3_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	return aeu_slope_time_show(kobj, attr, buf, SLOPE_T3);
+}
+
+static ssize_t aeu_slope_time_t4_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	return aeu_slope_time_store(kobj, attr, buf, count, SLOPE_T4);
+}
+
+static ssize_t aeu_slope_time_t4_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	return aeu_slope_time_show(kobj, attr, buf, SLOPE_T4);
+}
+
+static ssize_t aeu_playback_time_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	int val;
+	int ret;
+	struct anim_engine_unit *aeu = to_anim_engine_unit(kobj);
+	struct lp5812_chip *chip = aeu->led->priv;
+
+	ret = kstrtoint(buf, 0, &val);
+	if (ret)
+		return -EINVAL;
+
+	if (val < 0 || val > 3)
+		return -EINVAL;
+
+	mutex_lock(&chip->lock);
+	ret = led_aeu_playback_time_set_val(aeu, val);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	return count;
+}
+
+static ssize_t aeu_playback_time_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	int ret;
+	u8 val;
+	struct anim_engine_unit *aeu = to_anim_engine_unit(kobj);
+	struct lp5812_chip *chip = aeu->led->priv;
+
+	mutex_lock(&chip->lock);
+	ret = led_aeu_playback_time_get_val(aeu, &val);
+	mutex_unlock(&chip->lock);
+	if (ret)
+		return -EIO;
+
+	return sysfs_emit(buf, "%d\n", val);
+}
+
+static LP5812_KOBJ_ATTR_RW(pwm1, aeu_pwm1_show, aeu_pwm1_store);
+static LP5812_KOBJ_ATTR_RW(pwm2, aeu_pwm2_show, aeu_pwm2_store);
+static LP5812_KOBJ_ATTR_RW(pwm3, aeu_pwm3_show, aeu_pwm3_store);
+static LP5812_KOBJ_ATTR_RW(pwm4, aeu_pwm4_show, aeu_pwm4_store);
+static LP5812_KOBJ_ATTR_RW(pwm5, aeu_pwm5_show, aeu_pwm5_store);
+static LP5812_KOBJ_ATTR_RW(slope_time_t1, aeu_slope_time_t1_show,
+		aeu_slope_time_t1_store);
+static LP5812_KOBJ_ATTR_RW(slope_time_t2, aeu_slope_time_t2_show,
+		aeu_slope_time_t2_store);
+static LP5812_KOBJ_ATTR_RW(slope_time_t3, aeu_slope_time_t3_show,
+		aeu_slope_time_t3_store);
+static LP5812_KOBJ_ATTR_RW(slope_time_t4, aeu_slope_time_t4_show,
+		aeu_slope_time_t4_store);
+static LP5812_KOBJ_ATTR_RW(playback_time, aeu_playback_time_show,
+		aeu_playback_time_store);
+
+static struct attribute *aeu_kobj_attributes[] = {
+	&kobj_attr_pwm1.attr,
+	&kobj_attr_pwm2.attr,
+	&kobj_attr_pwm3.attr,
+	&kobj_attr_pwm4.attr,
+	&kobj_attr_pwm5.attr,
+	&kobj_attr_slope_time_t1.attr,
+	&kobj_attr_slope_time_t2.attr,
+	&kobj_attr_slope_time_t3.attr,
+	&kobj_attr_slope_time_t4.attr,
+	&kobj_attr_playback_time.attr,
+	NULL
+};
+
+static void aeu_init_properties(struct lp5812_led *led)
+{
+	int i;
+
+	for (i = 0; i < MAX_AEU; i++) {
+		led->aeu[i].aeu_name = aeu_name_array[i];
+		led->aeu[i].aeu_number = i + 1;
+		led->aeu[i].led = led;
+		led->aeu[i].enabled = 0;
+		led->aeu[i].attr_group.attrs = aeu_kobj_attributes;
+		kobject_init(&led->aeu[i].kobj, &aeu_ktype);
+	}
+}
+
+static int lp5812_probe(struct i2c_client *client)
+{
+	struct lp5812_chip *chip;
+	int i;
+	int ret;
+	u8 val;
+
+	chip = devm_kzalloc(&client->dev, sizeof(struct lp5812_chip),
+			GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+	mutex_init(&chip->lock);
+	chip->i2c_cl = client;
+	chip->dev = &client->dev;
+	chip->regs = &regs;
+	chip->command = NONE;
+	chip->attr_group.name = "lp5812_chip_setup";
+	chip->attr_group.attrs = lp5812_chip_attributes;
+	chip->chip_leds_map = chip_leds_map;
+	chip->u_drive_mode.drive_mode_val = 0x10;
+	chip->u_scan_order.scan_order_val = 0x00;
+
+	/* initialize property for each led */
+	for (i = 0; i < MAX_LEDS; i++) {
+		chip->leds[i].led_name = led_name_array[i];
+		chip->leds[i].led_number = i;
+		chip->leds[i].anim_base_addr = anim_base_addr_array[i];
+		chip->leds[i].enable = 0; /* LED disable as default */
+		chip->leds[i].mode = MANUAL; /* manual mode as default */
+		chip->leds[i].priv = chip;
+		chip->leds[i].led_playback.led_playback_val = 0;
+		chip->leds[i].start_stop_pause_time.time_val = 0;
+		/* sysfs for this led not be created */
+		chip->leds[i].is_sysfs_created = 0;
+		chip->leds[i].attr_group.attrs = led_kobj_attributes;
+		kobject_init(&chip->leds[i].kobj, &led_ktype);
+
+		/* init animation engine unit properties */
+		aeu_init_properties(&chip->leds[i]);
+
+		/* set autonomous animation config as default for all LEDs */
+		led_set_autonomous_animation_config(&chip->leds[i]);
+	}
+
+	i2c_set_clientdata(client, chip);
+
+	ret = sysfs_create_group(&chip->dev->kobj, &chip->attr_group);
+	if (ret)
+		return dev_err_probe(chip->dev, ret, "sysfs_create_group failed\n");
+
+	ret = lp5812_init_dev_config(chip, "tcmscan:4:0:1:2:3", 0);
+	if (ret)
+		return dev_err_probe(chip->dev, ret, "%s: lp5812_init_dev_config failed\n",
+			__func__);
+
+	ret = lp5812_initialize(chip);
+	if (ret)
+		return dev_err_probe(chip->dev, ret, "lp5812 initialize failed\n");
+
+	/* code to verify i2c read/write ok or not */
+	lp5812_read(chip, (u16)DEV_CONFIG2, &val);
+
+	lp5812_write(chip, (u16)LED_A1_AUTO_BASE_ADRR, 0x14);
+	lp5812_read(chip, (u16)LED_A1_AUTO_BASE_ADRR, &val);
+	/* End code to verify i2c read/write*/
+
+	return 0;
+}
+
+static void lp5812_remove(struct i2c_client *client)
+{
+	struct lp5812_chip *chip = i2c_get_clientdata(client);
+
+	mutex_destroy(&chip->lock);
+	leds_remove_existed_sysfs(chip);
+	sysfs_remove_group(&chip->dev->kobj, &chip->attr_group);
+
+	lp5812_disable_all_leds(chip);
+
+	lp5812_enable_disable(chip, 0);
+}
+
+static const struct i2c_device_id lp5812_id[] = {
+	{ "lp5812" },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(i2c, lp5812_id);
+
+#ifdef CONFIG_OF
+static const struct of_device_id of_lp5812_match[] = {
+	{ .compatible = "ti,lp5812", },
+	{/* NULL */}
+};
+
+MODULE_DEVICE_TABLE(of, of_lp5812_match);
+#endif
+
+static struct i2c_driver lp5812_driver = {
+	.driver = {
+		.name   = "lp5812",
+		.of_match_table = of_match_ptr(of_lp5812_match),
+	},
+	.probe          = lp5812_probe,
+	.remove         = lp5812_remove,
+	.id_table       = lp5812_id
+};
+
+module_i2c_driver(lp5812_driver);
+
+MODULE_DESCRIPTION("Texas Instruments LP5812 LED Driver");
+MODULE_AUTHOR("Jared Zhou");
+MODULE_LICENSE("GPL");
diff --git a/drivers/auxdisplay/lp5812.h b/drivers/auxdisplay/lp5812.h
new file mode 100644
index 000000000000..be48b9474116
--- /dev/null
+++ b/drivers/auxdisplay/lp5812.h
@@ -0,0 +1,348 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * LP5812 Driver Header
+ *
+ * Copyright (C) 2025 Texas Instruments
+ *
+ * Author: Jared Zhou <jared-zhou@ti.com>
+ */
+
+#ifndef _LP5812_H_
+#define _LP5812_H_
+
+#include <linux/kernel.h>
+#include <linux/i2c.h>
+#include <linux/sysfs.h>
+#include <linux/mutex.h>
+#include <linux/types.h>
+#include <linux/leds.h>
+#include <linux/delay.h>
+
+#define CHIP_EN_REG                     0x00
+
+#define DEV_CONFIG0                     0x01
+#define DEV_CONFIG1                     0x02
+#define DEV_CONFIG2                     0x03
+#define DEV_CONFIG3                     0x04
+#define DEV_CONFIG4                     0x05
+#define DEV_CONFIG5                     0x06
+#define DEV_CONFIG6                     0x07
+#define DEV_CONFIG7                     0x08
+#define DEV_CONFIG8                     0x09
+#define DEV_CONFIG9                     0x0A
+#define DEV_CONFIG10                    0x0B
+#define DEV_CONFIG11                    0x0c
+#define DEV_CONFIG12                    0x0D
+
+#define CMD_UPDATE_REG                  0x10
+#define CMD_START_REG                   0x11
+#define CMD_STOP_REG                    0x12
+#define CMD_PAUSE_REG                   0x13
+#define CMD_CONTINUE_REG                0x14
+
+#define LED_ENABLE_1_REG                0x20
+#define LED_ENABLE_2_REG                0x21
+
+#define FAULT_CLEAR_REG                 0x22
+#define RESET_REG                       0x23
+
+#define MANUAL_DC_LED_0_REG             0x30
+#define MANUAL_PWM_LED_0_REG            0x40
+#define AUTO_DC_LED_0_REG               0x50
+
+/* value for register */
+#define UPDATE_CMD_VAL                  0x55
+#define START_CMD_VAL                   0xFF
+#define STOP_CMD_VAL                    0xAA
+#define PAUSE_CMD_VAL                   0x33
+#define CONTINUE_CMD_VAL                0xCC
+
+#define CHIP_ENABLE                     0x01
+#define CHIP_DISABLE                    0x00
+
+#define FAULT_CLEAR_ALL                 0x07
+#define TSD_CLEAR_VAL                   0x04
+#define LSD_CLEAR_VAL                   0x02
+#define LOD_CLEAR_VAL                   0x01
+#define RESET_REG_VAL                   0x66
+
+#define LED0_AUTO_BASE_ADRR             0x80
+#define LED1_AUTO_BASE_ADRR             0x9A
+#define LED2_AUTO_BASE_ADRR             0xB4
+#define LED3_AUTO_BASE_ADRR             0xCE
+#define LED_A0_AUTO_BASE_ADRR           0xE8
+#define LED_A1_AUTO_BASE_ADRR           0x102
+#define LED_A2_AUTO_BASE_ADRR           0x11C
+#define LED_B0_AUTO_BASE_ADRR           0x136
+#define LED_B1_AUTO_BASE_ADRR           0x150
+#define LED_B2_AUTO_BASE_ADRR           0x16A
+#define LED_C0_AUTO_BASE_ADRR           0x184
+#define LED_C1_AUTO_BASE_ADRR           0x19E
+#define LED_C2_AUTO_BASE_ADRR           0x1B8
+#define LED_D0_AUTO_BASE_ADRR           0x1D2
+#define LED_D1_AUTO_BASE_ADRR           0x1EC
+#define LED_D2_AUTO_BASE_ADRR           0x206
+
+/* Flag Registers */
+#define TSD_CONFIG_STAT_REG             0x300
+#define LOD_STAT_1_REG                  0x301
+#define LOD_STAT_2_REG                  0x302
+#define LSD_STAT_1_REG                  0x303
+#define LSD_STAT_2_REG                  0x304
+
+#define AUTO_PWM_BASE_ADDR              0x305
+
+#define AEP_STATUS_0_REG                0x315
+#define AEP_STATUS_1_REG                0x316
+#define AEP_STATUS_2_REG                0x317
+#define AEP_STATUS_3_REG                0x318
+#define AEP_STATUS_4_REG                0x319
+#define AEP_STATUS_5_REG                0x31A
+#define AEP_STATUS_6_REG                0x31B
+#define AEP_STATUS_7_REG                0x31C
+
+#define LED0                            "led_0"
+#define LED1                            "led_1"
+#define LED2                            "led_2"
+#define LED3                            "led_3"
+#define LED_A0                          "led_A0"
+#define LED_A1                          "led_A1"
+#define LED_A2                          "led_A2"
+#define LED_B0                          "led_B0"
+#define LED_B1                          "led_B1"
+#define LED_B2                          "led_B2"
+#define LED_C0                          "led_C0"
+#define LED_C1                          "led_C1"
+#define LED_C2                          "led_C2"
+#define LED_D0                          "led_D0"
+#define LED_D1                          "led_D1"
+#define LED_D2                          "led_D2"
+
+/* Below define time for (start/stop/slope time) */
+#define TIME0                           "no time"
+#define TIME1                           "0.09s"
+#define TIME2                           "0.18s"
+#define TIME3                           "0.36s"
+#define TIME4                           "0.54s"
+#define TIME5                           "0.80s"
+#define TIME6                           "1.07s"
+#define TIME7                           "1.52s"
+#define TIME8                           "2.06s"
+#define TIME9                           "2.50s"
+#define TIME10                          "3.04s"
+#define TIME11                          "4.02s"
+#define TIME12                          "5.01s"
+#define TIME13                          "5.99s"
+#define TIME14                          "7.06s"
+#define TIME15                          "8.05s"
+/* End define time for (start/stop/slope time) */
+
+#define AEU1                            "AEU1"
+#define AEU2                            "AEU2"
+#define AEU3                            "AEU3"
+
+#define MAX_LEDS                        16
+#define MAX_TIME                        16
+#define MAX_AEU                         3
+
+#define LP5812_DEV_ATTR_RW(name)   \
+	DEVICE_ATTR_RW(name)
+#define LP5812_DEV_ATTR_RO(name)          \
+	DEVICE_ATTR_RO(name)
+#define LP5812_DEV_ATTR_WO(name)         \
+	DEVICE_ATTR_WO(name)
+
+#define LP5812_KOBJ_ATTR(_name, _mode, _show, _store) \
+	struct kobj_attribute kobj_attr_##_name = __ATTR(_name, _mode, _show, _store)
+#define LP5812_KOBJ_ATTR_RW(name, show, store) \
+	LP5812_KOBJ_ATTR(name, 0644, show, store)
+#define LP5812_KOBJ_ATTR_RO(name, show) \
+	LP5812_KOBJ_ATTR(name, 0444, show, NULL)
+#define LP5812_KOBJ_ATTR_WO(name, store) \
+	LP5812_KOBJ_ATTR(name, 0200, NULL, store)
+
+enum pwm_slope_time_num {
+	PWM1 = 1,
+	PWM2,
+	PWM3,
+	PWM4,
+	PWM5,
+	SLOPE_T1,
+	SLOPE_T2,
+	SLOPE_T3,
+	SLOPE_T4
+};
+
+enum dimming_type {
+	ANALOG,
+	PWM
+};
+
+enum pwm_dimming_scale {
+	LINEAR = 0,
+	EXPONENTIAL
+};
+
+enum control_mode {
+	MANUAL = 0,
+	AUTONOMOUS
+};
+
+enum device_command {
+	NONE,
+	UPDATE,
+	START,
+	STOP,
+	PAUSE,
+	CONTINUE
+};
+
+enum animation_addr {
+	AUTO_PAUSE = 0,
+	AUTO_PLAYBACK,
+	AEU1_PWM_1,
+	AEU1_PWM_2,
+	AEU1_PWM_3,
+	AEU1_PWM_4,
+	AEU1_PWM_5,
+	AEU1_T12,
+	AEU1_T34,
+	AEU1_PLAYBACK,
+	AEU2_PWM_1,
+	AEU2_PWM_2,
+	AEU2_PWM_3,
+	AEU2_PWM_4,
+	AEU2_PWM_5,
+	AEU2_T12,
+	AEU2_T34,
+	AEU2_PLAYBACK,
+	AEU3_PWM_1,
+	AEU3_PWM_2,
+	AEU3_PWM_3,
+	AEU3_PWM_4,
+	AEU3_PWM_5,
+	AEU3_T12,
+	AEU3_T34,
+	AEU3_PLAYBACK
+};
+
+enum drive_mode {
+	DIRECT_MODE = 0,
+	TCM_1_SCAN,
+	TCM_2_SCAN,
+	TCM_3_SCAN,
+	TCM_4_SCAN,
+	MIX_1_SCAN,
+	MIX_2_SCAN,
+	MIX_3_SCAN
+};
+
+enum aeu_select {
+	ONLY_AEU1,
+	AEU1_AEU2,
+	AEU1_AEU2_AEU3
+};
+
+union time {
+	struct {
+		u8 first:4;
+		u8 second:4;
+	} __packed s_time;
+	u8 time_val;
+}; /* type for start/stop pause time and slope time */
+
+union led_playback {
+	struct {
+		u8 led_playback_time:4;
+		u8 aeu_selection:2;
+		u8 reserved:2;
+	} __packed s_led_playback;
+	u8 led_playback_val;
+};
+
+union scan_order {
+	struct {
+		u8 scan_order_0:2;
+		u8 scan_order_1:2;
+		u8 scan_order_2:2;
+		u8 scan_order_3:2;
+	} __packed s_scan_order;
+	u8 scan_order_val;
+};
+
+union drive_mode_info {
+	struct {
+		u8 mix_sel_led_0:1;
+		u8 mix_sel_led_1:1;
+		u8 mix_sel_led_2:1;
+		u8 mix_sel_led_3:1;
+		u8 led_mode:3;
+		u8 pwm_fre:1;
+	} __packed s_drive_mode;
+	u8 drive_mode_val;
+};
+
+struct drive_mode_led_map {
+	const char *drive_mode;
+	const char **led_arr;
+};
+
+struct lp5812_specific_regs {
+	u16 enable_reg;
+	u16 reset_reg;
+	u16 update_cmd_reg;
+	u16 start_cmd_reg;
+	u16 stop_cmd_reg;
+	u16 pause_cmd_reg;
+	u16 continue_cmd_reg;
+	u16 fault_clear_reg;
+	u16 tsd_config_status_reg;
+};
+
+struct anim_engine_unit {
+	struct kobject                        kobj;
+	struct lp5812_led                     *led;
+	struct attribute_group                attr_group;
+	const char                            *aeu_name;
+	int                                   aeu_number; /* start from 1 */
+
+	/* To know led using this AEU or not*/
+	int                                   enabled;
+};
+
+struct lp5812_led {
+	struct kobject                        kobj;
+	struct lp5812_chip                    *priv;
+	struct attribute_group                attr_group;
+	int                                   enable;
+	enum control_mode                     mode;
+	enum dimming_type                     dimming_type;
+	u8                                    lod_lsd;
+	u8                                    auto_pwm;
+	u8                                    aep_status;
+	u16                                   anim_base_addr;
+	int                                   led_number; /* start from 0 */
+	int                                   is_sysfs_created;
+	const char                            *led_name;
+
+	union led_playback                    led_playback;
+	union time                            start_stop_pause_time;
+
+	struct anim_engine_unit               aeu[MAX_AEU];
+};
+
+struct lp5812_chip {
+	struct i2c_client                     *i2c_cl;
+	struct mutex                          lock; /* Protects access to device registers */
+	struct device                         *dev;
+	struct attribute_group                attr_group;
+	const struct lp5812_specific_regs     *regs;
+	const struct drive_mode_led_map       *chip_leds_map;
+	enum device_command                   command;
+	union scan_order                      u_scan_order;
+	union drive_mode_info                 u_drive_mode;
+
+	struct lp5812_led                     leds[MAX_LEDS];
+};
+
+#endif /*_LP5812_H_*/
-- 
2.25.1

Re: [PATCH v8 2/5] auxdisplay: add TI LP5812 4×3 Matrix RGB LED Driver

[Randy Dunlap]

Hi--

On 4/27/25 1:24 AM, Nam Tran wrote:
> diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
> index bedc6133f970..896f02b9a06b 100644
> --- a/drivers/auxdisplay/Kconfig
> +++ b/drivers/auxdisplay/Kconfig
> @@ -539,6 +539,23 @@ config ARM_CHARLCD
>  	  line and the Linux version on the second line, but that's
>  	  still useful.
>  
> +#
> +# TI LP5812 matrix RGB LED driver section
> +#
> +config LP5812

Missing one tab of indentation on all lines below here:

> +tristate "Enable LP5812 support LED matrix 4x3"
> +depends on I2C
> +help
> +  If you say Y here you get support for TI LP5812 LED driver.
> +
> +  The LP5812 is a 4 × 3 matrix RGB LED driver with autonomous
> +  animation engine control.
> +
> +  To compile this driver as a module, choose M here: the
> +  module will be called lp5812.
> +
> +  If unsure, say N.
> +
>  endif # AUXDISPLAY

-- 
~Randy

Re: [PATCH v8 2/5] auxdisplay: add TI LP5812 4×3 Matrix RGB LED Driver

[Nam Tran]

On Sun, 27 Apr 2025 Randy Dunlap wrote:

> Hi--
> 
> On 4/27/25 1:24 AM, Nam Tran wrote:
> > diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
> > index bedc6133f970..896f02b9a06b 100644
> > --- a/drivers/auxdisplay/Kconfig
> > +++ b/drivers/auxdisplay/Kconfig
> > @@ -539,6 +539,23 @@ config ARM_CHARLCD
> >  	  line and the Linux version on the second line, but that's
> >  	  still useful.
> >  
> > +#
> > +# TI LP5812 matrix RGB LED driver section
> > +#
> > +config LP5812
> 
> Missing one tab of indentation on all lines below here:
> 
> > +tristate "Enable LP5812 support LED matrix 4x3"
> > +depends on I2C
> > +help
> > +  If you say Y here you get support for TI LP5812 LED driver.
> > +
> > +  The LP5812 is a 4 × 3 matrix RGB LED driver with autonomous
> > +  animation engine control.
> > +
> > +  To compile this driver as a module, choose M here: the
> > +  module will be called lp5812.
> > +
> > +  If unsure, say N.
> > +
> >  endif # AUXDISPLAY

Thank you for pointing that out.
It was my mistake, and I’ll correct it.

Best regards,
Nam Tran

[PATCH v8 3/5] docs: ABI: Document LP5812 sysfs interfaces

[Nam Tran]

The LP5812 is a 4 × 3 matrix RGB LED driver
with autonomous animation engine control.

The driver provides interfaces to configure
LED modes manual/autonomous, set PWM/DC values,
and manage autonomous animation engines.

Signed-off-by: Nam Tran <trannamatk@gmail.com>
---
 .../ABI/testing/sysfs-bus-i2c-devices-lp5812  | 144 ++++++++++++++++++
 MAINTAINERS                                   |   1 +
 2 files changed, 145 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c-devices-lp5812

diff --git a/Documentation/ABI/testing/sysfs-bus-i2c-devices-lp5812 b/Documentation/ABI/testing/sysfs-bus-i2c-devices-lp5812
new file mode 100644
index 000000000000..04689663a643
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-i2c-devices-lp5812
@@ -0,0 +1,144 @@
+What:		/sys/bus/i2c/devices/.../lp5812_chip_setup/device_enable
+Date:		May 2025
+KernelVersion:	6.16
+Contact:	Nam Tran <trannamatk@gmail.com>
+Description:
+        Enables or disables the LP5812 device. (RW)
+        0 - Disable
+        1 - Enable
+
+What:		/sys/bus/i2c/devices/.../lp5812_chip_setup/dev_config
+Date:		May 2025
+KernelVersion:	6.16
+Contact:	Nam Tran <trannamatk@gmail.com>
+Description:
+        Configures drive mode and scan order. (RW)
+        Some valid values: tcmscan:4:0:1:2:3 (default), tcmscan:3:0:1:2, mixscan:2:2:0:3, mixscan:3:0:1:2:3
+
+What:		/sys/bus/i2c/devices/.../lp5812_chip_setup/device_command
+Date:		May 2025
+KernelVersion:	6.16
+Contact:	Nam Tran <trannamatk@gmail.com>
+Description:
+        Issues device-level commands. (WO)
+        Valid values: "update", "start", "stop", "pause", "continue"
+
+What:		/sys/bus/i2c/devices/.../lp5812_chip_setup/sw_reset
+Date:		May 2025
+KernelVersion:	6.16
+Contact:	Nam Tran <trannamatk@gmail.com>
+Description:
+        Triggers a software reset of the device. (WO)
+        1 - resets device
+        0 - does not reset device
+
+What:		/sys/bus/i2c/devices/.../lp5812_chip_setup/fault_clear
+Date:		May 2025
+KernelVersion:	6.16
+Contact:	Nam Tran <trannamatk@gmail.com>
+Description:
+        Clears fault status. (WO)
+        1 - clears fault status
+        0 - does not clear fault status
+
+What:		/sys/bus/i2c/devices/.../lp5812_chip_setup/tsd_config_status
+Date:		May 2025
+KernelVersion:	6.16
+Contact:	Nam Tran <trannamatk@gmail.com>
+Description:
+        Report the current thermal shutdown config status. (RO)
+
+What:		/sys/bus/i2c/devices/.../led_<id>/activate
+Date:		May 2025
+KernelVersion:	6.16
+Contact:	Nam Tran <trannamatk@gmail.com>
+Description:
+        Activate or deactivate the specified LED channel. (RW)
+        1 - Activate
+        0 - Deactivate
+
+What:		/sys/bus/i2c/devices/.../led_<id>/mode
+Date:		May 2025
+KernelVersion:	6.16
+Contact:	Nam Tran <trannamatk@gmail.com>
+Description:
+        Selects LED operation mode. (RW)
+        Valid values: "manual", "autonomous"
+
+What:		/sys/bus/i2c/devices/.../led_<id>/manual_dc
+Date:		May 2025
+KernelVersion:	6.16
+Contact:	Nam Tran <trannamatk@gmail.com>
+Description:
+        DC current level in manual mode. (RW)
+        Valid values: 0 - 255
+
+What:		/sys/bus/i2c/devices/.../led_<id>/manual_pwm
+Date:		May 2025
+KernelVersion:	6.16
+Contact:	Nam Tran <trannamatk@gmail.com>
+Description:
+        PWM duty cycle in manual mode. (RW)
+        Valid values: 0 - 255
+
+What:		/sys/bus/i2c/devices/.../led_<id>/autonomous_dc
+Date:		May 2025
+KernelVersion:	6.16
+Contact:	Nam Tran <trannamatk@gmail.com>
+Description:
+        DC current level used in autonomous mode. (RW)
+        Valid values: 0 - 255
+
+What:		/sys/bus/i2c/devices/.../led_<id>/autonomous_dc
+Date:		May 2025
+KernelVersion:	6.16
+Contact:	Nam Tran <trannamatk@gmail.com>
+Description:
+        DC current level used in autonomous mode. (RW)
+        Valid values: 0 - 255
+
+What:		/sys/bus/i2c/devices/.../led_<id>/pwm_dimming_scale
+Date:		May 2025
+KernelVersion:	6.16
+Contact:	Nam Tran <trannamatk@gmail.com>
+Description:
+        PWM dimming scale type. (RW)
+        Valid values: "linear", "exponential"
+
+What:		/sys/bus/i2c/devices/.../led_<id>/pwm_phase_align
+Date:		May 2025
+KernelVersion:	6.16
+Contact:	Nam Tran <trannamatk@gmail.com>
+Description:
+        Configures PWM phase alignment. (RW)
+        Valid values: "forward", "middle", "backward"
+
+What:		/sys/bus/i2c/devices/.../led_<id>/autonomous_animation
+Date:		May 2025
+KernelVersion:	6.16
+Contact:	Nam Tran <trannamatk@gmail.com>
+Description:
+        Controls AEU configuration and playback. (RW)
+        Format: (aeu number):(start pause time):(stop pause time):(playback time)
+        with aeu number 1, 2, 3; playback time 0 - 15
+
+What:		/sys/bus/i2c/devices/.../led_<id>/aep_status
+Date:		May 2025
+KernelVersion:	6.16
+Contact:	Nam Tran <trannamatk@gmail.com>
+Description:
+        Shows current animation pattern status, value from 0 to 7. (RO)
+
+What:		/sys/bus/i2c/devices/.../led_<id>/auto_pwm_val
+Date:		May 2025
+KernelVersion:	6.16
+Contact:	Nam Tran <trannamatk@gmail.com>
+Description:
+        Shows the pwm value in autonomous mode when pause the animation, value from 0 to 255. (RO)
+
+What:		/sys/bus/i2c/devices/.../led_<id>/lod_lsd
+Date:		May 2025
+KernelVersion:	6.16
+Contact:	Nam Tran <trannamatk@gmail.com>
+Description:
+        0 0 mean no lod and lsd fault detected, 1 1 mean lod and lsd fault detected (RO)
diff --git a/MAINTAINERS b/MAINTAINERS
index 091373b01dc2..5d8ee0801c69 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -24020,6 +24020,7 @@ TEXAS INSTRUMENTS' LP5812 4X3 MATRIX LED DRIVER
 M:	Nam Tran <trannamatk@gmail.com>
 L:	linux-kernel@vger.kernel.org
 S:	Maintained
+F:	Documentation/ABI/testing/sysfs-bus-i2c-devices-lp5812
 F:	Documentation/devicetree/bindings/auxdisplay/ti,lp5812.yaml
 F:	drivers/auxdisplay/Kconfig
 F:	drivers/auxdisplay/Makefile
-- 
2.25.1

[PATCH v8 4/5] docs: auxdisplay: document TI LP5812 RGB LED driver

[Nam Tran]

The driver provides sysfs interfaces to control
and configure the LP5812 device and its LED channels.

The documetation describes the chip's capabilities,
sysfs interface, and usage examples.

Signed-off-by: Nam Tran <trannamatk@gmail.com>
---
 .../admin-guide/auxdisplay/index.rst          |  3 +-
 .../admin-guide/auxdisplay/lp5812.rst         | 79 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 3 files changed, 82 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/admin-guide/auxdisplay/lp5812.rst

diff --git a/Documentation/admin-guide/auxdisplay/index.rst b/Documentation/admin-guide/auxdisplay/index.rst
index e466f0595248..5d627f067c77 100644
--- a/Documentation/admin-guide/auxdisplay/index.rst
+++ b/Documentation/admin-guide/auxdisplay/index.rst
@@ -5,8 +5,9 @@ Auxiliary Display Support
 .. toctree::
     :maxdepth: 1
 
-    ks0108.rst
     cfag12864b.rst
+    ks0108.rst
+    lp5812.rst
 
 .. only::  subproject and html
 
diff --git a/Documentation/admin-guide/auxdisplay/lp5812.rst b/Documentation/admin-guide/auxdisplay/lp5812.rst
new file mode 100644
index 000000000000..58d716f899d1
--- /dev/null
+++ b/Documentation/admin-guide/auxdisplay/lp5812.rst
@@ -0,0 +1,79 @@
+========================
+Kernel driver for lp5812
+========================
+
+* TI/National Semiconductor LP5812 LED Driver
+* Datasheet: https://www.ti.com/product/LP5812#tech-docs
+
+Authors: Jared Zhou <jared-zhou@ti.com>
+
+Description
+===========
+
+The LP5812 is a 4x3 matrix LED driver with support for both manual and
+autonomous animation control. It provides features such as:
+
+- PWM dimming and DC current control
+- Slope time configuration
+- Autonomous Engine Unit (AEU) for LED animation playback
+- Flexible scan and drive mode configuration
+
+This driver provides sysfs interfaces to control and configure the LP5812
+device and its LED channels.
+
+Sysfs Interface
+===============
+
+LP5812 device exposes a chip-level sysfs group:
+  /sys/bus/i2c/devices/<i2c-dev-addr>/lp5812_chip_setup/
+
+The following attributes are available at chip level:
+  - device_enable: Enable/disable the device (RW)
+  - dev_config: Configure drive mode and scan order (RW)
+  - device_command: Issue device-wide commands (WO)
+  - sw_reset: Reset the hardware (WO)
+  - fault_clear: Clear any device faults (WO)
+  - tsd_config_status: Read thermal shutdown config status (RO)
+
+Each LED channel is exposed as:
+  /sys/bus/i2c/devices/<i2c-dev-addr>/led_<id>/
+
+Each LED exposes the following attributes:
+  - activate: Activate or deactivate the LED (RW)
+  - mode: manual or autonomous mode (RW)
+  - manual_dc: DC current value (0–255) (RW)
+  - manual_pwm: PWM duty cycle (0–255) (RW)
+  - autonomous_dc: DC current in autonomous mode (RW)
+  - pwm_dimming_scale: linear or exponential (RW)
+  - pwm_phase_align: PWM alignment mode (RW)
+  - autonomous_animation: Config autonomous animation mode with aeu number, start pause time, stop pause time, playback time (RW)
+  - aep_status: autonomous engine pattern status (RO)
+  - auto_pwm_val: pwm value in autonomous mode when pause the animation (RO)
+  - lod_lsd: lod and lsd fault detected status (RO)
+
+Example Usage
+=============
+
+To control led_A0 in manual mode::
+    echo 1 > /sys/bus/i2c/drivers/lp5812/xxxx/lp5812_chip_setup/device_enable
+    echo 1 > /sys/bus/i2c/drivers/lp5812/xxxx/led_A0/activate
+    echo manual > /sys/bus/i2c/drivers/lp5812/xxxx/led_A0/mode
+    echo 100 > /sys/bus/i2c/drivers/lp5812/xxxx/led_A0/manual_dc
+    echo 100 > /sys/bus/i2c/drivers/lp5812/xxxx/led_A0/manual_pwm
+
+To control led_A0 in autonomous mode::
+    echo 1 > /sys/bus/i2c/drivers/lp5812/xxxx/lp5812_chip_setup/device_enable
+    echo 1 > /sys/bus/i2c/drivers/lp5812/xxxx/led_A0/activate
+    echo autonomous > /sys/bus/i2c/drivers/lp5812/xxxx/led_A0/mode
+    echo 1:10:10:15 > /sys/bus/i2c/drivers/lp5812/xxxx/led_A0/autonomous_animation
+    echo 100 > /sys/bus/i2c/drivers/lp5812/xxxx/led_A0/AEU1/pwm1
+    echo 100 > /sys/bus/i2c/drivers/lp5812/xxxx/led_A0/AEU1/pwm2
+    echo 100 > /sys/bus/i2c/drivers/lp5812/xxxx/led_A0/AEU1/pwm3
+    echo 100 > /sys/bus/i2c/drivers/lp5812/xxxx/led_A0/AEU1/pwm4
+    echo 100 > /sys/bus/i2c/drivers/lp5812/xxxx/led_A0/AEU1/pwm5
+    echo 5 > /sys/bus/i2c/drivers/lp5812/xxxx/led_A0/AEU1/slope_time_t1
+    echo 5 > /sys/bus/i2c/drivers/lp5812/xxxx/led_A0/AEU1/slope_time_t2
+    echo 5 > /sys/bus/i2c/drivers/lp5812/xxxx/led_A0/AEU1/slope_time_t3
+    echo 5 > /sys/bus/i2c/drivers/lp5812/xxxx/led_A0/AEU1/slope_time_t4
+    echo 1 > /sys/bus/i2c/drivers/lp5812/xxxx/led_A0/AEU1/playback_time
+    echo start > /sys/bus/i2c/drivers/lp5812/xxxx/lp5812_chip_setup/device_command
diff --git a/MAINTAINERS b/MAINTAINERS
index 5d8ee0801c69..dd504dee3274 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -24021,6 +24021,7 @@ M:	Nam Tran <trannamatk@gmail.com>
 L:	linux-kernel@vger.kernel.org
 S:	Maintained
 F:	Documentation/ABI/testing/sysfs-bus-i2c-devices-lp5812
+F:	Documentation/admin-guide/auxdisplay/lp5812.rst
 F:	Documentation/devicetree/bindings/auxdisplay/ti,lp5812.yaml
 F:	drivers/auxdisplay/Kconfig
 F:	drivers/auxdisplay/Makefile
-- 
2.25.1

[PATCH v8 5/5] arm64: dts: Add LP5812 node for Raspberry Pi 4 Model B

[Nam Tran]

Add the LP5812 LED driver node to the Device Tree for Raspberry Pi 4 B.
This enables the LED connected to the LP5812 to be controlled via I2C.

Signed-off-by: Nam Tran <trannamatk@gmail.com>
---
 arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts
index 353bb50ce542..2f058cf5d76e 100644
--- a/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts
+++ b/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts
@@ -152,6 +152,16 @@ &hdmi1 {
 	status = "okay";
 };
 
+&i2c1 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	led-controller@1b {
+		compatible = "ti,lp5812";
+		reg = <0x1b>;
+	};
+};
+
 &led_act {
 	gpios = <&gpio 42 GPIO_ACTIVE_HIGH>;
 };
-- 
2.25.1

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Andy Shevchenko]

On Sun, Apr 27, 2025 at 11:25 AM Nam Tran <trannamatk@gmail.com> wrote:
>
> This patch series adds support for the TI/National Semiconductor LP5812
> 4x3 matrix RGB LED driver. The driver supports features such as autonomous
> animation and time-cross-multiplexing (TCM) for dynamic LED effects.
>
> Signed-off-by: Nam Tran <trannamatk@gmail.com>
> ---
> Changes in v8:
> - Move driver to drivers/auxdisplay/ instead of drivers/leds/.
> - Rename files from leds-lp5812.c/.h to lp5812.c/.h.
> - Move ti,lp5812.yaml binding to auxdisplay/ directory,
>   and update the title and $id to match new path.
> - No functional changes to the binding itself (keep Reviewed-by).
> - Update commit messages and patch titles to reflect the move.
> - Link to v7: https://lore.kernel.org/linux-leds/20250422190121.46839-1-trannamatk@gmail.com/

Out of sudden without discussing with auxdisplay maintainers/reviewers?
Thanks, no.
Please, put into the cover letter the meaningful summary of what's
going on and why this becomes an auxdisplay issue. Brief review of the
bindings sounds more likely like LEDS or PWM subsystems.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Pavel Machek]

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

Hi!

> > This patch series adds support for the TI/National Semiconductor LP5812
> > 4x3 matrix RGB LED driver. The driver supports features such as autonomous
> > animation and time-cross-multiplexing (TCM) for dynamic LED effects.
> >
> > Signed-off-by: Nam Tran <trannamatk@gmail.com>
> > ---
> > Changes in v8:
> > - Move driver to drivers/auxdisplay/ instead of drivers/leds/.
> > - Rename files from leds-lp5812.c/.h to lp5812.c/.h.
> > - Move ti,lp5812.yaml binding to auxdisplay/ directory,
> >   and update the title and $id to match new path.
> > - No functional changes to the binding itself (keep Reviewed-by).
> > - Update commit messages and patch titles to reflect the move.
> > - Link to v7: https://lore.kernel.org/linux-leds/20250422190121.46839-1-trannamatk@gmail.com/
> 
> Out of sudden without discussing with auxdisplay maintainers/reviewers?
> Thanks, no.
> Please, put into the cover letter the meaningful summary of what's
> going on and why this becomes an auxdisplay issue. Brief review of the
> bindings sounds more likely like LEDS or PWM subsystems.

It is 4x3 matrix. That means it is not suitable for LEDs. I don't
believe it is suitable for PWM, either -- yes, it is 36 PWM outputs,
but...

Best regards,
								Pavel
-- 
I don't work for Nazis and criminals, and neither should you.
Boycott Putin, Trump, and Musk!

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

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Geert Uytterhoeven]

Hi Pavel,

On Mon, 28 Apr 2025 at 12:37, Pavel Machek <pavel@ucw.cz> wrote:
> > > This patch series adds support for the TI/National Semiconductor LP5812
> > > 4x3 matrix RGB LED driver. The driver supports features such as autonomous
> > > animation and time-cross-multiplexing (TCM) for dynamic LED effects.
> > >
> > > Signed-off-by: Nam Tran <trannamatk@gmail.com>
> > > ---
> > > Changes in v8:
> > > - Move driver to drivers/auxdisplay/ instead of drivers/leds/.
> > > - Rename files from leds-lp5812.c/.h to lp5812.c/.h.
> > > - Move ti,lp5812.yaml binding to auxdisplay/ directory,
> > >   and update the title and $id to match new path.
> > > - No functional changes to the binding itself (keep Reviewed-by).
> > > - Update commit messages and patch titles to reflect the move.
> > > - Link to v7: https://lore.kernel.org/linux-leds/20250422190121.46839-1-trannamatk@gmail.com/
> >
> > Out of sudden without discussing with auxdisplay maintainers/reviewers?
> > Thanks, no.
> > Please, put into the cover letter the meaningful summary of what's
> > going on and why this becomes an auxdisplay issue. Brief review of the
> > bindings sounds more likely like LEDS or PWM subsystems.
>
> It is 4x3 matrix. That means it is not suitable for LEDs. I don't
> believe it is suitable for PWM, either -- yes, it is 36 PWM outputs,
> but...

Is it intended to be used as a 4x3 matrix, or is this just an internal
wiring detail, and should it be exposed as 12 individual LEDs instead?

BTW, my first guess was that you just wanted to avoid the LED
maintainers becoming responsible for the extensive sysfs interface ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Nam Tran]

Hi Geert, Pavel,

Thank you, Pavel, for the confirmation.
Thank you, Geert, for the review and the question.

I would like to make it clearer.

On Mon, 28 Apr 2025 Geert Uytterhoeven wrote:

> Hi Pavel,
> 
> On Mon, 28 Apr 2025 at 12:37, Pavel Machek <pavel@ucw.cz> wrote:
> > > > This patch series adds support for the TI/National Semiconductor LP5812
> > > > 4x3 matrix RGB LED driver. The driver supports features such as autonomous
> > > > animation and time-cross-multiplexing (TCM) for dynamic LED effects.
> > > >
> > > > Signed-off-by: Nam Tran <trannamatk@gmail.com>
> > > > ---
> > > > Changes in v8:
> > > > - Move driver to drivers/auxdisplay/ instead of drivers/leds/.
> > > > - Rename files from leds-lp5812.c/.h to lp5812.c/.h.
> > > > - Move ti,lp5812.yaml binding to auxdisplay/ directory,
> > > >   and update the title and $id to match new path.
> > > > - No functional changes to the binding itself (keep Reviewed-by).
> > > > - Update commit messages and patch titles to reflect the move.
> > > > - Link to v7: https://lore.kernel.org/linux-leds/20250422190121.46839-1-trannamatk@gmail.com/
> > >
> > > Out of sudden without discussing with auxdisplay maintainers/reviewers?
> > > Thanks, no.
> > > Please, put into the cover letter the meaningful summary of what's
> > > going on and why this becomes an auxdisplay issue. Brief review of the
> > > bindings sounds more likely like LEDS or PWM subsystems.
> >
> > It is 4x3 matrix. That means it is not suitable for LEDs. I don't
> > believe it is suitable for PWM, either -- yes, it is 36 PWM outputs,
> > but...
> 
> Is it intended to be used as a 4x3 matrix, or is this just an internal
> wiring detail, and should it be exposed as 12 individual LEDs instead?

The 4×3 matrix is a real and fundamental aspect of the LP5812’s operation.
It is not just an internal wiring detail.
The device adopts a Time-Cross-Multiplexing (TCM) structure, where 4 output
pins control 12 LED dots individually through scanning. Each pin includes
both high-side and low-side drive circuits, meaning matrix multiplexing is
required for proper operation — it cannot be treated as 12 completely
independent LEDs.

Best regards,
Nam Tran

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Pavel Machek]

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

Hi!

> Thank you, Pavel, for the confirmation.
> Thank you, Geert, for the review and the question.
> 
> I would like to make it clearer.
> 
> On Mon, 28 Apr 2025 Geert Uytterhoeven wrote:
> 

> > > > > - Move driver to drivers/auxdisplay/ instead of drivers/leds/.
> > > > > - Rename files from leds-lp5812.c/.h to lp5812.c/.h.
> > > > > - Move ti,lp5812.yaml binding to auxdisplay/ directory,
> > > > >   and update the title and $id to match new path.
> > > > > - No functional changes to the binding itself (keep Reviewed-by).
> > > > > - Update commit messages and patch titles to reflect the move.
> > > > > - Link to v7: https://lore.kernel.org/linux-leds/20250422190121.46839-1-trannamatk@gmail.com/
> > > >
> > > > Out of sudden without discussing with auxdisplay maintainers/reviewers?
> > > > Thanks, no.
> > > > Please, put into the cover letter the meaningful summary of what's
> > > > going on and why this becomes an auxdisplay issue. Brief review of the
> > > > bindings sounds more likely like LEDS or PWM subsystems.
> > >
> > > It is 4x3 matrix. That means it is not suitable for LEDs. I don't
> > > believe it is suitable for PWM, either -- yes, it is 36 PWM outputs,
> > > but...
> > 
> > Is it intended to be used as a 4x3 matrix, or is this just an internal
> > wiring detail, and should it be exposed as 12 individual LEDs instead?
> 
> The 4×3 matrix is a real and fundamental aspect of the LP5812’s operation.
> It is not just an internal wiring detail.
> The device adopts a Time-Cross-Multiplexing (TCM) structure, where 4 output
> pins control 12 LED dots individually through scanning. Each pin includes
> both high-side and low-side drive circuits, meaning matrix multiplexing is
> required for proper operation — it cannot be treated as 12 completely
> independent LEDs.

Scanning is really a detail.

If this is used as rectangular 4x3 display, then it goes to auxdisplay.

If this is used as a power LED, SD activity LED, capslock and numlock
... placed randomly all around the device, then it goes LED subsystem.

Best regards,
								Pavel
-- 
I don't work for Nazis and criminals, and neither should you.
Boycott Putin, Trump, and Musk!

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

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Nam Tran]

On Mon, 28 Apr 2025 Pavel Machek wrote:

> Hi!
> 
> > Thank you, Pavel, for the confirmation.
> > Thank you, Geert, for the review and the question.
> > 
> > I would like to make it clearer.
> > 
> > On Mon, 28 Apr 2025 Geert Uytterhoeven wrote:
> > 
> 
> > > > > > - Move driver to drivers/auxdisplay/ instead of drivers/leds/.
> > > > > > - Rename files from leds-lp5812.c/.h to lp5812.c/.h.
> > > > > > - Move ti,lp5812.yaml binding to auxdisplay/ directory,
> > > > > >   and update the title and $id to match new path.
> > > > > > - No functional changes to the binding itself (keep Reviewed-by).
> > > > > > - Update commit messages and patch titles to reflect the move.
> > > > > > - Link to v7: https://lore.kernel.org/linux-leds/20250422190121.46839-1-trannamatk@gmail.com/
> > > > >
> > > > > Out of sudden without discussing with auxdisplay maintainers/reviewers?
> > > > > Thanks, no.
> > > > > Please, put into the cover letter the meaningful summary of what's
> > > > > going on and why this becomes an auxdisplay issue. Brief review of the
> > > > > bindings sounds more likely like LEDS or PWM subsystems.
> > > >
> > > > It is 4x3 matrix. That means it is not suitable for LEDs. I don't
> > > > believe it is suitable for PWM, either -- yes, it is 36 PWM outputs,
> > > > but...
> > > 
> > > Is it intended to be used as a 4x3 matrix, or is this just an internal
> > > wiring detail, and should it be exposed as 12 individual LEDs instead?
> > 
> > The 4×3 matrix is a real and fundamental aspect of the LP5812’s operation.
> > It is not just an internal wiring detail.
> > The device adopts a Time-Cross-Multiplexing (TCM) structure, where 4 output
> > pins control 12 LED dots individually through scanning. Each pin includes
> > both high-side and low-side drive circuits, meaning matrix multiplexing is
> > required for proper operation — it cannot be treated as 12 completely
> > independent LEDs.
> 
> Scanning is really a detail.
> 
> If this is used as rectangular 4x3 display, then it goes to auxdisplay.
> 
> If this is used as a power LED, SD activity LED, capslock and numlock
> ... placed randomly all around the device, then it goes LED subsystem.

The LP5812 is used for LED status indication in devices like smart speakers,
wearables, and routers, not as a structured rectangular display.

Given that, it seems to match the LED subsystem better than auxdisplay, doesn't it?

Best regards,
Nam Tran

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Andy Shevchenko]

On Tue, Apr 29, 2025 at 8:02 PM Nam Tran <trannamatk@gmail.com> wrote:
> On Mon, 28 Apr 2025 Pavel Machek wrote:
> > > On Mon, 28 Apr 2025 Geert Uytterhoeven wrote:
> >
> > > > > > > - Move driver to drivers/auxdisplay/ instead of drivers/leds/.
> > > > > > > - Rename files from leds-lp5812.c/.h to lp5812.c/.h.
> > > > > > > - Move ti,lp5812.yaml binding to auxdisplay/ directory,
> > > > > > >   and update the title and $id to match new path.
> > > > > > > - No functional changes to the binding itself (keep Reviewed-by).
> > > > > > > - Update commit messages and patch titles to reflect the move.
> > > > > > > - Link to v7: https://lore.kernel.org/linux-leds/20250422190121.46839-1-trannamatk@gmail.com/
> > > > > >
> > > > > > Out of sudden without discussing with auxdisplay maintainers/reviewers?
> > > > > > Thanks, no.
> > > > > > Please, put into the cover letter the meaningful summary of what's
> > > > > > going on and why this becomes an auxdisplay issue. Brief review of the
> > > > > > bindings sounds more likely like LEDS or PWM subsystems.
> > > > >
> > > > > It is 4x3 matrix. That means it is not suitable for LEDs. I don't
> > > > > believe it is suitable for PWM, either -- yes, it is 36 PWM outputs,
> > > > > but...
> > > >
> > > > Is it intended to be used as a 4x3 matrix, or is this just an internal
> > > > wiring detail, and should it be exposed as 12 individual LEDs instead?
> > >
> > > The 4×3 matrix is a real and fundamental aspect of the LP5812’s operation.
> > > It is not just an internal wiring detail.
> > > The device adopts a Time-Cross-Multiplexing (TCM) structure, where 4 output
> > > pins control 12 LED dots individually through scanning. Each pin includes
> > > both high-side and low-side drive circuits, meaning matrix multiplexing is
> > > required for proper operation — it cannot be treated as 12 completely
> > > independent LEDs.
> >
> > Scanning is really a detail.
> >
> > If this is used as rectangular 4x3 display, then it goes to auxdisplay.
> >
> > If this is used as a power LED, SD activity LED, capslock and numlock
> > ... placed randomly all around the device, then it goes LED subsystem.
>
> The LP5812 is used for LED status indication in devices like smart speakers,
> wearables, and routers, not as a structured rectangular display.
>
> Given that, it seems to match the LED subsystem better than auxdisplay, doesn't it?

I have mixed feelings about all this. As per hardware organisation it
sounds more like a matrix (for example. keyboard), where all entities
are accessed on a scanline, but at the same time each of the entities
may have orthogonal functions to each other. Have you checked with DRM
for the sake of completeness?
Personally I lean more to the something special, which doesn't fit
existing subsystems. Auxdisplay subsystem more or less about special
alphanumeric displays (with the exception of some FB kinda devices,
that were even discussed to have drivers be removed). Also maybe FB
might have something suitable, but in any case it looks quite
non-standard...


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Nam Tran]

On Tue, 29 Apr 2025 Andy Shevchenko wrote:

> On Tue, Apr 29, 2025 at 8:02 PM Nam Tran <trannamatk@gmail.com> wrote:
> > On Mon, 28 Apr 2025 Pavel Machek wrote:
> > > > On Mon, 28 Apr 2025 Geert Uytterhoeven wrote:
> > >
> > > > > > > > - Move driver to drivers/auxdisplay/ instead of drivers/leds/.
> > > > > > > > - Rename files from leds-lp5812.c/.h to lp5812.c/.h.
> > > > > > > > - Move ti,lp5812.yaml binding to auxdisplay/ directory,
> > > > > > > >   and update the title and $id to match new path.
> > > > > > > > - No functional changes to the binding itself (keep Reviewed-by).
> > > > > > > > - Update commit messages and patch titles to reflect the move.
> > > > > > > > - Link to v7: https://lore.kernel.org/linux-leds/20250422190121.46839-1-trannamatk@gmail.com/
> > > > > > >
> > > > > > > Out of sudden without discussing with auxdisplay maintainers/reviewers?
> > > > > > > Thanks, no.
> > > > > > > Please, put into the cover letter the meaningful summary of what's
> > > > > > > going on and why this becomes an auxdisplay issue. Brief review of the
> > > > > > > bindings sounds more likely like LEDS or PWM subsystems.
> > > > > >
> > > > > > It is 4x3 matrix. That means it is not suitable for LEDs. I don't
> > > > > > believe it is suitable for PWM, either -- yes, it is 36 PWM outputs,
> > > > > > but...
> > > > >
> > > > > Is it intended to be used as a 4x3 matrix, or is this just an internal
> > > > > wiring detail, and should it be exposed as 12 individual LEDs instead?
> > > >
> > > > The 4×3 matrix is a real and fundamental aspect of the LP5812’s operation.
> > > > It is not just an internal wiring detail.
> > > > The device adopts a Time-Cross-Multiplexing (TCM) structure, where 4 output
> > > > pins control 12 LED dots individually through scanning. Each pin includes
> > > > both high-side and low-side drive circuits, meaning matrix multiplexing is
> > > > required for proper operation — it cannot be treated as 12 completely
> > > > independent LEDs.
> > >
> > > Scanning is really a detail.
> > >
> > > If this is used as rectangular 4x3 display, then it goes to auxdisplay.
> > >
> > > If this is used as a power LED, SD activity LED, capslock and numlock
> > > ... placed randomly all around the device, then it goes LED subsystem.
> >
> > The LP5812 is used for LED status indication in devices like smart speakers,
> > wearables, and routers, not as a structured rectangular display.
> >
> > Given that, it seems to match the LED subsystem better than auxdisplay, doesn't it?
> 
> I have mixed feelings about all this. As per hardware organisation it
> sounds more like a matrix (for example. keyboard), where all entities
> are accessed on a scanline, but at the same time each of the entities
> may have orthogonal functions to each other. Have you checked with DRM
> for the sake of completeness?
> Personally I lean more to the something special, which doesn't fit
> existing subsystems. Auxdisplay subsystem more or less about special
> alphanumeric displays (with the exception of some FB kinda devices,
> that were even discussed to have drivers be removed). Also maybe FB
> might have something suitable, but in any case it looks quite
> non-standard...

I understand your mixed feelings about where the LP5812 fits within
the existing subsystems.

While the LP5812 uses a matrix-based structure for controlling LEDs,
it is not intended for displaying structured text or graphics. Instead,
it controls up to 4 RGB LEDs for status indication, where each RGB LED
consists of 3 individual color LEDs: red, green, and blue. Based on this,
I think it aligns more closely with the LED subsystem rather than DRM or FB.

Best regards,
Nam Tran

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Andy Shevchenko]

First of all, I just noticed that you excluded Lee from the
distribution list. Don't do that as he is a stakeholder here as well
since it has not been decided yet where to go with your stuff.

On Wed, May 7, 2025 at 7:42 PM Nam Tran <trannamatk@gmail.com> wrote:
> On Tue, 29 Apr 2025 Andy Shevchenko wrote:
> > On Tue, Apr 29, 2025 at 8:02 PM Nam Tran <trannamatk@gmail.com> wrote:
> > > On Mon, 28 Apr 2025 Pavel Machek wrote:
> > > > > On Mon, 28 Apr 2025 Geert Uytterhoeven wrote:
> > > >
> > > > > > > > > - Move driver to drivers/auxdisplay/ instead of drivers/leds/.
> > > > > > > > > - Rename files from leds-lp5812.c/.h to lp5812.c/.h.
> > > > > > > > > - Move ti,lp5812.yaml binding to auxdisplay/ directory,
> > > > > > > > >   and update the title and $id to match new path.
> > > > > > > > > - No functional changes to the binding itself (keep Reviewed-by).
> > > > > > > > > - Update commit messages and patch titles to reflect the move.
> > > > > > > > > - Link to v7: https://lore.kernel.org/linux-leds/20250422190121.46839-1-trannamatk@gmail.com/
> > > > > > > >
> > > > > > > > Out of sudden without discussing with auxdisplay maintainers/reviewers?
> > > > > > > > Thanks, no.
> > > > > > > > Please, put into the cover letter the meaningful summary of what's
> > > > > > > > going on and why this becomes an auxdisplay issue. Brief review of the
> > > > > > > > bindings sounds more likely like LEDS or PWM subsystems.
> > > > > > >
> > > > > > > It is 4x3 matrix. That means it is not suitable for LEDs. I don't
> > > > > > > believe it is suitable for PWM, either -- yes, it is 36 PWM outputs,
> > > > > > > but...
> > > > > >
> > > > > > Is it intended to be used as a 4x3 matrix, or is this just an internal
> > > > > > wiring detail, and should it be exposed as 12 individual LEDs instead?
> > > > >
> > > > > The 4×3 matrix is a real and fundamental aspect of the LP5812’s operation.
> > > > > It is not just an internal wiring detail.
> > > > > The device adopts a Time-Cross-Multiplexing (TCM) structure, where 4 output
> > > > > pins control 12 LED dots individually through scanning. Each pin includes
> > > > > both high-side and low-side drive circuits, meaning matrix multiplexing is
> > > > > required for proper operation — it cannot be treated as 12 completely
> > > > > independent LEDs.
> > > >
> > > > Scanning is really a detail.
> > > >
> > > > If this is used as rectangular 4x3 display, then it goes to auxdisplay.
> > > >
> > > > If this is used as a power LED, SD activity LED, capslock and numlock
> > > > ... placed randomly all around the device, then it goes LED subsystem.
> > >
> > > The LP5812 is used for LED status indication in devices like smart speakers,
> > > wearables, and routers, not as a structured rectangular display.
> > >
> > > Given that, it seems to match the LED subsystem better than auxdisplay, doesn't it?
> >
> > I have mixed feelings about all this. As per hardware organisation it
> > sounds more like a matrix (for example. keyboard), where all entities
> > are accessed on a scanline, but at the same time each of the entities
> > may have orthogonal functions to each other. Have you checked with DRM
> > for the sake of completeness?
> > Personally I lean more to the something special, which doesn't fit
> > existing subsystems. Auxdisplay subsystem more or less about special
> > alphanumeric displays (with the exception of some FB kinda devices,
> > that were even discussed to have drivers be removed). Also maybe FB
> > might have something suitable, but in any case it looks quite
> > non-standard...
>
> I understand your mixed feelings about where the LP5812 fits within
> the existing subsystems.
>
> While the LP5812 uses a matrix-based structure for controlling LEDs,
> it is not intended for displaying structured text or graphics. Instead,
> it controls up to 4 RGB LEDs for status indication, where each RGB LED
> consists of 3 individual color LEDs: red, green, and blue. Based on this,

So, you probably should have started with this. As I read above that
this has to reside in drivers/leds/rgb for colour ones which seems to
me closest to your case. On top you might add an upper level
management to prevent users from using patterns whenever the LEDs are
requested individually. So, this driver should represent 4 RGB leds
and, possibly, the upper layer with those fancy stuff like breathing.

At least, based on the above it's my formal NAK from an auxdisplay perspective.

> I think it aligns more closely with the LED subsystem rather than DRM or FB.

Right.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Lee Jones]

On Thu, 08 May 2025, Andy Shevchenko wrote:

> First of all, I just noticed that you excluded Lee from the
> distribution list. Don't do that as he is a stakeholder here as well
> since it has not been decided yet where to go with your stuff.
> 
> On Wed, May 7, 2025 at 7:42 PM Nam Tran <trannamatk@gmail.com> wrote:
> > On Tue, 29 Apr 2025 Andy Shevchenko wrote:
> > > On Tue, Apr 29, 2025 at 8:02 PM Nam Tran <trannamatk@gmail.com> wrote:
> > > > On Mon, 28 Apr 2025 Pavel Machek wrote:
> > > > > > On Mon, 28 Apr 2025 Geert Uytterhoeven wrote:
> > > > >
> > > > > > > > > > - Move driver to drivers/auxdisplay/ instead of drivers/leds/.
> > > > > > > > > > - Rename files from leds-lp5812.c/.h to lp5812.c/.h.
> > > > > > > > > > - Move ti,lp5812.yaml binding to auxdisplay/ directory,
> > > > > > > > > >   and update the title and $id to match new path.
> > > > > > > > > > - No functional changes to the binding itself (keep Reviewed-by).
> > > > > > > > > > - Update commit messages and patch titles to reflect the move.
> > > > > > > > > > - Link to v7: https://lore.kernel.org/linux-leds/20250422190121.46839-1-trannamatk@gmail.com/
> > > > > > > > >
> > > > > > > > > Out of sudden without discussing with auxdisplay maintainers/reviewers?
> > > > > > > > > Thanks, no.
> > > > > > > > > Please, put into the cover letter the meaningful summary of what's
> > > > > > > > > going on and why this becomes an auxdisplay issue. Brief review of the
> > > > > > > > > bindings sounds more likely like LEDS or PWM subsystems.
> > > > > > > >
> > > > > > > > It is 4x3 matrix. That means it is not suitable for LEDs. I don't
> > > > > > > > believe it is suitable for PWM, either -- yes, it is 36 PWM outputs,
> > > > > > > > but...
> > > > > > >
> > > > > > > Is it intended to be used as a 4x3 matrix, or is this just an internal
> > > > > > > wiring detail, and should it be exposed as 12 individual LEDs instead?
> > > > > >
> > > > > > The 4×3 matrix is a real and fundamental aspect of the LP5812’s operation.
> > > > > > It is not just an internal wiring detail.
> > > > > > The device adopts a Time-Cross-Multiplexing (TCM) structure, where 4 output
> > > > > > pins control 12 LED dots individually through scanning. Each pin includes
> > > > > > both high-side and low-side drive circuits, meaning matrix multiplexing is
> > > > > > required for proper operation — it cannot be treated as 12 completely
> > > > > > independent LEDs.
> > > > >
> > > > > Scanning is really a detail.
> > > > >
> > > > > If this is used as rectangular 4x3 display, then it goes to auxdisplay.
> > > > >
> > > > > If this is used as a power LED, SD activity LED, capslock and numlock
> > > > > ... placed randomly all around the device, then it goes LED subsystem.
> > > >
> > > > The LP5812 is used for LED status indication in devices like smart speakers,
> > > > wearables, and routers, not as a structured rectangular display.
> > > >
> > > > Given that, it seems to match the LED subsystem better than auxdisplay, doesn't it?
> > >
> > > I have mixed feelings about all this. As per hardware organisation it
> > > sounds more like a matrix (for example. keyboard), where all entities
> > > are accessed on a scanline, but at the same time each of the entities
> > > may have orthogonal functions to each other. Have you checked with DRM
> > > for the sake of completeness?
> > > Personally I lean more to the something special, which doesn't fit
> > > existing subsystems. Auxdisplay subsystem more or less about special
> > > alphanumeric displays (with the exception of some FB kinda devices,
> > > that were even discussed to have drivers be removed). Also maybe FB
> > > might have something suitable, but in any case it looks quite
> > > non-standard...
> >
> > I understand your mixed feelings about where the LP5812 fits within
> > the existing subsystems.
> >
> > While the LP5812 uses a matrix-based structure for controlling LEDs,
> > it is not intended for displaying structured text or graphics. Instead,
> > it controls up to 4 RGB LEDs for status indication, where each RGB LED
> > consists of 3 individual color LEDs: red, green, and blue. Based on this,
> 
> So, you probably should have started with this. As I read above that
> this has to reside in drivers/leds/rgb for colour ones which seems to
> me closest to your case. On top you might add an upper level
> management to prevent users from using patterns whenever the LEDs are
> requested individually. So, this driver should represent 4 RGB leds
> and, possibly, the upper layer with those fancy stuff like breathing.
> 
> At least, based on the above it's my formal NAK from an auxdisplay perspective.

This is fine.

Just be aware, before you submit to LEDs again, that you need to use
what is available in the LEDs subsystem to it's fullest, before
hand-rolling all of your own APIs.  The first submission didn't use a
single LED API.  This, as before, would be a big NACK also.

-- 
Lee Jones [李琼斯]

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Nam Tran]

On Thu, 8 May 2025 Lee Jones wrote:

> On Thu, 08 May 2025, Andy Shevchenko wrote:
> 
> > First of all, I just noticed that you excluded Lee from the
> > distribution list. Don't do that as he is a stakeholder here as well
> > since it has not been decided yet where to go with your stuff.
> > 
> > On Wed, May 7, 2025 at 7:42 PM Nam Tran <trannamatk@gmail.com> wrote:
> > > On Tue, 29 Apr 2025 Andy Shevchenko wrote:
> > > > On Tue, Apr 29, 2025 at 8:02 PM Nam Tran <trannamatk@gmail.com> wrote:
> > > > > On Mon, 28 Apr 2025 Pavel Machek wrote:
> > > > > > > On Mon, 28 Apr 2025 Geert Uytterhoeven wrote:
> > > > > >
> > > > > > > > > > > - Move driver to drivers/auxdisplay/ instead of drivers/leds/.
> > > > > > > > > > > - Rename files from leds-lp5812.c/.h to lp5812.c/.h.
> > > > > > > > > > > - Move ti,lp5812.yaml binding to auxdisplay/ directory,
> > > > > > > > > > >   and update the title and $id to match new path.
> > > > > > > > > > > - No functional changes to the binding itself (keep Reviewed-by).
> > > > > > > > > > > - Update commit messages and patch titles to reflect the move.
> > > > > > > > > > > - Link to v7: https://lore.kernel.org/linux-leds/20250422190121.46839-1-trannamatk@gmail.com/
> > > > > > > > > >
> > > > > > > > > > Out of sudden without discussing with auxdisplay maintainers/reviewers?
> > > > > > > > > > Thanks, no.
> > > > > > > > > > Please, put into the cover letter the meaningful summary of what's
> > > > > > > > > > going on and why this becomes an auxdisplay issue. Brief review of the
> > > > > > > > > > bindings sounds more likely like LEDS or PWM subsystems.
> > > > > > > > >
> > > > > > > > > It is 4x3 matrix. That means it is not suitable for LEDs. I don't
> > > > > > > > > believe it is suitable for PWM, either -- yes, it is 36 PWM outputs,
> > > > > > > > > but...
> > > > > > > >
> > > > > > > > Is it intended to be used as a 4x3 matrix, or is this just an internal
> > > > > > > > wiring detail, and should it be exposed as 12 individual LEDs instead?
> > > > > > >
> > > > > > > The 4×3 matrix is a real and fundamental aspect of the LP5812’s operation.
> > > > > > > It is not just an internal wiring detail.
> > > > > > > The device adopts a Time-Cross-Multiplexing (TCM) structure, where 4 output
> > > > > > > pins control 12 LED dots individually through scanning. Each pin includes
> > > > > > > both high-side and low-side drive circuits, meaning matrix multiplexing is
> > > > > > > required for proper operation — it cannot be treated as 12 completely
> > > > > > > independent LEDs.
> > > > > >
> > > > > > Scanning is really a detail.
> > > > > >
> > > > > > If this is used as rectangular 4x3 display, then it goes to auxdisplay.
> > > > > >
> > > > > > If this is used as a power LED, SD activity LED, capslock and numlock
> > > > > > ... placed randomly all around the device, then it goes LED subsystem.
> > > > >
> > > > > The LP5812 is used for LED status indication in devices like smart speakers,
> > > > > wearables, and routers, not as a structured rectangular display.
> > > > >
> > > > > Given that, it seems to match the LED subsystem better than auxdisplay, doesn't it?
> > > >
> > > > I have mixed feelings about all this. As per hardware organisation it
> > > > sounds more like a matrix (for example. keyboard), where all entities
> > > > are accessed on a scanline, but at the same time each of the entities
> > > > may have orthogonal functions to each other. Have you checked with DRM
> > > > for the sake of completeness?
> > > > Personally I lean more to the something special, which doesn't fit
> > > > existing subsystems. Auxdisplay subsystem more or less about special
> > > > alphanumeric displays (with the exception of some FB kinda devices,
> > > > that were even discussed to have drivers be removed). Also maybe FB
> > > > might have something suitable, but in any case it looks quite
> > > > non-standard...
> > >
> > > I understand your mixed feelings about where the LP5812 fits within
> > > the existing subsystems.
> > >
> > > While the LP5812 uses a matrix-based structure for controlling LEDs,
> > > it is not intended for displaying structured text or graphics. Instead,
> > > it controls up to 4 RGB LEDs for status indication, where each RGB LED
> > > consists of 3 individual color LEDs: red, green, and blue. Based on this,
> > 
> > So, you probably should have started with this. As I read above that
> > this has to reside in drivers/leds/rgb for colour ones which seems to
> > me closest to your case. On top you might add an upper level
> > management to prevent users from using patterns whenever the LEDs are
> > requested individually. So, this driver should represent 4 RGB leds
> > and, possibly, the upper layer with those fancy stuff like breathing.
> > 
> > At least, based on the above it's my formal NAK from an auxdisplay perspective.
> 
> This is fine.
> 
> Just be aware, before you submit to LEDs again, that you need to use
> what is available in the LEDs subsystem to it's fullest, before
> hand-rolling all of your own APIs.  The first submission didn't use a
> single LED API.  This, as before, would be a big NACK also.

Thanks for the clarification.

Just to confirm — the current version of the driver is customized to allow
user space to directly manipulate LP5812 registers and to support the
device’s full feature set. Because of this, it doesn’t follow the standard
LED interfaces.

Given that, would it be acceptable to submit this driver under the misc subsystem instead?

Best regards,
Nam Tran

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Andy Shevchenko]

On Thu, May 8, 2025 at 5:27 PM Nam Tran <trannamatk@gmail.com> wrote:
> On Thu, 8 May 2025 Lee Jones wrote:
> > On Thu, 08 May 2025, Andy Shevchenko wrote:
> > > On Wed, May 7, 2025 at 7:42 PM Nam Tran <trannamatk@gmail.com> wrote:

...

> > > At least, based on the above it's my formal NAK from an auxdisplay perspective.
> >
> > This is fine.
> >
> > Just be aware, before you submit to LEDs again, that you need to use
> > what is available in the LEDs subsystem to it's fullest, before
> > hand-rolling all of your own APIs.  The first submission didn't use a
> > single LED API.  This, as before, would be a big NACK also.
>
> Thanks for the clarification.
>
> Just to confirm — the current version of the driver is customized to allow
> user space to directly manipulate LP5812 registers and to support the
> device’s full feature set. Because of this, it doesn’t follow the standard
> LED interfaces.

But why? What's wrong with the LED ABI? (see also below question
before answering to this one)

> Given that, would it be acceptable to submit this driver under the misc subsystem instead?

But these are LEDs in the hardware and you can access them as 4
individual LEDs, right?


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Lee Jones]

On Thu, 08 May 2025, Andy Shevchenko wrote:

> On Thu, May 8, 2025 at 5:27 PM Nam Tran <trannamatk@gmail.com> wrote:
> > On Thu, 8 May 2025 Lee Jones wrote:
> > > On Thu, 08 May 2025, Andy Shevchenko wrote:
> > > > On Wed, May 7, 2025 at 7:42 PM Nam Tran <trannamatk@gmail.com> wrote:
> 
> ...
> 
> > > > At least, based on the above it's my formal NAK from an auxdisplay perspective.
> > >
> > > This is fine.
> > >
> > > Just be aware, before you submit to LEDs again, that you need to use
> > > what is available in the LEDs subsystem to it's fullest, before
> > > hand-rolling all of your own APIs.  The first submission didn't use a
> > > single LED API.  This, as before, would be a big NACK also.
> >
> > Thanks for the clarification.
> >
> > Just to confirm — the current version of the driver is customized to allow
> > user space to directly manipulate LP5812 registers and to support the
> > device’s full feature set. Because of this, it doesn’t follow the standard
> > LED interfaces.
> 
> But why? What's wrong with the LED ABI? (see also below question
> before answering to this one)
> 
> > Given that, would it be acceptable to submit this driver under the misc subsystem instead?
> 
> But these are LEDs in the hardware and you can access them as 4
> individual LEDs, right?

Right.  Please work with the API you are offered in the first instance.
My first assumption is always that this driver isn't as special as you
think it might be.

-- 
Lee Jones [李琼斯]

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Nam Tran]

On Thu, 8 May 2025 Lee Jones wrote:

> On Thu, 08 May 2025, Andy Shevchenko wrote:
> > On Thu, May 8, 2025 at 5:27 PM Nam Tran <trannamatk@gmail.com> wrote:
> > > On Thu, 8 May 2025 Lee Jones wrote:
> > > > On Thu, 08 May 2025, Andy Shevchenko wrote:
> > > > > On Wed, May 7, 2025 at 7:42 PM Nam Tran <trannamatk@gmail.com> wrote:
> > 
> > ...
> > 
> > > > > At least, based on the above it's my formal NAK from an auxdisplay perspective.
> > > >
> > > > This is fine.
> > > >
> > > > Just be aware, before you submit to LEDs again, that you need to use
> > > > what is available in the LEDs subsystem to it's fullest, before
> > > > hand-rolling all of your own APIs.  The first submission didn't use a
> > > > single LED API.  This, as before, would be a big NACK also.
> > >
> > > Thanks for the clarification.
> > >
> > > Just to confirm — the current version of the driver is customized to allow
> > > user space to directly manipulate LP5812 registers and to support the
> > > device’s full feature set. Because of this, it doesn’t follow the standard
> > > LED interfaces.
> > 
> > But why? What's wrong with the LED ABI? (see also below question
> > before answering to this one)
> > 
> > > Given that, would it be acceptable to submit this driver under the misc subsystem instead?
> > 
> > But these are LEDs in the hardware and you can access them as 4
> > individual LEDs, right?
> 
> Right.  Please work with the API you are offered in the first instance.
> My first assumption is always that this driver isn't as special as you
> think it might be.

In direct mode, we can access them as individual LEDS. User doesn't need
to select LEDs. In this mode, it is a simple LED driver.

However, user must select LEDs in scan mode. The hardware uses 4 pin to
display 12 LEDs (or 4 RGB LEDs). Ordering of LED selection also impact
to display capacibility. That why, I need to support another interface
for user to controll hardware's registers.

In mix mode, we can control an individual LED and up to 6 scan LEDs.
However, user must select the order of single LED and which LEDs will be
use for scan function.

The main point is user must have capacibility in write information to
hardware's registers to select LEDs in scan mode and mix mode.
 
Besides system modes (direct mode, scan mode, mix mode), each LED has
manual mode and autonomous mode.
 
A example steps to display a LED in manual mode
# Set drive mode is Scan mode with 4 scan. Scan order 0 is out_0,
# Scan order 1 is out_1, Scan order 2 is out_2, and Scan order 3 is out_3
echo tcmscan:4:0:1:2:3 > /sys/bus/i2c/drivers/lp5812/1-001b/lp5812_chip_setup/dev_config
# Enable led_A0
echo 1 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/activate
# Enable manual mode
echo manual > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/mode
# Set Dot Current
echo 100 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/manual_dc
# Set Manual PWM
echo 100 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/manual_pwm
 
However in autonomous mode, the steps are complicated
# Set drive mode is Scan mode with 4 scan. Scan order 0 is out_0,
# Scan order 1 is out_1, Scan order 2 is out_2, and Scan order 3 is out_3
echo tcmscan:4:0:1:2:3 > /sys/bus/i2c/drivers/lp5812/1-001b/lp5812_chip_setup/dev_config
# Enable led_A0
echo 1 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/activate
# Enable autonomous mode
echo autonomous > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/mode
# Config autonomous animation mode: (only use AEU1, start pause time: 3.04s,
# stop pause time: 3.04s, playback time: infinite time)
echo 1:10:10:15 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/mode
# Config AEU1 playback times
echo 1 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/AEU1/playback_time
# Config PWM
echo 100 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/AEU1/pwm1
echo 100 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/AEU1/pwm2
echo 100 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/AEU1/pwm3
echo 100 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/AEU1/pwm4
echo 100 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/AEU1/pwm5
# Config slope time
echo 5 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/AEU1/slope_time_t1
echo 5 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/AEU1/slope_time_t2
echo 5 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/AEU1/slope_time_t3
echo 5 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/AEU1/slope_time_t4
# Start autonomous
echo start > /sys/bus/i2c/drivers/lp5812/1-001b/lp5812_chip_setup/device_command
 
I think setting PWM also same as brightness_set API. However, there are
many PWM config for a LED and it is one of other config to make autonomous mode work.
Therefore, standard led API can use in some use cases only.

Please see the link below for a better visualization of how to configure the LP5812.
https://dev.ti.com/gallery/view/LED/LP581x/ver/0.10.0/

Best regards,
Nam Tran

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Andy Shevchenko]

On Sat, May 10, 2025 at 02:48:02PM +0700, Nam Tran wrote:
> On Thu, 8 May 2025 Lee Jones wrote:
> > On Thu, 08 May 2025, Andy Shevchenko wrote:
> > > On Thu, May 8, 2025 at 5:27 PM Nam Tran <trannamatk@gmail.com> wrote:
> > > > On Thu, 8 May 2025 Lee Jones wrote:
> > > > > On Thu, 08 May 2025, Andy Shevchenko wrote:
> > > > > > On Wed, May 7, 2025 at 7:42 PM Nam Tran <trannamatk@gmail.com> wrote:

...

> > > > > > At least, based on the above it's my formal NAK from an auxdisplay perspective.
> > > > >
> > > > > This is fine.
> > > > >
> > > > > Just be aware, before you submit to LEDs again, that you need to use
> > > > > what is available in the LEDs subsystem to it's fullest, before
> > > > > hand-rolling all of your own APIs.  The first submission didn't use a
> > > > > single LED API.  This, as before, would be a big NACK also.
> > > >
> > > > Thanks for the clarification.
> > > >
> > > > Just to confirm — the current version of the driver is customized to allow
> > > > user space to directly manipulate LP5812 registers and to support the
> > > > device’s full feature set. Because of this, it doesn’t follow the standard
> > > > LED interfaces.
> > > 
> > > But why? What's wrong with the LED ABI? (see also below question
> > > before answering to this one)
> > > 
> > > > Given that, would it be acceptable to submit this driver under the misc subsystem instead?
> > > 
> > > But these are LEDs in the hardware and you can access them as 4
> > > individual LEDs, right?
> > 
> > Right.  Please work with the API you are offered in the first instance.
> > My first assumption is always that this driver isn't as special as you
> > think it might be.
> 
> In direct mode, we can access them as individual LEDS. User doesn't need
> to select LEDs. In this mode, it is a simple LED driver.
> 
> However, user must select LEDs in scan mode. The hardware uses 4 pin to
> display 12 LEDs (or 4 RGB LEDs). Ordering of LED selection also impact
> to display capacibility. That why, I need to support another interface
> for user to controll hardware's registers.
> 
> In mix mode, we can control an individual LED and up to 6 scan LEDs.
> However, user must select the order of single LED and which LEDs will be
> use for scan function.
> 
> The main point is user must have capacibility in write information to
> hardware's registers to select LEDs in scan mode and mix mode.
>  
> Besides system modes (direct mode, scan mode, mix mode), each LED has
> manual mode and autonomous mode.
>  
> A example steps to display a LED in manual mode
> # Set drive mode is Scan mode with 4 scan. Scan order 0 is out_0,
> # Scan order 1 is out_1, Scan order 2 is out_2, and Scan order 3 is out_3
> echo tcmscan:4:0:1:2:3 > /sys/bus/i2c/drivers/lp5812/1-001b/lp5812_chip_setup/dev_config
> # Enable led_A0
> echo 1 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/activate
> # Enable manual mode
> echo manual > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/mode
> # Set Dot Current
> echo 100 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/manual_dc
> # Set Manual PWM
> echo 100 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/manual_pwm
>  
> However in autonomous mode, the steps are complicated
> # Set drive mode is Scan mode with 4 scan. Scan order 0 is out_0,
> # Scan order 1 is out_1, Scan order 2 is out_2, and Scan order 3 is out_3
> echo tcmscan:4:0:1:2:3 > /sys/bus/i2c/drivers/lp5812/1-001b/lp5812_chip_setup/dev_config
> # Enable led_A0
> echo 1 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/activate
> # Enable autonomous mode
> echo autonomous > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/mode
> # Config autonomous animation mode: (only use AEU1, start pause time: 3.04s,
> # stop pause time: 3.04s, playback time: infinite time)
> echo 1:10:10:15 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/mode
> # Config AEU1 playback times
> echo 1 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/AEU1/playback_time
> # Config PWM
> echo 100 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/AEU1/pwm1
> echo 100 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/AEU1/pwm2
> echo 100 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/AEU1/pwm3
> echo 100 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/AEU1/pwm4
> echo 100 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/AEU1/pwm5
> # Config slope time
> echo 5 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/AEU1/slope_time_t1
> echo 5 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/AEU1/slope_time_t2
> echo 5 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/AEU1/slope_time_t3
> echo 5 > /sys/bus/i2c/drivers/lp5812/1-001b/led_A0/AEU1/slope_time_t4
> # Start autonomous
> echo start > /sys/bus/i2c/drivers/lp5812/1-001b/lp5812_chip_setup/device_command
>  
> I think setting PWM also same as brightness_set API. However, there are
> many PWM config for a LED and it is one of other config to make autonomous mode work.
> Therefore, standard led API can use in some use cases only.
> 
> Please see the link below for a better visualization of how to configure the LP5812.
> https://dev.ti.com/gallery/view/LED/LP581x/ver/0.10.0/

To me it sounds like you should start from the small steps, i.e. do not
implement everything at once. And starting point of the 4 RGB LEDs sounds
the best approach to me. Then, if needed, you can always move on with
fancy features of this hardware on top of the existing code.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Nam Tran]

On Mon, 12 May 2025 Andy Shevchenko wrote:

> On Sat, May 10, 2025 at 02:48:02PM +0700, Nam Tran wrote:
> > On Thu, 8 May 2025 Lee Jones wrote:
> > > On Thu, 08 May 2025, Andy Shevchenko wrote:
> > > > On Thu, May 8, 2025 at 5:27 PM Nam Tran <trannamatk@gmail.com> wrote:
> > > > > On Thu, 8 May 2025 Lee Jones wrote:
> > > > > > On Thu, 08 May 2025, Andy Shevchenko wrote:

...

> > I think setting PWM also same as brightness_set API. However, there are
> > many PWM config for a LED and it is one of other config to make autonomous mode work.
> > Therefore, standard led API can use in some use cases only.
> > 
> > Please see the link below for a better visualization of how to configure the LP5812.
> > https://dev.ti.com/gallery/view/LED/LP581x/ver/0.10.0/
> 
> To me it sounds like you should start from the small steps, i.e. do not
> implement everything at once. And starting point of the 4 RGB LEDs sounds
> the best approach to me. Then, if needed, you can always move on with
> fancy features of this hardware on top of the existing code.

Thanks for the suggestion.
I understand your point and agree that starting with standard LED APIs is the preferred approach.

However, the LP5812 hardware offers more advanced features, and I’d like to support end users all
features as shown in the link: https://dev.ti.com/gallery/view/LED/LP581x/ver/0.10.0/.
It is easy for end user to investigate and use driver.

If I want to keep the current driver interface to meet this expectation, would it be acceptable
to move it to the misc subsystem to better support the hardware?

Best regards,
Nam Tran

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Andy Shevchenko]

On Mon, May 12, 2025 at 8:38 PM Nam Tran <trannamatk@gmail.com> wrote:
> On Mon, 12 May 2025 Andy Shevchenko wrote:
> > On Sat, May 10, 2025 at 02:48:02PM +0700, Nam Tran wrote:
> > > On Thu, 8 May 2025 Lee Jones wrote:
> > > > On Thu, 08 May 2025, Andy Shevchenko wrote:
> > > > > On Thu, May 8, 2025 at 5:27 PM Nam Tran <trannamatk@gmail.com> wrote:
> > > > > > On Thu, 8 May 2025 Lee Jones wrote:
> > > > > > > On Thu, 08 May 2025, Andy Shevchenko wrote:

...

> > > I think setting PWM also same as brightness_set API. However, there are
> > > many PWM config for a LED and it is one of other config to make autonomous mode work.
> > > Therefore, standard led API can use in some use cases only.
> > >
> > > Please see the link below for a better visualization of how to configure the LP5812.
> > > https://dev.ti.com/gallery/view/LED/LP581x/ver/0.10.0/
> >
> > To me it sounds like you should start from the small steps, i.e. do not
> > implement everything at once. And starting point of the 4 RGB LEDs sounds
> > the best approach to me. Then, if needed, you can always move on with
> > fancy features of this hardware on top of the existing code.
>
> Thanks for the suggestion.
> I understand your point and agree that starting with standard LED APIs is the preferred approach.
>
> However, the LP5812 hardware offers more advanced features, and I’d like to support end users all
> features as shown in the link: https://dev.ti.com/gallery/view/LED/LP581x/ver/0.10.0/.
> It is easy for end user to investigate and use driver.

I see. Good luck then!

> If I want to keep the current driver interface to meet this expectation, would it be acceptable
> to move it to the misc subsystem to better support the hardware?

Don't ask me, I don't know what you want at the end and I have not so
much time to invest in this anymore. Only what I'm sure about I
already expressed in the previous replies and emails.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Lee Jones]

On Mon, 12 May 2025, Andy Shevchenko wrote:

> On Mon, May 12, 2025 at 8:38 PM Nam Tran <trannamatk@gmail.com> wrote:
> > On Mon, 12 May 2025 Andy Shevchenko wrote:
> > > On Sat, May 10, 2025 at 02:48:02PM +0700, Nam Tran wrote:
> > > > On Thu, 8 May 2025 Lee Jones wrote:
> > > > > On Thu, 08 May 2025, Andy Shevchenko wrote:
> > > > > > On Thu, May 8, 2025 at 5:27 PM Nam Tran <trannamatk@gmail.com> wrote:
> > > > > > > On Thu, 8 May 2025 Lee Jones wrote:
> > > > > > > > On Thu, 08 May 2025, Andy Shevchenko wrote:
> 
> ...
> 
> > > > I think setting PWM also same as brightness_set API. However, there are
> > > > many PWM config for a LED and it is one of other config to make autonomous mode work.
> > > > Therefore, standard led API can use in some use cases only.
> > > >
> > > > Please see the link below for a better visualization of how to configure the LP5812.
> > > > https://dev.ti.com/gallery/view/LED/LP581x/ver/0.10.0/
> > >
> > > To me it sounds like you should start from the small steps, i.e. do not
> > > implement everything at once. And starting point of the 4 RGB LEDs sounds
> > > the best approach to me. Then, if needed, you can always move on with
> > > fancy features of this hardware on top of the existing code.
> >
> > Thanks for the suggestion.
> > I understand your point and agree that starting with standard LED APIs is the preferred approach.
> >
> > However, the LP5812 hardware offers more advanced features, and I’d like to support end users all
> > features as shown in the link: https://dev.ti.com/gallery/view/LED/LP581x/ver/0.10.0/.
> > It is easy for end user to investigate and use driver.
> 
> I see. Good luck then!
> 
> > If I want to keep the current driver interface to meet this expectation, would it be acceptable
> > to move it to the misc subsystem to better support the hardware?
> 
> Don't ask me, I don't know what you want at the end and I have not so
> much time to invest in this anymore. Only what I'm sure about I
> already expressed in the previous replies and emails.

Briefly spoke to Greg (now Cc:ed).  We can all discuss this together.

My 2c, whilst falling short of deep-diving, is as follows:

1. No one is going to enjoy reviewing a 3k line submission!

   - Instead, submit the smallest driver you can whilst still retaining
     functionality.  It is not good practice to fully enable a
     complicated driver such as this in a single submission - let alone
     a single patch.

2. Hand-rolling your own API from scratch is to be highly discouraged.

   - There seems to be quite a bit of overlap in functionality between
     your bespoke API and LED's.  The LED subsystem already supports a
     lot of what's being implemented here.  Instead of letting the user
     configure the device directly, why not offer some high level
     options and attempt to infer the rest.  If possible, the complexity
     of the device should be handed by driver, rather than placing the
     onus on the user.

3. Shoving this into Misc because you don't want to use the APIs that
   are already offered to you is not a good approach.

-- 
Lee Jones [李琼斯]

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Geert Uytterhoeven]

Hi Nam,

On Mon, 12 May 2025 at 19:38, Nam Tran <trannamatk@gmail.com> wrote:
> On Mon, 12 May 2025 Andy Shevchenko wrote:
> > On Sat, May 10, 2025 at 02:48:02PM +0700, Nam Tran wrote:
> > > On Thu, 8 May 2025 Lee Jones wrote:
> > > > On Thu, 08 May 2025, Andy Shevchenko wrote:
> > > > > On Thu, May 8, 2025 at 5:27 PM Nam Tran <trannamatk@gmail.com> wrote:
> > > > > > On Thu, 8 May 2025 Lee Jones wrote:
> > > > > > > On Thu, 08 May 2025, Andy Shevchenko wrote:
>
> ...
>
> > > I think setting PWM also same as brightness_set API. However, there are
> > > many PWM config for a LED and it is one of other config to make autonomous mode work.
> > > Therefore, standard led API can use in some use cases only.
> > >
> > > Please see the link below for a better visualization of how to configure the LP5812.
> > > https://dev.ti.com/gallery/view/LED/LP581x/ver/0.10.0/
> >
> > To me it sounds like you should start from the small steps, i.e. do not
> > implement everything at once. And starting point of the 4 RGB LEDs sounds
> > the best approach to me. Then, if needed, you can always move on with
> > fancy features of this hardware on top of the existing code.
>
> Thanks for the suggestion.
> I understand your point and agree that starting with standard LED APIs is the preferred approach.

i.e. a drivers/leds/ driver.

> However, the LP5812 hardware offers more advanced features, and I’d like to support end users all
> features as shown in the link: https://dev.ti.com/gallery/view/LED/LP581x/ver/0.10.0/.
> It is easy for end user to investigate and use driver.
>
> If I want to keep the current driver interface to meet this expectation, would it be acceptable
> to move it to the misc subsystem to better support the hardware?

I guess you can add custom sysfs controls for the advanced features
on top of the drivers/leds/ driver?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Lee Jones]

On Tue, 13 May 2025, Geert Uytterhoeven wrote:

> Hi Nam,
> 
> On Mon, 12 May 2025 at 19:38, Nam Tran <trannamatk@gmail.com> wrote:
> > On Mon, 12 May 2025 Andy Shevchenko wrote:
> > > On Sat, May 10, 2025 at 02:48:02PM +0700, Nam Tran wrote:
> > > > On Thu, 8 May 2025 Lee Jones wrote:
> > > > > On Thu, 08 May 2025, Andy Shevchenko wrote:
> > > > > > On Thu, May 8, 2025 at 5:27 PM Nam Tran <trannamatk@gmail.com> wrote:
> > > > > > > On Thu, 8 May 2025 Lee Jones wrote:
> > > > > > > > On Thu, 08 May 2025, Andy Shevchenko wrote:
> >
> > ...
> >
> > > > I think setting PWM also same as brightness_set API. However, there are
> > > > many PWM config for a LED and it is one of other config to make autonomous mode work.
> > > > Therefore, standard led API can use in some use cases only.
> > > >
> > > > Please see the link below for a better visualization of how to configure the LP5812.
> > > > https://dev.ti.com/gallery/view/LED/LP581x/ver/0.10.0/
> > >
> > > To me it sounds like you should start from the small steps, i.e. do not
> > > implement everything at once. And starting point of the 4 RGB LEDs sounds
> > > the best approach to me. Then, if needed, you can always move on with
> > > fancy features of this hardware on top of the existing code.
> >
> > Thanks for the suggestion.
> > I understand your point and agree that starting with standard LED APIs is the preferred approach.
> 
> i.e. a drivers/leds/ driver.
> 
> > However, the LP5812 hardware offers more advanced features, and I’d like to support end users all
> > features as shown in the link: https://dev.ti.com/gallery/view/LED/LP581x/ver/0.10.0/.
> > It is easy for end user to investigate and use driver.
> >
> > If I want to keep the current driver interface to meet this expectation, would it be acceptable
> > to move it to the misc subsystem to better support the hardware?
> 
> I guess you can add custom sysfs controls for the advanced features
> on top of the drivers/leds/ driver?

Yes, exactly that.

-- 
Lee Jones [李琼斯]

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Pavel Machek]

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

Hi!

> > > > Is it intended to be used as a 4x3 matrix, or is this just an internal
> > > > wiring detail, and should it be exposed as 12 individual LEDs instead?
> > > 
> > > The 4×3 matrix is a real and fundamental aspect of the LP5812’s operation.
> > > It is not just an internal wiring detail.
> > > The device adopts a Time-Cross-Multiplexing (TCM) structure, where 4 output
> > > pins control 12 LED dots individually through scanning. Each pin includes
> > > both high-side and low-side drive circuits, meaning matrix multiplexing is
> > > required for proper operation — it cannot be treated as 12 completely
> > > independent LEDs.
> > 
> > Scanning is really a detail.
> > 
> > If this is used as rectangular 4x3 display, then it goes to auxdisplay.
> > 
> > If this is used as a power LED, SD activity LED, capslock and numlock
> > ... placed randomly all around the device, then it goes LED subsystem.
> 
> The LP5812 is used for LED status indication in devices like smart speakers,
> wearables, and routers, not as a structured rectangular display.

Well, IIRC it also supports automated animations, and that does not
make sense on LED indicators. So... what device do _you_ have and how
exactly is it used there?

Best regards,
								Pavel
-- 
I don't work for Nazis and criminals, and neither should you.
Boycott Putin, Trump, and Musk!

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

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Nam Tran]

On Wed, 30 Apr 2025 Pavel Machek wrote:

> > > > > Is it intended to be used as a 4x3 matrix, or is this just an internal
> > > > > wiring detail, and should it be exposed as 12 individual LEDs instead?
> > > > 
> > > > The 4×3 matrix is a real and fundamental aspect of the LP5812’s operation.
> > > > It is not just an internal wiring detail.
> > > > The device adopts a Time-Cross-Multiplexing (TCM) structure, where 4 output
> > > > pins control 12 LED dots individually through scanning. Each pin includes
> > > > both high-side and low-side drive circuits, meaning matrix multiplexing is
> > > > required for proper operation — it cannot be treated as 12 completely
> > > > independent LEDs.
> > > 
> > > Scanning is really a detail.
> > > 
> > > If this is used as rectangular 4x3 display, then it goes to auxdisplay.
> > > 
> > > If this is used as a power LED, SD activity LED, capslock and numlock
> > > ... placed randomly all around the device, then it goes LED subsystem.
> > 
> > The LP5812 is used for LED status indication in devices like smart speakers,
> > wearables, and routers, not as a structured rectangular display.
> 
> Well, IIRC it also supports automated animations, and that does not
> make sense on LED indicators. So... what device do _you_ have and how
> exactly is it used there?

We’re using the LP5812 in a battery-powered smart speaker as a status indicator
with visually enhanced lighting — for example, breathing effects during standby
and fading effects during voice processing.

While the LP5812 supports autonomous animations, they’re used here to offload
real-time brightness control from the main controller. Each LED’s animation engine
enables smooth transitions like fade-in/fade-out and breathing effects.
These patterns are configured once and run autonomously, saving MCU load and power,
while still serving traditional indicator roles.

Best regards,
Nam Tran

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Pavel Machek]

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

Hi!

> > > Thank you, Pavel, for the confirmation.
> > > Thank you, Geert, for the review and the question.
> > > 
> > > I would like to make it clearer.
> > > 
> > > On Mon, 28 Apr 2025 Geert Uytterhoeven wrote:
> > > 
> > 
> > > > > > > - Move driver to drivers/auxdisplay/ instead of drivers/leds/.
> > > > > > > - Rename files from leds-lp5812.c/.h to lp5812.c/.h.
> > > > > > > - Move ti,lp5812.yaml binding to auxdisplay/ directory,
> > > > > > >   and update the title and $id to match new path.
> > > > > > > - No functional changes to the binding itself (keep Reviewed-by).
> > > > > > > - Update commit messages and patch titles to reflect the move.
> > > > > > > - Link to v7: https://lore.kernel.org/linux-leds/20250422190121.46839-1-trannamatk@gmail.com/
> > > > > >
> > > > > > Out of sudden without discussing with auxdisplay maintainers/reviewers?
> > > > > > Thanks, no.
> > > > > > Please, put into the cover letter the meaningful summary of what's
> > > > > > going on and why this becomes an auxdisplay issue. Brief review of the
> > > > > > bindings sounds more likely like LEDS or PWM subsystems.
> > > > >
> > > > > It is 4x3 matrix. That means it is not suitable for LEDs. I don't
> > > > > believe it is suitable for PWM, either -- yes, it is 36 PWM outputs,
> > > > > but...
> > > > 
> > > > Is it intended to be used as a 4x3 matrix, or is this just an internal
> > > > wiring detail, and should it be exposed as 12 individual LEDs instead?
> > > 
> > > The 4×3 matrix is a real and fundamental aspect of the LP5812’s operation.
> > > It is not just an internal wiring detail.
> > > The device adopts a Time-Cross-Multiplexing (TCM) structure, where 4 output
> > > pins control 12 LED dots individually through scanning. Each pin includes
> > > both high-side and low-side drive circuits, meaning matrix multiplexing is
> > > required for proper operation — it cannot be treated as 12 completely
> > > independent LEDs.
> > 
> > Scanning is really a detail.
> > 
> > If this is used as rectangular 4x3 display, then it goes to auxdisplay.
> > 
> > If this is used as a power LED, SD activity LED, capslock and numlock
> > ... placed randomly all around the device, then it goes LED subsystem.
> 
> The LP5812 is used for LED status indication in devices like smart speakers,
> wearables, and routers, not as a structured rectangular display.

Show us one device where it is used, photo would be best. I suspect
that the router use might indeed qualify for LED subsystem.

You described is at 4x3 display, but it is really used as 4xRGB?

> Given that, it seems to match the LED subsystem better than auxdisplay, doesn't it?

Seems so, yes. From description, I assumed it was 4x3xRGB, not 4xRGB.

Best regards,
								Pavel
-- 
I don't work for Nazis and criminals, and neither should you.
Boycott Putin, Trump, and Musk!

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

Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix LED driver

[Nam Tran]

On Sun, 27 Apr 2025 Andy Shevchenko wrote:

> On Sun, Apr 27, 2025 at 11:25 AM Nam Tran <trannamatk@gmail.com> wrote:
> >
> > This patch series adds support for the TI/National Semiconductor LP5812
> > 4x3 matrix RGB LED driver. The driver supports features such as autonomous
> > animation and time-cross-multiplexing (TCM) for dynamic LED effects.
> >
> > Signed-off-by: Nam Tran <trannamatk@gmail.com>
> > ---
> > Changes in v8:
> > - Move driver to drivers/auxdisplay/ instead of drivers/leds/.
> > - Rename files from leds-lp5812.c/.h to lp5812.c/.h.
> > - Move ti,lp5812.yaml binding to auxdisplay/ directory,
> >   and update the title and $id to match new path.
> > - No functional changes to the binding itself (keep Reviewed-by).
> > - Update commit messages and patch titles to reflect the move.
> > - Link to v7: https://lore.kernel.org/linux-leds/20250422190121.46839-1-trannamatk@gmail.com/
> 
> Out of sudden without discussing with auxdisplay maintainers/reviewers?
> Thanks, no.
> Please, put into the cover letter the meaningful summary of what's
> going on and why this becomes an auxdisplay issue. Brief review of the
> bindings sounds more likely like LEDS or PWM subsystems.

Apologies for moving the driver to auxdisplay without prior discussion with you
and the other auxdisplay maintainers.

The decision to move it was based on advice from Lee Jones (LED subsystem co-maintainer).
He reviewed the v7 series while it was still under drivers/leds/, and explicitly recommended
that I move it to drivers/auxdisplay/.
Reference: https://lore.kernel.org/linux-leds/20250425101112.GB1567507@google.com/

Here’s a brief summary of why LP5812 fits better in auxdisplay than in LEDS or PWM subsystems:
- 4 outputs drive 12 LEDs (4 RGB) using time-cross-multiplexing (TCM).
- An autonomous animation engine creates complex visual effects without CPU intervention.
- Supports analog current control, PWM dimming up to 24kHz, de-ghosting, and phase shifting,
  all targeting dynamic visual outputs rather than static LED states.

I will prepare a v9 with an updated cover letter summarizing this background.
I am also happy to make further adjustments based on your and other auxdisplay maintainers’ guidance.

Thanks for reviewing and helping me through the submission process.

Best regards,
Nam Tran