July 2021 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: flashrom.c: Reorder check_block_eraser() to avoid forward decl
by Anastasia Klimchuk (Code Review) 14 Jul '21

14 Jul '21

Attention is currently required from: Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56296 ) Change subject: flashrom.c: Reorder check_block_eraser() to avoid forward decl ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Patchset: PS2: Is this the kind of change that can be tested by comparing binaries before and after? -- To view, visit https://review.coreboot.org/c/flashrom/+/56296 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0906a6e581ce5135b58f6acc6339908dfa770a59 Gerrit-Change-Number: 56296 Gerrit-PatchSet: 2 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 14 Jul 2021 22:47:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: Make extract_param() static local
by Anastasia Klimchuk (Code Review) 14 Jul '21

14 Jul '21

Attention is currently required from: Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56295 ) Change subject: flashrom.c: Make extract_param() static local ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/56295 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I81f1cdb9df98c151201390edeb69c74defe7881f Gerrit-Change-Number: 56295 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 14 Jul 2021 22:38:57 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Replace freenode references
by Edward O'Callaghan (Code Review) 14 Jul '21

14 Jul '21

Attention is currently required from: Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56031 ) Change subject: Replace freenode references ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/56031 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I75e8f43c50dc4c3feede0250334a877cdaac8103 Gerrit-Change-Number: 56031 Gerrit-PatchSet: 2 Gerrit-Owner: 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: David Hendricks <david.hendricks(a)gmail.com> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 14 Jul 2021 11:46:48 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add support for TerribleFire TF530 SPI controller
by Edward O'Callaghan (Code Review) 14 Jul '21

14 Jul '21

Attention is currently required from: Stefan Reinauer, Paul Menzel, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54944 ) Change subject: Add support for TerribleFire TF530 SPI controller ...................................................................... Patch Set 1: Code-Review+1 (3 comments) File Makefile: https://review.coreboot.org/c/flashrom/+/54944/comment/94a81356_4e64285c PS1, Line 1211: ifeq ($(CONFIG_TF530_SPI), yes) : FEATURE_CFLAGS += -D'CONFIG_TF530_SPI=1' : PROGRAMMER_OBJS += tf530_spi.o : endif please also add to meson. File tf530_spi.c: https://review.coreboot.org/c/flashrom/+/54944/comment/3b4bf82b_8669b8e9 PS1, Line 62: static struct ConfigDev *cd = NULL; > I'll have a look at this again. […] One of the key ideas of flashrom becoming reentrant is allowing for extensible testing by improving the core API in how drivers are registered as to be more akin to that of the kernel. While it is clear the life-time of the handle is for the driver there should be not reason to not pack this into the data field of the spi master struct as is the case in the commits that Angel alludes to. https://review.coreboot.org/c/flashrom/+/54944/comment/139b2f8a_1279e6f8 PS1, Line 71: uint8_t busy = 0; : : while (busy == 0) { : busy = port->ctrl & TF53x_CTRL_BUSY; : } > nit: I'd remove the `busy` variable, the name is confusing (it's active-low): […] as it is a common pattern, seems like a good idea to have it in a small function with a timeout. See https://github.com/flashrom/flashrom/blob/master/realtek_mst_i2c_spi.c#L108 for the general concept. -- To view, visit https://review.coreboot.org/c/flashrom/+/54944 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I109eec4dec89650bb80d3d1da905d33f65f60f5a Gerrit-Change-Number: 54944 Gerrit-PatchSet: 1 Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 14 Jul 2021 11:45:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add AmigaOS suppport to flashrom
by Angel Pons (Code Review) 14 Jul '21

14 Jul '21

Attention is currently required from: Stefan Reinauer, Paul Menzel, Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54943 ) Change subject: Add AmigaOS suppport to flashrom ...................................................................... Patch Set 1: (1 comment) File Makefile: https://review.coreboot.org/c/flashrom/+/54943/comment/7f147b3e_30aa1055 PS1, Line 137: : ifeq ($(TARGET_OS), AmigaOS) > Can you add meson support as well? Does AmigaOS have meson support? -- To view, visit https://review.coreboot.org/c/flashrom/+/54943 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3e4a3885f88313aad019454fdbdf4be176b35330 Gerrit-Change-Number: 54943 Gerrit-PatchSet: 1 Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 14 Jul 2021 11:39:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add AmigaOS suppport to flashrom
by Edward O'Callaghan (Code Review) 14 Jul '21

14 Jul '21

Attention is currently required from: Stefan Reinauer, Paul Menzel, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54943 ) Change subject: Add AmigaOS suppport to flashrom ...................................................................... Patch Set 1: Code-Review+1 (2 comments) File Makefile: https://review.coreboot.org/c/flashrom/+/54943/comment/d8d7aec5_b4783c18 PS1, Line 137: : ifeq ($(TARGET_OS), AmigaOS) Can you add meson support as well? File cli_classic.c: https://review.coreboot.org/c/flashrom/+/54943/comment/f7153ca4_f416b430 PS1, Line 162: *argv[] > Yep. […] Is there any reason this shouldn't be const anyway? -- To view, visit https://review.coreboot.org/c/flashrom/+/54943 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3e4a3885f88313aad019454fdbdf4be176b35330 Gerrit-Change-Number: 54943 Gerrit-PatchSet: 1 Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 14 Jul 2021 11:34:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: Make extract_param() static local
by Nico Huber (Code Review) 14 Jul '21

14 Jul '21

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56295 ) Change subject: flashrom.c: Make extract_param() static local ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/56295 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I81f1cdb9df98c151201390edeb69c74defe7881f Gerrit-Change-Number: 56295 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 14 Jul 2021 10:38:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashchips: Add MX25L12873F
by Nico Huber (Code Review) 14 Jul '21

14 Jul '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/56054 ) Change subject: flashchips: Add MX25L12873F ...................................................................... flashchips: Add MX25L12873F Just add the name to the existing entry, as usual it is supposed to be compatible. Change-Id: I59c8067f15b5ceac5a2e2f8fe93431a465f17e23 Signed-off-by: Nico Huber <nico.h(a)gmx.de> Reviewed-on: https://review.coreboot.org/c/flashrom/+/56054 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M flashchips.c M flashchips.h 2 files changed, 2 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved diff --git a/flashchips.c b/flashchips.c index 6310c12..8cd3dfd 100644 --- a/flashchips.c +++ b/flashchips.c @@ -8359,7 +8359,7 @@ { .vendor = "Macronix", - .name = "MX25L12835F/MX25L12845E/MX25L12865E", + .name = "MX25L12835F/MX25L12845E/MX25L12865E/MX25L12873F", .bustype = BUS_SPI, .manufacture_id = MACRONIX_ID, .model_id = MACRONIX_MX25L12805D, diff --git a/flashchips.h b/flashchips.h index dd985a7..249e7b4 100644 --- a/flashchips.h +++ b/flashchips.h @@ -506,7 +506,7 @@ #define MACRONIX_MX25L1605 0x2015 /* MX25L1605 (64k 0x20); MX25L1605A/MX25L1606E/MX25L1608E (4k 0x20, 64k 0x52); MX25L1605D/MX25L1608D/MX25L1673E (4k 0x20) */ #define MACRONIX_MX25L3205 0x2016 /* MX25L3205, MX25L3205A (64k 0x20); MX25L3205D/MX25L3208D (4k 0x20); MX25L3206E/MX25L3208E (4k 0x20, 64k 0x52); MX25L3233F/MX25L3273E (4k 0x20, 32k 0x52) */ #define MACRONIX_MX25L6405 0x2017 /* MX25L6405, MX25L6405D (64k 0x20); MX25L6406E/MX25L6408E (4k 0x20); MX25L6436E/MX25L6445E/MX25L6465E/MX25L6473E (4k 0x20, 32k 0x52) */ -#define MACRONIX_MX25L12805D 0x2018 /* MX25L12805D (no 32k); MX25L12865E, MX25L12835F, MX25L12845E (32k 0x52) */ +#define MACRONIX_MX25L12805D 0x2018 /* MX25L12805D (no 32k); MX25L12865E, MX25L12835F, MX25L12845E, MX25L12873F (32k 0x52) */ #define MACRONIX_MX25L25635F 0x2019 /* Same as MX25L25639F, but the latter seems to not support REMS */ #define MACRONIX_MX25L1635D 0x2415 #define MACRONIX_MX25L1635E 0x2515 /* MX25L1635{E} */ -- To view, visit https://review.coreboot.org/c/flashrom/+/56054 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I59c8067f15b5ceac5a2e2f8fe93431a465f17e23 Gerrit-Change-Number: 56054 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: flashrom.c: Reorder read_by_layout() to avoid forward decl
by Edward O'Callaghan (Code Review) 14 Jul '21

14 Jul '21

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/56297 ) Change subject: flashrom.c: Reorder read_by_layout() to avoid forward decl ...................................................................... flashrom.c: Reorder read_by_layout() to avoid forward decl Help make groking flashrom.c fractionaly easier. BUG=none BRANCH=none TEST=builds Change-Id: Ifd6c152e3a1d84b59a876997e543127387f24d40 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M flashrom.c 1 file changed, 26 insertions(+), 27 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/97/56297/1 diff --git a/flashrom.c b/flashrom.c index 8d44c3d..debe60c 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1047,7 +1047,32 @@ return 0; } -static int read_by_layout(struct flashctx *, uint8_t *); +/** + * @brief Reads the included layout regions into a buffer. + * + * If there is no layout set in the given flash context, the whole chip will + * be read. + * + * @param flashctx Flash context to be used. + * @param buffer Buffer of full chip size to read into. + * @return 0 on success, + * 1 if any read fails. + */ +static int read_by_layout(struct flashctx *const flashctx, uint8_t *const buffer) +{ + const struct flashrom_layout *const layout = get_layout(flashctx); + const struct romentry *entry = NULL; + + while ((entry = layout_next_included(layout, entry))) { + const chipoff_t region_start = entry->start; + const chipsize_t region_len = entry->end - entry->start + 1; + + if (flashctx->chip->read(flashctx, buffer + region_start, region_start, region_len)) + return 1; + } + return 0; +} + int read_flash_to_file(struct flashctx *flash, const char *filename) { unsigned long size = flash->chip->total_size * 1024; @@ -1148,32 +1173,6 @@ return ret; } -/** - * @brief Reads the included layout regions into a buffer. - * - * If there is no layout set in the given flash context, the whole chip will - * be read. - * - * @param flashctx Flash context to be used. - * @param buffer Buffer of full chip size to read into. - * @return 0 on success, - * 1 if any read fails. - */ -static int read_by_layout(struct flashctx *const flashctx, uint8_t *const buffer) -{ - const struct flashrom_layout *const layout = get_layout(flashctx); - const struct romentry *entry = NULL; - - while ((entry = layout_next_included(layout, entry))) { - const chipoff_t region_start = entry->start; - const chipsize_t region_len = entry->end - entry->start + 1; - - if (flashctx->chip->read(flashctx, buffer + region_start, region_start, region_len)) - return 1; - } - return 0; -} - typedef int (*erasefn_t)(struct flashctx *, unsigned int addr, unsigned int len); /** * @private -- To view, visit https://review.coreboot.org/c/flashrom/+/56297 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ifd6c152e3a1d84b59a876997e543127387f24d40 Gerrit-Change-Number: 56297 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: flashrom.c: Reorder check_block_eraser() to avoid forward decl
by Edward O'Callaghan (Code Review) 14 Jul '21

14 Jul '21

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Angel Pons, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/56296 to look at the new patch set (#2). Change subject: flashrom.c: Reorder check_block_eraser() to avoid forward decl ...................................................................... flashrom.c: Reorder check_block_eraser() to avoid forward decl Help make groking flashrom.c fractionaly easier. BUG=none BRANCH=none TEST=builds Change-Id: I0906a6e581ce5135b58f6acc6339908dfa770a59 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M flashrom.c 1 file changed, 25 insertions(+), 27 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/96/56296/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/56296 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0906a6e581ce5135b58f6acc6339908dfa770a59 Gerrit-Change-Number: 56296 Gerrit-PatchSet: 2 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit July 2021