This patch series adds initial support for the TI LP5812,
a 4x3 matrix RGB LED driver with autonomous engine control.
This version provides a minimal, clean implementation focused
on core functionality only. The goal is to upstream a solid
foundation, with the expectation that additional features can
be added incrementally in future patches.

The driver integrates with the LED multicolor framework and
supports a set of basic sysfs interfaces for LED control and
chip management.

Signed-off-by: Nam Tran <trannamatk@gmail.com>
---
Changes in v14:
- Replaced inline constants with proper macros for readability and maintainability.
- Refactored lp5812_read() and lp5812_write() to simplify logic and improve clarity.
- Updated lp5812_fault_clear() to use switch() instead of if/else chain.
- Refactored parse_drive_mode() for cleaner logic, removed string parsing of concatenated data.
- Updated activate_store() and led_current_store() to replace strsep()/kstrtoint() parsing with sscanf().
- Removed redundant comments and renamed variables for better clarity.
- Link to v13: https://lore.kernel.org/lkml/20250818012654.143058-1-trannamatk@gmail.com/

Changes in v13:
- Fixes build warnings reported by kernel test robot:
  - Inconsistent indent in lp5812_probe()
  - Uninitialized variable 'ret' in lp5812_multicolor_brightness()
- Drop of_match_ptr() and directly assign of_match_table, as the driver is DT-only.
- Link to v12: https://lore.kernel.org/lkml/20250728065814.120769-1-trannamatk@gmail.com/

Changes in v12:
- Reordered helper functions above lp5812_probe() for better structure.
- Clarified DT-only support by removing fallback paths and i2c_device_id table.
- Directly assign platform_data to the correct pointer instead of relying on
  string comparisons (LP5812_SC_LED, LP5812_MC_LED) and container_of() casting.
  This simplifies the logic and avoids unnecessary type checks.
- Removed redundant messages.
- Update ABI documentation to reflect reduced feature set.
- Link to v11: https://lore.kernel.org/lkml/20250714172355.84609-1-trannamatk@gmail.com/

Changes in v11:
- Drop autonomous animation and other advanced features; reduce driver to core functionality only.
- Simplify LED parsing to use a unified path.
- Clean up and streamline code
  - Use alphabetically ordered includes
  - Remove redundant comments
  - Fix style issues (e.g., comment capitalization, code placement)
- Update ABI documentation to reflect reduced feature set.
- Link to v10: https://lore.kernel.org/lkml/20250618183205.113344-1-trannamatk@gmail.com/

Changes in v10:
- Address feedback on v9 regarding missing Reviewed-by tag
- Added explanation: binding structure changed significantly to integrate
  with the standard leds-class-multicolor.yaml schema and support multi-led@
  nodes with nested led@ subnodes. This change introduced a new patternProperties
  hierarchy and removed the previous flat led@ layout used in the earlier versions.
  So the Reviewed-by tag was dropped out of caution.
- Address binding document feedback
  - Use consistent quotes
  - Replace 'max-cur' with the standard 'led-max-microamp'
  - Remove 'led-cur' property
  - Fix mixed indentation
- Updated core driver to align with the updated binding schema.
- Address core driver feedback
  - Use for_each_available_child_of_node_scoped() to simplify the code
  - Add a return checks for lp5812_write() and lp5812_read()
  - Remove unneeded trailing commas
  - Fix unsafe usage of stack-allocated strings
- Link to v9: https://lore.kernel.org/lkml/20250617154020.7785-1-trannamatk@gmail.com/

Changes in v9:
- Move driver back to drivers/leds/rgb/
- Integrate with LED multicolor framework
- Refactor and simplify custom sysfs handling
- Extend Device Tree binding to support multi-led@ nodes using leds-class-multicolor.yaml
- Update documentation to reflect the updated sysfs.
- Link to v8: https://lore.kernel.org/lkml/20250427082447.138359-1-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 (4):
  dt-bindings: leds: add TI/National Semiconductor LP5812 LED Driver
  leds: add basic support for TI/National Semiconductor LP5812 LED
    Driver
  docs: ABI: Document LP5812 LED sysfs interfaces
  docs: leds: Document TI LP5812 LED driver

 .../ABI/testing/sysfs-bus-i2c-devices-lp5812  |   32 +
 .../ABI/testing/sysfs-class-led-lp5812        |   37 +
 .../devicetree/bindings/leds/ti,lp5812.yaml   |  229 ++++
 Documentation/leds/index.rst                  |    1 +
 Documentation/leds/leds-lp5812.rst            |   46 +
 MAINTAINERS                                   |   13 +
 drivers/leds/rgb/Kconfig                      |   13 +
 drivers/leds/rgb/Makefile                     |    1 +
 drivers/leds/rgb/leds-lp5812.c                | 1089 +++++++++++++++++
 drivers/leds/rgb/leds-lp5812.h                |  206 ++++
 10 files changed, 1667 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c-devices-lp5812
 create mode 100644 Documentation/ABI/testing/sysfs-class-led-lp5812
 create mode 100644 Documentation/devicetree/bindings/leds/ti,lp5812.yaml
 create mode 100644 Documentation/leds/leds-lp5812.rst
 create mode 100644 drivers/leds/rgb/leds-lp5812.c
 create mode 100644 drivers/leds/rgb/leds-lp5812.h


base-commit: b236920731dd90c3fba8c227aa0c4dee5351a639
-- 
2.25.1

On Sun, Sep 07, 2025 at 11:09:40PM +0700, Nam Tran wrote:
> This patch series adds initial support for the TI LP5812,
> a 4x3 matrix RGB LED driver with autonomous engine control.
> This version provides a minimal, clean implementation focused
> on core functionality only. The goal is to upstream a solid
> foundation, with the expectation that additional features can
> be added incrementally in future patches.
> 
> The driver integrates with the LED multicolor framework and
> supports a set of basic sysfs interfaces for LED control and
> chip management.
> 
> Signed-off-by: Nam Tran <trannamatk@gmail.com>

The sysfs api is really odd here.  WHy not do the same thing as this
other controller recently submitted does:
	https://lore.kernel.org/r/20250911-v6-14-topic-ti-lp5860-v3-0-390738ef9d71@pengutronix.de

but better yet, why does this need to be a kernel driver at all?  Why
can't you just control this directly from userspace with a program
there?

For USB, we generally do not allow these types of crazy apis to be added
to the kernel when controlling the device can be done from userspace.  I
think the same thing can happen here too, right?

thanks,

greg k-h

On Thu, 11 Sep 2025, Greg KH wrote:

> On Sun, Sep 07, 2025 at 11:09:40PM +0700, Nam Tran wrote:
> > This patch series adds initial support for the TI LP5812,
> > a 4x3 matrix RGB LED driver with autonomous engine control.
> > This version provides a minimal, clean implementation focused
> > on core functionality only. The goal is to upstream a solid
> > foundation, with the expectation that additional features can
> > be added incrementally in future patches.
> > 
> > The driver integrates with the LED multicolor framework and
> > supports a set of basic sysfs interfaces for LED control and
> > chip management.
> > 
> > Signed-off-by: Nam Tran <trannamatk@gmail.com>
> 
> The sysfs api is really odd here.  WHy not do the same thing as this
> other controller recently submitted does:
> 	https://lore.kernel.org/r/20250911-v6-14-topic-ti-lp5860-v3-0-390738ef9d71@pengutronix.de

Thank you for the feedback!
I agree that consistency is important, and I've reviewed the patch you referenced.

I also checked the LP5860 datasheet and noticed that its driver exposes sysfs entries
for configuring registers like `R_current_set`, `G_current_set`, and `B_current_set`.
Similarly, the LP5812 requires register-level configuration for operation.

In my driver, I've implemented the following sysfs attributes:
- '/sys/bus/i2c/devices/.../lp5812_chip_setup/dev_config' - Configures drive mode and
scan order (Dev_Config_1 and Dev_Config_2 registers).
- '/sys/bus/i2c/devices/.../lp5812_chip_setup/sw_reset' - Triggers a software reset of
the device (Reset register).
- '/sys/bus/i2c/devices/.../lp5812_chip_setup/fault_clear' - Clears fault status
(Fault_Clear register).
- '/sys/class/leds/led_<id>/activate' - Activate or deactivate the specified LED channel
in runtime (led_en_1, led_en_2 registers).
- '/sys/class/leds/led_<id>/led_current' - To change DC/PWM current level of each led
(Manual_DC_xx and Manual_PWM_xx registers).
- '/sys/class/leds/led_<id>/max_current' - To show max current setting (Dev_Config_0 register).
- '/sys/class/leds/led_<id>/lod_lsd' - To read lod, lsd status of each LED
(LOD_Status_0, LOD_Status_1, LSD_Status_0, LSD_Status_1 registers).

These attributes map directly to LP5812 registers. I’ve kept the interface minimal and
focused only on essential functionality needed to operate the device.

If any of these attributes seem unconventional or redundant, I’d appreciate clarification
so I can revise accordingly.

> but better yet, why does this need to be a kernel driver at all?  Why
> can't you just control this directly from userspace with a program
> there?

LP5812 is controlled via I2C, and its register map is non-trivial. Moving control to userspace
would require users to manually handle I2C transactions and understand the register layout,
which is error-prone and not user-friendly.

Moreover, the driver integrates with the LED multicolor framework, allowing standardized control
via existing userspace tools. This abstraction is difficult to achieve reliably from userspace alone.

> For USB, we generally do not allow these types of crazy apis to be added
> to the kernel when controlling the device can be done from userspace.  I
> think the same thing can happen here too, right?

USB devices benefit from standardized descriptors and interfaces, which reduce the need for custom
sysfs APIs. In contrast, LP5812 has no such standard interface, and some customization is necessary.

I’m open to improving the sysfs interface or moving parts to another method if that’s more appropriate.
Please let me know which specific changes you’d recommend.

For completeness, I considered these methods:
- sysfs: Recommended and standard for LED drivers.
- i2c-tools: Not recommended, intended for development/debug only.
- ioctl: Not recommended for new LED drivers.
- debugfs: For debugging only.
- Direct I2C register access: Requires users to know the register map and protocol.

Thanks again for the review!

Best regards,
Nam Tran

On Tue, Sep 23, 2025 at 01:13:41AM +0700, Nam Tran wrote:
> On Thu, 11 Sep 2025, Greg KH wrote:
> 
> > On Sun, Sep 07, 2025 at 11:09:40PM +0700, Nam Tran wrote:
> > > This patch series adds initial support for the TI LP5812,
> > > a 4x3 matrix RGB LED driver with autonomous engine control.
> > > This version provides a minimal, clean implementation focused
> > > on core functionality only. The goal is to upstream a solid
> > > foundation, with the expectation that additional features can
> > > be added incrementally in future patches.
> > > 
> > > The driver integrates with the LED multicolor framework and
> > > supports a set of basic sysfs interfaces for LED control and
> > > chip management.
> > > 
> > > Signed-off-by: Nam Tran <trannamatk@gmail.com>
> > 
> > The sysfs api is really odd here.  WHy not do the same thing as this
> > other controller recently submitted does:
> > 	https://lore.kernel.org/r/20250911-v6-14-topic-ti-lp5860-v3-0-390738ef9d71@pengutronix.de
> 
> Thank you for the feedback!
> I agree that consistency is important, and I've reviewed the patch you referenced.
> 
> I also checked the LP5860 datasheet and noticed that its driver exposes sysfs entries
> for configuring registers like `R_current_set`, `G_current_set`, and `B_current_set`.
> Similarly, the LP5812 requires register-level configuration for operation.
> 
> In my driver, I've implemented the following sysfs attributes:
> - '/sys/bus/i2c/devices/.../lp5812_chip_setup/dev_config' - Configures drive mode and
> scan order (Dev_Config_1 and Dev_Config_2 registers).
> - '/sys/bus/i2c/devices/.../lp5812_chip_setup/sw_reset' - Triggers a software reset of
> the device (Reset register).
> - '/sys/bus/i2c/devices/.../lp5812_chip_setup/fault_clear' - Clears fault status
> (Fault_Clear register).
> - '/sys/class/leds/led_<id>/activate' - Activate or deactivate the specified LED channel
> in runtime (led_en_1, led_en_2 registers).
> - '/sys/class/leds/led_<id>/led_current' - To change DC/PWM current level of each led
> (Manual_DC_xx and Manual_PWM_xx registers).
> - '/sys/class/leds/led_<id>/max_current' - To show max current setting (Dev_Config_0 register).
> - '/sys/class/leds/led_<id>/lod_lsd' - To read lod, lsd status of each LED
> (LOD_Status_0, LOD_Status_1, LSD_Status_0, LSD_Status_1 registers).
> 
> These attributes map directly to LP5812 registers. I’ve kept the interface minimal and
> focused only on essential functionality needed to operate the device.
> 
> If any of these attributes seem unconventional or redundant, I’d appreciate clarification
> so I can revise accordingly.
> 
> > but better yet, why does this need to be a kernel driver at all?  Why
> > can't you just control this directly from userspace with a program
> > there?
> 
> LP5812 is controlled via I2C, and its register map is non-trivial. Moving control to userspace
> would require users to manually handle I2C transactions and understand the register layout,
> which is error-prone and not user-friendly.

So you write it once in a library, or in a userspace program, and it is
done.  Don't expose these low-level things in a custom api that could be
done in userspace instead.

> Moreover, the driver integrates with the LED multicolor framework, allowing standardized control
> via existing userspace tools. This abstraction is difficult to achieve reliably from userspace alone.

But this is a custom api for the leds, not like any other one out there.
So how would it integrate with anything else?

> > For USB, we generally do not allow these types of crazy apis to be added
> > to the kernel when controlling the device can be done from userspace.  I
> > think the same thing can happen here too, right?
> 
> USB devices benefit from standardized descriptors and interfaces, which reduce the need for custom
> sysfs APIs. In contrast, LP5812 has no such standard interface, and some customization is necessary.

Many USB devices do not benifit from that at all, you directly control
them from userspace using vendor-specific apis.  Just like this device,
nothing different just because it is an i2c device.

> I’m open to improving the sysfs interface or moving parts to another method if that’s more appropriate.
> Please let me know which specific changes you’d recommend.

sysfs really doesn't seem to be the correct api here, you are making a
custom one just for this one device that is not shared by any other one,
so userspace has to write custom code to control it.  So why not just
write one program, in userspace, to handle it all at once, instead of 2?

> For completeness, I considered these methods:
> - sysfs: Recommended and standard for LED drivers.
> - i2c-tools: Not recommended, intended for development/debug only.
> - ioctl: Not recommended for new LED drivers.
> - debugfs: For debugging only.
> - Direct I2C register access: Requires users to know the register map and protocol.

A library will handle the i2c direct register access.  Again, do not
make custom sysfs apis if at all possible.

thanks,

greg k-h

On Mon, 22 Sep 2025, Greg KH wrote:

> On Tue, Sep 23, 2025 at 01:13:41AM +0700, Nam Tran wrote:
> > On Thu, 11 Sep 2025, Greg KH wrote:
> > 
> > > On Sun, Sep 07, 2025 at 11:09:40PM +0700, Nam Tran wrote:
> > > > This patch series adds initial support for the TI LP5812,
> > > > a 4x3 matrix RGB LED driver with autonomous engine control.
> > > > This version provides a minimal, clean implementation focused
> > > > on core functionality only. The goal is to upstream a solid
> > > > foundation, with the expectation that additional features can
> > > > be added incrementally in future patches.
> > > > 
> > > > The driver integrates with the LED multicolor framework and
> > > > supports a set of basic sysfs interfaces for LED control and
> > > > chip management.
> > > > 
> > > > Signed-off-by: Nam Tran <trannamatk@gmail.com>
> > > 
> > > The sysfs api is really odd here.  WHy not do the same thing as this
> > > other controller recently submitted does:
> > > 	https://lore.kernel.org/r/20250911-v6-14-topic-ti-lp5860-v3-0-390738ef9d71@pengutronix.de
> > 
> > Thank you for the feedback!
> > I agree that consistency is important, and I've reviewed the patch you referenced.
> > 
> > I also checked the LP5860 datasheet and noticed that its driver exposes sysfs entries
> > for configuring registers like `R_current_set`, `G_current_set`, and `B_current_set`.
> > Similarly, the LP5812 requires register-level configuration for operation.
> > 
> > In my driver, I've implemented the following sysfs attributes:
> > - '/sys/bus/i2c/devices/.../lp5812_chip_setup/dev_config' - Configures drive mode and
> > scan order (Dev_Config_1 and Dev_Config_2 registers).
> > - '/sys/bus/i2c/devices/.../lp5812_chip_setup/sw_reset' - Triggers a software reset of
> > the device (Reset register).
> > - '/sys/bus/i2c/devices/.../lp5812_chip_setup/fault_clear' - Clears fault status
> > (Fault_Clear register).
> > - '/sys/class/leds/led_<id>/activate' - Activate or deactivate the specified LED channel
> > in runtime (led_en_1, led_en_2 registers).
> > - '/sys/class/leds/led_<id>/led_current' - To change DC/PWM current level of each led
> > (Manual_DC_xx and Manual_PWM_xx registers).
> > - '/sys/class/leds/led_<id>/max_current' - To show max current setting (Dev_Config_0 register).
> > - '/sys/class/leds/led_<id>/lod_lsd' - To read lod, lsd status of each LED
> > (LOD_Status_0, LOD_Status_1, LSD_Status_0, LSD_Status_1 registers).
> > 
> > These attributes map directly to LP5812 registers. I’ve kept the interface minimal and
> > focused only on essential functionality needed to operate the device.
> > 
> > If any of these attributes seem unconventional or redundant, I’d appreciate clarification
> > so I can revise accordingly.
> > 
> > > but better yet, why does this need to be a kernel driver at all?  Why
> > > can't you just control this directly from userspace with a program
> > > there?
> > 
> > LP5812 is controlled via I2C, and its register map is non-trivial. Moving control to userspace
> > would require users to manually handle I2C transactions and understand the register layout,
> > which is error-prone and not user-friendly.
> 
> So you write it once in a library, or in a userspace program, and it is
> done.  Don't expose these low-level things in a custom api that could be
> done in userspace instead.
> 
> > Moreover, the driver integrates with the LED multicolor framework, allowing standardized control
> > via existing userspace tools. This abstraction is difficult to achieve reliably from userspace alone.
> 
> But this is a custom api for the leds, not like any other one out there.
> So how would it integrate with anything else?
> 
> > > For USB, we generally do not allow these types of crazy apis to be added
> > > to the kernel when controlling the device can be done from userspace.  I
> > > think the same thing can happen here too, right?
> > 
> > USB devices benefit from standardized descriptors and interfaces, which reduce the need for custom
> > sysfs APIs. In contrast, LP5812 has no such standard interface, and some customization is necessary.
> 
> Many USB devices do not benifit from that at all, you directly control
> them from userspace using vendor-specific apis.  Just like this device,
> nothing different just because it is an i2c device.
> 
> > I’m open to improving the sysfs interface or moving parts to another method if that’s more appropriate.
> > Please let me know which specific changes you’d recommend.
> 
> sysfs really doesn't seem to be the correct api here, you are making a
> custom one just for this one device that is not shared by any other one,
> so userspace has to write custom code to control it.  So why not just
> write one program, in userspace, to handle it all at once, instead of 2?
> 
> > For completeness, I considered these methods:
> > - sysfs: Recommended and standard for LED drivers.
> > - i2c-tools: Not recommended, intended for development/debug only.
> > - ioctl: Not recommended for new LED drivers.
> > - debugfs: For debugging only.
> > - Direct I2C register access: Requires users to know the register map and protocol.
> 
> A library will handle the i2c direct register access.  Again, do not
> make custom sysfs apis if at all possible.

Thank you very much for your valuable feedback.

I understand your suggestions and the overall strategy. I'm currently considering moving
some configurations to the device tree binding, allowing users to manage device settings
more flexibly through it.

For other interfaces, I plan to support them from userspace.
 
If this approach sounds good to you, I'll proceed to update the source code and submit a
new patch accordingly.

Thanks again for your review and support!

Best regards,
Nam Tran