[U-Boot] [PATCH v2 1/4] nand: Extend nand_(read|write)_skip_bad with *actual and limit parameters

[Tom Rini]

On Wed, Feb 27, 2013 at 12:17:07PM -0500, Tom Rini wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 02/27/2013 12:09 PM, Scott Wood wrote:
> > On 02/27/2013 10:49:55 AM, Tom Rini wrote:
> >> On Tue, Feb 26, 2013 at 10:56:08AM -0500, Tom Rini wrote:
> >> 
> >>> We make these two functions take a size_t pointer to how much
> >>> space was used on NAND to read or write the buffer (when
> >>> reads/writes happen) so that bad blocks can be accounted for.
> >>> We also make them take an loff_t limit on how much data can be
> >>> read or written.  This means that we can now catch the case of
> >>> when writing to a partition would exceed the partition size due
> >>> to bad blocks.  To do this we also need to make
> >> [snip]
> >>> int nand_read_skip_bad(nand_info_t *nand, loff_t offset,
> >>> size_t
> >> *length,
> >>> -               u_char *buffer) +               size_t *actual,
> >>> loff_t lim, u_char *buffer)
> >> [snip]
> >>> +    if (*actual > lim) { +        puts("Size of read exceeds
> >>> partition or device limit\n"); +        *length = 0; +
> >>> return -EFBIG; +    }
> >> 
> >> The more I look at this and try testing things, I think I
> >> shouldn't be introducing a change here.  Before you could do: 
> >> nand read ${address} partname-with-badblock
> >> 
> >> And it would suceed but bleed into the next partition if it
> >> wasn't the last one.  So your production system could do "nand
> >> read ${address} kernel" and be OK.  But with this change, it
> >> would fail because reading the whole partition is now too large
> >> with a bad block (you would need partition+(blocksize*number bad
> >> blocks).
> > 
> > You wouldn't be quite so OK if it were a write instead.
> 
> Correct.  But the check is now inside of nand_(read|write)_skip_bad.
> So the write case will fail (without trying to write), but the read
> case should not.
> 
> >> So I'm going to put this back to a check simply against requested
> >> size being greater than lim rather than required size greater
> >> than lim (since required size exceeds device is still caught).
> > 
> > No, please retain the check.  The other issue is a separate
> > (though related) bug, and there's a patch from Harvey Chapman to
> > deal with it.
> 
> I could be missing something, but I'm not sure how to otherwise adjust
> things here.  With all of the checking moved to nand_util.c and
> check_skip_len not knowing if we're a read or write it can only say
> "fits using X" or "exceeds device", then it's up to the caller to
> decide the next action.

OK, I see it now.  Yes, I think they should be able to live together
nicely.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130227/90d6f19c/attachment.pgp>