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

[U-Boot] [PATCH v2] 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>
---
Changes in v2:
-Add an eraseblock_size helper variable to make the calculation for finding
 the backup u-boot easier to understand
---
 drivers/mtd/nand/sunxi_nand_spl.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/sunxi_nand_spl.c b/drivers/mtd/nand/sunxi_nand_spl.c
index 5985534..b0e07aa 100644
--- a/drivers/mtd/nand/sunxi_nand_spl.c
+++ b/drivers/mtd/nand/sunxi_nand_spl.c
@@ -356,18 +356,32 @@ 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 eraseblock_size = CONFIG_SYS_NAND_U_BOOT_OFFS / 2;
+	const uint32_t boot_offsets[] = {
+		CONFIG_SYS_NAND_U_BOOT_OFFS,
+		CONFIG_SYS_NAND_U_BOOT_OFFS + eraseblock_size,
+	};
+	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 v2] sunxi_nand_spl: Be smarter about where to look for backup u-boot.bin

[Ian Campbell]

On Sun, 2015-09-20 at 15:39 -0400, 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
> 
> So if we cannot load u-boot from its primary offset we know exactly where
> to look for it.

Is it worth noting here (or perhaps in a code comment?) that the code
currently assumes that the first four ebs are of uniform size?

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Add an eraseblock_size helper variable to make the calculation for
> finding
>  the backup u-boot easier to understand
> ---
>  drivers/mtd/nand/sunxi_nand_spl.c | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/nand/sunxi_nand_spl.c
> b/drivers/mtd/nand/sunxi_nand_spl.c
> index 5985534..b0e07aa 100644
> --- a/drivers/mtd/nand/sunxi_nand_spl.c
> +++ b/drivers/mtd/nand/sunxi_nand_spl.c
> @@ -356,18 +356,32 @@ 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 eraseblock_size = CONFIG_SYS_NAND_U_BOOT_OFFS /
> 2;
> +	const uint32_t boot_offsets[] = {
> +		CONFIG_SYS_NAND_U_BOOT_OFFS,
> +		CONFIG_SYS_NAND_U_BOOT_OFFS + eraseblock_size,
> +	};
> +	const int syndrome = 0;
> +#endif
> +	int i;
>  
>  	if (offs == CONFIG_SYS_NAND_U_BOOT_OFFS) {
>  		for (i = 0; i < ARRAY_SIZE(boot_offsets); i++) {

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

[Hans de Goede]

Hi,

On 21-09-15 12:22, Ian Campbell wrote:
> On Sun, 2015-09-20 at 15:39 -0400, 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
>>
>> So if we cannot load u-boot from its primary offset we know exactly where
>> to look for it.
>
> Is it worth noting here (or perhaps in a code comment?) that the code
> currently assumes that the first four ebs are of uniform size?

The eraseblock size is a property of the nand, given a certain nand chip,
all eraseblocks on that chip always have the same size.

Regards,

Hans


>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> -Add an eraseblock_size helper variable to make the calculation for
>> finding
>>   the backup u-boot easier to understand
>> ---
>>   drivers/mtd/nand/sunxi_nand_spl.c | 26 ++++++++++++++++++++------
>>   1 file changed, 20 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/sunxi_nand_spl.c
>> b/drivers/mtd/nand/sunxi_nand_spl.c
>> index 5985534..b0e07aa 100644
>> --- a/drivers/mtd/nand/sunxi_nand_spl.c
>> +++ b/drivers/mtd/nand/sunxi_nand_spl.c
>> @@ -356,18 +356,32 @@ 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 eraseblock_size = CONFIG_SYS_NAND_U_BOOT_OFFS /
>> 2;
>> +	const uint32_t boot_offsets[] = {
>> +		CONFIG_SYS_NAND_U_BOOT_OFFS,
>> +		CONFIG_SYS_NAND_U_BOOT_OFFS + eraseblock_size,
>> +	};
>> +	const int syndrome = 0;
>> +#endif
>> +	int i;
>>
>>   	if (offs == CONFIG_SYS_NAND_U_BOOT_OFFS) {
>>   		for (i = 0; i < ARRAY_SIZE(boot_offsets); i++) {

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

[Ian Campbell]

On Mon, 2015-09-21 at 13:24 +0200, Hans de Goede wrote:
> Hi,
> 
> On 21-09-15 12:22, Ian Campbell wrote:
> > On Sun, 2015-09-20 at 15:39 -0400, 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
> > > 
> > > So if we cannot load u-boot from its primary offset we know exactly
> > > where
> > > to look for it.
> > 
> > Is it worth noting here (or perhaps in a code comment?) that the code
> > currently assumes that the first four ebs are of uniform size?
> 
> The eraseblock size is a property of the nand, given a certain nand chip,
> all eraseblocks on that chip always have the same size.

So they never have "boot erase zones" at either the start or end, which
divide one of the erase zones into more fine-grained sizes?

e.g. it used to be the case with NOR that with, say, a 128KB device you
would have zones of 3x32K, 1x16K, 2x8K or something like that rather than
simply 4x32K.

They don't do this with NAND then?

Ian.

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

[Hans de Goede]

Hi,

On 21-09-15 14:19, Ian Campbell wrote:
> On Mon, 2015-09-21 at 13:24 +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 21-09-15 12:22, Ian Campbell wrote:
>>> On Sun, 2015-09-20 at 15:39 -0400, 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
>>>>
>>>> So if we cannot load u-boot from its primary offset we know exactly
>>>> where
>>>> to look for it.
>>>
>>> Is it worth noting here (or perhaps in a code comment?) that the code
>>> currently assumes that the first four ebs are of uniform size?
>>
>> The eraseblock size is a property of the nand, given a certain nand chip,
>> all eraseblocks on that chip always have the same size.
>
> So they never have "boot erase zones" at either the start or end, which
> divide one of the erase zones into more fine-grained sizes?
>
> e.g. it used to be the case with NOR that with, say, a 128KB device you
> would have zones of 3x32K, 1x16K, 2x8K or something like that rather than
> simply 4x32K.
>
> They don't do this with NAND then?

To the best of my knowledge no.

Regards,

Hans

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

[Ian Campbell]

On Mon, 2015-09-21 at 15:19 +0200, Hans de Goede wrote:
> Hi,
> 
> On 21-09-15 14:19, Ian Campbell wrote:
> > On Mon, 2015-09-21 at 13:24 +0200, Hans de Goede wrote:
> > > Hi,
> > > 
> > > On 21-09-15 12:22, Ian Campbell wrote:
> > > > On Sun, 2015-09-20 at 15:39 -0400, 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
> > > > > 
> > > > > So if we cannot load u-boot from its primary offset we know
> > > > > exactly
> > > > > where
> > > > > to look for it.
> > > > 
> > > > Is it worth noting here (or perhaps in a code comment?) that the
> > > > code
> > > > currently assumes that the first four ebs are of uniform size?
> > > 
> > > The eraseblock size is a property of the nand, given a certain nand
> > > chip,
> > > all eraseblocks on that chip always have the same size.
> > 
> > So they never have "boot erase zones" at either the start or end, which
> > divide one of the erase zones into more fine-grained sizes?
> > 
> > e.g. it used to be the case with NOR that with, say, a 128KB device you
> > would have zones of 3x32K, 1x16K, 2x8K or something like that rather
> > than
> > simply 4x32K.
> > 
> > They don't do this with NAND then?
> 
> To the best of my knowledge no.

Thanks.

In which case:
Acked-by: Ian Campbell <ijc@hellion.org.uk>