Re: [LEDE-DEV] [PATCH 2/2] x86: Add board configs for the PC Engines APU2

[David Woodhouse]

On Mon, 2017-02-13 at 12:50 +0100, Felix Fietkau wrote:
> 
> Please leave out the /lib/.sh anti-pattern. Just use
> /tmp/sysinfo/board_name directly or (even better) introduce a generic
> function for getting the board name.

Once I stop /etc/diag.sh from having hard-coded board name stuff, we go
back to only one instance of reading /tmp/sysinfo/board_name — so I
dropped that part from https://github.com/lede-project/source/pull/824

Testing seems to show that the diag LED only really works from the
second boot though, since it isn't *configured* during the first boot.


smime.p7s
Description: S/MIME cryptographic signature
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

Re: [LEDE-DEV] [PATCH 2/2] x86: Add board configs for the PC Engines APU2

[Felix Fietkau]

On 2017-02-13 09:01, Chris Blake wrote:
> This adds the default LED and network settings for the PC Engines APU2
> when running under the x86 target.
> 
> Signed-off-by: Chris Blake 
> ---
>  target/linux/x86/base-files/etc/board.d/01_leds|  7 +++-
>  target/linux/x86/base-files/etc/board.d/02_network |  7 +++-
>  target/linux/x86/base-files/etc/diag.sh| 37 
> ++
>  target/linux/x86/base-files/lib/x86.sh | 13 
>  4 files changed, 62 insertions(+), 2 deletions(-)
>  create mode 100755 target/linux/x86/base-files/etc/diag.sh
>  create mode 100755 target/linux/x86/base-files/lib/x86.sh
> 
> diff --git a/target/linux/x86/base-files/lib/x86.sh 
> b/target/linux/x86/base-files/lib/x86.sh
> new file mode 100755
> index 000..40e4d1f
> --- /dev/null
> +++ b/target/linux/x86/base-files/lib/x86.sh
Please leave out the /lib/.sh anti-pattern. Just use
/tmp/sysinfo/board_name directly or (even better) introduce a generic
function for getting the board name.

- Felix

___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

Re: [LEDE-DEV] [PATCH 2/2] x86: Add board configs for the PC Engines APU2

[David Woodhouse]

On Mon, 2017-02-13 at 02:01 -0600, Chris Blake wrote:
> 
> +get_status_led() {
> +    case $(x86_board_name) in
> +    pc-engines-apu2)
> +    status_led="apu2:green:power"
> +    ;;
> +    esac
> +}

Ick. Why hard-code that and have board-specific stuff in more than one
place, instead of just configuring it with everything else, then
deferring to the configuration? Can't we do something like this
instead?

diff --git a/target/linux/x86/base-files/etc/diag.sh 
b/target/linux/x86/base-files/etc/diag.sh
index 2426161..c51d080 100755
--- a/target/linux/x86/base-files/etc/diag.sh
+++ b/target/linux/x86/base-files/etc/diag.sh
@@ -3,15 +3,26 @@
 # Copyright © 2017 OpenWrt.org
 #
 
+. /lib/functions.sh
 . /lib/functions/leds.sh
 . /lib/x86.sh
 
+match_diag_led() {
+   local name
+   local default
+   local sysfs
+   config_get name "$1" name
+   config_get default "$1" default
+   config_get sysfs "$1" sysfs
+
+   if [ "$name" = "DIAG" -a "$default" = "1" ]; then
+   status_led="$sysfs"
+   fi
+}
+
 get_status_led() {
-case $(x86_board_name) in
-pc-engines-apu2)
-status_led="apu2:green:power"
-;;
-esac
+   config_load system
+   config_foreach match_diag_led led
 }
 
 set_state() {
diff --git a/target/linux/x86/base-files/etc/board.d/01_leds 
b/target/linux/x86/base-files/etc/board.d/01_leds
index 05ddc71..50eac33 100755
--- a/target/linux/x86/base-files/etc/board.d/01_leds
+++ b/target/linux/x86/base-files/etc/board.d/01_leds
@@ -14,11 +14,12 @@ case "$board" in
 pc-engines-apu2)
    ucidef_set_led_netdev "wan" "WAN" "apu2:green:led3" "eth0"
    ucidef_set_led_netdev "lan" "LAN" "apu2:green:led2" "eth1"
+   ucidef_set_led_default "diag" "DIAG" "apu2:green:power" "1"
    ;;
 traverse-technologies-geos)
    ucidef_set_led_netdev "lan" "LAN" "geos:1" "br-lan" "tx rx"
    ucidef_set_led_netdev "wlan" "WiFi" "geos:2" "phy0tpt"
-   ucidef_set_led_default "diag" "DIAG" "geos:3" "0"
+   ucidef_set_led_default "diag" "DIAG" "geos:3" "1"
    ;;
 esac
 board_config_flush


smime.p7s
Description: S/MIME cryptographic signature
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev