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 <dbrownell@users.sourceforge.net>

diff --git a/src/flash/nor/core.c b/src/flash/nor/core.c
index 18012c650b55d0f03603e24f9094bbe6dd12b6a8..15e460a1b0f9469c17979895d2ba016809ab9117 100644 (file)
--- 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)
        {