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

Mugunthan V N mugunthanvnm at ti.com
Thu Apr 14 13:50:12 CEST 2016


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



More information about the U-Boot mailing list