Am 07.12.2015 22:12 schrieb "Chris Blake" <[email protected]>: > > Hey Yousong, > > looking at the hexdump comment, this function was actually based off > of the function mtd_get_mac_binary() which uses the same dd offset > logic as you can see in mtd_get_mac_binary_ubi(). If this were to be > changed, shouldn't the same change apply to mtd_get_mac_binary()? >
Yes, it should > I know this would require a new patch, but just want to make sure we > are on the same page. Previously, I thought mtd_get_mac_binary_ubi was a new function and not aware of its resemblence to mtd_get_mac_binary. A new patch is good if we think it can help improve the code even though it is mostly unrelated in the whole series. And a new nitpick crawls out below... > > Regards, > Chris Blake > > On Sun, Dec 6, 2015 at 10:05 PM, Yousong Zhou <[email protected]> wrote: > > Hello, Christian, a few nitpicks inline :) > > > > On 6 December 2015 at 06:48, Christian Lamparter > > <[email protected]> wrote: > >> From: Chris R Blake <[email protected]> > >> > >> The MR18 stores the ath9k eeprom values on the NAND. > >> This patch makes it possible to retrieve the images > >> from there. > >> > >> Signed-off-by: Chris R Blake <[email protected]> > >> --- > >> package/base-files/files/lib/functions/system.sh | 17 +++++++++++++++++ > >> .../base-files/etc/hotplug.d/firmware/10-ath9k-eeprom | 6 +++++- > >> 2 files changed, 22 insertions(+), 1 deletion(-) > >> > >> diff --git a/package/base-files/files/lib/functions/system.sh b/package/base-files/files/lib/functions/system.sh > >> index 8d75a5a..928a429 100644 > >> --- a/package/base-files/files/lib/functions/system.sh > >> +++ b/package/base-files/files/lib/functions/system.sh > >> @@ -41,6 +41,23 @@ mtd_get_mac_binary() { > >> dd bs=1 skip=$offset count=6 if=$part 2>/dev/null | hexdump -v -n 6 -e '5/1 "%02x:" 1/1 "%02x"' > >> } > >> > >> +mtd_get_mac_binary_ubi() { > >> + local mtdname="$1" > >> + local offset="$2" > >> + > >> + . /lib/upgrade/nand.sh > >> + > >> + local ubidev=$( nand_find_ubi $CI_UBIPART ) > >> + local part="$( nand_find_volume $ubidev $1 )" > > > > Quotes and padding whitespaces are not need here for consistency of code style. > > > >> + > >> + if [ -z "$part" ]; then > >> + echo "mtd_get_mac_binary: ubi partition $mtdname not found!" >&2 Here the error message should be reworded. yousong > >> + return > >> + fi > >> + > >> + dd bs=1 skip=$offset count=6 if=/dev/$part 2>/dev/null | hexdump -v -n 6 -e '5/1 "%02x:" 1/1 "%02x"' > > > > hexdump accepts an argument for offset with -s option > > > >> +} > >> + > >> mtd_get_part_size() { > >> local part_name=$1 > >> local first dev size erasesize name > >> diff --git a/target/linux/ar71xx/base-files/etc/hotplug.d/firmware/10-ath9k-eeprom b/target/linux/ar71xx/base-files/etc/hotplug.d/firmware/10-ath9k-eeprom > >> index b5f0588..7287809 100644 > >> --- a/target/linux/ar71xx/base-files/etc/hotplug.d/firmware/10-ath9k-eeprom > >> +++ b/target/linux/ar71xx/base-files/etc/hotplug.d/firmware/10-ath9k-eeprom > >> @@ -9,11 +9,14 @@ ath9k_eeprom_extract() { > >> local part=$1 > >> local offset=$2 > >> local count=$3 > >> + local ubidev=$( nand_find_ubi $CI_UBIPART ) > >> local mtd > >> > >> mtd=$(find_mtd_chardev $part) > >> [ -n "$mtd" ] || \ > >> - ath9k_eeprom_die "no mtd device found for partition $part" > >> + mtd="/dev/$(nand_find_volume $ubidev $part)" > >> + [ -n "$mtd" ] || \ > >> + ath9k_eeprom_die "no mtd device found for partition $part" > >> > > > > The indentation here can be misleading > > > >> dd if=$mtd of=/lib/firmware/$FIRMWARE bs=1 skip=$offset count=$count 2>/dev/null || \ > >> ath9k_eeprom_die "failed to extract from $mtd" > >> @@ -32,6 +35,7 @@ ath9k_patch_firmware_mac() { > >> . /lib/ar71xx.sh > >> . /lib/functions.sh > >> . /lib/functions/system.sh > >> +. /lib/upgrade/nand.sh > >> > > > > I suggest we move this part including the line "[ -e > > /lib/firmware/$FIRMWARE ] && exit 0" to top of this file. > > > > yousong > > > > > >> board=$(ar71xx_board_name) > >> > >> -- > >> 2.6.2 > >> _______________________________________________ > >> 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
_______________________________________________ openwrt-devel mailing list [email protected] https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
