[OpenWrt-Devel] [PATCH v2 2/2] ath79: add support for WD My Net Wi-Fi Range Extender

Christian Lamparter chunkeey at gmail.com
Fri Aug 17 19:05:40 EDT 2018 

On Friday, August 17, 2018 11:07:23 PM CEST Mathias Kresin wrote:
> Hey Christian,
> 
> I found something to nitpick about. Find my comments inline.
Thank you for the review, I'll wait a until early next week, 
maybe Jonas has a comment on the cybertan-trx too.

> [...]
> > 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 73f350cae2..065f2b90bf 100755
> > --- a/target/linux/ath79/base-files/etc/board.d/01_leds
> > +++ b/target/linux/ath79/base-files/etc/board.d/01_leds
> > @@ -88,6 +88,13 @@ tplink,tl-wr841-v11)
> >   	ucidef_set_led_switch "lan3" "LAN3" "tp-link:green:lan3" "switch0" "0x04"
> >   	ucidef_set_led_switch "lan4" "LAN4" "tp-link:green:lan4" "switch0" "0x02"
> >   	;;
> > +wd,mynet-wifi-rangeextender)
> > +	ucidef_set_led_netdev "lan" "LAN" "$boardname:green:lan" "eth0"
> > +	ucidef_set_rssimon "wlan0" "200000" "1"
> > +	ucidef_set_led_rssi "rssilow" "RSSILOW" "$boardname:rssi-low" "wlan0" "1" "40" "0" "6"
> > +	ucidef_set_led_rssi "rssimedium" "RSSIMED" "$boardname:blue:rssi-med" "wlan0" "30" "80" "-29" "5"
> > +	ucidef_set_led_rssi "rssihigh" "RSSIMAX" "$boardname:blue:rssi-max" "wlan0" "70" "100" "-69" "8"
> 
> I'm quite sure the last two parameters are not really used on your 
> boards. These are used to change the brightness of pwm leds. According 
> to your dts it looks like the rssi leds are "normal" ones.
> 
> I'm not sure about the first two parameters either. They define the 
> range for which the led should be bright. Where the min is 1% and the 
> max is 100%. I would expect to see some kind of stacking used:
> 
> low: 1 to 100%
> med: 33 to 100%
> max: 66 to 100%

this make sense, I'll make adjustments to the rules I copied & pasted
from the ar71xx.

> @@ -21,7 +21,8 @@ ath79_setup_interfaces()
> > tplink,tl-wr703n|\
> > ubnt,unifiac-lite|\
> > ubnt,unifiac-mesh|\
> > -       ubnt,unifi)
> > +       ubnt,unifi|\
> > +       wd,mynet-rangeextender)
> > ucidef_set_interface_lan "eth0"
Oops. wd,mynet-rangeextender, should have been wd,mynet-wifi-rangeextender

> > 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 f668a82fa2..d158496418 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
> > @@ -137,6 +137,11 @@ case "$FIRMWARE" in
> >   	ubnt,unifi)
> >   		ath9k_eeprom_extract "art" 4096 2048
> >   		;;
> > +	wd,mynet-wifi-rangeextender)
> > +		ath9k_eeprom_extract "art" 4096 4096
> > +		mac=$(nvram get wl0_hwaddr)
> > +		[ -n "$mac" ] && ath9k_patch_fw_mac_crc "$mac" 2
> 
> Any reason why we can't assume that we got a mac address? Without having 
> much though about it, I would prefer to see the sanity check in 
> ath9k_patch_fw_mac_crc instead (param length, all params set or 
> something like that).
Turns out ath9k_patch_fw_mac_crc() already checks the mac.
So yes, this extra check can go away.

> > diff --git a/target/linux/ath79/dts/ar7370_wd_mynet-wifi-rangeextender.dts b/target/linux/ath79/dts/ar7370_wd_mynet-wifi-rangeextender.dts
> > new file mode 100644
> > index 0000000000..8c2a4b07cb
> > --- /dev/null
> > +++ b/target/linux/ath79/dts/ar7370_wd_mynet-wifi-rangeextender.dts
> > @@ -0,0 +1,174 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> > +/dts-v1/;
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/input/input.h>
> > +
> > +#include "ar9344.dtsi"
> > +
> > +/ {
> > +	model = "Western Digital My Net Wi-Fi Range Extender";
> > +	compatible = "wd,mynet-wifi-rangeextender", "qca,ar7370", "qca,ar9344";
> 
> Does it really make sense to introduce an ar7370 compatible here. 
> According to my google foo, the AR7370 is a AR9344 without wifi.
> I would expect they are identical beside the wifi block.
True, while the chip under the shield says AR7370 [0], the kernel identifies
in /proc/cpuinfo as
|system type             : Atheros AR9344 rev 2

no problem, I change it back.

About the integrated wifi:

I do remember that the integrated wifi core in this device can be enabled.
However without any matching caldata or FEM and antenna the only signals it
will pick up are those from close by. It could be that the AR7370 really is
a lower binned AR9344. But let's leave it at that.

> > +
> > +	chosen {
> > +		bootargs = "console=ttyS0,115200n8";
> > +	};
> > +
> > +	aliases {
> > +		led-boot = &power;
> > +		led-failsafe = &power;
> > +		led-running = &power;
> > +		led-upgrade = &power;
> > +	};
> > +
> > + [...]
> > +&pcie {
> > +	status = "okay";
> > +
> > +	ath9k: wifi at 0,0 {
> > +		compatible = "pci168c,0030";
> > +		reg = <0x0000 0 0 0 0>;
> > +		qca,no-eeprom;
> > +		/* wifi MAC is stored in nvram */
> > +	};
> > +};
> > +
> > +&mdio0 {
> > +	status = "okay";
> > +
> > +	phy-mask = <0x10>;
> > +
> > +	phy4: ethernet-phy at 4 {
> > +		reg = <4>;
> > +	};
> > +};
> > +
> > +&eth0 {
> > +	status = "okay";
> > +
> > +	pll-data = <0x0e000000 0x3c000101 0x3c001313>;
> > +
> > +	/* ethernet MAC is stored in nvram */
> > +	phy-mode = "rgmii";
> > +	phy-handle = <&phy4>;
> > +
> > +	gmac-config {
> > +		device = <&gmac>;
> > +		rgmii-gmac0 = <1>;
> > +		rxd-delay = <3>;
> > +		rxdv-delay = <3>;
> > +	};
> > +};
> > +
> > +&mdio1 {
> > +	status = "disabled";
> > +};
> > diff --git a/target/linux/ath79/image/generic.mk b/target/linux/ath79/image/generic.mk
> > index 5c4079f23b..11102fbc78 100644
> > --- a/target/linux/ath79/image/generic.mk
> > +++ b/target/linux/ath79/image/generic.mk
> > @@ -1,5 +1,7 @@
> >   include ./common-netgear.mk
> >   
> > +DEVICE_VARS += ADDPATTERN_ID ADDPATTERN_VERSION
> > +
> >   define Device/avm_fritz300e
> >     ATH_SOC := ar7242
> >     DEVICE_TITLE := AVM FRITZ!WLAN Repeater 300E
> > @@ -213,3 +215,29 @@ define Device/phicomm_k2t
> >     DEVICE_PACKAGES := kmod-leds-reset kmod-ath10k ath10k-firmware-qca9888
> >   endef
> >   TARGET_DEVICES += phicomm_k2t
> > +
> > +define Build/cybertan-trx
> > +	@echo -n '' > $@-empty.bin
> > +	-$(STAGING_DIR_HOST)/bin/trx -o $@.new \
> > +		-f $(IMAGE_KERNEL) -F $@-empty.bin \
> > +		-x 32 -a 0x10000 -x -32 -f $@
> > +	-mv "$@.new" "$@"
> > +	-rm $@-empty.bin
> > +endef
> > +
> > +define Build/addpattern
> > +	-$(STAGING_DIR_HOST)/bin/addpattern -B $(ADDPATTERN_ID) -v v$(ADDPATTERN_VERSION) -i $@ -o $@.new
> > +	-mv "$@.new" "$@"
> > +endef
> 
> AFAIK the common pattern is add build commands prior to board defines. 
> Please do it here as well (rather looks like some rebase foo anyway)
Alright, done. 

Regards,
Christian

[0] <https://wiki.openwrt.org/_detail/media/wd/wd_mynet_wifi_range_extender_soc-and-ram.jpg?id=toh%3Awd%3Arext>




_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

More information about the openwrt-devel mailing list