[OpenWrt-Devel,1/2] ramips: fix RBM33G name and partitioning

This patch improves 5684d087418d176cfdef4e045e1950ca7ba3b09f by setting
the correct partition scheme for the RouterBoot section of the flash.

This section is subdivided in several segments, as they are on ar71xx
RB devices, albeit with different offsets and sizes. The naming convention
from ar71xx has been preserved, with an overlapping "RouterBoot" top level
partition added for clarity due to the many holes.

The OEM source code also define a "fake" partition at the beginning of
the secondary flash chip: to avoid trouble if OEM ever make use of that
space, we define it here.

The resulting partition scheme looks like this:
[    2.355095] Creating 6 MTD partitions on "spi0.0":
[    2.359872] 0x000000000000-0x000000040000 : "RouterBoot"
[    2.366197] 0x000000000000-0x00000000f000 : "routerboot"
[    2.372437] 0x00000000f000-0x000000010000 : "hard_config"
[    2.378818] 0x000000010000-0x00000001f000 : "routerboot2"
[    2.385200] 0x000000020000-0x000000021000 : "soft_config"
[    2.391503] 0x000000030000-0x000000031000 : "bios"
[    2.419283] Creating 2 MTD partitions on "spi0.1":
[    2.424062] 0x000000000000-0x000000040000 : "RouterBootFake"
[    2.430717] 0x000000040000-0x000001000000 : "firmware"

The device name is corrected to match the hardware-stored (in hard_config)
device name.

Leave a note in DTS to mention this device supports hardware crypto.
Leave a note in DTS to explain how the original author selected the SPI speed.

Note: more work is required to get rbcfg working on this device due to
endianness.

Tested-by: Tobias Schramm <tobleminer@gmail.com>
Signed-off-by: Thibaut VARÈNE <hacks@slashdirt.org>
---
 target/linux/ramips/dts/RBM33G.dts | 64 +++++++++++++++++++++++++++++---------
 1 file changed, 50 insertions(+), 14 deletions(-)

2018-07-19 19:12 GMT+02:00 Thibaut VARÈNE <hacks@slashdirt.org>:
> This patch improves 5684d087418d176cfdef4e045e1950ca7ba3b09f by setting
> the correct partition scheme for the RouterBoot section of the flash.
>
> This section is subdivided in several segments, as they are on ar71xx
> RB devices, albeit with different offsets and sizes. The naming convention
> from ar71xx has been preserved, with an overlapping "RouterBoot" top level
> partition added for clarity due to the many holes.
>
> The OEM source code also define a "fake" partition at the beginning of
> the secondary flash chip: to avoid trouble if OEM ever make use of that
> space, we define it here.
>
> The resulting partition scheme looks like this:
> [    2.355095] Creating 6 MTD partitions on "spi0.0":
> [    2.359872] 0x000000000000-0x000000040000 : "RouterBoot"
> [    2.366197] 0x000000000000-0x00000000f000 : "routerboot"
> [    2.372437] 0x00000000f000-0x000000010000 : "hard_config"
> [    2.378818] 0x000000010000-0x00000001f000 : "routerboot2"
> [    2.385200] 0x000000020000-0x000000021000 : "soft_config"
> [    2.391503] 0x000000030000-0x000000031000 : "bios"
> [    2.419283] Creating 2 MTD partitions on "spi0.1":
> [    2.424062] 0x000000000000-0x000000040000 : "RouterBootFake"
> [    2.430717] 0x000000040000-0x000001000000 : "firmware"
>
> The device name is corrected to match the hardware-stored (in hard_config)
> device name.
>
> Leave a note in DTS to mention this device supports hardware crypto.
> Leave a note in DTS to explain how the original author selected the SPI speed.
>
> Note: more work is required to get rbcfg working on this device due to
> endianness.
>
> Tested-by: Tobias Schramm <tobleminer@gmail.com>
> Signed-off-by: Thibaut VARÈNE <hacks@slashdirt.org>

FYI, I already NAK'ed the very same patch on github after a way to
short conversation on IRC follow by some more words on github.

I neither see the need to add notes for not yet working nodes
[personal preference] to the device tree source file, nor the need to
to create the overlapping partitions "RouterBoot" +
routerboot/hard_config/routerboot2/... [personal preference].

To get the dt compiler accepting the overlapping partitions without a
warning, a style was chosen completely different from all other dts
files in the target [maintenance reason]. Furthermore, nodes sharing
the same reg are usually (always?) expressed as child nodes in the
devicetree similar to [technical reason]:

partitions {
    compatible = "fixed-partitions";

    partition@0 {
        reg = <0 0x3000>;

        subpartition@0 {
            reg = <0 0x1000>;
        };

        subpartition@1000 {
            reg = <0x1000 0x2000>;
        };
    };

    partition@3000 {
        reg = <0x3000 0x10000>;
    };
};

To my knowledge, the above isn't possible with fixed-partitions. Which
either means fixed-partitions misses a feature or someone tries to use
it in a way not intended.

This time I'll leave it up to someone else to make a call. I tried my
best to turn it into something that I'm fine to accept and failed.

Mathias

> On 19 Jul 2018, at 19:46, Mathias Kresin <dev@kresin.me> wrote:
> 
> 
> To get the dt compiler accepting the overlapping partitions without a
> warning, a style was chosen completely different from all other dts
> files in the target [maintenance reason]. Furthermore, nodes sharing
> the same reg are usually (always?) expressed as child nodes in the
> devicetree similar to [technical reason]:
> 
> partitions {
>   compatible = "fixed-partitions";
> 
>   partition@0 {
>       reg = <0 0x3000>;
> 
>       subpartition@0 {
>           reg = <0 0x1000>;
>       };
> 
>       subpartition@1000 {
>           reg = <0x1000 0x2000>;
>       };
>   };
> 
>   partition@3000 {
>       reg = <0x3000 0x10000>;
>   };
> };
> 
> To my knowledge, the above isn't possible with fixed-partitions.

I don’t see why. AIUI DTS syntax is respected, all nodes have a unique name and all nodes refer to their top boundary as the unit address.

> Which
> either means fixed-partitions misses a feature or someone tries to use
> it in a way not intended.

The above is in line with the canonical way to define partitions in DTS, as documented in Documentation/devicetree/bindings/mtd/partition.txt

This method is apparently used in all bcm targets, as well as ath79, ipq and lantiq. I’d suggest that _not_ using this method is a bug, and not the correct way to go, as evidenced by the aforementioned documentation. Quoting:

	For backwards compatibility partitions as direct subnodes of the flash device are
	supported. This use is discouraged.

If this gets accepted it should be cherry-picked for 18.06.

T.