Jean-Francois Lessard

Hi @Mark Waples, first thanks for your detailed question completeness. It helps so much to understand your context! You need to define the other led entries in your device tree, so the tm16xx driver will know about them and it will create a corresponding sysfs led interface (i.e /sys/class/leds/display::lan/ path and alikes). Once the sysfs interface will be created, the LEDs will be controllable from user space, such as display-service does. Since the AIP650E0 controller is most likely tm1650 or fd650 compatible, it can drive a maximum of 4 grids and my guess is that it uses the 8th segment (index 7) of each grid for the LEDs (segments 0 to 6 being used for clock digits - which you reported to work). There should be no additional grid index involved since probing the module turns on all these LEDs. Try something like: ``` led@0,7 { reg = <0 7>; function = "lan"; } led@1,7 { reg = <1 7>; function = "wlan"; } led@2,7 { reg = <2 7>; function = "colon"; } led@3,7 { reg = < 3 7>; function = "usb"; } ``` First try this led device tree nodes to check if you can control these LEDs from sysfs. Then, you may need to change the grid indexes of these led entries to match your actual icons order. If required, "display-utils -c" may be used at that point to check that led name matches the icon (ex: LAN icon is ON while "LAN" text is shown on the digits). Then you should be good to go! Please report success or failure. Thank you.

@SpellKilleR you should see "Operating in transposed mode" in dmesg, if not it's either that the flag is not correctly set or you are running an older version of the code For the flag, it should be in the display controller node such as in this example: https://github.com/jefflessard/tm16xx-display/blob/ecacbdcf7963c7480bf69d1afe0c5c57bff57d46/devices/x96-max.dtso#L26 To get the latest code version either clone or pull https://github.com/jefflessard/tm16xx-display.git

@SpellKilleR it looks like you may need the transposed flag: https://github.com/jefflessard/tm16xx-display/issues/4

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!

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