| Message ID | 20171117000245.26400-1-cyril.bur@au1.ibm.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | external/pflash: Fix erasing within a single erase block | expand |
Cyril Bur <cyril.bur@au1.ibm.com> writes: > It is possible to erase within a single erase block. Currently the > pflash code assumes that if the erase starts part way into an erase > block it is because it needs to be aligned up to the boundary with the > next erase block. > > Doing an erase smaller than a single erase block will cause underflows > and looping forever on erase. > > Fixes: ae6cb86c2 ("external/pflash: Reinstate the progress bars") > Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> > --- > Side note, this bug is only possible on 64K erase granule platforms. > Also, this bug cannot be caught by our test suite as it stands because > the test suite tests with the file backend to blocklevel and it has an > erase granule of one. I'll have a think about perhaps adding changes so > that we might be able to catch this. Hrm..... Something we could perhaps try to test with op-test in the interim? Merged to master as of ba540e0be90f1055fd9d40b0a6858f96aff6b180
diff --git a/external/pflash/pflash.c b/external/pflash/pflash.c index d6b2b8ec..381df24f 100644 --- a/external/pflash/pflash.c +++ b/external/pflash/pflash.c @@ -325,16 +325,20 @@ static int erase_range(struct flash_details *flash, */ progress_init(size); if (start & erase_mask) { - /* Align to next erase block */ - rc = blocklevel_smart_erase(flash->bl, start, - flash->erase_granule - (start & erase_mask)); + /* + * Align to next erase block, or just do the entire + * thing if we fit within one erase block + */ + uint32_t first_size = MIN(size, (flash->erase_granule - (start & erase_mask))); + + rc = blocklevel_smart_erase(flash->bl, start, first_size); if (rc) { fprintf(stderr, "Failed to blocklevel_smart_erase(): %d\n", rc); return 1; } - size -= flash->erase_granule - (start & erase_mask); - done = flash->erase_granule - (start & erase_mask); - start += flash->erase_granule - (start & erase_mask); + size -= first_size; + done = first_size; + start += first_size; } progress_tick(done); while (size & ~(erase_mask)) { diff --git a/libflash/libflash.c b/libflash/libflash.c index bf2e58e3..ad25b613 100644 --- a/libflash/libflash.c +++ b/libflash/libflash.c @@ -23,10 +23,6 @@ #include "ecc.h" #include "blocklevel.h" -#ifndef MIN -#define MIN(a, b) ((a) < (b) ? (a) : (b)) -#endif - static const struct flash_info flash_info[] = { { 0xc22018, 0x01000000, FL_ERASE_ALL | FL_CAN_4B, "Macronix MXxxL12835F"}, { 0xc22019, 0x02000000, FL_ERASE_ALL | FL_CAN_4B, "Macronix MXxxL25635F"}, diff --git a/libflash/libflash.h b/libflash/libflash.h index ff3a9823..01b4d605 100644 --- a/libflash/libflash.h +++ b/libflash/libflash.h @@ -28,6 +28,10 @@ */ #include <libflash/errors.h> +#ifndef MIN +#define MIN(a, b) ((a) < (b) ? (a) : (b)) +#endif + /* Flash chip, opaque */ struct flash_chip; struct spi_flash_ctrl;
It is possible to erase within a single erase block. Currently the pflash code assumes that if the erase starts part way into an erase block it is because it needs to be aligned up to the boundary with the next erase block. Doing an erase smaller than a single erase block will cause underflows and looping forever on erase. Fixes: ae6cb86c2 ("external/pflash: Reinstate the progress bars") Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> --- Side note, this bug is only possible on 64K erase granule platforms. Also, this bug cannot be caught by our test suite as it stands because the test suite tests with the file backend to blocklevel and it has an erase granule of one. I'll have a think about perhaps adding changes so that we might be able to catch this. --- external/pflash/pflash.c | 16 ++++++++++------ libflash/libflash.c | 4 ---- libflash/libflash.h | 4 ++++ 3 files changed, 14 insertions(+), 10 deletions(-)