Jean-Francois Lessard

I should have specified that it's how to configure kernel command line parameters with the amlogic devices I own. The boot process is slightly different for these devices. I'm glad you found a way to make it work with your device anyway. With amlogic, there is also a '/boot/armbianEnv.txt' file that can be edited but it doesn't pickup command line arguments from there (as opposed to other Armbian versions). Don't you have such a file with your Armbian version? It may be an acceptable solution to edit the file after installation.

@paradigman it looks like an issue with your kernel build system, not the display driver. The kernel build system is either packaged with the kernel source code or the kernel headers of your very specific kernel version. Since the compiled module is once again specific to your kernel version, sharing compiled objects won't be of any help. Try to fix your kernel build system first.

@dfahren you can pull latest version from https://github.com/jefflessard/tm16xx-display (this is commit 47df76a). It now has the "default_value" parameter. You can test directly by setting the parameter value at modprobe. To have it automatically configured at boot time there are 2 options: 1. Using Linux kernel boot arguments by adding the parameter to /boot/extlinux/extlinux.conf. Something like adding "tm16xx.default_value=boot" to your "append" directive. 2. By configuring modprobe.d options of /etc/modprobe.d/tm16xx.conf with the following line: options tm16xx default_value="boot" Please test and reports your results.

Turning on all the LEDs (all 8s) should not be considered as user space content but rather a diagnostic mechanism to ensure the device is working properly (which do respect separation of concerns). Now, I do understand that you want to display some content as soon as possible and that relaying on user space mechanism would happen to late in the process, especially if you are targeting "in tree" kernel module. The appropriate approach would then be to have a kernel module parameter that would accept the default value to display. This default value could be configured using either kernel boot command line arguments or modprobe configuration file to "boot" or whatever the user prefers. Then, when the kernel module probes the device, it would initialize it it's value to default value instead of turning on all LEDs. I'll implement this when I will have some free time.

@jock perfectly explained my point! Thank you. I would add that the idea of open source is to allow collaboration among developers. So it's perfectly fine if you @dfahren want to change the code. Let's say you add a kernel module parameter that can be configured in the Linux kernel boot parameters to display some specific content, I would then be inclined to merge your pull request. Now, if you would prefer to have the content embedded in the code, I would rather suggest you to keep your code in a fork as it doesn't follow the separation of concerns.

@dfahren you can configure modprobe like the above to display "boot" without modifying the source code. You could even replace "echo boot" with "display-utils -t whatever" to scroll any text you would like. I really think this is a much better way to do so. It prevents having the driver module to manage content instead of the device. The idea of turning on all led at module probe is to ease diagnostic of module configuration, much like a car console that will turn on all subsystems LEDs when turning the ignition key. Personally, I don't understand why so many people want to display "boot" but it's up to you to customize the display the way you want.

@dfahren thanks for your feedback. I'll try to answer your questions but they are rather broad and not strictly related to the driver itself. Plus, it's difficult to have a clear understanding without knowing your DTS configuration. 1. The display device is usable as soon as the tm16xx device driver is loaded by the kernel during boot (which will turn on all display LEDs at this stage, showing 88:88). There is no way to load the driver before that. Some other LEDs might be hardware controlled or configured by u-boot preloader, before the kernel is loaded, so you can't compare against these. Currently display.service loads the driver using ExecStartPre at basic.target, which is somewhat the earliest it can using systemd. There might be other ways to load the driver a couple of milliseconds before but I don't think it worths investigating. Anyway, the display-service won't show the correct local time before network and ntp will have loaded properly (local time is only available from user space, not within kernel space). 2. Display brightness is set by using controller specific values, there is no amps to control. You can try setting different `/sys/class/leds/display/brightness` values to see if there's a bug (0 is off and `cat /sys/class/leds/display/max_brightness` being the max value for your configured controller). If brightness is not behaving as expected, it is likely that you have not configured the right controller in your `compatible` string of the display-controller node in your DTS. You can check the controller model by looking at the controller chip on your device main board. 3. The display would not work with incorrect I2C SDA and SDL pins configuration. I guess that your i2c-6 is another unrelated device in your DTS. You can check which i2c device is used by your display controller using `ls -l /sys/class/leds/display/device` 4. "link" sysfs is not to control a led status but rather to configure some specific led trigger. You need to check specific led trigger module documentation to understand how to use these sysfs attributes. If you want to turn on or off a led without using a led trigger, you can do so: ``` # turn on a specific led/symbol echo 1 > /sys/class/leds/display\:\:lan/brightness # turn off a specific led/symbol echo 0 > /sys/class/leds/display\:\:lan/brightness ``` Please refer to https://github.com/jefflessard/tm16xx-display/blob/main/README.md for tm16xx specific documentation. 5. Use `display-utils -a` as documented in tm16xx documentation at https://github.com/jefflessard/tm16xx-display/blob/main/README.md then you can update your DTS. 6. If using software i2c (i2c-gpio) you should not require pinctrl. If using hardware i2c, it depends on the specific hardware configuration of your device.

@L Jumadi following your finding on softdep, I've removed the /etc/modprobe.d/tm16xx.conf creation from the service-install and rather modprobe the dependent modules in the display.service as pre exec commands. I think this may be a cleaner solution since there is no real dependencies between the tm16xx module and the ledtrig modules, it's really about the display service which relies on them. Now, your idea of customizing the tm16xx module install to display boot is interesting! It ensure it's executed as soon as the module is loaded instead of waiting for the service to be loaded. On my side, I ended up with: cat /etc/modprobe.d/tm16xx.conf install tm16xx /sbin/modprobe --ignore-install tm16xx && \ /bin/echo boot > /sys/class/leds/display/value I won't include this in the code base, since I prefer that full led on to be default for diagnosis purposes. But it's a nice customization worth to be noted.

@MattWestB did you happen to configure a default led trigger in your display device tree? I think that was the issue. I've updated the tm16xx GitHub repo with a couple of changes: - fixed default trigger init value - fixed /usr/sbin service path - revised error management & verbosity of display service - added prompts to display check Please update to this latest version and confirm if it solves your issues (please tag me in your reply).

@jock I've replied to the other thread. I will review usage of workqueue later, but I think it may be related to something else. Since this is the workqueue that is writing to hardware device (i2c or spi), I guess that the kernel dump warning may appear when the cpu is hogging while accessing the hardware interface, such as i2c/spi interface misconfiguration or having multiple drivers trying to access the same interface (such as running OpenVFD at the same time). To answer your question about async led_set_brightness, the workqueue is used to manage concurrency issues. Using sysfs, you can set the individual led on/off or changing display/value but they are all dumped together on every write to the controller. A workqueue can only be scheduled one at a time (other schedule calls will be ignored if already scheduled), so it will pickup all the changes that happened concurrently once it's executed. Plus I have experienced cases where set brightness coming from ledtrig_timer (and/or ledtrig_netdev) were called from an irq/sortirq context, therefore causing kernel panic if writing directly to the device (ref related commit). Edit: Kernel dump should be solved as well as some other minor improvements: https://forum.armbian.com/topic/26978-csc-armbian-for-rk3318rk3328-tv-box-boards/?do=findComment&comment=204448

@jock and @MattWestB The module probe warning needs to be investigated further. About the display service, it's simply a bash script. It relies on existing other kernel modules to manages the icons: ledtrig_timer and ledtrig_netdev. ledtrig_timer is a basic timer trigger to make the colon blink every half second and is missing from @MattWestB stack trace. If the module is not loaded, the display service will fail on line "echo timer > /sys/class/leds/display::colon/trigger". So make sure the module ledtrig_timer is available and loaded first. "make service-install" adds a modprobe soft dependency on ledtrig_timer and ledtrig_netdev and will load them automatically if present. So you just need to get sure the modules are built and available. For testing the display LEDs and segments, the first thing to check is whether all LEDs and segments are turned on when probing the module (but before starting the display service). The driver purposely turns on everything on the controller can on probing. If the led doesn't turn on, it's a device tree / controller model / hardware problem. Then you can use "display-utils -c" and follow these instructions: Check your display configuration.

cat /sys/class/leds/display::colon/trigger will only show loaded modules. Your distribution should come with a set of additional loadable modules. There is no much option to make a led blink on a constant timing with the options you have right now.

You can try something like this for your the systemd unit file (I have not tested it): ``` [Unit] Description=Set LED display value to "boot" after tm16xx module loads After=systemd-modules-load.service ConditionPathExists=/sys/class/leds/display/value [Service] Type=oneshot ExecStart=/bin/sh -c 'echo "boot" > /sys/class/leds/display/value' [Install] WantedBy=sysinit.target ``` And enable it like this (rename set-boot-display.service to the name of you unit file): ``` sudo systemctl enable set-boot-display.service sudo systemctl start set-boot-display.service ``` Edit: Save this file as `set-boot-display.service` in `/etc/systemd/system/`,

You can check which led trigger modules are already built for your distribution: ``` ls /lib/modules/$(uname -r)/kernel/drivers/leds/trigger/ ``` If you don't have ledtrig-timer but you have ledtrig-transient, that would make it for the colon (see kernel doc about transient led trigger). Otherwise, you can reach out your distribution maintainers to add these modules. In last resort you could modify the display service to update the time every half second and making the colon blinks.

The idea behind 8888 is to turn on all LEDs for quick visual diagnostic when the driver is loaded, which I think is pretty useful. If you want something else to be shown during boot, you can create a simple service unit file and make it execute early in the boot process but just after the driver is loaded. It can then contain a simple command like ``` echo "boot" > /sys/class/leds/display/value ``` Or something fancier with something like: ``` display-utils -t "boot b**t b//t b00t" ``` Or you can even make your own script that animates text!