Jump to content

Recommended Posts

Posted

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

<
Posted

@Jean-Francois Lessard: First of all, thanks for your very nice tm16xx driver. I'm using it with my rather new TV box H96 Max V56. I finally managed to get all leds light up during boot. I don't use an overlay like in your source code but an amended dts file. So far so good. Now a couple of questions arose which I hope you can answer

  1. I noticed that the heartbeat led starts flashing way earlier than the lights of the blue front display. Is there a way to get this tm16xx driver starting up much earlier in the boot process?
  2. The display is rather dull. Is that normal? cat /sys/class/leds/display/brightness says 8
    May be we can change the dts file a bit so that the leds get more amps, is that possible?
  3. Kernel messages have the following lines which bother me. Do they point to a problem later on?
    i2c i2c-6: Not I2C compliant: can't read SDA
    Jan 31 17:42:03 h96-tvbox-3566 kernel: i2c i2c-6: Not I2C compliant: can't read SCL
    Jan 31 17:42:03 h96-tvbox-3566 kernel: i2c i2c-6: Bus may be unreliable
  4. The most annoying problem though is that I can't do
    didi@h96-tvbox-3566:~$ sudo echo 1 >/sys/class/leds/display::wlan/link
    bash: /sys/class/leds/display::wlan/link: Keine Berechtigung
    which means even as root I don't have permission to change an led from off to on (this is merely an example. All single leds and the 7-segments can't be switched on or off) What might cause this problem?
  5. Finally the topmost segment is on when in fact the bottommost segment is supposed to. So there is a mapping problem. By the way I took the overlay dts from your repo. Maybe I can tweak the mapping but I don't know how.
  6. Oh, there comes another one to my mind: Do we need pinctrl in dts or will it cause troubles down the road?

Your help is highly appreciated! All the best and

 

Cheers,

Deetsh

 

Posted (edited)

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

Edited by Jean-Francois Lessard
Posted

@Jean-Francois Lessard
Thanks for your comprehensive answers. They helped me a lot, but as ill luck would have it, new questions arose ...

  1. As far as I can tell, the driver is loaded at second 2.7, which is perfectly early enough for me. Once the driver is ready to operate 8888 should be visible. Well, should .. I managed to get it running once and then I changed the config or the board dts or whatever and I never reached this state again, what a sh*t! May be you can spot any error in the dts, but I have the feeling that the error lurks somewhere else.
    You are so right, one can comment accurately on my questions if there is no dts you can look at. This time I attach my full dts to this post, promised.
    BTW, like I said before, I have an H96 Max V56, which has an FD6551 on the board. So there is no confusion.
  2. Rest assured, brightness is behaving as expected but is waaay to dark for me. If only those leds had white color ...
    Well I have made the led display "brighter" in a rather unconventional way -> I used a saw and sawed out a little rectangle from the front. Maybe my tv box doesn't look as pleasing as before, but the led display is much brighter now. Form follows function , ya know?! 🤣
  3. Since the led display did work once, I think both SDA and SCL are configured correctly. i2c-6 is, in fact, the i2c node the kernel assigned to the tm16xx driver.
    When my tv box starts, somewhere in the kernel log I get
    1. i2c-gpio display-client: using lines 12 (SDA) and 11 (SCL)
    2. ...
    3. tm16xx 6-0024: Failed to set brightness: -6

    4. tm16xx 6-0024: Failed to initialize display: -6   (-6 is ENXIO -> no device)
      The driver simply is unable to talk to the chip and I really don't know why. BTW my kernel has version 6.12.12 and the tm16xx is not built in but is a .ko

  4. Understood, I tried them and they worked, but thanks for the explanation (I found them earlier on your Github pages.)

  5. Yep, later. Led display isn't working right now ... 🙂

  6. I have the feeling that using pinctrl would necessitate a "slight" rewrite of your driver and at the moment my inclination to go down this rabbit hole is, well, limited...
    But the question is,if there is a pinctrl definition regarding these two GPIO lines, will this definition interfere with the definition of the gpio lines in the display-client section?
    When using pinctrl I couldn't find any way to set the baud rate used for the communication. Is it unnecessary or is there some sort of auto-negotiation?

So if you have any idea how I can arrive at a working led display again, please(!) let me know. Currently I'm at my wits end.
All the best,
Deetsh

 

P.S.: oh, there is one thing I need to ask you: Do you know a way to make testing new versions of the dts easier than building an image and downloading in the emmc?

myboard.dts

Posted (edited)

@Jean-Francois Lessard
You won't believe it, but I found my error(s) ....
For the solution I had to look at the dts Paolo (https://github.com/paolosabatino/leds-fd6551) suggested in his code. I missed a line that effectively makes SDA really open-drain. However, pinctrl is not relevant in this context.

The fist line is bloody important, the second not so:
 

i2c-gpio,sda-output-only;
i2c-gpio,scl-output-only;

Although they do incur warnings in the kernel log, they are indispensable because without them, i2c signals don't reach the fd6551 controller.

 

Now everything works as expected. It shows (in fact, it showed ...) 88:88 right at the start and after that the actual time. Great!

I'm going to write a pull request for Nicolas' Github repo so that he can integrate it with the Armbian distribution.

I also edited your tm16xx source code to show 'boot' instead of 88:88, which is not my cup of tea.

You also have to make sure you compile the driver als built-in and not as a kernel object. So you get the 'boot' writing roughly around second 2.8 in the boot process when plugging in the mains adapter and this is early enough for me.
I attach all of the sources to this post. I hope you don't mind too much that I messed with your code and I apologize upfront.

 

Best wishes,
Deetsh

tm16xx_boot.c display-client-snippet.dts

Edited by dfahren
Amendments
Posted

@jock

Yes indeed, the driver written bei Jean-Francois is already integrated in Armbian by your patch, I know. Yes it works with the correct dts all is good. I struggled quite a bit until I arrived at the solution outlined in my last posting and I also wanted to have the writing 'boot' to show up right at the get-go in the LED line display. So I had to amend his driver a bit. 
One additional thing I could do to the driver is making the writing configurable in the dts, so that you only have to edit the text to show some other writing once the tv box boots up. This might be pretty easy, now I'm looking for some spare time ....

Posted

@dfahren Thanks for your efforts and reports on getting the tm16xx driver working on the H96 Max V56 TV box.  I'm looking forward to your pull request for the Armbian build, but also wondering if you have step-by-step instructions for integrating the correct DTS, since the tm16xx-display README does not address boxes that are not in the Device Table.  On my 4GB box I'm running the kernel `6.12.11-current-rockchip64` and `modprobe tm16xx` does load the driver but without knowing how to work with this specific box.

Posted
On 10/22/2024 at 5:10 AM, L Jumadi said:

in /etc/modprobe.d/tm16xx.conf :

 

install tm16xx /sbin/modprobe -q ledtrig_timer; \
                        /sbin/modprobe -q ledtrig_netdev; \
                        /sbin/modprobe -i tm16xx; \
                        echo boot > /sys/class/leds/display/value

 

Now it display boot immediately

 

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

Posted

@Jean-Francois Lessard

Yes, yes, you are so right, one can write this tm16xx.conf file as you state and 'boot' or whatever text will show in the line display. All well and good. My point, however, is that if you configured a text like this, you would see it much later than the red LEDs start to blink with heartbeat rate, but at that point in time the boot process is ongoing for quite a while (usually 2 to 3 seconds). I want to display 'boot' right at the start of the cpu and that means once the driver (which must be built-in, not a .ko) initializes has to set up the LEDs to display 'boot', which it does in my code.

 

But I think you wanted to put forward a different point ... maybe ... You don't want your code edited by some random guy from the Internet. It is already merged in Armbian code (by Paolo), seems to work and there is from your point of view no need to amend it, right? Let me check this out: Maybe I can override the function with my code in a different file (perhaps like in C++) and activate it with a CONFIG switch. I think this should be fine with you, right? The second point I mentioned earlier was, that I can make this writing "boot" configurable in the dts. Maybe I also pursue this idea, don't know right now. Everything is a question of time and inclination, right?

 

@watou

Your kernel is fine, your version is my version. Could be that your dts doesn't contain necessary definitions pertaining to the LED line display. Please check my dts file attached to the post above.

When you issue "modprobe -a tm16xx", please post the last lines of "sudo dmesg" output, then we can see what goes wrong.

Posted (edited)

@dfahren

Thanks for your reply. I believe my issue is that I cannot locate the steps to integrate your DTS snippet above into my system.  The tm16xx driver does load but generates no lines in dmesg, probably because it's not referenced in the device tree? Do you have the steps to add what is needed into my device tree?  Many thanks!

 

Edited by watou
proper reference
Posted
1 hour ago, dfahren said:

You don't want your code edited by some random guy from the Internet. It is already merged in Armbian code (by Paolo), seems to work and there is from your point of view no need to amend it, right?

I would say, and I bet @Jean-Francois Lessard agrees, that is not a matter of "messing with my code" thing, but rather a matter of principles.

A kernel module is supposed to do a kernel module or, In simple words, it interfaces the kernel with the hardware. The kernel means are to give the userspace a way to access the hardware.

The module should not run any user business (eg: display something user-specific at boot/shutdown/whatever), but should in turn provide a service for the userspace to control the hardware.

That's a simple principle in software engineering that's called "separation of responsibility", and is generally considered a good habit to agree with that principle.

 

By the way, if your board has one or more colored leds, they are generally set up in the device tree and access basic GPIO system, so they are turned on as soon as the kernel starts.

Posted

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

Posted (edited)

@jock

Zitat

The module should not run any user business (eg: display something user-specific at boot/shutdown/whatever), but should in turn provide a service for the userspace to control the hardware.

But the original tm16xx driver displays something! In fact all LEDs and all 8's are on. Do you consider this pheonomenon already a "user business". If so, why and if not, why not? Please explain.

Moreover, I have the impression our discussion might go "philosophical" and I also fear different people understand different things when they use "separation". I think you both refer to SOLID, which can be read up on here: https://en.wikipedia.org/wiki/SOLID

Unfortunately C is not C# or C++ so our possibilities are limited.

 

I'd definitively go the route Jean-Francois suggested with the tm16xx.conf file, but whatever is displayed in the end, such a config file is processed too late. Unfortunately. What can I do?

All the tv boxes I know display 'boot' or some "dancing segments" or something crazy like that. I'd rather have a simple text being displayed right at the start of the box. Nothing more nothing less. 

 

I think we agree that the functionality to display something on a line display is one thing and the "what" is the other (aka configuration). So it makes sense to delegate the "what" to a different file, for example, a suggestion I outlined above.

To cut this story short I try, like I mentioned at the outset, to create some sort of a "c overlay" activated by a CONFIG switch to enable the "boot" display, I think this the route with the least complaints, right?

Edited by dfahren
Posted

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.

 

Posted
vor 31 Minuten schrieb Jean-Francois Lessard:

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.

This also sounds like a good way if this sort of parameter can be read right at the start of the boot process. :thumbup:

Posted

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

Posted
4 hours ago, dfahren said:

Moreover, I have the impression our discussion might go "philosophical" and I also fear different people understand different things when they use "separation". I think you both refer to SOLID, which can be read up on here: https://en.wikipedia.org/wiki/SOLID

Unfortunately C is not C# or C++ so our possibilities are limited.

Well, not really. SOLID does not apply just to object oriented languages like C# or C++. They are principles, so by definition they always apply. Also note that the linux kernel is written in C (a non object oriented language) but most of the Linux kernel is indeed object oriented. It's a common mistake to think that OO software can be written with OO languages, but it is absolutely not true. OO languages makes easier and safer to handle OO principles, but you can apply the same OO principles even with C language.

 

Getting back from philosophycal, I also agree that a module parameter is the right choice to fulfill what you need and keep the separation of concerns. Perhaps you may also go with an udev rule.

I was wondering though if you want the led panel to show the string at the very power up of the board because you mentioned the stock firmware. In that case (the stock firmware) the boot loader (u-boot) is turning on the led panel chip and instructing it to show the boot string (via a driver, or even just tinkering directly with the I²C bus). Later the kernel boots and loads its own driver, and then changes the boot string with the clock or with the moving segments or whatever...

Posted (edited)

@Jean-Francois Lessard

Thanks for your revised version of your tm16xx driver. I downloaded it from your github repo and installed it in Armbian via a git patch. 

However, getting the configuration parameter into the driver was (at least for me) quite complicated and took me hours to resolve. But here comes what I found out:

  1. You proposed to configure the module parameter in extlinux.conf. Well, there is no such file in the Armbian kernel, or at least I could not find it.
  2. If that doesn't work, why not setting up a dedicated tm16xx.conf file in /etc/modprobe.d . I did that, but it has no effect. Once my Tv box boots up, it shows all 8s and all LEDs being on. May be this option works if you use insmod to install the driver, something I don't want to because I want the "boot" writing right at the start.
  3. I consulted the documentation of Armbian and found some sections about "armbianEnv.txt". A file in which you can define parameters being passed to the kernel and its modules. That sounds at least promising to me, so I tried to let the Armbian build system set it up for my TV box. Unfortunately, getting such a file accepted by the Armbian build system, which, to say the least, in its complexity IS NOT dumb man's friend .... is harder than one might think. Finally I arrived at a solution and what can I say, "boot" is displayed in the LED line dispaly at the same time the other LEDs of the board also light up. So this solution works.
    I attach the parameter file to this post. It has to go in the folder "userpatches/bootenv/rk35xx.txt" of the Armbian distribution.

So IMHO what remains to do is to amend the patch @jock has published for the current 6.12. Armbian distribution to include the new version of the tm16xx driver from you, Jean-Francois.

 

I thank both of you for being of great help to me. This H96MAXV56 gets better and better and may be one day all features of this TV box run smoothly satisfying everybody.

rk35xx.txt

Edited by dfahren
Posted (edited)
1 hour ago, dfahren said:

owever, getting the configuration parameter into the driver was (at least for me) quite complicated and took me hours to resolve. But here comes what I found out:

  1. You proposed to configure the module parameter in extlinux.conf. Well, there is no such file in the Armbian kernel, or at least I could not find it.

 

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.

Edited by Jean-Francois Lessard
Posted
Zitat

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.

Yes of course, there is such a file '/boot/armbianEnv.txt', but it is generated on the fly by the Armbian build system and if you don't know this build system in and out, you are pretty easily lost ...

You have to set specific environment variables and put the file mentioned in my post in a specific folder and then a valid and correct armbianEnv.txt will be generated by the build system.

 

I'm thinking about how I manage to get this in the standard Armbian build. I don't want to mess with the complex build process only to do something that is very specific for my TV box.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

Loading...
×
×
  • Create New...

Important Information

Terms of Use - Privacy Policy - Guidelines