On Fri, Dec 18, 2009 at 12:49 AM, David Brownell <davi...@pacbell.net> wrote:
> Much to my surprise, I observed a "flash erase_address ..."
> command erasing data which I didn't say should be erased.
>
> The issue turns out to be generic NOR flash code which was
> silently, and rather dangerously, morphing partial-sector
> references into unrequested whole-sector ones.
>
> This patch removes that low-level morphing. If desired, it
> can and should be done in higher level code.
> ---
> I suspect there's one GDB server path which should do that
> morphing, but it looks like a later one already handles it.
> Other than that, does anyone object to fixing this?
>
> NEWS | 2 ++
> src/flash/nor/core.c | 44 ++++++++++++++++++++++++++++++++++++--------
> 2 files changed, 38 insertions(+), 8 deletions(-)
>
> --- a/NEWS
> +++ b/NEWS
> @@ -39,6 +39,8 @@ Flash Layer:
> - <bank_name>: reference the bank with its defined name
> - <driver_name>[.N]: reference the driver's Nth bank
> New 'nand verify' command to check bank against an image file.
> + The "flash erase_address" command now rejects partial sectors;
> + previously it would silently erase data you did not request.
>
> Board, Target, and Interface Configuration Scripts:
> ARM9
> --- a/src/flash/nor/core.c
> +++ b/src/flash/nor/core.c
> @@ -279,11 +279,13 @@ int default_flash_blank_check(struct fla
>
> return ERROR_OK;
> }
> +
> /* erase given flash region, selects proper bank according to target and
> address */
> static int flash_iterate_address_range(struct target *target, uint32_t addr,
> uint32_t length,
> int (*callback)(struct flash_bank *bank, int first, int last))
> {
> struct flash_bank *c;
> + uint32_t last_addr = addr + length; /* first address AFTER end */
> int first = -1;
> int last = -1;
> int i;
> @@ -306,26 +308,52 @@ static int flash_iterate_address_range(s
> return callback(c, 0, c->num_sectors - 1);
> }
>
> - /* check whether it fits */
> + /* check whether it all fits in this bank */
> if (addr + length - 1 > c->base + c->size - 1)
> return ERROR_FLASH_DST_BREAKS_ALIGNMENT;
>
> + /** @todo: handle erasures that cross into adjacent banks */
> +
> addr -= c->base;
>
> for (i = 0; i < c->num_sectors; i++)
> {
> - /* check whether sector overlaps with the given range and is
> not yet erased */
> - if (addr < c->sectors[i].offset + c->sectors[i].size && addr
> + length > c->sectors[i].offset && c->sectors[i].is_erased != 1) {
> - /* if first is not set yet then this is the first
> sector */
> - if (first == -1)
> + struct flash_sector *f = c->sectors + i;
> +
> + /* start only on a sector boundary */
> + if (first < 0) {
> + /* is this the first sector? */
> + if (addr == f->offset)
> first = i;
> - last = i; /* and it is the last one so far in any
> case */
> + else if (addr < f->offset)
> + break;
> + }
> +
> + /* is this (also?) the last sector? */
> + if (last_addr == f->offset + f->size) {
> + last = i;
> + break;
> }
> +
> + /* MUST finish on a sector boundary */
> + if (last_addr <= f->offset)
> + break;
> }
>
> - if (first == -1 || last == -1)
> - return ERROR_OK;
> + /* invalid start or end address? */
> + if (first == -1 || last == -1) {
> + LOG_ERROR("address range 0x%8.8x .. 0x%8.8x "
> + "is not sector-aligned",
> + (unsigned) c->base + addr,
> + (unsigned) last_addr - 1);
> + return ERROR_FLASH_DST_BREAKS_ALIGNMENT;
> + }
>
> + /* The NOR driver may trim this range down, based on
> + * whether or not a given sector is already erased.
> + *
> + * REVISIT should *we* trim it... ?
> + */
> return callback(c, first, last);
> }
>
> _______________________________________________
> Openocd-development mailing list
> Openocd-development@lists.berlios.de
> https://lists.berlios.de/mailman/listinfo/openocd-development
>
After applying the latest patches 0.4-rc1 I encountered the following error:
halt
flash write_image erase image.bin 0x1000 bin
auto erase enabled
address range 0x00001000 .. 0x000135d3 is not sector-aligned
Command handler execution failed
in procedure 'flash' called at file "command.c", line 637
called at file "command.c", line 352
my image has 75220 bytes. According to your patch
3f18900b19f49a84ba9df56f2e089c255e610011, only whole sectors are
allowed. As far as I understand if my image has only one section, it
will not be padded (see flash_write_unlock) or in general the last
section will not be padded. Due to the new constraint of whole sectors
the last section will be rejected if it has the "wrong" size. Do I see
it right?
Yegor
_______________________________________________
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
