summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorØyvind Harboe <oyvind.harboe@zylin.com>2010-05-05 15:08:34 +0200
committerØyvind Harboe <oyvind.harboe@zylin.com>2010-05-05 15:24:25 +0200
commit737c9b6258c6e68714ae264ff36126eb5d382d6a (patch)
tree508ba9ecd384a3f4d2fda13231d5fe53161e8c78 /src
parentf7e0f3c285e9b1578184da886792e02d253ea687 (diff)
downloadopenocd_libswd-737c9b6258c6e68714ae264ff36126eb5d382d6a.tar.gz
openocd_libswd-737c9b6258c6e68714ae264ff36126eb5d382d6a.tar.bz2
openocd_libswd-737c9b6258c6e68714ae264ff36126eb5d382d6a.tar.xz
openocd_libswd-737c9b6258c6e68714ae264ff36126eb5d382d6a.zip
flash: stop caching protection state
There are a million reasons why cached protection state might be stale: power cycling of target, reset, code executing on the target, etc. The "flash protect_check" command is now gone. This is *always* executed when running a "flash info". As a bonus for more a more robust approach, lots of code could be deleted. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Diffstat (limited to 'src')
-rw-r--r--src/flash/nor/cfi.c11
-rw-r--r--src/flash/nor/core.c100
-rw-r--r--src/flash/nor/core.h7
-rw-r--r--src/flash/nor/tcl.c39
-rw-r--r--src/target/target.c13
5 files changed, 31 insertions, 139 deletions
diff --git a/src/flash/nor/cfi.c b/src/flash/nor/cfi.c
index 92b553b5..4ef41b9a 100644
--- a/src/flash/nor/cfi.c
+++ b/src/flash/nor/cfi.c
@@ -852,6 +852,17 @@ static int cfi_intel_protect(struct flash_bank *bank, int set, int first, int la
*/
if ((!set) && (!(pri_ext->feature_support & 0x20)))
{
+ /* FIX!!! this code path is broken!!!
+ *
+ * The correct approach is:
+ *
+ * 1. read out current protection status
+ *
+ * 2. override read out protection status w/unprotected.
+ *
+ * 3. re-protect what should be protected.
+ *
+ */
for (i = 0; i < bank->num_sectors; i++)
{
if (bank->sectors[i].is_protected == 1)
diff --git a/src/flash/nor/core.c b/src/flash/nor/core.c
index e07ca1f6..936f07ca 100644
--- a/src/flash/nor/core.c
+++ b/src/flash/nor/core.c
@@ -54,74 +54,27 @@ 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 (un?)protect just that sector.
- code including Lower level ddrivers may rely on this "first <= last"
- * invariant.
- */
/* callers may not supply illegal parameters ... */
if (first < 0 || first > last || last >= bank->num_sectors)
+ {
+ LOG_ERROR("illegal sector range");
return ERROR_FAIL;
+ }
/* force "set" to 0/1 */
set = !!set;
- /*
- * Filter out what trivial nonsense we can, so drivers don't have to.
+ /* DANGER!
*
- * 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 && i != last) {
- updated = true;
- first++;
- } else if (i == last && i != first) {
- 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!
- * We may have trimmed our parameters into this degenerate case.
+ * We must not use any cached information about protection state!!!!
*
- * FIXME repeating the "is_protected==set" test is a giveaway that
- * this fast-exit belongs earlier, in the trim-it-down loop; mve.
- * */
- if (first == last && bank->sectors[first].is_protected == set)
- return ERROR_OK;
-
-
- /* Note that we don't pass illegal parameters to drivers; any
- * trimming just turns one valid range into another one.
+ * There are a million things that could change the protect state:
+ *
+ * the target could have reset, power cycled, been hot plugged,
+ * the application could have run, etc.
+ *
+ * Drivers only receive valid sector range.
*/
retval = bank->driver->protect(bank, set, first, last);
if (retval != ERROR_OK)
@@ -754,34 +707,3 @@ 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;
- }
- }
-}
diff --git a/src/flash/nor/core.h b/src/flash/nor/core.h
index b1526774..1dfd721b 100644
--- a/src/flash/nor/core.h
+++ b/src/flash/nor/core.h
@@ -53,6 +53,10 @@ struct flash_sector
* Indication of protection status: 0 = unprotected/unlocked,
* 1 = protected/locked, other = unknown. Set by
* @c flash_driver_s::protect_check.
+ *
+ * This information must be considered stale immediately.
+ * A million things could make it stale: power cycle,
+ * reset of target, code running on target, etc.
*/
int is_protected;
};
@@ -124,9 +128,6 @@ int flash_unlock_address_range(struct target *target, uint32_t addr,
int flash_write(struct target *target,
struct image *image, uint32_t *written, int erase);
-/* invalidate cached state (targets may modify their own flash) */
-void nor_resume(struct target *target);
-
/**
* Forces targets to re-examine their erase/protection state.
* This routine must be called when the system may modify the status.
diff --git a/src/flash/nor/tcl.c b/src/flash/nor/tcl.c
index 947fd046..17c6e910 100644
--- a/src/flash/nor/tcl.c
+++ b/src/flash/nor/tcl.c
@@ -70,6 +70,11 @@ COMMAND_HANDLER(handle_flash_info_command)
if ((retval = p->driver->auto_probe(p)) != ERROR_OK)
return retval;
+ /* We must query the hardware to avoid printing stale information! */
+ retval = p->driver->protect_check(p);
+ if (retval != ERROR_OK)
+ return retval;
+
command_print(CMD_CTX,
"#%" PRIi32 " : %s at 0x%8.8" PRIx32 ", size 0x%8.8" PRIx32 ", buswidth %i, chipwidth %i",
i,
@@ -266,32 +271,6 @@ COMMAND_HANDLER(handle_flash_erase_address_command)
return retval;
}
-COMMAND_HANDLER(handle_flash_protect_check_command)
-{
- if (CMD_ARGC != 1)
- return ERROR_COMMAND_SYNTAX_ERROR;
-
- struct flash_bank *p;
- int retval = CALL_COMMAND_HANDLER(flash_command_get_bank, 0, &p);
- if (ERROR_OK != retval)
- return retval;
-
- if ((retval = p->driver->protect_check(p)) == ERROR_OK)
- {
- command_print(CMD_CTX, "successfully checked protect state");
- }
- else if (retval == ERROR_FLASH_OPERATION_FAILED)
- {
- command_print(CMD_CTX, "checking protection state failed (possibly unsupported) by flash #%s at 0x%8.8" PRIx32, CMD_ARGV[0], p->base);
- }
- else
- {
- command_print(CMD_CTX, "unknown error when checking protection state of flash bank '#%s' at 0x%8.8" PRIx32, CMD_ARGV[0], p->base);
- }
-
- return ERROR_OK;
-}
-
static int flash_check_sector_parameters(struct command_context *cmd_ctx,
uint32_t first, uint32_t last, uint32_t num_sectors)
{
@@ -707,14 +686,6 @@ static const struct command_registration flash_exec_command_handlers[] = {
"flash bank.",
},
{
- .name = "protect_check",
- .handler = handle_flash_protect_check_command,
- .mode = COMMAND_EXEC,
- .usage = "bank_id",
- .help = "Check protection state of all blocks in a "
- "flash bank.",
- },
- {
.name = "erase_sector",
.handler = handle_flash_erase_command,
.mode = COMMAND_EXEC,
diff --git a/src/target/target.c b/src/target/target.c
index d17bb744..37e515a6 100644
--- a/src/target/target.c
+++ b/src/target/target.c
@@ -474,19 +474,6 @@ int target_resume(struct target *target, int current, uint32_t address, int hand
if ((retval = target->type->resume(target, current, address, handle_breakpoints, debug_execution)) != ERROR_OK)
return retval;
- /* Invalidate any cached protect/erase/... flash status, since
- * almost all targets will now be able modify the flash by
- * themselves. We want flash drivers and infrastructure to
- * be able to rely on (non-invalidated) cached state.
- *
- * For now we require that algorithms provided by OpenOCD are
- * used only by code which properly maintains that cached state.
- * state
- *
- * REVISIT do the same for NAND ; maybe other flash flavors too...
- */
- if (!target->running_alg)
- nor_resume(target);
return retval;
}