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

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_ */