diff mbox series

[OpenWrt-Devel] ath79: Add GL.iNet GL-AR300M Family to /etc/board.d/01_leds

Message ID 20190302170150.15949-1-lede@allycomm.com
State Superseded, archived
Headers show
Series [OpenWrt-Devel] ath79: Add GL.iNet GL-AR300M Family to /etc/board.d/01_leds | expand

Commit Message

Jeff Kletsky March 2, 2019, 5:01 p.m. UTC 
From: Jeff Kletsky <git-commits@allycomm.com>

Previously missing, add the three variants;
-nand, -nor, -lite to the definitions in 01_leds

-Lite variant uses language-independent Ethernet
as its single port may be configured as WAN or LAN,
depending use case.

Non-lite variants thanks to Andreas Ziegler
https://patchwork.ozlabs.org/patch/1049396/

Runtime-tested: GL.iNet AR300M-Lite

Signed-off-by: Jeff Kletsky <git-commits@allycomm.com>
---
 target/linux/ath79/base-files/etc/board.d/01_leds | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Piotr Dymacz March 2, 2019, 5:32 p.m. UTC | #1 
Hi Jeff,

On 02.03.2019 18:01, Jeff Kletsky wrote:
> From: Jeff Kletsky <git-commits@allycomm.com>
> 
> Previously missing, add the three variants;
> -nand, -nor, -lite to the definitions in 01_leds
> 
> -Lite variant uses language-independent Ethernet
> as its single port may be configured as WAN or LAN,
> depending use case.

This introduces unnecessary inconsistency. If you look at the whole file 
you will find out that UCI section names (and LED name within the 
section) follow sysfs LED names.

Please, follow this common pattern _or_ update LED names if they don't 
match their functions.

> Non-lite variants thanks to Andreas Ziegler
> https://patchwork.ozlabs.org/patch/1049396/
> 
> Runtime-tested: GL.iNet AR300M-Lite
> 
> Signed-off-by: Jeff Kletsky <git-commits@allycomm.com>
> ---
>   target/linux/ath79/base-files/etc/board.d/01_leds | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/target/linux/ath79/base-files/etc/board.d/01_leds b/target/linux/ath79/base-files/etc/board.d/01_leds
> index a56fe58f76..a5c23e1f84 100755
> --- a/target/linux/ath79/base-files/etc/board.d/01_leds
> +++ b/target/linux/ath79/base-files/etc/board.d/01_leds
> @@ -63,6 +63,13 @@ glinet,gl-ar150)
>   	ucidef_set_led_netdev "wan" "WAN" "$boardname:green:wan" "eth0"
>   	ucidef_set_led_switch "lan" "LAN" "$boardname:green:lan" "switch0" "0x02"
>   	;;
> +glinet,gl-ar300m-nand|\
> +glinet,gl-ar300m-nor)
> +	ucidef_set_led_netdev "lan" "LAN" "gl-ar300m:green:wan" "eth0"
> +	;;

ucidef_set_led_netdev "wan" "WAN" ...

> +glinet,gl-ar300m-lite)
> +	ucidef_set_led_netdev "ethernet" "Ethernet" "gl-ar300m-lite:green:wan" "eth0"
> +	;;

Same here.

Also, please make sure used LED names are correct.
'qca9531_glinet_gl-ar300m.dtsi' doesn't contain 'gl-ar300m:green:wan'...
Jeff Kletsky March 2, 2019, 8:08 p.m. UTC | #2 
On 3/2/19 9:32 AM, Piotr Dymacz wrote:
> Hi Jeff,
>
> On 02.03.2019 18:01, Jeff Kletsky wrote:
>> [...]]
>> -Lite variant uses language-independent Ethernet
>> as its single port may be configured as WAN or LAN,
>> depending use case.
>
> This introduces unnecessary inconsistency. If you look at the whole 
> file you will find out that UCI section names (and LED name within the 
> section) follow sysfs LED names.
>
> Please, follow this common pattern _or_ update LED names if they don't 
> match their functions.
>
> [...]
>
> Also, please make sure used LED names are correct.
> 'qca9531_glinet_gl-ar300m.dtsi' doesn't contain 'gl-ar300m:green:wan'...
>

Piotr,

Thanks for the constructive comments.

Patches prepared and rebased on current `master`.

Waiting for word on a robust way to deliver them so that Patchwork
properly associates them with each of the two existing patches, rather
than creating new ones (when I've used `git email-patch --in-reply-to=`)
or mangling them (as seems to happen if I use `git format-patch` and
Thunderbird).


Jeff
diff mbox series

Patch

diff --git a/target/linux/ath79/base-files/etc/board.d/01_leds b/target/linux/ath79/base-files/etc/board.d/01_leds
index a56fe58f76..a5c23e1f84 100755
--- a/target/linux/ath79/base-files/etc/board.d/01_leds
+++ b/target/linux/ath79/base-files/etc/board.d/01_leds
@@ -63,6 +63,13 @@  glinet,gl-ar150)
 	ucidef_set_led_netdev "wan" "WAN" "$boardname:green:wan" "eth0"
 	ucidef_set_led_switch "lan" "LAN" "$boardname:green:lan" "switch0" "0x02"
 	;;
+glinet,gl-ar300m-nand|\
+glinet,gl-ar300m-nor)
+	ucidef_set_led_netdev "lan" "LAN" "gl-ar300m:green:wan" "eth0"
+	;;
+glinet,gl-ar300m-lite)
+	ucidef_set_led_netdev "ethernet" "Ethernet" "gl-ar300m-lite:green:wan" "eth0"
+	;;
 glinet,gl-x750)
 	ucidef_set_led_netdev "wan" "WAN" "$boardname:green:wan" "eth0"
 	;;