Jean-Francois Lessard

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

@L Jumadi Great news, I am glad the 8th segment icon works! Thanks for your feedback. I've reviewed the Makefile and I have removed an unused BUILD_DIR variable that was deleted in the clean target. I guess that your system somehow defined this variable to your build folder and that it caused the problem you were experiencing with the headers. Let's get into the display service since we now have a confirmed working driver. As you saw, the display service is simply a bash script. It relies on existing other kernel features to manages the icons. The line causing the problem ``` echo timer > /sys/class/leds/display::colon/trigger ``` is configuring the basic timer trigger to make the colon blink every half second (more details in Kernel documentation). My guess is that the kernel module providing this timer trigger has not loaded properly (maybe because of an issue you have with `make service-install`). Here are the commands that you can launch manually: ``` # load the modules for the current session modprobe -a ledtrig_timer ledtrig_netdev tm16xx # save the dependencies on other modules for next boots echo "softdep tm16xx pre: ledtrig_timer ledtrig_netdev" > /etc/modprobe.d/tm16xx.conf ```

Your kernel header configuration didn't had the right release version when the driver was built. Maybe the driver was built successfully prior of reinstalling the kernel headers. 1. Make sure your kernel headers configuration are restored ``` cat /lib/modules/$(uname -r)/build/include/config/kernel.release ``` Make sure it shows 6.7.12-edge-sunxi64 If not, you should reinstall the kernel headers. 2. Rebuild the tm16xx driver First make sure to clean the build so it will recompile the driver. Otherwise make may not detect that your kernel header configuration has changed since last build. In your tm16xx source code directory: ``` make clean ``` Then you can build the driver. Let's proceed step by step instead of using `make install`. ``` make module ``` then install the driver ``` sudo make module-install ``` and probe the driver ``` sudo rmmod tm16xx sudo modprobe tm16xx ``` 3. While not related to building the driver, you should be able install the service without permission denied error by using this command : ``` sudo make service-install ```

@L Jumadi I've had a look at some pictures of the Tanix TX6 and it doesn't look to have icons such as other displays found on amlogic and rockchip TV boxes. I have also reviewed the OpenVFD source code and I have found that it hardcoded a special case for the fd650 controller which uses an 8th segment as the time separator icon. So, I have updated the tm16xx driver's code to enable the 8th segments on fd650 controller to be used as an icon. You can pull the latest version from GitHub and rebuild / reinstall the driver. Your device tree should now be updated like this: ``` /dts-v1/; /plugin/; #include <dt-bindings/gpio/gpio.h> #include <dt-bindings/leds/common.h> &{/} { display-client { compatible = "i2c-gpio"; sda-gpios = <&pio 8 12 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>; /* PI12 */ scl-gpios = <&pio 8 11 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>; /* PI11 */ i2c-gpio,delay-us = <5>; #address-cells = <1>; #size-cells = <0>; display-controller@24 { compatible = "fdhisi,fd650"; reg = <0x24>; tm16xx,digits = [00 01 02 03]; tm16xx,segment-mapping = [00 01 02 03 04 05 06]; #address-cells = <2>; #size-cells = <0>; led@1,7 { reg = <1 7>; function = "colon"; }; }; }; }; ``` Note that I am not totally sure about which digit the time separator uses. Since your openvfd config is `openvfd_chars = [02 01 02 03 04];` and the first index (02) is for the icons, it should be digit 2, but it seems that OpenVFD somehow adds +1 to fd650 grid indexes. So the above device tree uses 1 as the colon grid index but you may have to test with different values between 0 and 3. Thanks for your time in testing my driver and allowing me to make it better!

@L Jumadi there is something weird about this open vfd configuration `openvfd_chars = [02 01 02 03 04];` and the resulting conversion in device tree for my driver. Grid index 02 is used twice. LEDs should not use the same grid index as the digits. There might be some special handling of this case in the OpenVFD driver (to be confirmed by reviewing OpenVFD code). The controller typically use an array of 5 grids (indexed from 0 to 4) where each grid has 7 segments. There are 4 grids used for the digits, where the segments are turned on or off to represent a specific number. For example, the number 8 has all segments on and the number 1 has only the 2 segments on (the ones on the right). The display assembly then uses the remaining grid to control the various icons (the LEDs) by mapping one segment to each of these icons. In your device tree `tm16xx,digits = [01 02 03 04];` maps the digits to grid indexes 1, 2, 3, 4 and LEDs also uses the grid index 2 for the LEDs: ``` led@2,[SEGMENT] { reg = <2 [SEGMENT]>; /* ... */ }; ``` I would start by commenting out the LEDs configuration in the device tree and then confirming which grid indexes are used for the digits by following Test alternative digits ordering. For example, you could try grid indexes "0 1 2 3" and check if it still starts from the 2nd digit. Once the correct grid indexes for the digits are found, you can update the `tm16xx,digits` accordingly, uncomment the LEDs configuration and change the LED grid index to the one that is not used in the digits. For example, if using digit indexes "0 1 2 3", the led grid index should be 4. Or if digits indexes are "1 2 3 4", led grid index should be 0. Edit: Looking at fd650 controller datasheet, it supports only 4 grids, so they will be indexed from 0 to 3. There is no need to test digit indexes over 3. It would also mean that your display has no LED icons as all grids are used for the digits. You can then completely remove the LEDs definition in the device tree. If your display has icons, its likely that you need to configure a different controller by changing the compatible string in the device tree. Both "fdhisi,fd655" and "fdhisi,fd6551" are pretty similar to fd650 but supports a 5th grid (so grid indexes goes from 0 to 4). The main difference between fd655 and fd6551 is the number of brightness levels supported (3 for fd655 and 8 for fd6551).

@L Jumadi can you share your DTS config for the display controller and LEDs?

@L Jumadi that is great news! It means driver has initialized the display device properly. Displaying 8888 is expected when device gets probed. It's the display service that will then set the time and LEDs on the display from user space. But before you start the service, you can check that segment and digit mappings are well configured by running `display-utils -c`. Please read Check your display configuration of README.md to know how to check and adjust these mappings. Once mappings are good, you can start the service by using `systemctl start display`.

@L Jumadi It looks like you should reverse the pin configuration of the data and clock pins Your openvfd config: ``` openvfd_gpio_clk = <&pio 8 11 0>; // this is the clock ping openvfd_gpio_dat = <&pio 8 12 0>; // this is the data pin ``` Should become: ``` sda-gpios = <&pio 8 12 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>; // this is the data pin scl-gpios = <&pio 8 11 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>; // this is the clock pin ``` You can also try setting gpio flags to 0 such as in your openvfd config. It would become: ``` sda-gpios = <&pio 8 12 0>; // this is the data pin scl-gpios = <&pio 8 11 0>; // this is the clock pin ``` Edit: I have added a check in tm16xx.c to report "Failed to initialize display" instead of "Display initialized successfully" when it fails to set brightness or to write display data. This won't fix your issue per se but at least it will add clarity when looking at dmesg.