Hello, I have a few suggestions inline below.
On Friday, May 24, 2019 11:27:17 PM CEST Linus Walleij wrote: > The DNS-313 isn't the only special board so let's bite the > bullet and create a case ladder in preparation for DIR-685. > > Signed-off-by: Linus Walleij <[email protected]> > --- > .../lib/preinit/05_set_ether_mac_gemini | 30 ++++++++++++------- > 1 file changed, 19 insertions(+), 11 deletions(-) > > diff --git > a/target/linux/gemini/base-files/lib/preinit/05_set_ether_mac_gemini > b/target/linux/gemini/base-files/lib/preinit/05_set_ether_mac_gemini > index 1ce5c8067ef0..ebd3ae0f55c5 100644 > --- a/target/linux/gemini/base-files/lib/preinit/05_set_ether_mac_gemini > +++ b/target/linux/gemini/base-files/lib/preinit/05_set_ether_mac_gemini > @@ -1,6 +1,25 @@ > #!/bin/sh > > set_ether_mac() { > + > + . /lib/functions.sh > + > + case $(board_name) in > + dlink,dns-313) > + # The DNS-313 has a special field in its RedBoot > + # binary that we need to check > + CONFIG_PARTITION="$(grep "RedBoot" /proc/mtd | cut -d: -f1)" > + if [ ! -z $CONFIG_PARTITION ] ; then This looks familiar. From afar, this is almost like package/base-files/files/lib/functions.sh's find_mtd_part with an extra check. > + DEVID="$(dd if=/dev/mtdblock0 bs=1 skip=119508 count=7 > 2>/dev/null)" I think find_mtd_part's result from above would be the perfect input for dd if=... > + if [ "x$DEVID" = "xdns-313" ] ; then > + MAC1="$(dd if=/dev/mtdblock0 bs=1 skip=119540 > count=6 2>/dev/null | hexdump -n6 -e '/1 ":%02X"' | sed s/^://g)" Isn't this like package/base-files/files/lib/functions/system.sh's mtd_get_mac_binary function? What does . /lib/functions.sh . /lib/functions/system.sh echo $(mtd_get_mac_binary RedBoot 119540) produce? it should be the same MAC. if it is you could use the get_mac_binary with the find_mtd_part from above and get the mac this way. > + ifconfig eth0 hw ether $MAC1 2>/dev/null I guess while we are at it, why not change it to "ip link set dev eth0 address $MAC1" in case the ifconfig deprecation ever materializes. > + return 0 > + fi > + fi > + ;; > + esac > + > # Most devices have a standard "VCTL" partition > CONFIG_PARTITION="$(grep "VCTL" /proc/mtd | cut -d: -f1)" > if [ ! -z $CONFIG_PARTITION ] ; then > @@ -12,17 +31,6 @@ set_ether_mac() { > return 0 > fi > > - # The DNS-313 has a special field in its RedBoot > - # binary that we need to check > - CONFIG_PARTITION="$(grep "RedBoot" /proc/mtd | cut -d: -f1)" > - if [ ! -z $CONFIG_PARTITION ] ; then > - DEVID="$(dd if=/dev/mtdblock0 bs=1 skip=119508 count=7 > 2>/dev/null)" > - if [ "x$DEVID" = "xdns-313" ] ; then > - MAC1="$(dd if=/dev/mtdblock0 bs=1 skip=119540 count=6 > 2>/dev/null | hexdump -n6 -e '/1 ":%02X"' | sed s/^://g)" > - ifconfig eth0 hw ether $MAC1 2>/dev/null > - return 0 > - fi > - fi > } > > boot_hook_add preinit_main set_ether_mac > _______________________________________________ openwrt-devel mailing list [email protected] https://lists.openwrt.org/mailman/listinfo/openwrt-devel
