@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}
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,
*/
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)
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)
{
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;
- }
- }
-}
* 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;
};
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.
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,
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)
{
.help = "Check erase state of all blocks in a "
"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,
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;
}
Code Review / openocd.git / commitdiff