diff mbox

[LEDE-DEV] kirkwood: disable fault LEDs by default

Message ID trinity-d7c73486-ebb4-424e-91d6-494f41c2d2c0-1477165610638@3capp-gmx-bs56
State Changes Requested
Headers show

Commit Message

p.wassi@gmx.at Oct. 22, 2016, 7:46 p.m. UTC
From: P.Wassi <p.wassi@gmx.at>

Set the default value for status-fault LEDs to '0'

Signed-off-by: P.Wassi <p.wassi@gmx.at>
---
The kirkwood-dockstar, kirkwood-goflex* and kirkwood-pogoplug devices have
one duo-color status LED (green+orange / health+fault).
For the dockstar and pogoplug the orange fault LED is enabled by default.
(This is not the case for goflex*-devices).
This patch unifies the behaviour and disabled the fault LED during bootup.

linux/kirkwood/base-files/etc/board.d/01_leds |    4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Mathias Kresin Oct. 22, 2016, 8:43 p.m. UTC | #1
2016-10-22 21:46 GMT+02:00  <p.wassi@gmx.at>:
> From: P.Wassi <p.wassi@gmx.at>
>
> Set the default value for status-fault LEDs to '0'
>
> Signed-off-by: P.Wassi <p.wassi@gmx.at>

Full name please!

> ---
> The kirkwood-dockstar, kirkwood-goflex* and kirkwood-pogoplug devices have
> one duo-color status LED (green+orange / health+fault).
> For the dockstar and pogoplug the orange fault LED is enabled by default.
> (This is not the case for goflex*-devices).
> This patch unifies the behaviour and disabled the fault LED during bootup.

This has to go into the commit message. The commit message should make
clean why this change is required. Have a look at
http://chris.beams.io/posts/git-commit/#why-not-how for a more
detailed (and better) explanation.

>
> linux/kirkwood/base-files/etc/board.d/01_leds |    4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/linux/kirkwood/base-files/etc/board.d/01_leds b/target/linux/kirkwood/base-files/etc/board.d/01_leds
> --- a/target/linux/kirkwood/base-files/etc/board.d/01_leds
> +++ b/target/linux/kirkwood/base-files/etc/board.d/01_leds
> @@ -13,7 +13,7 @@ board=$(kirkwood_board_name)
>  case "$board" in
>  "dockstar")
>         ucidef_set_led_default "health" "health" "status:green:health" "1"
> -       ucidef_set_led_default "fault" "fault" "status:orange:fault" "1"
> +       ucidef_set_led_default "fault" "fault" "status:orange:fault" "0"

status:orange:fault is on after boot because it is used as status_led.
What do you think about using the status:green:health led instead as
status_led in diag.sh (similar to the power led on other boards)?
Letting a fault indication led blinking (during boot) while everything
is fine, might not be the best idea.

Another benefit would be that both ucidef_set_led_default calls could
be dropped.

>         ;;
>  "linksys-audi")
>         ucidef_set_led_default "power" "power" "audi:green:power" "1"
> @@ -33,7 +33,7 @@ case "$board" in
>         ;;
>  "pogo_e02")
>         ucidef_set_led_default "health" "health" "pogo_e02:green:health" "1"
> -       ucidef_set_led_default "fault" "fault" "pogo_e02:orange:fault" "1"
> +       ucidef_set_led_default "fault" "fault" "pogo_e02:orange:fault" "0"

Same as above.

>         ;;
>  "guruplug-server-plus")
>         ucidef_set_led_timer "health" "health" "guruplug:red:health" "200" "800"
>
> _______________________________________________
> Lede-dev mailing list
> Lede-dev@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/lede-dev
p.wassi@gmx.at Oct. 23, 2016, 10:25 a.m. UTC | #2
Hi Mathias,

> status:orange:fault is on after boot because it is used as status_led.
Ah, I see. However, after changing the default value to '0' it is
off after boot (althoug set as status_led !)

> What do you think about using the status:green:health led instead as
> status_led in diag.sh (similar to the power led on other boards)?
Now that I think about it, two other issues come to my mind:
-) some time ago, there was a plan to have the LEDs' names reflect the boardname.
  https://lists.infradead.org/pipermail/lede-dev/2016-July/002050.html
  So the LEDs would not be called "status:green:health" but instead "dockstar:green:health"
  and so on...
-) currently, the code in etc/diag.sh is kind of broken, the target/board 'pogo_e02' does
  not have a LED called "status:orange:fault"; indeed it is called "pogo_e02:orange:fault"

Hmmm... I'll prepare some patch(es) for the entire kirkwood target.

Best regards,
P. Wassi
diff mbox

Patch

diff --git a/target/linux/kirkwood/base-files/etc/board.d/01_leds b/target/linux/kirkwood/base-files/etc/board.d/01_leds
--- a/target/linux/kirkwood/base-files/etc/board.d/01_leds
+++ b/target/linux/kirkwood/base-files/etc/board.d/01_leds
@@ -13,7 +13,7 @@  board=$(kirkwood_board_name)
 case "$board" in
 "dockstar")
        ucidef_set_led_default "health" "health" "status:green:health" "1"
-       ucidef_set_led_default "fault" "fault" "status:orange:fault" "1"
+       ucidef_set_led_default "fault" "fault" "status:orange:fault" "0"
        ;;
 "linksys-audi")
        ucidef_set_led_default "power" "power" "audi:green:power" "1"
@@ -33,7 +33,7 @@  case "$board" in
        ;;
 "pogo_e02")
        ucidef_set_led_default "health" "health" "pogo_e02:green:health" "1"
-       ucidef_set_led_default "fault" "fault" "pogo_e02:orange:fault" "1"
+       ucidef_set_led_default "fault" "fault" "pogo_e02:orange:fault" "0"
        ;;
 "guruplug-server-plus")
        ucidef_set_led_timer "health" "health" "guruplug:red:health" "200" "800"