From: Scott Wood <scottwood@freescale.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] U-boot nand bug, read.part should fail
Date: Fri, 8 Feb 2013 17:34:17 -0600 [thread overview]
Message-ID: <1360366457.22064.12@snotra> (raw)
In-Reply-To: <AE2124FA-64A9-49A4-A02A-ECCAB85BBA6F@3gfp.com> (from hchapman-uboot@3gfp.com on Fri Feb 8 10:44:30 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
next prev parent reply other threads:[~2013-02-08 23:34 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1360274360.27002.10@snotra>
2013-02-07 22:13 ` [U-Boot] U-boot nand bug, read.part should fail Harvey Chapman
2013-02-07 22:22 ` Scott Wood
2013-02-08 15:48 ` Harvey Chapman
2013-02-08 16:44 ` Harvey Chapman
2013-02-08 23:34 ` Scott Wood [this message]
2013-02-09 1:02 ` Harvey Chapman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1360366457.22064.12@snotra \
--to=scottwood@freescale.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox