The sunxi SPL NAND controller driver supports use 'BootROM'-like configs, that is, configs where the ECC bytes and real data are interleaved in the page instead of putting ECC bytes in the OOB area.

Doing that has several drawbacks: - since you're interleaving data and ECC bytes you can't use the whole page otherwise you might override the bad block marker with non-FF bytes. - to solve the bad block marker problem, the ROM code supports partially using the page, but this introduces a huge penalty both in term of read speed and NAND memory usage. While this is fine for rather small binaries(like the SPL one which is at maximum 24KB large), it becomes non-negligible for the bootloader image (several hundred of KB). - auto-detection of the page size is not reliable (this is in my opinion the biggest problem). If you get the page size wrong, you'll end up reading data at a different offset than what was specified by the caller and the reading may succeed (if valid data were written at this address).

For all those reasons I think it's wiser to completely remove support for 'syndrome' configs. If we ever need to support it again, then I'd recommend specifying all the config parameters through Kconfig options.

Signed-off-by: Boris Brezillon boris.brezillon@free-electrons.com --- drivers/mtd/nand/sunxi_nand_spl.c | 56 ++++++++++----------------------------- 1 file changed, 14 insertions(+), 42 deletions(-)

diff --git a/drivers/mtd/nand/sunxi_nand_spl.c b/drivers/mtd/nand/sunxi_nand_spl.c index b0e07aa..1739da2 100644 --- a/drivers/mtd/nand/sunxi_nand_spl.c +++ b/drivers/mtd/nand/sunxi_nand_spl.c @@ -119,9 +119,6 @@ const uint16_t random_seed[128] = { 0x7c57, 0x0fbe, 0x46ce, 0x4939, 0x6b17, 0x37bb, 0x3e91, 0x76db, };

-/* random seed used for syndrome calls */ -const uint16_t random_seed_syndrome = 0x4a80; - #define MAX_RETRIES 10

static int check_value_inner(int offset, int expected_bits, @@ -183,7 +180,7 @@ void nand_init(void) }

static int nand_read_page(int page_size, int ecc_strength, int ecc_page_size, - int addr_cycles, uint32_t real_addr, dma_addr_t dst, int syndrome) + int addr_cycles, uint32_t real_addr, dma_addr_t dst) { uint32_t val; int i, ecc_off = 0; @@ -209,17 +206,11 @@ static int nand_read_page(int page_size, int ecc_strength, int ecc_page_size, page = real_addr / page_size; column = real_addr % page_size;

- if (syndrome) - column += (column / ecc_page_size) * ecc_off; - /* clear ecc status */ writel(0, SUNXI_NFC_BASE + NFC_ECC_ST);

/* Choose correct seed */ - if (syndrome) - rand_seed = random_seed_syndrome; - else - rand_seed = random_seed[page % 128]; + rand_seed = random_seed[page % 128];

writel((rand_seed << 16) | NFC_ECC_RANDOM_EN | NFC_ECC_EN | NFC_ECC_PIPELINE | (ecc_mode << 12), @@ -228,9 +219,8 @@ static int nand_read_page(int page_size, int ecc_strength, int ecc_page_size, val = readl(SUNXI_NFC_BASE + NFC_CTL); writel(val | NFC_CTL_RAM_METHOD, SUNXI_NFC_BASE + NFC_CTL);

- if (!syndrome) - writel(page_size + (column / ecc_page_size) * ecc_off, - SUNXI_NFC_BASE + NFC_SPARE_AREA); + writel(page_size + (column / ecc_page_size) * ecc_off, + SUNXI_NFC_BASE + NFC_SPARE_AREA);

flush_dcache_range(dst, ALIGN(dst + ecc_page_size, ARCH_DMA_MINALIGN));

@@ -266,7 +256,7 @@ static int nand_read_page(int page_size, int ecc_strength, int ecc_page_size, writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_DATA_TRANS | NFC_PAGE_CMD | NFC_WAIT_FLAG | ((addr_cycles - 1) << NFC_ADDR_NUM_OFFSET) | - NFC_SEND_ADR | NFC_DATA_SWAP_METHOD | (syndrome ? NFC_SEQ : 0), + NFC_SEND_ADR | NFC_DATA_SWAP_METHOD, SUNXI_NFC_BASE + NFC_CMD);

if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_DMA_INT_FLAG, @@ -292,7 +282,7 @@ static int nand_read_page(int page_size, int ecc_strength, int ecc_page_size, }

static int nand_read_ecc(int page_size, int ecc_strength, int ecc_page_size, - int addr_cycles, uint32_t offs, uint32_t size, void *dest, int syndrome) + int addr_cycles, uint32_t offs, uint32_t size, void *dest) { void *end = dest + size;

@@ -301,16 +291,14 @@ static int nand_read_ecc(int page_size, int ecc_strength, int ecc_page_size,

for ( ;dest < end; dest += ecc_page_size, offs += ecc_page_size) { if (nand_read_page(page_size, ecc_strength, ecc_page_size, - addr_cycles, offs, (dma_addr_t)dest, - syndrome)) + addr_cycles, offs, (dma_addr_t)dest)) return -1; }

return 0; }

-static int nand_read_buffer(uint32_t offs, unsigned int size, void *dest, - int syndrome) +static int nand_read_buffer(uint32_t offs, unsigned int size, void *dest) { const struct { int page_size; @@ -337,7 +325,7 @@ static int nand_read_buffer(uint32_t offs, unsigned int size, void *dest, nand_configs[i].ecc_strength, nand_configs[i].ecc_page_size, nand_configs[i].addr_cycles, - offs, size, dest, syndrome) == 0) { + offs, size, dest) == 0) { debug("success\n"); nand_config = i; return 0; @@ -351,48 +339,32 @@ static int nand_read_buffer(uint32_t offs, unsigned int size, void *dest, nand_configs[nand_config].ecc_strength, nand_configs[nand_config].ecc_page_size, nand_configs[nand_config].addr_cycles, - offs, size, dest, syndrome); + offs, size, 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, - }; - 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. + * 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++) { if (nand_read_buffer(boot_offsets[i], size, - dest, syndrome) == 0) + dest) == 0) return 0; } return -1; }

- return nand_read_buffer(offs, size, dest, syndrome); + return nand_read_buffer(offs, size, dest); }

void nand_deselect(void)

On modern NAND it's more than recommended to have a backup copy of the u-boot binary to recover from corruption: bitflips are quite common on MLC NANDs, and the read-disturbance will corrupt your u-boot partitition more quickly than what you would see on an SLC NAND.

Add an extra Kconfig option to specify the offset of the redundant u-boot image.

Signed-off-by: Boris Brezillon boris.brezillon@free-electrons.com --- common/spl/spl_nand.c | 8 ++++++++ drivers/mtd/nand/Kconfig | 6 ++++++ 2 files changed, 14 insertions(+)

diff --git a/common/spl/spl_nand.c b/common/spl/spl_nand.c index 612bd4a..0bf0848 100644 --- a/common/spl/spl_nand.c +++ b/common/spl/spl_nand.c @@ -12,6 +12,9 @@

#ifndef CONFIG_SYS_NAND_U_BOOT_OFFS #define CONFIG_SYS_NAND_U_BOOT_OFFS CONFIG_SPL_NAND_U_BOOT_OFFS +#define CONFIG_SYS_NAND_U_BOOT_OFFS_REDUND CONFIG_SPL_NAND_U_BOOT_OFFS_REDUND +#else +#define CONFIG_SYS_NAND_U_BOOT_OFFS_REDUND CONFIG_SYS_NAND_U_BOOT_OFFS #endif

#if defined(CONFIG_SPL_NAND_RAW_ONLY) @@ -111,6 +114,11 @@ int spl_nand_load_image(void) #endif /* Load u-boot */ err = spl_nand_load_element(CONFIG_SYS_NAND_U_BOOT_OFFS, header); +#if CONFIG_SYS_NAND_U_BOOT_OFFS != CONFIG_SYS_NAND_U_BOOT_OFFS_REDUND + if (err) + err = spl_nand_load_element(CONFIG_SYS_NAND_U_BOOT_OFFS_REDUND, + header); +#endif nand_deselect(); return err; } diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig index 4b0f92c..2f1d1f7 100644 --- a/drivers/mtd/nand/Kconfig +++ b/drivers/mtd/nand/Kconfig @@ -110,6 +110,12 @@ config SPL_NAND_U_BOOT_OFFS Set the offset from the start of the nand where u-boot should be loaded from.

+config SPL_NAND_U_BOOT_OFFS_REDUND + hex "Location in NAND to read U-Boot from" + default SPL_NAND_U_BOOT_OFFS + help + Set the offset from the start of the nand where u-boot should be + loaded from.

config SPL_NAND_DENALI bool "Support Denali NAND controller for SPL"

check_value_xxx() helpers are using a 1ms delay between each test, which can be quite long for some operations (like a page read on an SLC NAND). Since we don't have anything to do but to poll this register, reduce the delay between each test to 1us.

While we're at it, rename the max_number_of_retries parameters and the MAX_RETRIES macro into timeout_us and DEFAULT_TIMEOUT_US to reflect that we're actually waiting a given amount of time and not only a number of retries.

Signed-off-by: Boris Brezillon boris.brezillon@free-electrons.com --- drivers/mtd/nand/sunxi_nand_spl.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/mtd/nand/sunxi_nand_spl.c b/drivers/mtd/nand/sunxi_nand_spl.c index 13e6eab..55b3c8a 100644 --- a/drivers/mtd/nand/sunxi_nand_spl.c +++ b/drivers/mtd/nand/sunxi_nand_spl.c @@ -119,35 +119,31 @@ const uint16_t random_seed[128] = { 0x7c57, 0x0fbe, 0x46ce, 0x4939, 0x6b17, 0x37bb, 0x3e91, 0x76db, };

-#define MAX_RETRIES 10 +#define DEFAULT_TIMEOUT_US 100000

static int check_value_inner(int offset, int expected_bits, - int max_number_of_retries, int negation) + int timeout_us, int negation) { - int retries = 0; do { int val = readl(offset) & expected_bits; if (negation ? !val : val) return 1; - mdelay(1); - retries++; - } while (retries < max_number_of_retries); + udelay(1); + } while (--timeout_us);

return 0; }

static inline int check_value(int offset, int expected_bits, - int max_number_of_retries) + int timeout_us) { - return check_value_inner(offset, expected_bits, - max_number_of_retries, 0); + return check_value_inner(offset, expected_bits, timeout_us, 0); }

static inline int check_value_negated(int offset, int unexpected_bits, - int max_number_of_retries) + int timeout_us) { - return check_value_inner(offset, unexpected_bits, - max_number_of_retries, 1); + return check_value_inner(offset, unexpected_bits, timeout_us, 1); }

void nand_init(void) @@ -162,7 +158,7 @@ void nand_init(void) SUNXI_NFC_BASE + NFC_CTL);

if (!check_value_negated(SUNXI_NFC_BASE + NFC_CTL, - NFC_CTL_RESET, MAX_RETRIES)) { + NFC_CTL_RESET, DEFAULT_TIMEOUT_US)) { printf("Couldn't initialize nand\n"); }

@@ -172,7 +168,7 @@ void nand_init(void) SUNXI_NFC_BASE + NFC_CMD);

if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG, - MAX_RETRIES)) { + DEFAULT_TIMEOUT_US)) { printf("Error timeout waiting for nand reset\n"); return; } @@ -260,14 +256,15 @@ static int nand_read_page(int page_size, int ecc_strength, int ecc_page_size, SUNXI_NFC_BASE + NFC_CMD);

if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_DMA_INT_FLAG, - MAX_RETRIES)) { + DEFAULT_TIMEOUT_US)) { printf("Error while initializing dma interrupt\n"); return -1; } writel(NFC_ST_DMA_INT_FLAG, SUNXI_NFC_BASE + NFC_ST);

if (!check_value_negated(SUNXI_DMA_BASE + SUNXI_DMA_CFG_REG0, - SUNXI_DMA_DDMA_CFG_REG_LOADING, MAX_RETRIES)) { + SUNXI_DMA_DDMA_CFG_REG_LOADING, + DEFAULT_TIMEOUT_US)) { printf("Error while waiting for dma transfer to finish\n"); return -1; }

NAND chips are supposed to expose their capabilities through advanced mechanisms like READID, ONFI or JEDEC parameter tables. While those methods are appropriate for the bootloader itself, it's way to complicated and takes too much space to fit in the SPL.

Replace those mechanisms by a dumb 'trial and error' mechanism.

With this new approach we can get rid of the fixed config list that was used in the sunxi NAND SPL driver.

Signed-off-by: Boris Brezillon boris.brezillon@free-electrons.com --- drivers/mtd/nand/sunxi_nand_spl.c | 262 +++++++++++++++++++++++++++++--------- 1 file changed, 204 insertions(+), 58 deletions(-)

diff --git a/drivers/mtd/nand/sunxi_nand_spl.c b/drivers/mtd/nand/sunxi_nand_spl.c index b43f2af..cc2e69b 100644 --- a/drivers/mtd/nand/sunxi_nand_spl.c +++ b/drivers/mtd/nand/sunxi_nand_spl.c @@ -103,6 +103,7 @@ struct nfc_config { int addr_cycles; int nseeds; bool randomize; + bool valid; };

/* minimal "boot0" style NAND support for Allwinner A20 */ @@ -219,6 +220,26 @@ static int nand_load_page(const struct nfc_config *conf, u32 offs) return 0; }

+static int nand_reset_column(void) +{ + writel((NFC_CMD_RNDOUTSTART << NFC_RANDOM_READ_CMD1_OFFSET) | + (NFC_CMD_RNDOUT << NFC_RANDOM_READ_CMD0_OFFSET) | + (NFC_CMD_RNDOUTSTART << NFC_READ_CMD_OFFSET), + SUNXI_NFC_BASE + NFC_RCMD_SET); + writel(0, SUNXI_NFC_BASE + NFC_ADDR_LOW); + writel(NFC_SEND_CMD1 | NFC_SEND_CMD2 | NFC_RAW_CMD | + (1 << NFC_ADDR_NUM_OFFSET) | NFC_SEND_ADR | NFC_CMD_RNDOUT, + SUNXI_NFC_BASE + NFC_CMD); + + if (!check_value(SUNXI_NFC_BASE + NFC_ST, NFC_ST_CMD_INT_FLAG, + DEFAULT_TIMEOUT_US)) { + printf("Error while initializing dma interrupt\n"); + return -1; + } + + return 0; +} + static int nand_read_page(const struct nfc_config *conf, u32 offs, void *dest, int len) { @@ -303,88 +324,213 @@ static int nand_read_page(const struct nfc_config *conf, u32 offs, return (val & 0x10000) ? 1 : 0; }

-static int nand_read_ecc(int page_size, int ecc_strength, int ecc_page_size, - int addr_cycles, uint32_t offs, uint32_t size, void *dest) +static int nand_max_ecc_strength(struct nfc_config *conf) { - void *end = dest + size; - static const u8 strengths[] = { 16, 24, 28, 32, 40, 48, 56, 60, 64 }; - struct nfc_config conf = { - .page_size = page_size, - .ecc_size = ecc_page_size, - .addr_cycles = addr_cycles, - .nseeds = ARRAY_SIZE(random_seed), - .randomize = true, - }; + static const int ecc_bytes[] = { 32, 46, 54, 60, 74, 88, 102, 110, 116 }; + int max_oobsize, max_ecc_bytes; + int nsectors = conf->page_size / conf->ecc_size; int i;

- for (i = 0; i < ARRAY_SIZE(strengths); i++) { - if (ecc_strength == strengths[i]) { - conf.ecc_strength = i; + /* + * ECC strength is limited by the size of the OOB area which is + * correlated with the page size. + */ + switch (conf->page_size) { + case 2048: + max_oobsize = 64; + break; + case 4096: + max_oobsize = 256; + break; + case 8196: + max_oobsize = 620; + break; + case 16384: + max_oobsize = 1664; + break; + default: + return -EINVAL; + } + + max_ecc_bytes = max_oobsize / nsectors; + + for (i = 0; i < ARRAY_SIZE(ecc_bytes); i++) { + if (ecc_bytes[i] > max_ecc_bytes) break; - } }

+ if (!i) + return -EINVAL;

- nand_apply_config(&conf); + return i - 1; +}

- for ( ;dest < end; dest += ecc_page_size, offs += page_size) { - if (nand_load_page(&conf, offs)) - return -1; +static int nand_detect_ecc_config(struct nfc_config *conf, u32 offs, + void *dest) +{ + /* NAND with pages > 4k will likely require 1k sector size. */ + int min_ecc_size = conf->page_size > 4096 ? 1024 : 512; + int page = offs / conf->page_size; + int ret;

- if (nand_read_page(&conf, offs, dest, page_size)) - return -1; + /* + * In most cases, 1k sectors are preferred over 512b ones, start + * testing this config first. + */ + for (conf->ecc_size = 1024; conf->ecc_size >= min_ecc_size; + conf->ecc_size >>= 1) { + int max_ecc_strength = nand_max_ecc_strength(conf); + + nand_apply_config(conf); + + /* + * We are starting from the maximum ECC strength because + * most of the time NAND vendors provide an OOB area that + * barely meets the ECC requirements. + */ + for (conf->ecc_strength = max_ecc_strength; + conf->ecc_strength >= 0; + conf->ecc_strength--) { + conf->randomize = false; + if (nand_reset_column()) + return -EIO; + + /* + * Only read the first sector to speedup detection. + */ + ret = nand_read_page(conf, offs, dest, conf->ecc_size); + if (!ret) { + return 0; + } else if (ret > 0) { + /* + * If page is empty we can't deduce anything + * about the ECC config => stop the detection. + */ + return -EINVAL; + } + + conf->randomize = true; + conf->nseeds = ARRAY_SIZE(random_seed); + do { + if (nand_reset_column()) + return -EIO; + + if (!nand_read_page(conf, offs, dest, + conf->ecc_size)) + return 0; + + /* + * Find the next ->nseeds value that would + * change the randomizer seed for the page + * we're trying to read. + */ + while (conf->nseeds >= 16) { + int seed = page % conf->nseeds; + + conf->nseeds >>= 1; + if (seed != page % conf->nseeds) + break; + } + } while (conf->nseeds >= 16); + } }

- return 0; + return -EINVAL; }

-static int nand_read_buffer(uint32_t offs, unsigned int size, void *dest) +static int nand_detect_config(struct nfc_config *conf, u32 offs, void *dest) { - const struct { - int page_size; - int ecc_strength; - int ecc_page_size; - int addr_cycles; - } nand_configs[] = { - { 8192, 40, 1024, 5 }, - { 16384, 56, 1024, 5 }, - { 8192, 24, 1024, 5 }, - { 4096, 24, 1024, 5 }, - }; - static int nand_config = -1; - int i; + if (conf->valid) + return 0;

- if (nand_config == -1) { - for (i = 0; i < ARRAY_SIZE(nand_configs); i++) { - debug("nand: trying page %d ecc %d / %d addr %d: ", - nand_configs[i].page_size, - nand_configs[i].ecc_strength, - nand_configs[i].ecc_page_size, - nand_configs[i].addr_cycles); - if (nand_read_ecc(nand_configs[i].page_size, - nand_configs[i].ecc_strength, - nand_configs[i].ecc_page_size, - nand_configs[i].addr_cycles, - offs, size, dest) == 0) { - debug("success\n"); - nand_config = i; + /* + * Modern NANDs are more likely than legacy ones, so we start testing + * with 5 address cycles. + */ + for (conf->addr_cycles = 5; + conf->addr_cycles >= 4; + conf->addr_cycles--) { + int max_page_size = conf->addr_cycles == 4 ? 2048 : 16384; + + /* + * Ignoring 1k pages cause I'm not even sure this case exist + * in the real world. + */ + for (conf->page_size = 2048; conf->page_size <= max_page_size; + conf->page_size <<= 1) { + if (nand_load_page(conf, offs)) + return -1; + + if (!nand_detect_ecc_config(conf, offs, dest)) { + conf->valid = true; return 0; } - debug("failed\n"); } - return -1; }

- return nand_read_ecc(nand_configs[nand_config].page_size, - nand_configs[nand_config].ecc_strength, - nand_configs[nand_config].ecc_page_size, - nand_configs[nand_config].addr_cycles, - offs, size, dest); + return -EINVAL; +} + +static int nand_read_buffer(struct nfc_config *conf, uint32_t offs, + unsigned int size, void *dest) +{ + int first_seed, page, ret; + + size = ALIGN(size, conf->page_size); + page = offs / conf->page_size; + first_seed = page % conf->nseeds; + + for (; size; size -= conf->page_size) { + if (nand_load_page(conf, offs)) + return -1; + + ret = nand_read_page(conf, offs, dest, conf->page_size); + /* + * The ->nseeds value should be equal to the number of pages + * in an eraseblock. Since we don't know this information in + * advance we might have picked a wrong value. + */ + if (ret < 0 && conf->randomize) { + int cur_seed = page % conf->nseeds; + + /* + * We already tried all the seed values => we are + * facing a real corruption. + */ + if (cur_seed < first_seed) + return -EIO; + + /* Try to adjust ->nseeds and read the page again... */ + conf->nseeds = cur_seed; + + if (nand_reset_column()) + return -EIO; + + /* ... it still fails => it's a real corruption. */ + if (nand_read_page(conf, offs, dest, conf->page_size)) + return -EIO; + } else if (ret && conf->randomize) { + memset(dest, 0xff, conf->page_size); + } + + page++; + offs += conf->page_size; + dest += conf->page_size; + } + + return 0; }

int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest) { - return nand_read_buffer(offs, size, dest); + static struct nfc_config conf = { }; + int ret; + + ret = nand_detect_config(&conf, offs, dest); + if (ret) + return ret; + + return nand_read_buffer(&conf, offs, size, dest); }

void nand_deselect(void)