[U-Boot] [PATCH 0/9] device model bringup of nand on am335x gp evm and am437x gp evm

[U-Boot] [PATCH 0/9] device model bringup of nand on am335x gp evm and am437x gp evm

[Mugunthan V N]

This patch seires adds nand uclass driver and enables omap_gpmc to
adopt driver model. This has been tested on AM335x GP EVM and
AM437x GP EVM (logs [1]).
Also pushed a branch for testing [2]

[1]: http://pastebin.ubuntu.com/15575957/
[2]: git://git.ti.com/~mugunthanvnm/ti-u-boot/mugunth-ti-u-boot.git dm-nand

Mugunthan V N (9):
  include: nand: move endif to end of file
  cmd: nand: abstract global variable usage for dm conversion
  drivers: nand: Kconfig: add NAND as Kconfig option
  drivers: nand: implement a NAND uclass
  drivers: nand: omap_gpmc: convert driver to adopt driver model
  am43xx_evm: nand: do not define DM_NAND for spl
  defconfig: am437x_gp_evm: enable NAND driver model
  am335x_evm: nand: do not define DM_NAND for spl
  defconfig: am335x_gp_evm: enable NAND driver model

 cmd/nand.c                      |  72 +++++++-------
 configs/am335x_gp_evm_defconfig |   2 +
 configs/am437x_gp_evm_defconfig |   2 +
 drivers/mtd/nand/Kconfig        |  22 ++++-
 drivers/mtd/nand/Makefile       |   2 +
 drivers/mtd/nand/nand-uclass.c  |  62 ++++++++++++
 drivers/mtd/nand/nand.c         |  14 ++-
 drivers/mtd/nand/omap_gpmc.c    | 205 ++++++++++++++++++++++++++++++++++++++++
 include/configs/am335x_evm.h    |   1 +
 include/configs/am43xx_evm.h    |   1 +
 include/dm/uclass-id.h          |   1 +
 include/nand.h                  |  27 +++++-
 12 files changed, 370 insertions(+), 41 deletions(-)
 create mode 100644 drivers/mtd/nand/nand-uclass.c

-- 
2.8.0.rc3.9.g44915db

[U-Boot] [PATCH 1/9] include: nand: move endif to end of file

[Mugunthan V N]

The terminator endif of ifdef _NAND_H_ should be at the
end of file as a fail safe.

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 include/nand.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/nand.h b/include/nand.h
index 7cbbbd3..4ea24ac 100644
--- a/include/nand.h
+++ b/include/nand.h
@@ -127,8 +127,6 @@ void board_nand_select_device(struct nand_chip *nand, int chip);
 
 __attribute__((noreturn)) void nand_boot(void);
 
-#endif
-
 #ifdef CONFIG_ENV_OFFSET_OOB
 #define ENV_OOB_MARKER 0x30425645 /*"EVB0" in little-endian -- offset is stored
 				    as block number*/
@@ -138,3 +136,5 @@ __attribute__((noreturn)) void nand_boot(void);
 int get_nand_env_oob(nand_info_t *nand, unsigned long *result);
 #endif
 int spl_nand_erase_one(int block, int page);
+
+#endif /* _NAND_H_ */
-- 
2.8.0.rc3.9.g44915db

[U-Boot] [PATCH 2/9] cmd: nand: abstract global variable usage for dm conversion

[Mugunthan V N]

nand_info is used all over the file so abstract it with
get_nand_dev_by_index() which will help for DM conversion.

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 cmd/nand.c              | 72 ++++++++++++++++++++++++++-----------------------
 drivers/mtd/nand/nand.c |  8 ++++--
 include/nand.h          | 18 +++++++++++++
 3 files changed, 62 insertions(+), 36 deletions(-)

diff --git a/cmd/nand.c b/cmd/nand.c
index a6b67e2..ffb8d43 100644
--- a/cmd/nand.c
+++ b/cmd/nand.c
@@ -114,21 +114,20 @@ free_dat:
 
 static int set_dev(int dev)
 {
-	if (dev < 0 || dev >= CONFIG_SYS_MAX_NAND_DEVICE ||
-	    !nand_info[dev].name) {
-		puts("No such device\n");
-		return -1;
-	}
+	nand_info_t *nand = get_nand_dev_by_index(dev);
+
+	if (!nand)
+		return -ENODEV;
 
 	if (nand_curr_device == dev)
 		return 0;
 
-	printf("Device %d: %s", dev, nand_info[dev].name);
+	printf("Device %d: %s", dev, nand->name);
 	puts("... is now current device\n");
 	nand_curr_device = dev;
 
 #ifdef CONFIG_SYS_NAND_SELECT_DEVICE
-	board_nand_select_device(nand_info[dev].priv, dev);
+	board_nand_select_device(nand->priv, dev);
 #endif
 
 	return 0;
@@ -188,7 +187,7 @@ int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[])
 {
 	int ret;
 	uint32_t oob_buf[ENV_OFFSET_SIZE/sizeof(uint32_t)];
-	nand_info_t *nand = &nand_info[0];
+	nand_info_t *nand = get_nand_dev_by_index(0);
 	char *cmd = argv[1];
 
 	if (CONFIG_SYS_MAX_NAND_DEVICE == 0 || !nand->name) {
@@ -213,9 +212,10 @@ int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[])
 		if (argc < 3)
 			goto usage;
 
+		nand = get_nand_dev_by_index(idx);
 		/* We don't care about size, or maxsize. */
 		if (mtd_arg_off(argv[2], &idx, &addr, &maxsize, &maxsize,
-				MTD_DEV_TYPE_NAND, nand_info[idx].size)) {
+				MTD_DEV_TYPE_NAND, nand->size)) {
 			puts("Offset or partition name expected\n");
 			return 1;
 		}
@@ -283,9 +283,14 @@ usage:
 
 static void nand_print_and_set_info(int idx)
 {
-	nand_info_t *nand = &nand_info[idx];
-	struct nand_chip *chip = nand->priv;
+	nand_info_t *nand;
+	struct nand_chip *chip;
 
+	nand = get_nand_dev_by_index(idx);
+	if (!nand)
+		return;
+
+	chip = nand->priv;
 	printf("Device %d: ", idx);
 	if (chip->numchips > 1)
 		printf("%dx ", chip->numchips);
@@ -348,7 +353,7 @@ static void adjust_size_for_badblocks(loff_t *size, loff_t offset, int dev)
 	/* We grab the nand info object here fresh because this is usually
 	 * called after arg_off_size() which can change the value of dev.
 	 */
-	nand_info_t *nand = &nand_info[dev];
+	nand_info_t *nand = get_nand_dev_by_index(dev);
 	loff_t maxoffset = offset + *size;
 	int badblocks = 0;
 
@@ -397,10 +402,8 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	if (strcmp(cmd, "info") == 0) {
 
 		putc('\n');
-		for (i = 0; i < CONFIG_SYS_MAX_NAND_DEVICE; i++) {
-			if (nand_info[i].name)
-				nand_print_and_set_info(i);
-		}
+		for (i = 0; i < CONFIG_SYS_MAX_NAND_DEVICE; i++)
+			nand_print_and_set_info(i);
 		return 0;
 	}
 
@@ -432,12 +435,7 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	 * one before these commands can run, even if a partition specifier
 	 * for another device is to be used.
 	 */
-	if (dev < 0 || dev >= CONFIG_SYS_MAX_NAND_DEVICE ||
-	    !nand_info[dev].name) {
-		puts("\nno devices available\n");
-		return 1;
-	}
-	nand = &nand_info[dev];
+	nand = get_nand_dev_by_index(dev);
 
 	if (strcmp(cmd, "bad") == 0) {
 		printf("\nDevice %d bad blocks:\n", dev);
@@ -496,13 +494,13 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		/* skip first two or three arguments, look for offset and size */
 		if (mtd_arg_off_size(argc - o, argv + o, &dev, &off, &size,
 				     &maxsize, MTD_DEV_TYPE_NAND,
-				     nand_info[dev].size) != 0)
+				     nand->size) != 0)
 			return 1;
 
 		if (set_dev(dev))
 			return 1;
 
-		nand = &nand_info[dev];
+		nand = get_nand_dev_by_index(dev);
 
 		memset(&opts, 0, sizeof(opts));
 		opts.offset = off;
@@ -561,13 +559,13 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 			if (mtd_arg_off(argv[3], &dev, &off, &size, &maxsize,
 					MTD_DEV_TYPE_NAND,
-					nand_info[dev].size))
+					nand->size))
 				return 1;
 
 			if (set_dev(dev))
 				return 1;
 
-			nand = &nand_info[dev];
+			nand = get_nand_dev_by_index(dev);
 
 			if (argc > 4 && !str2long(argv[4], &pagecount)) {
 				printf("'%s' is not a number\n", argv[4]);
@@ -584,7 +582,7 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			if (mtd_arg_off_size(argc - 3, argv + 3, &dev, &off,
 					     &size, &maxsize,
 					     MTD_DEV_TYPE_NAND,
-					     nand_info[dev].size) != 0)
+					     nand->size) != 0)
 				return 1;
 
 			if (set_dev(dev))
@@ -596,7 +594,7 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			rwsize = size;
 		}
 
-		nand = &nand_info[dev];
+		nand = get_nand_dev_by_index(dev);
 
 		if (!s || !strcmp(s, ".jffs2") ||
 		    !strcmp(s, ".e") || !strcmp(s, ".i")) {
@@ -727,13 +725,15 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 		if (mtd_arg_off_size(argc - 2, argv + 2, &dev, &off, &size,
 				     &maxsize, MTD_DEV_TYPE_NAND,
-				     nand_info[dev].size) < 0)
+				     nand->size) < 0)
 			return 1;
 
 		if (set_dev(dev))
 			return 1;
 
-		if (!nand_unlock(&nand_info[dev], off, size, allexcept)) {
+		nand = get_nand_dev_by_index(dev);
+
+		if (!nand_unlock(nand, off, size, allexcept)) {
 			puts("NAND flash successfully unlocked\n");
 		} else {
 			puts("Error unlocking NAND flash, "
@@ -899,6 +899,7 @@ static int do_nandboot(cmd_tbl_t *cmdtp, int flag, int argc,
 	struct mtd_device *dev;
 	struct part_info *part;
 	u8 pnum;
+	nand_info_t *nand;
 
 	if (argc >= 2) {
 		char *p = (argc == 2) ? argv[1] : argv[2];
@@ -914,8 +915,10 @@ static int do_nandboot(cmd_tbl_t *cmdtp, int flag, int argc,
 				addr = simple_strtoul(argv[1], NULL, 16);
 			else
 				addr = CONFIG_SYS_LOAD_ADDR;
-			return nand_load_image(cmdtp, &nand_info[dev->id->num],
-					       part->offset, addr, argv[0]);
+
+			nand = get_nand_dev_by_index(dev->id->num);
+			return nand_load_image(cmdtp, nand, part->offset,
+					       addr, argv[0]);
 		}
 	}
 #endif
@@ -957,14 +960,15 @@ usage:
 
 	idx = simple_strtoul(boot_device, NULL, 16);
 
-	if (idx < 0 || idx >= CONFIG_SYS_MAX_NAND_DEVICE || !nand_info[idx].name) {
+	nand = get_nand_dev_by_index(idx);
+	if (!nand) {
 		printf("\n** Device %d not available\n", idx);
 		bootstage_error(BOOTSTAGE_ID_NAND_AVAILABLE);
 		return 1;
 	}
 	bootstage_mark(BOOTSTAGE_ID_NAND_AVAILABLE);
 
-	return nand_load_image(cmdtp, &nand_info[idx], offset, addr, argv[0]);
+	return nand_load_image(cmdtp, nand, offset, addr, argv[0]);
 }
 
 U_BOOT_CMD(nboot, 4, 1, do_nandboot,
diff --git a/drivers/mtd/nand/nand.c b/drivers/mtd/nand/nand.c
index 8f0a921..524ead3 100644
--- a/drivers/mtd/nand/nand.c
+++ b/drivers/mtd/nand/nand.c
@@ -38,7 +38,7 @@ int nand_register(int devnum)
 	if (devnum >= CONFIG_SYS_MAX_NAND_DEVICE)
 		return -EINVAL;
 
-	mtd = &nand_info[devnum];
+	mtd = get_nand_dev_by_index(devnum);
 
 	sprintf(dev_name[devnum], "nand%d", devnum);
 	mtd->name = dev_name[devnum];
@@ -62,7 +62,7 @@ int nand_register(int devnum)
 #ifndef CONFIG_SYS_NAND_SELF_INIT
 static void nand_init_chip(int i)
 {
-	struct mtd_info *mtd = &nand_info[i];
+	struct mtd_info *mtd;
 	struct nand_chip *nand = &nand_chip[i];
 	ulong base_addr = base_address[i];
 	int maxchips = CONFIG_SYS_NAND_MAX_CHIPS;
@@ -70,6 +70,10 @@ static void nand_init_chip(int i)
 	if (maxchips < 1)
 		maxchips = 1;
 
+	mtd = get_nand_dev_by_index(i);
+	if (!mtd)
+		return;
+
 	mtd->priv = nand;
 	nand->IO_ADDR_R = nand->IO_ADDR_W = (void  __iomem *)base_addr;
 
diff --git a/include/nand.h b/include/nand.h
index 4ea24ac..23b2414 100644
--- a/include/nand.h
+++ b/include/nand.h
@@ -137,4 +137,22 @@ int get_nand_env_oob(nand_info_t *nand, unsigned long *result);
 #endif
 int spl_nand_erase_one(int block, int page);
 
+/*
+ * get_nand_dev_by_index - Get the nand info based in index.
+ *
+ * @dev - index to the nand device.
+ *
+ * returns pointer to the nand device info structure or NULL on failure.
+ */
+static inline nand_info_t *get_nand_dev_by_index(int dev)
+{
+	if (dev < 0 || dev >= CONFIG_SYS_MAX_NAND_DEVICE ||
+	    !nand_info[dev].name) {
+		puts("No such device\n");
+		return NULL;
+	}
+
+	return &nand_info[dev];
+}
+
 #endif /* _NAND_H_ */
-- 
2.8.0.rc3.9.g44915db

[U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option

[Mugunthan V N]

Add CONFIG_NAND as a Kconfig option so that it can be selected
using menuconfig or defconfig.

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 drivers/mtd/nand/Kconfig | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 2fc73ef..7787505 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -1,4 +1,12 @@
-menu "NAND Device Support"
+menuconfig NAND
+	bool "NAND device support"
+	help
+	  You must select Y to enable any NAND device support
+	  Generally if you have any NAND devices this is a given
+
+	  If unsure, say Y
+
+if NAND
 
 config SYS_NAND_SELF_INIT
 	bool
@@ -118,4 +126,4 @@ config SPL_NAND_DENALI
 
 endif
 
-endmenu
+endif
-- 
2.8.0.rc3.9.g44915db

[U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass

[Mugunthan V N]

Implement a NAND uclass so that the NAND devices can be
accessed via the DM framework.

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 drivers/mtd/nand/Kconfig       | 10 +++++++
 drivers/mtd/nand/Makefile      |  2 ++
 drivers/mtd/nand/nand-uclass.c | 62 ++++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/nand/nand.c        |  6 +++-
 include/dm/uclass-id.h         |  1 +
 include/nand.h                 |  5 ++++
 6 files changed, 85 insertions(+), 1 deletion(-)
 create mode 100644 drivers/mtd/nand/nand-uclass.c

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 7787505..53b57b8 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -8,6 +8,16 @@ menuconfig NAND
 
 if NAND
 
+config DM_NAND
+	bool "Enable driver model for NAND"
+	depends on NAND && DM
+	help
+	  Enable driver model for NAND. The NAND interface is then
+	  implemented by the NAND uclass. Multiple NAND devices can
+	  be attached and used. The 'nand' command works as normal.
+
+	  If the NAND drivers doesn't support DM, say N.
+
 config SYS_NAND_SELF_INIT
 	bool
 	help
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 6fb3718..7745705 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -39,6 +39,8 @@ endif # not spl
 
 ifdef NORMAL_DRIVERS
 
+obj-$(CONFIG_DM_NAND) += nand-uclass.o
+
 obj-$(CONFIG_NAND_ECC_BCH) += nand_bch.o
 
 obj-$(CONFIG_NAND_ATMEL) += atmel_nand.o
diff --git a/drivers/mtd/nand/nand-uclass.c b/drivers/mtd/nand/nand-uclass.c
new file mode 100644
index 0000000..d68d357
--- /dev/null
+++ b/drivers/mtd/nand/nand-uclass.c
@@ -0,0 +1,62 @@
+/*
+ * NAND uclass driver for NAND bus.
+ *
+ * (C) Copyright 2012-2016
+ *     Texas Instruments Incorporated, <www.ti.com>
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <nand.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#ifdef CONFIG_DM_NAND
+
+nand_info_t *get_nand_dev_by_index(int idx)
+{
+	nand_info_t *nand;
+	struct udevice *dev;
+	int ret;
+
+	ret = uclass_get_device(UCLASS_NAND, idx, &dev);
+	if (ret) {
+		error("NAND device (%d) not found\n", idx);
+		return NULL;
+	}
+
+	nand = (nand_info_t *)dev_get_uclass_priv(dev);
+	if (!nand) {
+		error("Nand device not ready\n");
+		return NULL;
+	}
+
+	return nand;
+}
+
+static int nand_child_pre_probe(struct udevice *dev)
+{
+	nand_info_t *nand = dev_get_uclass_priv(dev);
+	void *priv = dev_get_priv(dev);
+
+	/*
+	 * Store nand device priv pointer in nand_info so that
+	 * it can be used by nand command
+	 */
+	nand->priv = priv;
+
+	return 0;
+}
+
+UCLASS_DRIVER(nand) = {
+	.id				= UCLASS_NAND,
+	.name				= "nand",
+	.flags				= DM_UC_FLAG_SEQ_ALIAS,
+	.post_probe		= nand_child_pre_probe,
+	.per_device_auto_alloc_size	= sizeof(nand_info_t),
+};
+
+#endif
diff --git a/drivers/mtd/nand/nand.c b/drivers/mtd/nand/nand.c
index 524ead3..c03a7e5 100644
--- a/drivers/mtd/nand/nand.c
+++ b/drivers/mtd/nand/nand.c
@@ -21,7 +21,7 @@ int nand_curr_device = -1;
 
 nand_info_t nand_info[CONFIG_SYS_MAX_NAND_DEVICE];
 
-#ifndef CONFIG_SYS_NAND_SELF_INIT
+#if !defined(CONFIG_SYS_NAND_SELF_INIT) && !defined(CONFIG_DM_NAND)
 static struct nand_chip nand_chip[CONFIG_SYS_MAX_NAND_DEVICE];
 static ulong base_address[CONFIG_SYS_MAX_NAND_DEVICE] = CONFIG_SYS_NAND_BASE_LIST;
 #endif
@@ -63,8 +63,10 @@ int nand_register(int devnum)
 static void nand_init_chip(int i)
 {
 	struct mtd_info *mtd;
+#ifndef CONFIG_DM_NAND
 	struct nand_chip *nand = &nand_chip[i];
 	ulong base_addr = base_address[i];
+#endif
 	int maxchips = CONFIG_SYS_NAND_MAX_CHIPS;
 
 	if (maxchips < 1)
@@ -74,11 +76,13 @@ static void nand_init_chip(int i)
 	if (!mtd)
 		return;
 
+#ifndef CONFIG_DM_NAND
 	mtd->priv = nand;
 	nand->IO_ADDR_R = nand->IO_ADDR_W = (void  __iomem *)base_addr;
 
 	if (board_nand_init(nand))
 		return;
+#endif
 
 	if (nand_scan(mtd, maxchips))
 		return;
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 37c4176..6a88d39 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -49,6 +49,7 @@ enum uclass_id {
 	UCLASS_MMC,		/* SD / MMC card or chip */
 	UCLASS_MOD_EXP,		/* RSA Mod Exp device */
 	UCLASS_MTD,		/* Memory Technology Device (MTD) device */
+	UCLASS_NAND,		/* NAND bus */
 	UCLASS_NORTHBRIDGE,	/* Intel Northbridge / SDRAM controller */
 	UCLASS_PANEL,		/* Display panel, such as an LCD */
 	UCLASS_PANEL_BACKLIGHT,	/* Backlight controller for panel */
diff --git a/include/nand.h b/include/nand.h
index 23b2414..1580970 100644
--- a/include/nand.h
+++ b/include/nand.h
@@ -10,6 +10,7 @@
 #define _NAND_H_
 
 #include <config.h>
+#include <dm.h>
 
 /*
  * All boards using a given driver must convert to self-init
@@ -144,6 +145,7 @@ int spl_nand_erase_one(int block, int page);
  *
  * returns pointer to the nand device info structure or NULL on failure.
  */
+#ifndef CONFIG_DM_NAND
 static inline nand_info_t *get_nand_dev_by_index(int dev)
 {
 	if (dev < 0 || dev >= CONFIG_SYS_MAX_NAND_DEVICE ||
@@ -154,5 +156,8 @@ static inline nand_info_t *get_nand_dev_by_index(int dev)
 
 	return &nand_info[dev];
 }
+#else
+nand_info_t *get_nand_dev_by_index(int idx);
+#endif
 
 #endif /* _NAND_H_ */
-- 
2.8.0.rc3.9.g44915db

[U-Boot] [PATCH 5/9] drivers: nand: omap_gpmc: convert driver to adopt driver model

[Mugunthan V N]

adopt omap_gpmc driver to driver model.

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 drivers/mtd/nand/omap_gpmc.c | 205 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 205 insertions(+)

diff --git a/drivers/mtd/nand/omap_gpmc.c b/drivers/mtd/nand/omap_gpmc.c
index 4814fa2..df63491 100644
--- a/drivers/mtd/nand/omap_gpmc.c
+++ b/drivers/mtd/nand/omap_gpmc.c
@@ -16,6 +16,10 @@
 #include <nand.h>
 #include <linux/mtd/omap_elm.h>
 
+#include <dm.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
 #define BADBLOCK_MARKER_LENGTH	2
 #define SECTOR_BYTES		512
 #define ECCCLEAR		(0x1 << 8)
@@ -46,11 +50,22 @@ struct omap_nand_info {
 	enum omap_ecc ecc_scheme;
 	uint8_t cs;
 	uint8_t ws;		/* wait status pin (0,1) */
+	uint8_t bus_width;	/* Bus width of NAND device */
 };
 
+#ifndef CONFIG_DM_NAND
 /* We are wasting a bit of memory but al least we are safe */
 static struct omap_nand_info omap_nand_info[GPMC_MAX_CS];
 
+#else
+
+struct omap_gpmc_platdata {
+	struct omap_nand_info *omap_nand_info;
+	struct gpmc *gpmc_cfg;
+	int max_cs;
+};
+#endif
+
 /*
  * omap_nand_hwcontrol - Set the address pointers corretly for the
  *			following address/data/command operation
@@ -943,6 +958,8 @@ int __maybe_unused omap_nand_switch_ecc(uint32_t hardware, uint32_t eccstrength)
 }
 #endif /* CONFIG_SPL_BUILD */
 
+#ifndef CONFIG_DM_NAND
+
 /*
  * Board-specific NAND initialization. The following members of the
  * argument are board-specific:
@@ -1034,3 +1051,191 @@ int board_nand_init(struct nand_chip *nand)
 
 	return 0;
 }
+
+#else /* CONFIG_DM_NAND */
+
+static int omap_gpmc_probe(struct udevice *dev)
+{
+	struct nand_chip *nand = dev_get_priv(dev);
+	struct omap_gpmc_platdata *pdata = dev_get_platdata(dev);
+	struct gpmc *gpmc_cfg = pdata->gpmc_cfg;
+	int32_t gpmc_config = 0;
+	int ecc_opt;
+	int cs = cs_next++;
+	int err = 0;
+
+	while (cs < pdata->max_cs) {
+		/* Check if NAND type is set */
+		if ((readl(&gpmc_cfg->cs[cs].config1) & 0xC00) == 0x800) {
+			/* Found it!! */
+			break;
+		}
+		cs++;
+	}
+
+	if (cs >= pdata->max_cs) {
+		printf("nand: error: Unable to find NAND settings in GPMC Configuration - quitting\n");
+		return -ENODEV;
+	}
+
+	gpmc_config = readl(&gpmc_cfg->config);
+	/* Disable Write protect */
+	gpmc_config |= 0x10;
+	writel(gpmc_config, &gpmc_cfg->config);
+
+	nand->IO_ADDR_R = (void __iomem *)&gpmc_cfg->cs[cs].nand_dat;
+	nand->IO_ADDR_W = (void __iomem *)&gpmc_cfg->cs[cs].nand_cmd;
+	nand->priv	= &pdata->omap_nand_info[cs];
+	nand->cmd_ctrl	= omap_nand_hwcontrol;
+	nand->options	|= NAND_NO_PADDING | NAND_CACHEPRG;
+	nand->chip_delay = 100;
+	nand->ecc.layout = &omap_ecclayout;
+
+	/* configure driver and controller based on NAND device bus-width */
+	gpmc_config = readl(&gpmc_cfg->cs[cs].config1);
+	if (pdata->omap_nand_info[cs].bus_width == 16) {
+		nand->options |= NAND_BUSWIDTH_16;
+		writel(gpmc_config | (0x1 << 12), &gpmc_cfg->cs[cs].config1);
+	} else {
+		nand->options &= ~NAND_BUSWIDTH_16;
+		writel(gpmc_config & ~(0x1 << 12), &gpmc_cfg->cs[cs].config1);
+	}
+
+	ecc_opt = pdata->omap_nand_info[cs].ecc_scheme;
+	/* select ECC scheme */
+	if (ecc_opt != OMAP_ECC_HAM1_CODE_SW) {
+		err = omap_select_ecc_scheme(nand, ecc_opt,
+					     CONFIG_SYS_NAND_PAGE_SIZE,
+					     CONFIG_SYS_NAND_OOBSIZE);
+	} else {
+		/*
+		 * pagesize and oobsize are not required to
+		 * configure sw ecc-scheme
+		 */
+		err = omap_select_ecc_scheme(nand, ecc_opt, 0, 0);
+	}
+	if (err)
+		return err;
+
+#ifdef CONFIG_NAND_OMAP_GPMC_PREFETCH
+	nand->read_buf = omap_nand_read_prefetch;
+#else
+	if (nand->options & NAND_BUSWIDTH_16)
+		nand->read_buf = nand_read_buf16;
+	else
+		nand->read_buf = nand_read_buf;
+#endif
+
+	nand->dev_ready = omap_dev_ready;
+
+	return 0;
+}
+
+static int omap_gpmc_get_ecc_opt(int node, int elm_node)
+{
+	const void *fdt = gd->fdt_blob;
+	const char *ecc_str;
+	int ecc_opt = -ENOENT;
+
+	ecc_str = fdt_getprop(fdt, node, "ti,nand-ecc-opt", NULL);
+	if (!ecc_str) {
+		error("DT entry for ti,nand-ecc-opt not found\n");
+		return -ENOENT;
+	}
+
+	if (!strcmp(ecc_str, "sw")) {
+		ecc_opt = OMAP_ECC_HAM1_CODE_SW;
+	} else if (!strcmp(ecc_str, "ham1") ||
+		   !strcmp(ecc_str, "hw") ||
+		   !strcmp(ecc_str, "hw-romcode")) {
+		ecc_opt = OMAP_ECC_HAM1_CODE_HW;
+	} else if (!strcmp(ecc_str, "bch4")) {
+		if (elm_node > 0)
+			ecc_opt = OMAP_ECC_BCH4_CODE_HW;
+		else
+			ecc_opt = OMAP_ECC_BCH4_CODE_HW_DETECTION_SW;
+	} else if (!strcmp(ecc_str, "bch8")) {
+		if (elm_node > 0)
+			ecc_opt = OMAP_ECC_BCH8_CODE_HW;
+		else
+			ecc_opt = OMAP_ECC_BCH8_CODE_HW_DETECTION_SW;
+	} else if (!strcmp(ecc_str, "bch16")) {
+		if (elm_node > 0)
+			ecc_opt = OMAP_ECC_BCH16_CODE_HW;
+		else
+			error("BCH16 requires ELM support\n");
+	} else {
+		error("ti,nand-ecc-opt invalid value\n");
+		return -EINVAL;
+	}
+
+	return ecc_opt;
+}
+
+static int omap_gpmc_ofdata_to_platdata(struct udevice *dev)
+{
+	struct omap_gpmc_platdata *pdata = dev_get_platdata(dev);
+	const void *fdt = gd->fdt_blob;
+	int node = dev->of_offset;
+	int subnode;
+
+	pdata->gpmc_cfg = (struct gpmc *)dev_get_addr(dev);
+	pdata->max_cs = fdtdec_get_int(fdt, node, "gpmc,num-cs", -1);
+	if (pdata->max_cs < 0) {
+		error("max chip select not found in DT\n");
+		return -ENOENT;
+	}
+
+	pdata->omap_nand_info = calloc(pdata->max_cs,
+				       sizeof(struct omap_nand_info));
+	if (!pdata->omap_nand_info)
+		return -ENOMEM;
+
+	fdt_for_each_subnode(fdt, subnode, node) {
+		int cs;
+		int len;
+		int elm_node;
+		const char *name;
+		struct omap_nand_info *nand_info;
+
+		name = fdt_get_name(fdt, subnode, &len);
+		if (strncmp(name, "nand", 4))
+			continue;
+
+		cs = fdtdec_get_int(fdt, subnode, "reg", -1);
+		if (cs < 0 || cs >= pdata->max_cs) {
+			error("Invalid cs for nand device\n");
+			return -EINVAL;
+		}
+		nand_info = &pdata->omap_nand_info[cs];
+
+		/* get bus width 8 or 16, if not present 8 */
+		nand_info->bus_width = fdtdec_get_int(fdt, subnode,
+						      "nand-bus-width", 8);
+
+		elm_node = fdtdec_lookup_phandle(fdt, subnode, "ti,elm-id");
+
+		nand_info->ecc_scheme = omap_gpmc_get_ecc_opt(subnode,
+							      elm_node);
+		if (nand_info->ecc_scheme < 0)
+			return nand_info->ecc_scheme;
+	}
+	return 0;
+}
+
+static const struct udevice_id omap_gpmc_ids[] = {
+	{ .compatible = "ti,am3352-gpmc" },
+	{ }
+};
+
+U_BOOT_DRIVER(omap_gpmc) = {
+	.name	= "omap_gpmc",
+	.id	= UCLASS_NAND,
+	.of_match = omap_gpmc_ids,
+	.ofdata_to_platdata = omap_gpmc_ofdata_to_platdata,
+	.probe	= omap_gpmc_probe,
+	.priv_auto_alloc_size = sizeof(struct nand_chip),
+	.platdata_auto_alloc_size = sizeof(struct omap_gpmc_platdata),
+	.flags = DM_FLAG_ALLOC_PRIV_DMA,
+};
+#endif /* CONFIG_DM_NAND */
-- 
2.8.0.rc3.9.g44915db

[U-Boot] [PATCH 6/9] am43xx_evm: nand: do not define DM_NAND for spl

[Mugunthan V N]

Since OMAP's spl doesn't support DM currently, do not define
DM_NAND for spl build.

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 include/configs/am43xx_evm.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/configs/am43xx_evm.h b/include/configs/am43xx_evm.h
index fd3f6a7..9fbcdd4 100644
--- a/include/configs/am43xx_evm.h
+++ b/include/configs/am43xx_evm.h
@@ -146,6 +146,7 @@
 #undef CONFIG_DM_SPI
 #undef CONFIG_DM_SPI_FLASH
 #undef CONFIG_TIMER
+#undef CONFIG_DM_NAND
 #endif
 
 #ifndef CONFIG_SPL_BUILD
-- 
2.8.0.rc3.9.g44915db

[U-Boot] [PATCH 7/9] defconfig: am437x_gp_evm: enable NAND driver model

[Mugunthan V N]

Enable NAND driver model for am437x_gp_evm as omap_gpmc supports
driver model.

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 configs/am437x_gp_evm_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configs/am437x_gp_evm_defconfig b/configs/am437x_gp_evm_defconfig
index 356f6fd..e88c7d5 100644
--- a/configs/am437x_gp_evm_defconfig
+++ b/configs/am437x_gp_evm_defconfig
@@ -22,3 +22,5 @@ CONFIG_TIMER=y
 CONFIG_OMAP_TIMER=y
 CONFIG_USB=y
 CONFIG_USB_GADGET=y
+CONFIG_NAND=y
+CONFIG_DM_NAND=y
-- 
2.8.0.rc3.9.g44915db

[U-Boot] [PATCH 8/9] am335x_evm: nand: do not define DM_NAND for spl

[Mugunthan V N]

Since OMAP's spl doesn't support DM currently, do not define
DM_NAND for spl build.

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 include/configs/am335x_evm.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
index 29b693a..3da2e30 100644
--- a/include/configs/am335x_evm.h
+++ b/include/configs/am335x_evm.h
@@ -336,6 +336,7 @@
 #ifdef CONFIG_SPL_BUILD
 #undef CONFIG_DM_MMC
 #undef CONFIG_TIMER
+#undef CONFIG_DM_NAND
 #endif
 
 #if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_USBETH_SUPPORT)
-- 
2.8.0.rc3.9.g44915db

[U-Boot] [PATCH 9/9] defconfig: am335x_gp_evm: enable NAND driver model

[Mugunthan V N]

Enable NAND driver model for am335x_gp_evm as omap_gpmc supports
driver model.

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 configs/am335x_gp_evm_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configs/am335x_gp_evm_defconfig b/configs/am335x_gp_evm_defconfig
index 0c7eda7..5c00bbb 100644
--- a/configs/am335x_gp_evm_defconfig
+++ b/configs/am335x_gp_evm_defconfig
@@ -21,3 +21,5 @@ CONFIG_OMAP_TIMER=y
 CONFIG_USB=y
 CONFIG_USB_GADGET=y
 CONFIG_RSA=y
+CONFIG_NAND=y
+CONFIG_DM_NAND=y
-- 
2.8.0.rc3.9.g44915db

[U-Boot] [PATCH 1/9] include: nand: move endif to end of file

[Tom Rini]

On Fri, Apr 01, 2016 at 04:59:42PM +0530, Mugunthan V N wrote:

> The terminator endif of ifdef _NAND_H_ should be at the
> end of file as a fail safe.
> 
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160401/a5cf5e7b/attachment.sig>

[U-Boot] [PATCH 2/9] cmd: nand: abstract global variable usage for dm conversion

[Tom Rini]

On Fri, Apr 01, 2016 at 04:59:43PM +0530, Mugunthan V N wrote:

> nand_info is used all over the file so abstract it with
> get_nand_dev_by_index() which will help for DM conversion.
> 
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160401/df037655/attachment.sig>

[U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option

[Tom Rini]

On Fri, Apr 01, 2016 at 04:59:44PM +0530, Mugunthan V N wrote:

> Add CONFIG_NAND as a Kconfig option so that it can be selected
> using menuconfig or defconfig.
> 
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>

Good but you need to update configs/ to remove NAND from
CONFIG_SYS_EXTRA_OPTIONS and add CONFIG_NAND=y

Doing:
$ for F in `git grep -lE SYS_EXTRA.*NAND configs/`;do sed -i -e \
    's/,NAND//' $F && echo CONFIG_NAND=y >> $F;done
will get you most of the way there, then just a:
$ for C in `git status configs/ | grep modified: | cut -d / -f 2`;do \
  make O=$C $C savedefconfig && cp $C/defconfig configs/$C;done

To get it in the right spot, git add -p only the bits you really wanted,
done.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160401/4e1a88cf/attachment.sig>

[U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass

[Tom Rini]

On Fri, Apr 01, 2016 at 04:59:45PM +0530, Mugunthan V N wrote:

> Implement a NAND uclass so that the NAND devices can be
> accessed via the DM framework.
> 
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>

Minor:

> @@ -0,0 +1,62 @@
> +/*
> + * NAND uclass driver for NAND bus.
> + *
> + * (C) Copyright 2012-2016
> + *     Texas Instruments Incorporated, <www.ti.com>

The code isn't that old.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160401/0f7a2a42/attachment.sig>

[U-Boot] [PATCH 5/9] drivers: nand: omap_gpmc: convert driver to adopt driver model

[Tom Rini]

On Fri, Apr 01, 2016 at 04:59:46PM +0530, Mugunthan V N wrote:

> adopt omap_gpmc driver to driver model.
> 
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160401/081528d7/attachment.sig>

[U-Boot] [PATCH 6/9] am43xx_evm: nand: do not define DM_NAND for spl

[Tom Rini]

On Fri, Apr 01, 2016 at 04:59:47PM +0530, Mugunthan V N wrote:

> Since OMAP's spl doesn't support DM currently, do not define
> DM_NAND for spl build.
> 
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160401/67e68715/attachment.sig>

[U-Boot] [PATCH 7/9] defconfig: am437x_gp_evm: enable NAND driver model

[Tom Rini]

On Fri, Apr 01, 2016 at 04:59:48PM +0530, Mugunthan V N wrote:

> Enable NAND driver model for am437x_gp_evm as omap_gpmc supports
> driver model.
> 
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160401/22e9fb96/attachment.sig>

[U-Boot] [PATCH 8/9] am335x_evm: nand: do not define DM_NAND for spl

[Tom Rini]

On Fri, Apr 01, 2016 at 04:59:49PM +0530, Mugunthan V N wrote:

> Since OMAP's spl doesn't support DM currently, do not define
> DM_NAND for spl build.
> 
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160401/72680e7b/attachment.sig>

[U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option

[Mugunthan V N]

On Friday 01 April 2016 06:21 PM, Tom Rini wrote:
> On Fri, Apr 01, 2016 at 04:59:44PM +0530, Mugunthan V N wrote:
> 
>> Add CONFIG_NAND as a Kconfig option so that it can be selected
>> using menuconfig or defconfig.
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> 
> Good but you need to update configs/ to remove NAND from
> CONFIG_SYS_EXTRA_OPTIONS and add CONFIG_NAND=y
> 
> Doing:
> $ for F in `git grep -lE SYS_EXTRA.*NAND configs/`;do sed -i -e \
>     's/,NAND//' $F && echo CONFIG_NAND=y >> $F;done
> will get you most of the way there, then just a:
> $ for C in `git status configs/ | grep modified: | cut -d / -f 2`;do \
>   make O=$C $C savedefconfig && cp $C/defconfig configs/$C;done
> 
> To get it in the right spot, git add -p only the bits you really wanted,
> done.  Thanks!
> 

Will fix it in v2

Regards
Mugunthan V N

[U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass

[Mugunthan V N]

On Friday 01 April 2016 06:21 PM, Tom Rini wrote:
> On Fri, Apr 01, 2016 at 04:59:45PM +0530, Mugunthan V N wrote:
> 
>> Implement a NAND uclass so that the NAND devices can be
>> accessed via the DM framework.
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> 
> Minor:
> 
>> @@ -0,0 +1,62 @@
>> +/*
>> + * NAND uclass driver for NAND bus.
>> + *
>> + * (C) Copyright 2012-2016
>> + *     Texas Instruments Incorporated, <www.ti.com>
> 
> The code isn't that old.
> 

Copy paste, will fix in v2

Regards
Mugunthan V N

[U-Boot] [PATCH 2/9] cmd: nand: abstract global variable usage for dm conversion

[Scott Wood]

[Please CC me at this address rather than my NXP address]

On Fri, 2016-04-01 at 16:59 +0530, Mugunthan V N wrote:
> @@ -432,12 +435,7 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int
> argc, char * const argv[])
>  	 * one before these commands can run, even if a partition specifier
>  	 * for another device is to be used.
>  	 */
> -	if (dev < 0 || dev >= CONFIG_SYS_MAX_NAND_DEVICE ||
> -	    !nand_info[dev].name) {
> -		puts("\nno devices available\n");
> -		return 1;
> -	}
> -	nand = &nand_info[dev];
> +	nand = get_nand_dev_by_index(dev);
>  
>  	if (strcmp(cmd, "bad") == 0) {

You eliminated the error check -- now a NULL deref is likely if a bad dev is
requested.  Even if it's checked elsewhere when setting nand_curr_device, it's
possible that the initial default is bad (no NAND devices present, or device 0
failed).

>  		printf("\nDevice %d bad blocks:\n", dev);
> @@ -496,13 +494,13 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int
> argc, char * const argv[])
>  		/* skip first two or three arguments, look for offset and
> size */
>  		if (mtd_arg_off_size(argc - o, argv + o, &dev, &off, &size,
>  				     &maxsize, MTD_DEV_TYPE_NAND,
> -				     nand_info[dev].size) != 0)
> +				     nand->size) != 0)
>  			return 1;
>  
>  		if (set_dev(dev))
>  			return 1;
>  
> -		nand = &nand_info[dev];
> +		nand = get_nand_dev_by_index(dev);

Maybe have set_dev return the dev pointer?

Or have a global for the pointer rather than just the index, saving a bunch of
these calls.

-Scott

[U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option

[Scott Wood]

On Fri, 2016-04-01 at 08:51 -0400, Tom Rini wrote:
> On Fri, Apr 01, 2016 at 04:59:44PM +0530, Mugunthan V N wrote:
> 
> > Add CONFIG_NAND as a Kconfig option so that it can be selected
> > using menuconfig or defconfig.
> > 
> > Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> 
> Good but you need to update configs/ to remove NAND from
> CONFIG_SYS_EXTRA_OPTIONS and add CONFIG_NAND=y

NACK

That CONFIG_NAND is a target-local option used by some boards to indicate boot
source.  It is not equivalent to CONFIG_CMD_NAND which is what enables the
NAND subsystem.

-Scott

[U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass

[Scott Wood]

On Fri, 2016-04-01 at 16:59 +0530, Mugunthan V N wrote:
> +static int nand_child_pre_probe(struct udevice *dev)
> +{
> +	nand_info_t *nand = dev_get_uclass_priv(dev);
> +	void *priv = dev_get_priv(dev);
> +
> +	/*
> +	 * Store nand device priv pointer in nand_info so that
> +	 * it can be used by nand command
> +	 */
> +	nand->priv = priv;

Wouldn't it make more sense to have a pointer to the device in the NAND
struct, and let the driver manage both privs as it chooses?

> +
> +	return 0;
> +}
> +
> +UCLASS_DRIVER(nand) = {
> +	.id				= UCLASS_NAND,
> +	.name				= "nand",
> +	.flags				= DM_UC_FLAG_SEQ_ALIAS,
> +	.post_probe		= nand_child_pre_probe,
> +	.per_device_auto_alloc_size	= sizeof(nand_info_t),
> +};

Post probe = pre probe?

> @@ -63,8 +63,10 @@ int nand_register(int devnum)
>  static void nand_init_chip(int i)
>  {
>  	struct mtd_info *mtd;
> +#ifndef CONFIG_DM_NAND
>  	struct nand_chip *nand = &nand_chip[i];
>  	ulong base_addr = base_address[i];
> +#endif
>  	int maxchips = CONFIG_SYS_NAND_MAX_CHIPS;
>  
>  	if (maxchips < 1)
> @@ -74,11 +76,13 @@ static void nand_init_chip(int i)
>  	if (!mtd)
>  		return;
>  
> +#ifndef CONFIG_DM_NAND
>  	mtd->priv = nand;
>  	nand->IO_ADDR_R = nand->IO_ADDR_W = (void  __iomem *)base_addr;
>  
>  	if (board_nand_init(nand))
>  		return;
> +#endif
>  
>  	if (nand_scan(mtd, maxchips))
>  		return;

Please do not use this code for DM.  Have drivers' probe functions call
nand_scan (split into ident and tail if necessary) as is done with
 CONFIG_SYS_NAND_SELF_INIT.

> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index 37c4176..6a88d39 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -49,6 +49,7 @@ enum uclass_id {
>  	UCLASS_MMC,		/* SD / MMC card or chip */
>  	UCLASS_MOD_EXP,		/* RSA Mod Exp device */
>  	UCLASS_MTD,		/* Memory Technology Device (MTD) device
> */
> +	UCLASS_NAND,		/* NAND bus */

NAND is not a bus.

> +#ifndef CONFIG_DM_NAND
>  static inline nand_info_t *get_nand_dev_by_index(int dev)
>  {
>  	if (dev < 0 || dev >= CONFIG_SYS_MAX_NAND_DEVICE ||
> @@ -154,5 +156,8 @@ static inline nand_info_t *get_nand_dev_by_index(int
> dev)
>  
>  	return &nand_info[dev];
>  }
> +#else
> +nand_info_t *get_nand_dev_by_index(int idx);
> +#endif

Please use "if X/else", not "if !X/else".

-Scott

[U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass

[Scott Wood]

On Fri, 2016-04-01 at 18:25 -0500, Scott Wood wrote:
> On Fri, 2016-04-01 at 16:59 +0530, Mugunthan V N wrote:
> > +static int nand_child_pre_probe(struct udevice *dev)
> > +{
> > +	nand_info_t *nand = dev_get_uclass_priv(dev);
> > +	void *priv = dev_get_priv(dev);
> > +
> > +	/*
> > +	 * Store nand device priv pointer in nand_info so that
> > +	 * it can be used by nand command
> > +	 */
> > +	nand->priv = priv;
> 
> Wouldn't it make more sense to have a pointer to the device in the NAND
> struct, and let the driver manage both privs as it chooses?

This makes even less sense after seeing patch 5/9, which assumes dev priv is
nand_info_t, and stores its own data in nand->priv.  Won't this overwrite
that?

-Scott

[U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option

[Tom Rini]

On Fri, Apr 01, 2016 at 06:07:18PM -0500, Scott Wood wrote:

> On Fri, 2016-04-01 at 08:51 -0400, Tom Rini wrote:
> > On Fri, Apr 01, 2016 at 04:59:44PM +0530, Mugunthan V N wrote:
> > 
> > > Add CONFIG_NAND as a Kconfig option so that it can be selected
> > > using menuconfig or defconfig.
> > > 
> > > Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> > 
> > Good but you need to update configs/ to remove NAND from
> > CONFIG_SYS_EXTRA_OPTIONS and add CONFIG_NAND=y
> 
> NACK
> 
> That CONFIG_NAND is a target-local option used by some boards to indicate boot
> source.  It is not equivalent to CONFIG_CMD_NAND which is what enables the
> NAND subsystem.

Exactly!  We need to have 'NAND' moved from CONFIG_SYS_EXTRA_OPTIONS and
into a real Kconfig entry.  That said, I think this might have forgotten
to enable it in other places too but just 'NAND' needs to migrate out of
where it is.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160401/0ba70567/attachment.sig>

[U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option

[Scott Wood]

On Fri, 2016-04-01 at 19:41 -0400, Tom Rini wrote:
> On Fri, Apr 01, 2016 at 06:07:18PM -0500, Scott Wood wrote:
> 
> > On Fri, 2016-04-01 at 08:51 -0400, Tom Rini wrote:
> > > On Fri, Apr 01, 2016 at 04:59:44PM +0530, Mugunthan V N wrote:
> > > 
> > > > Add CONFIG_NAND as a Kconfig option so that it can be selected
> > > > using menuconfig or defconfig.
> > > > 
> > > > Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> > > 
> > > Good but you need to update configs/ to remove NAND from
> > > CONFIG_SYS_EXTRA_OPTIONS and add CONFIG_NAND=y
> > 
> > NACK
> > 
> > That CONFIG_NAND is a target-local option used by some boards to indicate
> > boot
> > source.  It is not equivalent to CONFIG_CMD_NAND which is what enables the
> > NAND subsystem.
> 
> Exactly!  We need to have 'NAND' moved from CONFIG_SYS_EXTRA_OPTIONS and
> into a real Kconfig entry.  That said, I think this might have forgotten
> to enable it in other places too but just 'NAND' needs to migrate out of
> where it is.

If we must start using NAND rather than CMD_NAND to enable the NAND subsystem
(which is what this patch does), then a lot more than that needs to change. 
 We'll need a new name for the boot source selection, and we'll need to
kconfigize all the boards that enable CONFIG_CMD_NAND via the board config
header.

-Scott

[U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option

[Tom Rini]

On Fri, Apr 01, 2016 at 06:45:03PM -0500, Scott Wood wrote:
> On Fri, 2016-04-01 at 19:41 -0400, Tom Rini wrote:
> > On Fri, Apr 01, 2016 at 06:07:18PM -0500, Scott Wood wrote:
> > 
> > > On Fri, 2016-04-01 at 08:51 -0400, Tom Rini wrote:
> > > > On Fri, Apr 01, 2016 at 04:59:44PM +0530, Mugunthan V N wrote:
> > > > 
> > > > > Add CONFIG_NAND as a Kconfig option so that it can be selected
> > > > > using menuconfig or defconfig.
> > > > > 
> > > > > Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> > > > 
> > > > Good but you need to update configs/ to remove NAND from
> > > > CONFIG_SYS_EXTRA_OPTIONS and add CONFIG_NAND=y
> > > 
> > > NACK
> > > 
> > > That CONFIG_NAND is a target-local option used by some boards to indicate
> > > boot
> > > source.  It is not equivalent to CONFIG_CMD_NAND which is what enables the
> > > NAND subsystem.
> > 
> > Exactly!  We need to have 'NAND' moved from CONFIG_SYS_EXTRA_OPTIONS and
> > into a real Kconfig entry.  That said, I think this might have forgotten
> > to enable it in other places too but just 'NAND' needs to migrate out of
> > where it is.
> 
> If we must start using NAND rather than CMD_NAND to enable the NAND subsystem
> (which is what this patch does), then a lot more than that needs to change. 
>  We'll need a new name for the boot source selection, and we'll need to
> kconfigize all the boards that enable CONFIG_CMD_NAND via the board config
> header.

OK, I see your point now too.  Yes, we need to tackle NAND/CMD_NAND
Kconfig stuff (including the tangled web of CMD_NAND being how we toggle
NAND the functionality) as a pre-patch series to this.  But it needs
doing :)

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160401/e9337838/attachment.sig>

[U-Boot] [PATCH 5/9] drivers: nand: omap_gpmc: convert driver to adopt driver model

[Scott Wood]

On Fri, 2016-04-01 at 16:59 +0530, Mugunthan V N wrote:
> adopt omap_gpmc driver to driver model.
> 
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
>  drivers/mtd/nand/omap_gpmc.c | 205
> +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 205 insertions(+)
> 
> diff --git a/drivers/mtd/nand/omap_gpmc.c b/drivers/mtd/nand/omap_gpmc.c
> index 4814fa2..df63491 100644
> --- a/drivers/mtd/nand/omap_gpmc.c
> +++ b/drivers/mtd/nand/omap_gpmc.c
> @@ -16,6 +16,10 @@
>  #include <nand.h>
>  #include <linux/mtd/omap_elm.h>
>  
> +#include <dm.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
>  #define BADBLOCK_MARKER_LENGTH	2
>  #define SECTOR_BYTES		512
>  #define ECCCLEAR		(0x1 << 8)
> @@ -46,11 +50,22 @@ struct omap_nand_info {
>  	enum omap_ecc ecc_scheme;
>  	uint8_t cs;
>  	uint8_t ws;		/* wait status pin (0,1) */
> +	uint8_t bus_width;	/* Bus width of NAND device */
>  };
>  
> +#ifndef CONFIG_DM_NAND
>  /* We are wasting a bit of memory but al least we are safe */
>  static struct omap_nand_info omap_nand_info[GPMC_MAX_CS];
>  
> +#else
> +
> +struct omap_gpmc_platdata {
> +	struct omap_nand_info *omap_nand_info;
> +	struct gpmc *gpmc_cfg;
> +	int max_cs;
> +};
> +#endif
> +
>  /*
>   * omap_nand_hwcontrol - Set the address pointers corretly for the
>   *			following address/data/command operation
> @@ -943,6 +958,8 @@ int __maybe_unused omap_nand_switch_ecc(uint32_t
> hardware, uint32_t eccstrength)
>  }
>  #endif /* CONFIG_SPL_BUILD */
>  
> +#ifndef CONFIG_DM_NAND
> +
>  /*
>   * Board-specific NAND initialization. The following members of the
>   * argument are board-specific:
> @@ -1034,3 +1051,191 @@ int board_nand_init(struct nand_chip *nand)
>  
>  	return 0;
>  }

> +
> +#else /* CONFIG_DM_NAND */
> +
> +static int omap_gpmc_probe(struct udevice *dev)
> +{
> +	struct nand_chip *nand = dev_get_priv(dev);
> +	struct omap_gpmc_platdata *pdata = dev_get_platdata(dev);
> +	struct gpmc *gpmc_cfg = pdata->gpmc_cfg;
> +	int32_t gpmc_config = 0;
> +	int ecc_opt;
> +	int cs = cs_next++;
> +	int err = 0;
> +
> +	while (cs < pdata->max_cs) {
> +		/* Check if NAND type is set */
> +		if ((readl(&gpmc_cfg->cs[cs].config1) & 0xC00) == 0x800) {
> +			/* Found it!! */
> +			break;
> +		}
> +		cs++;
> +	}
> +
> +	if (cs >= pdata->max_cs) {
> +		printf("nand: error: Unable to find NAND settings in GPMC
> Configuration - quitting\n");
> +		return -ENODEV;
> +	}
> +
> +	gpmc_config = readl(&gpmc_cfg->config);
> +	/* Disable Write protect */
> +	gpmc_config |= 0x10;
> +	writel(gpmc_config, &gpmc_cfg->config);
> +
> +	nand->IO_ADDR_R = (void __iomem *)&gpmc_cfg->cs[cs].nand_dat;
> +	nand->IO_ADDR_W = (void __iomem *)&gpmc_cfg->cs[cs].nand_cmd;
> +	nand->priv	= &pdata->omap_nand_info[cs];
> +	nand->cmd_ctrl	= omap_nand_hwcontrol;
> +	nand->options	|= NAND_NO_PADDING | NAND_CACHEPRG;
> +	nand->chip_delay = 100;
> +	nand->ecc.layout = &omap_ecclayout;
> +
> +	/* configure driver and controller based on NAND device bus-width
> */
> +	gpmc_config = readl(&gpmc_cfg->cs[cs].config1);
> +	if (pdata->omap_nand_info[cs].bus_width == 16) {
> +		nand->options |= NAND_BUSWIDTH_16;
> +		writel(gpmc_config | (0x1 << 12), &gpmc_cfg
> ->cs[cs].config1);
> +	} else {
> +		nand->options &= ~NAND_BUSWIDTH_16;
> +		writel(gpmc_config & ~(0x1 << 12), &gpmc_cfg
> ->cs[cs].config1);
> +	}
> +
> +	ecc_opt = pdata->omap_nand_info[cs].ecc_scheme;
> +	/* select ECC scheme */
> +	if (ecc_opt != OMAP_ECC_HAM1_CODE_SW) {
> +		err = omap_select_ecc_scheme(nand, ecc_opt,
> +					     CONFIG_SYS_NAND_PAGE_SIZE,
> +					     CONFIG_SYS_NAND_OOBSIZE);
> +	} else {
> +		/*
> +		 * pagesize and oobsize are not required to
> +		 * configure sw ecc-scheme
> +		 */
> +		err = omap_select_ecc_scheme(nand, ecc_opt, 0, 0);
> +	}
> +	if (err)
> +		return err;
> +
> +#ifdef CONFIG_NAND_OMAP_GPMC_PREFETCH
> +	nand->read_buf = omap_nand_read_prefetch;
> +#else
> +	if (nand->options & NAND_BUSWIDTH_16)
> +		nand->read_buf = nand_read_buf16;
> +	else
> +		nand->read_buf = nand_read_buf;
> +#endif
> +
> +	nand->dev_ready = omap_dev_ready;
> +
> +	return 0;
> +}

Is it really necessary to duplicate the entire probe func based on DM?

-Scott

[U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass

[Mugunthan V N]

Hi Scott

On Saturday 02 April 2016 05:01 AM, Scott Wood wrote:
> On Fri, 2016-04-01 at 18:25 -0500, Scott Wood wrote:
>> On Fri, 2016-04-01 at 16:59 +0530, Mugunthan V N wrote:
>>> +static int nand_child_pre_probe(struct udevice *dev)
>>> +{
>>> +	nand_info_t *nand = dev_get_uclass_priv(dev);
>>> +	void *priv = dev_get_priv(dev);
>>> +
>>> +	/*
>>> +	 * Store nand device priv pointer in nand_info so that
>>> +	 * it can be used by nand command
>>> +	 */
>>> +	nand->priv = priv;
>>
>> Wouldn't it make more sense to have a pointer to the device in the NAND
>> struct, and let the driver manage both privs as it chooses?
> 
> This makes even less sense after seeing patch 5/9, which assumes dev priv is
> nand_info_t, and stores its own data in nand->priv.  Won't this overwrite
> that?

This nand is not the same as in omap_gpmc driver priv nand, here nand
specifies nand_info_t which is "struct mtd_info" and in omap_gpmc driver
it is "struct nand_chip".

In class:
(nand_info_t) nand->priv = driver priv, ie nand_chip

In driver:
(nand_chip) nand->priv = struct omap_nand_info * (internal to driver)

So both are different and used for different purposes.

Regards
Mugunthan V N

[U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option

[Mugunthan V N]

Scott/Tom

On Saturday 02 April 2016 05:55 AM, Tom Rini wrote:
> On Fri, Apr 01, 2016 at 06:45:03PM -0500, Scott Wood wrote:
>> On Fri, 2016-04-01 at 19:41 -0400, Tom Rini wrote:
>>> On Fri, Apr 01, 2016 at 06:07:18PM -0500, Scott Wood wrote:
>>>
>>>> On Fri, 2016-04-01 at 08:51 -0400, Tom Rini wrote:
>>>>> On Fri, Apr 01, 2016 at 04:59:44PM +0530, Mugunthan V N wrote:
>>>>>
>>>>>> Add CONFIG_NAND as a Kconfig option so that it can be selected
>>>>>> using menuconfig or defconfig.
>>>>>>
>>>>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>>>>>
>>>>> Good but you need to update configs/ to remove NAND from
>>>>> CONFIG_SYS_EXTRA_OPTIONS and add CONFIG_NAND=y
>>>>
>>>> NACK
>>>>
>>>> That CONFIG_NAND is a target-local option used by some boards to indicate
>>>> boot
>>>> source.  It is not equivalent to CONFIG_CMD_NAND which is what enables the
>>>> NAND subsystem.
>>>
>>> Exactly!  We need to have 'NAND' moved from CONFIG_SYS_EXTRA_OPTIONS and
>>> into a real Kconfig entry.  That said, I think this might have forgotten
>>> to enable it in other places too but just 'NAND' needs to migrate out of
>>> where it is.
>>
>> If we must start using NAND rather than CMD_NAND to enable the NAND subsystem
>> (which is what this patch does), then a lot more than that needs to change. 
>>  We'll need a new name for the boot source selection, and we'll need to
>> kconfigize all the boards that enable CONFIG_CMD_NAND via the board config
>> header.
> 
> OK, I see your point now too.  Yes, we need to tackle NAND/CMD_NAND
> Kconfig stuff (including the tangled web of CMD_NAND being how we toggle
> NAND the functionality) as a pre-patch series to this.  But it needs
> doing :)

Should I be moving back NAND to "CONFIG_SYS_EXTRA_OPTIONS"?

Since CONFIG_CMD_NAND is used to enable NAND subsystem, so move
CONFIG_CMD_NAND to drivers/mtd/nand/Kconfig?

or am I missing something?

Regards
Mugunthan V N

[U-Boot] [PATCH 5/9] drivers: nand: omap_gpmc: convert driver to adopt driver model

[Mugunthan V N]

On Monday 04 April 2016 10:52 PM, Scott Wood wrote:
> On Fri, 2016-04-01 at 16:59 +0530, Mugunthan V N wrote:
>> adopt omap_gpmc driver to driver model.
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>> ---
>>  drivers/mtd/nand/omap_gpmc.c | 205
>> +++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 205 insertions(+)
>>
>> diff --git a/drivers/mtd/nand/omap_gpmc.c b/drivers/mtd/nand/omap_gpmc.c
>> index 4814fa2..df63491 100644
>> --- a/drivers/mtd/nand/omap_gpmc.c
>> +++ b/drivers/mtd/nand/omap_gpmc.c
>> @@ -16,6 +16,10 @@
>>  #include <nand.h>
>>  #include <linux/mtd/omap_elm.h>
>>  
>> +#include <dm.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>>  #define BADBLOCK_MARKER_LENGTH	2
>>  #define SECTOR_BYTES		512
>>  #define ECCCLEAR		(0x1 << 8)
>> @@ -46,11 +50,22 @@ struct omap_nand_info {
>>  	enum omap_ecc ecc_scheme;
>>  	uint8_t cs;
>>  	uint8_t ws;		/* wait status pin (0,1) */
>> +	uint8_t bus_width;	/* Bus width of NAND device */
>>  };
>>  
>> +#ifndef CONFIG_DM_NAND
>>  /* We are wasting a bit of memory but al least we are safe */
>>  static struct omap_nand_info omap_nand_info[GPMC_MAX_CS];
>>  
>> +#else
>> +
>> +struct omap_gpmc_platdata {
>> +	struct omap_nand_info *omap_nand_info;
>> +	struct gpmc *gpmc_cfg;
>> +	int max_cs;
>> +};
>> +#endif
>> +
>>  /*
>>   * omap_nand_hwcontrol - Set the address pointers corretly for the
>>   *			following address/data/command operation
>> @@ -943,6 +958,8 @@ int __maybe_unused omap_nand_switch_ecc(uint32_t
>> hardware, uint32_t eccstrength)
>>  }
>>  #endif /* CONFIG_SPL_BUILD */
>>  
>> +#ifndef CONFIG_DM_NAND
>> +
>>  /*
>>   * Board-specific NAND initialization. The following members of the
>>   * argument are board-specific:
>> @@ -1034,3 +1051,191 @@ int board_nand_init(struct nand_chip *nand)
>>  
>>  	return 0;
>>  }
> 
>> +
>> +#else /* CONFIG_DM_NAND */
>> +
>> +static int omap_gpmc_probe(struct udevice *dev)
>> +{
>> +	struct nand_chip *nand = dev_get_priv(dev);
>> +	struct omap_gpmc_platdata *pdata = dev_get_platdata(dev);
>> +	struct gpmc *gpmc_cfg = pdata->gpmc_cfg;
>> +	int32_t gpmc_config = 0;
>> +	int ecc_opt;
>> +	int cs = cs_next++;
>> +	int err = 0;
>> +
>> +	while (cs < pdata->max_cs) {
>> +		/* Check if NAND type is set */
>> +		if ((readl(&gpmc_cfg->cs[cs].config1) & 0xC00) == 0x800) {
>> +			/* Found it!! */
>> +			break;
>> +		}
>> +		cs++;
>> +	}
>> +
>> +	if (cs >= pdata->max_cs) {
>> +		printf("nand: error: Unable to find NAND settings in GPMC
>> Configuration - quitting\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	gpmc_config = readl(&gpmc_cfg->config);
>> +	/* Disable Write protect */
>> +	gpmc_config |= 0x10;
>> +	writel(gpmc_config, &gpmc_cfg->config);
>> +
>> +	nand->IO_ADDR_R = (void __iomem *)&gpmc_cfg->cs[cs].nand_dat;
>> +	nand->IO_ADDR_W = (void __iomem *)&gpmc_cfg->cs[cs].nand_cmd;
>> +	nand->priv	= &pdata->omap_nand_info[cs];
>> +	nand->cmd_ctrl	= omap_nand_hwcontrol;
>> +	nand->options	|= NAND_NO_PADDING | NAND_CACHEPRG;
>> +	nand->chip_delay = 100;
>> +	nand->ecc.layout = &omap_ecclayout;
>> +
>> +	/* configure driver and controller based on NAND device bus-width
>> */
>> +	gpmc_config = readl(&gpmc_cfg->cs[cs].config1);
>> +	if (pdata->omap_nand_info[cs].bus_width == 16) {
>> +		nand->options |= NAND_BUSWIDTH_16;
>> +		writel(gpmc_config | (0x1 << 12), &gpmc_cfg
>> ->cs[cs].config1);
>> +	} else {
>> +		nand->options &= ~NAND_BUSWIDTH_16;
>> +		writel(gpmc_config & ~(0x1 << 12), &gpmc_cfg
>> ->cs[cs].config1);
>> +	}
>> +
>> +	ecc_opt = pdata->omap_nand_info[cs].ecc_scheme;
>> +	/* select ECC scheme */
>> +	if (ecc_opt != OMAP_ECC_HAM1_CODE_SW) {
>> +		err = omap_select_ecc_scheme(nand, ecc_opt,
>> +					     CONFIG_SYS_NAND_PAGE_SIZE,
>> +					     CONFIG_SYS_NAND_OOBSIZE);
>> +	} else {
>> +		/*
>> +		 * pagesize and oobsize are not required to
>> +		 * configure sw ecc-scheme
>> +		 */
>> +		err = omap_select_ecc_scheme(nand, ecc_opt, 0, 0);
>> +	}
>> +	if (err)
>> +		return err;
>> +
>> +#ifdef CONFIG_NAND_OMAP_GPMC_PREFETCH
>> +	nand->read_buf = omap_nand_read_prefetch;
>> +#else
>> +	if (nand->options & NAND_BUSWIDTH_16)
>> +		nand->read_buf = nand_read_buf16;
>> +	else
>> +		nand->read_buf = nand_read_buf;
>> +#endif
>> +
>> +	nand->dev_ready = omap_dev_ready;
>> +
>> +	return 0;
>> +}
> 
> Is it really necessary to duplicate the entire probe func based on DM?
> 

It is not duplicated directly, if it is duplicate I would had a fuction
defined and call it in both DM and non-DM case. Removed some of the
#if/else. Only TODO I left is I have not remove nand driver callbacks
and move it to DM ops structure, which will be done soon.

Regards
Mugunthan V N

[U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option

[Tom Rini]

On Tue, Apr 05, 2016 at 02:07:46PM +0530, Mugunthan V N wrote:
> Scott/Tom
> 
> On Saturday 02 April 2016 05:55 AM, Tom Rini wrote:
> > On Fri, Apr 01, 2016 at 06:45:03PM -0500, Scott Wood wrote:
> >> On Fri, 2016-04-01 at 19:41 -0400, Tom Rini wrote:
> >>> On Fri, Apr 01, 2016 at 06:07:18PM -0500, Scott Wood wrote:
> >>>
> >>>> On Fri, 2016-04-01 at 08:51 -0400, Tom Rini wrote:
> >>>>> On Fri, Apr 01, 2016 at 04:59:44PM +0530, Mugunthan V N wrote:
> >>>>>
> >>>>>> Add CONFIG_NAND as a Kconfig option so that it can be selected
> >>>>>> using menuconfig or defconfig.
> >>>>>>
> >>>>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> >>>>>
> >>>>> Good but you need to update configs/ to remove NAND from
> >>>>> CONFIG_SYS_EXTRA_OPTIONS and add CONFIG_NAND=y
> >>>>
> >>>> NACK
> >>>>
> >>>> That CONFIG_NAND is a target-local option used by some boards to indicate
> >>>> boot
> >>>> source.  It is not equivalent to CONFIG_CMD_NAND which is what enables the
> >>>> NAND subsystem.
> >>>
> >>> Exactly!  We need to have 'NAND' moved from CONFIG_SYS_EXTRA_OPTIONS and
> >>> into a real Kconfig entry.  That said, I think this might have forgotten
> >>> to enable it in other places too but just 'NAND' needs to migrate out of
> >>> where it is.
> >>
> >> If we must start using NAND rather than CMD_NAND to enable the NAND subsystem
> >> (which is what this patch does), then a lot more than that needs to change. 
> >>  We'll need a new name for the boot source selection, and we'll need to
> >> kconfigize all the boards that enable CONFIG_CMD_NAND via the board config
> >> header.
> > 
> > OK, I see your point now too.  Yes, we need to tackle NAND/CMD_NAND
> > Kconfig stuff (including the tangled web of CMD_NAND being how we toggle
> > NAND the functionality) as a pre-patch series to this.  But it needs
> > doing :)
> 
> Should I be moving back NAND to "CONFIG_SYS_EXTRA_OPTIONS"?
> 
> Since CONFIG_CMD_NAND is used to enable NAND subsystem, so move
> CONFIG_CMD_NAND to drivers/mtd/nand/Kconfig?
> 
> or am I missing something?

I would like to see, but I want to hear Scott's opinion, move to
CONFIG_NAND is what enables NAND support for everyone (so yes, lots of
defconfigs will need an update) so that we can move it all over to
Kconfig.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160408/a5b56fad/attachment.sig>

[U-Boot] [PATCH 2/9] cmd: nand: abstract global variable usage for dm conversion

[Mugunthan V N]

On Saturday 02 April 2016 04:27 AM, Scott Wood wrote:
> [Please CC me at this address rather than my NXP address]
> 
> On Fri, 2016-04-01 at 16:59 +0530, Mugunthan V N wrote:
>> @@ -432,12 +435,7 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int
>> argc, char * const argv[])
>>  	 * one before these commands can run, even if a partition specifier
>>  	 * for another device is to be used.
>>  	 */
>> -	if (dev < 0 || dev >= CONFIG_SYS_MAX_NAND_DEVICE ||
>> -	    !nand_info[dev].name) {
>> -		puts("\nno devices available\n");
>> -		return 1;
>> -	}
>> -	nand = &nand_info[dev];
>> +	nand = get_nand_dev_by_index(dev);
>>  
>>  	if (strcmp(cmd, "bad") == 0) {
> 
> You eliminated the error check -- now a NULL deref is likely if a bad dev is
> requested.  Even if it's checked elsewhere when setting nand_curr_device, it's
> possible that the initial default is bad (no NAND devices present, or device 0
> failed).

Will fix this in my v2.

> 
>>  		printf("\nDevice %d bad blocks:\n", dev);
>> @@ -496,13 +494,13 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int
>> argc, char * const argv[])
>>  		/* skip first two or three arguments, look for offset and
>> size */
>>  		if (mtd_arg_off_size(argc - o, argv + o, &dev, &off, &size,
>>  				     &maxsize, MTD_DEV_TYPE_NAND,
>> -				     nand_info[dev].size) != 0)
>> +				     nand->size) != 0)
>>  			return 1;
>>  
>>  		if (set_dev(dev))
>>  			return 1;
>>  
>> -		nand = &nand_info[dev];
>> +		nand = get_nand_dev_by_index(dev);
> 
> Maybe have set_dev return the dev pointer?
> 
> Or have a global for the pointer rather than just the index, saving a bunch of
> these calls.
> 

Taking the same to global pointer can be done as a separate patch.

Regards
Mugunthan V N

[U-Boot] [PATCH 2/9] cmd: nand: abstract global variable usage for dm conversion

[Mugunthan V N]

On Friday 01 April 2016 04:59 PM, Mugunthan V N wrote:
> nand_info is used all over the file so abstract it with
> get_nand_dev_by_index() which will help for DM conversion.
> 
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
>  cmd/nand.c              | 72 ++++++++++++++++++++++++++-----------------------
>  drivers/mtd/nand/nand.c |  8 ++++--
>  include/nand.h          | 18 +++++++++++++
>  3 files changed, 62 insertions(+), 36 deletions(-)
> 
> diff --git a/cmd/nand.c b/cmd/nand.c
> index a6b67e2..ffb8d43 100644
> --- a/cmd/nand.c
> +++ b/cmd/nand.c
> @@ -114,21 +114,20 @@ free_dat:
>  
>  static int set_dev(int dev)
>  {
> -	if (dev < 0 || dev >= CONFIG_SYS_MAX_NAND_DEVICE ||
> -	    !nand_info[dev].name) {
> -		puts("No such device\n");
> -		return -1;
> -	}
> +	nand_info_t *nand = get_nand_dev_by_index(dev);
> +
> +	if (!nand)
> +		return -ENODEV;
>  
>  	if (nand_curr_device == dev)
>  		return 0;
>  
> -	printf("Device %d: %s", dev, nand_info[dev].name);
> +	printf("Device %d: %s", dev, nand->name);
>  	puts("... is now current device\n");
>  	nand_curr_device = dev;
>  
>  #ifdef CONFIG_SYS_NAND_SELECT_DEVICE
> -	board_nand_select_device(nand_info[dev].priv, dev);
> +	board_nand_select_device(nand->priv, dev);
>  #endif
>  
>  	return 0;
> @@ -188,7 +187,7 @@ int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[])
>  {
>  	int ret;
>  	uint32_t oob_buf[ENV_OFFSET_SIZE/sizeof(uint32_t)];
> -	nand_info_t *nand = &nand_info[0];
> +	nand_info_t *nand = get_nand_dev_by_index(0);
>  	char *cmd = argv[1];
>  
>  	if (CONFIG_SYS_MAX_NAND_DEVICE == 0 || !nand->name) {
> @@ -213,9 +212,10 @@ int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[])
>  		if (argc < 3)
>  			goto usage;
>  
> +		nand = get_nand_dev_by_index(idx);
>  		/* We don't care about size, or maxsize. */
>  		if (mtd_arg_off(argv[2], &idx, &addr, &maxsize, &maxsize,
> -				MTD_DEV_TYPE_NAND, nand_info[idx].size)) {
> +				MTD_DEV_TYPE_NAND, nand->size)) {
>  			puts("Offset or partition name expected\n");
>  			return 1;
>  		}
> @@ -283,9 +283,14 @@ usage:
>  
>  static void nand_print_and_set_info(int idx)
>  {
> -	nand_info_t *nand = &nand_info[idx];
> -	struct nand_chip *chip = nand->priv;
> +	nand_info_t *nand;
> +	struct nand_chip *chip;
>  
> +	nand = get_nand_dev_by_index(idx);
> +	if (!nand)
> +		return;
> +
> +	chip = nand->priv;
>  	printf("Device %d: ", idx);
>  	if (chip->numchips > 1)
>  		printf("%dx ", chip->numchips);
> @@ -348,7 +353,7 @@ static void adjust_size_for_badblocks(loff_t *size, loff_t offset, int dev)
>  	/* We grab the nand info object here fresh because this is usually
>  	 * called after arg_off_size() which can change the value of dev.
>  	 */
> -	nand_info_t *nand = &nand_info[dev];
> +	nand_info_t *nand = get_nand_dev_by_index(dev);
>  	loff_t maxoffset = offset + *size;
>  	int badblocks = 0;
>  
> @@ -397,10 +402,8 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  	if (strcmp(cmd, "info") == 0) {
>  
>  		putc('\n');
> -		for (i = 0; i < CONFIG_SYS_MAX_NAND_DEVICE; i++) {
> -			if (nand_info[i].name)
> -				nand_print_and_set_info(i);
> -		}
> +		for (i = 0; i < CONFIG_SYS_MAX_NAND_DEVICE; i++)
> +			nand_print_and_set_info(i);
>  		return 0;
>  	}
>  
> @@ -432,12 +435,7 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  	 * one before these commands can run, even if a partition specifier
>  	 * for another device is to be used.
>  	 */
> -	if (dev < 0 || dev >= CONFIG_SYS_MAX_NAND_DEVICE ||
> -	    !nand_info[dev].name) {
> -		puts("\nno devices available\n");
> -		return 1;
> -	}
> -	nand = &nand_info[dev];
> +	nand = get_nand_dev_by_index(dev);
>  
>  	if (strcmp(cmd, "bad") == 0) {
>  		printf("\nDevice %d bad blocks:\n", dev);
> @@ -496,13 +494,13 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  		/* skip first two or three arguments, look for offset and size */
>  		if (mtd_arg_off_size(argc - o, argv + o, &dev, &off, &size,
>  				     &maxsize, MTD_DEV_TYPE_NAND,
> -				     nand_info[dev].size) != 0)
> +				     nand->size) != 0)
>  			return 1;
>  
>  		if (set_dev(dev))
>  			return 1;
>  
> -		nand = &nand_info[dev];
> +		nand = get_nand_dev_by_index(dev);
>  
>  		memset(&opts, 0, sizeof(opts));
>  		opts.offset = off;
> @@ -561,13 +559,13 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  
>  			if (mtd_arg_off(argv[3], &dev, &off, &size, &maxsize,
>  					MTD_DEV_TYPE_NAND,
> -					nand_info[dev].size))
> +					nand->size))
>  				return 1;
>  
>  			if (set_dev(dev))
>  				return 1;
>  
> -			nand = &nand_info[dev];
> +			nand = get_nand_dev_by_index(dev);
>  
>  			if (argc > 4 && !str2long(argv[4], &pagecount)) {
>  				printf("'%s' is not a number\n", argv[4]);
> @@ -584,7 +582,7 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  			if (mtd_arg_off_size(argc - 3, argv + 3, &dev, &off,
>  					     &size, &maxsize,
>  					     MTD_DEV_TYPE_NAND,
> -					     nand_info[dev].size) != 0)
> +					     nand->size) != 0)
>  				return 1;
>  
>  			if (set_dev(dev))
> @@ -596,7 +594,7 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  			rwsize = size;
>  		}
>  
> -		nand = &nand_info[dev];
> +		nand = get_nand_dev_by_index(dev);
>  
>  		if (!s || !strcmp(s, ".jffs2") ||
>  		    !strcmp(s, ".e") || !strcmp(s, ".i")) {
> @@ -727,13 +725,15 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  
>  		if (mtd_arg_off_size(argc - 2, argv + 2, &dev, &off, &size,
>  				     &maxsize, MTD_DEV_TYPE_NAND,
> -				     nand_info[dev].size) < 0)
> +				     nand->size) < 0)
>  			return 1;
>  
>  		if (set_dev(dev))
>  			return 1;
>  
> -		if (!nand_unlock(&nand_info[dev], off, size, allexcept)) {
> +		nand = get_nand_dev_by_index(dev);
> +
> +		if (!nand_unlock(nand, off, size, allexcept)) {
>  			puts("NAND flash successfully unlocked\n");
>  		} else {
>  			puts("Error unlocking NAND flash, "
> @@ -899,6 +899,7 @@ static int do_nandboot(cmd_tbl_t *cmdtp, int flag, int argc,
>  	struct mtd_device *dev;
>  	struct part_info *part;
>  	u8 pnum;
> +	nand_info_t *nand;
>  
>  	if (argc >= 2) {
>  		char *p = (argc == 2) ? argv[1] : argv[2];
> @@ -914,8 +915,10 @@ static int do_nandboot(cmd_tbl_t *cmdtp, int flag, int argc,
>  				addr = simple_strtoul(argv[1], NULL, 16);
>  			else
>  				addr = CONFIG_SYS_LOAD_ADDR;
> -			return nand_load_image(cmdtp, &nand_info[dev->id->num],
> -					       part->offset, addr, argv[0]);
> +
> +			nand = get_nand_dev_by_index(dev->id->num);
> +			return nand_load_image(cmdtp, nand, part->offset,
> +					       addr, argv[0]);
>  		}
>  	}
>  #endif
> @@ -957,14 +960,15 @@ usage:
>  
>  	idx = simple_strtoul(boot_device, NULL, 16);
>  
> -	if (idx < 0 || idx >= CONFIG_SYS_MAX_NAND_DEVICE || !nand_info[idx].name) {
> +	nand = get_nand_dev_by_index(idx);
> +	if (!nand) {
>  		printf("\n** Device %d not available\n", idx);
>  		bootstage_error(BOOTSTAGE_ID_NAND_AVAILABLE);
>  		return 1;
>  	}
>  	bootstage_mark(BOOTSTAGE_ID_NAND_AVAILABLE);
>  
> -	return nand_load_image(cmdtp, &nand_info[idx], offset, addr, argv[0]);
> +	return nand_load_image(cmdtp, nand, offset, addr, argv[0]);
>  }
>  
>  U_BOOT_CMD(nboot, 4, 1, do_nandboot,
> diff --git a/drivers/mtd/nand/nand.c b/drivers/mtd/nand/nand.c
> index 8f0a921..524ead3 100644
> --- a/drivers/mtd/nand/nand.c
> +++ b/drivers/mtd/nand/nand.c
> @@ -38,7 +38,7 @@ int nand_register(int devnum)
>  	if (devnum >= CONFIG_SYS_MAX_NAND_DEVICE)
>  		return -EINVAL;
>  
> -	mtd = &nand_info[devnum];
> +	mtd = get_nand_dev_by_index(devnum);
>  
>  	sprintf(dev_name[devnum], "nand%d", devnum);
>  	mtd->name = dev_name[devnum];
> @@ -62,7 +62,7 @@ int nand_register(int devnum)
>  #ifndef CONFIG_SYS_NAND_SELF_INIT
>  static void nand_init_chip(int i)
>  {
> -	struct mtd_info *mtd = &nand_info[i];
> +	struct mtd_info *mtd;
>  	struct nand_chip *nand = &nand_chip[i];
>  	ulong base_addr = base_address[i];
>  	int maxchips = CONFIG_SYS_NAND_MAX_CHIPS;
> @@ -70,6 +70,10 @@ static void nand_init_chip(int i)
>  	if (maxchips < 1)
>  		maxchips = 1;
>  
> +	mtd = get_nand_dev_by_index(i);
> +	if (!mtd)
> +		return;

Found an issue here, in non-dm case get_nand_dev_by_index() will return
NULL as this is the first access and none of the nand_info[] is
initialized. below is the fix. Will include this in with v2 series

+#ifdef CONFIG_DM_NAND
+       mtd = get_nand_dev_by_index(i);
+       if (!mtd)
+               return;
+#else
+       mtd = &nand_info[i];
+#endif
+

Regards
Mugunthan V N

> +
>  	mtd->priv = nand;
>  	nand->IO_ADDR_R = nand->IO_ADDR_W = (void  __iomem *)base_addr;
>  
> diff --git a/include/nand.h b/include/nand.h
> index 4ea24ac..23b2414 100644
> --- a/include/nand.h
> +++ b/include/nand.h
> @@ -137,4 +137,22 @@ int get_nand_env_oob(nand_info_t *nand, unsigned long *result);
>  #endif
>  int spl_nand_erase_one(int block, int page);
>  
> +/*
> + * get_nand_dev_by_index - Get the nand info based in index.
> + *
> + * @dev - index to the nand device.
> + *
> + * returns pointer to the nand device info structure or NULL on failure.
> + */
> +static inline nand_info_t *get_nand_dev_by_index(int dev)
> +{
> +	if (dev < 0 || dev >= CONFIG_SYS_MAX_NAND_DEVICE ||
> +	    !nand_info[dev].name) {
> +		puts("No such device\n");
> +		return NULL;
> +	}
> +
> +	return &nand_info[dev];
> +}
> +
>  #endif /* _NAND_H_ */
> 

[U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option

[Scott Wood]

On Fri, 2016-04-08 at 15:45 -0400, Tom Rini wrote:
> On Tue, Apr 05, 2016 at 02:07:46PM +0530, Mugunthan V N wrote:
> > Scott/Tom
> > 
> > On Saturday 02 April 2016 05:55 AM, Tom Rini wrote:
> > > On Fri, Apr 01, 2016 at 06:45:03PM -0500, Scott Wood wrote:
> > > > On Fri, 2016-04-01 at 19:41 -0400, Tom Rini wrote:
> > > > > On Fri, Apr 01, 2016 at 06:07:18PM -0500, Scott Wood wrote:
> > > > > 
> > > > > > On Fri, 2016-04-01 at 08:51 -0400, Tom Rini wrote:
> > > > > > > On Fri, Apr 01, 2016 at 04:59:44PM +0530, Mugunthan V N wrote:
> > > > > > > 
> > > > > > > > Add CONFIG_NAND as a Kconfig option so that it can be selected
> > > > > > > > using menuconfig or defconfig.
> > > > > > > > 
> > > > > > > > Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> > > > > > > 
> > > > > > > Good but you need to update configs/ to remove NAND from
> > > > > > > CONFIG_SYS_EXTRA_OPTIONS and add CONFIG_NAND=y
> > > > > > 
> > > > > > NACK
> > > > > > 
> > > > > > That CONFIG_NAND is a target-local option used by some boards to
> > > > > > indicate
> > > > > > boot
> > > > > > source.  It is not equivalent to CONFIG_CMD_NAND which is what
> > > > > > enables the
> > > > > > NAND subsystem.
> > > > > 
> > > > > Exactly!  We need to have 'NAND' moved from CONFIG_SYS_EXTRA_OPTIONS
> > > > > and
> > > > > into a real Kconfig entry.  That said, I think this might have
> > > > > forgotten
> > > > > to enable it in other places too but just 'NAND' needs to migrate
> > > > > out of
> > > > > where it is.
> > > > 
> > > > If we must start using NAND rather than CMD_NAND to enable the NAND
> > > > subsystem
> > > > (which is what this patch does), then a lot more than that needs to
> > > > change. 
> > > >  We'll need a new name for the boot source selection, and we'll need
> > > > to
> > > > kconfigize all the boards that enable CONFIG_CMD_NAND via the board
> > > > config
> > > > header.
> > > 
> > > OK, I see your point now too.  Yes, we need to tackle NAND/CMD_NAND
> > > Kconfig stuff (including the tangled web of CMD_NAND being how we toggle
> > > NAND the functionality) as a pre-patch series to this.  But it needs
> > > doing :)
> > 
> > Should I be moving back NAND to "CONFIG_SYS_EXTRA_OPTIONS"?
> > 
> > Since CONFIG_CMD_NAND is used to enable NAND subsystem, so move
> > CONFIG_CMD_NAND to drivers/mtd/nand/Kconfig?
> > 
> > or am I missing something?
> 
> I would like to see, but I want to hear Scott's opinion, move to
> CONFIG_NAND is what enables NAND support for everyone (so yes, lots of
> defconfigs will need an update) so that we can move it all over to
> Kconfig.

I'm fine with changing the names as long as everything gets updated properly. 
 The boards which use CONFIG_NAND to mean NAND boot (which is a relic of an
old and dead config mechanism that parsed extra words added to a target name)
can be changed to CONFIG_NAND_BOOT.

-Scott

[U-Boot] [PATCH 1/9] include: nand: move endif to end of file

[Simon Glass]

On 1 April 2016 at 05:29, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> The terminator endif of ifdef _NAND_H_ should be at the
> end of file as a fail safe.
>
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
>  include/nand.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass

[Simon Glass]

Hi Mugunthan,

On 1 April 2016 at 05:29, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> Implement a NAND uclass so that the NAND devices can be
> accessed via the DM framework.
>
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
>  drivers/mtd/nand/Kconfig       | 10 +++++++
>  drivers/mtd/nand/Makefile      |  2 ++
>  drivers/mtd/nand/nand-uclass.c | 62 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/mtd/nand/nand.c        |  6 +++-
>  include/dm/uclass-id.h         |  1 +
>  include/nand.h                 |  5 ++++
>  6 files changed, 85 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/mtd/nand/nand-uclass.c
>
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index 7787505..53b57b8 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -8,6 +8,16 @@ menuconfig NAND
>
>  if NAND
>
> +config DM_NAND
> +       bool "Enable driver model for NAND"
> +       depends on NAND && DM
> +       help
> +         Enable driver model for NAND. The NAND interface is then
> +         implemented by the NAND uclass. Multiple NAND devices can
> +         be attached and used. The 'nand' command works as normal.
> +
> +         If the NAND drivers doesn't support DM, say N.
> +
>  config SYS_NAND_SELF_INIT
>         bool
>         help
> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> index 6fb3718..7745705 100644
> --- a/drivers/mtd/nand/Makefile
> +++ b/drivers/mtd/nand/Makefile
> @@ -39,6 +39,8 @@ endif # not spl
>
>  ifdef NORMAL_DRIVERS
>
> +obj-$(CONFIG_DM_NAND) += nand-uclass.o
> +
>  obj-$(CONFIG_NAND_ECC_BCH) += nand_bch.o
>
>  obj-$(CONFIG_NAND_ATMEL) += atmel_nand.o
> diff --git a/drivers/mtd/nand/nand-uclass.c b/drivers/mtd/nand/nand-uclass.c
> new file mode 100644
> index 0000000..d68d357
> --- /dev/null
> +++ b/drivers/mtd/nand/nand-uclass.c
> @@ -0,0 +1,62 @@
> +/*
> + * NAND uclass driver for NAND bus.
> + *
> + * (C) Copyright 2012-2016
> + *     Texas Instruments Incorporated, <www.ti.com>
> + *
> + * SPDX-License-Identifier:     GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <nand.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#ifdef CONFIG_DM_NAND
> +
> +nand_info_t *get_nand_dev_by_index(int idx)
> +{
> +       nand_info_t *nand;
> +       struct udevice *dev;
> +       int ret;
> +
> +       ret = uclass_get_device(UCLASS_NAND, idx, &dev);
> +       if (ret) {
> +               error("NAND device (%d) not found\n", idx);

This should return -ENODEV. Also please avoid printf() in drivers. The
caller can report the error.

> +               return NULL;
> +       }
> +
> +       nand = (nand_info_t *)dev_get_uclass_priv(dev);
> +       if (!nand) {
> +               error("Nand device not ready\n");
> +               return NULL;
> +       }

But if the device has been probed ((with uclass_get_device()) then
this cannot be NULL.

> +
> +       return nand;
> +}
> +
> +static int nand_child_pre_probe(struct udevice *dev)
> +{
> +       nand_info_t *nand = dev_get_uclass_priv(dev);
> +       void *priv = dev_get_priv(dev);

Please use the actual struct here, not void.

> +
> +       /*
> +        * Store nand device priv pointer in nand_info so that
> +        * it can be used by nand command
> +        */
> +       nand->priv = priv;
()> +
> +       return 0;
> +}
> +
> +UCLASS_DRIVER(nand) = {
> +       .id                             = UCLASS_NAND,
> +       .name                           = "nand",
> +       .flags                          = DM_UC_FLAG_SEQ_ALIAS,
> +       .post_probe             = nand_child_pre_probe,
> +       .per_device_auto_alloc_size     = sizeof(nand_info_t),
> +};
> +
> +#endif
> diff --git a/drivers/mtd/nand/nand.c b/drivers/mtd/nand/nand.c
> index 524ead3..c03a7e5 100644
> --- a/drivers/mtd/nand/nand.c
> +++ b/drivers/mtd/nand/nand.c
> @@ -21,7 +21,7 @@ int nand_curr_device = -1;
>
>  nand_info_t nand_info[CONFIG_SYS_MAX_NAND_DEVICE];
>
> -#ifndef CONFIG_SYS_NAND_SELF_INIT
> +#if !defined(CONFIG_SYS_NAND_SELF_INIT) && !defined(CONFIG_DM_NAND)
>  static struct nand_chip nand_chip[CONFIG_SYS_MAX_NAND_DEVICE];
>  static ulong base_address[CONFIG_SYS_MAX_NAND_DEVICE] = CONFIG_SYS_NAND_BASE_LIST;
>  #endif
> @@ -63,8 +63,10 @@ int nand_register(int devnum)
>  static void nand_init_chip(int i)
>  {
>         struct mtd_info *mtd;
> +#ifndef CONFIG_DM_NAND
>         struct nand_chip *nand = &nand_chip[i];
>         ulong base_addr = base_address[i];
> +#endif
>         int maxchips = CONFIG_SYS_NAND_MAX_CHIPS;
>
>         if (maxchips < 1)
> @@ -74,11 +76,13 @@ static void nand_init_chip(int i)
>         if (!mtd)
>                 return;
>
> +#ifndef CONFIG_DM_NAND
>         mtd->priv = nand;
>         nand->IO_ADDR_R = nand->IO_ADDR_W = (void  __iomem *)base_addr;
>
>         if (board_nand_init(nand))
>                 return;
> +#endif
>
>         if (nand_scan(mtd, maxchips))
>                 return;
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index 37c4176..6a88d39 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -49,6 +49,7 @@ enum uclass_id {
>         UCLASS_MMC,             /* SD / MMC card or chip */
>         UCLASS_MOD_EXP,         /* RSA Mod Exp device */
>         UCLASS_MTD,             /* Memory Technology Device (MTD) device */
> +       UCLASS_NAND,            /* NAND bus */

Is 'bus' the right word? Perhaps 'device'?

>         UCLASS_NORTHBRIDGE,     /* Intel Northbridge / SDRAM controller */
>         UCLASS_PANEL,           /* Display panel, such as an LCD */
>         UCLASS_PANEL_BACKLIGHT, /* Backlight controller for panel */
> diff --git a/include/nand.h b/include/nand.h
> index 23b2414..1580970 100644
> --- a/include/nand.h
> +++ b/include/nand.h
> @@ -10,6 +10,7 @@
>  #define _NAND_H_
>
>  #include <config.h>
> +#include <dm.h>
>
>  /*
>   * All boards using a given driver must convert to self-init
> @@ -144,6 +145,7 @@ int spl_nand_erase_one(int block, int page);
>   *
>   * returns pointer to the nand device info structure or NULL on failure.
>   */
> +#ifndef CONFIG_DM_NAND
>  static inline nand_info_t *get_nand_dev_by_index(int dev)
>  {
>         if (dev < 0 || dev >= CONFIG_SYS_MAX_NAND_DEVICE ||
> @@ -154,5 +156,8 @@ static inline nand_info_t *get_nand_dev_by_index(int dev)
>
>         return &nand_info[dev];
>  }
> +#else
> +nand_info_t *get_nand_dev_by_index(int idx);
> +#endif

Can you please document in this header file what dev_get_priv() holds
for a NAND device, and dev_get_uclass_priv().

>
>  #endif /* _NAND_H_ */
> --
> 2.8.0.rc3.9.g44915db
>

Regards,
Simon

[U-Boot] [PATCH 6/9] am43xx_evm: nand: do not define DM_NAND for spl

[Simon Glass]

On 1 April 2016 at 05:29, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> Since OMAP's spl doesn't support DM currently, do not define
> DM_NAND for spl build.
>
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
>  include/configs/am43xx_evm.h | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 7/9] defconfig: am437x_gp_evm: enable NAND driver model

[Simon Glass]

On 1 April 2016 at 06:52, Tom Rini <trini@konsulko.com> wrote:
> On Fri, Apr 01, 2016 at 04:59:48PM +0530, Mugunthan V N wrote:
>
>> Enable NAND driver model for am437x_gp_evm as omap_gpmc supports
>> driver model.
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 8/9] am335x_evm: nand: do not define DM_NAND for spl

[Simon Glass]

On 1 April 2016 at 06:52, Tom Rini <trini@konsulko.com> wrote:
> On Fri, Apr 01, 2016 at 04:59:49PM +0530, Mugunthan V N wrote:
>
>> Since OMAP's spl doesn't support DM currently, do not define
>> DM_NAND for spl build.
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 9/9] defconfig: am335x_gp_evm: enable NAND driver model

[Simon Glass]

On 1 April 2016 at 05:29, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> Enable NAND driver model for am335x_gp_evm as omap_gpmc supports
> driver model.
>
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
>  configs/am335x_gp_evm_defconfig | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass

[Mugunthan V N]

On Wednesday 20 April 2016 08:09 PM, Simon Glass wrote:
> Hi Mugunthan,
> 
> On 1 April 2016 at 05:29, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>> Implement a NAND uclass so that the NAND devices can be
>> accessed via the DM framework.
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>> ---
>>  drivers/mtd/nand/Kconfig       | 10 +++++++
>>  drivers/mtd/nand/Makefile      |  2 ++
>>  drivers/mtd/nand/nand-uclass.c | 62 ++++++++++++++++++++++++++++++++++++++++++
>>  drivers/mtd/nand/nand.c        |  6 +++-
>>  include/dm/uclass-id.h         |  1 +
>>  include/nand.h                 |  5 ++++
>>  6 files changed, 85 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/mtd/nand/nand-uclass.c
>>
>> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
>> index 7787505..53b57b8 100644
>> --- a/drivers/mtd/nand/Kconfig
>> +++ b/drivers/mtd/nand/Kconfig
>> @@ -8,6 +8,16 @@ menuconfig NAND
>>
>>  if NAND
>>
>> +config DM_NAND
>> +       bool "Enable driver model for NAND"
>> +       depends on NAND && DM
>> +       help
>> +         Enable driver model for NAND. The NAND interface is then
>> +         implemented by the NAND uclass. Multiple NAND devices can
>> +         be attached and used. The 'nand' command works as normal.
>> +
>> +         If the NAND drivers doesn't support DM, say N.
>> +
>>  config SYS_NAND_SELF_INIT
>>         bool
>>         help
>> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
>> index 6fb3718..7745705 100644
>> --- a/drivers/mtd/nand/Makefile
>> +++ b/drivers/mtd/nand/Makefile
>> @@ -39,6 +39,8 @@ endif # not spl
>>
>>  ifdef NORMAL_DRIVERS
>>
>> +obj-$(CONFIG_DM_NAND) += nand-uclass.o
>> +
>>  obj-$(CONFIG_NAND_ECC_BCH) += nand_bch.o
>>
>>  obj-$(CONFIG_NAND_ATMEL) += atmel_nand.o
>> diff --git a/drivers/mtd/nand/nand-uclass.c b/drivers/mtd/nand/nand-uclass.c
>> new file mode 100644
>> index 0000000..d68d357
>> --- /dev/null
>> +++ b/drivers/mtd/nand/nand-uclass.c
>> @@ -0,0 +1,62 @@
>> +/*
>> + * NAND uclass driver for NAND bus.
>> + *
>> + * (C) Copyright 2012-2016
>> + *     Texas Instruments Incorporated, <www.ti.com>
>> + *
>> + * SPDX-License-Identifier:     GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <errno.h>
>> +#include <nand.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +#ifdef CONFIG_DM_NAND
>> +
>> +nand_info_t *get_nand_dev_by_index(int idx)
>> +{
>> +       nand_info_t *nand;
>> +       struct udevice *dev;
>> +       int ret;
>> +
>> +       ret = uclass_get_device(UCLASS_NAND, idx, &dev);
>> +       if (ret) {
>> +               error("NAND device (%d) not found\n", idx);
> 
> This should return -ENODEV. Also please avoid printf() in drivers. The
> caller can report the error.
> 

Return type is pointer so returned NULL and NULL denotes no dev.
Will change this error to debug in v2.

>> +               return NULL;
>> +       }
>> +
>> +       nand = (nand_info_t *)dev_get_uclass_priv(dev);
>> +       if (!nand) {
>> +               error("Nand device not ready\n");
>> +               return NULL;
>> +       }
> 
> But if the device has been probed ((with uclass_get_device()) then
> this cannot be NULL.

This check is just a failsafe. ideally it should not fail here.

> 
>> +
>> +       return nand;
>> +}
>> +
>> +static int nand_child_pre_probe(struct udevice *dev)
>> +{
>> +       nand_info_t *nand = dev_get_uclass_priv(dev);
>> +       void *priv = dev_get_priv(dev);
> 
> Please use the actual struct here, not void.

nand->priv is a void *, so used void *

> 
>> +
>> +       /*
>> +        * Store nand device priv pointer in nand_info so that
>> +        * it can be used by nand command
>> +        */
>> +       nand->priv = priv;
> ()> +
>> +       return 0;
>> +}
>> +
>> +UCLASS_DRIVER(nand) = {
>> +       .id                             = UCLASS_NAND,
>> +       .name                           = "nand",
>> +       .flags                          = DM_UC_FLAG_SEQ_ALIAS,
>> +       .post_probe             = nand_child_pre_probe,
>> +       .per_device_auto_alloc_size     = sizeof(nand_info_t),
>> +};
>> +
>> +#endif
>> diff --git a/drivers/mtd/nand/nand.c b/drivers/mtd/nand/nand.c
>> index 524ead3..c03a7e5 100644
>> --- a/drivers/mtd/nand/nand.c
>> +++ b/drivers/mtd/nand/nand.c
>> @@ -21,7 +21,7 @@ int nand_curr_device = -1;
>>
>>  nand_info_t nand_info[CONFIG_SYS_MAX_NAND_DEVICE];
>>
>> -#ifndef CONFIG_SYS_NAND_SELF_INIT
>> +#if !defined(CONFIG_SYS_NAND_SELF_INIT) && !defined(CONFIG_DM_NAND)
>>  static struct nand_chip nand_chip[CONFIG_SYS_MAX_NAND_DEVICE];
>>  static ulong base_address[CONFIG_SYS_MAX_NAND_DEVICE] = CONFIG_SYS_NAND_BASE_LIST;
>>  #endif
>> @@ -63,8 +63,10 @@ int nand_register(int devnum)
>>  static void nand_init_chip(int i)
>>  {
>>         struct mtd_info *mtd;
>> +#ifndef CONFIG_DM_NAND
>>         struct nand_chip *nand = &nand_chip[i];
>>         ulong base_addr = base_address[i];
>> +#endif
>>         int maxchips = CONFIG_SYS_NAND_MAX_CHIPS;
>>
>>         if (maxchips < 1)
>> @@ -74,11 +76,13 @@ static void nand_init_chip(int i)
>>         if (!mtd)
>>                 return;
>>
>> +#ifndef CONFIG_DM_NAND
>>         mtd->priv = nand;
>>         nand->IO_ADDR_R = nand->IO_ADDR_W = (void  __iomem *)base_addr;
>>
>>         if (board_nand_init(nand))
>>                 return;
>> +#endif
>>
>>         if (nand_scan(mtd, maxchips))
>>                 return;
>> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
>> index 37c4176..6a88d39 100644
>> --- a/include/dm/uclass-id.h
>> +++ b/include/dm/uclass-id.h
>> @@ -49,6 +49,7 @@ enum uclass_id {
>>         UCLASS_MMC,             /* SD / MMC card or chip */
>>         UCLASS_MOD_EXP,         /* RSA Mod Exp device */
>>         UCLASS_MTD,             /* Memory Technology Device (MTD) device */
>> +       UCLASS_NAND,            /* NAND bus */
> 
> Is 'bus' the right word? Perhaps 'device'?

Yeah, will fix this in v2

> 
>>         UCLASS_NORTHBRIDGE,     /* Intel Northbridge / SDRAM controller */
>>         UCLASS_PANEL,           /* Display panel, such as an LCD */
>>         UCLASS_PANEL_BACKLIGHT, /* Backlight controller for panel */
>> diff --git a/include/nand.h b/include/nand.h
>> index 23b2414..1580970 100644
>> --- a/include/nand.h
>> +++ b/include/nand.h
>> @@ -10,6 +10,7 @@
>>  #define _NAND_H_
>>
>>  #include <config.h>
>> +#include <dm.h>
>>
>>  /*
>>   * All boards using a given driver must convert to self-init
>> @@ -144,6 +145,7 @@ int spl_nand_erase_one(int block, int page);
>>   *
>>   * returns pointer to the nand device info structure or NULL on failure.
>>   */
>> +#ifndef CONFIG_DM_NAND
>>  static inline nand_info_t *get_nand_dev_by_index(int dev)
>>  {
>>         if (dev < 0 || dev >= CONFIG_SYS_MAX_NAND_DEVICE ||
>> @@ -154,5 +156,8 @@ static inline nand_info_t *get_nand_dev_by_index(int dev)
>>
>>         return &nand_info[dev];
>>  }
>> +#else
>> +nand_info_t *get_nand_dev_by_index(int idx);
>> +#endif
> 
> Can you please document in this header file what dev_get_priv() holds
> for a NAND device, and dev_get_uclass_priv().

Will document this in v2

Regards
Mugunthan V N

[U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass

[Simon Glass]

Hi Mugunthan,

On 20 April 2016 at 23:55, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> On Wednesday 20 April 2016 08:09 PM, Simon Glass wrote:
>> Hi Mugunthan,
>>
>> On 1 April 2016 at 05:29, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>>> Implement a NAND uclass so that the NAND devices can be
>>> accessed via the DM framework.
>>>
>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>>> ---
>>>  drivers/mtd/nand/Kconfig       | 10 +++++++
>>>  drivers/mtd/nand/Makefile      |  2 ++
>>>  drivers/mtd/nand/nand-uclass.c | 62 ++++++++++++++++++++++++++++++++++++++++++
>>>  drivers/mtd/nand/nand.c        |  6 +++-
>>>  include/dm/uclass-id.h         |  1 +
>>>  include/nand.h                 |  5 ++++
>>>  6 files changed, 85 insertions(+), 1 deletion(-)
>>>  create mode 100644 drivers/mtd/nand/nand-uclass.c
>>>
>>> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
>>> index 7787505..53b57b8 100644
>>> --- a/drivers/mtd/nand/Kconfig
>>> +++ b/drivers/mtd/nand/Kconfig
>>> @@ -8,6 +8,16 @@ menuconfig NAND
>>>
>>>  if NAND
>>>
>>> +config DM_NAND
>>> +       bool "Enable driver model for NAND"
>>> +       depends on NAND && DM
>>> +       help
>>> +         Enable driver model for NAND. The NAND interface is then
>>> +         implemented by the NAND uclass. Multiple NAND devices can
>>> +         be attached and used. The 'nand' command works as normal.
>>> +
>>> +         If the NAND drivers doesn't support DM, say N.
>>> +
>>>  config SYS_NAND_SELF_INIT
>>>         bool
>>>         help
>>> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
>>> index 6fb3718..7745705 100644
>>> --- a/drivers/mtd/nand/Makefile
>>> +++ b/drivers/mtd/nand/Makefile
>>> @@ -39,6 +39,8 @@ endif # not spl
>>>
>>>  ifdef NORMAL_DRIVERS
>>>
>>> +obj-$(CONFIG_DM_NAND) += nand-uclass.o
>>> +
>>>  obj-$(CONFIG_NAND_ECC_BCH) += nand_bch.o
>>>
>>>  obj-$(CONFIG_NAND_ATMEL) += atmel_nand.o
>>> diff --git a/drivers/mtd/nand/nand-uclass.c b/drivers/mtd/nand/nand-uclass.c
>>> new file mode 100644
>>> index 0000000..d68d357
>>> --- /dev/null
>>> +++ b/drivers/mtd/nand/nand-uclass.c
>>> @@ -0,0 +1,62 @@
>>> +/*
>>> + * NAND uclass driver for NAND bus.
>>> + *
>>> + * (C) Copyright 2012-2016
>>> + *     Texas Instruments Incorporated, <www.ti.com>
>>> + *
>>> + * SPDX-License-Identifier:     GPL-2.0+
>>> + */
>>> +
>>> +#include <common.h>
>>> +#include <dm.h>
>>> +#include <errno.h>
>>> +#include <nand.h>
>>> +
>>> +DECLARE_GLOBAL_DATA_PTR;
>>> +
>>> +#ifdef CONFIG_DM_NAND
>>> +
>>> +nand_info_t *get_nand_dev_by_index(int idx)
>>> +{
>>> +       nand_info_t *nand;
>>> +       struct udevice *dev;
>>> +       int ret;
>>> +
>>> +       ret = uclass_get_device(UCLASS_NAND, idx, &dev);
>>> +       if (ret) {
>>> +               error("NAND device (%d) not found\n", idx);
>>
>> This should return -ENODEV. Also please avoid printf() in drivers. The
>> caller can report the error.
>>
>
> Return type is pointer so returned NULL and NULL denotes no dev.
> Will change this error to debug in v2.

OK, I see that this is the existing API.

>
>>> +               return NULL;
>>> +       }
>>> +
>>> +       nand = (nand_info_t *)dev_get_uclass_priv(dev);
>>> +       if (!nand) {
>>> +               error("Nand device not ready\n");
>>> +               return NULL;
>>> +       }
>>
>> But if the device has been probed ((with uclass_get_device()) then
>> this cannot be NULL.
>
> This check is just a failsafe. ideally it should not fail here.

I cannot get to probe the device without this data, since this would
be a bug in DM. So the check has no value.

[snip]

Regards,
Simon

[U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass

[Mugunthan V N]

On Thursday 21 April 2016 07:41 PM, Simon Glass wrote:
> Hi Mugunthan,
> 
> On 20 April 2016 at 23:55, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>> On Wednesday 20 April 2016 08:09 PM, Simon Glass wrote:
>>> Hi Mugunthan,
>>>
>>> On 1 April 2016 at 05:29, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>>>> Implement a NAND uclass so that the NAND devices can be
>>>> accessed via the DM framework.
>>>>
>>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>>>> ---
>>>>  drivers/mtd/nand/Kconfig       | 10 +++++++
>>>>  drivers/mtd/nand/Makefile      |  2 ++
>>>>  drivers/mtd/nand/nand-uclass.c | 62 ++++++++++++++++++++++++++++++++++++++++++
>>>>  drivers/mtd/nand/nand.c        |  6 +++-
>>>>  include/dm/uclass-id.h         |  1 +
>>>>  include/nand.h                 |  5 ++++
>>>>  6 files changed, 85 insertions(+), 1 deletion(-)
>>>>  create mode 100644 drivers/mtd/nand/nand-uclass.c
>>>>
>>>> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
>>>> index 7787505..53b57b8 100644
>>>> --- a/drivers/mtd/nand/Kconfig
>>>> +++ b/drivers/mtd/nand/Kconfig
>>>> @@ -8,6 +8,16 @@ menuconfig NAND
>>>>
>>>>  if NAND
>>>>
>>>> +config DM_NAND
>>>> +       bool "Enable driver model for NAND"
>>>> +       depends on NAND && DM
>>>> +       help
>>>> +         Enable driver model for NAND. The NAND interface is then
>>>> +         implemented by the NAND uclass. Multiple NAND devices can
>>>> +         be attached and used. The 'nand' command works as normal.
>>>> +
>>>> +         If the NAND drivers doesn't support DM, say N.
>>>> +
>>>>  config SYS_NAND_SELF_INIT
>>>>         bool
>>>>         help
>>>> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
>>>> index 6fb3718..7745705 100644
>>>> --- a/drivers/mtd/nand/Makefile
>>>> +++ b/drivers/mtd/nand/Makefile
>>>> @@ -39,6 +39,8 @@ endif # not spl
>>>>
>>>>  ifdef NORMAL_DRIVERS
>>>>
>>>> +obj-$(CONFIG_DM_NAND) += nand-uclass.o
>>>> +
>>>>  obj-$(CONFIG_NAND_ECC_BCH) += nand_bch.o
>>>>
>>>>  obj-$(CONFIG_NAND_ATMEL) += atmel_nand.o
>>>> diff --git a/drivers/mtd/nand/nand-uclass.c b/drivers/mtd/nand/nand-uclass.c
>>>> new file mode 100644
>>>> index 0000000..d68d357
>>>> --- /dev/null
>>>> +++ b/drivers/mtd/nand/nand-uclass.c
>>>> @@ -0,0 +1,62 @@
>>>> +/*
>>>> + * NAND uclass driver for NAND bus.
>>>> + *
>>>> + * (C) Copyright 2012-2016
>>>> + *     Texas Instruments Incorporated, <www.ti.com>
>>>> + *
>>>> + * SPDX-License-Identifier:     GPL-2.0+
>>>> + */
>>>> +
>>>> +#include <common.h>
>>>> +#include <dm.h>
>>>> +#include <errno.h>
>>>> +#include <nand.h>
>>>> +
>>>> +DECLARE_GLOBAL_DATA_PTR;
>>>> +
>>>> +#ifdef CONFIG_DM_NAND
>>>> +
>>>> +nand_info_t *get_nand_dev_by_index(int idx)
>>>> +{
>>>> +       nand_info_t *nand;
>>>> +       struct udevice *dev;
>>>> +       int ret;
>>>> +
>>>> +       ret = uclass_get_device(UCLASS_NAND, idx, &dev);
>>>> +       if (ret) {
>>>> +               error("NAND device (%d) not found\n", idx);
>>>
>>> This should return -ENODEV. Also please avoid printf() in drivers. The
>>> caller can report the error.
>>>
>>
>> Return type is pointer so returned NULL and NULL denotes no dev.
>> Will change this error to debug in v2.
> 
> OK, I see that this is the existing API.
> 
>>
>>>> +               return NULL;
>>>> +       }
>>>> +
>>>> +       nand = (nand_info_t *)dev_get_uclass_priv(dev);
>>>> +       if (!nand) {
>>>> +               error("Nand device not ready\n");
>>>> +               return NULL;
>>>> +       }
>>>
>>> But if the device has been probed ((with uclass_get_device()) then
>>> this cannot be NULL.
>>
>> This check is just a failsafe. ideally it should not fail here.
> 
> I cannot get to probe the device without this data, since this would
> be a bug in DM. So the check has no value.
> 

Okay then I will remove this check as it doesn't add value.

Regards
Mugunthan V N