From 620310bcc64a0ba9103c4c05300fe9d25cc92b12 Mon Sep 17 00:00:00 2001 From: David Brownell Date: Thu, 15 Apr 2010 19:48:55 -0700 Subject: [PATCH] NOR/core bugfix: restore invariants The The patch labeled "CFI CORE: bug-fix protect single sector" was merged rged without some requested bugfixes. Most significantly it broke invariants in the code, invalidating descriptions and changing the calling convention for underlying drivers. (It (Also wasn't CFI-specific...) Fix that, and Include an update from Antonio Borneo for the degenerate "nothing to do" case, (although that's still in the wrong location. which is presumably why that is it was working in some cases but not all.) src/flash/nor/core.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) Signed-off-by: David Brownell --- src/flash/nor/core.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/flash/nor/core.c b/src/flash/nor/core.c index 18012c65..15e460a1 100644 --- a/src/flash/nor/core.c +++ b/src/flash/nor/core.c @@ -56,7 +56,10 @@ 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 */ + /* 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) @@ -90,10 +93,10 @@ scan: * REVISIT we could handle discontiguous regions by issuing * more than one driver request. How much would that matter? */ - if (i == first) { + if (i == first && i != last) { updated = true; first++; - } else if (i == last) { + } else if (i == last && i != first) { updated = true; last--; } @@ -107,11 +110,19 @@ scan: goto scan; } - /* Single sector, already protected? Nothing to do! */ - if (first > last) + /* Single sector, already protected? Nothing to do! + * We may have trimmed our parameters into this degenerate case. + * + * 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. + */ retval = bank->driver->protect(bank, set, first, last); if (retval != ERROR_OK) { -- 2.39.5