[PATCH] mtd: parsers: trx: Rewrite partition parsing

[Rafał Miłecki]

On 16.10.2022 12:40, Linus Walleij wrote:
> The TRX parser has never been fully implemented, the commit splitting
> it out of the bcm47xx parser says "There is still some place for
> improvement".
> 
> Here are some improvements:
> 
> 1. First partition includes TRX header offset:
> 
> The parser can currently produce output like this:
> 0x00000000001c-0x000000280000 : "linux"
> mtd: partition "linux" doesn't start on an erase/write block
> boundary -- force read-only
> 
> This is because the TRX header is not included into the
> partition, while it should be: the vendor code does this,
> and when replacing the kernel in flash we certainly want
> to write it along with a new TRX tag as well. Right now
> we cannot replace it from within Linux due to this
> limitation.

You're trying to workaround mtd write limitation by hacking mtd
partition. TRX header is NOT a part of kernel image. It should not be
part of the "linux" MTD partition.

If you really want that covered you can add partition for it like
0x000000000000-0x00000000001c : "header"
but I see little point in that.


> 2. Scan for several TRX headers
> 
> Currently only the first flash block is scanned for a TRX
> header, but several flashes have multiple TRX headers
> each with up to 3 partitions.

That's because it's a TRX parser. It's meant to parse TRX container.
Just that.

We can't have TRX parser handle all possible cases of its usage. They
depend on vendor solutions and would just bloat TRX code.

Let's keep things simple. Let TRX parser just parse TRX. Use another
parser to deal with your specific flash space layout.


> 3. Determine extents of the data (rootfs) partition.
> 
> While still a bit hacky (just scanning forward for UBI
> magic) we check where the rootfs volume actually ends,
> and do not assume it fills the rest of the flash memory.
> 
> 4. Add free space as a separate partition.
> 
> Before this I could not mount my UBI rootfs because
> the rootfs gets too big: all remaining space will put
> into the second detected partition so the UBI partition
> has the wrong size and will not attach. After this patch
> it mounts just fine.
> 
> Vendor code partition detection (D-Link DWL-8610AP):
> 
> Creating 5 MTD partitions on "brcmnand":
> 0x000000000000-0x000002800000 : "linux"
> 0x000000280000-0x000002800000 : "rootfs"
> 0x000002800000-0x000005000000 : "linux2"
> 0x000002a80000-0x000005000000 : "rootfs2"
> 0x000005000000-0x000008000000 : "jffs2"
> 
> Before this patch:
> 
> Creating 2 MTD partitions on "brcmnand.0":
> 0x00000000001c-0x000000280000 : "linux"
> mtd: partition "linux" doesn't start on an erase/write
>    block boundary -- force read-only
> 0x000000280000-0x000008000000 : "ubi"
> 
> After this patch:
> 
> 5 trx partitions found on MTD device brcmnand.0
> Creating 5 MTD partitions on "brcmnand.0":
> 0x000000000000-0x000000280000 : "linux"
> 0x000000280000-0x000002800000 : "ubi"
> 0x000002800000-0x000002a80000 : "linux2"
> 0x000002a80000-0x000005000000 : "ubi2"
> 0x000005000000-0x000008000000 : "free"

This proves you're just trying to bloat TRX parser with vendor specific
flash space layout. This parser SHOULD NOT care about extra partitions
or free spaces used out of actual TRX.


> Cc: Rafał Miłecki <zajec5 at gmail.com>
> Cc: Hauke Mehrtens <hauke at hauke-m.de>
> Signed-off-by: Linus Walleij <linus.walleij at linaro.org>

So this is really a NO from me.

I extraced TRX parser from bcm47xxpart to make it generic & reusable.
It seems to works well to me. See Asus RT-AC88U, Phicomm K3 and TP-Link
Archer routers (DTS files).

We have support for subpartitions in MTD so that's a way to go.

So please just use/write a proper parser for your device flash space.
Make it register "firmware" (or similar) MTD partition for actual TRX
content. Then let TRX parser do its job.