* [U-Boot] [PATCH 0/9] device model bringup of nand on am335x gp evm and am437x gp evm
@ 2016-04-01 11:29 Mugunthan V N
2016-04-01 11:29 ` [U-Boot] [PATCH 1/9] include: nand: move endif to end of file Mugunthan V N
` (8 more replies)
0 siblings, 9 replies; 44+ messages in thread
From: Mugunthan V N @ 2016-04-01 11:29 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 1/9] include: nand: move endif to end of file
2016-04-01 11:29 [U-Boot] [PATCH 0/9] device model bringup of nand on am335x gp evm and am437x gp evm Mugunthan V N
@ 2016-04-01 11:29 ` Mugunthan V N
2016-04-01 12:51 ` Tom Rini
2016-04-20 14:39 ` Simon Glass
2016-04-01 11:29 ` [U-Boot] [PATCH 2/9] cmd: nand: abstract global variable usage for dm conversion Mugunthan V N
` (7 subsequent siblings)
8 siblings, 2 replies; 44+ messages in thread
From: Mugunthan V N @ 2016-04-01 11:29 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 2/9] cmd: nand: abstract global variable usage for dm conversion
2016-04-01 11:29 [U-Boot] [PATCH 0/9] device model bringup of nand on am335x gp evm and am437x gp evm Mugunthan V N
2016-04-01 11:29 ` [U-Boot] [PATCH 1/9] include: nand: move endif to end of file Mugunthan V N
@ 2016-04-01 11:29 ` Mugunthan V N
2016-04-01 12:51 ` Tom Rini
` (2 more replies)
2016-04-01 11:29 ` [U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option Mugunthan V N
` (6 subsequent siblings)
8 siblings, 3 replies; 44+ messages in thread
From: Mugunthan V N @ 2016-04-01 11:29 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option
2016-04-01 11:29 [U-Boot] [PATCH 0/9] device model bringup of nand on am335x gp evm and am437x gp evm Mugunthan V N
2016-04-01 11:29 ` [U-Boot] [PATCH 1/9] include: nand: move endif to end of file Mugunthan V N
2016-04-01 11:29 ` [U-Boot] [PATCH 2/9] cmd: nand: abstract global variable usage for dm conversion Mugunthan V N
@ 2016-04-01 11:29 ` Mugunthan V N
2016-04-01 12:51 ` Tom Rini
2016-04-01 11:29 ` [U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass Mugunthan V N
` (5 subsequent siblings)
8 siblings, 1 reply; 44+ messages in thread
From: Mugunthan V N @ 2016-04-01 11:29 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass
2016-04-01 11:29 [U-Boot] [PATCH 0/9] device model bringup of nand on am335x gp evm and am437x gp evm Mugunthan V N
` (2 preceding siblings ...)
2016-04-01 11:29 ` [U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option Mugunthan V N
@ 2016-04-01 11:29 ` Mugunthan V N
2016-04-01 12:51 ` Tom Rini
` (2 more replies)
2016-04-01 11:29 ` [U-Boot] [PATCH 5/9] drivers: nand: omap_gpmc: convert driver to adopt driver model Mugunthan V N
` (4 subsequent siblings)
8 siblings, 3 replies; 44+ messages in thread
From: Mugunthan V N @ 2016-04-01 11:29 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 5/9] drivers: nand: omap_gpmc: convert driver to adopt driver model
2016-04-01 11:29 [U-Boot] [PATCH 0/9] device model bringup of nand on am335x gp evm and am437x gp evm Mugunthan V N
` (3 preceding siblings ...)
2016-04-01 11:29 ` [U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass Mugunthan V N
@ 2016-04-01 11:29 ` Mugunthan V N
2016-04-01 12:51 ` Tom Rini
2016-04-04 17:22 ` Scott Wood
2016-04-01 11:29 ` [U-Boot] [PATCH 6/9] am43xx_evm: nand: do not define DM_NAND for spl Mugunthan V N
` (3 subsequent siblings)
8 siblings, 2 replies; 44+ messages in thread
From: Mugunthan V N @ 2016-04-01 11:29 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 6/9] am43xx_evm: nand: do not define DM_NAND for spl
2016-04-01 11:29 [U-Boot] [PATCH 0/9] device model bringup of nand on am335x gp evm and am437x gp evm Mugunthan V N
` (4 preceding siblings ...)
2016-04-01 11:29 ` [U-Boot] [PATCH 5/9] drivers: nand: omap_gpmc: convert driver to adopt driver model Mugunthan V N
@ 2016-04-01 11:29 ` Mugunthan V N
2016-04-01 12:51 ` Tom Rini
2016-04-20 14:39 ` Simon Glass
2016-04-01 11:29 ` [U-Boot] [PATCH 7/9] defconfig: am437x_gp_evm: enable NAND driver model Mugunthan V N
` (2 subsequent siblings)
8 siblings, 2 replies; 44+ messages in thread
From: Mugunthan V N @ 2016-04-01 11:29 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 7/9] defconfig: am437x_gp_evm: enable NAND driver model
2016-04-01 11:29 [U-Boot] [PATCH 0/9] device model bringup of nand on am335x gp evm and am437x gp evm Mugunthan V N
` (5 preceding siblings ...)
2016-04-01 11:29 ` [U-Boot] [PATCH 6/9] am43xx_evm: nand: do not define DM_NAND for spl Mugunthan V N
@ 2016-04-01 11:29 ` Mugunthan V N
2016-04-01 12:52 ` Tom Rini
2016-04-01 11:29 ` [U-Boot] [PATCH 8/9] am335x_evm: nand: do not define DM_NAND for spl Mugunthan V N
2016-04-01 11:29 ` [U-Boot] [PATCH 9/9] defconfig: am335x_gp_evm: enable NAND driver model Mugunthan V N
8 siblings, 1 reply; 44+ messages in thread
From: Mugunthan V N @ 2016-04-01 11:29 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 8/9] am335x_evm: nand: do not define DM_NAND for spl
2016-04-01 11:29 [U-Boot] [PATCH 0/9] device model bringup of nand on am335x gp evm and am437x gp evm Mugunthan V N
` (6 preceding siblings ...)
2016-04-01 11:29 ` [U-Boot] [PATCH 7/9] defconfig: am437x_gp_evm: enable NAND driver model Mugunthan V N
@ 2016-04-01 11:29 ` Mugunthan V N
2016-04-01 12:52 ` Tom Rini
2016-04-01 11:29 ` [U-Boot] [PATCH 9/9] defconfig: am335x_gp_evm: enable NAND driver model Mugunthan V N
8 siblings, 1 reply; 44+ messages in thread
From: Mugunthan V N @ 2016-04-01 11:29 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 9/9] defconfig: am335x_gp_evm: enable NAND driver model
2016-04-01 11:29 [U-Boot] [PATCH 0/9] device model bringup of nand on am335x gp evm and am437x gp evm Mugunthan V N
` (7 preceding siblings ...)
2016-04-01 11:29 ` [U-Boot] [PATCH 8/9] am335x_evm: nand: do not define DM_NAND for spl Mugunthan V N
@ 2016-04-01 11:29 ` Mugunthan V N
2016-04-20 14:39 ` Simon Glass
8 siblings, 1 reply; 44+ messages in thread
From: Mugunthan V N @ 2016-04-01 11:29 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 1/9] include: nand: move endif to end of file
2016-04-01 11:29 ` [U-Boot] [PATCH 1/9] include: nand: move endif to end of file Mugunthan V N
@ 2016-04-01 12:51 ` Tom Rini
2016-04-20 14:39 ` Simon Glass
1 sibling, 0 replies; 44+ messages in thread
From: Tom Rini @ 2016-04-01 12:51 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 2/9] cmd: nand: abstract global variable usage for dm conversion
2016-04-01 11:29 ` [U-Boot] [PATCH 2/9] cmd: nand: abstract global variable usage for dm conversion Mugunthan V N
@ 2016-04-01 12:51 ` Tom Rini
2016-04-01 22:57 ` Scott Wood
2016-04-14 11:50 ` Mugunthan V N
2 siblings, 0 replies; 44+ messages in thread
From: Tom Rini @ 2016-04-01 12:51 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option
2016-04-01 11:29 ` [U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option Mugunthan V N
@ 2016-04-01 12:51 ` Tom Rini
2016-04-01 14:57 ` Mugunthan V N
2016-04-01 23:07 ` Scott Wood
0 siblings, 2 replies; 44+ messages in thread
From: Tom Rini @ 2016-04-01 12:51 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass
2016-04-01 11:29 ` [U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass Mugunthan V N
@ 2016-04-01 12:51 ` Tom Rini
2016-04-01 14:59 ` Mugunthan V N
2016-04-01 23:25 ` Scott Wood
2016-04-20 14:39 ` Simon Glass
2 siblings, 1 reply; 44+ messages in thread
From: Tom Rini @ 2016-04-01 12:51 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 5/9] drivers: nand: omap_gpmc: convert driver to adopt driver model
2016-04-01 11:29 ` [U-Boot] [PATCH 5/9] drivers: nand: omap_gpmc: convert driver to adopt driver model Mugunthan V N
@ 2016-04-01 12:51 ` Tom Rini
2016-04-04 17:22 ` Scott Wood
1 sibling, 0 replies; 44+ messages in thread
From: Tom Rini @ 2016-04-01 12:51 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 6/9] am43xx_evm: nand: do not define DM_NAND for spl
2016-04-01 11:29 ` [U-Boot] [PATCH 6/9] am43xx_evm: nand: do not define DM_NAND for spl Mugunthan V N
@ 2016-04-01 12:51 ` Tom Rini
2016-04-20 14:39 ` Simon Glass
1 sibling, 0 replies; 44+ messages in thread
From: Tom Rini @ 2016-04-01 12:51 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 7/9] defconfig: am437x_gp_evm: enable NAND driver model
2016-04-01 11:29 ` [U-Boot] [PATCH 7/9] defconfig: am437x_gp_evm: enable NAND driver model Mugunthan V N
@ 2016-04-01 12:52 ` Tom Rini
2016-04-20 14:39 ` Simon Glass
0 siblings, 1 reply; 44+ messages in thread
From: Tom Rini @ 2016-04-01 12:52 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 8/9] am335x_evm: nand: do not define DM_NAND for spl
2016-04-01 11:29 ` [U-Boot] [PATCH 8/9] am335x_evm: nand: do not define DM_NAND for spl Mugunthan V N
@ 2016-04-01 12:52 ` Tom Rini
2016-04-20 14:39 ` Simon Glass
0 siblings, 1 reply; 44+ messages in thread
From: Tom Rini @ 2016-04-01 12:52 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option
2016-04-01 12:51 ` Tom Rini
@ 2016-04-01 14:57 ` Mugunthan V N
2016-04-01 23:07 ` Scott Wood
1 sibling, 0 replies; 44+ messages in thread
From: Mugunthan V N @ 2016-04-01 14:57 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass
2016-04-01 12:51 ` Tom Rini
@ 2016-04-01 14:59 ` Mugunthan V N
0 siblings, 0 replies; 44+ messages in thread
From: Mugunthan V N @ 2016-04-01 14:59 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 2/9] cmd: nand: abstract global variable usage for dm conversion
2016-04-01 11:29 ` [U-Boot] [PATCH 2/9] cmd: nand: abstract global variable usage for dm conversion Mugunthan V N
2016-04-01 12:51 ` Tom Rini
@ 2016-04-01 22:57 ` Scott Wood
2016-04-13 11:01 ` Mugunthan V N
2016-04-14 11:50 ` Mugunthan V N
2 siblings, 1 reply; 44+ messages in thread
From: Scott Wood @ 2016-04-01 22:57 UTC (permalink / raw)
To: u-boot
[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
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option
2016-04-01 12:51 ` Tom Rini
2016-04-01 14:57 ` Mugunthan V N
@ 2016-04-01 23:07 ` Scott Wood
2016-04-01 23:41 ` Tom Rini
1 sibling, 1 reply; 44+ messages in thread
From: Scott Wood @ 2016-04-01 23:07 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass
2016-04-01 11:29 ` [U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass Mugunthan V N
2016-04-01 12:51 ` Tom Rini
@ 2016-04-01 23:25 ` Scott Wood
2016-04-01 23:31 ` Scott Wood
2016-04-20 14:39 ` Simon Glass
2 siblings, 1 reply; 44+ messages in thread
From: Scott Wood @ 2016-04-01 23:25 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass
2016-04-01 23:25 ` Scott Wood
@ 2016-04-01 23:31 ` Scott Wood
2016-04-05 8:27 ` Mugunthan V N
0 siblings, 1 reply; 44+ messages in thread
From: Scott Wood @ 2016-04-01 23:31 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option
2016-04-01 23:07 ` Scott Wood
@ 2016-04-01 23:41 ` Tom Rini
2016-04-01 23:45 ` Scott Wood
0 siblings, 1 reply; 44+ messages in thread
From: Tom Rini @ 2016-04-01 23:41 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option
2016-04-01 23:41 ` Tom Rini
@ 2016-04-01 23:45 ` Scott Wood
2016-04-02 0:25 ` Tom Rini
0 siblings, 1 reply; 44+ messages in thread
From: Scott Wood @ 2016-04-01 23:45 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option
2016-04-01 23:45 ` Scott Wood
@ 2016-04-02 0:25 ` Tom Rini
2016-04-05 8:37 ` Mugunthan V N
0 siblings, 1 reply; 44+ messages in thread
From: Tom Rini @ 2016-04-02 0:25 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 5/9] drivers: nand: omap_gpmc: convert driver to adopt driver model
2016-04-01 11:29 ` [U-Boot] [PATCH 5/9] drivers: nand: omap_gpmc: convert driver to adopt driver model Mugunthan V N
2016-04-01 12:51 ` Tom Rini
@ 2016-04-04 17:22 ` Scott Wood
2016-04-05 8:41 ` Mugunthan V N
1 sibling, 1 reply; 44+ messages in thread
From: Scott Wood @ 2016-04-04 17:22 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass
2016-04-01 23:31 ` Scott Wood
@ 2016-04-05 8:27 ` Mugunthan V N
0 siblings, 0 replies; 44+ messages in thread
From: Mugunthan V N @ 2016-04-05 8:27 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option
2016-04-02 0:25 ` Tom Rini
@ 2016-04-05 8:37 ` Mugunthan V N
2016-04-08 19:45 ` Tom Rini
0 siblings, 1 reply; 44+ messages in thread
From: Mugunthan V N @ 2016-04-05 8:37 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 5/9] drivers: nand: omap_gpmc: convert driver to adopt driver model
2016-04-04 17:22 ` Scott Wood
@ 2016-04-05 8:41 ` Mugunthan V N
0 siblings, 0 replies; 44+ messages in thread
From: Mugunthan V N @ 2016-04-05 8:41 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option
2016-04-05 8:37 ` Mugunthan V N
@ 2016-04-08 19:45 ` Tom Rini
2016-04-17 1:38 ` Scott Wood
0 siblings, 1 reply; 44+ messages in thread
From: Tom Rini @ 2016-04-08 19:45 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 2/9] cmd: nand: abstract global variable usage for dm conversion
2016-04-01 22:57 ` Scott Wood
@ 2016-04-13 11:01 ` Mugunthan V N
0 siblings, 0 replies; 44+ messages in thread
From: Mugunthan V N @ 2016-04-13 11:01 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 2/9] cmd: nand: abstract global variable usage for dm conversion
2016-04-01 11:29 ` [U-Boot] [PATCH 2/9] cmd: nand: abstract global variable usage for dm conversion Mugunthan V N
2016-04-01 12:51 ` Tom Rini
2016-04-01 22:57 ` Scott Wood
@ 2016-04-14 11:50 ` Mugunthan V N
2 siblings, 0 replies; 44+ messages in thread
From: Mugunthan V N @ 2016-04-14 11:50 UTC (permalink / raw)
To: u-boot
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_ */
>
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option
2016-04-08 19:45 ` Tom Rini
@ 2016-04-17 1:38 ` Scott Wood
0 siblings, 0 replies; 44+ messages in thread
From: Scott Wood @ 2016-04-17 1:38 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 1/9] include: nand: move endif to end of file
2016-04-01 11:29 ` [U-Boot] [PATCH 1/9] include: nand: move endif to end of file Mugunthan V N
2016-04-01 12:51 ` Tom Rini
@ 2016-04-20 14:39 ` Simon Glass
1 sibling, 0 replies; 44+ messages in thread
From: Simon Glass @ 2016-04-20 14:39 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass
2016-04-01 11:29 ` [U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass Mugunthan V N
2016-04-01 12:51 ` Tom Rini
2016-04-01 23:25 ` Scott Wood
@ 2016-04-20 14:39 ` Simon Glass
2016-04-21 5:55 ` Mugunthan V N
2 siblings, 1 reply; 44+ messages in thread
From: Simon Glass @ 2016-04-20 14:39 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 6/9] am43xx_evm: nand: do not define DM_NAND for spl
2016-04-01 11:29 ` [U-Boot] [PATCH 6/9] am43xx_evm: nand: do not define DM_NAND for spl Mugunthan V N
2016-04-01 12:51 ` Tom Rini
@ 2016-04-20 14:39 ` Simon Glass
1 sibling, 0 replies; 44+ messages in thread
From: Simon Glass @ 2016-04-20 14:39 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 7/9] defconfig: am437x_gp_evm: enable NAND driver model
2016-04-01 12:52 ` Tom Rini
@ 2016-04-20 14:39 ` Simon Glass
0 siblings, 0 replies; 44+ messages in thread
From: Simon Glass @ 2016-04-20 14:39 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 8/9] am335x_evm: nand: do not define DM_NAND for spl
2016-04-01 12:52 ` Tom Rini
@ 2016-04-20 14:39 ` Simon Glass
0 siblings, 0 replies; 44+ messages in thread
From: Simon Glass @ 2016-04-20 14:39 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 9/9] defconfig: am335x_gp_evm: enable NAND driver model
2016-04-01 11:29 ` [U-Boot] [PATCH 9/9] defconfig: am335x_gp_evm: enable NAND driver model Mugunthan V N
@ 2016-04-20 14:39 ` Simon Glass
0 siblings, 0 replies; 44+ messages in thread
From: Simon Glass @ 2016-04-20 14:39 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass
2016-04-20 14:39 ` Simon Glass
@ 2016-04-21 5:55 ` Mugunthan V N
2016-04-21 14:11 ` Simon Glass
0 siblings, 1 reply; 44+ messages in thread
From: Mugunthan V N @ 2016-04-21 5:55 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass
2016-04-21 5:55 ` Mugunthan V N
@ 2016-04-21 14:11 ` Simon Glass
2016-04-22 5:06 ` Mugunthan V N
0 siblings, 1 reply; 44+ messages in thread
From: Simon Glass @ 2016-04-21 14:11 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* [U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass
2016-04-21 14:11 ` Simon Glass
@ 2016-04-22 5:06 ` Mugunthan V N
0 siblings, 0 replies; 44+ messages in thread
From: Mugunthan V N @ 2016-04-22 5:06 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 44+ messages in thread
end of thread, other threads:[~2016-04-22 5:06 UTC | newest]
Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-01 11:29 [U-Boot] [PATCH 0/9] device model bringup of nand on am335x gp evm and am437x gp evm Mugunthan V N
2016-04-01 11:29 ` [U-Boot] [PATCH 1/9] include: nand: move endif to end of file Mugunthan V N
2016-04-01 12:51 ` Tom Rini
2016-04-20 14:39 ` Simon Glass
2016-04-01 11:29 ` [U-Boot] [PATCH 2/9] cmd: nand: abstract global variable usage for dm conversion Mugunthan V N
2016-04-01 12:51 ` Tom Rini
2016-04-01 22:57 ` Scott Wood
2016-04-13 11:01 ` Mugunthan V N
2016-04-14 11:50 ` Mugunthan V N
2016-04-01 11:29 ` [U-Boot] [PATCH 3/9] drivers: nand: Kconfig: add NAND as Kconfig option Mugunthan V N
2016-04-01 12:51 ` Tom Rini
2016-04-01 14:57 ` Mugunthan V N
2016-04-01 23:07 ` Scott Wood
2016-04-01 23:41 ` Tom Rini
2016-04-01 23:45 ` Scott Wood
2016-04-02 0:25 ` Tom Rini
2016-04-05 8:37 ` Mugunthan V N
2016-04-08 19:45 ` Tom Rini
2016-04-17 1:38 ` Scott Wood
2016-04-01 11:29 ` [U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass Mugunthan V N
2016-04-01 12:51 ` Tom Rini
2016-04-01 14:59 ` Mugunthan V N
2016-04-01 23:25 ` Scott Wood
2016-04-01 23:31 ` Scott Wood
2016-04-05 8:27 ` Mugunthan V N
2016-04-20 14:39 ` Simon Glass
2016-04-21 5:55 ` Mugunthan V N
2016-04-21 14:11 ` Simon Glass
2016-04-22 5:06 ` Mugunthan V N
2016-04-01 11:29 ` [U-Boot] [PATCH 5/9] drivers: nand: omap_gpmc: convert driver to adopt driver model Mugunthan V N
2016-04-01 12:51 ` Tom Rini
2016-04-04 17:22 ` Scott Wood
2016-04-05 8:41 ` Mugunthan V N
2016-04-01 11:29 ` [U-Boot] [PATCH 6/9] am43xx_evm: nand: do not define DM_NAND for spl Mugunthan V N
2016-04-01 12:51 ` Tom Rini
2016-04-20 14:39 ` Simon Glass
2016-04-01 11:29 ` [U-Boot] [PATCH 7/9] defconfig: am437x_gp_evm: enable NAND driver model Mugunthan V N
2016-04-01 12:52 ` Tom Rini
2016-04-20 14:39 ` Simon Glass
2016-04-01 11:29 ` [U-Boot] [PATCH 8/9] am335x_evm: nand: do not define DM_NAND for spl Mugunthan V N
2016-04-01 12:52 ` Tom Rini
2016-04-20 14:39 ` Simon Glass
2016-04-01 11:29 ` [U-Boot] [PATCH 9/9] defconfig: am335x_gp_evm: enable NAND driver model Mugunthan V N
2016-04-20 14:39 ` Simon Glass
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.