| Message ID | 20170913065946.11262-1-sjitindarsingh@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | pflash: Fix erase command for unaligned start address | expand |
On Wed, 2017-09-13 at 16:59 +1000, Suraj Jitindar Singh wrote: > The erase_range() function handles erasing the flash for a given start > address and length, and can handle an unaligned start address and > length. However in the unaligned start address case we are incorrectly > calculating the remaining size which can lead to incomplete erases. > > If we're going to update the remaining size based on what the start > address was then we probably want to do that before we overide the > origin start address. So rearrange the code so that this is indeed the > case. > > Reported-by: Pridhiviraj Paidipeddi <ppaidipe@in.ibm.com> > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com> Good find. Stewart: I know you usually do, but just to be sure, if you could respond with the merged sha1 of this so that I'll remember to add a testcase for this, I'm surprised this wasn't caught in testing. Reviewed-by: Cyril Bur <cyril.bur@au1.ibm.com> > --- > external/pflash/pflash.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/external/pflash/pflash.c b/external/pflash/pflash.c > index bfc975f..5b1be3c 100644 > --- a/external/pflash/pflash.c > +++ b/external/pflash/pflash.c > @@ -331,9 +331,9 @@ static int erase_range(struct flash_details *flash, > fprintf(stderr, "Failed to blocklevel_smart_erase(): %d\n", rc); > return 1; > } > - start += flash->erase_granule - (start & erase_mask); > size -= flash->erase_granule - (start & erase_mask); > done = flash->erase_granule - (start & erase_mask); > + start += flash->erase_granule - (start & erase_mask); > } > progress_tick(done); > while (size & ~(erase_mask)) {
Cyril Bur <cyrilbur@gmail.com> writes: > On Wed, 2017-09-13 at 16:59 +1000, Suraj Jitindar Singh wrote: >> The erase_range() function handles erasing the flash for a given start >> address and length, and can handle an unaligned start address and >> length. However in the unaligned start address case we are incorrectly >> calculating the remaining size which can lead to incomplete erases. >> >> If we're going to update the remaining size based on what the start >> address was then we probably want to do that before we overide the >> origin start address. So rearrange the code so that this is indeed the >> case. >> >> Reported-by: Pridhiviraj Paidipeddi <ppaidipe@in.ibm.com> >> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com> > > Good find. > Stewart: I know you usually do, but just to be sure, if you could > respond with the merged sha1 of this so that I'll remember to add a > testcase for this, I'm surprised this wasn't caught in testing. Merged as of aefd3ffce6ed8f586290c0044446024f6e6fdcd5 Moar tests always welcomed. :)
diff --git a/external/pflash/pflash.c b/external/pflash/pflash.c index bfc975f..5b1be3c 100644 --- a/external/pflash/pflash.c +++ b/external/pflash/pflash.c @@ -331,9 +331,9 @@ static int erase_range(struct flash_details *flash, fprintf(stderr, "Failed to blocklevel_smart_erase(): %d\n", rc); return 1; } - start += flash->erase_granule - (start & erase_mask); size -= flash->erase_granule - (start & erase_mask); done = flash->erase_granule - (start & erase_mask); + start += flash->erase_granule - (start & erase_mask); } progress_tick(done); while (size & ~(erase_mask)) {
The erase_range() function handles erasing the flash for a given start address and length, and can handle an unaligned start address and length. However in the unaligned start address case we are incorrectly calculating the remaining size which can lead to incomplete erases. If we're going to update the remaining size based on what the start address was then we probably want to do that before we overide the origin start address. So rearrange the code so that this is indeed the case. Reported-by: Pridhiviraj Paidipeddi <ppaidipe@in.ibm.com> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com> --- external/pflash/pflash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)