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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <20250510074802.8400-1-trannamatk@gmail.com>
Date: Sat, 10 May 2025 14:48:02 +0700
From: Nam Tran <trannamatk@...il.com>
To: lee@...nel.org
Cc: andy@...nel.org,
	geert@...ux-m68k.org,
	pavel@....cz,
	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 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@...il.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@...il.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