January 2023 - flashrom-gerrit

[M] Change in flashrom[master]: jedec.c: Rewrite control flow procedurally
by Edward O'Callaghan (Code Review) 30 Jan '23

30 Jan '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/72608 ) Change subject: jedec.c: Rewrite control flow procedurally ...................................................................... jedec.c: Rewrite control flow procedurally Drop goto usage in fav of loop constructs. Change-Id: I0927ed40e54cc7e114a57dc40e3614f4825a0ca9 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M jedec.c 1 file changed, 44 insertions(+), 34 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/72608/1 diff --git a/jedec.c b/jedec.c index 123a397..3df9cf2 100644 --- a/jedec.c +++ b/jedec.c @@ -352,30 +352,27 @@ static int write_byte_program_jedec_common(const struct flashctx *flash, const uint8_t *src, chipaddr dst, unsigned int mask) { - int tried = 0, failed = 0; - chipaddr bios = flash->virtual_memory; + int tries = 0; /* If the data is 0xFF, don't program it and don't complain. */ if (*src == 0xFF) { return 0; } -retry: - /* Issue JEDEC Byte Program command */ - start_program_jedec_common(flash, mask); + for (; tries < MAX_REFLASH_TRIES; tries++) { + const chipaddr bios = flash->virtual_memory; + /* Issue JEDEC Byte Program command */ + start_program_jedec_common(flash, mask); - /* transfer data from source to destination */ - chip_writeb(flash, *src, dst); - toggle_ready_jedec(flash, bios); + /* transfer data from source to destination */ + chip_writeb(flash, *src, dst); + toggle_ready_jedec(flash, bios); - if (chip_readb(flash, dst) != *src && tried++ < MAX_REFLASH_TRIES) { - goto retry; + if (chip_readb(flash, dst) == *src) + break; } - if (tried >= MAX_REFLASH_TRIES) - failed = 1; - - return failed; + return (tries >= MAX_REFLASH_TRIES) ? 1 : 0; } /* chunksize is 1 */ @@ -404,40 +401,41 @@ static int write_page_write_jedec_common(struct flashctx *flash, const uint8_t *src, unsigned int start, unsigned int page_size) { - unsigned int i; - int tried = 0, failed; + int tries = 0, failed; const uint8_t *s = src; - chipaddr bios = flash->virtual_memory; + const chipaddr bios = flash->virtual_memory; chipaddr dst = bios + start; chipaddr d = dst; const unsigned int mask = getaddrmask(flash->chip); -retry: - /* Issue JEDEC Start Program command */ - start_program_jedec_common(flash, mask); + for (; tries < MAX_REFLASH_TRIES; tries++) { + /* Issue JEDEC Start Program command */ + start_program_jedec_common(flash, mask); - /* transfer data from source to destination */ - for (i = 0; i < page_size; i++) { - /* If the data is 0xFF, don't program it */ - if (*src != 0xFF) - chip_writeb(flash, *src, dst); - dst++; - src++; - } + /* transfer data from source to destination */ + for (unsigned int i = 0; i < page_size; i++) { + /* If the data is 0xFF, don't program it */ + if (*src != 0xFF) + chip_writeb(flash, *src, dst); + dst++; + src++; + } - toggle_ready_jedec(flash, dst - 1); + toggle_ready_jedec(flash, dst - 1); - dst = d; - src = s; - failed = verify_range(flash, src, start, page_size); + dst = d; + src = s; + failed = verify_range(flash, src, start, page_size); + if (!failed) + break; - if (failed && tried++ < MAX_REFLASH_TRIES) { msg_cerr("retrying.\n"); - goto retry; } + if (failed) { msg_cerr(" page 0x%" PRIxPTR " failed!\n", (d - bios) / page_size); } + return failed; } -- To view, visit https://review.coreboot.org/c/flashrom/+/72608 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0927ed40e54cc7e114a57dc40e3614f4825a0ca9 Gerrit-Change-Number: 72608 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[L] Change in flashrom[master]: jedec.c: Move printlock stuff into printlock.c
by Edward O'Callaghan (Code Review) 30 Jan '23

30 Jan '23

Attention is currently required from: Thomas Heijligen. Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/72607 ) Change subject: jedec.c: Move printlock stuff into printlock.c ...................................................................... jedec.c: Move printlock stuff into printlock.c Change-Id: Iacaa16c81e141aac30feb6871700c4fdc9eec8e9 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M Makefile M jedec.c M meson.build A printlock.c 4 files changed, 225 insertions(+), 190 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/07/72607/1 diff --git a/Makefile b/Makefile index a7ab00b..fe28ab9 100644 --- a/Makefile +++ b/Makefile @@ -379,7 +379,7 @@ ############################################################################### # Flash chip drivers and bus support infrastructure. -CHIP_OBJS = jedec.o stm50.o w39.o w29ee011.o \ +CHIP_OBJS = jedec.o printlock.o stm50.o w39.o w29ee011.o \ sst28sf040.o 82802ab.o \ sst49lfxxxc.o sst_fwhub.o edi.o flashchips.o spi.o spi25.o spi25_statusreg.o \ spi95.o opaque.o sfdp.o en29lv640b.o at45db.o s25f.o \ diff --git a/jedec.c b/jedec.c index 3f27337..123a397 100644 --- a/jedec.c +++ b/jedec.c @@ -483,192 +483,3 @@ return 0; } - -struct unlockblock { - unsigned int size; - unsigned int count; -}; - -typedef int (*unlockblock_func)(const struct flashctx *flash, chipaddr offset); -static int regspace2_walk_unlockblocks(const struct flashctx *flash, const struct unlockblock *block, unlockblock_func func) -{ - chipaddr off = flash->virtual_registers + 2; - while (block->count != 0) { - unsigned int j; - for (j = 0; j < block->count; j++) { - if (func(flash, off)) - return -1; - off += block->size; - } - block++; - } - return 0; -} - -#define REG2_RWLOCK ((1 << 2) | (1 << 0)) -#define REG2_LOCKDOWN (1 << 1) -#define REG2_MASK (REG2_RWLOCK | REG2_LOCKDOWN) - -static int printlock_regspace2_block(const struct flashctx *flash, chipaddr lockreg) -{ - 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"); - break; - case 1: - msg_cdbg("Write Lock (Default State).\n"); - break; - case 2: - msg_cdbg("Locked Open (Full Access, Locked Down).\n"); - break; - case 3: - msg_cdbg("Write Lock, Locked Down.\n"); - break; - case 4: - msg_cdbg("Read Lock.\n"); - break; - case 5: - msg_cdbg("Read/Write Lock.\n"); - break; - case 6: - msg_cdbg("Read Lock, Locked Down.\n"); - break; - case 7: - msg_cdbg("Read/Write Lock, Locked Down.\n"); - break; - } - return 0; -} - -static int printlock_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, &printlock_regspace2_block); -} - -int printlock_regspace2_uniform_64k(struct flashctx *flash) -{ - return printlock_regspace2_uniform(flash, 64 * 1024); -} - -int printlock_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, &printlock_regspace2_block); -} - -int printlock_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, &printlock_regspace2_block); -} - -/* Try to change the lock register at address lockreg from cur to new. - * - * - Try to unlock the lock bit if requested and it is currently set (although this is probably futile). - * - Try to change the read/write bits if requested. - * - Try to set the lockdown bit if requested. - * Return an error immediately if any of this fails. */ -static int changelock_regspace2_block(const struct flashctx *flash, chipaddr lockreg, uint8_t cur, uint8_t new) -{ - /* Only allow changes to known read/write/lockdown bits */ - if (((cur ^ new) & ~REG2_MASK) != 0) { - msg_cerr("Invalid lock change from 0x%02x to 0x%02x requested at 0x%0*" PRIxPTR "!\n" - "Please report a bug at flashrom(a)flashrom.org\n", - cur, new, PRIxPTR_WIDTH, lockreg); - return -1; - } - - /* Exit early if no change (of read/write/lockdown bits) was requested. */ - if (((cur ^ new) & REG2_MASK) == 0) { - msg_cdbg2("Lock bits at 0x%0*" PRIxPTR " not changed.\n", PRIxPTR_WIDTH, lockreg); - return 0; - } - - /* Normally the lockdown bit can not be cleared. Try nevertheless if requested. */ - if ((cur & REG2_LOCKDOWN) && !(new & REG2_LOCKDOWN)) { - chip_writeb(flash, cur & ~REG2_LOCKDOWN, lockreg); - cur = chip_readb(flash, lockreg); - if ((cur & REG2_LOCKDOWN) == REG2_LOCKDOWN) { - msg_cwarn("Lockdown can't be removed at 0x%0*" PRIxPTR "! New value: 0x%02x.\n", - PRIxPTR_WIDTH, lockreg, cur); - return -1; - } - } - - /* Change read and/or write bit */ - if ((cur ^ new) & REG2_RWLOCK) { - /* Do not lockdown yet. */ - uint8_t wanted = (cur & ~REG2_RWLOCK) | (new & REG2_RWLOCK); - chip_writeb(flash, wanted, lockreg); - cur = chip_readb(flash, lockreg); - if (cur != wanted) { - msg_cerr("Changing lock bits failed at 0x%0*" PRIxPTR "! New value: 0x%02x.\n", - PRIxPTR_WIDTH, lockreg, cur); - return -1; - } - msg_cdbg("Changed lock bits at 0x%0*" PRIxPTR " to 0x%02x.\n", - PRIxPTR_WIDTH, lockreg, cur); - } - - /* Eventually, enable lockdown if requested. */ - if (!(cur & REG2_LOCKDOWN) && (new & REG2_LOCKDOWN)) { - chip_writeb(flash, new, lockreg); - cur = chip_readb(flash, lockreg); - if (cur != new) { - msg_cerr("Enabling lockdown FAILED at 0x%0*" PRIxPTR "! New value: 0x%02x.\n", - PRIxPTR_WIDTH, lockreg, cur); - return -1; - } - msg_cdbg("Enabled lockdown at 0x%0*" PRIxPTR ".\n", PRIxPTR_WIDTH, lockreg); - } - - return 0; -} - -static int unlock_regspace2_block_generic(const struct flashctx *flash, chipaddr lockreg) -{ - 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, 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_generic); -} - -int unlock_regspace2_uniform_64k(struct flashctx *flash) -{ - return unlock_regspace2_uniform(flash, 64 * 1024); -} - -int unlock_regspace2_uniform_32k(struct flashctx *flash) -{ - return unlock_regspace2_uniform(flash, 32 * 1024); -} - -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_generic); -} - -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_generic); -} diff --git a/meson.build b/meson.build index 36cb4f8..96a647b 100644 --- a/meson.build +++ b/meson.build @@ -60,6 +60,7 @@ 'helpers_fileio.c', 'ich_descriptors.c', 'jedec.c', + 'printlock.c', 'layout.c', 'libflashrom.c', 'opaque.c', diff --git a/printlock.c b/printlock.c new file mode 100644 index 0000000..d05251e --- /dev/null +++ b/printlock.c @@ -0,0 +1,213 @@ +/* + * This file is part of the flashrom project. + * + * Copyright (C) 2000 Silicon Integrated System Corporation + * Copyright (C) 2006 Giampiero Giancipoli <gianci(a)email.it> + * Copyright (C) 2006 coresystems GmbH <info(a)coresystems.de> + * Copyright (C) 2007-2012 Carl-Daniel Hailfinger + * Copyright (C) 2009 Sean Nelson <audiohacked(a)gmail.com> + * Copyright (C) 2014 Stefan Tauner + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include "flash.h" +#include "chipdrivers.h" + + +struct unlockblock { + unsigned int size; + unsigned int count; +}; + +typedef int (*unlockblock_func)(const struct flashctx *flash, chipaddr offset); +static int regspace2_walk_unlockblocks(const struct flashctx *flash, const struct unlockblock *block, unlockblock_func func) +{ + chipaddr off = flash->virtual_registers + 2; + while (block->count != 0) { + unsigned int j; + for (j = 0; j < block->count; j++) { + if (func(flash, off)) + return -1; + off += block->size; + } + block++; + } + return 0; +} + +#define REG2_RWLOCK ((1 << 2) | (1 << 0)) +#define REG2_LOCKDOWN (1 << 1) +#define REG2_MASK (REG2_RWLOCK | REG2_LOCKDOWN) + +static int printlock_regspace2_block(const struct flashctx *flash, chipaddr lockreg) +{ + 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"); + break; + case 1: + msg_cdbg("Write Lock (Default State).\n"); + break; + case 2: + msg_cdbg("Locked Open (Full Access, Locked Down).\n"); + break; + case 3: + msg_cdbg("Write Lock, Locked Down.\n"); + break; + case 4: + msg_cdbg("Read Lock.\n"); + break; + case 5: + msg_cdbg("Read/Write Lock.\n"); + break; + case 6: + msg_cdbg("Read Lock, Locked Down.\n"); + break; + case 7: + msg_cdbg("Read/Write Lock, Locked Down.\n"); + break; + } + return 0; +} + +static int printlock_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, &printlock_regspace2_block); +} + +int printlock_regspace2_uniform_64k(struct flashctx *flash) +{ + return printlock_regspace2_uniform(flash, 64 * 1024); +} + +int printlock_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, &printlock_regspace2_block); +} + +int printlock_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, &printlock_regspace2_block); +} + +/* Try to change the lock register at address lockreg from cur to new. + * + * - Try to unlock the lock bit if requested and it is currently set (although this is probably futile). + * - Try to change the read/write bits if requested. + * - Try to set the lockdown bit if requested. + * Return an error immediately if any of this fails. */ +static int changelock_regspace2_block(const struct flashctx *flash, chipaddr lockreg, uint8_t cur, uint8_t new) +{ + /* Only allow changes to known read/write/lockdown bits */ + if (((cur ^ new) & ~REG2_MASK) != 0) { + msg_cerr("Invalid lock change from 0x%02x to 0x%02x requested at 0x%0*" PRIxPTR "!\n" + "Please report a bug at flashrom(a)flashrom.org\n", + cur, new, PRIxPTR_WIDTH, lockreg); + return -1; + } + + /* Exit early if no change (of read/write/lockdown bits) was requested. */ + if (((cur ^ new) & REG2_MASK) == 0) { + msg_cdbg2("Lock bits at 0x%0*" PRIxPTR " not changed.\n", PRIxPTR_WIDTH, lockreg); + return 0; + } + + /* Normally the lockdown bit can not be cleared. Try nevertheless if requested. */ + if ((cur & REG2_LOCKDOWN) && !(new & REG2_LOCKDOWN)) { + chip_writeb(flash, cur & ~REG2_LOCKDOWN, lockreg); + cur = chip_readb(flash, lockreg); + if ((cur & REG2_LOCKDOWN) == REG2_LOCKDOWN) { + msg_cwarn("Lockdown can't be removed at 0x%0*" PRIxPTR "! New value: 0x%02x.\n", + PRIxPTR_WIDTH, lockreg, cur); + return -1; + } + } + + /* Change read and/or write bit */ + if ((cur ^ new) & REG2_RWLOCK) { + /* Do not lockdown yet. */ + uint8_t wanted = (cur & ~REG2_RWLOCK) | (new & REG2_RWLOCK); + chip_writeb(flash, wanted, lockreg); + cur = chip_readb(flash, lockreg); + if (cur != wanted) { + msg_cerr("Changing lock bits failed at 0x%0*" PRIxPTR "! New value: 0x%02x.\n", + PRIxPTR_WIDTH, lockreg, cur); + return -1; + } + msg_cdbg("Changed lock bits at 0x%0*" PRIxPTR " to 0x%02x.\n", + PRIxPTR_WIDTH, lockreg, cur); + } + + /* Eventually, enable lockdown if requested. */ + if (!(cur & REG2_LOCKDOWN) && (new & REG2_LOCKDOWN)) { + chip_writeb(flash, new, lockreg); + cur = chip_readb(flash, lockreg); + if (cur != new) { + msg_cerr("Enabling lockdown FAILED at 0x%0*" PRIxPTR "! New value: 0x%02x.\n", + PRIxPTR_WIDTH, lockreg, cur); + return -1; + } + msg_cdbg("Enabled lockdown at 0x%0*" PRIxPTR ".\n", PRIxPTR_WIDTH, lockreg); + } + + return 0; +} + +static int unlock_regspace2_block_generic(const struct flashctx *flash, chipaddr lockreg) +{ + 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, 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_generic); +} + +int unlock_regspace2_uniform_64k(struct flashctx *flash) +{ + return unlock_regspace2_uniform(flash, 64 * 1024); +} + +int unlock_regspace2_uniform_32k(struct flashctx *flash) +{ + return unlock_regspace2_uniform(flash, 32 * 1024); +} + +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_generic); +} + +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_generic); +} -- To view, visit https://review.coreboot.org/c/flashrom/+/72607 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iacaa16c81e141aac30feb6871700c4fdc9eec8e9 Gerrit-Change-Number: 72607 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: jedec.c: Trivial code style fix
by Edward O'Callaghan (Code Review) 30 Jan '23

30 Jan '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/72606 ) Change subject: jedec.c: Trivial code style fix ...................................................................... jedec.c: Trivial code style fix Change-Id: I42ebdda07512d0a84a6bd6d0630f96c40f039259 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M jedec.c 1 file changed, 12 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/06/72606/1 diff --git a/jedec.c b/jedec.c index b44b9c6..e97468e 100644 --- a/jedec.c +++ b/jedec.c @@ -196,8 +196,7 @@ if (probe_timing_enter) programmer_delay(flash, probe_timing_enter); /* Reset chip to a clean slate */ - if ((chip->feature_bits & FEATURE_RESET_MASK) == FEATURE_LONG_RESET) - { + if ((chip->feature_bits & FEATURE_RESET_MASK) == FEATURE_LONG_RESET) { chip_writeb(flash, 0xAA, bios + ((shifted ? 0x2AAA : 0x5555) & mask)); if (probe_timing_exit) programmer_delay(flash, 10); @@ -239,8 +238,7 @@ } /* Issue JEDEC Product ID Exit command */ - if ((chip->feature_bits & FEATURE_RESET_MASK) == FEATURE_LONG_RESET) - { + if ((chip->feature_bits & FEATURE_RESET_MASK) == FEATURE_LONG_RESET) { chip_writeb(flash, 0xAA, bios + ((shifted ? 0x2AAA : 0x5555) & mask)); if (probe_timing_exit) programmer_delay(flash, 10); -- To view, visit https://review.coreboot.org/c/flashrom/+/72606 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I42ebdda07512d0a84a6bd6d0630f96c40f039259 Gerrit-Change-Number: 72606 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: jedec.c: Consolidate op,operand sequencing into func
by Edward O'Callaghan (Code Review) 30 Jan '23

30 Jan '23

Attention is currently required from: Stefan Reinauer. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72502 ) Change subject: jedec.c: Consolidate op,operand sequencing into func ...................................................................... Patch Set 3: (1 comment) This change is ready for review. Patchset: PS2: > ```jedec.c: In function 'erase_sector_jedec': […] sorted. -- To view, visit https://review.coreboot.org/c/flashrom/+/72502 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9a1b211b49777f6d7f62a052975c1759e036c8df Gerrit-Change-Number: 72502 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Mon, 30 Jan 2023 00:51:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: jedec.c: Collapse probe_jedec() indirection
by Edward O'Callaghan (Code Review) 30 Jan '23

30 Jan '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/72504 ) Change subject: jedec.c: Collapse probe_jedec() indirection ...................................................................... jedec.c: Collapse probe_jedec() indirection Change-Id: I57c27bcf25c5d9ce10fb9c74d9be6ab3544ac7ba Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M jedec.c 1 file changed, 14 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/04/72504/1 diff --git a/jedec.c b/jedec.c index 697a239..b44b9c6 100644 --- a/jedec.c +++ b/jedec.c @@ -165,11 +165,12 @@ return 1; } -static int probe_jedec_common(struct flashctx *flash, unsigned int mask) +int probe_jedec(struct flashctx *flash) { - chipaddr bios = flash->virtual_memory; + const chipaddr bios = flash->virtual_memory; const struct flashchip *chip = flash->chip; - bool shifted = (flash->chip->feature_bits & FEATURE_ADDR_SHIFTED); + const bool shifted = (flash->chip->feature_bits & FEATURE_ADDR_SHIFTED); + const unsigned int mask = getaddrmask(flash->chip); uint8_t id1, id2; uint32_t largeid1, largeid2; uint32_t flashcontent1, flashcontent2; @@ -518,12 +519,6 @@ return 0; } -int probe_jedec(struct flashctx *flash) -{ - const unsigned int mask = getaddrmask(flash->chip); - return probe_jedec_common(flash, mask); -} - struct unlockblock { unsigned int size; unsigned int count; -- To view, visit https://review.coreboot.org/c/flashrom/+/72504 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I57c27bcf25c5d9ce10fb9c74d9be6ab3544ac7ba Gerrit-Change-Number: 72504 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: dummyflasher: fix propagation of register_*_master() return values
by Anastasia Klimchuk (Code Review) 30 Jan '23

30 Jan '23

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/72430 ) Change subject: dummyflasher: fix propagation of register_*_master() return values ...................................................................... dummyflasher: fix propagation of register_*_master() return values This patch checks return value of register_*_master() so that in case of an error this error is not ignored anymore. TEST=ninja test Change-Id: I3a7eeb3cdd814db18b0717ae8b40ecadb4c32f7c Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> Reviewed-on: https://review.coreboot.org/c/flashrom/+/72430 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M dummyflasher.c 1 file changed, 25 insertions(+), 6 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/dummyflasher.c b/dummyflasher.c index 1960e92..93f183a 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -1345,6 +1345,7 @@ static int dummy_init(const struct programmer_cfg *cfg) { + int ret = 0; struct stat image_stat; struct emu_data *data = calloc(1, sizeof(*data)); @@ -1410,15 +1411,15 @@ } if (dummy_buses_supported & BUS_PROG) - register_opaque_master(&opaque_master_dummyflasher, data); + ret |= register_opaque_master(&opaque_master_dummyflasher, data); if (dummy_buses_supported & BUS_NONSPI) - register_par_master(&par_master_dummyflasher, - dummy_buses_supported & BUS_NONSPI, - data); + ret |= register_par_master(&par_master_dummyflasher, + dummy_buses_supported & BUS_NONSPI, + data); if (dummy_buses_supported & BUS_SPI) - register_spi_master(&spi_master_dummyflasher, data); + ret |= register_spi_master(&spi_master_dummyflasher, data); - return 0; + return ret; } const struct programmer_entry programmer_dummy = { -- To view, visit https://review.coreboot.org/c/flashrom/+/72430 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3a7eeb3cdd814db18b0717ae8b40ecadb4c32f7c Gerrit-Change-Number: 72430 Gerrit-PatchSet: 2 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[master]: flashrom.c: Fix skip flag typo in read_flash()
by Edward O'Callaghan (Code Review) 30 Jan '23

30 Jan '23

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/72454 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: flashrom.c: Fix skip flag typo in read_flash() ...................................................................... flashrom.c: Fix skip flag typo in read_flash() Only picked up by internal test infra in ChromeOS when instrumenting futility(1). The following error was incured, ``` >> Starting firmware updater. >> Target image: images/bios-kindred.ro-12672-141-0.rw-12672-141-0.bin (RO:Google_Kindred.12672.141.0, RW/A:Google_Kindred.12672.141.0, RW/B:Google_Kindred.12672.141.0). INFO: update_firmware: Loading current system firmware... INFO: load_system_firmware: flashrom -r <IMAGE> -p host Warning: Setting BIOS Control at 0xdc from 0x8b to 0x89 failed. New value is 0x8b. At least some flash regions are read protected. You have to use a flash layout and include only accessible regions. For write operations, you'll additionally need the --noverify-all switch. See manpage for more details. read_flash: cannot read inside Management Engine region (0x001000..0x3fffff). Read operation failed! ERROR: do_update: Cannot load system active firmware. ``` despite the appropriate flag being set within futility(1). BUG=b:266748702 TEST=cros fw updates. Change-Id: Ie04cd62020ca29775fc66a81d2fadd32a5aab5cd Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/72454 Reviewed-by: Elyes Haouas <ehaouas(a)noos.fr> Reviewed-by: Nikolai Artemiev <nartemiev(a)google.com> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M flashrom.c 1 file changed, 39 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Elyes Haouas: Looks good to me, approved Nikolai Artemiev: Looks good to me, but someone else must approve Anastasia Klimchuk: Looks good to me, approved diff --git a/flashrom.c b/flashrom.c index e4f218e..3e21416 100644 --- a/flashrom.c +++ b/flashrom.c @@ -573,7 +573,7 @@ uint8_t *rbuf = buf + addr - start; if (region.read_prot) { - if (flash->flags.skip_unwritable_regions) { + if (flash->flags.skip_unreadable_regions) { msg_gdbg("%s: cannot read inside %s region (%#08x..%#08x), " "filling (%#08x..%#08x) with erased value instead.\n", __func__, region.name, region.start, region.end - 1, -- To view, visit https://review.coreboot.org/c/flashrom/+/72454 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie04cd62020ca29775fc66a81d2fadd32a5aab5cd Gerrit-Change-Number: 72454 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Sam McNally <sammc(a)google.com> Gerrit-MessageType: merged

1 0

[M] Change in flashrom[master]: jedec.c: Consolidate op,operand sequencing into func
by Stefan Reinauer (Code Review) 29 Jan '23

29 Jan '23

Attention is currently required from: Edward O'Callaghan. Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72502 ) Change subject: jedec.c: Consolidate op,operand sequencing into func ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: ```jedec.c: In function 'erase_sector_jedec': jedec.c:309:40: error: 'bios' undeclared (first use in this function) 309 | toggle_ready_jedec_slow(flash, bios); | ^~~~ jedec.c:309:40: note: each undeclared identifier is reported only once for each function it appears in jedec.c: In function 'erase_block_jedec': jedec.c:322:40: error: 'bios' undeclared (first use in this function) 322 | toggle_ready_jedec_slow(flash, bios); | ^~~~ jedec.c: In function 'erase_chip_block_jedec': jedec.c:341:40: error: 'bios' undeclared (first use in this function) 341 | toggle_ready_jedec_slow(flash, bios); | ^~~~ ``` -- To view, visit https://review.coreboot.org/c/flashrom/+/72502 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9a1b211b49777f6d7f62a052975c1759e036c8df Gerrit-Change-Number: 72502 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Sun, 29 Jan 2023 04:55:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: jedec.c: Collapse erase_chip_block_jedec() indirection
by Stefan Reinauer (Code Review) 29 Jan '23

29 Jan '23

Attention is currently required from: Edward O'Callaghan. Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72503 ) Change subject: jedec.c: Collapse erase_chip_block_jedec() indirection ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/72503 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6e4a63e4935d4b69b66cbd1888f62b6af066a2ac Gerrit-Change-Number: 72503 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Sun, 29 Jan 2023 04:39:12 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: jedec.c: Collapse erase_block_jedec() indirection
by Stefan Reinauer (Code Review) 29 Jan '23

29 Jan '23

Attention is currently required from: Edward O'Callaghan. Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72501 ) Change subject: jedec.c: Collapse erase_block_jedec() indirection ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/72501 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia9b539d7fb4a54ba6cc56730e96d35044e291844 Gerrit-Change-Number: 72501 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Sun, 29 Jan 2023 04:37:35 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit January 2023