This is a list of patches cleaning up some things I noticed while
working with barebox boot loader support for NAND node of Lenovo ix4-300d.
Patch 1 removes a flash partition node for the whole flash. The flash
as a whole is already best represented by the NAND device itself.
Patch 2 moves the stock flash partitions to a partitions sub-node. This
will also ease removal of stock flash partition layout for barebox and
other boot loaders.
Patch 3 cleans the flash partitions ranges by prefixing them to 32-bit
lower-case hex numbers. Also, a stale 'x' is removed from one partition
name offset.
Patch 4 finally adds ECC properties for 4-bit BCH ECC used by the ix4-300d
flash.
Sebastian
Sebastian Hesselbarth (4):
ARM: dt: mvebu: ix4-300d: remove whole flash partition
ARM: dt: mvebu: ix4-300d: move partitions to partition sub-node
ARM: dt: mvebu: ix4-300d: Cleanup NAND partition ranges
ARM: dt: mvebu: ix4-300d: Add ECC properties to NAND flash
arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts | 76 +++++++++++++------------
1 file changed, 39 insertions(+), 37 deletions(-)
---
Cc: Jason Cooper <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Gregory Clement <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Pawel Moll <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: Kumar Gala <[email protected]>
Cc: Russell King <[email protected]>
Cc: Benoit Masson <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
--
2.1.4
Current NAND node has an additional flash partition for the whole
flash overlapping with real partitions. Remove this partition as
the whole flash is already represented by the NAND device itself.
Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Cc: Jason Cooper <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Gregory Clement <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Pawel Moll <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: Kumar Gala <[email protected]>
Cc: Russell King <[email protected]>
Cc: Benoit Masson <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts | 5 -----
1 file changed, 5 deletions(-)
diff --git a/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts b/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
index 58b500873bfd..30a0a6eac645 100644
--- a/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
+++ b/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
@@ -183,11 +183,6 @@
label = "boot";
reg = <0xE00000 0x3F200000>;
};
-
- partition@flash {
- label = "flash";
- reg = <0x0 0x40000000>;
- };
};
};
};
--
2.1.4
NAND flash partitions should be part of a partitions sub-node
not the flash node itself. Move the partitions which will also
allow different bootloaders get rid of the stock partitions
easily by removing the partitions node.
Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Cc: Jason Cooper <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Gregory Clement <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Pawel Moll <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: Kumar Gala <[email protected]>
Cc: Russell King <[email protected]>
Cc: Benoit Masson <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts | 67 +++++++++++++------------
1 file changed, 36 insertions(+), 31 deletions(-)
diff --git a/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts b/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
index 30a0a6eac645..76781fd18624 100644
--- a/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
+++ b/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
@@ -151,37 +151,42 @@
marvell,nand-enable-arbiter;
nand-on-flash-bbt;
- partition@0 {
- label = "u-boot";
- reg = <0x0000000 0xe0000>;
- read-only;
- };
-
- partition@e0000 {
- label = "u-boot-env";
- reg = <0xe0000 0x20000>;
- read-only;
- };
-
- partition@100000 {
- label = "u-boot-env2";
- reg = <0x100000 0x20000>;
- read-only;
- };
-
- partition@120000 {
- label = "zImage";
- reg = <0x120000 0x400000>;
- };
-
- partition@520000 {
- label = "initrd";
- reg = <0x520000 0x400000>;
- };
-
- partition@xE00000 {
- label = "boot";
- reg = <0xE00000 0x3F200000>;
+ partitions {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ partition@0 {
+ label = "u-boot";
+ reg = <0x0000000 0xe0000>;
+ read-only;
+ };
+
+ partition@e0000 {
+ label = "u-boot-env";
+ reg = <0xe0000 0x20000>;
+ read-only;
+ };
+
+ partition@100000 {
+ label = "u-boot-env2";
+ reg = <0x100000 0x20000>;
+ read-only;
+ };
+
+ partition@120000 {
+ label = "zImage";
+ reg = <0x120000 0x400000>;
+ };
+
+ partition@520000 {
+ label = "initrd";
+ reg = <0x520000 0x400000>;
+ };
+
+ partition@xE00000 {
+ label = "boot";
+ reg = <0xE00000 0x3F200000>;
+ };
};
};
};
--
2.1.4
Prefix all partition reg properties to 32-bit to ease readability.
While at it, also remove a stale x in front of boot partition
offset and make some upper-case hex numbers lower-case.
Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Cc: Jason Cooper <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Gregory Clement <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Pawel Moll <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: Kumar Gala <[email protected]>
Cc: Russell King <[email protected]>
Cc: Benoit Masson <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts b/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
index 76781fd18624..13cf69a8d0fb 100644
--- a/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
+++ b/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
@@ -157,35 +157,35 @@
partition@0 {
label = "u-boot";
- reg = <0x0000000 0xe0000>;
+ reg = <0x00000000 0x000e0000>;
read-only;
};
partition@e0000 {
label = "u-boot-env";
- reg = <0xe0000 0x20000>;
+ reg = <0x000e0000 0x00020000>;
read-only;
};
partition@100000 {
label = "u-boot-env2";
- reg = <0x100000 0x20000>;
+ reg = <0x00100000 0x00020000>;
read-only;
};
partition@120000 {
label = "zImage";
- reg = <0x120000 0x400000>;
+ reg = <0x00120000 0x00400000>;
};
partition@520000 {
label = "initrd";
- reg = <0x520000 0x400000>;
+ reg = <0x00520000 0x00400000>;
};
- partition@xE00000 {
+ partition@e00000 {
label = "boot";
- reg = <0xE00000 0x3F200000>;
+ reg = <0x00e00000 0x3f200000>;
};
};
};
--
2.1.4
The NAND device found on Lenovo ix4-300d uses 4-bit BCH ECC protection.
Add the corresponding properties to the NAND node.
Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Cc: Jason Cooper <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Gregory Clement <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Pawel Moll <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: Kumar Gala <[email protected]>
Cc: Russell King <[email protected]>
Cc: Benoit Masson <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts b/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
index 13cf69a8d0fb..e4bf83c4bd2f 100644
--- a/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
+++ b/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
@@ -150,6 +150,8 @@
marvell,nand-keep-config;
marvell,nand-enable-arbiter;
nand-on-flash-bbt;
+ nand-ecc-strength = <4>;
+ nand-ecc-step-size = <512>;
partitions {
#address-cells = <1>;
--
2.1.4
On Sat, Nov 28, 2015 at 12:14:05PM +0100, Sebastian Hesselbarth wrote:
> Current NAND node has an additional flash partition for the whole
> flash overlapping with real partitions. Remove this partition as
> the whole flash is already represented by the NAND device itself.
If i remember correctly, we discussed this when the contribution was
made. I think the stock firmware might use this for applying updates.
Maybe Benoit can comment?
If so, removing this will break compatibility with stock firmware. Do
we want to do that? There are a few other mvebu dts files with a
partition spanning the whole flash. Should we remove them as well?
Andrew
On Sat, Nov 28, 2015 at 12:14:06PM +0100, Sebastian Hesselbarth wrote:
> NAND flash partitions should be part of a partitions sub-node
> not the flash node itself. Move the partitions which will also
> allow different bootloaders get rid of the stock partitions
> easily by removing the partitions node.
>
> Signed-off-by: Sebastian Hesselbarth <[email protected]>
Humm, did not know that. Quoting Documentation/devicetree/bindings/mtd/partition.txt:
The partition table should be a subnode of the mtd node and
should be named 'partitions'. Partitions are defined in subnodes
of the partitions node.
For backwards compatibility partitions as direct subnodes of the
mtd device are supported. This use is discouraged.
It also looks like none of the other MVEBU maintainers know that
either, since a quick look at the .dts files shows very few have a
partitions node.
Acked-by: Andrew Lunn <[email protected]>
Thanks
Andrew
> ---
> Cc: Jason Cooper <[email protected]>
> Cc: Andrew Lunn <[email protected]>
> Cc: Gregory Clement <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Pawel Moll <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Ian Campbell <[email protected]>
> Cc: Kumar Gala <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: Benoit Masson <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts | 67 +++++++++++++------------
> 1 file changed, 36 insertions(+), 31 deletions(-)
>
> diff --git a/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts b/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
> index 30a0a6eac645..76781fd18624 100644
> --- a/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
> +++ b/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
> @@ -151,37 +151,42 @@
> marvell,nand-enable-arbiter;
> nand-on-flash-bbt;
>
> - partition@0 {
> - label = "u-boot";
> - reg = <0x0000000 0xe0000>;
> - read-only;
> - };
> -
> - partition@e0000 {
> - label = "u-boot-env";
> - reg = <0xe0000 0x20000>;
> - read-only;
> - };
> -
> - partition@100000 {
> - label = "u-boot-env2";
> - reg = <0x100000 0x20000>;
> - read-only;
> - };
> -
> - partition@120000 {
> - label = "zImage";
> - reg = <0x120000 0x400000>;
> - };
> -
> - partition@520000 {
> - label = "initrd";
> - reg = <0x520000 0x400000>;
> - };
> -
> - partition@xE00000 {
> - label = "boot";
> - reg = <0xE00000 0x3F200000>;
> + partitions {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + partition@0 {
> + label = "u-boot";
> + reg = <0x0000000 0xe0000>;
> + read-only;
> + };
> +
> + partition@e0000 {
> + label = "u-boot-env";
> + reg = <0xe0000 0x20000>;
> + read-only;
> + };
> +
> + partition@100000 {
> + label = "u-boot-env2";
> + reg = <0x100000 0x20000>;
> + read-only;
> + };
> +
> + partition@120000 {
> + label = "zImage";
> + reg = <0x120000 0x400000>;
> + };
> +
> + partition@520000 {
> + label = "initrd";
> + reg = <0x520000 0x400000>;
> + };
> +
> + partition@xE00000 {
> + label = "boot";
> + reg = <0xE00000 0x3F200000>;
> + };
> };
> };
> };
> --
> 2.1.4
>
On Sat, Nov 28, 2015 at 12:14:07PM +0100, Sebastian Hesselbarth wrote:
> Prefix all partition reg properties to 32-bit to ease readability.
> While at it, also remove a stale x in front of boot partition
> offset and make some upper-case hex numbers lower-case.
>
> Signed-off-by: Sebastian Hesselbarth <[email protected]>
Acked-by: Andrew Lunn <[email protected]>
Andrew
> ---
> Cc: Jason Cooper <[email protected]>
> Cc: Andrew Lunn <[email protected]>
> Cc: Gregory Clement <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Pawel Moll <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Ian Campbell <[email protected]>
> Cc: Kumar Gala <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: Benoit Masson <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts b/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
> index 76781fd18624..13cf69a8d0fb 100644
> --- a/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
> +++ b/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
> @@ -157,35 +157,35 @@
>
> partition@0 {
> label = "u-boot";
> - reg = <0x0000000 0xe0000>;
> + reg = <0x00000000 0x000e0000>;
> read-only;
> };
>
> partition@e0000 {
> label = "u-boot-env";
> - reg = <0xe0000 0x20000>;
> + reg = <0x000e0000 0x00020000>;
> read-only;
> };
>
> partition@100000 {
> label = "u-boot-env2";
> - reg = <0x100000 0x20000>;
> + reg = <0x00100000 0x00020000>;
> read-only;
> };
>
> partition@120000 {
> label = "zImage";
> - reg = <0x120000 0x400000>;
> + reg = <0x00120000 0x00400000>;
> };
>
> partition@520000 {
> label = "initrd";
> - reg = <0x520000 0x400000>;
> + reg = <0x00520000 0x00400000>;
> };
>
> - partition@xE00000 {
> + partition@e00000 {
> label = "boot";
> - reg = <0xE00000 0x3F200000>;
> + reg = <0x00e00000 0x3f200000>;
> };
> };
> };
> --
> 2.1.4
>
On Sat, Nov 28, 2015 at 12:14:08PM +0100, Sebastian Hesselbarth wrote:
> The NAND device found on Lenovo ix4-300d uses 4-bit BCH ECC protection.
> Add the corresponding properties to the NAND node.
>
> Signed-off-by: Sebastian Hesselbarth <[email protected]>
Acked-by: Andrew Lunn <[email protected]>
Andrew
On 28.11.2015 17:52, Andrew Lunn wrote:
> On Sat, Nov 28, 2015 at 12:14:05PM +0100, Sebastian Hesselbarth wrote:
>> Current NAND node has an additional flash partition for the whole
>> flash overlapping with real partitions. Remove this partition as
>> the whole flash is already represented by the NAND device itself.
>
> If i remember correctly, we discussed this when the contribution was
> made. I think the stock firmware might use this for applying updates.
> Maybe Benoit can comment?
Yes, please.
> If so, removing this will break compatibility with stock firmware. Do
> we want to do that? There are a few other mvebu dts files with a
> partition spanning the whole flash. Should we remove them as well?
Well, there is already a mtd device that spans the whole flash so
what is the purpose of another "partition" that isn't a part but
all of the device? Actually, I doubt that a FW update will wipe
the flash as a whole, i.e. including boot loader, boot env, user
config.
Anyway, let's see if Benoit can shed some light on this.
FWIW, neither single partitions nor a combined partitions node
should be a direct sub-node of the _controller_ but a NAND
_device_ node instead. Luckily, multi-device systems are not that
common, so I guess we wait with it until such a system pops up for
testing.
Sebastian
On 28.11.2015 18:00, Andrew Lunn wrote:
> On Sat, Nov 28, 2015 at 12:14:06PM +0100, Sebastian Hesselbarth wrote:
>> NAND flash partitions should be part of a partitions sub-node
>> not the flash node itself. Move the partitions which will also
>> allow different bootloaders get rid of the stock partitions
>> easily by removing the partitions node.
>>
>> Signed-off-by: Sebastian Hesselbarth <[email protected]>
>
> Humm, did not know that. Quoting Documentation/devicetree/bindings/mtd/partition.txt:
>
> The partition table should be a subnode of the mtd node and
> should be named 'partitions'. Partitions are defined in subnodes
> of the partitions node.
>
> For backwards compatibility partitions as direct subnodes of the
> mtd device are supported. This use is discouraged.
>
> It also looks like none of the other MVEBU maintainers know that
> either, since a quick look at the .dts files shows very few have a
> partitions node.
Me neither, Linus Walleij's latest contribution to the pogoplug
series showed it to me. And while I am working on barebox support
for the ix4, I always wanted to remove the stock partitions easily.
Barebox always uses internal registers at 0xf1000000 so it will
never boot that stupid stock kernel that depends on 0xd0000000
registers.
> Acked-by: Andrew Lunn <[email protected]>
>
> Thanks
> Andrew
ditto ;)
Sebastian
>> ---
>> Cc: Jason Cooper <[email protected]>
>> Cc: Andrew Lunn <[email protected]>
>> Cc: Gregory Clement <[email protected]>
>> Cc: Rob Herring <[email protected]>
>> Cc: Pawel Moll <[email protected]>
>> Cc: Mark Rutland <[email protected]>
>> Cc: Ian Campbell <[email protected]>
>> Cc: Kumar Gala <[email protected]>
>> Cc: Russell King <[email protected]>
>> Cc: Benoit Masson <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> ---
>> arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts | 67 +++++++++++++------------
>> 1 file changed, 36 insertions(+), 31 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts b/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
>> index 30a0a6eac645..76781fd18624 100644
>> --- a/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
>> +++ b/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
>> @@ -151,37 +151,42 @@
>> marvell,nand-enable-arbiter;
>> nand-on-flash-bbt;
>>
>> - partition@0 {
>> - label = "u-boot";
>> - reg = <0x0000000 0xe0000>;
>> - read-only;
>> - };
>> -
>> - partition@e0000 {
>> - label = "u-boot-env";
>> - reg = <0xe0000 0x20000>;
>> - read-only;
>> - };
>> -
>> - partition@100000 {
>> - label = "u-boot-env2";
>> - reg = <0x100000 0x20000>;
>> - read-only;
>> - };
>> -
>> - partition@120000 {
>> - label = "zImage";
>> - reg = <0x120000 0x400000>;
>> - };
>> -
>> - partition@520000 {
>> - label = "initrd";
>> - reg = <0x520000 0x400000>;
>> - };
>> -
>> - partition@xE00000 {
>> - label = "boot";
>> - reg = <0xE00000 0x3F200000>;
>> + partitions {
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> +
>> + partition@0 {
>> + label = "u-boot";
>> + reg = <0x0000000 0xe0000>;
>> + read-only;
>> + };
>> +
>> + partition@e0000 {
>> + label = "u-boot-env";
>> + reg = <0xe0000 0x20000>;
>> + read-only;
>> + };
>> +
>> + partition@100000 {
>> + label = "u-boot-env2";
>> + reg = <0x100000 0x20000>;
>> + read-only;
>> + };
>> +
>> + partition@120000 {
>> + label = "zImage";
>> + reg = <0x120000 0x400000>;
>> + };
>> +
>> + partition@520000 {
>> + label = "initrd";
>> + reg = <0x520000 0x400000>;
>> + };
>> +
>> + partition@xE00000 {
>> + label = "boot";
>> + reg = <0xE00000 0x3F200000>;
>> + };
>> };
>> };
>> };
>> --
>> 2.1.4
>>
>From mobile
> Le 28 nov. 2015 à 18:38, Sebastian Hesselbarth <[email protected]> a écrit :
>
>> On 28.11.2015 17:52, Andrew Lunn wrote:
>>> On Sat, Nov 28, 2015 at 12:14:05PM +0100, Sebastian Hesselbarth wrote:
>>> Current NAND node has an additional flash partition for the whole
>>> flash overlapping with real partitions. Remove this partition as
>>> the whole flash is already represented by the NAND device itself.
>>
>> If i remember correctly, we discussed this when the contribution was
>> made. I think the stock firmware might use this for applying updates.
>> Maybe Benoit can comment?
>
> Yes, please.
>From my memory since I'm not running the stock firmware it uses the
MTD device directly. This is safe to remove. I was not very contort
able with this flash dts part it was copied over from a netgear mevbu
device ...
>
>> If so, removing this will break compatibility with stock firmware. Do
>> we want to do that? There are a few other mvebu dts files with a
>> partition spanning the whole flash. Should we remove them as well?
>
> Well, there is already a mtd device that spans the whole flash so
> what is the purpose of another "partition" that isn't a part but
> all of the device? Actually, I doubt that a FW update will wipe
> the flash as a whole, i.e. including boot loader, boot env, user
> config.
>
> Anyway, let's see if Benoit can shed some light on this.
>
> FWIW, neither single partitions nor a combined partitions node
> should be a direct sub-node of the _controller_ but a NAND
> _device_ node instead. Luckily, multi-device systems are not that
> common, so I guess we wait with it until such a system pops up for
> testing.
>
> Sebastian
>
>
>On Sat, Nov 28, 2015 at 12:14:05PM +0100, Sebastian Hesselbarth wrote:
> Current NAND node has an additional flash partition for the whole
> flash overlapping with real partitions. Remove this partition as
> the whole flash is already represented by the NAND device itself.
>
> Signed-off-by: Sebastian Hesselbarth <[email protected]>
Given Benoit comment
Acked-by: Andrew Lunn <[email protected]>
Andrew
Sebastian,
Adding Ezequiel Garcia in Cc.
On Sat, 28 Nov 2015 12:14:08 +0100, Sebastian Hesselbarth wrote:
> The NAND device found on Lenovo ix4-300d uses 4-bit BCH ECC protection.
> Add the corresponding properties to the NAND node.
If the ONFI information from the NAND flash say that it requires 4 bits
per 512, then there should be no need to add this information to the
Device Tree as the pxa3xx_nand driver by default uses the ONFI
information.
Those properties are only needed when for some reason the vendor has
chosen to use a ECC strength that doesn't match with the one advertised
by the flash in its ONFI information (either stronger or weaker). But
in this case, your commit log is confusing, because it says that the
"NAND device ... uses 4-bit BCH ECC protection". If it really does,
then the patch is not needed :-)
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
On 29.11.2015 15:35, Thomas Petazzoni wrote:
> Adding Ezequiel Garcia in Cc.
>
> On Sat, 28 Nov 2015 12:14:08 +0100, Sebastian Hesselbarth wrote:
>> The NAND device found on Lenovo ix4-300d uses 4-bit BCH ECC protection.
>> Add the corresponding properties to the NAND node.
>
> If the ONFI information from the NAND flash say that it requires 4 bits
> per 512, then there should be no need to add this information to the
> Device Tree as the pxa3xx_nand driver by default uses the ONFI
> information.
Thomas,
as said in the cover letter, this is also DT cleanup with barebox
bootloader in mind. I do not accept what Linux' pxa3xx_nand driver
is doing as a reference here ;)
> Those properties are only needed when for some reason the vendor has
> chosen to use a ECC strength that doesn't match with the one advertised
> by the flash in its ONFI information (either stronger or weaker). But
> in this case, your commit log is confusing, because it says that the
> "NAND device ... uses 4-bit BCH ECC protection". If it really does,
> then the patch is not needed :-)
I agree that if ONFI is already advertising 4/512 ECC (and it is), we
do not need the properties. Anyway, IIRC barebox does not yet properly
parse ONFI or at least it does not derive minimum ECC settings from it.
I'll have to have a closer look at barebox' ONFI parsing capabilites
and can live with this patch not applied even though it does no harm.
Sebastian
Hi Sebastian and Andrew,
On sam., nov. 28 2015, Andrew Lunn <[email protected]> wrote:
>>On Sat, Nov 28, 2015 at 12:14:05PM +0100, Sebastian Hesselbarth wrote:
>> Current NAND node has an additional flash partition for the whole
>> flash overlapping with real partitions. Remove this partition as
>> the whole flash is already represented by the NAND device itself.
>>
>> Signed-off-by: Sebastian Hesselbarth <[email protected]>
>
> Given Benoit comment
>
> Acked-by: Andrew Lunn <[email protected]>
Applied on mvebu/dt
Thanks,
Gregory
>
> Andrew
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
Hi Sebastian,
On sam., nov. 28 2015, Sebastian Hesselbarth <[email protected]> wrote:
> NAND flash partitions should be part of a partitions sub-node
> not the flash node itself. Move the partitions which will also
> allow different bootloaders get rid of the stock partitions
> easily by removing the partitions node.
>
> Signed-off-by: Sebastian Hesselbarth <[email protected]>
Applied on mvebu/dt
Thanks,
Gregory
> ---
> Cc: Jason Cooper <[email protected]>
> Cc: Andrew Lunn <[email protected]>
> Cc: Gregory Clement <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Pawel Moll <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Ian Campbell <[email protected]>
> Cc: Kumar Gala <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: Benoit Masson <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts | 67 +++++++++++++------------
> 1 file changed, 36 insertions(+), 31 deletions(-)
>
> diff --git a/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts b/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
> index 30a0a6eac645..76781fd18624 100644
> --- a/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
> +++ b/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
> @@ -151,37 +151,42 @@
> marvell,nand-enable-arbiter;
> nand-on-flash-bbt;
>
> - partition@0 {
> - label = "u-boot";
> - reg = <0x0000000 0xe0000>;
> - read-only;
> - };
> -
> - partition@e0000 {
> - label = "u-boot-env";
> - reg = <0xe0000 0x20000>;
> - read-only;
> - };
> -
> - partition@100000 {
> - label = "u-boot-env2";
> - reg = <0x100000 0x20000>;
> - read-only;
> - };
> -
> - partition@120000 {
> - label = "zImage";
> - reg = <0x120000 0x400000>;
> - };
> -
> - partition@520000 {
> - label = "initrd";
> - reg = <0x520000 0x400000>;
> - };
> -
> - partition@xE00000 {
> - label = "boot";
> - reg = <0xE00000 0x3F200000>;
> + partitions {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + partition@0 {
> + label = "u-boot";
> + reg = <0x0000000 0xe0000>;
> + read-only;
> + };
> +
> + partition@e0000 {
> + label = "u-boot-env";
> + reg = <0xe0000 0x20000>;
> + read-only;
> + };
> +
> + partition@100000 {
> + label = "u-boot-env2";
> + reg = <0x100000 0x20000>;
> + read-only;
> + };
> +
> + partition@120000 {
> + label = "zImage";
> + reg = <0x120000 0x400000>;
> + };
> +
> + partition@520000 {
> + label = "initrd";
> + reg = <0x520000 0x400000>;
> + };
> +
> + partition@xE00000 {
> + label = "boot";
> + reg = <0xE00000 0x3F200000>;
> + };
> };
> };
> };
> --
> 2.1.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
Hi Sebastian and Andrew,
On sam., nov. 28 2015, Andrew Lunn <[email protected]> wrote:
> On Sat, Nov 28, 2015 at 12:14:07PM +0100, Sebastian Hesselbarth wrote:
>> Prefix all partition reg properties to 32-bit to ease readability.
>> While at it, also remove a stale x in front of boot partition
>> offset and make some upper-case hex numbers lower-case.
>>
>> Signed-off-by: Sebastian Hesselbarth <[email protected]>
>
> Acked-by: Andrew Lunn <[email protected]>
Applied on mvebu/dt
Thanks,
Gregory
>
> Andrew
>
>> ---
>> Cc: Jason Cooper <[email protected]>
>> Cc: Andrew Lunn <[email protected]>
>> Cc: Gregory Clement <[email protected]>
>> Cc: Rob Herring <[email protected]>
>> Cc: Pawel Moll <[email protected]>
>> Cc: Mark Rutland <[email protected]>
>> Cc: Ian Campbell <[email protected]>
>> Cc: Kumar Gala <[email protected]>
>> Cc: Russell King <[email protected]>
>> Cc: Benoit Masson <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> ---
>> arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts | 14 +++++++-------
>> 1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts b/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
>> index 76781fd18624..13cf69a8d0fb 100644
>> --- a/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
>> +++ b/arch/arm/boot/dts/armada-xp-lenovo-ix4-300d.dts
>> @@ -157,35 +157,35 @@
>>
>> partition@0 {
>> label = "u-boot";
>> - reg = <0x0000000 0xe0000>;
>> + reg = <0x00000000 0x000e0000>;
>> read-only;
>> };
>>
>> partition@e0000 {
>> label = "u-boot-env";
>> - reg = <0xe0000 0x20000>;
>> + reg = <0x000e0000 0x00020000>;
>> read-only;
>> };
>>
>> partition@100000 {
>> label = "u-boot-env2";
>> - reg = <0x100000 0x20000>;
>> + reg = <0x00100000 0x00020000>;
>> read-only;
>> };
>>
>> partition@120000 {
>> label = "zImage";
>> - reg = <0x120000 0x400000>;
>> + reg = <0x00120000 0x00400000>;
>> };
>>
>> partition@520000 {
>> label = "initrd";
>> - reg = <0x520000 0x400000>;
>> + reg = <0x00520000 0x00400000>;
>> };
>>
>> - partition@xE00000 {
>> + partition@e00000 {
>> label = "boot";
>> - reg = <0xE00000 0x3F200000>;
>> + reg = <0x00e00000 0x3f200000>;
>> };
>> };
>> };
>> --
>> 2.1.4
>>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
Hi Sebastian,
On lun., nov. 30 2015, Sebastian Hesselbarth <[email protected]> wrote:
> On 29.11.2015 15:35, Thomas Petazzoni wrote:
>> Adding Ezequiel Garcia in Cc.
>>
>> On Sat, 28 Nov 2015 12:14:08 +0100, Sebastian Hesselbarth wrote:
>>> The NAND device found on Lenovo ix4-300d uses 4-bit BCH ECC protection.
>>> Add the corresponding properties to the NAND node.
>>
>> If the ONFI information from the NAND flash say that it requires 4 bits
>> per 512, then there should be no need to add this information to the
>> Device Tree as the pxa3xx_nand driver by default uses the ONFI
>> information.
>
> Thomas,
>
> as said in the cover letter, this is also DT cleanup with barebox
> bootloader in mind. I do not accept what Linux' pxa3xx_nand driver
> is doing as a reference here ;)
>
>> Those properties are only needed when for some reason the vendor has
>> chosen to use a ECC strength that doesn't match with the one advertised
>> by the flash in its ONFI information (either stronger or weaker). But
>> in this case, your commit log is confusing, because it says that the
>> "NAND device ... uses 4-bit BCH ECC protection". If it really does,
>> then the patch is not needed :-)
>
> I agree that if ONFI is already advertising 4/512 ECC (and it is), we
> do not need the properties. Anyway, IIRC barebox does not yet properly
> parse ONFI or at least it does not derive minimum ECC settings from it.
>
> I'll have to have a closer look at barebox' ONFI parsing capabilites
> and can live with this patch not applied even though it does no harm.
So for now, I don't apply it.
Thanks,
Gregory
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com