November 2022 - flashrom-gerrit

[M] Change in flashrom[master]: layout: Factor out flash_region structure from romentry
by Angel Pons (Code Review) 11 Nov '22

11 Nov '22

Attention is currently required from: Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69196 ) Change subject: layout: Factor out flash_region structure from romentry ...................................................................... Patch Set 2: (1 comment) File include/layout.h: https://review.coreboot.org/c/flashrom/+/69196/comment/2ed656db_abf085a4 PS2, Line 42: bool read_prot; : bool write_prot; > @Nik, when you hijack this patch maybe drop these in this patch and add in a subsequent patch when y […] +1 -- To view, visit https://review.coreboot.org/c/flashrom/+/69196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I768742b73db901df5b5208fcbcb8a324a06014c2 Gerrit-Change-Number: 69196 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Fri, 11 Nov 2022 22:21:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: cli_classic.c: Be consistent with pointer types
by Angel Pons (Code Review) 11 Nov '22

11 Nov '22

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/69451 ) Change subject: cli_classic.c: Be consistent with pointer types ...................................................................... cli_classic.c: Be consistent with pointer types With `i586-pc-msdosdjgpp-gcc (GCC) 12.2.0`, `uint32_t` is defined as `long unsigned int`, which is not the same as `unsigned int`. As the `flashrom_layout_get_region_range()` function is part of libflashrom API, adjust `cli_classic.c` instead to avoid type mismatches. Change-Id: Ie8f5bc0d9296f7c6b8f8a351b53052f5fe86b09d Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M cli_classic.c 1 file changed, 17 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/51/69451/1 diff --git a/cli_classic.c b/cli_classic.c index 574afff..d6b14bf 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -127,7 +127,7 @@ return true; } -static int parse_wp_range(uint32_t *start, uint32_t *len) +static int parse_wp_range(unsigned int *start, unsigned int *len) { char *endptr = NULL, *token = NULL; @@ -569,7 +569,7 @@ int namelen, opt, i, j; int startchip = -1, chipcount = 0, option_index = 0; int operation_specified = 0; - uint32_t wp_start = 0, wp_len = 0; + unsigned int wp_start = 0, wp_len = 0; bool force = false, ifd = false, fmap = false; #if CONFIG_PRINT_WIKI == 1 bool list_supported_wiki = false; -- To view, visit https://review.coreboot.org/c/flashrom/+/69451 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie8f5bc0d9296f7c6b8f8a351b53052f5fe86b09d Gerrit-Change-Number: 69451 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: meson: Move programmer test sources into programmer definition
by Felix Singer (Code Review) 11 Nov '22

11 Nov '22

Felix Singer has submitted this change. ( https://review.coreboot.org/c/flashrom/+/68162 ) ( 16 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: meson: Move programmer test sources into programmer definition ...................................................................... meson: Move programmer test sources into programmer definition Move the definition of the programmer test source files into the dictionary defining the programmers itself. This way there is a better overview about which of the available programmers have tests and which don't. Also, to keep the tests working, iterate over all programmers and add their test source files to the list of sources that should be built. Signed-off-by: Felix Singer <felixsinger(a)posteo.net> Change-Id: I307faaf8a9f7ae3c54bd96e7d871a3abb8aadea3 Reviewed-on: https://review.coreboot.org/c/flashrom/+/68162 Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M meson.build M tests/meson.build 2 files changed, 36 insertions(+), 9 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, but someone else must approve Anastasia Klimchuk: Looks good to me, approved diff --git a/meson.build b/meson.build index 3a6cc41..d4a8ad1 100644 --- a/meson.build +++ b/meson.build @@ -188,6 +188,7 @@ 'deps' : [ libusb1 ], 'groups' : [ group_usb, group_external ], 'srcs' : files('dediprog.c', 'usbdev.c'), + 'test_srcs' : files('tests/dediprog.c'), 'flags' : [ '-DCONFIG_DEDIPROG=1' ], }, 'developerbox_spi' : { @@ -217,6 +218,7 @@ }, 'dummy' : { 'srcs' : files('dummyflasher.c'), + 'test_srcs' : files('tests/dummyflasher.c'), 'flags' : [ '-DCONFIG_DUMMY=1' ], }, 'ft2232_spi' : { @@ -285,6 +287,7 @@ 'deps' : [ linux_headers ], 'groups' : [ group_internal ], 'srcs' : files('linux_mtd.c'), + 'test_srcs' : files('tests/linux_mtd.c'), 'flags' : [ '-DCONFIG_LINUX_MTD=1' ], }, 'linux_spi' : { @@ -292,6 +295,7 @@ 'deps' : [ linux_headers ], # internal / external? 'srcs' : files('linux_spi.c'), + 'test_srcs' : files('tests/linux_spi.c'), 'flags' : [ '-DCONFIG_LINUX_SPI=1' ], }, 'parade_lspcon' : { @@ -299,6 +303,7 @@ 'deps' : [ linux_headers ], 'groups' : [ group_i2c ], 'srcs' : files('parade_lspcon.c', 'i2c_helper_linux.c'), + 'test_srcs' : files('tests/parade_lspcon.c'), 'flags' : [ '-DCONFIG_PARADE_LSPCON=1' ], 'default' : false }, @@ -307,6 +312,7 @@ 'deps' : [ linux_headers ], 'groups' : [ group_i2c ], 'srcs' : files('mediatek_i2c_spi.c', 'i2c_helper_linux.c'), + 'test_srcs' : files('tests/mediatek_i2c_spi.c'), 'flags' : [ '-DCONFIG_MEDIATEK_I2C_SPI=1' ], 'default' : false, }, @@ -362,6 +368,7 @@ 'deps' : [ libpci ], 'groups' : [ group_pci, group_internal ], 'srcs' : files('nicrealtek.c', 'pcidev.c'), + 'test_srcs' : files('tests/nicrealtek.c'), 'flags' : [ '-DCONFIG_NICREALTEK=1' ], }, 'ogp_spi' : { @@ -387,6 +394,7 @@ 'deps' : [ libusb1 ], 'groups' : [ group_usb, group_external ], 'srcs' : files('raiden_debug_spi.c', 'usb_device.c'), + 'test_srcs' : files('tests/raiden_debug_spi.c'), 'flags' : [ '-DCONFIG_RAIDEN_DEBUG_SPI=1' ], }, 'rayer_spi' : { @@ -401,6 +409,7 @@ 'deps' : [ linux_headers ], 'groups' : [ group_i2c ], 'srcs' : files('realtek_mst_i2c_spi.c', 'i2c_helper_linux.c'), + 'test_srcs' : files('tests/realtek_mst_i2c_spi.c'), 'flags' : [ '-DCONFIG_REALTEK_MST_I2C_SPI=1' ], 'default' : false, }, @@ -447,6 +456,7 @@ 'deps' : p_data.get('deps', []), 'groups' : p_data.get('groups', []), 'srcs' : p_data.get('srcs', []), + 'test_srcs': p_data.get('test_srcs', []), 'flags' : p_data.get('flags', []), 'default' : p_data.get('default', true), } diff --git a/tests/meson.build b/tests/meson.build index 66adb92..893cca8 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -19,15 +19,6 @@ 'flashrom.c', 'spi25.c', 'lifecycle.c', - 'dummyflasher.c', - 'nicrealtek.c', - 'raiden_debug_spi.c', - 'dediprog.c', - 'linux_mtd.c', - 'linux_spi.c', - 'parade_lspcon.c', - 'mediatek_i2c_spi.c', - 'realtek_mst_i2c_spi.c', 'layout.c', 'chip.c', 'chip_wp.c', @@ -37,6 +28,10 @@ test_srcs += programmer.get('dummy').get('srcs') endif +foreach p_name, p_data : programmer + test_srcs += p_data.get('test_srcs') +endforeach + mocks = [ '-Wl,--wrap=strdup', '-Wl,--wrap=physunmap', -- 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: 18 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

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

11 Nov '22

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/69133 ) ( 5 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: tree/: Convert flashchip erase_block func ptr to enumerate ...................................................................... tree/: Convert flashchip erase_block func ptr to enumerate 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> Reviewed-on: https://review.coreboot.org/c/flashrom/+/69133 Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- 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, 127 insertions(+), 47 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/at45db.c b/at45db.c index 53e15ec..7877330 100644 --- a/at45db.c +++ b/at45db.c @@ -80,9 +80,10 @@ 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]; + if (eraser->block_erase == 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 08e3969..815f489 100644 --- a/flashrom.c +++ b/flashrom.c @@ -336,6 +336,57 @@ return extract_param(&cfg->params, param_name, ","); } +/* special unit-test hook */ +erasefunc_t *g_test_erase_injector; + +static 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 int check_block_eraser(const struct flashctx *flash, int k, int log) { struct block_eraser eraser = flash->chip->block_erasers[k]; @@ -1099,7 +1150,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 470d1fe..3b07afe 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 03c1e2c..238d010 100644 --- a/include/flash.h +++ b/include/flash.h @@ -268,41 +268,44 @@ 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 flashchip { const char *vendor; @@ -366,7 +369,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 579643c..1d9cdc9 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: 8 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[master]: ichspi: clear byte count in ich_start_hwseq_xfer()
by Angel Pons (Code Review) 11 Nov '22

11 Nov '22

Attention is currently required from: Nikolai Artemiev. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69422 ) Change subject: ichspi: clear byte count in ich_start_hwseq_xfer() ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/69422 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2535bdfb77ff370bddcb507a229bbf4119681cdf Gerrit-Change-Number: 69422 Gerrit-PatchSet: 2 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 11 Nov 2022 12:26:06 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: ichspi: Factor out common hwseq_xfer logic into helpers
by Angel Pons (Code Review) 11 Nov '22

11 Nov '22

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62869 ) Change subject: ichspi: Factor out common hwseq_xfer logic into helpers ...................................................................... Patch Set 30: (1 comment) File ichspi.c: https://review.coreboot.org/c/flashrom/+/62869/comment/08b9dca7_cd60c3d1 PS30, Line 1368: hsfc &= ~HSFC_FDBC; /* clear byte count */ This got lost, CB:69422 fixes it -- To view, visit https://review.coreboot.org/c/flashrom/+/62869 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic9fd50841449e02f476a8834f4642d6ecad36dc3 Gerrit-Change-Number: 62869 Gerrit-PatchSet: 30 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Fri, 11 Nov 2022 12:25:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: ichspi: clear byte count in ich_start_hwseq_xfer()
by Angel Pons (Code Review) 11 Nov '22

11 Nov '22

Attention is currently required from: Nikolai Artemiev. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69422 ) Change subject: ichspi: clear byte count in ich_start_hwseq_xfer() ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: Did you test if `ich_hwseq_block_erase()` still works? Looking at CB:62869 it did not clear the byte count before. -- To view, visit https://review.coreboot.org/c/flashrom/+/69422 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2535bdfb77ff370bddcb507a229bbf4119681cdf Gerrit-Change-Number: 69422 Gerrit-PatchSet: 2 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 11 Nov 2022 12:25:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

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

11 Nov '22

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/69131 ) ( 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )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> Reviewed-on: https://review.coreboot.org/c/flashrom/+/69131 Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- 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,026 insertions(+), 1,971 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved -- 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: 5 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in flashrom[master]: flashrom_tester: Change the wp_toggle semantics
by Evan Benn (Code Review) 11 Nov '22

11 Nov '22

Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69417 ) Change subject: flashrom_tester: Change the wp_toggle semantics ...................................................................... Patch Set 3: (1 comment) File util/flashrom_tester/flashrom/src/flashromlib.rs: https://review.coreboot.org/c/flashrom/+/69417/comment/23a4bdfe_c372dc05 PS3, Line 104: fn wp_toggle(&self, en: bool) -> Result<bool, FlashromError> { Sorry Nik, somehow I dropped the part where I was changing the libflashrom behaviour too. Took a while to debug that. -- To view, visit https://review.coreboot.org/c/flashrom/+/69417 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia01d612d988e6580a7c5f0fd448ccc319ce9b181 Gerrit-Change-Number: 69417 Gerrit-PatchSet: 3 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Fri, 11 Nov 2022 05:19:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom_tester: Change the wp_toggle semantics
by Evan Benn (Code Review) 11 Nov '22

11 Nov '22

Hello build bot (Jenkins), Nikolai Artemiev, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/69417 to look at the new patch set (#3). Change subject: flashrom_tester: Change the wp_toggle semantics ...................................................................... flashrom_tester: Change the wp_toggle semantics wp_toggle and wp_range had some confusing behaviour where enabling wp would set a range, but disabling wp would not unset the range (explicitly). This was a way to workaround the MTD kernel driver semantics. Now make things very explicit, enabling software write protect will set the range to the whole chip. Disabling write protect will set the range to 0,0. This makes all drivers behave the same as MTD, and documents the exact behaviour explicitly. BUG=b:244663741 BRANCH=None TEST=flashrom_tester --libflashrom host # MTD and non-MTD TEST=flashrom_tester --flashrom_binary # MTD and non-MTD Change-Id: Ia01d612d988e6580a7c5f0fd448ccc319ce9b181 Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M util/flashrom_tester/flashrom/src/cmd.rs M util/flashrom_tester/flashrom/src/flashromlib.rs M util/flashrom_tester/flashrom/src/lib.rs 3 files changed, 37 insertions(+), 25 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/17/69417/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/69417 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia01d612d988e6580a7c5f0fd448ccc319ce9b181 Gerrit-Change-Number: 69417 Gerrit-PatchSet: 3 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2022