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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
