OpenNuvoton / NUC970_U-Boot_v2016.11

U-Boot v2016.11 for NUC970, N9H30, and NUC980
28 stars 45 forks source link

spl nand driver bug: No ECC functions supplied; hardware ECC not possible #16

Closed qianfan-Zhao closed 5 years ago

qianfan-Zhao commented 5 years ago

I am debuging u-boot now, download u-boot-spl.bin to SDRAM and run directly.

Sometimes u-boot report No ECC functions supplied; hardware ECC not possible and sometimes not. It's very strange.

printf("ecc mode: %d\n", ecc->mode);
    printf("%s: %x %x %x\n", __func__, ecc->calculate, ecc->correct, ecc->hwctl);

    switch (ecc->mode) {
    case NAND_ECC_HW_OOB_FIRST:
        /* Similar to NAND_ECC_HW, but a separate read_page handle */
        if (!ecc->calculate || !ecc->correct || !ecc->hwctl) {
            pr_warn("No ECC functions supplied; hardware ECC not possible\n");
            /*BUG();*/while(1);
        }
        if (!ecc->read_page)
            ecc->read_page = nand_read_page_hwecc_oob_first;

Dump those there variables and found that ecc->correct and ecc->hwctl is a valid functions which can match with .map file. But ecc->calculate is not a valid function, seems is a garbage (not a const number) number.

Here are the results of multiple tests:

nand_scan_tail: 5FEEA3F1 177C 1778
nand_scan_tail: 5FEEA7F1 177C 1778
nand_scan_tail: 5FFEA7F1 177C 1778
nand_scan_tail: 5FFEA7F5 177C 1778

(There is no case that ecc->calculate is equal to 0 in this binary, but it does appear in other binary.)

The variable nand is a global static data and based on C's language, the global data should initialized to zero after poweron, but it's not.

void board_init_f(unsigned long bootflag)
{
    struct nand_chip *nand = &nand_chip[0];
    struct mtd_info *mtd = nand_to_mtd(nand);
    ulong base_addr = base_address[0];
    int maxchips = CONFIG_SYS_NAND_MAX_CHIPS;
    __attribute__((noreturn)) void (*uboot)(void);

    nuc970_serial_init();
    printf("NAND boot!\n");

    if (maxchips < 1)
        maxchips = 1;

    nand->IO_ADDR_R = nand->IO_ADDR_W = (void  __iomem *)base_addr;

    if (board_nand_init(nand))
        return;

Add a memset before using nand, the garbage number was gone.

void board_init_f(unsigned long bootflag)
{
    struct nand_chip *nand = &nand_chip[0];
    struct mtd_info *mtd = nand_to_mtd(nand);
    ulong base_addr = base_address[0];
    int maxchips = CONFIG_SYS_NAND_MAX_CHIPS;
    __attribute__((noreturn)) void (*uboot)(void);

    nuc970_serial_init();
    printf("NAND boot!\n");

    if (maxchips < 1)
        maxchips = 1;

    memset(nand, 0, sizeof(*nand));
nand_scan_tail: 0 178C 1788
nand_scan_tail: 0 178C 1788

Some there has a two problems we need fixed:

  1. Why the global data are not initilized?
  2. Should provide ecc->calculate
qianfan-Zhao commented 5 years ago

Please check the code in crt0.S.

/*
 * This file handles the target-independent stages of the U-Boot
 * start-up where a C runtime environment is needed. Its entry point
 * is _main and is branched into from the target's start.S file.
 *
 * _main execution sequence is:
 *
 * 1. Set up initial environment for calling board_init_f().
 *    This environment only provides a stack and a place to store
 *    the GD ('global data') structure, both located in some readily
 *    available RAM (SRAM, locked cache...). In this context, VARIABLE
 *    global data, initialized or not (BSS), are UNAVAILABLE; only
 *    CONSTANT initialized data are available. GD should be zeroed
 *    before board_init_f() is called.
 *
 * 2. Call board_init_f(). This function prepares the hardware for
 *    execution from system RAM (DRAM, DDR...) As system RAM may not
 *    be available yet, , board_init_f() must use the current GD to
 *    store any data which must be passed on to later stages. These
 *    data include the relocation destination, the future stack, and
 *    the future GD location.
 *
 * 3. Set up intermediate environment where the stack and GD are the
 *    ones allocated by board_init_f() in system RAM, but BSS and
 *    initialized non-const data are still not available.
 *
 * 4a.For U-Boot proper (not SPL), call relocate_code(). This function
 *    relocates U-Boot from its current location into the relocation
 *    destination computed by board_init_f().
 *
 * 4b.For SPL, board_init_f() just returns (to crt0). There is no
 *    code relocation in SPL.
 *
 * 5. Set up final environment for calling board_init_r(). This
 *    environment has BSS (initialized to 0), initialized non-const
 *    data (initialized to their intended value), and stack in system
 *    RAM (for SPL moving the stack and GD into RAM is optional - see
 *    CONFIG_SPL_STACK_R). GD has retained values set by board_init_f().
 *
 * 6. For U-Boot proper (not SPL), some CPUs have some work left to do
 *    at this point regarding memory, so call c_runtime_cpu_setup.
 *
 * 7. Branch to board_init_r().
 *
 * For more information see 'Board Initialisation Flow in README.
 */

board_init_f do some init work and try load u-boot to DRAM and run, in that times the BSS sections are UNAVAILABLE, so ecc->calculate is a garbage number. Coincidentally, this value is non-zero most of the time, so this error was not detected before.

yachen commented 5 years ago

Hi,

We've pushed a patch 57859403043c249cb4f5a354ff74d1346edfddfc to fix this issue. Thanks a lot.

Sincerely,

Yi-An Chen