[OpenWrt-Devel] ath79: add support of Netgear WNR3800 (Ch)
diff mbox series

Message ID 1565200503-4224-1-git-send-email-hanipouspilot@gmail.com
State New
Headers show
Series
  • [OpenWrt-Devel] ath79: add support of Netgear WNR3800 (Ch)
Related show

Commit Message

Dmitry Tunin Aug. 7, 2019, 5:55 p.m. UTC
Add support for the ar71xx supported Netgear WNR3800 (Ch) to ath79.
The device is identical to WNR3800 except NETGEAR_BOARD_ID.

Signed-off-by: Dmitry Tunin <hanipouspilot@gmail.com>
---
 .../linux/ath79/base-files/etc/board.d/02_network  |  3 +-
 .../etc/hotplug.d/firmware/10-ath9k-eeprom         |  6 ++--
 .../linux/ath79/dts/ar7161_netgear_wndr3800ch.dts  | 36 ++++++++++++++++++++++
 target/linux/ath79/image/generic.mk                |  8 +++++
 4 files changed, 50 insertions(+), 3 deletions(-)
 create mode 100644 target/linux/ath79/dts/ar7161_netgear_wndr3800ch.dts

Comments

Adrian Schmutzler Aug. 7, 2019, 6:31 p.m. UTC | #1
Hi,

> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] On Behalf Of Dmitry Tunin
> Sent: Mittwoch, 7. August 2019 19:55
> To: openwrt-devel@lists.openwrt.org
> Cc: Dmitry Tunin <hanipouspilot@gmail.com>
> Subject: [OpenWrt-Devel] [PATCH] ath79: add support of Netgear WNR3800 (Ch)
> 
> Add support for the ar71xx supported Netgear WNR3800 (Ch) to ath79.
> The device is identical to WNR3800 except NETGEAR_BOARD_ID.

in commit title/description, you are using "WNR" instead of "WNDR" ...

Despite, from a quick research, it looks like the device is called "WNDR3800CH" most frequently.
I would update the name accordingly.

Note that you also have to update to DEVICE_VENDOR/DEVICE_MODEL syntax ...

Best

Adrian
Dmitry Tunin Aug. 7, 2019, 6:40 p.m. UTC | #2
> in commit title/description, you are using "WNR" instead of "WNDR" ...
Oops.

> Despite, from a quick research, it looks like the device is called "WNDR3800CH" most frequently.
> I would update the name accordingly.
I copied the ar71xx name, but I agree.

> Note that you also have to update to DEVICE_VENDOR/DEVICE_MODEL syntax ...
I don't quite get it. Where is this syntax?
Dmitry Tunin Aug. 7, 2019, 6:46 p.m. UTC | #3
Maybe DEVICE_VARIANT := CH is a better choice, since it is the same device.

ср, 7 авг. 2019 г. в 21:40, Dmitry Tunin <hanipouspilot@gmail.com>:
>
> > in commit title/description, you are using "WNR" instead of "WNDR" ...
> Oops.
>
> > Despite, from a quick research, it looks like the device is called "WNDR3800CH" most frequently.
> > I would update the name accordingly.
> I copied the ar71xx name, but I agree.
>
> > Note that you also have to update to DEVICE_VENDOR/DEVICE_MODEL syntax ...
> I don't quite get it. Where is this syntax?
Adrian Schmutzler Aug. 7, 2019, 6:47 p.m. UTC | #4
> > Note that you also have to update to DEVICE_VENDOR/DEVICE_MODEL syntax ...
> I don't quite get it. Where is this syntax?

Here:

> +define Device/netgear_wndr3800ch
> +  $(Device/netgear_wndr3800)
> +  DEVICE_TITLE := NETGEAR WNDR3800 (Ch)
> +  NETGEAR_BOARD_ID := WNDR3800CH
> +  SUPPORTED_DEVICES += wndr3800ch
> +endef
> +TARGET_DEVICES += netgear_wndr3800ch

Instead of DEVICE_TITLE, use DEVICE_VENDOR/DEVICE_MODEL.
In this particular case, DEVICE_VENDOR is already inherited from a parent definition, so you only need DEVICE_MODEL.

However, having looked at this another time:

Please do not inherit one device from another. Use common definitions to inherit from.
In this particular case, it will be easiest to just copy the definitions from wndr3800 (and thus inherit from wndr3x00). I don't think it's worth creating another shared definition because of the remaining 3 duplicate lines.

Best

Adrian
Adrian Schmutzler Aug. 7, 2019, 6:52 p.m. UTC | #5
> Maybe DEVICE_VARIANT := CH is a better choice, since it is the same device.

Well, is it a variant? If yes, CH implies Switzerland to me, which most probably is wrong.
Despite, if you decide to use CH as variant, you would have to introduce hyphens to device definition name and compatible, i.e. "netgear,wndr3800-ch".

If the CH stands for China, as I've also read when googleing, I would prefer having CN here. This will then very much depend upon whether the CH is used officially somewhere or whether it's just an invention of previous OpenWrt versions ... (Where in the latter case I'd actually change to CN as we did for other devices ...).

Best

Adrian
Dmitry Tunin Aug. 7, 2019, 6:59 p.m. UTC | #6
Sent v2.

ср, 7 авг. 2019 г. в 21:48, Adrian Schmutzler <mail@adrianschmutzler.de>:
>
> > > Note that you also have to update to DEVICE_VENDOR/DEVICE_MODEL syntax ...
> > I don't quite get it. Where is this syntax?
>
> Here:
>
> > +define Device/netgear_wndr3800ch
> > +  $(Device/netgear_wndr3800)
> > +  DEVICE_TITLE := NETGEAR WNDR3800 (Ch)
> > +  NETGEAR_BOARD_ID := WNDR3800CH
> > +  SUPPORTED_DEVICES += wndr3800ch
> > +endef
> > +TARGET_DEVICES += netgear_wndr3800ch
>
> Instead of DEVICE_TITLE, use DEVICE_VENDOR/DEVICE_MODEL.
> In this particular case, DEVICE_VENDOR is already inherited from a parent definition, so you only need DEVICE_MODEL.
>
> However, having looked at this another time:
>
> Please do not inherit one device from another. Use common definitions to inherit from.
> In this particular case, it will be easiest to just copy the definitions from wndr3800 (and thus inherit from wndr3x00). I don't think it's worth creating another shared definition because of the remaining 3 duplicate lines.
>
> Best
>
> Adrian
Dmitry Tunin Aug. 7, 2019, 7:01 p.m. UTC | #7
> If the CH stands for China, as I've also read when googleing, I would prefer having CN here. This will then very much depend upon whether the CH is used officially somewhere or whether it's just an invention of previous OpenWrt versions ... (Where in the latter case I'd actually change to CN as we did for other devices ...).

CH is used in BOARD_ID.
Adrian Schmutzler Aug. 7, 2019, 7:07 p.m. UTC | #8
> CH is used in BOARD_ID.

Ah, I see. I'm fine with the solution in v2.

You still have one WNR in the commit description and you can remove the DEVICE_VENDOR, as it is still inherited.

Best

Adrian
Dmitry Tunin Aug. 7, 2019, 7:11 p.m. UTC | #9
> You still have one WNR in the commit description and you can remove the DEVICE_VENDOR, as it is still inherited.
It looks like all the file should be changed to VENDOR/MODEL, but you
are correct.
Dmitry Tunin Aug. 7, 2019, 7:27 p.m. UTC | #10
With VENDOR/MODEL it doesn't appear in menuconfig.
So v1 with WNDR fix should work.

ср, 7 авг. 2019 г. в 22:11, Dmitry Tunin <hanipouspilot@gmail.com>:
>
> > You still have one WNR in the commit description and you can remove the DEVICE_VENDOR, as it is still inherited.
> It looks like all the file should be changed to VENDOR/MODEL, but you
> are correct.
Adrian Schmutzler Aug. 7, 2019, 7:56 p.m. UTC | #11
Are you using recent master?

DEVICE_MODEL is the way to go now.

> -----Original Message-----
> From: Dmitry Tunin [mailto:hanipouspilot@gmail.com]
> Sent: Mittwoch, 7. August 2019 21:27
> To: Adrian Schmutzler <mail@adrianschmutzler.de>
> Cc: OpenWrt Development List <openwrt-devel@lists.openwrt.org>
> Subject: Re: [OpenWrt-Devel] [PATCH] ath79: add support of Netgear
> WNR3800 (Ch)
> 
> With VENDOR/MODEL it doesn't appear in menuconfig.
> So v1 with WNDR fix should work.
> 
> ср, 7 авг. 2019 г. в 22:11, Dmitry Tunin <hanipouspilot@gmail.com>:
> >
> > > You still have one WNR in the commit description and you can remove
> the DEVICE_VENDOR, as it is still inherited.
> > It looks like all the file should be changed to VENDOR/MODEL, but you
> > are correct.
Dmitry Tunin Aug. 7, 2019, 7:58 p.m. UTC | #12
> Are you using recent master?
>
> DEVICE_MODEL is the way to go now.
Yes, I do.
Dmitry Tunin Aug. 7, 2019, 8:01 p.m. UTC | #13
Oops again. I didn't pull master for a while, because was playing with 19.07.

ср, 7 авг. 2019 г. в 22:58, Dmitry Tunin <hanipouspilot@gmail.com>:
>
> > Are you using recent master?
> >
> > DEVICE_MODEL is the way to go now.
> Yes, I do.

Patch
diff mbox series

diff --git a/target/linux/ath79/base-files/etc/board.d/02_network b/target/linux/ath79/base-files/etc/board.d/02_network
index 84c83ff..fd66602 100755
--- a/target/linux/ath79/base-files/etc/board.d/02_network
+++ b/target/linux/ath79/base-files/etc/board.d/02_network
@@ -157,7 +157,8 @@  ath79_setup_interfaces()
 		;;
 	netgear,wndr3700|\
 	netgear,wndr3700v2|\
-	netgear,wndr3800)
+	netgear,wndr3800|\
+	netgear,wndr3800ch)
 		ucidef_set_interfaces_lan_wan "eth0" "eth1"
 		ucidef_add_switch "switch0" \
 			"0:lan:4" "1:lan:3" "2:lan:2" "3:lan:1" "5u@eth0"
diff --git a/target/linux/ath79/base-files/etc/hotplug.d/firmware/10-ath9k-eeprom b/target/linux/ath79/base-files/etc/hotplug.d/firmware/10-ath9k-eeprom
index e6b6d2f..2e5f455 100644
--- a/target/linux/ath79/base-files/etc/hotplug.d/firmware/10-ath9k-eeprom
+++ b/target/linux/ath79/base-files/etc/hotplug.d/firmware/10-ath9k-eeprom
@@ -200,7 +200,8 @@  case "$FIRMWARE" in
 	buffalo,wzr-hp-ag300h|\
 	netgear,wndr3700|\
 	netgear,wndr3700v2|\
-	netgear,wndr3800)
+	netgear,wndr3800|\
+	netgear,wndr3800ch)
 		ath9k_eeprom_extract "art" 4096 3768
 		;;
 	dlink,dir-825-b1)
@@ -217,7 +218,8 @@  case "$FIRMWARE" in
 	buffalo,wzr-hp-ag300h|\
 	netgear,wndr3700|\
 	netgear,wndr3700v2|\
-	netgear,wndr3800)
+	netgear,wndr3800|\
+	netgear,wndr3800ch)
 		ath9k_eeprom_extract "art" 20480 3768
 		;;
 	dlink,dir-825-b1)
diff --git a/target/linux/ath79/dts/ar7161_netgear_wndr3800ch.dts b/target/linux/ath79/dts/ar7161_netgear_wndr3800ch.dts
new file mode 100644
index 0000000..693b897
--- /dev/null
+++ b/target/linux/ath79/dts/ar7161_netgear_wndr3800ch.dts
@@ -0,0 +1,36 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/dts-v1/;
+
+#include "ar7161_netgear_wndr3700.dtsi"
+
+/ {
+	compatible = "netgear,wndr3800ch", "qca,ar7161";
+	model = "Netgear WNDR3800 (Ch)";
+};
+
+&partitions {
+	partition@0 {
+		label = "u-boot";
+		reg = <0x000000 0x050000>;
+		read-only;
+	};
+
+	partition@50000 {
+		label = "u-boot-env";
+		reg = <0x050000 0x020000>;
+		read-only;
+	};
+
+	partition@70000 {
+		label = "firmware";
+		reg = <0x070000 0xf80000>;
+		compatible = "netgear,uimage";
+	};
+
+	art: partition@ff0000 {
+		label = "art";
+		reg = <0xff0000 0x010000>;
+		read-only;
+	};
+};
+
diff --git a/target/linux/ath79/image/generic.mk b/target/linux/ath79/image/generic.mk
index d5f67b8..e163151 100644
--- a/target/linux/ath79/image/generic.mk
+++ b/target/linux/ath79/image/generic.mk
@@ -647,6 +647,14 @@  define Device/netgear_wndr3800
 endef
 TARGET_DEVICES += netgear_wndr3800
 
+define Device/netgear_wndr3800ch
+  $(Device/netgear_wndr3800)
+  DEVICE_TITLE := NETGEAR WNDR3800 (Ch)
+  NETGEAR_BOARD_ID := WNDR3800CH
+  SUPPORTED_DEVICES += wndr3800ch
+endef
+TARGET_DEVICES += netgear_wndr3800ch
+
 define Device/phicomm_k2t
   ATH_SOC := qca9563
   DEVICE_TITLE := Phicomm K2T