On 26/05/17 09:02, Haojian Zhuang wrote:
On 2017/5/26 15:10, Amit Kucheria wrote:
On Mon, May 22, 2017 at 7:15 PM, Haojian Zhuang haojian.zhuang@linaro.org wrote:
On 2017/5/22 21:31, Amit Kucheria wrote:
On Mon, May 22, 2017 at 5:07 PM, Daniel Thompson daniel.thompson@linaro.org wrote:
I don't suggest to blink in bootloader. Most of LEDs are connected to GPIO pins, not PWM pins. Blinking is hard to implement especially for panic.
Yeah, sorry, the bootloader was a copy paste error across all events.
We don't really need PWM, do we? Yes, the CPU will get involved in blinking a LED and it won't be accurately periodic but this is about providing yet another indicator to the developer/user, it can always be disabled.
If interrupt is supported in bootloader, it's easy to implement. But I'm afraid that a lot of proprietary bootloaders can't support interrupt handler. So only using the ON/OFF state is easier accepted in bootloader.
Agree. We end up having to make fairly big hacks to the polling loop of non-interrupt supporting bootloaders to support blinking.
Adding fire-and-forget pokes to specific events is a much tidier change.
Specifically, the LEDS_TRIGGER_PANIC Kconfig option allows any LED to be automatically be configured as a panic indicator. Similarly LEDS_TRIGGER_HEARTBEAT and LEDS_TRIGGER_DISK should give us activity indications. drivers/leds/trigger is a treasure trove for anybody that wants to tinker with this. However, it doesn't seem like there is currently a way to attach more than one trigger to each led.
So here's a revised strawman proposal. We could document it in a wikipage after we're happy with it, (B=blink, DC=don't care):
USR1 USR2 USR3 USR4 Event
OFF ON OFF ON bootloader panic ON OFF OFF OFF Entry into bootloader
I think that only these two above rows are for bootloader. Do we need to indicate whether it's in fastboot mode or not?
I think we should (not least because that was the user request that kicked off this thread).
However... I think we might want to define the event carefully. Probably better to attach the event to detecting that a fastboot button or switch is engaged (both u-boot and UEFI allow fastboot to be entered by UART command... it could be optional whether light comes on or not when entering by UART command). This is also motivated by minimising changes needed to non-platform code.
Daniel.
-- OFF OFF OFF B kernel panic B DC DC DC heartbeat (1Hz in kernel) DC B DC DC onboard storage i/o (1Hz, kernel) DC DC B DC SD card i/o (1Hz, kernel)
In fact, on the DB410c, USER1 is already set to heartbeat and I could get USER2 and USER3 to blink on mmc0 and mmc1 activity after setting the appropriate triggers. The PANIC trigger is a relatively new trigger (merged April 28), so once that is turned on, the last four patterns should just work on the DB410c.
Regards, Amit