Adopted Jernej's changes to switch LED definitions on Orange Pis

[ThomasKaiser]

No description provided.

[markh-de]

Yes, for individual changes the fexc is the right pointer, and I have of course been using it for testing the LEDs' polarities.
However, I just wanted to point out that changing the Green LED's polarity to Low-Active when it's really wired High-Active on the PCB feels like a hack to me. Because it prevents users from usefully setting up triggers like "heartbeat" or "mmc0". The "timer" is not such a big issue, as you can swap "on" and "off" delays. But in general, setting the LED to the "wrong" polarity renders most triggers useless and forces users to adapt their userland tools to set inverted values.
To me it would seem cleaner to set the polarities just as the manufacturer defined them, so that the LED class driver work as expected and instead let the kernel set the corresponding GPIO to active (High) asap during boot-up.
Aren't there any other facilities that can turn the Green LED on (GPIO High) when booting? Does section "[boot_init_gpio]" help? Is trigger value "default-on" assignable in fex? Can u-boot do it? Just some ideas.
Thanks for your time!

[ThomasKaiser]

I don't get what you want. You want me to write something different into a fex file? What do you expect that happens then: https://github.com/O-Computers/linux-sunxi/blob/h3/drivers/leds/leds-sunxi.c#L77

It would be better if you come up with a working example (both code and explanation what has changed)

[markh-de]

Sorry for possibly having bothered you, Thomas. Feel free to ignore the topic, and enjoy your vacation.

My question has been answered: The change was done on purpose for having the green LED act as Power indicator. And it does not only affect the Orange Pi PC, but is an intended functional change for multiple boards. So there is no reason to change anything.

In my previous post I just wanted to point out that inverting the LED polarity might break (at least affect) some use-cases of the LED triggers (/sys/class/leds/...), e.g. "mmc0" trigger. Therefore I suggested to turn on the LED not by inverting its polarity, but by using other means to set its GPIO pin early during boot-up. So that the user does not have to deal with the inverted logic if he wants to use the LED for other purposes later on during runtime.

Most users won't care about the LEDs, so this might not seem like an issue, I admit. Just recently I discovered the LED triggers and use them now as the primary system status feedback (PPP and SSH tunnel Go/NoGo signalling on red, mmc0 status on green) for my offsite server.

OK, no problem. I will just adapt everything to my personal preferences.
Thanks!

[ThomasKaiser]

Sorry for possibly having bothered you

Sorry, I still don't get what you want. We just exchanged red and green, the triggers remained the same:

[none] mmc0 mmc1 timer heartbeat backlight default-on

What's your proposal? Which change where should bring which benefit? I just checked it (built extra an image from scratch) and I can set every of the above triggers from user space regardless of the defaults we use/changed. Or is the problem that there aren't that much triggers available as with sun7i (A20)?

[markh-de]

Yes, of course you can use them, but the polarity differs from the user's expectation. The user might expect to have the "natural" polarity. That is, he wants the LED to go off if he sets the brightness setting to 0 (just one example) and vice versa.

Now, what happens to the green LED when you set its trigger to "default-on"? It switches off, right? That's what I mean. It's not what the user expects. And it renders most triggers useless as they are supposed to let the LED light up when something interesting is happening, not to go out.

My suggestion was simply to switch the LED on during boot (e.g., by assigning "default-on" as the default trigger), but leave the active level at its natural position (as wired on the PCB). I see that this is not possible with the current implementation of leds-sunxi.c, unfortunately. It seems the Cubieboard has a more advanced version of that driver, but I doubt that I find the time to have a closer look at it.

[ThomasKaiser]

Thx for being more precise. Not I get what's wrong. Only applies to the green led where brightness and none|default-on have reversed meaning. The red one is not affected.

Since all we did is just change the role of both leds in script.bin I would suspect that's a driver issue. Well let's try it out:

[ o.k. ] ... 0015-sunxi-leds.patch [ succeeded ]

Nope, breaks everything :)

[markh-de]

What do you mean by driver issue? I don't see any. Everything works as designed. You've changed the definition of the active state for the green LED, therefore it always behaves contrary to the commanded state.
Pardon if I over-explain it (I don't know if you are a hardware guy), but if you configure the LED GPIO pin to active-low, then it sinks the current (pin low) in active state (LED meant to be on), while for active-high it sources current (pin high) in active state. The setting in the fex file defines how each GPIO/LED is wired on the board. In case of Orange Pi PC, both are wired active-high.
Should we try to extend the leds-sunxi.c driver so that we can assign a default trigger via fex file and leave the active state as it is really wired on the board?

[ThomasKaiser]

I think I understand now. Changed the definition (have a look in the middle of this commit) and tried to solve the problem that the green led now would be active even after a shutdown with redefining it to heartbeat in armhwinfo's stop case.

[markh-de]

Thanks for your work! Haven't applied it yet, but the changeset seems very promising. Didn't know that it's that easy to set the default level (I'm talking about the <1> instead of <0> in the fex). Else I would have suggested it right from the start.
Again, thanks for making that great little piece of hardware so much fun to use!