diff mbox series

[OpenWrt-Devel] bcm63xx: DGND3700v1: device tree improvements

Message ID 20027397.f1LIkykH4V@tool
State Changes Requested
Headers show
Series [OpenWrt-Devel] bcm63xx: DGND3700v1: device tree improvements | expand

Commit Message

Daniel González Cabanelas May 23, 2020, 10:24 p.m. UTC
Improve the device tree file and related board data for the DGND3700v1/
DGND3800B router:
 - Improve LEDs definitions, use shorter names.
 - Make the board name more readable.
 - Fix the switch LAN labels, the order is reversed.
 - Use the real name of the board for the board name instead of device
   name.
 - Use standarized names for partition nodes and leds.

Signed-off-by: Daniel González Cabanelas <dgcbueu@gmail.com>
---
 .../bcm63xx/base-files/etc/board.d/01_leds    | 12 ++--
 .../dts/bcm6368-netgear-dgnd3700-v1.dts       | 64 +++++++++----------
 .../549-board_DGND3700v1_3800B.patch          |  2 +-
 3 files changed, 39 insertions(+), 39 deletions(-)

Comments

Adrian Schmutzler May 23, 2020, 11:01 p.m. UTC | #1
Hi,

> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org]
> On Behalf Of Daniel González Cabanelas
> Sent: Sonntag, 24. Mai 2020 00:24
> To: openwrt-devel@lists.openwrt.org
> Cc: noltari@gmail.com
> Subject: [OpenWrt-Devel] [PATCH] bcm63xx: DGND3700v1: device tree
> improvements
> 
> Improve the device tree file and related board data for the DGND3700v1/
> DGND3800B router:
>  - Improve LEDs definitions, use shorter names.
>  - Make the board name more readable.
>  - Fix the switch LAN labels, the order is reversed.
>  - Use the real name of the board for the board name instead of device
>    name.
>  - Use standarized names for partition nodes and leds.

This deals with several different issues at the same time. I'd prefer to have it split up (e.g. separate board name change from LED changes and switch changes).

> 
> Signed-off-by: Daniel González Cabanelas <dgcbueu@gmail.com>
> ---
>  .../bcm63xx/base-files/etc/board.d/01_leds    | 12 ++--
>  .../dts/bcm6368-netgear-dgnd3700-v1.dts       | 64 +++++++++----------
>  .../549-board_DGND3700v1_3800B.patch          |  2 +-
>  3 files changed, 39 insertions(+), 39 deletions(-)
> 
> diff --git a/target/linux/bcm63xx/base-files/etc/board.d/01_leds
> b/target/linux/bcm63xx/base-files/etc/board.d/01_leds
> index 91d67f0c0b..6b82d9e952 100755
> --- a/target/linux/bcm63xx/base-files/etc/board.d/01_leds
> +++ b/target/linux/bcm63xx/base-files/etc/board.d/01_leds
> @@ -66,12 +66,12 @@ inventel,livebox-1)
>  	ucidef_set_led_netdev "wlan0" "WIFI" "Livebox1:red:wifi" "wlan0"
>  	;;
>  netgear,dgnd3700-v1)
> -	ucidef_set_led_netdev "lan" "LAN" "DGND3700v1_3800B:green:lan"
> "eth0.1"
> -	ucidef_set_led_netdev "wan" "WAN"
> "DGND3700v1_3800B:green:inet" "eth0.2"
> -	ucidef_set_led_netdev "wlan0" "WIFI2G"
> "DGND3700v1_3800B:green:wifi2g" "wlan0"
> -	ucidef_set_led_netdev "wlan1" "WIFI5G"
> "DGND3700v1_3800B:blue:wifi5g" "wlan1"
> -	ucidef_set_led_usbdev "usb1" "USB1"
> "DGND3700v1_3800B:green:usb-back" "1-1"
> -	ucidef_set_led_usbdev "usb2" "USB2"
> "DGND3700v1_3800B:green:usb-front" "1-2"
> +	ucidef_set_led_netdev "lan" "LAN" "$model:green:lan" "eth0.1"
> +	ucidef_set_led_netdev "wan" "WAN" "$model:green:inet" "eth0.2"
> +	ucidef_set_led_netdev "wlan0" "WIFI2G" "$model:green:wifi2g"
> "wlan0"
> +	ucidef_set_led_netdev "wlan1" "WIFI5G" "$model:blue:wifi5g"
> "wlan1"
> +	ucidef_set_led_usbdev "usb1" "USB1" "$model:green:usb-back" "1-
> 1"
> +	ucidef_set_led_usbdev "usb2" "USB2" "$model:green:usb-front" "1-
> 2"

Is there any way to do sysupgrade on these devices? If yes, you will need migration of the names in /etc/config/system ...

>  	;;
>  netgear,dgnd3700-v2)
>  	ucidef_set_led_netdev "lan" "LAN" "$model:green:ethernet" "eth0"
> diff --git a/target/linux/bcm63xx/dts/bcm6368-netgear-dgnd3700-v1.dts
> b/target/linux/bcm63xx/dts/bcm6368-netgear-dgnd3700-v1.dts
> index 546b0b1f60..c17bb3a600 100644
> --- a/target/linux/bcm63xx/dts/bcm6368-netgear-dgnd3700-v1.dts
> +++ b/target/linux/bcm63xx/dts/bcm6368-netgear-dgnd3700-v1.dts
> @@ -5,12 +5,12 @@
>  #include <dt-bindings/input/input.h>
> 
>  / {
> -	model = "Netgear DGND3700v1/DGND3800B";
> +	model = "Netgear DGND3700v1 / DGND3800B";

I don't think this is really necessary ...

>  	compatible = "netgear,dgnd3700-v1", "brcm,bcm6368";
> 
>  	aliases {
>  		led-boot = &led_power_green;
> -		led-failsafe = &led_power_green;
> +		led-failsafe = &led_power_red;

This should be a separate commit again.

>  		led-running = &led_power_green;
>  		led-upgrade = &led_power_green;
>  	};
> @@ -51,49 +51,49 @@
>  	leds {
>  		compatible = "gpio-leds";
> 
> -		dsl_green {
> -			label = "DGND3700v1_3800B:green:dsl";
> +		led@2 {

I don't know whether this is different on bcm63xx, but based on what I'm used to the old node name is preferred (dsl_green).

Best

Adrian

> +			label = "dgnd3700-v1:green:dsl";
>  			gpios = <&pinctrl 2 1>;
>  		};
> -		inet_red {
> -			label = "DGND3700v1_3800B:red:inet";
> +		led@4 {
> +			label = "dgnd3700-v1:red:inet";
>  			gpios = <&pinctrl 4 1>;
>  		};
> -		inet_green {
> -			label = "DGND3700v1_3800B:green:inet";
> +		led@5 {
> +			label = "dgnd3700-v1:green:inet";
>  			gpios = <&pinctrl 5 1>;
>  		};
> -		wps_green {
> -			label = "DGND3700v1_3800B:green:wps";
> +		led@11 {
> +			label = "dgnd3700-v1:green:wps";
>  			gpios = <&pinctrl 11 1>;
>  		};
> -		usbfront_green {
> -			label = "DGND3700v1_3800B:green:usb-front";
> +		led@13 {
> +			label = "dgnd3700-v1:green:usb-front";
>  			gpios = <&pinctrl 13 1>;
>  		};
> -		usbback_green {
> -			label = "DGND3700v1_3800B:green:usb-back";
> +		led@14 {
> +			label = "dgnd3700-v1:green:usb-back";
>  			gpios = <&pinctrl 14 1>;
>  		};
> -		power_red {
> -			label = "DGND3700v1_3800B:red:power";
> +		led_power_red: led@22 {
> +			label = "dgnd3700-v1:red:power";
>  			gpios = <&pinctrl 22 1>;
>  		};
> -		lan_green {
> -			label = "DGND3700v1_3800B:green:lan";
> +		led@23 {
> +			label = "dgnd3700-v1:green:lan";
>  			gpios = <&pinctrl 23 1>;
>  		};
> -		led_power_green: power_green {
> -			label = "DGND3700v1_3800B:green:power";
> +		led_power_green: led@24 {
> +			label = "dgnd3700-v1:green:power";
>  			gpios = <&pinctrl 24 1>;
>  			default-state = "on";
>  		};
> -		wifi2g_green {
> -			label = "DGND3700v1_3800B:green:wifi2g";
> +		led@26 {
> +			label = "dgnd3700-v1:green:wifi2g";
>  			gpios = <&pinctrl 26 1>;
>  		};
> -		wifi5g_blue {
> -			label = "DGND3700v1_3800B:blue:wifi5g";
> +		led@27 {
> +			label = "dgnd3700-v1:blue:wifi5g";
>  			gpios = <&pinctrl 27 1>;
>  		};
>  	};
> @@ -107,25 +107,25 @@
>  		#address-cells = <1>;
>  		#size-cells = <1>;
> 
> -		cfe@0 {
> +		partition@0 {
>  			label = "CFE";
>  			reg = <0x000000 0x020000>;
>  			read-only;
>  		};
> 
> -		linux@20000 {
> +		partition@20000 {
>  			label = "linux";
>  			reg = <0x020000 0x1e20000>;
>  			compatible = "brcm,bcm963xx-imagetag";
>  		};
> 
> -		board_data@1e40000 {
> +		partition@1e40000 {
>  			label = "board_data";
>  			reg = <0x1e40000 0x1a0000>;
>  			read-only;
>  		};
> 
> -		nvram@1fe0000 {
> +		partition@1fe0000 {
>  			label = "nvram";
>  			reg = <0x1fe0000 0x20000>;
>  		};
> @@ -156,22 +156,22 @@
> 
>  			lan@1 {
>  				reg = <1>;
> -				label = "lan1";
> +				label = "lan4";
>  			};
> 
>  			lan@2 {
>  				reg = <2>;
> -				label = "lan2";
> +				label = "lan3";
>  			};
> 
>  			lan@3 {
>  				reg = <3>;
> -				label = "lan3";
> +				label = "lan2";
>  			};
> 
>  			lan@4 {
>  				reg = <4>;
> -				label = "lan4";
> +				label = "lan1";
>  			};
> 
>  			cpu@8 {
> diff --git a/target/linux/bcm63xx/patches-5.4/549-
> board_DGND3700v1_3800B.patch b/target/linux/bcm63xx/patches-5.4/549-
> board_DGND3700v1_3800B.patch
> index 936aab115b..7569e9643e 100644
> --- a/target/linux/bcm63xx/patches-5.4/549-
> board_DGND3700v1_3800B.patch
> +++ b/target/linux/bcm63xx/patches-5.4/549-
> board_DGND3700v1_3800B.patch
> @@ -5,7 +5,7 @@
>   };
> 
>  +static struct board_info __initdata board_DGND3700v1_3800B = {
> -+	.name				= "DGND3700v1_3800B",
> ++	.name				= "U12L144T01",
>  +	.expected_cpu_id		= 0x6368,
>  +
>  +	.has_pci			= 1,
> --
> 2.26.2
> 
> 
> 
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Daniel González Cabanelas May 24, 2020, 9:05 a.m. UTC | #2
Hi Adrian:

El dom., 24 may. 2020 a las 1:01, <mail@adrianschmutzler.de> escribió:
>
> Hi,
>
> > -----Original Message-----
> > From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org]
> > On Behalf Of Daniel González Cabanelas
> > Sent: Sonntag, 24. Mai 2020 00:24
> > To: openwrt-devel@lists.openwrt.org
> > Cc: noltari@gmail.com
> > Subject: [OpenWrt-Devel] [PATCH] bcm63xx: DGND3700v1: device tree
> > improvements
> >
> > Improve the device tree file and related board data for the DGND3700v1/
> > DGND3800B router:
> >  - Improve LEDs definitions, use shorter names.
> >  - Make the board name more readable.
> >  - Fix the switch LAN labels, the order is reversed.
> >  - Use the real name of the board for the board name instead of device
> >    name.
> >  - Use standarized names for partition nodes and leds.
>
> This deals with several different issues at the same time. I'd prefer to have it split up (e.g. separate board name change from LED changes and switch changes).
>

I can't see the benefit of flooding with commits on every negligible
change. These are just cosmetic changes which won't affect the
behavior of the device, and wont produce any unexpected bug, I've made
the opportune tests.

> >
> > Signed-off-by: Daniel González Cabanelas <dgcbueu@gmail.com>
> > ---
> >  .../bcm63xx/base-files/etc/board.d/01_leds    | 12 ++--
> >  .../dts/bcm6368-netgear-dgnd3700-v1.dts       | 64 +++++++++----------
> >  .../549-board_DGND3700v1_3800B.patch          |  2 +-
> >  3 files changed, 39 insertions(+), 39 deletions(-)
> >
> > diff --git a/target/linux/bcm63xx/base-files/etc/board.d/01_leds
> > b/target/linux/bcm63xx/base-files/etc/board.d/01_leds
> > index 91d67f0c0b..6b82d9e952 100755
> > --- a/target/linux/bcm63xx/base-files/etc/board.d/01_leds
> > +++ b/target/linux/bcm63xx/base-files/etc/board.d/01_leds
> > @@ -66,12 +66,12 @@ inventel,livebox-1)
> >       ucidef_set_led_netdev "wlan0" "WIFI" "Livebox1:red:wifi" "wlan0"
> >       ;;
> >  netgear,dgnd3700-v1)
> > -     ucidef_set_led_netdev "lan" "LAN" "DGND3700v1_3800B:green:lan"
> > "eth0.1"
> > -     ucidef_set_led_netdev "wan" "WAN"
> > "DGND3700v1_3800B:green:inet" "eth0.2"
> > -     ucidef_set_led_netdev "wlan0" "WIFI2G"
> > "DGND3700v1_3800B:green:wifi2g" "wlan0"
> > -     ucidef_set_led_netdev "wlan1" "WIFI5G"
> > "DGND3700v1_3800B:blue:wifi5g" "wlan1"
> > -     ucidef_set_led_usbdev "usb1" "USB1"
> > "DGND3700v1_3800B:green:usb-back" "1-1"
> > -     ucidef_set_led_usbdev "usb2" "USB2"
> > "DGND3700v1_3800B:green:usb-front" "1-2"
> > +     ucidef_set_led_netdev "lan" "LAN" "$model:green:lan" "eth0.1"
> > +     ucidef_set_led_netdev "wan" "WAN" "$model:green:inet" "eth0.2"
> > +     ucidef_set_led_netdev "wlan0" "WIFI2G" "$model:green:wifi2g"
> > "wlan0"
> > +     ucidef_set_led_netdev "wlan1" "WIFI5G" "$model:blue:wifi5g"
> > "wlan1"
> > +     ucidef_set_led_usbdev "usb1" "USB1" "$model:green:usb-back" "1-
> > 1"
> > +     ucidef_set_led_usbdev "usb2" "USB2" "$model:green:usb-front" "1-
> > 2"
>
> Is there any way to do sysupgrade on these devices? If yes, you will need migration of the names in /etc/config/system ...
>

The sysupgrade works out of the box using the default_do_upgrade. Not
sure what I need to review, can you be more specific?

> >       ;;
> >  netgear,dgnd3700-v2)
> >       ucidef_set_led_netdev "lan" "LAN" "$model:green:ethernet" "eth0"
> > diff --git a/target/linux/bcm63xx/dts/bcm6368-netgear-dgnd3700-v1.dts
> > b/target/linux/bcm63xx/dts/bcm6368-netgear-dgnd3700-v1.dts
> > index 546b0b1f60..c17bb3a600 100644
> > --- a/target/linux/bcm63xx/dts/bcm6368-netgear-dgnd3700-v1.dts
> > +++ b/target/linux/bcm63xx/dts/bcm6368-netgear-dgnd3700-v1.dts
> > @@ -5,12 +5,12 @@
> >  #include <dt-bindings/input/input.h>
> >
> >  / {
> > -     model = "Netgear DGND3700v1/DGND3800B";
> > +     model = "Netgear DGND3700v1 / DGND3800B";
>
> I don't think this is really necessary ...
>
> >       compatible = "netgear,dgnd3700-v1", "brcm,bcm6368";
> >
> >       aliases {
> >               led-boot = &led_power_green;
> > -             led-failsafe = &led_power_green;
> > +             led-failsafe = &led_power_red;
>
> This should be a separate commit again.
>

One commit per line, on the same file, really?, again this is a minor change.

> >               led-running = &led_power_green;
> >               led-upgrade = &led_power_green;
> >       };
> > @@ -51,49 +51,49 @@
> >       leds {
> >               compatible = "gpio-leds";
> >
> > -             dsl_green {
> > -                     label = "DGND3700v1_3800B:green:dsl";
> > +             led@2 {
>
> I don't know whether this is different on bcm63xx, but based on what I'm used to the old node name is preferred (dsl_green).
>

Well, I'll never know what's the best way for naming a led node. I've
taken the partitions nodes as a reference, and the LEDs device tree
documentation also use this way as an example.

Regards

> Best
>
> Adrian
>
> > +                     label = "dgnd3700-v1:green:dsl";
> >                       gpios = <&pinctrl 2 1>;
> >               };
> > -             inet_red {
> > -                     label = "DGND3700v1_3800B:red:inet";
> > +             led@4 {
> > +                     label = "dgnd3700-v1:red:inet";
> >                       gpios = <&pinctrl 4 1>;
> >               };
> > -             inet_green {
> > -                     label = "DGND3700v1_3800B:green:inet";
> > +             led@5 {
> > +                     label = "dgnd3700-v1:green:inet";
> >                       gpios = <&pinctrl 5 1>;
> >               };
> > -             wps_green {
> > -                     label = "DGND3700v1_3800B:green:wps";
> > +             led@11 {
> > +                     label = "dgnd3700-v1:green:wps";
> >                       gpios = <&pinctrl 11 1>;
> >               };
> > -             usbfront_green {
> > -                     label = "DGND3700v1_3800B:green:usb-front";
> > +             led@13 {
> > +                     label = "dgnd3700-v1:green:usb-front";
> >                       gpios = <&pinctrl 13 1>;
> >               };
> > -             usbback_green {
> > -                     label = "DGND3700v1_3800B:green:usb-back";
> > +             led@14 {
> > +                     label = "dgnd3700-v1:green:usb-back";
> >                       gpios = <&pinctrl 14 1>;
> >               };
> > -             power_red {
> > -                     label = "DGND3700v1_3800B:red:power";
> > +             led_power_red: led@22 {
> > +                     label = "dgnd3700-v1:red:power";
> >                       gpios = <&pinctrl 22 1>;
> >               };
> > -             lan_green {
> > -                     label = "DGND3700v1_3800B:green:lan";
> > +             led@23 {
> > +                     label = "dgnd3700-v1:green:lan";
> >                       gpios = <&pinctrl 23 1>;
> >               };
> > -             led_power_green: power_green {
> > -                     label = "DGND3700v1_3800B:green:power";
> > +             led_power_green: led@24 {
> > +                     label = "dgnd3700-v1:green:power";
> >                       gpios = <&pinctrl 24 1>;
> >                       default-state = "on";
> >               };
> > -             wifi2g_green {
> > -                     label = "DGND3700v1_3800B:green:wifi2g";
> > +             led@26 {
> > +                     label = "dgnd3700-v1:green:wifi2g";
> >                       gpios = <&pinctrl 26 1>;
> >               };
> > -             wifi5g_blue {
> > -                     label = "DGND3700v1_3800B:blue:wifi5g";
> > +             led@27 {
> > +                     label = "dgnd3700-v1:blue:wifi5g";
> >                       gpios = <&pinctrl 27 1>;
> >               };
> >       };
> > @@ -107,25 +107,25 @@
> >               #address-cells = <1>;
> >               #size-cells = <1>;
> >
> > -             cfe@0 {
> > +             partition@0 {
> >                       label = "CFE";
> >                       reg = <0x000000 0x020000>;
> >                       read-only;
> >               };
> >
> > -             linux@20000 {
> > +             partition@20000 {
> >                       label = "linux";
> >                       reg = <0x020000 0x1e20000>;
> >                       compatible = "brcm,bcm963xx-imagetag";
> >               };
> >
> > -             board_data@1e40000 {
> > +             partition@1e40000 {
> >                       label = "board_data";
> >                       reg = <0x1e40000 0x1a0000>;
> >                       read-only;
> >               };
> >
> > -             nvram@1fe0000 {
> > +             partition@1fe0000 {
> >                       label = "nvram";
> >                       reg = <0x1fe0000 0x20000>;
> >               };
> > @@ -156,22 +156,22 @@
> >
> >                       lan@1 {
> >                               reg = <1>;
> > -                             label = "lan1";
> > +                             label = "lan4";
> >                       };
> >
> >                       lan@2 {
> >                               reg = <2>;
> > -                             label = "lan2";
> > +                             label = "lan3";
> >                       };
> >
> >                       lan@3 {
> >                               reg = <3>;
> > -                             label = "lan3";
> > +                             label = "lan2";
> >                       };
> >
> >                       lan@4 {
> >                               reg = <4>;
> > -                             label = "lan4";
> > +                             label = "lan1";
> >                       };
> >
> >                       cpu@8 {
> > diff --git a/target/linux/bcm63xx/patches-5.4/549-
> > board_DGND3700v1_3800B.patch b/target/linux/bcm63xx/patches-5.4/549-
> > board_DGND3700v1_3800B.patch
> > index 936aab115b..7569e9643e 100644
> > --- a/target/linux/bcm63xx/patches-5.4/549-
> > board_DGND3700v1_3800B.patch
> > +++ b/target/linux/bcm63xx/patches-5.4/549-
> > board_DGND3700v1_3800B.patch
> > @@ -5,7 +5,7 @@
> >   };
> >
> >  +static struct board_info __initdata board_DGND3700v1_3800B = {
> > -+    .name                           = "DGND3700v1_3800B",
> > ++    .name                           = "U12L144T01",
> >  +    .expected_cpu_id                = 0x6368,
> >  +
> >  +    .has_pci                        = 1,
> > --
> > 2.26.2
> >
> >
> >
> >
> >
> > _______________________________________________
> > openwrt-devel mailing list
> > openwrt-devel@lists.openwrt.org
> > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Adrian Schmutzler May 24, 2020, 11:15 a.m. UTC | #3
Hi Daniel,

> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org]
> On Behalf Of Daniel González Cabanelas
> Sent: Sonntag, 24. Mai 2020 11:06
> To: mail@adrianschmutzler.de
> Cc: openwrt-devel@lists.openwrt.org; Álvaro Fernández Rojas
> <noltari@gmail.com>
> Subject: Re: [OpenWrt-Devel] [PATCH] bcm63xx: DGND3700v1: device tree
> improvements
> 
> Hi Adrian:
> 
> El dom., 24 may. 2020 a las 1:01, <mail@adrianschmutzler.de> escribió:
> >
> > Hi,
> >
> > > -----Original Message-----
> > > From: openwrt-devel [mailto:openwrt-devel-
> bounces@lists.openwrt.org]
> > > On Behalf Of Daniel González Cabanelas
> > > Sent: Sonntag, 24. Mai 2020 00:24
> > > To: openwrt-devel@lists.openwrt.org
> > > Cc: noltari@gmail.com
> > > Subject: [OpenWrt-Devel] [PATCH] bcm63xx: DGND3700v1: device tree
> > > improvements
> > >
> > > Improve the device tree file and related board data for the
> > > DGND3700v1/ DGND3800B router:
> > >  - Improve LEDs definitions, use shorter names.
> > >  - Make the board name more readable.
> > >  - Fix the switch LAN labels, the order is reversed.
> > >  - Use the real name of the board for the board name instead of device
> > >    name.
> > >  - Use standarized names for partition nodes and leds.
> >
> > This deals with several different issues at the same time. I'd prefer to have
> it split up (e.g. separate board name change from LED changes and switch
> changes).
> >
> 
> I can't see the benefit of flooding with commits on every negligible change.
> These are just cosmetic changes which won't affect the behavior of the
> device, and wont produce any unexpected bug, I've made the opportune
> tests.

Well, one of the possible bugs would be that LEDs won't work after sysupgrade, see below.
I don't think that everything should be separate, but I don't like completely different things stuffed together.

> 
> > >
> > > Signed-off-by: Daniel González Cabanelas <dgcbueu@gmail.com>
> > > ---
> > >  .../bcm63xx/base-files/etc/board.d/01_leds    | 12 ++--
> > >  .../dts/bcm6368-netgear-dgnd3700-v1.dts       | 64 +++++++++----------
> > >  .../549-board_DGND3700v1_3800B.patch          |  2 +-
> > >  3 files changed, 39 insertions(+), 39 deletions(-)
> > >
> > > diff --git a/target/linux/bcm63xx/base-files/etc/board.d/01_leds
> > > b/target/linux/bcm63xx/base-files/etc/board.d/01_leds
> > > index 91d67f0c0b..6b82d9e952 100755
> > > --- a/target/linux/bcm63xx/base-files/etc/board.d/01_leds
> > > +++ b/target/linux/bcm63xx/base-files/etc/board.d/01_leds
> > > @@ -66,12 +66,12 @@ inventel,livebox-1)
> > >       ucidef_set_led_netdev "wlan0" "WIFI" "Livebox1:red:wifi" "wlan0"
> > >       ;;
> > >  netgear,dgnd3700-v1)
> > > -     ucidef_set_led_netdev "lan" "LAN" "DGND3700v1_3800B:green:lan"
> > > "eth0.1"
> > > -     ucidef_set_led_netdev "wan" "WAN"
> > > "DGND3700v1_3800B:green:inet" "eth0.2"
> > > -     ucidef_set_led_netdev "wlan0" "WIFI2G"
> > > "DGND3700v1_3800B:green:wifi2g" "wlan0"
> > > -     ucidef_set_led_netdev "wlan1" "WIFI5G"
> > > "DGND3700v1_3800B:blue:wifi5g" "wlan1"
> > > -     ucidef_set_led_usbdev "usb1" "USB1"
> > > "DGND3700v1_3800B:green:usb-back" "1-1"
> > > -     ucidef_set_led_usbdev "usb2" "USB2"
> > > "DGND3700v1_3800B:green:usb-front" "1-2"
> > > +     ucidef_set_led_netdev "lan" "LAN" "$model:green:lan" "eth0.1"
> > > +     ucidef_set_led_netdev "wan" "WAN" "$model:green:inet" "eth0.2"
> > > +     ucidef_set_led_netdev "wlan0" "WIFI2G" "$model:green:wifi2g"
> > > "wlan0"
> > > +     ucidef_set_led_netdev "wlan1" "WIFI5G" "$model:blue:wifi5g"
> > > "wlan1"
> > > +     ucidef_set_led_usbdev "usb1" "USB1" "$model:green:usb-back"
> > > + "1-
> > > 1"
> > > +     ucidef_set_led_usbdev "usb2" "USB2" "$model:green:usb-front"
> > > + "1-
> > > 2"
> >
> > Is there any way to do sysupgrade on these devices? If yes, you will need
> migration of the names in /etc/config/system ...
> >
> 
> The sysupgrade works out of the box using the default_do_upgrade. Not
> sure what I need to review, can you be more specific?

On a newly installed device there won't be a problem. 01_leds will generate the LED entries in /etc/config/system on firstboot, and after that the names used there won't change anymore, even on upgrade. In contrast, the LED names in the device tree will change with every upgrade, so that this change results in the LED settings becoming broken without a reset of config files. This can be healed with a migration script, e.g.
https://github.com/openwrt/openwrt/blob/master/target/linux/ramips/mt76x8/base-files/etc/uci-defaults/04_led_migration

That's BTW the reason why we haven't changed these for consistency so far on this target.

> 
> > >       ;;
> > >  netgear,dgnd3700-v2)
> > >       ucidef_set_led_netdev "lan" "LAN" "$model:green:ethernet" "eth0"
> > > diff --git
> > > a/target/linux/bcm63xx/dts/bcm6368-netgear-dgnd3700-v1.dts
> > > b/target/linux/bcm63xx/dts/bcm6368-netgear-dgnd3700-v1.dts
> > > index 546b0b1f60..c17bb3a600 100644
> > > --- a/target/linux/bcm63xx/dts/bcm6368-netgear-dgnd3700-v1.dts
> > > +++ b/target/linux/bcm63xx/dts/bcm6368-netgear-dgnd3700-v1.dts
> > > @@ -5,12 +5,12 @@
> > >  #include <dt-bindings/input/input.h>
> > >
> > >  / {
> > > -     model = "Netgear DGND3700v1/DGND3800B";
> > > +     model = "Netgear DGND3700v1 / DGND3800B";
> >
> > I don't think this is really necessary ...
> >
> > >       compatible = "netgear,dgnd3700-v1", "brcm,bcm6368";
> > >
> > >       aliases {
> > >               led-boot = &led_power_green;
> > > -             led-failsafe = &led_power_green;
> > > +             led-failsafe = &led_power_red;
> >
> > This should be a separate commit again.
> >
> 
> One commit per line, on the same file, really?, again this is a minor change.

Yes, but it's a non-cosmetic (with respect to its effect) behavior change that is not at all connected to the rest.

Just imagine we want to backport this or the fixed port order to 19.07, but don't want to mess with LED names. Separate commits for separate topics make sense.

> 
> > >               led-running = &led_power_green;
> > >               led-upgrade = &led_power_green;
> > >       };
> > > @@ -51,49 +51,49 @@
> > >       leds {
> > >               compatible = "gpio-leds";
> > >
> > > -             dsl_green {
> > > -                     label = "DGND3700v1_3800B:green:dsl";
> > > +             led@2 {
> >
> > I don't know whether this is different on bcm63xx, but based on what I'm
> used to the old node name is preferred (dsl_green).
> >
> 
> Well, I'll never know what's the best way for naming a led node. I've taken
> the partitions nodes as a reference, and the LEDs device tree documentation
> also use this way as an example.

Openwrt seems to generally use the name-based scheme (the one already there), and since there is no reason for changing that and it's unconnected to the rest of your changes, please just drop these changes (of course, only the node name, not the label changes).

Best

Adrian

> 
> Regards
> 
> > Best
> >
> > Adrian
> >
> > > +                     label = "dgnd3700-v1:green:dsl";
> > >                       gpios = <&pinctrl 2 1>;
> > >               };
> > > -             inet_red {
> > > -                     label = "DGND3700v1_3800B:red:inet";
> > > +             led@4 {
> > > +                     label = "dgnd3700-v1:red:inet";
> > >                       gpios = <&pinctrl 4 1>;
> > >               };
> > > -             inet_green {
> > > -                     label = "DGND3700v1_3800B:green:inet";
> > > +             led@5 {
> > > +                     label = "dgnd3700-v1:green:inet";
> > >                       gpios = <&pinctrl 5 1>;
> > >               };
> > > -             wps_green {
> > > -                     label = "DGND3700v1_3800B:green:wps";
> > > +             led@11 {
> > > +                     label = "dgnd3700-v1:green:wps";
> > >                       gpios = <&pinctrl 11 1>;
> > >               };
> > > -             usbfront_green {
> > > -                     label = "DGND3700v1_3800B:green:usb-front";
> > > +             led@13 {
> > > +                     label = "dgnd3700-v1:green:usb-front";
> > >                       gpios = <&pinctrl 13 1>;
> > >               };
> > > -             usbback_green {
> > > -                     label = "DGND3700v1_3800B:green:usb-back";
> > > +             led@14 {
> > > +                     label = "dgnd3700-v1:green:usb-back";
> > >                       gpios = <&pinctrl 14 1>;
> > >               };
> > > -             power_red {
> > > -                     label = "DGND3700v1_3800B:red:power";
> > > +             led_power_red: led@22 {
> > > +                     label = "dgnd3700-v1:red:power";
> > >                       gpios = <&pinctrl 22 1>;
> > >               };
> > > -             lan_green {
> > > -                     label = "DGND3700v1_3800B:green:lan";
> > > +             led@23 {
> > > +                     label = "dgnd3700-v1:green:lan";
> > >                       gpios = <&pinctrl 23 1>;
> > >               };
> > > -             led_power_green: power_green {
> > > -                     label = "DGND3700v1_3800B:green:power";
> > > +             led_power_green: led@24 {
> > > +                     label = "dgnd3700-v1:green:power";
> > >                       gpios = <&pinctrl 24 1>;
> > >                       default-state = "on";
> > >               };
> > > -             wifi2g_green {
> > > -                     label = "DGND3700v1_3800B:green:wifi2g";
> > > +             led@26 {
> > > +                     label = "dgnd3700-v1:green:wifi2g";
> > >                       gpios = <&pinctrl 26 1>;
> > >               };
> > > -             wifi5g_blue {
> > > -                     label = "DGND3700v1_3800B:blue:wifi5g";
> > > +             led@27 {
> > > +                     label = "dgnd3700-v1:blue:wifi5g";
> > >                       gpios = <&pinctrl 27 1>;
> > >               };
> > >       };
> > > @@ -107,25 +107,25 @@
> > >               #address-cells = <1>;
> > >               #size-cells = <1>;
> > >
> > > -             cfe@0 {
> > > +             partition@0 {
> > >                       label = "CFE";
> > >                       reg = <0x000000 0x020000>;
> > >                       read-only;
> > >               };
> > >
> > > -             linux@20000 {
> > > +             partition@20000 {
> > >                       label = "linux";
> > >                       reg = <0x020000 0x1e20000>;
> > >                       compatible = "brcm,bcm963xx-imagetag";
> > >               };
> > >
> > > -             board_data@1e40000 {
> > > +             partition@1e40000 {
> > >                       label = "board_data";
> > >                       reg = <0x1e40000 0x1a0000>;
> > >                       read-only;
> > >               };
> > >
> > > -             nvram@1fe0000 {
> > > +             partition@1fe0000 {
> > >                       label = "nvram";
> > >                       reg = <0x1fe0000 0x20000>;
> > >               };
> > > @@ -156,22 +156,22 @@
> > >
> > >                       lan@1 {
> > >                               reg = <1>;
> > > -                             label = "lan1";
> > > +                             label = "lan4";
> > >                       };
> > >
> > >                       lan@2 {
> > >                               reg = <2>;
> > > -                             label = "lan2";
> > > +                             label = "lan3";
> > >                       };
> > >
> > >                       lan@3 {
> > >                               reg = <3>;
> > > -                             label = "lan3";
> > > +                             label = "lan2";
> > >                       };
> > >
> > >                       lan@4 {
> > >                               reg = <4>;
> > > -                             label = "lan4";
> > > +                             label = "lan1";
> > >                       };
> > >
> > >                       cpu@8 {
> > > diff --git a/target/linux/bcm63xx/patches-5.4/549-
> > > board_DGND3700v1_3800B.patch b/target/linux/bcm63xx/patches-
> 5.4/549-
> > > board_DGND3700v1_3800B.patch
> > > index 936aab115b..7569e9643e 100644
> > > --- a/target/linux/bcm63xx/patches-5.4/549-
> > > board_DGND3700v1_3800B.patch
> > > +++ b/target/linux/bcm63xx/patches-5.4/549-
> > > board_DGND3700v1_3800B.patch
> > > @@ -5,7 +5,7 @@
> > >   };
> > >
> > >  +static struct board_info __initdata board_DGND3700v1_3800B = {
> > > -+    .name                           = "DGND3700v1_3800B",
> > > ++    .name                           = "U12L144T01",
> > >  +    .expected_cpu_id                = 0x6368,
> > >  +
> > >  +    .has_pci                        = 1,
> > > --
> > > 2.26.2
> > >
> > >
> > >
> > >
> > >
> > > _______________________________________________
> > > openwrt-devel mailing list
> > > openwrt-devel@lists.openwrt.org
> > > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Daniel González Cabanelas May 24, 2020, 12:42 p.m. UTC | #4
Hi Adrian

El dom., 24 may. 2020 a las 13:15, <mail@adrianschmutzler.de> escribió:
>
> Hi Daniel,
>
> > -----Original Message-----
> > From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org]
> > On Behalf Of Daniel González Cabanelas
> > Sent: Sonntag, 24. Mai 2020 11:06
> > To: mail@adrianschmutzler.de
> > Cc: openwrt-devel@lists.openwrt.org; Álvaro Fernández Rojas
> > <noltari@gmail.com>
> > Subject: Re: [OpenWrt-Devel] [PATCH] bcm63xx: DGND3700v1: device tree
> > improvements
> >
> > Hi Adrian:
> >
> > El dom., 24 may. 2020 a las 1:01, <mail@adrianschmutzler.de> escribió:
> > >
> > > Hi,
> > >
> > > > -----Original Message-----
> > > > From: openwrt-devel [mailto:openwrt-devel-
> > bounces@lists.openwrt.org]
> > > > On Behalf Of Daniel González Cabanelas
> > > > Sent: Sonntag, 24. Mai 2020 00:24
> > > > To: openwrt-devel@lists.openwrt.org
> > > > Cc: noltari@gmail.com
> > > > Subject: [OpenWrt-Devel] [PATCH] bcm63xx: DGND3700v1: device tree
> > > > improvements
> > > >
> > > > Improve the device tree file and related board data for the
> > > > DGND3700v1/ DGND3800B router:
> > > >  - Improve LEDs definitions, use shorter names.
> > > >  - Make the board name more readable.
> > > >  - Fix the switch LAN labels, the order is reversed.
> > > >  - Use the real name of the board for the board name instead of device
> > > >    name.
> > > >  - Use standarized names for partition nodes and leds.
> > >
> > > This deals with several different issues at the same time. I'd prefer to have
> > it split up (e.g. separate board name change from LED changes and switch
> > changes).
> > >
> >
> > I can't see the benefit of flooding with commits on every negligible change.
> > These are just cosmetic changes which won't affect the behavior of the
> > device, and wont produce any unexpected bug, I've made the opportune
> > tests.
>
> Well, one of the possible bugs would be that LEDs won't work after sysupgrade, see below.
> I don't think that everything should be separate, but I don't like completely different things stuffed together.
>
Not really a bug, and the solution is easy from the point of view of
any user. It would be the reason for non backporting these changes,
not a good idea having a diferent behavior of leds on the same Openwrt
version. It's more intuitive for any user which decides to upgrade to
a new version and if something  isn't working, they think there is a
missconconfiguration issue somewhere. I'm almost sure 100% of users
make a reset to defaults on this case.

> >
> > > >
> > > > Signed-off-by: Daniel González Cabanelas <dgcbueu@gmail.com>
> > > > ---
> > > >  .../bcm63xx/base-files/etc/board.d/01_leds    | 12 ++--
> > > >  .../dts/bcm6368-netgear-dgnd3700-v1.dts       | 64 +++++++++----------
> > > >  .../549-board_DGND3700v1_3800B.patch          |  2 +-
> > > >  3 files changed, 39 insertions(+), 39 deletions(-)
> > > >
> > > > diff --git a/target/linux/bcm63xx/base-files/etc/board.d/01_leds
> > > > b/target/linux/bcm63xx/base-files/etc/board.d/01_leds
> > > > index 91d67f0c0b..6b82d9e952 100755
> > > > --- a/target/linux/bcm63xx/base-files/etc/board.d/01_leds
> > > > +++ b/target/linux/bcm63xx/base-files/etc/board.d/01_leds
> > > > @@ -66,12 +66,12 @@ inventel,livebox-1)
> > > >       ucidef_set_led_netdev "wlan0" "WIFI" "Livebox1:red:wifi" "wlan0"
> > > >       ;;
> > > >  netgear,dgnd3700-v1)
> > > > -     ucidef_set_led_netdev "lan" "LAN" "DGND3700v1_3800B:green:lan"
> > > > "eth0.1"
> > > > -     ucidef_set_led_netdev "wan" "WAN"
> > > > "DGND3700v1_3800B:green:inet" "eth0.2"
> > > > -     ucidef_set_led_netdev "wlan0" "WIFI2G"
> > > > "DGND3700v1_3800B:green:wifi2g" "wlan0"
> > > > -     ucidef_set_led_netdev "wlan1" "WIFI5G"
> > > > "DGND3700v1_3800B:blue:wifi5g" "wlan1"
> > > > -     ucidef_set_led_usbdev "usb1" "USB1"
> > > > "DGND3700v1_3800B:green:usb-back" "1-1"
> > > > -     ucidef_set_led_usbdev "usb2" "USB2"
> > > > "DGND3700v1_3800B:green:usb-front" "1-2"
> > > > +     ucidef_set_led_netdev "lan" "LAN" "$model:green:lan" "eth0.1"
> > > > +     ucidef_set_led_netdev "wan" "WAN" "$model:green:inet" "eth0.2"
> > > > +     ucidef_set_led_netdev "wlan0" "WIFI2G" "$model:green:wifi2g"
> > > > "wlan0"
> > > > +     ucidef_set_led_netdev "wlan1" "WIFI5G" "$model:blue:wifi5g"
> > > > "wlan1"
> > > > +     ucidef_set_led_usbdev "usb1" "USB1" "$model:green:usb-back"
> > > > + "1-
> > > > 1"
> > > > +     ucidef_set_led_usbdev "usb2" "USB2" "$model:green:usb-front"
> > > > + "1-
> > > > 2"
> > >
> > > Is there any way to do sysupgrade on these devices? If yes, you will need
> > migration of the names in /etc/config/system ...
> > >
> >
> > The sysupgrade works out of the box using the default_do_upgrade. Not
> > sure what I need to review, can you be more specific?
>
> On a newly installed device there won't be a problem. 01_leds will generate the LED entries in /etc/config/system on firstboot, and after that the names used there won't change anymore, even on upgrade. In contrast, the LED names in the device tree will change with every upgrade, so that this change results in the LED settings becoming broken without a reset of config files. This can be healed with a migration script, e.g.
> https://github.com/openwrt/openwrt/blob/master/target/linux/ramips/mt76x8/base-files/etc/uci-defaults/04_led_migration
>
> That's BTW the reason why we haven't changed these for consistency so far on this target.
>
It's nice having a script for making those migrations. It makes the
life easier for end users, but life harder for patching this stuff.

> >
> > > >       ;;
> > > >  netgear,dgnd3700-v2)
> > > >       ucidef_set_led_netdev "lan" "LAN" "$model:green:ethernet" "eth0"
> > > > diff --git
> > > > a/target/linux/bcm63xx/dts/bcm6368-netgear-dgnd3700-v1.dts
> > > > b/target/linux/bcm63xx/dts/bcm6368-netgear-dgnd3700-v1.dts
> > > > index 546b0b1f60..c17bb3a600 100644
> > > > --- a/target/linux/bcm63xx/dts/bcm6368-netgear-dgnd3700-v1.dts
> > > > +++ b/target/linux/bcm63xx/dts/bcm6368-netgear-dgnd3700-v1.dts
> > > > @@ -5,12 +5,12 @@
> > > >  #include <dt-bindings/input/input.h>
> > > >
> > > >  / {
> > > > -     model = "Netgear DGND3700v1/DGND3800B";
> > > > +     model = "Netgear DGND3700v1 / DGND3800B";
> > >
> > > I don't think this is really necessary ...
> > >
I forgot to comment this. The change makes the device model more
readable.. It's purely cosmetic and keeps coherency with the device
title used in the OpenWrt wiki.

> > > >       compatible = "netgear,dgnd3700-v1", "brcm,bcm6368";
> > > >
> > > >       aliases {
> > > >               led-boot = &led_power_green;
> > > > -             led-failsafe = &led_power_green;
> > > > +             led-failsafe = &led_power_red;
> > >
> > > This should be a separate commit again.
> > >
> >
> > One commit per line, on the same file, really?, again this is a minor change.
>
> Yes, but it's a non-cosmetic (with respect to its effect) behavior change that is not at all connected to the rest.

Might be better patching all boards, having multicolor power leds, at
the same time?

>
> Just imagine we want to backport this or the fixed port order to 19.07, but don't want to mess with LED names. Separate commits for separate topics make sense.
>
I can imagine it. But being realistic it won't ever happen.

> >
> > > >               led-running = &led_power_green;
> > > >               led-upgrade = &led_power_green;
> > > >       };
> > > > @@ -51,49 +51,49 @@
> > > >       leds {
> > > >               compatible = "gpio-leds";
> > > >
> > > > -             dsl_green {
> > > > -                     label = "DGND3700v1_3800B:green:dsl";
> > > > +             led@2 {
> > >
> > > I don't know whether this is different on bcm63xx, but based on what I'm
> > used to the old node name is preferred (dsl_green).
> > >
> >
> > Well, I'll never know what's the best way for naming a led node. I've taken
> > the partitions nodes as a reference, and the LEDs device tree documentation
> > also use this way as an example.
>
> Openwrt seems to generally use the name-based scheme (the one already there), and since there is no reason for changing that and it's unconnected to the rest of your changes, please just drop these changes (of course, only the node name, not the label changes).

I've really made this change to keep coherency with his brother
DGND3700v2, supported since about a week ago, also belonging to this
target.

Regards
>
> Best
>
> Adrian
>
> >
> > Regards
> >
> > > Best
> > >
> > > Adrian
> > >
> > > > +                     label = "dgnd3700-v1:green:dsl";
> > > >                       gpios = <&pinctrl 2 1>;
> > > >               };
> > > > -             inet_red {
> > > > -                     label = "DGND3700v1_3800B:red:inet";
> > > > +             led@4 {
> > > > +                     label = "dgnd3700-v1:red:inet";
> > > >                       gpios = <&pinctrl 4 1>;
> > > >               };
> > > > -             inet_green {
> > > > -                     label = "DGND3700v1_3800B:green:inet";
> > > > +             led@5 {
> > > > +                     label = "dgnd3700-v1:green:inet";
> > > >                       gpios = <&pinctrl 5 1>;
> > > >               };
> > > > -             wps_green {
> > > > -                     label = "DGND3700v1_3800B:green:wps";
> > > > +             led@11 {
> > > > +                     label = "dgnd3700-v1:green:wps";
> > > >                       gpios = <&pinctrl 11 1>;
> > > >               };
> > > > -             usbfront_green {
> > > > -                     label = "DGND3700v1_3800B:green:usb-front";
> > > > +             led@13 {
> > > > +                     label = "dgnd3700-v1:green:usb-front";
> > > >                       gpios = <&pinctrl 13 1>;
> > > >               };
> > > > -             usbback_green {
> > > > -                     label = "DGND3700v1_3800B:green:usb-back";
> > > > +             led@14 {
> > > > +                     label = "dgnd3700-v1:green:usb-back";
> > > >                       gpios = <&pinctrl 14 1>;
> > > >               };
> > > > -             power_red {
> > > > -                     label = "DGND3700v1_3800B:red:power";
> > > > +             led_power_red: led@22 {
> > > > +                     label = "dgnd3700-v1:red:power";
> > > >                       gpios = <&pinctrl 22 1>;
> > > >               };
> > > > -             lan_green {
> > > > -                     label = "DGND3700v1_3800B:green:lan";
> > > > +             led@23 {
> > > > +                     label = "dgnd3700-v1:green:lan";
> > > >                       gpios = <&pinctrl 23 1>;
> > > >               };
> > > > -             led_power_green: power_green {
> > > > -                     label = "DGND3700v1_3800B:green:power";
> > > > +             led_power_green: led@24 {
> > > > +                     label = "dgnd3700-v1:green:power";
> > > >                       gpios = <&pinctrl 24 1>;
> > > >                       default-state = "on";
> > > >               };
> > > > -             wifi2g_green {
> > > > -                     label = "DGND3700v1_3800B:green:wifi2g";
> > > > +             led@26 {
> > > > +                     label = "dgnd3700-v1:green:wifi2g";
> > > >                       gpios = <&pinctrl 26 1>;
> > > >               };
> > > > -             wifi5g_blue {
> > > > -                     label = "DGND3700v1_3800B:blue:wifi5g";
> > > > +             led@27 {
> > > > +                     label = "dgnd3700-v1:blue:wifi5g";
> > > >                       gpios = <&pinctrl 27 1>;
> > > >               };
> > > >       };
> > > > @@ -107,25 +107,25 @@
> > > >               #address-cells = <1>;
> > > >               #size-cells = <1>;
> > > >
> > > > -             cfe@0 {
> > > > +             partition@0 {
> > > >                       label = "CFE";
> > > >                       reg = <0x000000 0x020000>;
> > > >                       read-only;
> > > >               };
> > > >
> > > > -             linux@20000 {
> > > > +             partition@20000 {
> > > >                       label = "linux";
> > > >                       reg = <0x020000 0x1e20000>;
> > > >                       compatible = "brcm,bcm963xx-imagetag";
> > > >               };
> > > >
> > > > -             board_data@1e40000 {
> > > > +             partition@1e40000 {
> > > >                       label = "board_data";
> > > >                       reg = <0x1e40000 0x1a0000>;
> > > >                       read-only;
> > > >               };
> > > >
> > > > -             nvram@1fe0000 {
> > > > +             partition@1fe0000 {
> > > >                       label = "nvram";
> > > >                       reg = <0x1fe0000 0x20000>;
> > > >               };
> > > > @@ -156,22 +156,22 @@
> > > >
> > > >                       lan@1 {
> > > >                               reg = <1>;
> > > > -                             label = "lan1";
> > > > +                             label = "lan4";
> > > >                       };
> > > >
> > > >                       lan@2 {
> > > >                               reg = <2>;
> > > > -                             label = "lan2";
> > > > +                             label = "lan3";
> > > >                       };
> > > >
> > > >                       lan@3 {
> > > >                               reg = <3>;
> > > > -                             label = "lan3";
> > > > +                             label = "lan2";
> > > >                       };
> > > >
> > > >                       lan@4 {
> > > >                               reg = <4>;
> > > > -                             label = "lan4";
> > > > +                             label = "lan1";
> > > >                       };
> > > >
> > > >                       cpu@8 {
> > > > diff --git a/target/linux/bcm63xx/patches-5.4/549-
> > > > board_DGND3700v1_3800B.patch b/target/linux/bcm63xx/patches-
> > 5.4/549-
> > > > board_DGND3700v1_3800B.patch
> > > > index 936aab115b..7569e9643e 100644
> > > > --- a/target/linux/bcm63xx/patches-5.4/549-
> > > > board_DGND3700v1_3800B.patch
> > > > +++ b/target/linux/bcm63xx/patches-5.4/549-
> > > > board_DGND3700v1_3800B.patch
> > > > @@ -5,7 +5,7 @@
> > > >   };
> > > >
> > > >  +static struct board_info __initdata board_DGND3700v1_3800B = {
> > > > -+    .name                           = "DGND3700v1_3800B",
> > > > ++    .name                           = "U12L144T01",
> > > >  +    .expected_cpu_id                = 0x6368,
> > > >  +
> > > >  +    .has_pci                        = 1,
> > > > --
> > > > 2.26.2
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > _______________________________________________
> > > > openwrt-devel mailing list
> > > > openwrt-devel@lists.openwrt.org
> > > > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
> >
> > _______________________________________________
> > openwrt-devel mailing list
> > openwrt-devel@lists.openwrt.org
> > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
diff mbox series

Patch

diff --git a/target/linux/bcm63xx/base-files/etc/board.d/01_leds b/target/linux/bcm63xx/base-files/etc/board.d/01_leds
index 91d67f0c0b..6b82d9e952 100755
--- a/target/linux/bcm63xx/base-files/etc/board.d/01_leds
+++ b/target/linux/bcm63xx/base-files/etc/board.d/01_leds
@@ -66,12 +66,12 @@  inventel,livebox-1)
 	ucidef_set_led_netdev "wlan0" "WIFI" "Livebox1:red:wifi" "wlan0"
 	;;
 netgear,dgnd3700-v1)
-	ucidef_set_led_netdev "lan" "LAN" "DGND3700v1_3800B:green:lan" "eth0.1"
-	ucidef_set_led_netdev "wan" "WAN" "DGND3700v1_3800B:green:inet" "eth0.2"
-	ucidef_set_led_netdev "wlan0" "WIFI2G" "DGND3700v1_3800B:green:wifi2g" "wlan0"
-	ucidef_set_led_netdev "wlan1" "WIFI5G" "DGND3700v1_3800B:blue:wifi5g" "wlan1"
-	ucidef_set_led_usbdev "usb1" "USB1" "DGND3700v1_3800B:green:usb-back" "1-1"
-	ucidef_set_led_usbdev "usb2" "USB2" "DGND3700v1_3800B:green:usb-front" "1-2"
+	ucidef_set_led_netdev "lan" "LAN" "$model:green:lan" "eth0.1"
+	ucidef_set_led_netdev "wan" "WAN" "$model:green:inet" "eth0.2"
+	ucidef_set_led_netdev "wlan0" "WIFI2G" "$model:green:wifi2g" "wlan0"
+	ucidef_set_led_netdev "wlan1" "WIFI5G" "$model:blue:wifi5g" "wlan1"
+	ucidef_set_led_usbdev "usb1" "USB1" "$model:green:usb-back" "1-1"
+	ucidef_set_led_usbdev "usb2" "USB2" "$model:green:usb-front" "1-2"
 	;;
 netgear,dgnd3700-v2)
 	ucidef_set_led_netdev "lan" "LAN" "$model:green:ethernet" "eth0"
diff --git a/target/linux/bcm63xx/dts/bcm6368-netgear-dgnd3700-v1.dts b/target/linux/bcm63xx/dts/bcm6368-netgear-dgnd3700-v1.dts
index 546b0b1f60..c17bb3a600 100644
--- a/target/linux/bcm63xx/dts/bcm6368-netgear-dgnd3700-v1.dts
+++ b/target/linux/bcm63xx/dts/bcm6368-netgear-dgnd3700-v1.dts
@@ -5,12 +5,12 @@ 
 #include <dt-bindings/input/input.h>
 
 / {
-	model = "Netgear DGND3700v1/DGND3800B";
+	model = "Netgear DGND3700v1 / DGND3800B";
 	compatible = "netgear,dgnd3700-v1", "brcm,bcm6368";
 
 	aliases {
 		led-boot = &led_power_green;
-		led-failsafe = &led_power_green;
+		led-failsafe = &led_power_red;
 		led-running = &led_power_green;
 		led-upgrade = &led_power_green;
 	};
@@ -51,49 +51,49 @@ 
 	leds {
 		compatible = "gpio-leds";
 
-		dsl_green {
-			label = "DGND3700v1_3800B:green:dsl";
+		led@2 {
+			label = "dgnd3700-v1:green:dsl";
 			gpios = <&pinctrl 2 1>;
 		};
-		inet_red {
-			label = "DGND3700v1_3800B:red:inet";
+		led@4 {
+			label = "dgnd3700-v1:red:inet";
 			gpios = <&pinctrl 4 1>;
 		};
-		inet_green {
-			label = "DGND3700v1_3800B:green:inet";
+		led@5 {
+			label = "dgnd3700-v1:green:inet";
 			gpios = <&pinctrl 5 1>;
 		};
-		wps_green {
-			label = "DGND3700v1_3800B:green:wps";
+		led@11 {
+			label = "dgnd3700-v1:green:wps";
 			gpios = <&pinctrl 11 1>;
 		};
-		usbfront_green {
-			label = "DGND3700v1_3800B:green:usb-front";
+		led@13 {
+			label = "dgnd3700-v1:green:usb-front";
 			gpios = <&pinctrl 13 1>;
 		};
-		usbback_green {
-			label = "DGND3700v1_3800B:green:usb-back";
+		led@14 {
+			label = "dgnd3700-v1:green:usb-back";
 			gpios = <&pinctrl 14 1>;
 		};
-		power_red {
-			label = "DGND3700v1_3800B:red:power";
+		led_power_red: led@22 {
+			label = "dgnd3700-v1:red:power";
 			gpios = <&pinctrl 22 1>;
 		};
-		lan_green {
-			label = "DGND3700v1_3800B:green:lan";
+		led@23 {
+			label = "dgnd3700-v1:green:lan";
 			gpios = <&pinctrl 23 1>;
 		};
-		led_power_green: power_green {
-			label = "DGND3700v1_3800B:green:power";
+		led_power_green: led@24 {
+			label = "dgnd3700-v1:green:power";
 			gpios = <&pinctrl 24 1>;
 			default-state = "on";
 		};
-		wifi2g_green {
-			label = "DGND3700v1_3800B:green:wifi2g";
+		led@26 {
+			label = "dgnd3700-v1:green:wifi2g";
 			gpios = <&pinctrl 26 1>;
 		};
-		wifi5g_blue {
-			label = "DGND3700v1_3800B:blue:wifi5g";
+		led@27 {
+			label = "dgnd3700-v1:blue:wifi5g";
 			gpios = <&pinctrl 27 1>;
 		};
 	};
@@ -107,25 +107,25 @@ 
 		#address-cells = <1>;
 		#size-cells = <1>;
 
-		cfe@0 {
+		partition@0 {
 			label = "CFE";
 			reg = <0x000000 0x020000>;
 			read-only;
 		};
 
-		linux@20000 {
+		partition@20000 {
 			label = "linux";
 			reg = <0x020000 0x1e20000>;
 			compatible = "brcm,bcm963xx-imagetag";
 		};
 
-		board_data@1e40000 {
+		partition@1e40000 {
 			label = "board_data";
 			reg = <0x1e40000 0x1a0000>;
 			read-only;
 		};
 
-		nvram@1fe0000 {
+		partition@1fe0000 {
 			label = "nvram";
 			reg = <0x1fe0000 0x20000>;
 		};
@@ -156,22 +156,22 @@ 
 
 			lan@1 {
 				reg = <1>;
-				label = "lan1";
+				label = "lan4";
 			};
 
 			lan@2 {
 				reg = <2>;
-				label = "lan2";
+				label = "lan3";
 			};
 
 			lan@3 {
 				reg = <3>;
-				label = "lan3";
+				label = "lan2";
 			};
 
 			lan@4 {
 				reg = <4>;
-				label = "lan4";
+				label = "lan1";
 			};
 
 			cpu@8 {
diff --git a/target/linux/bcm63xx/patches-5.4/549-board_DGND3700v1_3800B.patch b/target/linux/bcm63xx/patches-5.4/549-board_DGND3700v1_3800B.patch
index 936aab115b..7569e9643e 100644
--- a/target/linux/bcm63xx/patches-5.4/549-board_DGND3700v1_3800B.patch
+++ b/target/linux/bcm63xx/patches-5.4/549-board_DGND3700v1_3800B.patch
@@ -5,7 +5,7 @@ 
  };
  
 +static struct board_info __initdata board_DGND3700v1_3800B = {
-+	.name				= "DGND3700v1_3800B",
++	.name				= "U12L144T01",
 +	.expected_cpu_id		= 0x6368,
 +
 +	.has_pci			= 1,