November 2022 - flashrom-gerrit

[XL] Change in flashrom[master]: tree/: Convert flashchips db to use indirection for erase_block
by Edward O'Callaghan (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/69131 to look at the new patch set (#2). Change subject: tree/: Convert flashchips db to use indirection for erase_block ...................................................................... tree/: Convert flashchips db to use indirection for erase_block This paves the way to allow for the conversion of flashchip erase_block func ptr to enumerate values. This change should be a NOP. TEST=`diff -u <(objdump -D flashchips.o_bk) <(objdump -D flashchips.o)`. Change-Id: I122295ec9add0fe0efd27273c9725e5d64f6dbe2 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M at45db.c M atapromise.c M flashchips.c M flashrom.c M include/flash.h M sfdp.c M spi25.c M tests/chip.c M tests/chip_wp.c 9 files changed, 2,022 insertions(+), 1,971 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/31/69131/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/69131 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I122295ec9add0fe0efd27273c9725e5d64f6dbe2 Gerrit-Change-Number: 69131 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: tests: Undefine _FORTIFY_SOURCE for unit tests to avoid _chk variants
by Peter Marheine (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Anastasia Klimchuk. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68432 ) Change subject: tests: Undefine _FORTIFY_SOURCE for unit tests to avoid _chk variants ...................................................................... Patch Set 5: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/68432 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I70cb1cd90d1f377ff4606acad3c1b514120ae4f7 Gerrit-Change-Number: 68432 Gerrit-PatchSet: 5 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 02 Nov 2022 02:27:35 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: tests: Undefine _FORTIFY_SOURCE for unit tests to avoid _chk variants
by Anastasia Klimchuk (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Peter Marheine. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68432 ) Change subject: tests: Undefine _FORTIFY_SOURCE for unit tests to avoid _chk variants ...................................................................... Patch Set 5: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/68432/comment/f1ea4fdb_40148912 PS3, Line 11: sence > sense Done Patchset: PS3: > Should we go ahead and remove the existing _chk wraps as well? Yes good idea, removed. -- To view, visit https://review.coreboot.org/c/flashrom/+/68432 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I70cb1cd90d1f377ff4606acad3c1b514120ae4f7 Gerrit-Change-Number: 68432 Gerrit-PatchSet: 5 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Wed, 02 Nov 2022 02:26:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tests: Add prefix to io_mock functions not to clash with macros
by Anastasia Klimchuk (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Anastasia Klimchuk. Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Angel Pons, Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/68433 to look at the new patch set (#5). Change subject: tests: Add prefix to io_mock functions not to clash with macros ...................................................................... tests: Add prefix to io_mock functions not to clash with macros Flashrom I/O mock functions need to be renamed so that they do not have name clash with standard I/O, because the latter are allowed to be macros. Adding a prefix to flashrom mock functions avoids them being accidentally expanded. Standard I/O functions are expanded and flashrom mocks stays as they are. BUG=b:237606255 TEST=ninja test 1) gcc 12.2.0 on Debian 2) clang 15.0 on Chromium OS Ticket: https://ticket.coreboot.org/issues/411 Change-Id: I7998a8fb1b9e65621e12adbfab5460a245d5606b Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/chip.c M tests/io_mock.c M tests/io_mock.h M tests/linux_mtd.c M tests/linux_spi.c M tests/parade_lspcon.c M tests/realtek_mst_i2c_spi.c M tests/tests.c 8 files changed, 65 insertions(+), 43 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/33/68433/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/68433 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7998a8fb1b9e65621e12adbfab5460a245d5606b Gerrit-Change-Number: 68433 Gerrit-PatchSet: 5 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: tests: Undefine _FORTIFY_SOURCE for unit tests to avoid _chk variants
by Anastasia Klimchuk (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Anastasia Klimchuk. Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Angel Pons, Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/68432 to look at the new patch set (#5). Change subject: tests: Undefine _FORTIFY_SOURCE for unit tests to avoid _chk variants ...................................................................... tests: Undefine _FORTIFY_SOURCE for unit tests to avoid _chk variants The option _FORTIFY_SOURCE, when enabled, can result in some functions being expanded into _chk variants. For example, `fprintf` can get expanded into `__fprintf_chk`. This makes sense for building a real binary, but is not needed for unit tests. In unit test environment all those functions are wrapped. In the example above, both `fprintf` and `__fprintf_chk` needed to be mocked. Disabling _FORTIFY_SOURCE avoids expanding functions into _chk variants, without any loss of testing coverage because that would be wrapped/mocked anyway. This patch also removes two existing _chk wraps because they are not needed anymore. BUG=b:237606255 TEST=ninja test on 1) gcc 12.2.0 on Debian 2) clang 15.0 on Chromium OS Ticket: https://ticket.coreboot.org/issues/411 Change-Id: I70cb1cd90d1f377ff4606acad3c1b514120ae4f7 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/meson.build M tests/tests.c 2 files changed, 31 insertions(+), 18 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/68432/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/68432 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I70cb1cd90d1f377ff4606acad3c1b514120ae4f7 Gerrit-Change-Number: 68432 Gerrit-PatchSet: 5 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tests: Add prefix to io_mock functions not to clash with macros
by Anastasia Klimchuk (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Anastasia Klimchuk. Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Angel Pons, Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/68433 to look at the new patch set (#4). Change subject: tests: Add prefix to io_mock functions not to clash with macros ...................................................................... tests: Add prefix to io_mock functions not to clash with macros Flashrom I/O mock functions need to be renamed so that they do not have name clash with standard I/O, because the latter are allowed to be macros. Adding a prefix to flashrom mock functions avoids them being accidentally expanded. Standard I/O functions are expanded and flashrom mocks stays as they are. BUG=b:237606255 TEST=ninja test 1) gcc 12.2.0 on Debian 2) clang 15.0 on Chromium OS Ticket: https://ticket.coreboot.org/issues/411 Change-Id: I7998a8fb1b9e65621e12adbfab5460a245d5606b Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/chip.c M tests/io_mock.c M tests/io_mock.h M tests/linux_mtd.c M tests/linux_spi.c M tests/parade_lspcon.c M tests/realtek_mst_i2c_spi.c M tests/tests.c 8 files changed, 65 insertions(+), 43 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/33/68433/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/68433 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7998a8fb1b9e65621e12adbfab5460a245d5606b Gerrit-Change-Number: 68433 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: tests: Undefine _FORTIFY_SOURCE for unit tests to avoid _chk variants
by Anastasia Klimchuk (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Anastasia Klimchuk. Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Angel Pons, Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/68432 to look at the new patch set (#4). Change subject: tests: Undefine _FORTIFY_SOURCE for unit tests to avoid _chk variants ...................................................................... tests: Undefine _FORTIFY_SOURCE for unit tests to avoid _chk variants The option _FORTIFY_SOURCE, when enabled, can result in some functions being expanded into _chk variants. For example, `fprintf` can get expanded into `__fprintf_chk`. This makes sence for building a real binary, but is not needed for unit tests. In unit test environment all those functions are wrapped. In the example above, both `fprintf` and `__fprintf_chk` needed to be mocked. Disabling _FORTIFY_SOURCE avoids expanding functions into _chk variants, without any loss of testing coverage because that would be wrapped/mocked anyway. This patch also removes two existing _chk wraps because they are not needed anymore. BUG=b:237606255 TEST=ninja test on 1) gcc 12.2.0 on Debian 2) clang 15.0 on Chromium OS Ticket: https://ticket.coreboot.org/issues/411 Change-Id: I70cb1cd90d1f377ff4606acad3c1b514120ae4f7 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/meson.build M tests/tests.c 2 files changed, 31 insertions(+), 18 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/68432/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/68432 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I70cb1cd90d1f377ff4606acad3c1b514120ae4f7 Gerrit-Change-Number: 68432 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tree/: Convert flashchip erase_block func ptr to enumerate [ALT]
by Edward O'Callaghan (Code Review) 02 Nov '22

02 Nov '22

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/69133 ) Change subject: tree/: Convert flashchip erase_block func ptr to enumerate [ALT] ...................................................................... tree/: Convert flashchip erase_block func ptr to enumerate [ALT] This forges the way for flashchips.c to be pure declarative data and lookup functions for dispatch to be pure. This means that the flashchips data could be extracted out to be agnostic data of the flashrom code and algorithms. Change-Id: I02ae7e4c67c5bf34ec2fd7ffe4af8a2aba6fd5e5 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M at45db.c M flashrom.c M include/chipdrivers.h M include/flash.h M sfdp.c M spi25.c M tests/chip.c 7 files changed, 135 insertions(+), 50 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/33/69133/1 diff --git a/at45db.c b/at45db.c index 53e15ec..eeebf49 100644 --- a/at45db.c +++ b/at45db.c @@ -80,9 +80,11 @@ unsigned int i, j; unsigned int cnt = 0; for (i = 0; i < NUM_ERASEFUNCTIONS; i++) { - if (flash->chip->block_erasers[i].block_erase == SPI_ERASE_AT45DB_SECTOR) { + const struct block_eraser *const eraser = &flash->chip->block_erasers[i]; + erasefunc_t *erase_func = lookup_erase_func_ptr_(eraser); + if (erase_func == &spi_erase_at45db_sector) { for (j = 0; j < NUM_ERASEREGIONS; j++) { - cnt += flash->chip->block_erasers[i].eraseblocks[j].count; + cnt += eraser->eraseblocks[j].count; } } } diff --git a/flashrom.c b/flashrom.c index cf6a59d..1f36d45 100644 --- a/flashrom.c +++ b/flashrom.c @@ -337,22 +337,79 @@ return extract_param(&cfg->params, param_name, ","); } +/* special unit-test hook */ +erasefunc_t *g_test_erase_injector; + +erasefunc_t *lookup_erase_func_ptr_(const struct block_eraser *const eraser) +{ + switch (eraser->block_erase) { + case SPI_BLOCK_ERASE_EMULATION: return &spi_block_erase_emulation; + case SPI_BLOCK_ERASE_20: return &spi_block_erase_20; + case SPI_BLOCK_ERASE_21: return &spi_block_erase_21; + case SPI_BLOCK_ERASE_40: return NULL; // FIXME unhandled &spi_block_erase_40; + case SPI_BLOCK_ERASE_50: return &spi_block_erase_50; + case SPI_BLOCK_ERASE_52: return &spi_block_erase_52; + case SPI_BLOCK_ERASE_53: return &spi_block_erase_53; + case SPI_BLOCK_ERASE_5C: return &spi_block_erase_5c; + case SPI_BLOCK_ERASE_60: return &spi_block_erase_60; + case SPI_BLOCK_ERASE_62: return &spi_block_erase_62; + case SPI_BLOCK_ERASE_81: return &spi_block_erase_81; + case SPI_BLOCK_ERASE_C4: return &spi_block_erase_c4; + case SPI_BLOCK_ERASE_C7: return &spi_block_erase_c7; + case SPI_BLOCK_ERASE_D7: return &spi_block_erase_d7; + case SPI_BLOCK_ERASE_D8: return &spi_block_erase_d8; + case SPI_BLOCK_ERASE_DB: return &spi_block_erase_db; + case SPI_BLOCK_ERASE_DC: return &spi_block_erase_dc; + case S25FL_BLOCK_ERASE: return &s25fl_block_erase; + case S25FS_BLOCK_ERASE_D8: return &s25fs_block_erase_d8; + case JEDEC_SECTOR_ERASE: return &erase_sector_jedec; // TODO rename to &jedec_sector_erase; + case JEDEC_BLOCK_ERASE: return &erase_block_jedec; // TODO rename to &jedec_block_erase; + case JEDEC_CHIP_BLOCK_ERASE: return &erase_chip_block_jedec; // TODO rename to &jedec_chip_block_erase; + case OPAQUE_ERASE: return &erase_opaque; // TODO rename to &opqaue_erase; + case SPI_ERASE_AT45CS_SECTOR: return &spi_erase_at45cs_sector; + case SPI_ERASE_AT45DB_BLOCK: return &spi_erase_at45db_block; + case SPI_ERASE_AT45DB_CHIP: return &spi_erase_at45db_chip; + case SPI_ERASE_AT45DB_PAGE: return &spi_erase_at45db_page; + case SPI_ERASE_AT45DB_SECTOR: return &spi_erase_at45db_sector; + case ERASE_CHIP_28SF040: return &erase_chip_28sf040; + case ERASE_SECTOR_28SF040: return &erase_sector_28sf040; + case ERASE_BLOCK_82802AB: return &erase_block_82802ab; + case ERASE_SECTOR_49LFXXXC: return &erase_sector_49lfxxxc; + case STM50_SECTOR_ERASE: return &erase_sector_stm50; // TODO rename to &stm50_sector_erase; + case EDI_CHIP_BLOCK_ERASE: return &edi_chip_block_erase; + case TEST_ERASE_INJECTOR: return g_test_erase_injector; + /* default: total function, 0 indicates no erase function set. + * We explicitly do not want a default catch-all case in the switch + * to ensure unhandled enum's are compiler warnings. + */ + case NO_BLOCK_ERASE_FUNC: return NULL; + }; + + return NULL; +} +static erasefunc_t *lookup_erase_func_ptr(const struct flashchip *chip, int idx) +{ + const struct block_eraser *const eraser = &chip->block_erasers[idx]; + return lookup_erase_func_ptr_(eraser); +} + static int check_block_eraser(const struct flashctx *flash, int k, int log) { struct block_eraser eraser = flash->chip->block_erasers[k]; + erasefunc_t *erase_func = lookup_erase_func_ptr(flash->chip, k); - if (eraser.block_erase == NO_BLOCK_ERASE_FUNC && !eraser.eraseblocks[0].count) { + if (erase_func == NO_BLOCK_ERASE_FUNC && !eraser.eraseblocks[0].count) { if (log) msg_cdbg("not defined. "); return 1; } - if (eraser.block_erase == NO_BLOCK_ERASE_FUNC && eraser.eraseblocks[0].count) { + if (erase_func == NO_BLOCK_ERASE_FUNC && eraser.eraseblocks[0].count) { if (log) msg_cdbg("eraseblock layout is known, but matching " "block erase function is not implemented. "); return 1; } - if (eraser.block_erase != NO_BLOCK_ERASE_FUNC && !eraser.eraseblocks[0].count) { + if (erase_func != NO_BLOCK_ERASE_FUNC && !eraser.eraseblocks[0].count) { if (log) msg_cdbg("block erase function found, but " "eraseblock layout is not defined. "); @@ -1096,7 +1153,8 @@ msg_cdbg(", "); msg_cdbg("0x%06x-0x%06x:", info->erase_start, info->erase_end); - ret = per_blockfn(flashctx, info, eraser->block_erase); + erasefunc_t *erase_func = lookup_erase_func_ptr_(eraser); + ret = per_blockfn(flashctx, info, erase_func); if (ret) return ret; } diff --git a/include/chipdrivers.h b/include/chipdrivers.h index 61d43c9..fc8e6ed 100644 --- a/include/chipdrivers.h +++ b/include/chipdrivers.h @@ -52,8 +52,8 @@ int spi_block_erase_d8(struct flashctx *flash, unsigned int addr, unsigned int blocklen); int spi_block_erase_db(struct flashctx *flash, unsigned int addr, unsigned int blocklen); int spi_block_erase_dc(struct flashctx *flash, unsigned int addr, unsigned int blocklen); -erasefunc_t *spi_get_erasefn_from_opcode(uint8_t opcode); -uint8_t spi_get_opcode_from_erasefn(erasefunc_t *func); +enum block_erase_func spi_get_erasefn_from_opcode(uint8_t opcode); +uint8_t spi_get_opcode_from_erasefn(enum block_erase_func func); int spi_chip_write_1(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); int spi_nbyte_read(struct flashctx *flash, unsigned int addr, uint8_t *bytes, unsigned int len); int spi_read_chunked(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len, unsigned int chunksize); diff --git a/include/flash.h b/include/flash.h index 8639cd3..ce7ed81 100644 --- a/include/flash.h +++ b/include/flash.h @@ -268,41 +268,46 @@ typedef int (read_func_t)(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len); int read_flash(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len); -#define NO_BLOCK_ERASE_FUNC NULL -#define SPI_BLOCK_ERASE_EMULATION &spi_block_erase_emulation -#define SPI_BLOCK_ERASE_20 &spi_block_erase_20 -#define SPI_BLOCK_ERASE_21 &spi_block_erase_21 -#define SPI_BLOCK_ERASE_40 &spi_block_erase_40 -#define SPI_BLOCK_ERASE_50 &spi_block_erase_50 -#define SPI_BLOCK_ERASE_52 &spi_block_erase_52 -#define SPI_BLOCK_ERASE_53 &spi_block_erase_53 -#define SPI_BLOCK_ERASE_5C &spi_block_erase_5c -#define SPI_BLOCK_ERASE_60 &spi_block_erase_60 -#define SPI_BLOCK_ERASE_62 &spi_block_erase_62 -#define SPI_BLOCK_ERASE_81 &spi_block_erase_81 -#define SPI_BLOCK_ERASE_C4 &spi_block_erase_c4 -#define SPI_BLOCK_ERASE_C7 &spi_block_erase_c7 -#define SPI_BLOCK_ERASE_D7 &spi_block_erase_d7 -#define SPI_BLOCK_ERASE_D8 &spi_block_erase_d8 -#define SPI_BLOCK_ERASE_DB &spi_block_erase_db -#define SPI_BLOCK_ERASE_DC &spi_block_erase_dc -#define S25FL_BLOCK_ERASE &s25fl_block_erase -#define S25FS_BLOCK_ERASE_D8 &s25fs_block_erase_d8 -#define JEDEC_SECTOR_ERASE &erase_sector_jedec -#define JEDEC_BLOCK_ERASE &erase_block_jedec -#define JEDEC_CHIP_BLOCK_ERASE &erase_chip_block_jedec -#define OPAQUE_ERASE &erase_opaque -#define SPI_ERASE_AT45CS_SECTOR &spi_erase_at45cs_sector -#define SPI_ERASE_AT45DB_BLOCK &spi_erase_at45db_block -#define SPI_ERASE_AT45DB_CHIP &spi_erase_at45db_chip -#define SPI_ERASE_AT45DB_PAGE &spi_erase_at45db_page -#define SPI_ERASE_AT45DB_SECTOR &spi_erase_at45db_sector -#define ERASE_CHIP_28SF040 &erase_chip_28sf040 -#define ERASE_SECTOR_28SF040 &erase_sector_28sf040 -#define ERASE_BLOCK_82802AB &erase_block_82802ab -#define ERASE_SECTOR_49LFXXXC &erase_sector_49lfxxxc -#define STM50_SECTOR_ERASE &erase_sector_stm50 -#define EDI_CHIP_BLOCK_ERASE &edi_chip_block_erase +enum block_erase_func { + NO_BLOCK_ERASE_FUNC = 0, /* 0 indicates no block erase function set. */ + SPI_BLOCK_ERASE_EMULATION = 1, + SPI_BLOCK_ERASE_20, + SPI_BLOCK_ERASE_21, + SPI_BLOCK_ERASE_40, + SPI_BLOCK_ERASE_50, + SPI_BLOCK_ERASE_52, + SPI_BLOCK_ERASE_53, + SPI_BLOCK_ERASE_5C, + SPI_BLOCK_ERASE_60, + SPI_BLOCK_ERASE_62, + SPI_BLOCK_ERASE_81, + SPI_BLOCK_ERASE_C4, + SPI_BLOCK_ERASE_C7, + SPI_BLOCK_ERASE_D7, + SPI_BLOCK_ERASE_D8, + SPI_BLOCK_ERASE_DB, + SPI_BLOCK_ERASE_DC, + S25FL_BLOCK_ERASE, + S25FS_BLOCK_ERASE_D8, + JEDEC_SECTOR_ERASE, + JEDEC_BLOCK_ERASE, + JEDEC_CHIP_BLOCK_ERASE, + OPAQUE_ERASE, + SPI_ERASE_AT45CS_SECTOR, + SPI_ERASE_AT45DB_BLOCK, + SPI_ERASE_AT45DB_CHIP, + SPI_ERASE_AT45DB_PAGE, + SPI_ERASE_AT45DB_SECTOR, + ERASE_CHIP_28SF040, + ERASE_SECTOR_28SF040, + ERASE_BLOCK_82802AB, + ERASE_SECTOR_49LFXXXC, + STM50_SECTOR_ERASE, + EDI_CHIP_BLOCK_ERASE, + TEST_ERASE_INJECTOR, /* special case must come last. */ +}; +struct block_eraser; +erasefunc_t *lookup_erase_func_ptr_(const struct block_eraser *const eraser); struct flashchip { const char *vendor; @@ -366,7 +371,7 @@ } eraseblocks[NUM_ERASEREGIONS]; /* a block_erase function should try to erase one block of size * 'blocklen' at address 'blockaddr' and return 0 on success. */ - int (*block_erase) (struct flashctx *flash, unsigned int blockaddr, unsigned int blocklen); + enum block_erase_func block_erase; } block_erasers[NUM_ERASEFUNCTIONS]; int (*printlock) (struct flashctx *flash); diff --git a/sfdp.c b/sfdp.c index a09827d..ccf840f 100644 --- a/sfdp.c +++ b/sfdp.c @@ -81,7 +81,7 @@ { int i; uint32_t total_size = chip->total_size * 1024; - erasefunc_t *erasefn = spi_get_erasefn_from_opcode(opcode); + enum block_erase_func erasefn = spi_get_erasefn_from_opcode(opcode); if (erasefn == NO_BLOCK_ERASE_FUNC || total_size == 0 || block_size == 0 || total_size % block_size != 0) { diff --git a/spi25.c b/spi25.c index fb94748..da870be 100644 --- a/spi25.c +++ b/spi25.c @@ -619,7 +619,7 @@ } static const struct { - erasefunc_t *func; + enum block_erase_func func; uint8_t opcode; } function_opcode_list[] = { {SPI_BLOCK_ERASE_20, 0x20}, @@ -639,7 +639,7 @@ {SPI_BLOCK_ERASE_DC, 0xdc}, }; -erasefunc_t *spi_get_erasefn_from_opcode(uint8_t opcode) +enum block_erase_func spi_get_erasefn_from_opcode(uint8_t opcode) { size_t i; for (i = 0; i < ARRAY_SIZE(function_opcode_list); i++) { @@ -651,14 +651,14 @@ return NO_BLOCK_ERASE_FUNC; } -uint8_t spi_get_opcode_from_erasefn(erasefunc_t *func) +uint8_t spi_get_opcode_from_erasefn(enum block_erase_func func) { size_t i; for (i = 0; i < ARRAY_SIZE(function_opcode_list); i++) { if (function_opcode_list[i].func == func) return function_opcode_list[i].opcode; } - msg_cinfo("%s: unknown erase function (0x%p). Please report " + msg_cinfo("%s: unknown erase function (0x%d). Please report " "this at flashrom(a)flashrom.org\n", __func__, func); return 0x00; //Assuming 0x00 is not a erase function opcode } diff --git a/tests/chip.c b/tests/chip.c index f09c020..d9f5bfb 100644 --- a/tests/chip.c +++ b/tests/chip.c @@ -148,6 +148,7 @@ extern write_func_t *g_test_write_injector; extern read_func_t *g_test_read_injector; +extern erasefunc_t *g_test_erase_injector; static const struct flashchip chip_8MiB = { .vendor = "aklm", @@ -160,7 +161,7 @@ {{ /* All blocks within total size of the chip. */ .eraseblocks = { {2 * MiB, 4} }, - .block_erase = block_erase_chip, + .block_erase = TEST_ERASE_INJECTOR, }}, }; @@ -208,6 +209,7 @@ g_test_write_injector = write_chip; g_test_read_injector = read_chip; + g_test_erase_injector = block_erase_chip; struct flashrom_flashctx flashctx = { 0 }; struct flashrom_layout *layout; struct flashchip mock_chip = chip_8MiB; @@ -268,6 +270,7 @@ g_test_write_injector = write_chip; g_test_read_injector = read_chip; + g_test_erase_injector = block_erase_chip; struct flashrom_flashctx flashctx = { 0 }; struct flashrom_layout *layout; struct flashchip mock_chip = chip_8MiB; @@ -341,6 +344,7 @@ g_test_write_injector = write_chip; g_test_read_injector = read_chip; + g_test_erase_injector = block_erase_chip; struct flashrom_flashctx flashctx = { 0 }; struct flashrom_layout *layout; struct flashchip mock_chip = chip_8MiB; @@ -440,6 +444,7 @@ g_test_write_injector = write_chip; g_test_read_injector = read_chip; + g_test_erase_injector = block_erase_chip; struct flashrom_flashctx flashctx = { 0 }; struct flashrom_layout *layout; struct flashchip mock_chip = chip_8MiB; -- To view, visit https://review.coreboot.org/c/flashrom/+/69133 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I02ae7e4c67c5bf34ec2fd7ffe4af8a2aba6fd5e5 Gerrit-Change-Number: 69133 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: meson: Move programmer test sources into programmer definition
by Anastasia Klimchuk (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68162 ) Change subject: meson: Move programmer test sources into programmer definition ...................................................................... Patch Set 16: Code-Review+1 (2 comments) File meson.build: https://review.coreboot.org/c/flashrom/+/68162/comment/a6448335_ee63435a PS16, Line 191: 'test_srcs' : Is it possible to align all `:` ? Maybe it's just me... but it jumps into my eyes that `:`s are not aligned, and same for the loop below :) File tests/meson.build: https://review.coreboot.org/c/flashrom/+/68162/comment/08ceadfa_504bbc2b PS10, Line 32: p_data += { : 'testsrc' : p_data.get('testsrc', []), : } : srcs += p_data.get('testsrc') > I just noticed I made a typo and that's why it didn't work. oopsie 😄 […] Good to know it works! And looks even better in the latest patchset! -- To view, visit https://review.coreboot.org/c/flashrom/+/68162 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I307faaf8a9f7ae3c54bd96e7d871a3abb8aadea3 Gerrit-Change-Number: 68162 Gerrit-PatchSet: 16 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 02 Nov 2022 01:48:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[XL] Change in flashrom[master]: tree/: Convert flashchip erase_block func ptr to enumerate
by Edward O'Callaghan (Code Review) 02 Nov '22

02 Nov '22

Attention is currently required from: Felix Singer, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67240 ) Change subject: tree/: Convert flashchip erase_block func ptr to enumerate ...................................................................... Patch Set 15: (1 comment) Patchset: PS15: > Besides the at45db changes, looks OK. […] I've done what you have asked of me https://review.coreboot.org/c/flashrom/+/69131 Although this change was extensively tested as well as has a breakdown into its constituents (abandon patches) from which it was squashed together. Personally I feel there is more opportunity to introduce bugs messing around between representations between commits even though your idea in principle is good. In any case I have done what you have asked in CB:69131 -- To view, visit https://review.coreboot.org/c/flashrom/+/67240 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I58415a4f2589812b26d284c96876e88f8d9acf52 Gerrit-Change-Number: 67240 Gerrit-PatchSet: 15 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 02 Nov 2022 01:47:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2022