November 2023 - flashrom-gerrit

[S] Change in flashrom[main]: serial: Fix sp_flush_incoming for serprog TCP connections
by Anastasia Klimchuk (Code Review) 23 Nov '23

23 Nov '23

Attention is currently required from: Stanislav Ponomarev. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79112?usp=email ) Change subject: serial: Fix sp_flush_incoming for serprog TCP connections ...................................................................... Patch Set 2: (10 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/79112/comment/fb1cbbc2_c157a4b1 : PS2, Line 9: pet maybe remove "pet" :) https://review.coreboot.org/c/flashrom/+/79112/comment/d8c511d4_c19fff25 : PS2, Line 9: I was working on an During the development of https://review.coreboot.org/c/flashrom/+/79112/comment/bc61d4c4_ebd18b6a : PS2, Line 10: when i implemented implementation of https://review.coreboot.org/c/flashrom/+/79112/comment/6f375d08_fe6d1e03 : PS2, Line 11: i uppercase https://review.coreboot.org/c/flashrom/+/79112/comment/8b5d3424_7b66fe1c : PS2, Line 14: I added This patch adds https://review.coreboot.org/c/flashrom/+/79112/comment/7667ca81_fcec0ab7 : PS2, Line 18: After that TESTED= Patchset: PS2: Thank you for you patch! I added some comments. I really like your original commit message, it explains everything very well. But I added few comments on it, just because typically commit message is written is more formal style (since it stays in git history forever). Also when I got an email about this patch, it does not have display name (literally email from "Name of user not set"). I think it's probably display name or full name in Gerrit account is empty, or both. You can set it to anything, any nickname is fine. File serial.c: https://review.coreboot.org/c/flashrom/+/79112/comment/67cf5fda_70113442 : PS2, Line 380: int ret = tcflush(sp_fd, TCIFLUSH); : if (ret == 0) : return; We typically use more compact variation. I don't see this exact ret value is used again, so you can do like this: if (!tcflush(sp_fd, TCIFLUSH)) return; https://review.coreboot.org/c/flashrom/+/79112/comment/a4ad7027_d311b8db : PS2, Line 392: ret > 0 A question about return codes: `serialport_read_nonblock` can return positive or negative error codes, I am wondering does positive return code reliably indicates that no more data available to read? I just read through the comment and the code of `serialport_read_nonblock` and I can't decide on that :) If it works reliably, maybe you can add a bit longer comment, like "Positive error code indicates no more data available, which is normal situation and means flush completed successfully". https://review.coreboot.org/c/flashrom/+/79112/comment/9d204200_bfbf6988 : PS2, Line 401: return; I am thinking, maybe change slightly with less returns. It's harder to read when every second line returns. if (errno == ENOTTY) { <... serialport_read_nonblock loop ...> if (ret < 0) msg_perr("Could not flush serial port incoming buffer: read has failed"); } else { msg_perr_strerror("Could not flush serial port incoming buffer: "); } -- To view, visit https://review.coreboot.org/c/flashrom/+/79112?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I9724a2fcd4a41dede2c15f83877efa6c3b0b7fae Gerrit-Change-Number: 79112 Gerrit-PatchSet: 2 Gerrit-Owner: Stanislav Ponomarev <me(a)stasponomarev.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stanislav Ponomarev <me(a)stasponomarev.com> Gerrit-Comment-Date: Thu, 23 Nov 2023 08:22:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: Remove dependency on C23 __has_include()
by Anastasia Klimchuk (Code Review) 22 Nov '23

22 Nov '23

Attention is currently required from: Anton Samsonov, Anton Samsonov, Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77089?usp=email ) Change subject: Remove dependency on C23 __has_include() ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: > > add testing info in commit message. […] Looks good! And you already wrote it, just copy paste into commit message. There is no "golden example" of how testing info should look like - it depends on the patch, the main idea is to give this info just in case. Why I am asking in this patch: it touches different environments, and knowing which environment you tested, might be useful. No strict rules, the paragraph you wrote looks good. Thank you! -- To view, visit https://review.coreboot.org/c/flashrom/+/77089?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ic544963ffd29626ae0a21bdddb1c78850cc43ec6 Gerrit-Change-Number: 77089 Gerrit-PatchSet: 4 Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Anton Samsonov <avscomputing(a)gmail.com> Gerrit-CC: Anton Samsonov <contact-launchpad(a)zxlab.ru> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Attention: Anton Samsonov <contact-launchpad(a)zxlab.ru> Gerrit-Comment-Date: Wed, 22 Nov 2023 11:35:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Anton Samsonov <contact-launchpad(a)zxlab.ru> Gerrit-MessageType: comment

1 0

[XL] Change in flashrom[main]: Abandon Change-Id: I5d4e4f1fe72b22c9f31073b02c9b8f40cf2a45ad
by Angel Pons (Code Review) 21 Nov '23

21 Nov '23

Angel Pons has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/79156?usp=email ) Change subject: Abandon Change-Id: I5d4e4f1fe72b22c9f31073b02c9b8f40cf2a45ad ...................................................................... Abandoned Author cannot abandon for some reason, see CB:79152 instead -- To view, visit https://review.coreboot.org/c/flashrom/+/79156?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I5d4e4f1fe72b22c9f31073b02c9b8f40cf2a45ad Gerrit-Change-Number: 79156 Gerrit-PatchSet: 3 Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Anton Samsonov <contact-launchpad(a)zxlab.ru> Gerrit-MessageType: abandon

1 0

[XL] Change in flashrom[main]: Abandon Change-Id: I5d4e4f1fe72b22c9f31073b02c9b8f40cf2a45ad
by Angel Pons (Code Review) 21 Nov '23

21 Nov '23

Attention is currently required from: Anton Samsonov, Anton Samsonov, Thomas Heijligen. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79156?usp=email ) Change subject: Abandon Change-Id: I5d4e4f1fe72b22c9f31073b02c9b8f40cf2a45ad ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: > I only see "Edit" button, that's all. Huh, that's weird. Anyway, will abandon this one for you. I replied on CB:79152 on how to get Jenkins to re-run. -- To view, visit https://review.coreboot.org/c/flashrom/+/79156?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I5d4e4f1fe72b22c9f31073b02c9b8f40cf2a45ad Gerrit-Change-Number: 79156 Gerrit-PatchSet: 3 Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Anton Samsonov <contact-launchpad(a)zxlab.ru> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Attention: Anton Samsonov <contact-launchpad(a)zxlab.ru> Gerrit-Comment-Date: Tue, 21 Nov 2023 12:28:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anton Samsonov <contact-launchpad(a)zxlab.ru> Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: Abandon (Unable to trick Jenkins to re-check after adding Signed-off-...
by Angel Pons (Code Review) 21 Nov '23

21 Nov '23

Attention is currently required from: Anton Samsonov, Anton Samsonov, Thomas Heijligen. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79152?usp=email ) Change subject: Abandon (Unable to trick Jenkins to re-check after adding Signed-off-by) Change-Id: I8694e618878823a9e96b1f2bcfa63f6c71d3c2ed ...................................................................... Patch Set 3: (2 comments) Patchset: PS3: To have Jenkins re-check, you can change something in the code. File Makefile: https://review.coreboot.org/c/flashrom/+/79152/comment/f8be7cd3_d39869f5 : PS2, Line 395: ifneq ($(VERSION_GIT),) > > Does this work properly when `VERSION_GIT` is not defined? […] Ack -- To view, visit https://review.coreboot.org/c/flashrom/+/79152?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I8694e618878823a9e96b1f2bcfa63f6c71d3c2ed Gerrit-Change-Number: 79152 Gerrit-PatchSet: 3 Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Anton Samsonov <contact-launchpad(a)zxlab.ru> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Attention: Anton Samsonov <contact-launchpad(a)zxlab.ru> Gerrit-Comment-Date: Tue, 21 Nov 2023 12:27:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anton Samsonov <contact-launchpad(a)zxlab.ru> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: Remove dependency on C23 __has_include()
by Anton Samsonov (Code Review) 21 Nov '23

21 Nov '23

Attention is currently required from: Anastasia Klimchuk, Anton Samsonov, Thomas Heijligen. Anton Samsonov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77089?usp=email ) Change subject: Remove dependency on C23 __has_include() ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: > add testing info in commit message. Lacking relevant examples in git log, I can only add something like this: Tested with <getopt.h> and <pci/pci.h> using GNU Make 4.1, 4.2.1, 4.4.1 and Meson 0.56.0, 1.2.1 against GCC 13.2.1 and GCC 5.5-, 7.3-compatible (EDG 4.14-, 5.1-based) on openSuSE Tumbleweed and custom LFS distribution. Do you think such information is really worth to be included in the log? -- To view, visit https://review.coreboot.org/c/flashrom/+/77089?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ic544963ffd29626ae0a21bdddb1c78850cc43ec6 Gerrit-Change-Number: 77089 Gerrit-PatchSet: 4 Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Anton Samsonov <avscomputing(a)gmail.com> Gerrit-CC: Anton Samsonov <contact-launchpad(a)zxlab.ru> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Comment-Date: Tue, 21 Nov 2023 12:12:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: Abandon (Unable to trick Jenkins to re-check after adding Signed-off-...
by Anton Samsonov (Code Review) 21 Nov '23

21 Nov '23

Attention is currently required from: Angel Pons, Anton Samsonov, Thomas Heijligen. Anton Samsonov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79152?usp=email ) Change subject: Abandon (Unable to trick Jenkins to re-check after adding Signed-off-by) Change-Id: I8694e618878823a9e96b1f2bcfa63f6c71d3c2ed ...................................................................... Patch Set 3: (1 comment) File Makefile: https://review.coreboot.org/c/flashrom/+/79152/comment/61318b51_81d72eb6 : PS2, Line 395: ifneq ($(VERSION_GIT),) > Does this work properly when `VERSION_GIT` is not defined? The neat part is that `VERSION_GIT` is *always* defined, according to GNU Make syntax: Note that ifdef only tests whether a variable has a value. It does not expand the variable to see if that value is nonempty. Consequently, tests using ifdef return true for all definitions except those like foo =. To test for an empty value, use ifeq ($(foo),). -- To view, visit https://review.coreboot.org/c/flashrom/+/79152?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I8694e618878823a9e96b1f2bcfa63f6c71d3c2ed Gerrit-Change-Number: 79152 Gerrit-PatchSet: 3 Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Anton Samsonov <contact-launchpad(a)zxlab.ru> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Comment-Date: Tue, 21 Nov 2023 11:50:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: Abandon (Unable to trick Jenkins to re-check after adding Signed-off-...
by Anton Samsonov (Code Review) 21 Nov '23

21 Nov '23

Attention is currently required from: Anton Samsonov, Thomas Heijligen. Anton Samsonov has uploaded a new patch set (#3) to the change originally created by Anton Samsonov. ( https://review.coreboot.org/c/flashrom/+/79152?usp=email ) Change subject: Abandon (Unable to trick Jenkins to re-check after adding Signed-off-by) Change-Id: I8694e618878823a9e96b1f2bcfa63f6c71d3c2ed ...................................................................... Abandon (Unable to trick Jenkins to re-check after adding Signed-off-by) Change-Id: I8694e618878823a9e96b1f2bcfa63f6c71d3c2ed --- M Makefile 1 file changed, 3 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/52/79152/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/79152?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I8694e618878823a9e96b1f2bcfa63f6c71d3c2ed Gerrit-Change-Number: 79152 Gerrit-PatchSet: 3 Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru> Gerrit-MessageType: newpatchset

1 0

[XL] Change in flashrom[main]: Abandon Change-Id: I5d4e4f1fe72b22c9f31073b02c9b8f40cf2a45ad
by Anton Samsonov (Code Review) 21 Nov '23

21 Nov '23

Attention is currently required from: Angel Pons, Anton Samsonov, Thomas Heijligen. Anton Samsonov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79156?usp=email ) Change subject: Abandon Change-Id: I5d4e4f1fe72b22c9f31073b02c9b8f40cf2a45ad ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: > You can abandon this change yourself by clicking the "Abandon" button on the top right corner. I only see "Edit" button, that's all. -- To view, visit https://review.coreboot.org/c/flashrom/+/79156?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I5d4e4f1fe72b22c9f31073b02c9b8f40cf2a45ad Gerrit-Change-Number: 79156 Gerrit-PatchSet: 3 Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Anton Samsonov <contact-launchpad(a)zxlab.ru> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Comment-Date: Tue, 21 Nov 2023 11:28:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: Makefile: Fix version string for non-Git builds
by Angel Pons (Code Review) 21 Nov '23

21 Nov '23

Attention is currently required from: Anton Samsonov, Thomas Heijligen. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79152?usp=email ) Change subject: Makefile: Fix version string for non-Git builds ...................................................................... Patch Set 2: (1 comment) File Makefile: https://review.coreboot.org/c/flashrom/+/79152/comment/d7eed972_8759ba85 : PS2, Line 395: ifneq ($(VERSION_GIT),) Does this work properly when `VERSION_GIT` is not defined? -- To view, visit https://review.coreboot.org/c/flashrom/+/79152?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I8694e618878823a9e96b1f2bcfa63f6c71d3c2ed Gerrit-Change-Number: 79152 Gerrit-PatchSet: 2 Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Comment-Date: Tue, 21 Nov 2023 11:24:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2023