[PATCH 0/5] collected fallout of porting an ATSAMA5D27 based board
Hi,
while porting an ATSAMA5D2 based board (booting from NAND flash with UBI) I stumbled over generic, NAND- and UBI related asperity:
1. tiny-printf does not handle NULL arguments to '%s' in a proper way 2. vtbl_check() has an useless debug output due to a typo 3. NAND: An informative output fails badly, if the NAND vendor is unknown 4. the nand_atmel driver lacks a vital initialisation
OK, you might say the nand_atmel NAND driver is obsolete, but it was the only solution to get booting from NAND running. The new DM based NAND driver refused to do anything usefull, so I dropped it after spending a couple of days debugging it:
<RANT>brilliant!</RANT>
regards Benedikt Spranger
Benedikt Spranger (5): tiny-printf: Handle NULL pointer argument to %s drivers/mtd/ubispl/ubispl.c: Fix error message mtd: nand: raw: Fix potential NULL pointer dereference mtd: nand: Update NAND manufacturer Ids mtd: nand: raw: atmel_nand: Add missing nand_scan_ident()
drivers/mtd/nand/raw/atmel_nand.c | 4 ++++ drivers/mtd/nand/raw/nand_base.c | 8 ++++---- drivers/mtd/nand/raw/nand_ids.c | 22 ++++++++++++---------- drivers/mtd/ubispl/ubispl.c | 2 +- include/linux/mtd/rawnand.h | 23 +++++++++++++---------- lib/tiny-printf.c | 2 +- 6 files changed, 35 insertions(+), 26 deletions(-)
Hi,
while porting an ATSAMA5D2 based board (booting from NAND flash with UBI) I stumbled over generic, NAND- and UBI related asperity:
1. tiny-printf does not handle NULL arguments to '%s' in a proper way 2. vtbl_check() has an useless debug output due to a typo 3. NAND: An informative output fails badly, if the NAND vendor is unknown 4. the nand_atmel driver lacks a vital initialisation
OK, you might say the nand_atmel NAND driver is obsolete, but it was the only solution to get booting from NAND running. The new DM based NAND driver refused to do anything usefull, so I dropped it after spending a couple of days debugging it:
<RANT>brilliant!</RANT>
regards Benedikt Spranger
Benedikt Spranger (5): tiny-printf: Handle NULL pointer argument to %s drivers/mtd/ubispl/ubispl.c: Fix error message mtd: nand: raw: Fix potential NULL pointer dereference mtd: nand: Update NAND manufacturer Ids mtd: nand: raw: atmel_nand: Add missing nand_scan_ident()
drivers/mtd/nand/raw/atmel_nand.c | 4 ++++ drivers/mtd/nand/raw/nand_base.c | 8 ++++---- drivers/mtd/nand/raw/nand_ids.c | 22 ++++++++++++---------- drivers/mtd/ubispl/ubispl.c | 2 +- include/linux/mtd/rawnand.h | 23 +++++++++++++---------- lib/tiny-printf.c | 2 +- 6 files changed, 35 insertions(+), 26 deletions(-)
On 10/18/24 11:30, Benedikt Spranger wrote:
Hi,
while porting an ATSAMA5D2 based board (booting from NAND flash with UBI) I stumbled over generic, NAND- and UBI related asperity:
- tiny-printf does not handle NULL arguments to '%s' in a proper way
- vtbl_check() has an useless debug output due to a typo
- NAND: An informative output fails badly, if the NAND vendor is unknown
- the nand_atmel driver lacks a vital initialisation
OK, you might say the nand_atmel NAND driver is obsolete, but it was the only solution to get booting from NAND running. The new DM based NAND driver refused to do anything usefull, so I dropped it after spending a couple of days debugging it:
<RANT>brilliant!</RANT>
regards Benedikt Spranger
Benedikt Spranger (5): tiny-printf: Handle NULL pointer argument to %s drivers/mtd/ubispl/ubispl.c: Fix error message mtd: nand: raw: Fix potential NULL pointer dereference mtd: nand: Update NAND manufacturer Ids mtd: nand: raw: atmel_nand: Add missing nand_scan_ident()
drivers/mtd/nand/raw/atmel_nand.c | 4 ++++ drivers/mtd/nand/raw/nand_base.c | 8 ++++---- drivers/mtd/nand/raw/nand_ids.c | 22 ++++++++++++---------- drivers/mtd/ubispl/ubispl.c | 2 +- include/linux/mtd/rawnand.h | 23 +++++++++++++---------- lib/tiny-printf.c | 2 +- 6 files changed, 35 insertions(+), 26 deletions(-)
Coming back to these patches I had a look and except a typo , it looks good. I can take these through at91 tree or maybe Michael you want to have a look as they touch the MTD/NAND drivers ?
Thanks, Eugen
On 11/12/24 15:45, Eugen Hristev wrote:
On 10/18/24 11:30, Benedikt Spranger wrote:
Hi,
while porting an ATSAMA5D2 based board (booting from NAND flash with UBI) I stumbled over generic, NAND- and UBI related asperity:
- tiny-printf does not handle NULL arguments to '%s' in a proper way
- vtbl_check() has an useless debug output due to a typo
- NAND: An informative output fails badly, if the NAND vendor is unknown
- the nand_atmel driver lacks a vital initialisation
OK, you might say the nand_atmel NAND driver is obsolete, but it was the only solution to get booting from NAND running. The new DM based NAND driver refused to do anything usefull, so I dropped it after spending a couple of days debugging it:
<RANT>brilliant!</RANT>
regards Benedikt Spranger
Benedikt Spranger (5): tiny-printf: Handle NULL pointer argument to %s drivers/mtd/ubispl/ubispl.c: Fix error message mtd: nand: raw: Fix potential NULL pointer dereference mtd: nand: Update NAND manufacturer Ids mtd: nand: raw: atmel_nand: Add missing nand_scan_ident()
drivers/mtd/nand/raw/atmel_nand.c | 4 ++++ drivers/mtd/nand/raw/nand_base.c | 8 ++++---- drivers/mtd/nand/raw/nand_ids.c | 22 ++++++++++++---------- drivers/mtd/ubispl/ubispl.c | 2 +- include/linux/mtd/rawnand.h | 23 +++++++++++++---------- lib/tiny-printf.c | 2 +- 6 files changed, 35 insertions(+), 26 deletions(-)
Coming back to these patches I had a look and except a typo , it looks good. I can take these through at91 tree or maybe Michael you want to have a look as they touch the MTD/NAND drivers ?
Thanks, Eugen
I have applied patches 1 and 2 to u-boot-at91/next . The other three would need a resubmit .
Eugen
HI all
On Tue, Nov 19, 2024 at 4:46 PM Eugen Hristev eugen.hristev@linaro.org wrote:
On 11/12/24 15:45, Eugen Hristev wrote:
On 10/18/24 11:30, Benedikt Spranger wrote:
Hi,
while porting an ATSAMA5D2 based board (booting from NAND flash with UBI) I stumbled over generic, NAND- and UBI related asperity:
- tiny-printf does not handle NULL arguments to '%s' in a proper way
- vtbl_check() has an useless debug output due to a typo
- NAND: An informative output fails badly, if the NAND vendor is unknown
- the nand_atmel driver lacks a vital initialisation
OK, you might say the nand_atmel NAND driver is obsolete, but it was the only solution to get booting from NAND running. The new DM based NAND driver refused to do anything usefull, so I dropped it after spending a couple of days debugging it:
<RANT>brilliant!</RANT>
I totally missed those patches. I'm really concerned about it. I will review the respin
Michael
regards Benedikt Spranger
Benedikt Spranger (5): tiny-printf: Handle NULL pointer argument to %s drivers/mtd/ubispl/ubispl.c: Fix error message mtd: nand: raw: Fix potential NULL pointer dereference mtd: nand: Update NAND manufacturer Ids mtd: nand: raw: atmel_nand: Add missing nand_scan_ident()
drivers/mtd/nand/raw/atmel_nand.c | 4 ++++ drivers/mtd/nand/raw/nand_base.c | 8 ++++---- drivers/mtd/nand/raw/nand_ids.c | 22 ++++++++++++---------- drivers/mtd/ubispl/ubispl.c | 2 +- include/linux/mtd/rawnand.h | 23 +++++++++++++---------- lib/tiny-printf.c | 2 +- 6 files changed, 35 insertions(+), 26 deletions(-)
Coming back to these patches I had a look and except a typo , it looks good. I can take these through at91 tree or maybe Michael you want to have a look as they touch the MTD/NAND drivers ?
Thanks, Eugen
I have applied patches 1 and 2 to u-boot-at91/next . The other three would need a resubmit .
Eugen
A NULL pointer argument to %s causes a NULL pointer dereference in the fixed width numerical printout code, since p is overwritten with NULL. In case of %s width is 0. Check width before dereferencing the pointer.
Signed-off-by: Benedikt Spranger b.spranger@linutronix.de Reviewed-by: John Ogness john.ogness@linutronix.de --- lib/tiny-printf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c index 9a70c6095b3..b2f31c4004a 100644 --- a/lib/tiny-printf.c +++ b/lib/tiny-printf.c @@ -311,7 +311,7 @@ static int _vprintf(struct printf_info *info, const char *fmt, va_list va)
*info->bf = 0; info->bf = p; - while (*info->bf++ && width > 0) + while (width > 0 && info->bf && *info->bf++) width--; while (width-- > 0) info->putc(info, lz ? '0' : ' ');
The bad CRC error message has transposed characters, which render the output useless:
"bad CRC at record 213: #08x, not #08x" instead of "bad CRC at record 213: #00000000, not #4be31f4d"
Fix the error message.
Signed-off-by: Benedikt Spranger b.spranger@linutronix.de Reviewed-by: John Ogness john.ogness@linutronix.de --- drivers/mtd/ubispl/ubispl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/ubispl/ubispl.c b/drivers/mtd/ubispl/ubispl.c index 90a7c4c6f9e..9face5fae15 100644 --- a/drivers/mtd/ubispl/ubispl.c +++ b/drivers/mtd/ubispl/ubispl.c @@ -113,7 +113,7 @@ static int vtbl_check(struct ubi_scan_info *ubi,
crc = crc32(UBI_CRC32_INIT, &vtbl[i], UBI_VTBL_RECORD_SIZE_CRC); if (be32_to_cpu(vtbl[i].crc) != crc) { - ubi_err("bad CRC at record %u: %#08x, not %#08x", + ubi_err("bad CRC at record %u: #%08x, not #%08x", i, crc, be32_to_cpu(vtbl[i].crc)); ubi_dump_vtbl_record(&vtbl[i], i); return 1;
A NAND manufacture ID may not be found in nand_manuf_ids[] database. In case of an ONFI or JEDEC NAND a crutial NULL pointer check is missing and printing out the manufacture name result in a NULL pointer dereference. Instead of adding additional NULL pointer checks ensure a valid nand_menufacture_desc and remove the checks entirely.
Signed-off-by: Benedikt Spranger b.spranger@linutronix.de Reviewed-by: John Ogness john.ogness@linutronix.de --- drivers/mtd/nand/raw/nand_base.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c index 4401bdcdb90..5dbc47bfe5f 100644 --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -4241,7 +4241,7 @@ static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip, * @id: manufacturer ID * * Returns a nand_manufacturer_desc object if the manufacturer is defined - * in the NAND manufacturers database, NULL otherwise. + * in the NAND manufacturers database, "Unknown" entry otherwise. */ static const struct nand_manufacturer *nand_get_manufacturer_desc(u8 id) { @@ -4252,7 +4252,7 @@ static const struct nand_manufacturer *nand_get_manufacturer_desc(u8 id) return &nand_manuf_ids[i]; }
- return NULL; + return &nand_manuf_ids[i]; }
/* @@ -4425,13 +4425,13 @@ ident_done: else if (chip->jedec_version) pr_info("%s %s\n", manufacturer_desc->name, chip->jedec_params.model); - else if (manufacturer_desc) + else pr_info("%s %s\n", manufacturer_desc->name, type->name); #else if (chip->jedec_version) pr_info("%s %s\n", manufacturer_desc->name, chip->jedec_params.model); - else if (manufacturer_desc) + else pr_info("%s %s\n", manufacturer_desc->name, type->name); #endif
Align manufacturer Ids with the Id list from Linux kernel v6.11. While at it, sort the entries in alphabetical order.
Signed-off-by: Benedikt Spranger b.spranger@linutronix.de Reviewed-by: John Ogness john.ogness@linutronix.de --- drivers/mtd/nand/raw/nand_ids.c | 22 ++++++++++++---------- include/linux/mtd/rawnand.h | 23 +++++++++++++---------- 2 files changed, 25 insertions(+), 20 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_ids.c b/drivers/mtd/nand/raw/nand_ids.c index 4f46378ffe1..e40facf774c 100644 --- a/drivers/mtd/nand/raw/nand_ids.c +++ b/drivers/mtd/nand/raw/nand_ids.c @@ -180,20 +180,22 @@ struct nand_flash_dev nand_flash_ids[] = {
/* Manufacturer IDs */ struct nand_manufacturer nand_manuf_ids[] = { - {NAND_MFR_TOSHIBA, "Toshiba", &toshiba_nand_manuf_ops}, - {NAND_MFR_SAMSUNG, "Samsung", &samsung_nand_manuf_ops}, + {NAND_MFR_AMD, "AMD/Spansion", &amd_nand_manuf_ops}, + {NAND_MFR_ATO, "ATO"}, + {NAND_MFR_EON, "Eon"}, + {NAND_MFR_EON, "ESMT"}, {NAND_MFR_FUJITSU, "Fujitsu"}, - {NAND_MFR_NATIONAL, "National"}, - {NAND_MFR_RENESAS, "Renesas"}, - {NAND_MFR_STMICRO, "ST Micro"}, {NAND_MFR_HYNIX, "Hynix", &hynix_nand_manuf_ops}, - {NAND_MFR_MICRON, "Micron", µn_nand_manuf_ops}, - {NAND_MFR_AMD, "AMD/Spansion", &amd_nand_manuf_ops}, + {NAND_MFR_INTEL, "Intel"}, {NAND_MFR_MACRONIX, "Macronix", ¯onix_nand_manuf_ops}, - {NAND_MFR_EON, "Eon"}, + {NAND_MFR_MICRON, "Micron", µn_nand_manuf_ops}, + {NAND_MFR_NATIONAL, "National"}, + {NAND_MFR_RENESAS, "Renesas"}, + {NAND_MFR_SAMSUNG, "Samsung", &samsung_nand_manuf_ops}, {NAND_MFR_SANDISK, "SanDisk"}, - {NAND_MFR_INTEL, "Intel"}, - {NAND_MFR_ATO, "ATO"}, + {NAND_MFR_STMICRO, "ST Micro"}, + {NAND_MFR_TOSHIBA, "Toshiba", &toshiba_nand_manuf_ops}, + {NAND_MFR_WINBOND, "Winbond"}, {0x0, "Unknown"} };
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h index 537c62424a1..eaa6ec84b07 100644 --- a/include/linux/mtd/rawnand.h +++ b/include/linux/mtd/rawnand.h @@ -1030,20 +1030,23 @@ static inline void *nand_get_manufacturer_data(struct nand_chip *chip) /* * NAND Flash Manufacturer ID Codes */ -#define NAND_MFR_TOSHIBA 0x98 -#define NAND_MFR_SAMSUNG 0xec +#define NAND_MFR_AMD 0x01 +#define NAND_MFR_ATO 0x9b +#define NAND_MFR_EON 0x92 +#define NAND_MFR_ESMT 0xc8 #define NAND_MFR_FUJITSU 0x04 -#define NAND_MFR_NATIONAL 0x8f -#define NAND_MFR_RENESAS 0x07 -#define NAND_MFR_STMICRO 0x20 #define NAND_MFR_HYNIX 0xad -#define NAND_MFR_MICRON 0x2c -#define NAND_MFR_AMD 0x01 +#define NAND_MFR_INTEL 0x89 #define NAND_MFR_MACRONIX 0xc2 -#define NAND_MFR_EON 0x92 +#define NAND_MFR_MICRON 0x2c +#define NAND_MFR_NATIONAL 0x8f +#define NAND_MFR_RENESAS 0x07 +#define NAND_MFR_SAMSUNG 0xec #define NAND_MFR_SANDISK 0x45 -#define NAND_MFR_INTEL 0x89 -#define NAND_MFR_ATO 0x9b +#define NAND_MFR_STMICRO 0x20 +/* Kioxia is new name of Toshiba memory. */ +#define NAND_MFR_TOSHIBA 0x98 +#define NAND_MFR_WINBOND 0xef
/* The maximum expected count of bytes in the NAND ID sequence */ #define NAND_MAX_ID_LEN 8
Hi Benedikt ,
On 10/18/24 11:30, Benedikt Spranger wrote:
Align manufacturer Ids with the Id list from Linux kernel v6.11. While at it, sort the entries in alphabetical order.
Signed-off-by: Benedikt Spranger b.spranger@linutronix.de Reviewed-by: John Ogness john.ogness@linutronix.de
drivers/mtd/nand/raw/nand_ids.c | 22 ++++++++++++---------- include/linux/mtd/rawnand.h | 23 +++++++++++++---------- 2 files changed, 25 insertions(+), 20 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_ids.c b/drivers/mtd/nand/raw/nand_ids.c index 4f46378ffe1..e40facf774c 100644 --- a/drivers/mtd/nand/raw/nand_ids.c +++ b/drivers/mtd/nand/raw/nand_ids.c @@ -180,20 +180,22 @@ struct nand_flash_dev nand_flash_ids[] = {
/* Manufacturer IDs */ struct nand_manufacturer nand_manuf_ids[] = {
- {NAND_MFR_TOSHIBA, "Toshiba", &toshiba_nand_manuf_ops},
- {NAND_MFR_SAMSUNG, "Samsung", &samsung_nand_manuf_ops},
- {NAND_MFR_AMD, "AMD/Spansion", &amd_nand_manuf_ops},
- {NAND_MFR_ATO, "ATO"},
- {NAND_MFR_EON, "Eon"},
- {NAND_MFR_EON, "ESMT"},
Did you mean NAND_MFR_ESMT here ?
{NAND_MFR_FUJITSU, "Fujitsu"},
- {NAND_MFR_NATIONAL, "National"},
- {NAND_MFR_RENESAS, "Renesas"},
- {NAND_MFR_STMICRO, "ST Micro"}, {NAND_MFR_HYNIX, "Hynix", &hynix_nand_manuf_ops},
- {NAND_MFR_MICRON, "Micron", µn_nand_manuf_ops},
- {NAND_MFR_AMD, "AMD/Spansion", &amd_nand_manuf_ops},
- {NAND_MFR_INTEL, "Intel"}, {NAND_MFR_MACRONIX, "Macronix", ¯onix_nand_manuf_ops},
- {NAND_MFR_EON, "Eon"},
- {NAND_MFR_MICRON, "Micron", µn_nand_manuf_ops},
- {NAND_MFR_NATIONAL, "National"},
- {NAND_MFR_RENESAS, "Renesas"},
- {NAND_MFR_SAMSUNG, "Samsung", &samsung_nand_manuf_ops}, {NAND_MFR_SANDISK, "SanDisk"},
- {NAND_MFR_INTEL, "Intel"},
- {NAND_MFR_ATO, "ATO"},
- {NAND_MFR_STMICRO, "ST Micro"},
- {NAND_MFR_TOSHIBA, "Toshiba", &toshiba_nand_manuf_ops},
- {NAND_MFR_WINBOND, "Winbond"}, {0x0, "Unknown"}
};
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h index 537c62424a1..eaa6ec84b07 100644 --- a/include/linux/mtd/rawnand.h +++ b/include/linux/mtd/rawnand.h @@ -1030,20 +1030,23 @@ static inline void *nand_get_manufacturer_data(struct nand_chip *chip) /*
- NAND Flash Manufacturer ID Codes
*/ -#define NAND_MFR_TOSHIBA 0x98 -#define NAND_MFR_SAMSUNG 0xec +#define NAND_MFR_AMD 0x01 +#define NAND_MFR_ATO 0x9b +#define NAND_MFR_EON 0x92 +#define NAND_MFR_ESMT 0xc8 #define NAND_MFR_FUJITSU 0x04 -#define NAND_MFR_NATIONAL 0x8f -#define NAND_MFR_RENESAS 0x07 -#define NAND_MFR_STMICRO 0x20 #define NAND_MFR_HYNIX 0xad -#define NAND_MFR_MICRON 0x2c -#define NAND_MFR_AMD 0x01 +#define NAND_MFR_INTEL 0x89 #define NAND_MFR_MACRONIX 0xc2 -#define NAND_MFR_EON 0x92 +#define NAND_MFR_MICRON 0x2c +#define NAND_MFR_NATIONAL 0x8f +#define NAND_MFR_RENESAS 0x07 +#define NAND_MFR_SAMSUNG 0xec #define NAND_MFR_SANDISK 0x45 -#define NAND_MFR_INTEL 0x89 -#define NAND_MFR_ATO 0x9b +#define NAND_MFR_STMICRO 0x20 +/* Kioxia is new name of Toshiba memory. */ +#define NAND_MFR_TOSHIBA 0x98 +#define NAND_MFR_WINBOND 0xef
/* The maximum expected count of bytes in the NAND ID sequence */ #define NAND_MAX_ID_LEN 8
On Tue, 12 Nov 2024 15:39:23 +0200 Eugen Hristev eugen.hristev@linaro.org wrote:
Hi Eugen,
On 10/18/24 11:30, Benedikt Spranger wrote:
Align manufacturer Ids with the Id list from Linux kernel v6.11. While at it, sort the entries in alphabetical order.
Signed-off-by: Benedikt Spranger b.spranger@linutronix.de Reviewed-by: John Ogness john.ogness@linutronix.de
drivers/mtd/nand/raw/nand_ids.c | 22 ++++++++++++---------- include/linux/mtd/rawnand.h | 23 +++++++++++++---------- 2 files changed, 25 insertions(+), 20 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_ids.c b/drivers/mtd/nand/raw/nand_ids.c index 4f46378ffe1..e40facf774c 100644 --- a/drivers/mtd/nand/raw/nand_ids.c +++ b/drivers/mtd/nand/raw/nand_ids.c @@ -180,20 +180,22 @@ struct nand_flash_dev nand_flash_ids[] = {
/* Manufacturer IDs */ struct nand_manufacturer nand_manuf_ids[] = {
- {NAND_MFR_TOSHIBA, "Toshiba", &toshiba_nand_manuf_ops},
- {NAND_MFR_SAMSUNG, "Samsung", &samsung_nand_manuf_ops},
- {NAND_MFR_AMD, "AMD/Spansion", &amd_nand_manuf_ops},
- {NAND_MFR_ATO, "ATO"},
- {NAND_MFR_EON, "Eon"},
- {NAND_MFR_EON, "ESMT"},
Did you mean NAND_MFR_ESMT here ?
Yes, fatfingered that...
Regards Benedikt Spranger
On 11/12/24 15:57, Benedikt Spranger wrote:
On Tue, 12 Nov 2024 15:39:23 +0200 Eugen Hristev eugen.hristev@linaro.org wrote:
Hi Eugen,
On 10/18/24 11:30, Benedikt Spranger wrote:
Align manufacturer Ids with the Id list from Linux kernel v6.11. While at it, sort the entries in alphabetical order.
Signed-off-by: Benedikt Spranger b.spranger@linutronix.de Reviewed-by: John Ogness john.ogness@linutronix.de
drivers/mtd/nand/raw/nand_ids.c | 22 ++++++++++++---------- include/linux/mtd/rawnand.h | 23 +++++++++++++---------- 2 files changed, 25 insertions(+), 20 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_ids.c b/drivers/mtd/nand/raw/nand_ids.c index 4f46378ffe1..e40facf774c 100644 --- a/drivers/mtd/nand/raw/nand_ids.c +++ b/drivers/mtd/nand/raw/nand_ids.c @@ -180,20 +180,22 @@ struct nand_flash_dev nand_flash_ids[] = {
/* Manufacturer IDs */ struct nand_manufacturer nand_manuf_ids[] = {
- {NAND_MFR_TOSHIBA, "Toshiba", &toshiba_nand_manuf_ops},
- {NAND_MFR_SAMSUNG, "Samsung", &samsung_nand_manuf_ops},
- {NAND_MFR_AMD, "AMD/Spansion", &amd_nand_manuf_ops},
- {NAND_MFR_ATO, "ATO"},
- {NAND_MFR_EON, "Eon"},
- {NAND_MFR_EON, "ESMT"},
Did you mean NAND_MFR_ESMT here ?
Yes, fatfingered that...
Regards Benedikt Spranger
Unfortunately you will need a new revision anyway.
I get this while building sama5d4ek_nandflash_defconfig
arm-none-linux-gnueabihf-ld.bfd: drivers/mtd/nand/raw/nand_base.o: in function `nand_reset_data_interface': ..drivers/mtd/nand/raw/nand_base.c:948:(.text.nand_reset+0xc): undefined reference to `nand_get_default_data_interface' arm-none-linux-gnueabihf-ld.bfd: drivers/mtd/nand/raw/nand_base.o: in function `nand_flash_detect_onfi': ..drivers/mtd/nand/raw/nand_base.c:3884:(.text.nand_detect+0x318): undefined reference to `nand_manuf_ids' arm-none-linux-gnueabihf-ld.bfd: drivers/mtd/nand/raw/nand_base.c:3884:(.text.nand_detect+0x31c): undefined reference to `nand_flash_ids' arm-none-linux-gnueabihf-ld.bfd: drivers/mtd/nand/raw/nand_base.o: in function `nand_init_data_interface': drivers/mtd/nand/raw/nand_base.c:1047:(.text.nand_scan_ident+0x1ec): undefined reference to `onfi_init_data_interface' make[1]: *** [scripts/Makefile.xpl:542: spl/u-boot-spl] Error 1
In board_nand_init() the used NAND flash is not evaluated. This left vital parts of internal structures uninitialized and SPL NAND flash access is broken.
Add the missing nand_scan_ident() to board_nand_init().
Signed-off-by: Benedikt Spranger b.spranger@linutronix.de Reviewed-by: John Ogness john.ogness@linutronix.de --- drivers/mtd/nand/raw/atmel_nand.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/mtd/nand/raw/atmel_nand.c b/drivers/mtd/nand/raw/atmel_nand.c index 4dbf7b47135..cf7ac54ea13 100644 --- a/drivers/mtd/nand/raw/atmel_nand.c +++ b/drivers/mtd/nand/raw/atmel_nand.c @@ -1423,6 +1423,10 @@ int board_nand_init(struct nand_chip *nand) nand->bbt_options |= NAND_BBT_USE_FLASH; #endif
+ ret = nand_scan_ident(mtd, CONFIG_SYS_NAND_MAX_CHIPS, NULL); + if (ret) + return ret; + #ifdef CONFIG_ATMEL_NAND_HWECC #ifdef CONFIG_ATMEL_NAND_HW_PMECC ret = atmel_pmecc_nand_init_params(nand, mtd);
Hello Benedikt,
Am Fri, Oct 18, 2024 at 10:30:00AM +0200 schrieb Benedikt Spranger:
Hi,
while porting an ATSAMA5D2 based board (booting from NAND flash with UBI) I stumbled over generic, NAND- and UBI related asperity:
- tiny-printf does not handle NULL arguments to '%s' in a proper way
- vtbl_check() has an useless debug output due to a typo
- NAND: An informative output fails badly, if the NAND vendor is unknown
- the nand_atmel driver lacks a vital initialisation
OK, you might say the nand_atmel NAND driver is obsolete, but it was the only solution to get booting from NAND running. The new DM based NAND driver refused to do anything usefull, so I dropped it after spending a couple of days debugging it:
Strange. I have at least three different boards with SAMA5D27 successfully booting from NAND flash with the new DM based driver (unfortunately none of them upstreamed, which also won't change in the foreseeable future, sorry).
Do you use at91bootstrap as 2nd level bootloader like me or something else? For me it was basically getting the dts for U-Boot correct, but I got that running with all U-Boot releases since 2023.04 up to 2024.10. Could you share some more detail?
<RANT>brilliant!</RANT>
Did you talk with Microchip about it?
Greets Alex
regards Benedikt Spranger
Benedikt Spranger (5): tiny-printf: Handle NULL pointer argument to %s drivers/mtd/ubispl/ubispl.c: Fix error message mtd: nand: raw: Fix potential NULL pointer dereference mtd: nand: Update NAND manufacturer Ids mtd: nand: raw: atmel_nand: Add missing nand_scan_ident()
drivers/mtd/nand/raw/atmel_nand.c | 4 ++++ drivers/mtd/nand/raw/nand_base.c | 8 ++++---- drivers/mtd/nand/raw/nand_ids.c | 22 ++++++++++++---------- drivers/mtd/ubispl/ubispl.c | 2 +- include/linux/mtd/rawnand.h | 23 +++++++++++++---------- lib/tiny-printf.c | 2 +- 6 files changed, 35 insertions(+), 26 deletions(-)
-- 2.45.2
On Fri, 18 Oct 2024 15:11:20 +0200 Alexander Dahl ada@thorsis.com wrote:
Hello Alexander,
Am Fri, Oct 18, 2024 at 10:30:00AM +0200 schrieb Benedikt Spranger:
OK, you might say the nand_atmel NAND driver is obsolete, but it was the only solution to get booting from NAND running. The new DM based NAND driver refused to do anything usefull, so I dropped it after spending a couple of days debugging it:
Strange. I have at least three different boards with SAMA5D27 successfully booting from NAND flash with the new DM based driver (unfortunately none of them upstreamed, which also won't change in the foreseeable future, sorry).
OK.
Do you use at91bootstrap as 2nd level bootloader like me or something else?
I use the U-Boot SPL. There is no UBI support in at91boostrap. There were some attemps, but... No interest at the at91bootstrap side.
For me it was basically getting the dts for U-Boot correct, but I got that running with all U-Boot releases since 2023.04 up to 2024.10. Could you share some more detail?
I face all the trouble in SPL. And since the SPL is the essential part here (UBI support) I gave up at one point. I simply couldn't any usefull read data from the NAND flash, but all 0.
Regards Benedikt Spranger
Hello Benedikt,
Am Fri, Oct 18, 2024 at 04:19:08PM +0200 schrieb Benedikt Spranger:
On Fri, 18 Oct 2024 15:11:20 +0200 Alexander Dahl ada@thorsis.com wrote:
Hello Alexander,
Am Fri, Oct 18, 2024 at 10:30:00AM +0200 schrieb Benedikt Spranger:
OK, you might say the nand_atmel NAND driver is obsolete, but it was the only solution to get booting from NAND running. The new DM based NAND driver refused to do anything usefull, so I dropped it after spending a couple of days debugging it:
Strange. I have at least three different boards with SAMA5D27 successfully booting from NAND flash with the new DM based driver (unfortunately none of them upstreamed, which also won't change in the foreseeable future, sorry).
OK.
Do you use at91bootstrap as 2nd level bootloader like me or something else?
I use the U-Boot SPL. There is no UBI support in at91boostrap. There were some attemps, but... No interest at the at91bootstrap side.
Yeah, been there. Wanted at91bootstrap to boot from SPI-NOR which is _not_ Quad-SPI, and the feature request was closed with "if you wanna do it by yourself, ask our support for help". :-/
For me it was basically getting the dts for U-Boot correct, but I got that running with all U-Boot releases since 2023.04 up to 2024.10. Could you share some more detail?
I face all the trouble in SPL. And since the SPL is the essential part here (UBI support) I gave up at one point. I simply couldn't any usefull read data from the NAND flash, but all 0.
You can not store bootstrap or SPL in UBI, it must sit in first sector of raw NAND, I suppose that's how you load SPL? Did you try reading/writing raw NAND from there? I'm not familiar with SPL, does it consider dts? I could share my dts parts if that helps you.
(What I do here: SoC loads at91bootstrap from raw NAND at offset 0, at91bootstrap loads U-Boot from raw NAND at some offset like 0x20000, U-Boot (proper) loads everything else from UBI.)
Greets Alex
Hello,
On 10/21/24 09:03, Alexander Dahl wrote:
Hello Benedikt,
Am Fri, Oct 18, 2024 at 04:19:08PM +0200 schrieb Benedikt Spranger:
On Fri, 18 Oct 2024 15:11:20 +0200 Alexander Dahl ada@thorsis.com wrote:
Hello Alexander,
Am Fri, Oct 18, 2024 at 10:30:00AM +0200 schrieb Benedikt Spranger:
OK, you might say the nand_atmel NAND driver is obsolete, but it was the only solution to get booting from NAND running. The new DM based NAND driver refused to do anything usefull, so I dropped it after spending a couple of days debugging it:
Strange. I have at least three different boards with SAMA5D27 successfully booting from NAND flash with the new DM based driver (unfortunately none of them upstreamed, which also won't change in the foreseeable future, sorry).
OK.
Do you use at91bootstrap as 2nd level bootloader like me or something else?
I use the U-Boot SPL. There is no UBI support in at91boostrap. There were some attemps, but... No interest at the at91bootstrap side.
It's not about interest. There are two main reasons: First, all open source UBI stacks are GPL. At91bootstrap is not GPL and it cannot take GPL restricted code (being MIT ). Second, all UBI stacks are huge in terms of code and memory. At91bootstrap is a second stage that should fit in 16k , 32k or 64k depending on the platform. So far this restriction could not be met by any UBI implementation. ... and yes, doing a custom in-house small UBI implementation would be challenging, and costly.
Yeah, been there. Wanted at91bootstrap to boot from SPI-NOR which is _not_ Quad-SPI, and the feature request was closed with "if you wanna do it by yourself, ask our support for help". :-/
iirc at91bootstrap can boot from raw spi-nor (yes, not UBI)
For me it was basically getting the dts for U-Boot correct, but I got that running with all U-Boot releases since 2023.04 up to 2024.10. Could you share some more detail?
I face all the trouble in SPL. And since the SPL is the essential part here (UBI support) I gave up at one point. I simply couldn't any usefull read data from the NAND flash, but all 0.
You can not store bootstrap or SPL in UBI, it must sit in first sector of raw NAND, I suppose that's how you load SPL? Did you try reading/writing raw NAND from there? I'm not familiar with SPL, does it consider dts? I could share my dts parts if that helps you.
SPL should use dts. However on at91 platforms, SPL is dangerously close to the SRAM limit where it should fit, and it has been challenging in the last years to either make it fit or remove it.
(What I do here: SoC loads at91bootstrap from raw NAND at offset 0, at91bootstrap loads U-Boot from raw NAND at some offset like 0x20000, U-Boot (proper) loads everything else from UBI.)
Either way, it is interesting to see that the U-boot proper NAND dm driver does not work without at91bootstrap. There may be NAND quirks in at91bootstrap board init that may influence this (pins resets, timing delays, etc).
You can test NAND driver by booting at91bootstrap and Uboot from another medium, like sd-card. In this way, at91bootstrap will not touch NAND device, and you have a clean U-boot booted.
Eugen
Greets Alex
Hello Eugen,
Am Mon, Oct 21, 2024 at 10:17:54AM +0300 schrieb Eugen Hristev:
Hello,
On 10/21/24 09:03, Alexander Dahl wrote:
Hello Benedikt,
Am Fri, Oct 18, 2024 at 04:19:08PM +0200 schrieb Benedikt Spranger:
On Fri, 18 Oct 2024 15:11:20 +0200 Alexander Dahl ada@thorsis.com wrote:
Hello Alexander,
Am Fri, Oct 18, 2024 at 10:30:00AM +0200 schrieb Benedikt Spranger:
OK, you might say the nand_atmel NAND driver is obsolete, but it was the only solution to get booting from NAND running. The new DM based NAND driver refused to do anything usefull, so I dropped it after spending a couple of days debugging it:
Strange. I have at least three different boards with SAMA5D27 successfully booting from NAND flash with the new DM based driver (unfortunately none of them upstreamed, which also won't change in the foreseeable future, sorry).
OK.
Do you use at91bootstrap as 2nd level bootloader like me or something else?
I use the U-Boot SPL. There is no UBI support in at91boostrap. There were some attemps, but... No interest at the at91bootstrap side.
It's not about interest. There are two main reasons: First, all open source UBI stacks are GPL. At91bootstrap is not GPL and it cannot take GPL restricted code (being MIT ). Second, all UBI stacks are huge in terms of code and memory. At91bootstrap is a second stage that should fit in 16k , 32k or 64k depending on the platform. So far this restriction could not be met by any UBI implementation. ... and yes, doing a custom in-house small UBI implementation would be challenging, and costly.
I suspected technical reasons, but nice to know it's also licensing.
(Side note: To increase robustness we considered storing bootloaders (and maybe U-Boot env) in SPI-NOR flash with longer data retention, and use UBI on raw NAND for everything else.)
Yeah, been there. Wanted at91bootstrap to boot from SPI-NOR which is _not_ Quad-SPI, and the feature request was closed with "if you wanna do it by yourself, ask our support for help". :-/
iirc at91bootstrap can boot from raw spi-nor (yes, not UBI)
Speaking of at91bootstrap 3.x and later: It can, but only if you use the QSPI port, it does not work with flexcom for example. See https://github.com/linux4sam/at91bootstrap/issues/181 for reference.
For me it was basically getting the dts for U-Boot correct, but I got that running with all U-Boot releases since 2023.04 up to 2024.10. Could you share some more detail?
I face all the trouble in SPL. And since the SPL is the essential part here (UBI support) I gave up at one point. I simply couldn't any usefull read data from the NAND flash, but all 0.
You can not store bootstrap or SPL in UBI, it must sit in first sector of raw NAND, I suppose that's how you load SPL? Did you try reading/writing raw NAND from there? I'm not familiar with SPL, does it consider dts? I could share my dts parts if that helps you.
SPL should use dts. However on at91 platforms, SPL is dangerously close to the SRAM limit where it should fit, and it has been challenging in the last years to either make it fit or remove it.
(What I do here: SoC loads at91bootstrap from raw NAND at offset 0, at91bootstrap loads U-Boot from raw NAND at some offset like 0x20000, U-Boot (proper) loads everything else from UBI.)
Either way, it is interesting to see that the U-boot proper NAND dm driver does not work without at91bootstrap. There may be NAND quirks in at91bootstrap board init that may influence this (pins resets, timing delays, etc).
Think this is a misunderstanding here. I have this combination running, and from the discussions on NAND issues on at91 on this list I think at least two developers at Microchip, too. From my understanding U-Boot proper completely (re)initializes everything required to get NAND to work. I can not speak about U-Boot SPL though, never tried that.
You can test NAND driver by booting at91bootstrap and Uboot from another medium, like sd-card. In this way, at91bootstrap will not touch NAND device, and you have a clean U-boot booted.
Lets hear what Benedikt has tried. ;-)
Greets Alex
Yeah, been there. Wanted at91bootstrap to boot from SPI-NOR which is _not_ Quad-SPI, and the feature request was closed with "if you wanna do it by yourself, ask our support for help". :-/
iirc at91bootstrap can boot from raw spi-nor (yes, not UBI)
Speaking of at91bootstrap 3.x and later: It can, but only if you use the QSPI port, it does not work with flexcom for example. See https://github.com/linux4sam/at91bootstrap/issues/181 for reference.
Judging by the answer, we are both right, my memory was saying it works, and apparently for some older SoCs it's also implemented, while not for sam9x60 at the moment. I remember using a flexcom in SPI mode was fairly simple and should work out of the box with the existing SPI driver in at91bootstrap, just that the flexcom mode has to be correctly configured, pinout, etc.
On Mon, 21 Oct 2024 10:17:54 +0300 Eugen Hristev eugen.hristev@linaro.org wrote:
Hi,
Do you use at91bootstrap as 2nd level bootloader like me or something else?
I use the U-Boot SPL. There is no UBI support in at91boostrap. There were some attemps, but... No interest at the at91bootstrap side.
It's not about interest. There are two main reasons: First, all open source UBI stacks are GPL. At91bootstrap is not GPL and it cannot take GPL restricted code (being MIT ).
I do not recall any issue mentioned about a license issue, when I send out UBI patches for the AT91boostrap over a decade ago. It was a simple "nice, I put it in a branch, but we do not need it." I followed the UBI topic in AT91bootstrap loosely after that.
Second, all UBI stacks are huge in terms of code and memory.
OK. Maybe we have a different view on huge. The board I am supporting now is the successor of a AT91SAM9263 board. This (old) board use AT91boootstrap with UBI support. BTW: The original requirement for UBI support was to fit into 4k (PPC 440 bootloader).
At91bootstrap is a second stage that should fit in 16k , 32k or 64k depending on the platform. So far this restriction could not be met by any UBI implementation.
OK. This need some more in depth view: A SPL needs UBI read support only. This support can be done in less then 4k, as I mentioned before. So UBI full read/write support? You're right. It does not fit.
Needed read support? It definitly fit (and works fine for over a decade).
... and yes, doing a custom in-house small UBI implementation would be challenging, and costly.
If you look at ubispl, you got some clue, what's needed. The UBI support to the AT91boostrap (for the AT91SAM9263) is about ~450 LOC. Managable, but why should I do it, when U-Boot SPL does the job...
(What I do here: SoC loads at91bootstrap from raw NAND at offset 0, at91bootstrap loads U-Boot from raw NAND at some offset like 0x20000, U-Boot (proper) loads everything else from UBI.)
Either way, it is interesting to see that the U-boot proper NAND dm driver does not work without at91bootstrap.
To be clear: The U-Boot *SPL* does not work with the NAND dm driver.
The old NAND driver does. So from a developers view I can spend some spare cycles on that, from a overall project view: we use the old driver, since it is working.
Regards Benedikt Spranger
On 10/21/24 15:30, Benedikt Spranger wrote:
On Mon, 21 Oct 2024 10:17:54 +0300 Eugen Hristev eugen.hristev@linaro.org wrote:
Hi,
Do you use at91bootstrap as 2nd level bootloader like me or something else?
I use the U-Boot SPL. There is no UBI support in at91boostrap. There were some attemps, but... No interest at the at91bootstrap side.
It's not about interest. There are two main reasons: First, all open source UBI stacks are GPL. At91bootstrap is not GPL and it cannot take GPL restricted code (being MIT ).
I do not recall any issue mentioned about a license issue, when I send out UBI patches for the AT91boostrap over a decade ago. It was a simple "nice, I put it in a branch, but we do not need it." I followed the UBI topic in AT91bootstrap loosely after that.
The patches that I saw ( I do not recall whether they were yours or not) while I was maintaining the project were GPL and about 4000 LOC. It was adding more than 12k to the size.
Second, all UBI stacks are huge in terms of code and memory.
OK. Maybe we have a different view on huge. The board I am supporting now is the successor of a AT91SAM9263 board. This (old) board use AT91boootstrap with UBI support. BTW: The original requirement for UBI support was to fit into 4k (PPC 440 bootloader).
At91bootstrap is a second stage that should fit in 16k , 32k or 64k depending on the platform. So far this restriction could not be met by any UBI implementation.
OK. This need some more in depth view: A SPL needs UBI read support only. This support can be done in less then 4k, as I mentioned before. So UBI full read/write support? You're right. It does not fit.
Needed read support? It definitly fit (and works fine for over a decade).
I am glad you see you have an implementation that works. I am no longer maintaining that project so I cannot say anything about the possibility of ever merging it.
... and yes, doing a custom in-house small UBI implementation would be challenging, and costly.
If you look at ubispl, you got some clue, what's needed. The UBI support to the AT91boostrap (for the AT91SAM9263) is about ~450 LOC. Managable, but why should I do it, when U-Boot SPL does the job...
I can understand that. Nobody asked you to do it.
(What I do here: SoC loads at91bootstrap from raw NAND at offset 0, at91bootstrap loads U-Boot from raw NAND at some offset like 0x20000, U-Boot (proper) loads everything else from UBI.)
Either way, it is interesting to see that the U-boot proper NAND dm driver does not work without at91bootstrap.
To be clear: The U-Boot *SPL* does not work with the NAND dm driver.
The old NAND driver does. So from a developers view I can spend some spare cycles on that, from a overall project view: we use the old driver, since it is working.
Ok, so my understanding was wrong, in fact you cannot use NAND dm in the SPL. It may be that there are some missing components from U-boot proper that are not built in (or some quirk/initialization) So this can be looked into by the people interested, and I can understand that you have a solution that works for you.
Regards Benedikt Spranger
Am Mon, 21 Oct 2024 08:03:35 +0200 schrieb Alexander Dahl ada@thorsis.com:
Hello Benedikt,
Am Fri, Oct 18, 2024 at 04:19:08PM +0200 schrieb Benedikt Spranger:
On Fri, 18 Oct 2024 15:11:20 +0200 Alexander Dahl ada@thorsis.com wrote:
Hello Alexander,
Am Fri, Oct 18, 2024 at 10:30:00AM +0200 schrieb Benedikt Spranger:
OK, you might say the nand_atmel NAND driver is obsolete, but it was the only solution to get booting from NAND running. The new DM based NAND driver refused to do anything usefull, so I dropped it after spending a couple of days debugging it:
Strange. I have at least three different boards with SAMA5D27 successfully booting from NAND flash with the new DM based driver (unfortunately none of them upstreamed, which also won't change in the foreseeable future, sorry).
OK.
Do you use at91bootstrap as 2nd level bootloader like me or something else?
I use the U-Boot SPL. There is no UBI support in at91boostrap. There were some attemps, but... No interest at the at91bootstrap side.
Yeah, been there. Wanted at91bootstrap to boot from SPI-NOR which is _not_ Quad-SPI, and the feature request was closed with "if you wanna do it by yourself, ask our support for help". :-/
For me it was basically getting the dts for U-Boot correct, but I got that running with all U-Boot releases since 2023.04 up to 2024.10. Could you share some more detail?
I face all the trouble in SPL. And since the SPL is the essential part here (UBI support) I gave up at one point. I simply couldn't any usefull read data from the NAND flash, but all 0.
You can not store bootstrap or SPL in UBI, it must sit in first sector of raw NAND, I suppose that's how you load SPL?
Sorry for not beeing that clear. The SPL sits in the first Page in NAND flash. It does it's magic and then load the U-Boot payload from an UBI volume (Or directly load a kernel). Then it "starts" the payload.
Did you try reading/writing raw NAND from there?
I try to read from SPL. That's all what ubispl wants. No need for write support here.
I'm not familiar with SPL, does it consider dts?
It consider dts.
I could share my dts parts if that helps you.
That would be great. I definitly give it a try. So here is a quick extract (EBI is also used otherwise) of what I did. Hopefully I did not forget anything...
--- a/arch/arm/dts/at91-sama5d27_ABC.dts +++ b/arch/arm/dts/at91-sama5d27_ABC.dts @@ -63,7 +63,62 @@ bias-disable; bootph-all; }; + + pinctrl_nand0_default: nand0_default { + pinmux = + <PIN_PA0__D0>, + <PIN_PA1__D1>, + <PIN_PA2__D2>, + <PIN_PA3__D3>, + <PIN_PA4__D4>, + <PIN_PA5__D5>, + <PIN_PA6__D6>, + <PIN_PA7__D7>, + <PIN_PA8__NWE_NANDWE>, + <PIN_PA9__NCS3>, + <PIN_PA10__A21_NANDALE>, + <PIN_PA11__A22_NANDCLE>, + <PIN_PA12__NRD_NANDOE>, + <PIN_PA21__NANDRDY>; + bias-disable; + }; }; }; }; }; + +&ebi { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_nand0_default>; + status = "okay"; + + nand_controller: nand-controller { + status = "okay"; + + nand@3 { + reg = <0x3 0x0 0x800000>; + //atmel,rb = <0>; + nand-bus-width = <8>; + nand-ecc-mode = "hw"; + nand-ecc-strength = <4>; + nand-ecc-step-size = <512>; + nand-on-flash-bbt; + + partitions { + compatible = "fixed-partitions"; + #address-cells = <1>; + #size-cells = <1>; + + spl@0 { + label = "spl"; + reg = <0x0 0x40000>; + }; + + ubi@40000 { + label = "UBI"; + reg = <0x40000 0x0>; + }; + }; + }; + }; +};
(What I do here: SoC loads at91bootstrap from raw NAND at offset 0, at91bootstrap loads U-Boot from raw NAND at some offset like 0x20000, U-Boot (proper) loads everything else from UBI.)
From my experience that can cause significant headache starting after 5-6 years in use. That's what I try to avoid :)
Regards Benedikt Spranger
Hello Benedikt,
Am Mon, Oct 21, 2024 at 12:20:17PM +0200 schrieb Benedikt Spranger:
The SPL sits in the first Page in NAND flash. It does it's magic and then load the U-Boot payload from an UBI volume (Or directly load a kernel). Then it "starts" the payload.
Did you try reading/writing raw NAND from there?
I try to read from SPL. That's all what ubispl wants. No need for write support here.
I'm not familiar with SPL, does it consider dts?
It consider dts.
I could share my dts parts if that helps you.
That would be great. I definitly give it a try. So here is a quick extract (EBI is also used otherwise) of what I did.
Using EBI for accessing an FPGA here on one of three boards. So I'll share that dts excerpt from my v2024.10 branch.
Hopefully I did not forget anything...
--- a/arch/arm/dts/at91-sama5d27_ABC.dts +++ b/arch/arm/dts/at91-sama5d27_ABC.dts @@ -63,7 +63,62 @@ bias-disable; bootph-all; };
pinctrl_nand0_default: nand0_default {pinmux =<PIN_PA0__D0>,<PIN_PA1__D1>,<PIN_PA2__D2>,<PIN_PA3__D3>,<PIN_PA4__D4>,<PIN_PA5__D5>,<PIN_PA6__D6>,<PIN_PA7__D7>,<PIN_PA8__NWE_NANDWE>,<PIN_PA9__NCS3>,<PIN_PA10__A21_NANDALE>,<PIN_PA11__A22_NANDCLE>,<PIN_PA12__NRD_NANDOE>,<PIN_PA21__NANDRDY>;bias-disable;}; }; }; };};
+&ebi {
pinctrl-names = "default";pinctrl-0 = <&pinctrl_nand0_default>;status = "okay";nand_controller: nand-controller {status = "okay";nand@3 {reg = <0x3 0x0 0x800000>;//atmel,rb = <0>;nand-bus-width = <8>;nand-ecc-mode = "hw";nand-ecc-strength = <4>;nand-ecc-step-size = <512>;nand-on-flash-bbt;partitions {compatible = "fixed-partitions";#address-cells = <1>;#size-cells = <1>;spl@0 {label = "spl";reg = <0x0 0x40000>;};ubi@40000 {label = "UBI";reg = <0x40000 0x0>;};};};};+};
I have no partitions defined, but that should not hurt. Split pinctrl settings here, but that's basically because I have more child nodes on the EBI node in Linux which I don't use in U-Boot here. Should not make such a difference.
Do you have external pull-ups for the A21_NANDALE, A22_NANDCLE, NANDRDY, and NCS3 lines? Maybe cross check that with the board code you probably have for the old non-DM NAND driver.
Now my example:
&ebi { pinctrl-names = "default"; pinctrl-0 = <&pinctrl_ebi_default>; status = "okay";
nand_controller: nand-controller { pinctrl-names = "default"; pinctrl-0 = <&pinctrl_nand_default>; status = "okay";
nand@3 { reg = <0x3 0x0 0x800000>; atmel,rb = <0>;
nand-bus-width = <8>; nand-ecc-mode = "hw"; nand-on-flash-bbt; nand-ecc-strength = <8>; nand-ecc-step-size = <512>; label = "atmel_nand"; }; }; };
&pioA { pinctrl_ebi_default: ebi_default { rd_we_data { pinmux = <PIN_PA22__D0>, <PIN_PA23__D1>, <PIN_PA24__D2>, <PIN_PA25__D3>, <PIN_PA26__D4>, <PIN_PA27__D5>, <PIN_PA28__D6>, <PIN_PA29__D7>, <PIN_PA30__NWE_NANDWE>, <PIN_PB2__NRD_NANDOE>; bias-disable; atmel,drive-strength = <ATMEL_PIO_DRVSTR_ME>; }; };
pinctrl_nand_default: nand_default { ale_cle_rdy_cs { pinmux = <PIN_PB0__A21_NANDALE>, <PIN_PB1__A22_NANDCLE>, <PIN_PC8__NANDRDY>, <PIN_PA31__NCS3>; bias-pull-up; }; }; };
Notice mine uses IO Set 1 here, but your's should work with IO Set 2 as well.
(What I do here: SoC loads at91bootstrap from raw NAND at offset 0, at91bootstrap loads U-Boot from raw NAND at some offset like 0x20000, U-Boot (proper) loads everything else from UBI.)
From my experience that can cause significant headache starting after 5-6 years in use. That's what I try to avoid :)
I'm aware of that. The data retention time for the NAND flash used here is 10 years according to datasheet. No big deal on Linux when running ubihealthd for everything stored in UBI, but everything outside of that is about to fail some day. :-/
Greets Alex
participants (4)
-
Alexander Dahl -
Benedikt Spranger -
Eugen Hristev -
Michael Nazzareno Trimarchi