hi Daniel,

On Mon, May 22, 2017 at 1:37 PM, Daniel Thompson daniel.thompson@linaro.org wrote:

Hi Folks

I've ended up talking to a few people recently about LEDs.

CE and EE have four user lights which are often hooked up in different ways on each board.

good to bring that up (again). It's been discussed several times in the past, but i don't think there is any good conclusion as of yet...

Are there any common recommendations about default LED hook ups for the kernel (e.g. USR1 -> heartbeat, USR2 -> disk I/O, etc)?

I will try to find the old thread.. but the overall agreement was: LED1 -> heartbeat LED2 -> on board storage i/o LED3 -> SD card i/o LED4 -> free/available

At some point, we talked about adding that as recommendations in the specs, iirc. Maybe Amit will remember more on this topic.

We also discussed about calling the LED with the same name in DTS so that all LED appear at the same place in /sys/class/led for all 96boards which would be good.

Similarly it might be useful to have a couple of events nominated where we recommend a firmware level bootloader[1] change LED status. Some examples:

• Set USR1 on when bootloader starts up (a NOR/eMMC/SD is not empty light).
• Set USR3 on when entering fastboot mode (if supported)
• Set USR3 off when leaving fastboot mode, including via "fastboot boot" (if supported)
• Set USR1 off and USR2, 3 and 4 on if bootloader panics.

This might be a good idea. Might be difficult in the QCOM case, but that's a separate issue, and it shouldn't prevent any discussion..

Especially for CE board (where users often don't have a UART adapter) even the above simple hints that the bootloader has been fetched from somewhere is useful.

I know this can't be a rule, only "good advice" (like asking CE boards to put console on LS-UART1 if possible). However, is there any appetite to converge for the boards we have influence over?

Yes, I think so.

Daniel.

[1] Here I definitely mean EDK2 rather than grub. If we have multiple chained firmware level bootloaders (e.g. LK -> u-boot) things are less clear although, as it happens, if *both* implemented the above proposal the results would still be perfectly useable from a user perspective.

---

Dev mailing list Dev@lists.96boards.org https://lists.96boards.org/mailman/listinfo/dev

On 22/05/17 13:35, Haojian Zhuang wrote:

...

...

Similarly it might be useful to have a couple of events nominated where we recommend a firmware level bootloader[1] change LED status. Some examples:

• Set USR1 on when bootloader starts up (a NOR/eMMC/SD is not empty light).
• Set USR3 on when entering fastboot mode (if supported)
• Set USR3 off when leaving fastboot mode, including via "fastboot boot" (if supported)
• Set USR1 off and USR2, 3 and 4 on if bootloader panics.

At this point I should admit I mus-remembered the spec (the diagram in the spec numbers the lights from 0-3 not from 1-4).

Currently I think Hikey performs as above if you subtrace 1 from all the numbers ;-) .

...

This might be a good idea. Might be difficult in the QCOM case, but that's a separate issue, and it shouldn't prevent any discussion..

I like this idea. And I have a few comments.

If bootloader boots kernel, should USR1 keep on or off?

I don't really see any need for the bootloader to hand over with all LEDs off.

Actually users are often very bad at describing intermediate phases of boot. Thus if we want to attach a change of state to the "I tried to load the kernel" event then I would prefer we turn the second LED on rather than turn the first one off. Thus we get a useful combination of lights if the kernel fails to boot (assuming the kernel has drivers enough to turn the lights off again).

Basically, it's easier to "trust" a user who reports the board is stuck and these LEDs are on because it is unambiguous. Some users so humble that they describe the light on/off sequence in an unconvincing manner (they are worried their problem might be due to a mistake on their part and so somehow convey the impression that they might be watching it wrong)!

Daniel.

On Mon, May 22, 2017 at 5:07 PM, Daniel Thompson daniel.thompson@linaro.org wrote:

Hi Folks

I've ended up talking to a few people recently about LEDs.

CE and EE have four user lights which are often hooked up in different ways on each board.

Are there any common recommendations about default LED hook ups for the kernel (e.g. USR1 -> heartbeat, USR2 -> disk I/O, etc)?

There aren't, AFAIK. But perhaps we can make some progress this time with Mani's help. Homogenising the LED names in DT and sysfs is certainly something that he's working on.

Similarly it might be useful to have a couple of events nominated where we recommend a firmware level bootloader[1] change LED status. Some examples:

• Set USR1 on when bootloader starts up (a NOR/eMMC/SD is not empty light).
• Set USR3 on when entering fastboot mode (if supported)
• Set USR3 off when leaving fastboot mode, including via "fastboot boot" (if supported)
• Set USR1 off and USR2, 3 and 4 on if bootloader panics.

You want to create you own implementation of BIOS codes from the PC world :)

Unless, you make it mandatory though, they won't be reliably implemented across boards. And making them mandatory defeats the purpose of these being user LEDs. So I'm loathe to require this in the specs.

Especially for CE board (where users often don't have a UART adapter) even the above simple hints that the bootloader has been fetched from somewhere is useful.

I know this can't be a rule, only "good advice" (like asking CE boards to put console on LS-UART1 if possible). However, is there any appetite to converge for the boards we have influence over?

IMHO, one could start by implementing a few of these suggestions for a board or two and get that merged into the relevant codebases. If there is enough traction perhaps we might start seeing convergence?

We could have something as elaborate as follows and document it in a wikipage for future boards, if needed (B=blink, DC=don't care):

USR1 USR2 USR3 USR4 Event ------------------------------------------------------------ OFF B OFF B bootloader panic B B B B Transition from bootloader to kernel (blink at 2Hz for 2s?) OFF B B B kernel panic B DC DC DC heartbeat (1Hz, bootloader and kernel) DC B DC DC onboard storage i/o (1Hz, bootloader and kernel) DC DC B DC SD card i/o (1Hz, bootloader and kernel)

Getting even one board to do this might set the trend but I see it as a bunch of small projects for anybody in the community that'd like to get their hands dirty. Perhaps list it in the page the Robert is going to maintain for ideas on starter projects?

Regards, Amit

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:

...

Hi Folks

I've ended up talking to a few people recently about LEDs.

CE and EE have four user lights which are often hooked up in different ways on each board.

Are there any common recommendations about default LED hook ups for the kernel (e.g. USR1 -> heartbeat, USR2 -> disk I/O, etc)?

There aren't, AFAIK. But perhaps we can make some progress this time with Mani's help. Homogenising the LED names in DT and sysfs is certainly something that he's working on.

...

Similarly it might be useful to have a couple of events nominated where we recommend a firmware level bootloader[1] change LED status. Some examples:

• Set USR1 on when bootloader starts up (a NOR/eMMC/SD is not empty light).
• Set USR3 on when entering fastboot mode (if supported)
• Set USR3 off when leaving fastboot mode, including via "fastboot boot" (if supported)
• Set USR1 off and USR2, 3 and 4 on if bootloader panics.

You want to create you own implementation of BIOS codes from the PC world :)

Unless, you make it mandatory though, they won't be reliably implemented across boards. And making them mandatory defeats the purpose of these being user LEDs. So I'm loathe to require this in the specs.

...

Especially for CE board (where users often don't have a UART adapter) even the above simple hints that the bootloader has been fetched from somewhere is useful.

I know this can't be a rule, only "good advice" (like asking CE boards to put console on LS-UART1 if possible). However, is there any appetite to converge for the boards we have influence over?

IMHO, one could start by implementing a few of these suggestions for a board or two and get that merged into the relevant codebases. If there is enough traction perhaps we might start seeing convergence?

We could have something as elaborate as follows and document it in a wikipage for future boards, if needed (B=blink, DC=don't care):

USR1 USR2 USR3 USR4 Event

OFF B OFF B bootloader panic B B B B Transition from bootloader to kernel (blink at 2Hz for 2s?) OFF B B B kernel panic B DC DC DC heartbeat (1Hz, bootloader and kernel) DC B DC DC onboard storage i/o (1Hz, bootloader and kernel) DC DC B DC SD card i/o (1Hz, bootloader and kernel)

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.

Getting even one board to do this might set the trend but I see it as a bunch of small projects for anybody in the community that'd like to get their hands dirty. Perhaps list it in the page the Robert is going to maintain for ideas on starter projects?

Regards, Amit _______________________________________________ Dev mailing list Dev@lists.96boards.org https://lists.96boards.org/mailman/listinfo/dev

On 22/05/17 14:31, Amit Kucheria wrote:

...

Similarly it might be useful to have a couple of events nominated where we recommend a firmware level bootloader[1] change LED status. Some examples:

• Set USR1 on when bootloader starts up (a NOR/eMMC/SD is not empty light).
• Set USR3 on when entering fastboot mode (if supported)
• Set USR3 off when leaving fastboot mode, including via "fastboot boot" (if supported)
• Set USR1 off and USR2, 3 and 4 on if bootloader panics.

You want to create you own implementation of BIOS codes from the PC world :)

Unless, you make it mandatory though, they won't be reliably implemented across boards. And making them mandatory defeats the purpose of these being user LEDs. So I'm loathe to require this in the specs.

Do we currently formally recommend console on LS-UART1 for CE boards anywhere? I really consider this sort of thing as similar (essentially "arbitrary" choices that boards that we recommend as good practice).

...

Especially for CE board (where users often don't have a UART adapter) even the above simple hints that the bootloader has been fetched from somewhere is useful.

I know this can't be a rule, only "good advice" (like asking CE boards to put console on LS-UART1 if possible). However, is there any appetite to converge for the boards we have influence over?

IMHO, one could start by implementing a few of these suggestions for a board or two and get that merged into the relevant codebases. If there is enough traction perhaps we might start seeing convergence?

The list above *is* a description of the current Hikey UEFI behaviour (and I believe Hikey960 will eventually behave similarly) although I cannot tell by observation whether it is bootloader or kernel that causes USR1 to go out.

We could have something as elaborate as follows and document it in a wikipage for future boards, if needed (B=blink, DC=don't care):

USR1 USR2 USR3 USR4 Event

OFF B OFF B bootloader panic B B B B Transition from bootloader to kernel (blink at 2Hz for 2s?) OFF B B B kernel panic B DC DC DC heartbeat (1Hz, bootloader and kernel) DC B DC DC onboard storage i/o (1Hz, bootloader and kernel) DC DC B DC SD card i/o (1Hz, bootloader and kernel)

Bootloaders may not have timers required to blink anything (at least not whilst they are still doing something useful). Until the OS starts we need fire-and-forget actions (on/off/DC).

Getting even one board to do this might set the trend but I see it as a bunch of small projects for anybody in the community that'd like to get their hands dirty. Perhaps list it in the page the Robert is going to maintain for ideas on starter projects?

This was originally a user feature request: https://discuss.96boards.org/t/status-led-for-fastboot-mode-feature-request/...

I escalated here because so many boards suffer from "dark" boot sequences (even after hikey blazed a trail). Ultimately all the *implementers* have UART so unless they are told dark boot sequences are unnerving for users they never realize it.

Daniel.

On Fri, May 26, 2017 at 12:40 PM, Amit Kucheria amit.kucheria@linaro.org 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.

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

I misread the timestamp, it was merged in April last year in v4.6. So it is just a matter of enabling it in a newer kernel when DB410c upgrades to v4.9.

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

On Mon, May 29, 2017 at 12:43 PM, Nicolas Dechesne nicolas.dechesne@linaro.org wrote:

On Fri, May 26, 2017 at 9:14 AM, Amit Kucheria amit.kucheria@linaro.org wrote:

...

I misread the timestamp, it was merged in April last year in v4.6. So it is just a matter of enabling it in a newer kernel when DB410c upgrades to v4.9.

we have upgraded to 4.9 already, can you send a patch to https://lists.96boards.org/mailman/listinfo/dragonboard? thanks!

Here are a couple of patches on top of the LT 4.9 kernel to fix the issue. This sets up USER4 led on the DB410c to blink on a kernel panic.

Pardon the attachments and not sending a pull request but 2-factor authentication has messed up my git-send-email workflow somehow. :-/

On Wed, Jun 28, 2017 at 3:56 PM, Nicolas Dechesne nicolas.dechesne@linaro.org wrote:

On Wed, Jun 28, 2017 at 11:25 AM, Amit Kucheria amit.kucheria@linaro.org wrote:

...

Here are a couple of patches on top of the LT 4.9 kernel to fix the issue. This sets up USER4 led on the DB410c to blink on a kernel panic.

In PATCH 2/2, could/should we use panic-indicator instead of default-trigger?

Yup, that is indeed better. Didn't know about that property. I'll send a new patch.