Message-ID: <CAHp75VcVmTwS-zw=o5=m1-x0XC67BKBVWae2mMKZQH=qLCxZwg@mail.gmail.com>
Date: Tue, 29 Apr 2025 22:47:22 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Nam Tran <trannamatk@...il.com>
Cc: pavel@....cz, andy@...nel.org, geert@...ux-m68k.org, robh@...nel.org, 
	krzk+dt@...nel.org, conor+dt@...nel.org, christophe.jaillet@...adoo.fr, 
	corbet@....net, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-doc@...r.kernel.org, florian.fainelli@...adcom.com, 
	bcm-kernel-feedback-list@...adcom.com, linux-rpi-kernel@...ts.infradead.org, 
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v8 0/5] auxdisplay: add support for TI LP5812 4x3 Matrix
 LED driver

On Tue, Apr 29, 2025 at 8:02 PM Nam Tran <trannamatk@...il.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

Powered by blists - more mailing lists