<div dir="ltr">Hi - thanks for review; reply inline<br><div class="gmail_extra"><br><div class="gmail_quote">On 9 February 2015 at 20:52, Florian Fainelli <span dir="ltr"><<a href="mailto:florian@openwrt.org" target="_blank">florian@openwrt.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 09/02/15 08:29, Will Sheppard wrote:<br>
> Patchset to essentially add custom TRX header to all firmware produced.<br>
><br>
> This is most useful for the Belkin routers from my experience. I'm not<br>
> how other trx based firmwares modify the header for their own purposes.<br>
><br>
> This is applied across the board so that the kernel, trx tools and mtd<br>
> tools are all compiled with the specified header.<br>
><br>
> I have modified the kernel specifically for the brcm47xx based boards to<br>
> ensure the mtd parser looks for the correct magic. NOTE: This function will,<br>
> for other boards, fail.<br>
<br>
</span>I don't think this is desirable at all to have, for many reasons:<br>
<br>
- how can an use figure out the proper TRX_MAGIC value he/she should use?<br></blockquote><div><br></div><div>There's simply no hard-and-fast rule for this - just looking at a factory binary might not help - it worked for me. <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
- this is extremely error prone and can result, as you mention in<br>
failing to work with existing devices with well-defined TRX headers<br></blockquote><div><br></div><div>Error prone inasmuch as it could be the wrong value and thus brick the router? ( I guess I'm always thinking from a "have a serial port" perspective ) </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
- how can we get some level of reproducibility for both the kernel and<br>
firmware tools builds?<br>
<br>
I would really encourage you to extend the existing code to take an<br>
arbitrary list of TRX magics, such that you can have maybe a single file<br>
to modify and both "mtd" and the kernel can use that list of magic<br>
values accordingly.<br></blockquote><div><br></div><div>I agree. As the mtd utility looks for this value though, it would have to know what router it was being used on - i'm not sure that is available as a value in user-space is it? </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Out of curiosity, how many different TRX magic values should we have to<br>
support with or without your patches?<br></blockquote><div><br></div><div>I know that Belkin seem to use different magics i.e. not HDR0, I'm not sure about other manufacturers. But presumably these values could be added as they become known.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
><br>
> Will Sheppard (7):<br>
>   config: Add option to specify custom TRX header<br>
>   mtd: Add custom trx magic option to mtd tool<br>
>   mtd: Fix: Use TRX_MAGIC define not hard-coded number<br>
>   trx: Add custom TRX option to trx firmware tool<br>
>   mtd: Add debug showing header magic in use<br>
>   kernel: bcm47xx: Add custom TRX header option to kernel<br>
>   mtd: Fix makefile to work with quilt as per wiki<br>
><br>
>  config/Config-images.in                            | 15 ++++++++++<br>
>  package/system/mtd/Makefile                        |  9 +++++-<br>
>  package/system/mtd/src/trx.c                       | 10 +++++--<br>
>  .../patches-3.14/162-Belkin_custom_trx_magic.patch | 35 ++++++++++++++++++++++<br>
>  tools/firmware-utils/Makefile                      |  4 +++<br>
>  tools/firmware-utils/src/trx.c                     | 10 ++++++-<br>
>  6 files changed, 79 insertions(+), 4 deletions(-)<br>
>  create mode 100644 target/linux/brcm47xx/patches-3.14/162-Belkin_custom_trx_magic.patch<br>
><br>
<br>
</div></div></blockquote></div><br></div></div>