[U-Boot] U-boot nand bug, read.part should fail

Scott Wood scottwood at freescale.com
Sat Feb 9 00:34:17 CET 2013


On 02/08/2013 10:44:30 AM, Harvey Chapman wrote:
> Eh, I shouldn't post code that quickly… Try this:
> 
> diff --git a/common/cmd_nand.c b/common/cmd_nand.c
> --- a/common/cmd_nand.c
> +++ b/common/cmd_nand.c
> @@ -621,60 +621,80 @@ int do_nand(cmd_tbl_t * cmdtp, int flag,
> 
>  		nand = &nand_info[dev];
> 
>  		s = strchr(cmd, '.');
> 
>  		if (s && !strcmp(s, ".raw")) {
>  			raw = 1;
> 
>  			if (arg_off(argv[3], &dev, &off, &size))
>  				return 1;
> 
>  			if (argc > 4 && !str2long(argv[4], &pagecount))  
> {
>  				printf("'%s' is not a number\n",  
> argv[4]);
>  				return 1;
>  			}
> 
>  			if (pagecount * nand->writesize > size) {
>  				puts("Size exceeds partition or device  
> limit\n");
>  				return -1;
>  			}
> 
>  			rwsize = pagecount * (nand->writesize +  
> nand->oobsize);
>  		} else {
>  			if (arg_off_size(argc - 3, argv + 3, &dev,
>  						&off, &size) != 0)
>  				return 1;
> 
>  			rwsize = size;
>  		}
> 
> +                /* If no size was given, it has been calculated for  
> us as
> +                 * the remainder of the chip or partition from  
> offset. Adjust
> +                 * down for bad blocks, if necessary.
> +                 */

This should go inside the "not raw" path of the previous "if" statement.

Please use tabs to indent.

> +                if (argc < 5) {
> +                        nand_info_t *nand = &nand_info[dev];

We already have "nand" in this context.

> +                        int size = rwsize;

We already have "size" -- and you don't even seem to use it.

> +                        int maxoffset = off + rwsize;
> +                        int offset = off;

Offsets do not fit in "int".  Use loff_t.

> +                        int badblocks = 0;
> +                        for (offset = off; offset < maxoffset;  
> offset += nand->erasesize)
> +                                if (nand_block_isbad(nand, offset))
> +                                        badblocks++;

Braces around multi-line "if" bodies (even if a single statement).

Please leave a blank line after the variable declaration block.

Maybe move this to its own function (having both "offset" and "off" is  
unpleasant, plus this function is way too big already).

We need this adjustment to be made to erase.part/chip as well.

-Scott


More information about the U-Boot mailing list