March 2023 - flashrom-gerrit - mail.coreboot.org

[M] Change in flashrom[master]: flashrom: rewrite flashbuses_to_text()
by Alexander Goncharov (Code Review) 02 Mar '23

02 Mar '23

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Alexander Goncharov. Hello Felix Singer, build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/73039 to look at the new patch set (#5). Change subject: flashrom: rewrite flashbuses_to_text() ...................................................................... flashrom: rewrite flashbuses_to_text() The previous implementation had no error handling, as a result the flashrom could crash if the computer ran out of memory. The new version returns NULL in such cases. Also rewrite lots of `if` conditions to one cycle, store a name of buses in an array. The caller always expected a non-null value, so change its behavior to handle possible null value. As far as `printf()` and `free()` can handle null pointers, do nothing with such callers. Change-Id: I59b9044c99b4ba6c00d8c97f1e91af09d70dce2c Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> Ticket: https://ticket.coreboot.org/issues/408 --- M flashrom.c M print.c 2 files changed, 67 insertions(+), 19 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/39/73039/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/73039 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I59b9044c99b4ba6c00d8c97f1e91af09d70dce2c Gerrit-Change-Number: 73039 Gerrit-PatchSet: 5 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: meson: revert to meson version 0.53.0
by Anastasia Klimchuk (Code Review) 02 Mar '23

02 Mar '23

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73361 ) Change subject: meson: revert to meson version 0.53.0 ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/73361/comment/41b572dd_4ec7c91a PS1, Line 10: Pass arguments to `sphinx-doc` via `'-Dkey=value` I feel that I am missing something, but where this is in the patch? -- To view, visit https://review.coreboot.org/c/flashrom/+/73361 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff9b8307c741a247a652cf666935c9485fa493fa Gerrit-Change-Number: 73361 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: 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-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Thu, 02 Mar 2023 10:33:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: meson: revert to meson version 0.53.0
by Alexander Goncharov (Code Review) 02 Mar '23

02 Mar '23

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73361 ) Change subject: meson: revert to meson version 0.53.0 ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/73361/comment/4877f9c6_99ae5ed0 PS1, Line 9: 0.63.2 Typo. https://packages.debian.org/bullseye/meson says about meson 0.56.2-1 -- To view, visit https://review.coreboot.org/c/flashrom/+/73361 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff9b8307c741a247a652cf666935c9485fa493fa Gerrit-Change-Number: 73361 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: 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-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 02 Mar 2023 10:20:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tree/: Drop default_spi_probe_opcode for NULL case
by Anastasia Klimchuk (Code Review) 02 Mar '23

02 Mar '23

Attention is currently required from: Edward O'Callaghan, Arthur Heymans, Aarya, Nicholas Chin. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70264 ) Change subject: tree/: Drop default_spi_probe_opcode for NULL case ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/70264 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id502c5d2596ad1db52faf05723083620e4c52c12 Gerrit-Change-Number: 70264 Gerrit-PatchSet: 6 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Thu, 02 Mar 2023 09:32:54 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: spi: Make default cmd helpers static internal
by Anastasia Klimchuk (Code Review) 02 Mar '23

02 Mar '23

Attention is currently required from: Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73337 ) Change subject: spi: Make default cmd helpers static internal ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/73337 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4ef95846c2f005cf4aa727f31548c6877d2d4801 Gerrit-Change-Number: 73337 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Thu, 02 Mar 2023 09:09:00 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: ch341a_spi: Refactor singleton states into reentrant pattern
by Anastasia Klimchuk (Code Review) 02 Mar '23

02 Mar '23

Attention is currently required from: Alexander Goncharov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72807 ) Change subject: ch341a_spi: Refactor singleton states into reentrant pattern ...................................................................... Patch Set 3: Code-Review+2 (2 comments) File ch341a_spi.c: https://review.coreboot.org/c/flashrom/+/72807/comment/856554d4_e8aff083 PS3, Line 80: struct libusb_transfer *transfer_out; : struct libusb_transfer *transfer_ins[USB_IN_TRANSFERS]; : : /* Accumulate delays to be plucked between CS deassertion and CS assertions. */ : unsigned int stored_delay_us; : : struct libusb_device_handle *handle; > Yes, they will. […] Done https://review.coreboot.org/c/flashrom/+/72807/comment/de6e6644_9abfe871 PS3, Line 461: msg_pwarn("Cannot detach the existing USB driver. Claiming the interface may fail. %s\n", : libusb_error_name(ret)); > This is warn branch. […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/72807 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9fe72bff88b7f8778c1199bdab8ba43bf32e1c8c Gerrit-Change-Number: 72807 Gerrit-PatchSet: 3 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-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Thu, 02 Mar 2023 08:56:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: wbsio_spi.c: Convert to direct driver entry
by Thomas Heijligen (Code Review) 02 Mar '23

02 Mar '23

Attention is currently required from: Edward O'Callaghan. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72864 ) Change subject: wbsio_spi.c: Convert to direct driver entry ...................................................................... Patch Set 2: (3 comments) Patchset: PS2: The entries in `test_build.sh` are missing File wbsio_spi.c: https://review.coreboot.org/c/flashrom/+/72864/comment/7a78cf2b_9c56a2f4 PS2, Line 240: msg_perr("There is currently no way to determine if the programmer works on a board. " This programmer is known to work only on the 'Intel D201GLY' mainboard. Set 'allow_brick=yes` as programmer parameter if you are sure you know what you are doing. https://review.coreboot.org/c/flashrom/+/72864/comment/3082d854_cc56be22 PS2, Line 265: const struct programmer_entry programmer_wbsio_spi = { ``devs.(note|note)`` is not allowed to be `NULL` -- To view, visit https://review.coreboot.org/c/flashrom/+/72864 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I54b166048089bc502b99a345c02e91894590894e Gerrit-Change-Number: 72864 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Thu, 02 Mar 2023 08:53:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: dummyflasher: Add basic WP support for opaque VARIABLE_SIZE chip
by Thomas Heijligen (Code Review) 02 Mar '23

02 Mar '23

Thomas Heijligen has submitted this change. ( https://review.coreboot.org/c/flashrom/+/73289 ) ( 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: dummyflasher: Add basic WP support for opaque VARIABLE_SIZE chip ...................................................................... dummyflasher: Add basic WP support for opaque VARIABLE_SIZE chip Since VARIABLE_SIZE emulated chips do not correspond to actual flash chip models, no active protection modes are supported: - read_wp_cfg always returns mode=disabled,range=0,0 - write_wp_cfg only accepts mode=disabled,range=0,0 However this is sufficient to support use cases where the user just needs to verify that write protection is not enabled, as is the case in some futility unit tests. BUG=b:238694831,b:260531154 BRANCH=none TEST=none Change-Id: I4348e0175b8c743365904f5e61fdb69e3f4f4db5 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/73289 Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Thomas Heijligen <src(a)posteo.de> Tested-by: Edward O'Callaghan <quasisec(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M dummyflasher.c 1 file changed, 58 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Verified; Looks good to me, approved Thomas Heijligen: Looks good to me, but someone else must approve diff --git a/dummyflasher.c b/dummyflasher.c index a68889b..fa43bcd 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -932,6 +932,33 @@ { } +static enum flashrom_wp_result dummy_wp_read_cfg(struct flashrom_wp_cfg *cfg, struct flashctx *flash) +{ + cfg->mode = FLASHROM_WP_MODE_DISABLED; + cfg->range.start = 0; + cfg->range.len = 0; + + return FLASHROM_WP_OK; +} + +static enum flashrom_wp_result dummy_wp_write_cfg(struct flashctx *flash, const struct flashrom_wp_cfg *cfg) +{ + if (cfg->mode != FLASHROM_WP_MODE_DISABLED) + return FLASHROM_WP_ERR_MODE_UNSUPPORTED; + + if (cfg->range.start != 0 || cfg->range.len != 0) + return FLASHROM_WP_ERR_RANGE_UNSUPPORTED; + + return FLASHROM_WP_OK; +} + +static enum flashrom_wp_result dummy_wp_get_available_ranges(struct flashrom_wp_ranges **list, struct flashctx *flash) +{ + /* Not supported */ + return FLASHROM_WP_ERR_RANGE_LIST_UNAVAILABLE; +} + + static const struct spi_master spi_master_dummyflasher = { .map_flash_region = dummy_map, .unmap_flash_region = dummy_unmap, @@ -968,6 +995,9 @@ .erase = dummy_opaque_erase, .shutdown = dummy_shutdown, .delay = dummy_nop_delay, + .wp_read_cfg = dummy_wp_read_cfg, + .wp_write_cfg = dummy_wp_write_cfg, + .wp_get_ranges = dummy_wp_get_available_ranges, }; static int init_data(const struct programmer_cfg *cfg, -- To view, visit https://review.coreboot.org/c/flashrom/+/73289 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4348e0175b8c743365904f5e61fdb69e3f4f4db5 Gerrit-Change-Number: 73289 Gerrit-PatchSet: 5 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> 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]: flashrom: rewrite flashbuses_to_text()
by Anastasia Klimchuk (Code Review) 02 Mar '23

02 Mar '23

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Alexander Goncharov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73039 ) Change subject: flashrom: rewrite flashbuses_to_text() ...................................................................... Patch Set 4: (4 comments) File flashrom.c: https://review.coreboot.org/c/flashrom/+/73039/comment/0b55dc0d_59cfeab5 PS4, Line 918: Get You can leave "return" in this case, I think return sounds good here. https://review.coreboot.org/c/flashrom/+/73039/comment/a8ffe165_edecbaa7 PS4, Line 924: char *ret, *ptr; I am going to ask silly question: why do you need both? If there is a reason, I think it can be a comment. https://review.coreboot.org/c/flashrom/+/73039/comment/d4482b8d_7964f041 PS4, Line 935: Once all chipsets and flash chips have been updated An open question to everyone who reads this: what does it even mean? :) Specifically, I am wondering what should happen so that this fixme can be fixed. https://review.coreboot.org/c/flashrom/+/73039/comment/fae9aaca_19543361 PS4, Line 938: if (bustype == BUS_NONSPI) : return strdup("Non-SPI"); : if (bustype == BUS_NONE) : return strdup(bustypes[bustype]); These first two cases (BUS_NONSPI and BUS_NONE) are specially handled in the code, I understand why, but what looks a bit odd is that the former is hard-coded, the latter is taken from an array. Why the difference? Do you need BUS_NONE is the array? Maybe make it hard-coded too, and remove from array? -- To view, visit https://review.coreboot.org/c/flashrom/+/73039 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I59b9044c99b4ba6c00d8c97f1e91af09d70dce2c Gerrit-Change-Number: 73039 Gerrit-PatchSet: 4 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Thu, 02 Mar 2023 08:34:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashchips: Mark XMC XM25QH256C as TEST_OK_PR
by Anastasia Klimchuk (Code Review) 02 Mar '23

02 Mar '23

Attention is currently required from: Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73362 ) Change subject: flashchips: Mark XMC XM25QH256C as TEST_OK_PR ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/73362 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I68d0315f7b29f27ac84374ea7cc69dca207bbacb Gerrit-Change-Number: 73362 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Thu, 02 Mar 2023 07:50:00 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2023