From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mugunthan V N <mugunthanvnm@ti.com>
Date: Wed, 13 Apr 2016 16:31:53 +0530
Subject: [U-Boot] [PATCH 2/9] cmd: nand: abstract global variable usage
 for dm conversion
In-Reply-To: <1459551426.32510.36.camel@buserror.net>
References: <1459510190-26306-1-git-send-email-mugunthanvnm@ti.com>
 <1459510190-26306-3-git-send-email-mugunthanvnm@ti.com>
 <1459551426.32510.36.camel@buserror.net>
Message-ID: <570E2721.5080007@ti.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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