diff mbox

[U-Boot,v2,02/15] cmd: nand: abstract global variable usage for dm conversion

Message ID 20170131213717.30745-3-grygorii.strashko@ti.com
State Changes Requested
Delegated to: Tom Rini
Headers show

Commit Message

Grygorii Strashko Jan. 31, 2017, 9:37 p.m. UTC
From: Mugunthan V N <mugunthanvnm@ti.com>

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>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 cmd/nand.c                   | 69 +++++++++++++++++++++++++-------------------
 drivers/mtd/nand/nand.c      | 23 +++++++++++----
 drivers/mtd/nand/omap_gpmc.c |  7 ++---
 include/nand.h               |  9 ++++++
 4 files changed, 67 insertions(+), 41 deletions(-)

Comments

Tom Rini Feb. 6, 2017, 1:57 a.m. UTC | #1
On Tue, Jan 31, 2017 at 03:37:04PM -0600, Grygorii Strashko wrote:

> From: Mugunthan V N <mugunthanvnm@ti.com>
> 
> 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>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

Reviewed-by: Tom Rini <trini@konsulko.com>
Simon Glass Feb. 6, 2017, 3:33 p.m. UTC | #2
On 31 January 2017 at 13:37, Grygorii Strashko <grygorii.strashko@ti.com> wrote:
> From: Mugunthan V N <mugunthanvnm@ti.com>
>
> 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>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  cmd/nand.c                   | 69 +++++++++++++++++++++++++-------------------
>  drivers/mtd/nand/nand.c      | 23 +++++++++++----
>  drivers/mtd/nand/omap_gpmc.c |  7 ++---
>  include/nand.h               |  9 ++++++
>  4 files changed, 67 insertions(+), 41 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>
Tom Rini Feb. 8, 2017, 9:23 p.m. UTC | #3
On Tue, Jan 31, 2017 at 03:37:04PM -0600, Grygorii Strashko wrote:

> From: Mugunthan V N <mugunthanvnm@ti.com>
> 
> 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>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

So this is incomplete and breaks omap3_beagle booting for example, we
hang during bootup.
Grygorii Strashko Feb. 9, 2017, 5:51 p.m. UTC | #4
Hi Tom,

On 02/05/2017 07:57 PM, Tom Rini wrote:
> On Tue, Jan 31, 2017 at 03:37:04PM -0600, Grygorii Strashko wrote:
> 
>> From: Mugunthan V N <mugunthanvnm@ti.com>
>>
>> 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>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> 
> Reviewed-by: Tom Rini <trini@konsulko.com>
> 

Sry, I've not received your last comment:
"So this is incomplete and breaks omap3_beagle booting for example, we
hang during bootup."

I have one question - was boot tested only with this patch or with whole series?
Tom Rini Feb. 9, 2017, 6:12 p.m. UTC | #5
On Thu, Feb 09, 2017 at 11:51:38AM -0600, Grygorii Strashko wrote:
> Hi Tom,
> 
> On 02/05/2017 07:57 PM, Tom Rini wrote:
> > On Tue, Jan 31, 2017 at 03:37:04PM -0600, Grygorii Strashko wrote:
> > 
> >> From: Mugunthan V N <mugunthanvnm@ti.com>
> >>
> >> 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>
> >> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> > 
> > Reviewed-by: Tom Rini <trini@konsulko.com>
> > 
> 
> Sry, I've not received your last comment:
> "So this is incomplete and breaks omap3_beagle booting for example, we
> hang during bootup."
> 
> I have one question - was boot tested only with this patch or with whole series?

I tested with up until the TI driver was converted itself.  So... if
this series requires all drivers to have been converted in order to not
hang, then you need to convert all of the drivers to use the helper to
not break bisectability.
diff mbox

Patch

diff --git a/cmd/nand.c b/cmd/nand.c
index c16ec77..f2b440e 100644
--- a/cmd/nand.c
+++ b/cmd/nand.c
@@ -115,20 +115,20 @@  free_dat:
 
 static int set_dev(int dev)
 {
-	if (dev < 0 || dev >= CONFIG_SYS_MAX_NAND_DEVICE || !nand_info[dev]) {
-		puts("No such device\n");
-		return -1;
-	}
+	struct mtd_info *mtd = get_nand_dev_by_index(dev);
+
+	if (!mtd)
+		return -ENODEV;
 
 	if (nand_curr_device == dev)
 		return 0;
 
-	printf("Device %d: %s", dev, nand_info[dev]->name);
+	printf("Device %d: %s", dev, mtd->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(mtd->priv, dev);
 #endif
 
 	return 0;
@@ -188,7 +188,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)];
-	struct mtd_info *mtd = nand_info[0];
+	struct mtd_info *mtd = get_nand_dev_by_index(0);
 	char *cmd = argv[1];
 
 	if (CONFIG_SYS_MAX_NAND_DEVICE == 0 || !mtd) {
@@ -213,9 +213,10 @@  int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[])
 		if (argc < 3)
 			goto usage;
 
+		mtd = 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, mtd->size)) {
 			puts("Offset or partition name expected\n");
 			return 1;
 		}
@@ -283,9 +284,14 @@  usage:
 
 static void nand_print_and_set_info(int idx)
 {
-	struct mtd_info *mtd = nand_info[idx];
-	struct nand_chip *chip = mtd_to_nand(mtd);
+	struct mtd_info *mtd;
+	struct nand_chip *chip;
+
+	mtd = get_nand_dev_by_index(idx);
+	if (!mtd)
+		return;
 
+	chip = mtd_to_nand(mtd);
 	printf("Device %d: ", idx);
 	if (chip->numchips > 1)
 		printf("%dx ", chip->numchips);
@@ -348,7 +354,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.
 	 */
-	struct mtd_info *mtd = nand_info[dev];
+	struct mtd_info *mtd = get_nand_dev_by_index(dev);
 	loff_t maxoffset = offset + *size;
 	int badblocks = 0;
 
@@ -397,10 +403,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])
-				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 +436,11 @@  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]) {
+	mtd = get_nand_dev_by_index(dev);
+	if (!mtd) {
 		puts("\nno devices available\n");
 		return 1;
 	}
-	mtd = nand_info[dev];
 
 	if (strcmp(cmd, "bad") == 0) {
 		printf("\nDevice %d bad blocks:\n", dev);
@@ -496,13 +499,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)
+				     mtd->size) != 0)
 			return 1;
 
 		if (set_dev(dev))
 			return 1;
 
-		mtd = nand_info[dev];
+		mtd = get_nand_dev_by_index(dev);
 
 		memset(&opts, 0, sizeof(opts));
 		opts.offset = off;
@@ -565,13 +568,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))
+					mtd->size))
 				return 1;
 
 			if (set_dev(dev))
 				return 1;
 
-			mtd = nand_info[dev];
+			mtd = get_nand_dev_by_index(dev);
 
 			if (argc > 4 && !str2long(argv[4], &pagecount)) {
 				printf("'%s' is not a number\n", argv[4]);
@@ -588,7 +591,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)
+					     mtd->size) != 0)
 				return 1;
 
 			if (set_dev(dev))
@@ -600,7 +603,7 @@  static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			rwsize = size;
 		}
 
-		mtd = nand_info[dev];
+		mtd = get_nand_dev_by_index(dev);
 
 		if (!s || !strcmp(s, ".jffs2") ||
 		    !strcmp(s, ".e") || !strcmp(s, ".i")) {
@@ -760,13 +763,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)
+				     mtd->size) < 0)
 			return 1;
 
 		if (set_dev(dev))
 			return 1;
 
-		if (!nand_unlock(nand_info[dev], off, size, allexcept)) {
+		mtd = get_nand_dev_by_index(dev);
+
+		if (!nand_unlock(mtd, off, size, allexcept)) {
 			puts("NAND flash successfully unlocked\n");
 		} else {
 			puts("Error unlocking NAND flash, "
@@ -929,6 +934,7 @@  static int do_nandboot(cmd_tbl_t *cmdtp, int flag, int argc,
 	char *boot_device = NULL;
 	int idx;
 	ulong addr, offset = 0;
+	struct mtd_info *mtd;
 #if defined(CONFIG_CMD_MTDPARTS)
 	struct mtd_device *dev;
 	struct part_info *part;
@@ -948,8 +954,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]);
+
+			mtd = get_nand_dev_by_index(dev->id->num);
+			return nand_load_image(cmdtp, mtd, part->offset,
+					       addr, argv[0]);
 		}
 	}
 #endif
@@ -991,14 +999,15 @@  usage:
 
 	idx = simple_strtoul(boot_device, NULL, 16);
 
-	if (idx < 0 || idx >= CONFIG_SYS_MAX_NAND_DEVICE || !nand_info[idx]) {
+	mtd = get_nand_dev_by_index(idx);
+	if (!mtd) {
 		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, mtd, 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 0551241..18c346a 100644
--- a/drivers/mtd/nand/nand.c
+++ b/drivers/mtd/nand/nand.c
@@ -19,7 +19,6 @@  DECLARE_GLOBAL_DATA_PTR;
 
 int nand_curr_device = -1;
 
-
 struct mtd_info *nand_info[CONFIG_SYS_MAX_NAND_DEVICE];
 
 #ifndef CONFIG_SYS_NAND_SELF_INIT
@@ -31,12 +30,23 @@  static char dev_name[CONFIG_SYS_MAX_NAND_DEVICE][8];
 
 static unsigned long total_nand_size; /* in kiB */
 
+struct mtd_info *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];
+}
+
 int nand_mtd_to_devnum(struct mtd_info *mtd)
 {
 	int i;
 
-	for (i = 0; i < ARRAY_SIZE(nand_info); i++) {
-		if (mtd && nand_info[i] == mtd)
+	for (i = 0; i < CONFIG_SYS_MAX_NAND_DEVICE; i++) {
+		if (mtd && get_nand_dev_by_index(i) == mtd)
 			return i;
 	}
 
@@ -101,8 +111,9 @@  static void create_mtd_concat(void)
 	int i;
 
 	for (i = 0; i < CONFIG_SYS_MAX_NAND_DEVICE; i++) {
-		if (nand_info[i] != NULL) {
-			nand_info_list[nand_devices_found] = nand_info[i];
+		struct mtd_info *mtd = get_nand_dev_by_index(i);
+		if (mtd != NULL) {
+			nand_info_list[nand_devices_found] = mtd;
 			nand_devices_found++;
 		}
 	}
@@ -148,7 +159,7 @@  void nand_init(void)
 	/*
 	 * Select the chip in the board/cpu specific driver
 	 */
-	board_nand_select_device(mtd_to_nand(nand_info[nand_curr_device]),
+	board_nand_select_device(mtd_to_nand(get_nand_dev_by_index(nand_curr_device)),
 				 nand_curr_device);
 #endif
 
diff --git a/drivers/mtd/nand/omap_gpmc.c b/drivers/mtd/nand/omap_gpmc.c
index f4f0de3..b540bc3 100644
--- a/drivers/mtd/nand/omap_gpmc.c
+++ b/drivers/mtd/nand/omap_gpmc.c
@@ -894,17 +894,14 @@  static int omap_select_ecc_scheme(struct nand_chip *nand,
 int __maybe_unused omap_nand_switch_ecc(uint32_t hardware, uint32_t eccstrength)
 {
 	struct nand_chip *nand;
-	struct mtd_info *mtd;
+	struct mtd_info *mtd = get_nand_dev_by_index(nand_curr_device);
 	int err = 0;
 
-	if (nand_curr_device < 0 ||
-	    nand_curr_device >= CONFIG_SYS_MAX_NAND_DEVICE ||
-	    !nand_info[nand_curr_device]) {
+	if (!mtd) {
 		printf("nand: error: no NAND devices found\n");
 		return -ENODEV;
 	}
 
-	mtd = nand_info[nand_curr_device];
 	nand = mtd_to_nand(mtd);
 	nand->options |= NAND_OWN_BUFFERS;
 	nand->options &= ~NAND_SUBPAGE_READ;
diff --git a/include/nand.h b/include/nand.h
index b6eb223..6c785a0 100644
--- a/include/nand.h
+++ b/include/nand.h
@@ -145,3 +145,12 @@  int spl_nand_erase_one(int block, int page);
 
 /* platform specific init functions */
 void sunxi_nand_init(void);
+
+/*
+ * 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.
+ */
+struct mtd_info *get_nand_dev_by_index(int dev);