[OpenWrt-Devel] [PATCH v6 5/6] ar71xx: scan nand ubi partition for ath9k eeprom files

Chris Blake chrisrblake93 at gmail.com
Wed Dec 9 09:21:26 EST 2015


On Mon, Dec 7, 2015 at 7:48 PM, Yousong Zhou <yszhou4tech at gmail.com> wrote:
> 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
>
>

Will do, and thanks for the advice. As for UBI I was able to get it
working, so expect all requested changes to be in the next patch
revision. (along with a patch to fix up mtd_get_mac_binary)

Thanks again!
- Chris Blake

>> 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