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

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

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

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!

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

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

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!

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

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!

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!

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

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

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

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

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 [李琼斯]

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

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

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 [李琼斯]

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

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

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

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

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 [李琼斯]

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

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 [李琼斯]