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@xxxxxxxxx> 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@xxxxxxxxx> 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