Hello, 2015-09-28 16:52 GMT+02:00 Comman Kang <[email protected]>: > Mostly done, except this one > >>+ hiwifi-hc5*61) >>+ __fac_mac=`strings /dev/mtd7 | grep 'fac_mac = >>..:..:..:..:..:..'` >>+ lan_mac=`expr "$__fac_mac" : '.*\(..:..:..:..:..:..\)' | tr >>'[A-Z]' '[a-z]'` > > Is that really needed? > >>+ [ -n "$lan_mac" ] || lan_mac=$(cat >>/sys/class/net/eth0/address) >>+ wan_mac=$(macaddr_add "$lan_mac" 1) >>+ ;; > > > > That is needed, here is the result on my router. > > > root@Hiwifi:~# __fac_mac=`strings /dev/mtd7 | grep 'fac_mac = > ..:..:..:..:..:..'` > root@Hiwifi:~# echo $__fac_mac > Vfac_mac = D4:EE:07:25:6C:D6 > root@Hiwifi:~# lan_mac=`expr "$__fac_mac" : '.*\(..:..:..:..:..:..\)' | tr > '[A-Z]' '[a-z]'` > root@Hiwifi:~# echo $lan_mac > d4:ee:07:25:6c:d6 > > > So that line is needed, __fac_mac itself is not a valid mac
Maybe I wasn't too precise - my comment was about whole part with MAC reading from mtd7. I saw in all three of your dts files: "mtd-mac-address = <&factory 0x4>;". So, is the "lan_mac=$(cat /sys/class/net/eth0/address)" not enough here? Are those addresses different? What's more, I really don't like hard coded mtd number here. Maybe you could make it more clean and universal, using helper functions "find_mtd_index" and "mtd_get_mac_ascii"? > V2 is uploaded, please take a look. Thanks again~ Thanks for all fixes! I will take a look at v2 and send my comments, if any, soon. Cheers, Piotr > > 在 15/9/28 下午7:48，“Piotr Dymacz”<[email protected]> 写入: > >>Hello, >> >>Please, see my comments inline, below. >> >>PS. Sorry for being so pedantic :) >> >>Cheers, >>Piotr >> >>2015-09-28 12:46 GMT+02:00 Comman Kang <[email protected]>: >>> HiWiFi HC5661/5761/5861 models are manufactured by http://www.hiwifi.com. >>> These models have similar hardware specs(MT7620A + 128M DDR2 + 16M flash). >>> This patch adds support for them. >>> >>> The original author is Justin Liu ([email protected]). I ported the patch to >>> trunk and submitted it here with his approval. >>> >>> Signed-off-by: Xiaoning Kang <[email protected]> >>> >>> >>> >>> diff --git a/target/linux/ramips/base-files/etc/board.d/01_leds >>> b/target/linux/ramips/base-files/etc/board.d/01_leds >>> index a9959e3..512fdc1 100755 >>> --- a/target/linux/ramips/base-files/etc/board.d/01_leds >>> +++ b/target/linux/ramips/base-files/etc/board.d/01_leds >>> @@ -137,6 +137,24 @@ hg255d) >>> set_usb_led "$board:green:usb" >>> ucidef_set_led_interface "lan" "$board:green:internet" >>> ;; >>> +hiwifi-hc5661) >>> + ucidef_set_led_default "system" "system" "$board:blue:system" "1" >>> + ucidef_set_led_netdev "internet" "internet" "$board:blue:internet" >>> "eth0.2" >>> + set_wifi_led "$board:blue:wlan-2p4" >>> + ;; >>> +hiwifi-hc5761) >>> + ucidef_set_led_default "system" "system" "$board:blue:system" "1" >>> + ucidef_set_led_netdev "internet" "internet" "$board:blue:internet" >>> "eth0.2" >>> + set_wifi_led "$board:blue:wlan-2p4" >>> + ucidef_set_led_netdev "wifi5g" "wifi5g" "$board:blue:wlan-5p" "rai0" >>> + ;; >>> +hiwifi-hc5861) >>> + ucidef_set_led_default "system" "system" "$board:blue:system" "1" >>> + ucidef_set_led_netdev "internet" "internet" "$board:blue:internet" >>> "eth0.2" >>> + set_wifi_led "$board:blue:wlan-2p4" >> >>Why not something like "wlan2g", "wlan5g" like in other boards (not in >>ramips target, but take a look for example at ar71xx)? >> >>> + ucidef_set_led_netdev "wifi5g" "wifi5g" "$board:blue:wlan-5p" "rai0" >>> + ucidef_set_led_default "turbo" "turbo" "$board:blue:turbo" "0" >>> + ;; >> >>General convention is to use only model name as board name. >>So, it would be better to use "hc5661" instead of "hiwifi-hc5661" etc. >> >>> hpm) >>> ucidef_set_led_default "power" "POWER" "$board:orange:power" "1" >>> ucidef_set_led_netdev "eth" "ETH" "$board:green:eth" "eth0" >>> diff --git a/target/linux/ramips/base-files/etc/board.d/02_network >>> b/target/linux/ramips/base-files/etc/board.d/02_network >>> index 75cccae..33c0b35 100755 >>> --- a/target/linux/ramips/base-files/etc/board.d/02_network >>> +++ b/target/linux/ramips/base-files/etc/board.d/02_network >>> @@ -170,6 +170,12 @@ ramips_setup_interfaces() >>> ucidef_add_switch_vlan "switch1" "1" "0 1 2 3 6t" >>> ucidef_add_switch_vlan "switch1" "2" "4 6t" >>> ;; >>> + hiwifi-hc5*61) >>> + ucidef_set_interfaces_lan_wan "eth0.1" "eth0.2" >>> + ucidef_add_switch "switch0" "1" "1" >>> + ucidef_add_switch_vlan "switch0" "1" "1 2 3 4 5 6t" >>> + ucidef_add_switch_vlan "switch0" "2" "0 6t" >>> + ;; >> >>There is other board with the same definition: >> >>y1s) >> ucidef_set_interfaces_lan_wan "eth0.1" "eth0.2" >> ucidef_add_switch "switch0" "1" "1" >> ucidef_add_switch_vlan "switch0" "1" "1 2 3 4 5 6t" >> ucidef_add_switch_vlan "switch0" "2" "0 6t" >> ;; >> >>Please, combine them together and reorder (keep all boards in >>alphabetical order). >> >>> m2m) >>> ucidef_add_switch "switch0" "4" >>> ucidef_set_interface_lan "eth0" >>> @@ -293,6 +299,12 @@ ramips_setup_macs() >>> e1700) >>> wan_mac=$(mtd_get_mac_ascii config WAN_MAC_ADDR) >>> ;; >>> + hiwifi-hc5*61) >>> + __fac_mac=`strings /dev/mtd7 | grep 'fac_mac = >>> ..:..:..:..:..:..'` >>> + lan_mac=`expr "$__fac_mac" : '.*\(..:..:..:..:..:..\)' | tr >>> '[A-Z]' '[a-z]'` >> >>Is that really needed? >> >>> + [ -n "$lan_mac" ] || lan_mac=$(cat >>> /sys/class/net/eth0/address) >>> + wan_mac=$(macaddr_add "$lan_mac" 1) >>> + ;; >>> ht-tm02) >>> lan_mac=$(cat /sys/class/net/eth0/address) >>> ;; >>> diff --git a/target/linux/ramips/base-files/etc/diag.sh >>> b/target/linux/ramips/base-files/etc/diag.sh >>> index 7fc6f29..867b0fd 100644 >>> --- a/target/linux/ramips/base-files/etc/diag.sh >>> +++ b/target/linux/ramips/base-files/etc/diag.sh >>> @@ -94,6 +94,9 @@ get_status_led() { >>> y1s) >>> status_led="$board:blue:power" >>> ;; >>> + hiwifi-hc5*61) >>> + status_led="$board:blue:system" >>> + ;; >> >>There are other boards with the same status_led definition: >> >>mpr-a1|\ >>mpr-a2) >> set_wifi_led "$board:blue:system" >> ;; >> >>Please, combine them together and reorder (keep all boards in >>alphabetical order). >> >>> db-wrt01|\ >>> esr-9753) >>> status_led="$board:orange:power" >>> diff --git a/target/linux/ramips/base-files/lib/ramips.sh >>> b/target/linux/ramips/base-files/lib/ramips.sh >>> index d242235..a11d62e 100755 >>> --- a/target/linux/ramips/base-files/lib/ramips.sh >>> +++ b/target/linux/ramips/base-files/lib/ramips.sh >>> @@ -169,6 +169,15 @@ ramips_board_detect() { >>> *"FreeStation5") >>> name="freestation5" >>> ;; >>> + *"HiWiFi HC5661") >>> + name="hiwifi-hc5661" >>> + ;; >>> + *"HiWiFi HC5761") >>> + name="hiwifi-hc5761" >>> + ;; >>> + *"HiWiFi HC5861") >>> + name="hiwifi-hc5861" >>> + ;; >> >>Please, use only model names - it's general convention. >>There is no reason to use whole "manufacturer + model" string. >> >>> *"HG255D") >>> name="hg255d" >>> ;; >>> diff --git a/target/linux/ramips/base-files/lib/upgrade/platform.sh >>> b/target/linux/ramips/base-files/lib/upgrade/platform.sh >>> index 2f6c624..0dc051b 100755 >>> --- a/target/linux/ramips/base-files/lib/upgrade/platform.sh >>> +++ b/target/linux/ramips/base-files/lib/upgrade/platform.sh >>> @@ -56,6 +56,7 @@ platform_check_image() { >>> fonera20n|\ >>> freestation5|\ >>> hg255d|\ >>> + hiwifi-hc5*61 |\ >> >>Please, use "|\" instead of " |\". >> >>> hlk-rm04|\ >>> hpm|\ >>> ht-tm02|\ >>> _______________________________________________ >>> openwrt-devel mailing list >>> [email protected] >>> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel > > _______________________________________________ openwrt-devel mailing list [email protected] https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
