Re: [OpenWrt-Devel] [PATCH 2/2] ramips: fix RBM11G name and partitioning
> Le 20 juil. 2018 à 19:56, Martin Blumenstingl
> a écrit :
>
>> On Thu, Jul 19, 2018 at 7:12 PM Thibaut VARÈNE wrote:
>>
>> This patch improves faf64056ddd46992a75b1e277d94541c7251035c 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 resulting partition scheme looks like this:
>> [2.477826] Creating 7 MTD partitions on "spi0.0":
>> [2.482604] 0x-0x0004 : "RouterBoot"
>> [2.488948] 0x-0xf000 : "routerboot"
>> [2.495289] 0xf000-0x0001 : "hard_config"
>> [2.501596] 0x0001-0x0001f000 : "routerboot2"
>> [2.507966] 0x0002-0x00021000 : "soft_config"
>> [2.514307] 0x0003-0x00031000 : "bios"
>> [2.520108] 0x0004-0x0100 : "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.
> I find this bit of information strange since it has nothing to do with
> the other changes. also: what kind of hardware crypto does it support?
Documentation note as there’s ongoing work to support these devices: easier to
grep than to check individual devices specs. If all mt7621s have this onboard
device then indeed the note is unnecessary.
>> 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
>> Signed-off-by: Thibaut VARÈNE
>> ---
>> target/linux/ramips/dts/RBM11G.dts | 62
>> +++---
>> 1 file changed, 45 insertions(+), 17 deletions(-)
>>
>> diff --git a/target/linux/ramips/dts/RBM11G.dts
>> b/target/linux/ramips/dts/RBM11G.dts
>> index f312093a22..079b4fc146 100644
>> --- a/target/linux/ramips/dts/RBM11G.dts
>> +++ b/target/linux/ramips/dts/RBM11G.dts
>> @@ -7,7 +7,7 @@
>>
>> / {
>>compatible = "mikrotik,rbm11g", "mediatek,mt7621-soc";
>> - model = "MikroTik RBM11G";
>> + model = "MikroTik RouterBOARD M11G";
> why do you need to change the model when updating the partitions? the
> commit message doesn't explain this
This started as a cleanup patch which I didn’t expect to be this controversial
so I didn’t split out this one liner.
>>aliases {
>>led-status = _usr;
>> @@ -90,29 +90,54 @@
>>#size-cells = <1>;
>>compatible = "jedec,spi-nor";
>>reg = <0>;
>> + // XXX empiric value to obtain actual 10MHz SCK at the chip
>>spi-max-frequency = <3125000>;
>>
>> - partition@0 {
>> - label = "routerboot";
>> - reg = <0x00 0x00F000>;
>> - read-only;
>> - };
>> -
>> - factory: partition@f000 {
>> - label = "factory";
>> - reg = <0x00F000 0x031000>;
>> - read-only;
>> - };
>> -
>> - partition@4 {
>> - label = "firmware";
>> - reg = <0x04 0xFC>;
>> + partitions {
>> + compatible = "fixed-partitions";
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> +
>> + partition@0 {
>> + label = "RouterBoot";
>> + reg = <0x0 0x4>;
>> + read-only;
>> + };
>> +
>> + routerboot@0 {
>> + reg = <0x0 0xf000>;
>> + read-only;
>> + };
> isn't the recommended node name "partition" nowadays?
Not according to kernel documentation.
> both partitions above would then become "partition@0" -> whether the
> second node overwrites the first one (since both node names and
> addresses/offsets are identical) or both are added to the resulting
> .dtb depends on the dtc (device tree compiler) version, so I highly
> recommend *NOT* doing this
>
>> + hard_config: hard_config@f000 {
>> + reg = <0xf000 0x1000>;
>> + read-only;
>> + };
>> +
>> + routerboot2@1 {
>> + reg = <0x1 0xf000>;
>> + read-only;
>> + };
>> +
>> + soft_config@2 {
>> +
Re: [OpenWrt-Devel] [PATCH 2/2] ramips: fix RBM11G name and partitioning
The sender domain has a DMARC Reject/Quarantine policy which disallows
sending mailing list messages using the original "From" header.
To mitigate this problem, the original message has been wrapped
automatically by the mailing list software.--- Begin Message ---
On Thu, Jul 19, 2018 at 7:12 PM Thibaut VARÈNE wrote:
>
> This patch improves faf64056ddd46992a75b1e277d94541c7251035c 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 resulting partition scheme looks like this:
> [2.477826] Creating 7 MTD partitions on "spi0.0":
> [2.482604] 0x-0x0004 : "RouterBoot"
> [2.488948] 0x-0xf000 : "routerboot"
> [2.495289] 0xf000-0x0001 : "hard_config"
> [2.501596] 0x0001-0x0001f000 : "routerboot2"
> [2.507966] 0x0002-0x00021000 : "soft_config"
> [2.514307] 0x0003-0x00031000 : "bios"
> [2.520108] 0x0004-0x0100 : "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.
I find this bit of information strange since it has nothing to do with
the other changes. also: what kind of hardware crypto does it support?
> 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
> Signed-off-by: Thibaut VARÈNE
> ---
> target/linux/ramips/dts/RBM11G.dts | 62
> +++---
> 1 file changed, 45 insertions(+), 17 deletions(-)
>
> diff --git a/target/linux/ramips/dts/RBM11G.dts
> b/target/linux/ramips/dts/RBM11G.dts
> index f312093a22..079b4fc146 100644
> --- a/target/linux/ramips/dts/RBM11G.dts
> +++ b/target/linux/ramips/dts/RBM11G.dts
> @@ -7,7 +7,7 @@
>
> / {
> compatible = "mikrotik,rbm11g", "mediatek,mt7621-soc";
> - model = "MikroTik RBM11G";
> + model = "MikroTik RouterBOARD M11G";
why do you need to change the model when updating the partitions? the
commit message doesn't explain this
>
> aliases {
> led-status = _usr;
> @@ -90,29 +90,54 @@
> #size-cells = <1>;
> compatible = "jedec,spi-nor";
> reg = <0>;
> + // XXX empiric value to obtain actual 10MHz SCK at the chip
> spi-max-frequency = <3125000>;
>
> - partition@0 {
> - label = "routerboot";
> - reg = <0x00 0x00F000>;
> - read-only;
> - };
> -
> - factory: partition@f000 {
> - label = "factory";
> - reg = <0x00F000 0x031000>;
> - read-only;
> - };
> -
> - partition@4 {
> - label = "firmware";
> - reg = <0x04 0xFC>;
> + partitions {
> + compatible = "fixed-partitions";
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + partition@0 {
> + label = "RouterBoot";
> + reg = <0x0 0x4>;
> + read-only;
> + };
> +
> + routerboot@0 {
> + reg = <0x0 0xf000>;
> + read-only;
> + };
isn't the recommended node name "partition" nowadays?
both partitions above would then become "partition@0" -> whether the
second node overwrites the first one (since both node names and
addresses/offsets are identical) or both are added to the resulting
.dtb depends on the dtc (device tree compiler) version, so I highly
recommend *NOT* doing this
> + hard_config: hard_config@f000 {
> + reg = <0xf000 0x1000>;
> + read-only;
> + };
> +
> + routerboot2@1 {
> + reg = <0x1 0xf000>;
> + read-only;
> + };
> +
> + soft_config@2 {
> + reg = <0x2 0x1000>;
> + };
> +
> + // valid data only extends up to 0x4f but make it
> 0x1000 to match ar71xx
> + bios@3 {
> + reg = <0x3 0x1000>;
> +
Re: [OpenWrt-Devel] [PATCH 2/2] ramips: fix RBM11G name and partitioning
2018-07-19 19:12 GMT+02:00 Thibaut VARÈNE :
> This patch improves faf64056ddd46992a75b1e277d94541c7251035c 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 resulting partition scheme looks like this:
> [2.477826] Creating 7 MTD partitions on "spi0.0":
> [2.482604] 0x-0x0004 : "RouterBoot"
> [2.488948] 0x-0xf000 : "routerboot"
> [2.495289] 0xf000-0x0001 : "hard_config"
> [2.501596] 0x0001-0x0001f000 : "routerboot2"
> [2.507966] 0x0002-0x00021000 : "soft_config"
> [2.514307] 0x0003-0x00031000 : "bios"
> [2.520108] 0x0004-0x0100 : "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
> Signed-off-by: Thibaut VARÈNE
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 0x1>;
};
};
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
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH 2/2] ramips: fix RBM11G name and partitioning
> On 19 Jul 2018, at 19:46, Mathias Kresin 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 0x1>;
>};
> };
>
> 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.
T.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
