Previously we added the offset of the virtual register in several functions,
which produced segfaults. This patch renames a few parameters hence the
rather big diff.

Thanks to Roman Lebedev for reporting this issue and testing the patch.

Signed-off-by: Stefan Tauner <stefan.tauner@alumni.tuwien.ac.at>
---

Yes, you are right Roman, stupid error on my side. I forgot to add
the negation I announced in my previous email: line 660 should
contain ~REG2_MASK instead of just REG2_MASK.

 chipdrivers.h |  1 -
 jedec.c       | 65 +++++++++++++++++++++++++++--------------------------------
 2 files changed, 30 insertions(+), 36 deletions(-)

diff --git a/chipdrivers.h b/chipdrivers.h
index 8529c74..cac94f3 100644
--- a/chipdrivers.h
+++ b/chipdrivers.h
@@ -150,7 +150,6 @@ int unlock_regspace2_uniform_32k(struct flashctx *flash);
 int unlock_regspace2_uniform_64k(struct flashctx *flash);
 int unlock_regspace2_block_eraser_0(struct flashctx *flash);
 int unlock_regspace2_block_eraser_1(struct flashctx *flash);
-int unlock_regspace2_block(const struct flashctx *flash, chipaddr off);
 int printlock_regspace2_uniform_64k(struct flashctx *flash);
 int printlock_regspace2_block_eraser_0(struct flashctx *flash);
 int printlock_regspace2_block_eraser_1(struct flashctx *flash);
diff --git a/jedec.c b/jedec.c
index 1345b89..f92b354 100644
--- a/jedec.c
+++ b/jedec.c
@@ -589,11 +589,10 @@ static int regspace2_walk_unlockblocks(const struct flashctx *flash, const struc
 #define REG2_LOCKDOWN (1 << 1)
 #define REG2_MASK (REG2_RWLOCK | REG2_LOCKDOWN)

-static int printlock_regspace2_block(const struct flashctx *flash, chipaddr offset)
+static int printlock_regspace2_block(const struct flashctx *flash, chipaddr lockreg)
 {
-       chipaddr wrprotect = flash->virtual_registers + offset + 2;
-       uint8_t state = chip_readb(flash, wrprotect);
-       msg_cdbg("Lock status of block at 0x%0*" PRIxPTR " is ", PRIxPTR_WIDTH, offset);
+       uint8_t state = chip_readb(flash, lockreg);
+       msg_cdbg("Lock status of block at 0x%0*" PRIxPTR " is ", PRIxPTR_WIDTH, lockreg);
        switch (state & REG2_MASK) {
        case 0:
                msg_cdbg("Full Access.\n");
@@ -656,47 +655,44 @@ int printlock_regspace2_block_eraser_1(struct flashctx *flash)
        return regspace2_walk_unlockblocks(flash, unlockblocks, &printlock_regspace2_block);
 }

-static int changelock_regspace2_block(const struct flashctx *flash, chipaddr offset, uint8_t new_bits)
+static int changelock_regspace2_block(const struct flashctx *flash, chipaddr lockreg, uint8_t old, uint8_t new)
 {
-       chipaddr wrprotect = flash->virtual_registers + offset + 2;
-       uint8_t old;
-
-       if (new_bits & ~REG2_MASK) {
-               msg_cerr("Invalid locking change 0x%02x requested at 0x%0*" PRIxPTR "! "
+       if (((old ^ new) & ~REG2_MASK) != 0) {

+               msg_cerr("Invalid locking change from 0x%02x to 0x%02x requested at 0x%0*" PRIxPTR "!\n"
                         "Please report a bug at flashrom@flashrom.org\n",
-                        new_bits, PRIxPTR_WIDTH, offset);
+                        old, new, PRIxPTR_WIDTH, lockreg);
                return -1;
        }
-       old = chip_readb(flash, wrprotect);
        /* Early exist if no change (of read/write/lockdown) was requested. */
-       if (((old ^ new_bits) & REG2_MASK) == 0) {
-               msg_cdbg2("Locking status at 0x%0*" PRIxPTR " not changed\n", PRIxPTR_WIDTH, offset);
+       if (((old ^ new) & REG2_MASK) == 0) {
+               msg_cdbg2("Locking status at 0x%0*" PRIxPTR " not changed\n", PRIxPTR_WIDTH, lockreg);
                return 0;
        }
        /* Normally lockdowns can not be cleared. Try nevertheless if requested. */
-       if ((old & REG2_LOCKDOWN) && !(new_bits & REG2_LOCKDOWN)) {
-               chip_writeb(flash, old & ~REG2_LOCKDOWN, wrprotect);
-               if (chip_readb(flash, wrprotect) != (old & ~REG2_LOCKDOWN)) {
-                       msg_cerr("Lockdown can't be removed at 0x%0*" PRIxPTR "!\n", PRIxPTR_WIDTH, offset);
+       if ((old & REG2_LOCKDOWN) && !(new & REG2_LOCKDOWN)) {
+               chip_writeb(flash, old & ~REG2_LOCKDOWN, lockreg);
+               if (chip_readb(flash, lockreg) != (old & ~REG2_LOCKDOWN)) {
+                       msg_cerr("Lockdown can't be removed at 0x%0*" PRIxPTR "!\n", PRIxPTR_WIDTH, lockreg);
                        return -1;
                }
        }
        /* Change read or write lock? */
-       if ((old ^ new_bits) & REG2_RWLOCK) {
+       if ((old ^ new) & REG2_RWLOCK) {
                /* Do not lockdown yet. */
-               msg_cdbg("Changing locking status at 0x%0*" PRIxPTR " to 0x%02x\n", PRIxPTR_WIDTH, offset, new_bits & REG2_RWLOCK);
-               chip_writeb(flash, new_bits & REG2_RWLOCK, wrprotect);
-               if (chip_readb(flash, wrprotect) != (new_bits & REG2_RWLOCK)) {
-                       msg_cerr("Locking status change FAILED at 0x%0*" PRIxPTR "!\n", PRIxPTR_WIDTH, offset);
+               msg_cdbg("Changing locking status at 0x%0*" PRIxPTR " to 0x%02x\n",
+                        PRIxPTR_WIDTH, lockreg, new & REG2_RWLOCK);
+               chip_writeb(flash, new & REG2_RWLOCK, lockreg);
+               if (chip_readb(flash, lockreg) != (new & REG2_RWLOCK)) {
+                       msg_cerr("Locking status change FAILED at 0x%0*" PRIxPTR "!\n", PRIxPTR_WIDTH, lockreg);
                        return -1;
                }
        }
        /* Enable lockdown if requested. */
-       if (!(old & REG2_LOCKDOWN) && (new_bits & REG2_LOCKDOWN)) {
-               msg_cdbg("Enabling lockdown at 0x%0*" PRIxPTR "\n", PRIxPTR_WIDTH, offset);
-               chip_writeb(flash, new_bits, wrprotect);
-               if (chip_readb(flash, wrprotect) != new_bits) {
-                       msg_cerr("Enabling lockdown FAILED at 0x%0*" PRIxPTR "!\n", PRIxPTR_WIDTH, offset);
+       if (!(old & REG2_LOCKDOWN) && (new & REG2_LOCKDOWN)) {
+               msg_cdbg("Enabling lockdown at 0x%0*" PRIxPTR "\n", PRIxPTR_WIDTH, lockreg);
+               chip_writeb(flash, new, lockreg);
+               if (chip_readb(flash, lockreg) != new) {
+                       msg_cerr("Enabling lockdown FAILED at 0x%0*" PRIxPTR "!\n", PRIxPTR_WIDTH, lockreg);
                        return -1;
                }
        }
@@ -704,19 +700,18 @@ static int changelock_regspace2_block(const struct flashctx *flash, chipaddr off
        return 0;
 }

-int unlock_regspace2_block(const struct flashctx *flash, chipaddr off)
+static int unlock_regspace2_block_generic(const struct flashctx *flash, chipaddr lockreg)
 {
-       chipaddr wrprotect = flash->virtual_registers + off + 2;
-       uint8_t old = chip_readb(flash, wrprotect);
+       uint8_t old = chip_readb(flash, lockreg);
        /* We don't care for the lockdown bit as long as the RW locks are 0 after we're done */
-       return changelock_regspace2_block(flash, off, old & ~REG2_RWLOCK);
+       return changelock_regspace2_block(flash, lockreg, old, old & ~REG2_RWLOCK);
 }

 static int unlock_regspace2_uniform(struct flashctx *flash, unsigned long block_size)
 {
        const unsigned int elems = flash->chip->total_size * 1024 / block_size;
        struct unlockblock blocks[2] = {{.size = block_size, .count = elems}};
-       return regspace2_walk_unlockblocks(flash, blocks, &unlock_regspace2_block);
+       return regspace2_walk_unlockblocks(flash, blocks, &unlock_regspace2_block_generic);
 }

 int unlock_regspace2_uniform_64k(struct flashctx *flash)
@@ -734,7 +729,7 @@ int unlock_regspace2_block_eraser_0(struct flashctx *flash)
        // FIXME: this depends on the eraseblocks not to be filled up completely (i.e. to be null-terminated).
        const struct unlockblock *unlockblocks =
                (const struct unlockblock *)flash->chip->block_erasers[0].eraseblocks;
-       return regspace2_walk_unlockblocks(flash, unlockblocks, &unlock_regspace2_block);
+       return regspace2_walk_unlockblocks(flash, unlockblocks, &unlock_regspace2_block_generic);
 }

 int unlock_regspace2_block_eraser_1(struct flashctx *flash)
@@ -742,6 +737,6 @@ int unlock_regspace2_block_eraser_1(struct flashctx *flash)
        // FIXME: this depends on the eraseblocks not to be filled up completely (i.e. to be null-terminated).
        const struct unlockblock *unlockblocks =
                (const struct unlockblock *)flash->chip->block_erasers[1].eraseblocks;
-       return regspace2_walk_unlockblocks(flash, unlockblocks, &unlock_regspace2_block);
+       return regspace2_walk_unlockblocks(flash, unlockblocks, &unlock_regspace2_block_generic);
 }

--
Kind regards, Stefan Tauner