[OpenWrt-Devel] [PATCH 2/3] ramips: Add base-files for HiWiFi HC5x61 models
Piotr Dymacz
pepe2k at gmail.com
Mon Sep 28 12:57:29 EDT 2015
Hello,
2015-09-28 16:52 GMT+02:00 Comman Kang <kangxn at 163.com>:
> 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 at Hiwifi:~# __fac_mac=`strings /dev/mtd7 | grep 'fac_mac = ..:..:..:..:..:..'`
> root at Hiwifi:~# echo $__fac_mac
> Vfac_mac = D4:EE:07:25:6C:D6
> root at Hiwifi:~# lan_mac=`expr "$__fac_mac" : '.*\(..:..:..:..:..:..\)' | tr '[A-Z]' '[a-z]'`
> root at 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”<pepe2k at gmail.com> 写入:
>
>>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 <kangxn at 163.com>:
>>> 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 (rssnsj at gmail.com). I ported the patch to trunk and submitted it here with his approval.
>>>
>>> Signed-off-by: Xiaoning Kang <kangxn at 163.com>
>>>
>>>
>>>
>>> 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
>>> openwrt-devel at lists.openwrt.org
>>> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
>
>
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
More information about the openwrt-devel
mailing list