Re: [OpenWrt-Devel] [PATCH v1 0/7] Add custom TRX header option

2015-02-10 Thread Rafał Miłecki
On 9 February 2015 at 21:52, Florian Fainelli flor...@openwrt.org wrote:
 On 09/02/15 08:29, Will Sheppard wrote:
 Patchset to essentially add custom TRX header to all firmware produced.

 This is most useful for the Belkin routers from my experience. I'm not
 how other trx based firmwares modify the header for their own purposes.

 This is applied across the board so that the kernel, trx tools and mtd
 tools are all compiled with the specified header.

 I have modified the kernel specifically for the brcm47xx based boards to
 ensure the mtd parser looks for the correct magic. NOTE: This function will,
 for other boards, fail.

 I don't think this is desirable at all to have, for many reasons:

A quick comment: I agree.

This patchset looks hacky to me with all these magics set in config file.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel


[OpenWrt-Devel] [PATCH v1 0/7] Add custom TRX header option

2015-02-09 Thread Will Sheppard
Patchset to essentially add custom TRX header to all firmware produced.

This is most useful for the Belkin routers from my experience. I'm not
how other trx based firmwares modify the header for their own purposes.

This is applied across the board so that the kernel, trx tools and mtd 
tools are all compiled with the specified header.

I have modified the kernel specifically for the brcm47xx based boards to
ensure the mtd parser looks for the correct magic. NOTE: This function will, 
for other boards, fail.

Will Sheppard (7):
  config: Add option to specify custom TRX header
  mtd: Add custom trx magic option to mtd tool
  mtd: Fix: Use TRX_MAGIC define not hard-coded number
  trx: Add custom TRX option to trx firmware tool
  mtd: Add debug showing header magic in use
  kernel: bcm47xx: Add custom TRX header option to kernel
  mtd: Fix makefile to work with quilt as per wiki

 config/Config-images.in| 15 ++
 package/system/mtd/Makefile|  9 +-
 package/system/mtd/src/trx.c   | 10 +--
 .../patches-3.14/162-Belkin_custom_trx_magic.patch | 35 ++
 tools/firmware-utils/Makefile  |  4 +++
 tools/firmware-utils/src/trx.c | 10 ++-
 6 files changed, 79 insertions(+), 4 deletions(-)
 create mode 100644 
target/linux/brcm47xx/patches-3.14/162-Belkin_custom_trx_magic.patch

-- 
1.9.1
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH v1 0/7] Add custom TRX header option

2015-02-09 Thread Florian Fainelli
On 09/02/15 08:29, Will Sheppard wrote:
 Patchset to essentially add custom TRX header to all firmware produced.
 
 This is most useful for the Belkin routers from my experience. I'm not
 how other trx based firmwares modify the header for their own purposes.
 
 This is applied across the board so that the kernel, trx tools and mtd 
 tools are all compiled with the specified header.
 
 I have modified the kernel specifically for the brcm47xx based boards to
 ensure the mtd parser looks for the correct magic. NOTE: This function will, 
 for other boards, fail.

I don't think this is desirable at all to have, for many reasons:

- how can an use figure out the proper TRX_MAGIC value he/she should use?

- this is extremely error prone and can result, as you mention in
failing to work with existing devices with well-defined TRX headers

- how can we get some level of reproducibility for both the kernel and
firmware tools builds?

I would really encourage you to extend the existing code to take an
arbitrary list of TRX magics, such that you can have maybe a single file
to modify and both mtd and the kernel can use that list of magic
values accordingly.

Out of curiosity, how many different TRX magic values should we have to
support with or without your patches?

 
 Will Sheppard (7):
   config: Add option to specify custom TRX header
   mtd: Add custom trx magic option to mtd tool
   mtd: Fix: Use TRX_MAGIC define not hard-coded number
   trx: Add custom TRX option to trx firmware tool
   mtd: Add debug showing header magic in use
   kernel: bcm47xx: Add custom TRX header option to kernel
   mtd: Fix makefile to work with quilt as per wiki
 
  config/Config-images.in| 15 ++
  package/system/mtd/Makefile|  9 +-
  package/system/mtd/src/trx.c   | 10 +--
  .../patches-3.14/162-Belkin_custom_trx_magic.patch | 35 
 ++
  tools/firmware-utils/Makefile  |  4 +++
  tools/firmware-utils/src/trx.c | 10 ++-
  6 files changed, 79 insertions(+), 4 deletions(-)
  create mode 100644 
 target/linux/brcm47xx/patches-3.14/162-Belkin_custom_trx_magic.patch
 
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH v1 0/7] Add custom TRX header option

2015-02-09 Thread Will Sheppard
Hi - thanks for review; reply inline

On 9 February 2015 at 20:52, Florian Fainelli flor...@openwrt.org wrote:

 On 09/02/15 08:29, Will Sheppard wrote:
  Patchset to essentially add custom TRX header to all firmware produced.
 
  This is most useful for the Belkin routers from my experience. I'm not
  how other trx based firmwares modify the header for their own purposes.
 
  This is applied across the board so that the kernel, trx tools and mtd
  tools are all compiled with the specified header.
 
  I have modified the kernel specifically for the brcm47xx based boards to
  ensure the mtd parser looks for the correct magic. NOTE: This function
 will,
  for other boards, fail.

 I don't think this is desirable at all to have, for many reasons:

 - how can an use figure out the proper TRX_MAGIC value he/she should use?


There's simply no hard-and-fast rule for this - just looking at a factory
binary might not help - it worked for me.


 - this is extremely error prone and can result, as you mention in
 failing to work with existing devices with well-defined TRX headers


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 )


 - how can we get some level of reproducibility for both the kernel and
 firmware tools builds?

 I would really encourage you to extend the existing code to take an
 arbitrary list of TRX magics, such that you can have maybe a single file
 to modify and both mtd and the kernel can use that list of magic
 values accordingly.


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?


 Out of curiosity, how many different TRX magic values should we have to
 support with or without your patches?


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.


 
  Will Sheppard (7):
config: Add option to specify custom TRX header
mtd: Add custom trx magic option to mtd tool
mtd: Fix: Use TRX_MAGIC define not hard-coded number
trx: Add custom TRX option to trx firmware tool
mtd: Add debug showing header magic in use
kernel: bcm47xx: Add custom TRX header option to kernel
mtd: Fix makefile to work with quilt as per wiki
 
   config/Config-images.in| 15 ++
   package/system/mtd/Makefile|  9 +-
   package/system/mtd/src/trx.c   | 10 +--
   .../patches-3.14/162-Belkin_custom_trx_magic.patch | 35
 ++
   tools/firmware-utils/Makefile  |  4 +++
   tools/firmware-utils/src/trx.c | 10 ++-
   6 files changed, 79 insertions(+), 4 deletions(-)
   create mode 100644
 target/linux/brcm47xx/patches-3.14/162-Belkin_custom_trx_magic.patch
 


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH v1 0/7] Add custom TRX header option

2015-02-09 Thread Florian Fainelli
On 09/02/15 13:12, Will Sheppard wrote:
 Hi - thanks for review; reply inline
 
 On 9 February 2015 at 20:52, Florian Fainelli flor...@openwrt.org
 mailto:flor...@openwrt.org wrote:
 
 On 09/02/15 08:29, Will Sheppard wrote:
  Patchset to essentially add custom TRX header to all firmware produced.
 
  This is most useful for the Belkin routers from my experience. I'm not
  how other trx based firmwares modify the header for their own purposes.
 
  This is applied across the board so that the kernel, trx tools and mtd
  tools are all compiled with the specified header.
 
  I have modified the kernel specifically for the brcm47xx based boards to
  ensure the mtd parser looks for the correct magic. NOTE: This function 
 will,
  for other boards, fail.
 
 I don't think this is desirable at all to have, for many reasons:
 
 - how can an use figure out the proper TRX_MAGIC value he/she should
 use?
 
 
 There's simply no hard-and-fast rule for this - just looking at a
 factory binary might not help - it worked for me. 

That was more of a rhetorical question, since this is not easy, someone
has to do it, and preferably in a way that makes the build reliable for
end-users.

 
 
 - this is extremely error prone and can result, as you mention in
 failing to work with existing devices with well-defined TRX headers
 
 
 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 ) 

Error prone on multiple levels: what happens if I wipe-out my original
config and can't remember the correct 32-bits magic value? What if the
option is renamed, what I upgrade the kernel but not mtd, etc...

 
 
 - how can we get some level of reproducibility for both the kernel and
 firmware tools builds?
 
 I would really encourage you to extend the existing code to take an
 arbitrary list of TRX magics, such that you can have maybe a single file
 to modify and both mtd and the kernel can use that list of magic
 values accordingly.
 
 
 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?

You can certainly extract the board you are running on, but that assumes
you already have a working user-space and kernel, hence the partition
parser needs to work etc... chicken and egg. The alternative, which is
likely what is happening today, is that you need to flash a special
image, with a special magic that only supports a particular set of
boards using a defined TRX magic value, that way you can become (at
least at the MTD level) board agnostic.

 
 
 Out of curiosity, how many different TRX magic values should we have to
 support with or without your patches?
 
 
 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.

That is the direction I would take, just add them one by one such that
you can control exactly when both user-space and kernel space can
properly support a given model (or family of).

 
 
 
  Will Sheppard (7):
config: Add option to specify custom TRX header
mtd: Add custom trx magic option to mtd tool
mtd: Fix: Use TRX_MAGIC define not hard-coded number
trx: Add custom TRX option to trx firmware tool
mtd: Add debug showing header magic in use
kernel: bcm47xx: Add custom TRX header option to kernel
mtd: Fix makefile to work with quilt as per wiki
 
   config/Config-images.in| 15 ++
   package/system/mtd/Makefile|  9 +-
   package/system/mtd/src/trx.c   | 10 +--
   .../patches-3.14/162-Belkin_custom_trx_magic.patch | 35
 ++
   tools/firmware-utils/Makefile  |  4 +++
   tools/firmware-utils/src/trx.c | 10 ++-
   6 files changed, 79 insertions(+), 4 deletions(-)
   create mode 100644
 target/linux/brcm47xx/patches-3.14/162-Belkin_custom_trx_magic.patch
 
 
 
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel