X-Git-Url: https://review.openocd.org/gitweb?p=openocd.git;a=blobdiff_plain;f=src%2Fflash%2Fnor%2Fcore.c;h=18012c650b55d0f03603e24f9094bbe6dd12b6a8;hp=9083ed15ef92a95dda02aa4f0d9362922c2b4905;hb=300f0f53c54fab5dd994a8bc42edbdc6115ef7b9;hpb=8c730aaee22a505cf66666be3ff28734ac885418

diff --git a/src/flash/nor/core.c b/src/flash/nor/core.c
index 9083ed15ef..18012c650b 100644
--- a/src/flash/nor/core.c
+++ b/src/flash/nor/core.c
@@ -36,7 +36,7 @@
  * primarily support access from Tcl scripts or from GDB.
  */
 
-struct flash_bank *flash_banks;
+static struct flash_bank *flash_banks;
 
 int flash_driver_erase(struct flash_bank *bank, int first, int last)
 {
@@ -54,6 +54,63 @@ int flash_driver_erase(struct flash_bank *bank, int first, int last)
 int flash_driver_protect(struct flash_bank *bank, int set, int first, int last)
 {
 	int retval;
+	bool updated = false;
+
+	/* NOTE: "first == last" means protect just that sector */
+
+	/* callers may not supply illegal parameters ... */
+	if (first < 0 || first > last || last >= bank->num_sectors)
+		return ERROR_FAIL;
+
+	/* force "set" to 0/1 */
+	set = !!set;
+
+	/*
+	 * Filter out what trivial nonsense we can, so drivers don't have to.
+	 *
+	 * Don't tell drivers to change to the current state...  it's needless,
+	 * and reducing the amount of work to be done (potentially to nothing)
+	 * speeds at least some things up.
+	 */
+scan:
+	for (int i = first; i <= last; i++) {
+		struct flash_sector *sector = bank->sectors + i;
+
+		/* Only filter requests to protect the already-protected, or
+		 * to unprotect the already-unprotected.  Changing from the
+		 * unknown state (-1) to a known one is unwise but allowed;
+		 * protection status is best checked first.
+		 */
+		if (sector->is_protected != set)
+			continue;
+
+		/* Shrink this range of sectors from the start; don't overrun
+		 * the end.  Also shrink from the end; don't overun the start.
+		 *
+		 * REVISIT we could handle discontiguous regions by issuing
+		 * more than one driver request.  How much would that matter?
+		 */
+		if (i == first) {
+			updated = true;
+			first++;
+		} else if (i == last) {
+			updated = true;
+			last--;
+		}
+	}
+
+	/* updating the range affects the tests in the scan loop above; so
+	 * re-scan, to make sure we didn't miss anything.
+	 */
+	if (updated) {
+		updated = false;
+		goto scan;
+	}
+
+	/* Single sector, already protected?  Nothing to do! */
+	if (first > last)
+		return ERROR_OK;
+
 
 	retval = bank->driver->protect(bank, set, first, last);
 	if (retval != ERROR_OK)
@@ -287,9 +344,22 @@ int default_flash_blank_check(struct flash_bank *bank)
 	return ERROR_OK;
 }
 
-/* erase given flash region, selects proper bank according to target and address */
+/* Manipulate given flash region, selecting the bank according to target
+ * and address.  Maps an address range to a set of sectors, and issues
+ * the callback() on that set ... e.g. to erase or unprotect its members.
+ *
+ * (Note a current bad assumption:  that protection operates on the same
+ * size sectors as erase operations use.)
+ *
+ * The "pad_reason" parameter is a kind of boolean:  when it's NULL, the
+ * range must fit those sectors exactly.  This is clearly safe; it can't
+ * erase data which the caller said to leave alone, for example.  If it's
+ * non-NULL, rather than failing, extra data in the first and/or last
+ * sectors will be added to the range, and that reason string is used when
+ * warning about those additions.
+ */
 static int flash_iterate_address_range(struct target *target,
-		uint32_t addr, uint32_t length,
+		char *pad_reason, uint32_t addr, uint32_t length,
 		int (*callback)(struct flash_bank *bank, int first, int last))
 {
 	struct flash_bank *c;
@@ -311,14 +381,20 @@ static int flash_iterate_address_range(struct target *target,
 	{
 		/* special case, erase whole bank when length is zero */
 		if (addr != c->base)
+		{
+			LOG_ERROR("Whole bank access must start at beginning of bank.");
 			return ERROR_FLASH_DST_BREAKS_ALIGNMENT;
+		}
 
 		return callback(c, 0, c->num_sectors - 1);
 	}
 
 	/* check whether it all fits in this bank */
 	if (addr + length - 1 > c->base + c->size - 1)
+	{
+		LOG_ERROR("Flash access does not fit into bank.");
 		return ERROR_FLASH_DST_BREAKS_ALIGNMENT;
+	}
 
 	/** @todo: handle erasures that cross into adjacent banks */
 
@@ -328,18 +404,53 @@ static int flash_iterate_address_range(struct target *target,
 	for (i = 0; i < c->num_sectors; i++)
 	{
 		struct flash_sector *f = c->sectors + i;
+		uint32_t end = f->offset + f->size;
 
 		/* start only on a sector boundary */
 		if (first < 0) {
+			/* scanned past the first sector? */
+			if (addr < f->offset)
+				break;
+
 			/* is this the first sector? */
 			if (addr == f->offset)
 				first = i;
-			else if (addr < f->offset)
-				break;
+
+			/* Does this need head-padding?  If so, pad and warn;
+			 * or else force an error.
+			 *
+			 * Such padding can make trouble, since *WE* can't
+			 * ever know if that data was in use.  The warning
+			 * should help users sort out messes later.
+			 */
+			else if (addr < end && pad_reason) {
+				/* FIXME say how many bytes (e.g. 80 KB) */
+				LOG_WARNING("Adding extra %s range, "
+						"%#8.8x to %#8.8x",
+					pad_reason,
+					(unsigned) f->offset,
+					(unsigned) addr - 1);
+				first = i;
+			} else
+				continue;
 		}
 
 		/* is this (also?) the last sector? */
-		if (last_addr == f->offset + f->size) {
+		if (last_addr == end) {
+			last = i;
+			break;
+		}
+
+		/* Does this need tail-padding?  If so, pad and warn;
+		 * or else force an error.
+		 */
+		if (last_addr < end && pad_reason) {
+			/* FIXME say how many bytes (e.g. 80 KB) */
+			LOG_WARNING("Adding extra %s range, "
+					"%#8.8x to %#8.8x",
+				pad_reason,
+				(unsigned) last_addr,
+				(unsigned) end - 1);
 			last = i;
 			break;
 		}
@@ -354,22 +465,21 @@ static int flash_iterate_address_range(struct target *target,
 		LOG_ERROR("address range 0x%8.8x .. 0x%8.8x "
 				"is not sector-aligned",
 				(unsigned) (c->base + addr),
-				(unsigned) (last_addr - 1));
+				(unsigned) (c->base + 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... ?
+	/* The NOR driver may trim this range down, based on what
+	 * sectors are already erased/unprotected.  GDB currently
+	 * blocks such optimizations.
 	 */
 	return callback(c, first, last);
 }
 
 int flash_erase_address_range(struct target *target,
-		uint32_t addr, uint32_t length)
+		bool pad, uint32_t addr, uint32_t length)
 {
-	return flash_iterate_address_range(target,
+	return flash_iterate_address_range(target, pad ? "erase" : NULL,
 			addr, length, &flash_driver_erase);
 }
 
@@ -380,7 +490,11 @@ static int flash_driver_unprotect(struct flash_bank *bank, int first, int last)
 
 static int flash_unlock_address_range(struct target *target, uint32_t addr, uint32_t length)
 {
-	return flash_iterate_address_range(target,
+	/* By default, pad to sector boundaries ... the real issue here
+	 * is that our (only) caller *permanently* removes protection,
+	 * and doesn't restore it.
+	 */
+	return flash_iterate_address_range(target, "unprotect",
 			addr, length, &flash_driver_unprotect);
 }
 
@@ -394,6 +508,12 @@ int flash_write_unlock(struct target *target, struct image *image,
 	struct flash_bank *c;
 	int *padding;
 
+	/* REVISIT do_pad should perhaps just be another parameter.
+	 * GDB wouldn't ever need it, since it erases separately.
+	 * But "flash write_image" commands might want that option.
+	 */
+	bool do_pad = false;
+
 	section = 0;
 	section_offset = 0;
 
@@ -470,7 +590,8 @@ int flash_write_unlock(struct target *target, struct image *image,
 			 * In both cases, the extra writes slow things down.
 			 */
 
-			/* if we have multiple sections within our image, flash programming could fail due to alignment issues
+			/* if we have multiple sections within our image,
+			 * flash programming could fail due to alignment issues
 			 * attempt to rebuild a consecutive buffer for the flash loader */
 			pad_bytes = (image->sections[section_last + 1].base_address) - (run_address + run_size);
 			if ((run_address + run_size + pad_bytes) > (c->base + c->size))
@@ -560,7 +681,8 @@ int flash_write_unlock(struct target *target, struct image *image,
 			if (erase)
 			{
 				/* calculate and erase sectors */
-				retval = flash_erase_address_range(target, run_address, run_size);
+				retval = flash_erase_address_range(target,
+						do_pad, run_address, run_size);
 			}
 		}
 
@@ -592,3 +714,34 @@ int flash_write(struct target *target, struct image *image,
 {
 	return flash_write_unlock(target, image, written, erase, false);
 }
+
+/**
+ * Invalidates cached flash state which a target can change as it runs.
+ *
+ * @param target The target being resumed
+ *
+ * OpenOCD caches some flash state for brief periods.  For example, a sector
+ * that is protected must be unprotected before OpenOCD tries to write it,
+ * Also, a sector that's not erased must be erased before it's written.
+ *
+ * As a rule, OpenOCD and target firmware can both modify the flash, so when
+ * a target starts running, OpenOCD needs to invalidate its cached state.
+ */
+void nor_resume(struct target *target)
+{
+	struct flash_bank *bank;
+
+	for (bank = flash_banks; bank; bank = bank->next) {
+		int i;
+
+		if (bank->target != target)
+			continue;
+
+		for (i = 0; i < bank->num_sectors; i++) {
+			struct flash_sector *sector = bank->sectors + i;
+
+			sector->is_erased = -1;
+			sector->is_protected = -1;
+		}
+	}
+}