[OpenWrt-Devel] [PATCH v6 5/6] ar71xx: scan nand ubi partition for ath9k eeprom files
Yousong Zhou
yszhou4tech at gmail.com
Mon Dec 7 20:48:07 EST 2015
On 7 December 2015 at 23:55, Chris Blake <chrisrblake93 at gmail.com> wrote:
> Yousong,
>
> Sorry to come back so fast, but how would moving the skip to hexdump
> help the code in any way? For example, we are currently using:
>
> dd bs=1 skip=$offset count=6 if=/dev/$part 2>/dev/null | hexdump -v -n
> 6 -e '5/1 "%02x:" 1/1 "%02x"'
>
> If we move the skip to hexdump, then we would also need to remove the
> count from dd, which means the entire partition is then piped into
> hexdump which to me seems to be a lot less efficient than the current
> implementation above.
>
I used the following command before for fetching MAC addresses from
mtd devices. Not sure if it also works for ubi devices...
hexdump -v -n 6 -s 0x1fc00 -e '5/1 "%02x:" 1/1 "%02x""\n"' /dev/mtd2
P.S. please avoid top posting
Cheers,
yousong
> On Mon, Dec 7, 2015 at 9:42 AM, Yousong Zhou <yszhou4tech at gmail.com> wrote:
>>
>> Am 07.12.2015 22:12 schrieb "Chris Blake" <chrisrblake93 at gmail.com>:
>>>
>>> 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 <yszhou4tech at gmail.com>
>>> wrote:
>>> > Hello, Christian, a few nitpicks inline :)
>>> >
>>> > On 6 December 2015 at 06:48, Christian Lamparter
>>> > <chunkeey at googlemail.com> wrote:
>>> >> From: Chris R Blake <chrisrblake93 at gmail.com>
>>> >>
>>> >> 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 <chrisrblake93 at gmail.com>
>>> >> ---
>>> >> 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
>>> >> 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
_______________________________________________
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