From: Øyvind H. <go...@us...> - 2010-05-06 07:43:41
|
This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "Main OpenOCD repository". The branch, master has been updated via 737c9b6258c6e68714ae264ff36126eb5d382d6a (commit) from f7e0f3c285e9b1578184da886792e02d253ea687 (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- commit 737c9b6258c6e68714ae264ff36126eb5d382d6a Author: Ãyvind Harboe <oyv...@zy...> Date: Wed May 5 15:08:34 2010 +0200 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 <oyv...@zy...> diff --git a/doc/openocd.texi b/doc/openocd.texi index d311c8e..a4c4de2 100644 --- a/doc/openocd.texi +++ b/doc/openocd.texi @@ -4129,9 +4129,8 @@ The @var{num} parameter is a value shown by @command{flash banks}. @deffn Command {flash info} num Print info about flash bank @var{num} The @var{num} parameter is a value shown by @command{flash banks}. -The information includes per-sector protect status, which may be -incorrect (outdated) unless you first issue a -@command{flash protect_check num} command. +This command will first query the hardware, it does not print cached +and possibly stale information. @end deffn @anchor{flash protect} @@ -4144,14 +4143,6 @@ specifies "to the end of the flash bank". The @var{num} parameter is a value shown by @command{flash banks}. @end deffn -@deffn Command {flash protect_check} num -Check protection state of sectors in flash bank @var{num}. -The @var{num} parameter is a value shown by @command{flash banks}. -@comment @option{flash erase_sector} using the same syntax. -This updates the protection information displayed by @option{flash info}. -(Code execution may have invalidated any state records kept by OpenOCD.) -@end deffn - @anchor{Flash Driver List} @section Flash Driver List As noted above, the @command{flash bank} command requires a driver name, diff --git a/src/flash/nor/cfi.c b/src/flash/nor/cfi.c index 92b553b..4ef41b9 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 e07ca1f..936f07c 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 b152677..1dfd721 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 947fd04..17c6e91 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 d17bb74..37e515a 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; } ----------------------------------------------------------------------- Summary of changes: doc/openocd.texi | 13 +----- src/flash/nor/cfi.c | 11 +++++ src/flash/nor/core.c | 100 +++++-------------------------------------------- src/flash/nor/core.h | 7 ++- src/flash/nor/tcl.c | 39 ++----------------- src/target/target.c | 13 ------ 6 files changed, 33 insertions(+), 150 deletions(-) hooks/post-receive -- Main OpenOCD repository |