On 15 November 2017 at 06:14, Wilson Lee wilson.lee@ni.com wrote:

Putting board_nand_init() function inside NAND driver was not appropriate due to it doesn't allow board vendor to customise their NAND initialization code such as adding NAND lock/unlock code.

This commit was to move the board_nand_init() function from NAND driver to board.c file. This allow customization of board_nand_init() function.

Signed-off-by: Wilson Lee wilson.lee@ni.com Cc: Joe Hershberger joe.hershberger@ni.com Cc: Keng Soon Cheah keng.soon.cheah@ni.com Cc: Chen Yee Chew chen.yee.chew@ni.com Cc: Albert Aribaud albert.u.boot@aribaud.net Cc: Michal Simek michal.simek@xilinx.com Cc: Siva Durga Prasad Paladugu siva.durga.paladugu@xilinx.com Cc: Scott Wood oss@buserror.net

---

arch/arm/mach-zynq/include/mach/nand.h | 9 +++++++++ drivers/mtd/nand/zynq_nand.c | 6 ++++-- 2 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 arch/arm/mach-zynq/include/mach/nand.h

diff --git a/arch/arm/mach-zynq/include/mach/nand.h b/arch/arm/mach-zynq/include/mach/nand.h new file mode 100644 index 0000000..61ef45f --- /dev/null +++ b/arch/arm/mach-zynq/include/mach/nand.h @@ -0,0 +1,9 @@ +/*

• • Copyright (C) 2017 National Instruments Corp.
• • SPDX-License-Identifier: GPL-2.0+
• */

+#include <nand.h>

+void zynq_nand_init(void); diff --git a/drivers/mtd/nand/zynq_nand.c b/drivers/mtd/nand/zynq_nand.c index dec2c41..076b878 100644 --- a/drivers/mtd/nand/zynq_nand.c +++ b/drivers/mtd/nand/zynq_nand.c @@ -1006,7 +1006,7 @@ static int zynq_nand_device_ready(struct mtd_info *mtd) return 0; }

-static int zynq_nand_init(struct nand_chip *nand_chip, int devnum) +int zynq_nand_init(struct nand_chip *nand_chip, int devnum) { struct zynq_nand_info *xnand; struct mtd_info *mtd; @@ -1192,12 +1192,14 @@ fail: return err; }

+#ifdef CONFIG_SYS_NAND_SELF_INIT

Just found this patch while debugging a NAND problem using current U-Boot upstream, on a custom board. In fact, I wrote a patch which is an exact revert of this one, until I noticed the board_nand_init is exported on purpose.

A few observations:

1) The driver selects CONFIG_SYS_NAND_SELF_INIT, which makes this ifdef a no-op. I'm guessing vendors are using the driver without the self-init mode. Is that the case?

2) This driver looks broken in upstream, refusing to initalize properly. The reason is that get_nand_dev_by_index() was being called before nand_register(), thus returning a pointer into uninitialized memory. In other words, the struct mtd_info used by the driver is total junk.

The fix is simple, I cooked a patch for it:

diff --git a/drivers/mtd/nand/zynq_nand.c b/drivers/mtd/nand/zynq_nand.c index 6494196049f1..9f6ff3d045c2 100644 --- a/drivers/mtd/nand/zynq_nand.c +++ b/drivers/mtd/nand/zynq_nand.c @@ -1025,7 +1025,7 @@ int zynq_nand_init(struct nand_chip *nand_chip, int devnum) }

xnand->nand_base = (void __iomem *)ZYNQ_NAND_BASEADDR; - mtd = get_nand_dev_by_index(0); + mtd = nand_to_mtd(nand_chip);

However, I am not sure about sending this patch upstream, because it assumes the driver is used on self-init mode only.

Any hints on how this should be fixed properly? Perhaps we should find a cleaner path into a lock/unlock hook in upstream.

static struct nand_chip nand_chip[CONFIG_SYS_MAX_NAND_DEVICE];

-void board_nand_init(void) +void __weak board_nand_init(void) { struct nand_chip *nand = &nand_chip[0];

    if (zynq_nand_init(nand, 0))
            puts("ZYNQ NAND init failed\n");

}

+#endif

2.7.4

---

U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot