leds: add new LED driver for TI LP5812
Subsystems (4)
Nam Tran
maintainercover leds: add new LED driver for TI LP5812
From: Nam Tran <[email protected]>
To: Pavel Machek <[email protected]>, Lee Jones <[email protected]>, Krzysztof Kozlowski <[email protected]>, Rob Herring <[email protected]>, Conor Dooley <[email protected]>
Cc: [email protected], [email protected], [email protected]
This patch series adds support for the Texas Instruments LP5812 LED driver. Patch 1 adds the Device Tree (DT) bindings documentation.
Patch 2 introduces the core driver implementation.
The LP5812 is an I2C-controlled LED driver capable of driving up to 4 LEDs with independent brightness control and pattern programming. This driver provides support through the Linux LED framework and includes device tree bindings for proper integration.
This driver has been tested on Raspberry Pi 4 B using kernel version 6.14.0.
I kindly request feedback from the community to ensure that this driver adheres to the Linux LED subsystem standards.
Thank you for your consideration.
Best regards,
Nam
---
Changes in v2:
- Renamed DT bindings file from leds-lp5812.yaml to ti,lp5812.yaml.
- Added LP5812 device tree node to bcm2711-rpi-4b.dts for Raspberry Pi 4B.
- Updated core driver implementation to fix build errors in kernel version 6.14.0-rc4. (The v1 patch was only compatible with kernel version 6.1.93)
Nam Tran
maintainerPATCH v2 1/2 dt-bindings: leds: Add LP5812 LED driver bindings
This documentation ensures proper integration of the LP5812 in Device Tree-based systems.
--- .../devicetree/bindings/leds/ti,lp5812.yaml | 34 +++++++++++++++++++ MAINTAINERS | 6 ++++ 2 files changed, 40 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/ti,lp5812.yaml
Documentation/devicetree/bindings/leds/ti,lp5812.yaml new file +34 -0
@@ -0,0 +1,34 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 + +title: LED driver for LP5812 LED from Texas Instruments. + +maintainers: + +description: | + The LP5812 is an I2C LED Driver that can support LED matrix 4x3. + +properties: + compatible: + const: ti,lp5812 + + reg: + description: I2C slave address + maxItems: 1 + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + i2c1 { + lp5812@1b { + compatible = "ti,lp5812"; + reg = <0x1b>; + }; + }; + +...
MAINTAINERS +6 -0
@@ -23494,6 +23494,12 @@ S: Maintained F: Documentation/devicetree/bindings/leds/backlight/ti,lp8864.yaml F: drivers/leds/leds-lp8864.c +TEXAS INSTRUMENTS' LP5812 LED DRIVER +M: Nam Tran <[email protected]> +L: [email protected] +S: Maintained +F: Documentation/devicetree/bindings/leds/ti,lp5812.yaml + TEXAS INSTRUMENTS' SYSTEM CONTROL INTERFACE (TISCI) PROTOCOL DRIVER M: Nishanth Menon <[email protected]> M: Tero Kristo <[email protected]>
Signature · 1 line
2.25.1
Rob Herring (Arm)
maintainerRe: [PATCH v2 1/2] dt-bindings: leds: Add LP5812 LED driver bindings
Quoted 10 lines · Nam Tran
This documentation ensures proper integration of the LP5812 in Device Tree-based systems.
Signed-off-by Nam Tran <[email protected]>--- .../devicetree/bindings/leds/ti,lp5812.yaml | 34 +++++++++++++++++++ MAINTAINERS | 6 ++++ 2 files changed, 40 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/ti,lp5812.yaml
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:./Documentation/devicetree/bindings/leds/ti,lp5812.yaml:4:1: [error] syntax error: expected '<document start>', but found '<block mapping start>' (syntax) ./Documentation/devicetree/bindings/leds/ti,lp5812.yaml:6:13: [error] empty value in block mapping (empty-values)
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/leds/ti,lp5812.yaml: ignoring, error parsing file ./Documentation/devicetree/bindings/leds/ti,lp5812.yaml:4:1: expected '<document start>', but found ('<block mapping start>',) make[2]: *** Deleting file 'Documentation/devicetree/bindings/leds/ti,lp5812.example.dts' Documentation/devicetree/bindings/leds/ti,lp5812.yaml:4:1: expected '<document start>', but found ('<block mapping start>',) make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/leds/ti,lp5812.example.dts] Error 1 make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1511: dt_binding_check] Error 2
make: *** [Makefile:251: __sub-make] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Re: [PATCH v2 1/2] dt-bindings: leds: Add LP5812 LED driver bindings
In reply to Nam TranQuoted 2 lines · Nam Tran
This documentation ensures proper integration of the LP5812 in Device Tree-based systems.
Nothing improved above.
Please read my previous comment and implement it or respond to it, if you disagree.
A nit, subject: drop second/last, redundant "bindings". The "dt-bindings" prefix is already stating that these are bindings. See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
Quoted 14 lines
Signed-off-by Nam Tran <[email protected]>--- .../devicetree/bindings/leds/ti,lp5812.yaml | 34 +++++++++++++++++++ MAINTAINERS | 6 ++++ 2 files changed, 40 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/ti,lp5812.yamlDocumentation/devicetree/bindings/leds/ti,lp5812.yaml new file +0
-0
Documentation/devicetree/bindings/leds/ti,lp5812.yaml line 4
Quoted 4 lines
Documentation/devicetree/bindings/leds/ti,lp5812.yaml new file +4
-0@@ -0,0 +1,34 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 + +title: LED driver for LP5812 LED from Texas Instruments.
LED driver as Linux driver? Or LP5812 LED driver as electrical driver? Decide, but if first then rewrite it to describe hardware.
Also drop full stop. Titles do not have full stops.
Documentation/devicetree/bindings/leds/ti,lp5812.yaml line 8
Quoted 4 lines
Documentation/devicetree/bindings/leds/ti,lp5812.yaml new file +4
-0+ +maintainers: + +description: |
Do not need '|' unless you need to preserve formatting.
Documentation/devicetree/bindings/leds/ti,lp5812.yaml line 9
Quoted 1 line
Documentation/devicetree/bindings/leds/ti,lp5812.yaml new file +1
-0+ The LP5812 is an I2C LED Driver that can support LED matrix 4x3.
So Linux driver? Drop driver references and describe hardware.
Documentation/devicetree/bindings/leds/ti,lp5812.yaml line 16
Quoted 7 lines
Documentation/devicetree/bindings/leds/ti,lp5812.yaml new file +7
-0+ +properties: + compatible: + const: ti,lp5812 + + reg: + description: I2C slave address
Drop description
Documentation/devicetree/bindings/leds/ti,lp5812.yaml line 21
Quoted 5 lines
Documentation/devicetree/bindings/leds/ti,lp5812.yaml new file +5
-0+ maxItems: 1 + +required: + - compatible + - reg
No ref to LED common schema? No other properties? This is too incomplete.
Documentation/devicetree/bindings/leds/ti,lp5812.yaml line 27
Quoted 6 lines
Documentation/devicetree/bindings/leds/ti,lp5812.yaml new file +6
-0+ +additionalProperties: false + +examples: + - | + i2c1 {
i2c
Documentation/devicetree/bindings/leds/ti,lp5812.yaml line 28
Quoted 1 line
Documentation/devicetree/bindings/leds/ti,lp5812.yaml new file +1
-0+ lp5812@1b {
Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
Quoted 11 lines
Documentation/devicetree/bindings/leds/ti,lp5812.yaml new file +6
-0+ compatible = "ti,lp5812"; + reg = <0x1b>; + }; + }; + +...MAINTAINERS +0
-0
MAINTAINERS S: Maintained · line 23497
Quoted 4 lines
MAINTAINERS +1
-0@@ -23494,6 +23494,12 @@ S: Maintained F: Documentation/devicetree/bindings/leds/backlight/ti,lp8864.yaml F: drivers/leds/leds-lp8864.c +TEXAS INSTRUMENTS' LP5812 LED DRIVER
5 is before 8, so this does not look sorted.
And finally, please test before posting...
Best regards,
Krzysztof
Nam Tran
maintainerPATCH v2 2/2 leds: add new LED driver for TI LP5812
In reply to Nam TranThe chip can drive LED matrix 4x3.
This driver uses I2C to communicate with the chip.
--- MAINTAINERS | 6 + .../arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts | 7 + drivers/leds/Kconfig | 13 + drivers/leds/Makefile | 2 + drivers/leds/leds-lp5812-common.c | 1097 ++++++++ drivers/leds/leds-lp5812-common.h | 323 +++ drivers/leds/leds-lp5812-regs.h | 96 + drivers/leds/leds-lp5812.c | 2317 +++++++++++++++++ 8 files changed, 3861 insertions(+) create mode 100644 drivers/leds/leds-lp5812-common.c create mode 100644 drivers/leds/leds-lp5812-common.h create mode 100644 drivers/leds/leds-lp5812-regs.h create mode 100644 drivers/leds/leds-lp5812.c
MAINTAINERS +6 -0
@@ -23499,6 +23499,12 @@ M: Nam Tran <[email protected]> L: [email protected] S: Maintained F: Documentation/devicetree/bindings/leds/ti,lp5812.yaml +F: drivers/leds/Kconfig +F: drivers/leds/Makefile +F: drivers/leds/leds-lp5812-common.c +F: drivers/leds/leds-lp5812-common.h +F: drivers/leds/leds-lp5812-regs.h +F: drivers/leds/leds-lp5812.c TEXAS INSTRUMENTS' SYSTEM CONTROL INTERFACE (TISCI) PROTOCOL DRIVER M: Nishanth Menon <[email protected]>
arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts +7 -0
@@ -270,3 +270,10 @@ &vec { &wifi_pwrseq { reset-gpios = <&expgpio 1 GPIO_ACTIVE_LOW>; }; + +&i2c1 { + lp5812@1b { + compatible = "ti,lp5812"; + reg = <0x1b>; + }; +};
drivers/leds/Kconfig +13 -0
@@ -483,6 +483,19 @@ config LEDS_LP5569 Driver provides direct control via LED class and interface for programming the engines. +config LEDS_LP5812 + bool "Enable LP5812 support LED matrix 4x3" + depends on LEDS_CLASS && I2C + depends on LEDS_CLASS_MULTICOLOR + help + If you say Y here you get support for TI LP5812 LED driver. + + The LP5812 is an I2C-based RGB LED driver that supports a 4x3 LED matrix. + It provides independent brightness control and color mixing for each LED. + This driver integrates with the Linux LED framework and LED multicolor class. + + If unsure, say N. + config LEDS_LP8501 tristate "LED Support for TI LP8501 LED driver chip" depends on LEDS_CLASS && I2C
drivers/leds/Makefile +2 -0
@@ -54,6 +54,8 @@ obj-$(CONFIG_LEDS_LP5523) += leds-lp5523.o obj-$(CONFIG_LEDS_LP5562) += leds-lp5562.o obj-$(CONFIG_LEDS_LP5569) += leds-lp5569.o obj-$(CONFIG_LEDS_LP55XX_COMMON) += leds-lp55xx-common.o +obj-$(CONFIG_LEDS_LP5812) += leds-lp5812.o +leds-lp5812-objs := leds-lp5812.o leds-lp5812-common.o obj-$(CONFIG_LEDS_LP8501) += leds-lp8501.o obj-$(CONFIG_LEDS_LP8788) += leds-lp8788.o obj-$(CONFIG_LEDS_LP8860) += leds-lp8860.o
drivers/leds/leds-lp5812-common.c new file +1097 -0
1,097 lines not on this page · raw message
drivers/leds/leds-lp5812-common.h new file +323 -0
323 lines not on this page · raw message
drivers/leds/leds-lp5812-regs.h new file +96 -0
96 lines not on this page · raw message
drivers/leds/leds-lp5812.c new file +2317 -0
1,435 lines not on this page · raw message
The rest of this diff is not shown. View the raw message.
Signature · 1 line
2.25.1
Part of the diff is not shown here. View the raw message.
Quoted 2 lines · Nam Tran
The chip can drive LED matrix 4x3.
This driver uses I2C to communicate with the chip.
You decided to ignore my previous question, but it still valid. Respond to it.
Sorry, if you keep ignoring reviewers, you will not get anywhere.
Quoted 5 lines
Signed-off-by Nam Tran <[email protected]>--- MAINTAINERS | 6 + .../arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts | 7 +
No, DTS is not part of LED. You cannot combine it in the same patch.
Quoted 34 lines
drivers/leds/Kconfig | 13 + drivers/leds/Makefile | 2 + drivers/leds/leds-lp5812-common.c | 1097 ++++++++ drivers/leds/leds-lp5812-common.h | 323 +++ drivers/leds/leds-lp5812-regs.h | 96 + drivers/leds/leds-lp5812.c | 2317 +++++++++++++++++ 8 files changed, 3861 insertions(+) create mode 100644 drivers/leds/leds-lp5812-common.c create mode 100644 drivers/leds/leds-lp5812-common.h create mode 100644 drivers/leds/leds-lp5812-regs.h create mode 100644 drivers/leds/leds-lp5812.cMAINTAINERS +6
-0@@ -23499,6 +23499,12 @@ M: Nam Tran <[email protected]> L: [email protected] S: Maintained F: Documentation/devicetree/bindings/leds/ti,lp5812.yaml +F: drivers/leds/Kconfig +F: drivers/leds/Makefile +F: drivers/leds/leds-lp5812-common.c +F: drivers/leds/leds-lp5812-common.h +F: drivers/leds/leds-lp5812-regs.h +F: drivers/leds/leds-lp5812.c TEXAS INSTRUMENTS' SYSTEM CONTROL INTERFACE (TISCI) PROTOCOL DRIVER M: Nishanth Menon <[email protected]>arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts +0
-0
arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts &vec { · line 275
Quoted 6 lines
arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts +3
-0@@ -270,3 +270,10 @@ &vec { &wifi_pwrseq { reset-gpios = <&expgpio 1 GPIO_ACTIVE_LOW>; }; + +&i2c1 { + lp5812@1b {
Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
Quoted 12 lines
arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts +4
-0+ compatible = "ti,lp5812"; + reg = <0x1b>; + }; +};drivers/leds/Kconfig +0
-0@@ -483,6 +483,19 @@ config LEDS_LP5569 Driver provides direct control via LED class and interface for programming the engines.
...
Quoted 9 lines
drivers/leds/Kconfig +9
-0+ pr_info("[DEBUG]: sysfs_create_group successfully\n"); + + ret = lp5812_init_dev_config(chip, "tcmscan:4:0:1:2:3", 0); + if (ret) { + pr_info("[DEBUG]: %s: lp5812_init_dev_config failed\n", + __func__); + return ret; + } + pr_info("[DEBUG]: lp5812_init_dev_config successfully\n");
drivers/leds/Kconfig
Quoted 10 lines
drivers/leds/Kconfig +10
-0+ /* initialize lp5812 chip */ + ret = lp5812_initialize(chip); + + /* code to verify i2c read/write ok or not */ + lp5812_read(chip, (u16)DEV_CONFIG2, &val); + pr_info("[DEBUG]: DEV_CONFIG2 value: 0x%02X\n", val); + + lp5812_write(chip, (u16)LED_A1_AUTO_BASE_ADRR, 0x14); + lp5812_read(chip, (u16)LED_A1_AUTO_BASE_ADRR, &val); + pr_info("[DEBUG]: LED_A1_AUTO_BASE_ADRR value: 0x%02X\n", val);
So entire driver is full of such debug code, looking straight from some Android vendor tree.
No, that's not how upstreaming works. You now copy all bad habits, all poor code and all the issues we fixed. Start from scratch from latest mainline driver and customize it to your needs.
Best regards,
Krzysztof
kernel test robot
build botkernel test robot noticed the following build errors:
Hi Nam,
kernel test robot noticed the following build errors:
[auto build test ERROR on lee-leds/for-leds-next]
[also build test ERROR on robh/for-next linus/master v6.14-rc5] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Nam-Tran/dt-bindings-leds-Add-LP5812-LED-driver-bindings/20250226-011634 base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/leds.git for-leds-next patch link: https://lore.kernel.org/r/20250225170601.21334-3-trannamatk%40gmail.com patch subject: [PATCH v2 2/2] leds: add new LED driver for TI LP5812 config: x86_64-randconfig-006-20250303 (https://download.01.org/0day-ci/archive/20250304/[email protected]/config) compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250304/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
>> ld.lld: error: undefined symbol: i2c_register_driver >>> referenced by leds-lp5812.c:2313 (drivers/leds/leds-lp5812.c:2313) >>> vmlinux.o:(lp5812_driver_init) -- >> ld.lld: error: undefined symbol: i2c_del_driver >>> referenced by leds-lp5812.c:2313 (drivers/leds/leds-lp5812.c:2313) >>> vmlinux.o:(lp5812_driver_exit)
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Rob Herring (Arm)
maintainerQuoted 20 lines · Nam Tran
From: Nam Tran <[email protected]>
To: Pavel Machek <[email protected]>, Lee Jones <[email protected]>, Krzysztof Kozlowski <[email protected]>, Rob Herring <[email protected]>, Conor Dooley <[email protected]>
Cc: [email protected], [email protected], [email protected]This patch series adds support for the Texas Instruments LP5812 LED driver. Patch 1 adds the Device Tree (DT) bindings documentation.
Patch 2 introduces the core driver implementation.The LP5812 is an I2C-controlled LED driver capable of driving up to 4 LEDs with independent brightness control and pattern programming. This driver provides support through the Linux LED framework and includes device tree bindings for proper integration.
This driver has been tested on Raspberry Pi 4 B using kernel version 6.14.0.
I kindly request feedback from the community to ensure that this driver adheres to the Linux LED subsystem standards.
Thank you for your consideration.
Best regards,
Nam
Quoted 10 lines
---
Changes in v2:
- Renamed DT bindings file fromleds-lp5812.yamltoti,lp5812.yaml.
- Added LP5812 device tree node tobcm2711-rpi-4b.dtsfor Raspberry Pi 4B.
- Updated core driver implementation to fix build errors in kernel version 6.14.0-rc4. (The v1 patch was only compatible with kernel version 6.1.93)Signed-off-by Nam Tran <[email protected]>
My bot found new DTB warnings on the .dts files added or changed in this series.
Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings are fixed by another series. Ultimately, it is up to the platform maintainer whether these warnings are acceptable or not. No need to reply unless the platform maintainer has comments.
If you already ran DT checks and didn't see these error(s), then make sure dt-schema is up to date:
pip3 install dtschema --upgrade
New warnings running 'make CHECK_DTBS=y for arch/arm/boot/dts/broadcom/' for [email protected]:
arch/arm/boot/dts/broadcom/bcm2711-rpi-400.dtb: /soc/i2c@7e804000/lp5812@1b: failed to match any schema with compatible: ['ti,lp5812'] arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dtb: /soc/i2c@7e804000/lp5812@1b: failed to match any schema with compatible: ['ti,lp5812']