[U-Boot] [PATCH] sunxi_nand_spl: Be smarter about where to look for backup u-boot.bin

[U-Boot] [PATCH] sunxi_nand_spl: Be smarter about where to look for backup u-boot.bin

[Hans de Goede]

We know when u-boot is written to its own partition, in this case the
layout always is:

eb 0 spl
eb 1 spl-backup
eb 2 u-boot
eb 3 u-boot-backup

eb: erase-block

So if we cannot load u-boot from its primary offset we know exactly where
to look for it.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mtd/nand/sunxi_nand_spl.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/sunxi_nand_spl.c b/drivers/mtd/nand/sunxi_nand_spl.c
index bf9b1b1..00770f3 100644
--- a/drivers/mtd/nand/sunxi_nand_spl.c
+++ b/drivers/mtd/nand/sunxi_nand_spl.c
@@ -355,18 +355,31 @@ static int nand_read_buffer(uint32_t offs, unsigned int size, void *dest,
 
 int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest)
 {
+#if CONFIG_SYS_NAND_U_BOOT_OFFS == CONFIG_SPL_PAD_TO
+	/*
+	 * u-boot-dtb.bin appended to SPL, use syndrome (like the BROM does)
+	 * and try different erase block sizes to find the backup.
+	 */
 	const uint32_t boot_offsets[] = {
 		0 * 1024 * 1024 + CONFIG_SYS_NAND_U_BOOT_OFFS,
 		1 * 1024 * 1024 + CONFIG_SYS_NAND_U_BOOT_OFFS,
 		2 * 1024 * 1024 + CONFIG_SYS_NAND_U_BOOT_OFFS,
 		4 * 1024 * 1024 + CONFIG_SYS_NAND_U_BOOT_OFFS,
 	};
-	int i, syndrome;
-
-	if (CONFIG_SYS_NAND_U_BOOT_OFFS == CONFIG_SPL_PAD_TO)
-		syndrome = 1; /* u-boot-dtb.bin appended to SPL */
-	else
-		syndrome = 0; /* u-boot-dtb.bin on its own partition */
+	const int syndrome = 1;
+#else
+	/*
+	 * u-boot-dtb.bin on its own partition, do not use syndrome, u-boot
+	 * partition sits after 2 eraseblocks (spl, spl-backup), look for
+	 * backup u-boot 1 erase block further.
+	 */
+	const uint32_t boot_offsets[] = {
+		CONFIG_SYS_NAND_U_BOOT_OFFS,
+		CONFIG_SYS_NAND_U_BOOT_OFFS + CONFIG_SYS_NAND_U_BOOT_OFFS / 2,
+	};
+	const int syndrome = 0;
+#endif
+	int i;
 
 	if (offs == CONFIG_SYS_NAND_U_BOOT_OFFS) {
 		for (i = 0; i < ARRAY_SIZE(boot_offsets); i++) {
-- 
2.4.3

[U-Boot] [PATCH] sunxi_nand_spl: Be smarter about where to look for backup u-boot.bin

[Ian Campbell]

On Mon, 2015-08-31 at 17:42 +0200, Hans de Goede wrote:
> We know when u-boot is written to its own partition, in this case the
> layout always is:
> 
> eb 0 spl
> eb 1 spl-backup
> eb 2 u-boot
> eb 3 u-boot-backup
> 
> eb: erase-block

These are all the same size on this particular chip/all known sunxi
SoCs, right?

> So if we cannot load u-boot from its primary offset we know exactly where
> to look for it.
> +#else
> +> 	> /*
> +> 	>  * u-boot-dtb.bin on its own partition, do not use syndrome, u-boot
> +> 	>  * partition sits after 2 eraseblocks (spl, spl-backup), look for
> +> 	>  * backup u-boot 1 erase block further.
> +> 	>  */
> +> 	> const uint32_t boot_offsets[] = {
> +> 	> 	> CONFIG_SYS_NAND_U_BOOT_OFFS,
> +> 	> 	> CONFIG_SYS_NAND_U_BOOT_OFFS + CONFIG_SYS_NAND_U_BOOT_OFFS / 2,
> +> 	> };
> +> 	> const int syndrome = 0;
> +#endif

The relationship between half of SYS_NAND_U_BOOT_OFFS and an erase
block here is a bit non-obvious in this context (the offset is 2 blocks
so one block is half that).

Is there no suitable #define for the eb size which could be used here
(and ideally in the definition of U_BOOT_OFFS too)? Could we add one?

Ian.

[U-Boot] [PATCH] sunxi_nand_spl: Be smarter about where to look for backup u-boot.bin

[Hans de Goede]

Hi,

On 01-09-15 09:05, Ian Campbell wrote:
> On Mon, 2015-08-31 at 17:42 +0200, Hans de Goede wrote:
>> We know when u-boot is written to its own partition, in this case the
>> layout always is:
>>
>> eb 0 spl
>> eb 1 spl-backup
>> eb 2 u-boot
>> eb 3 u-boot-backup
>>
>> eb: erase-block
>
> These are all the same size on this particular chip/all known sunxi
> SoCs, right?

Nope, erase-block sizes is a property of the nand flash used, not of
the SoC, so this can vary per board (and on some boards like the
mk802 of which many production runs where done chances are it may
vary between production runs...).

>> So if we cannot load u-boot from its primary offset we know exactly where
>> to look for it.
>> +#else
>> +> 	> /*
>> +> 	>  * u-boot-dtb.bin on its own partition, do not use syndrome, u-boot
>> +> 	>  * partition sits after 2 eraseblocks (spl, spl-backup), look for
>> +> 	>  * backup u-boot 1 erase block further.
>> +> 	>  */
>> +> 	> const uint32_t boot_offsets[] = {
>> +> 	> 	> CONFIG_SYS_NAND_U_BOOT_OFFS,
>> +> 	> 	> CONFIG_SYS_NAND_U_BOOT_OFFS + CONFIG_SYS_NAND_U_BOOT_OFFS / 2,
>> +> 	> };
>> +> 	> const int syndrome = 0;
>> +#endif
>
> The relationship between half of SYS_NAND_U_BOOT_OFFS and an erase
> block here is a bit non-obvious in this context (the offset is 2 blocks
> so one block is half that).
>
> Is there no suitable #define for the eb size which could be used here
> (and ideally in the definition of U_BOOT_OFFS too)? Could we add one?

This is all still developing a bit some testing has shown that it is possible
for u-boot.bin to suffer from non correctable ecc errors while it sits in
a non bad erase block (looks like the erase block should actually be marked
bad, but isn't). In cases like this it may help to write multiple copies of
u-boot at 1MiB offsets (since u-boot will fit in 1 MiB) so maybe I need to
revise this patch to use 1MiB offsets instead.

So for now we should probably just let this patch sit in my wip branch for a
bit until we've a better handle on this.

Regards,

Hans







>
> Ian.
>

[U-Boot] [PATCH] sunxi_nand_spl: Be smarter about where to look for backup u-boot.bin

[Ian Campbell]

On Tue, 2015-09-01 at 10:52 +0200, Hans de Goede wrote:
> Hi,
> 
> On 01-09-15 09:05, Ian Campbell wrote:
> > On Mon, 2015-08-31 at 17:42 +0200, Hans de Goede wrote:
> >> We know when u-boot is written to its own partition, in this case 
> the
> >> layout always is:
> >>
> >> eb 0 spl
> >> eb 1 spl-backup
> >> eb 2 u-boot
> >> eb 3 u-boot-backup
> >>
> >> eb: erase-block
> >
> > These are all the same size on this particular chip/all known sunxi
> > SoCs, right?
> 
> Nope, erase-block sizes is a property of the nand flash used, not of
> the SoC, so this can vary per board (and on some boards like the
> mk802 of which many production runs where done chances are it may
> vary between production runs...).

This is what I started out thinking but somehow convinced myself the
NAND was on the SoC itself... Partly because the division used in the
code below assumes some amount of uniformity (or at least some
relationship between the offset of eb2 and its size).

> >> So if we cannot load u-boot from its primary offset we know exactly where
> >> to look for it.
> >> +#else
> >> +> > 	> > /*
> >> +> > 	> >  * u-boot-dtb.bin on its own partition, do not use syndrome, u-boot
> >> +> > 	> >  * partition sits after 2 eraseblocks (spl, spl-backup), look for
> >> +> > 	> >  * backup u-boot 1 erase block further.
> >> +> > 	> >  */
> >> +> > 	> > const uint32_t boot_offsets[] = {
> >> +> > 	> > > 	> > CONFIG_SYS_NAND_U_BOOT_OFFS,
> >> +> > 	> > > 	> > CONFIG_SYS_NAND_U_BOOT_OFFS + CONFIG_SYS_NAND_U_BOOT_OFFS / 2,
> >> +> > 	> > };
> >> +> > 	> > const int syndrome = 0;
> >> +#endif
> >
> > The relationship between half of SYS_NAND_U_BOOT_OFFS and an erase
> > block here is a bit non-obvious in this context (the offset is 2 blocks
> > so one block is half that).
> >
> > Is there no suitable #define for the eb size which could be used here
> > (and ideally in the definition of U_BOOT_OFFS too)? Could we add one?
> 
> This is all still developing a bit some testing has shown that it is possible
> for u-boot.bin to suffer from non correctable ecc errors while it sits in
> a non bad erase block (looks like the erase block should actually be marked
> bad, but isn't). In cases like this it may help to write multiple copies of
> u-boot at 1MiB offsets (since u-boot will fit in 1 MiB) so maybe I need to
> revise this patch to use 1MiB offsets instead.

Sure.

> So for now we should probably just let this patch sit in my wip branch for a
> bit until we've a better handle on this.

OK.

> 
> Regards,
> 
> Hans
> 
> 
> 
> 
> 
> 
> 
> >
> > Ian.
> >
>