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

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

02 Mar '23

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk, Alexander Goncharov. Thomas Heijligen 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 2: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/73361/comment/da546866_923be864 PS1, Line 9: 0.63.2 > Typo. https://packages.debian.org/bullseye/meson says about meson 0.56. […] Thanks for spotting. https://review.coreboot.org/c/flashrom/+/73361/comment/726b5c92_98124394 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? In `doc/meson.build` where sphinx is called as custom_target. Originally I used `FLASHROM_VERSION=meson.project_version()` in the environment of the custom_target and `os.getenv('FLASHROM_VERSION')` in `doc/conf.py` (the configuration of sphinx) to pass the version information. Now I pass `-Drelease=meson.project_version()` as parameter of the `sphinx` call, which directly writes the `release` variable in sphinx. From `sphinx-build`: ``` -D setting=value override a setting in configuration file ``` Is it now better to understand? Should I put this comment also in the commit message? -- 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: 2 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Thu, 02 Mar 2023 11:37:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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

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

02 Mar '23

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan. Hello build bot (Jenkins), Edward O'Callaghan, Anastasia Klimchuk, Alexander Goncharov, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/73361 to look at the new patch set (#2). Change subject: meson: revert to meson version 0.53.0 ...................................................................... meson: revert to meson version 0.53.0 Debian Bullseye, current stable release, ships only with meson 0.56.2. Pass arguments to `sphinx-doc` via `'-Dkey=value` instead of environment variables to stay compatible with older meson version. Change-Id: Iff9b8307c741a247a652cf666935c9485fa493fa Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M doc/conf.py M doc/meson.build M meson.build 3 files changed, 17 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/61/73361/2 -- 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: 2 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-MessageType: newpatchset

1 0

[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, Anastasia Klimchuk. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73039 ) Change subject: flashrom: rewrite flashbuses_to_text() ...................................................................... Patch Set 5: (3 comments) File flashrom.c: https://review.coreboot.org/c/flashrom/+/73039/comment/af0ed7f2_7316e0a5 PS4, Line 918: Get > You can leave "return" in this case, I think return sounds good here. Done https://review.coreboot.org/c/flashrom/+/73039/comment/0ceb87d6_6b4fcc95 PS4, Line 924: char *ret, *ptr; > I am going to ask silly question: why do you need both? […] Hmm... `strcat_realloc()` may return NULL. In such case `ret = strcat_realloc(ret, ...);` we will lose the pointer to the allocated space => memory leak https://review.coreboot.org/c/flashrom/+/73039/comment/df5da46d_f8e86b9f 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, […] You're definitely right, your suggestion will simplify everything, thanks! Done. -- 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 02 Mar 2023 10:39:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[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

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2023