[U-Boot] [PATCH] Add +size syntax to nand commands to pad lengths to page sizes.

[Scott Wood <scottwood@freescale.com>]

Josh Karabin wrote:
>> Why not support plussed for read as well?
> 
> "read" isn't strictly necessary, since the existing code permits lengths
> that result in page-unaligned reads.

Would be nice to keep the syntax consistent and not error if the user 
does provide a plus, though.

> Other operations are a little obvious.  "erase" implicitly extends the
> page size already if it needs to,

That should probably be changed to only do so if plus is used -- it's 
just as dangerous as implicitly rounding up with write (more so, since 
the blocks are bigger).

> "write" is the only operation that would ever need to depend on
> +{filesize} for which I could think of a non contrived example, so I
> stopped there. 

Erase is also quite likely to use {filesize} -- and I could see it being 
used for load as well.

> That said, would you prefer me to support plussed "read"
> or any of the other operations (erase, unlock)?

Yes.

>>> +			if (plussed) {
>>> +				int tailsize = size & (nand->writesize - 1);
>>> +				memset ((u_char *)addr + size, 0xff, nand->writesize - tailsize);
>>> +				size += nand->writesize - tailsize;
>> NACK, you cannot write to arbitrary memory beyond the end of the range
>> specified.  Allocate a buffer to hold the partial page.
> 
> OK.  I expected this, but I wanted to ask a question about it.
> 
> Are there actual callers for which this would cause a problem, or is
> this intended to make the code future proof?  I couldn't find any, so I
> assume that the objection is the latter?

The caller is the user typing a command, and while most of the time it 
would be harmless, it's unexpected behavior.

> Commands like "tftpboot" and
> "loadb" already stomp on memory without allocating it, so I wasn't sure
> why the nand commands should be handled differently.

Stomping on memory that the user asks to be stomped on is one thing -- 
stomping on a few extra bytes beyond the end of that region is another 
(and if those other commands do that, I'd rather not emulate them).

>>> +			}
>>> +		}
>> Please keep lines under 80 characters.
>>
> 
> Will do.  I noticed a few other lines in the file that hadn't, so
> figured it wasn't an enforced standard.  It will be corrected.

Sometimes it's deliberately overlooked because the wrapped version would 
be too awkward, but most of the time those long lines got in there by 
accident.

-Scott