- flashrom-gerrit - mail.coreboot.org

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

24 Nov '23

Attention is currently required from: Anton Samsonov, Anton Samsonov, Thomas Heijligen. Hello Thomas Heijligen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/79152?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: Makefile: Fix version string for non-Git builds ...................................................................... Makefile: Fix version string for non-Git builds Change-Id: I8694e618878823a9e96b1f2bcfa63f6c71d3c2ed Signed-off-by: Anton Samsonov <devel(a)zxlab.ru> --- M Makefile 1 file changed, 4 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/52/79152/4 -- 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: 4 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-MessageType: newpatchset

1 0

[XS] Change in flashrom[main]: docs: Add guideline about Gerrit display names
by Stanislav Ponomarev (Code Review) 24 Nov '23

24 Nov '23

Attention is currently required from: Anastasia Klimchuk, Peter Marheine, Thomas Heijligen. Stanislav Ponomarev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79250?usp=email ) Change subject: docs: Add guideline about Gerrit display names ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Patchset: PS1: sounds clear 😊 -- To view, visit https://review.coreboot.org/c/flashrom/+/79250?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: I69b6f5c1c56a2798dd156582cb5fa246b2396ab3 Gerrit-Change-Number: 79250 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Stanislav Ponomarev <me(a)stasponomarev.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Fri, 24 Nov 2023 10:19:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

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

24 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 4: Code-Review+2 (2 comments) Patchset: PS2: > Thank you for your detailed review! […] This is not the first time I see "Name of user not set", I think this means it's time to update the documentation. I created a patch CB:79250 and added you, you can vote +1 or add comments. Thank you! File serial.c: https://review.coreboot.org/c/flashrom/+/79112/comment/49c30cfc_5fce4d9b : PS2, Line 401: return; > yeah, i'm a bit uncomfortable with this too, but i actually have no idea what to do if something els […] It looks good now. Thank you for detailed comments in the code. And FIXME is definitely is not needed anymore, we have an error handling now! -- 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: 4 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: Fri, 24 Nov 2023 08:33:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Stanislav Ponomarev <me(a)stasponomarev.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: docs: Add guideline about Gerrit display names
by Anastasia Klimchuk (Code Review) 24 Nov '23

24 Nov '23

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/79250?usp=email ) Change subject: docs: Add guideline about Gerrit display names ...................................................................... docs: Add guideline about Gerrit display names If none of "Full name" or "Display name" is set for a Gerrit account, code reviews emails are sent from "Name of user not set" instead of a user name. It is worth clarifying explicitly in the docs. Change-Id: I69b6f5c1c56a2798dd156582cb5fa246b2396ab3 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/dev_guide/development_guide.rst 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/50/79250/1 diff --git a/doc/dev_guide/development_guide.rst b/doc/dev_guide/development_guide.rst index 60c5625..247a31a 100644 --- a/doc/dev_guide/development_guide.rst +++ b/doc/dev_guide/development_guide.rst @@ -183,6 +183,8 @@ #. Add an e-mail address so that Gerrit can send notifications to you about your patch. #. Upload an SSH public key, or click the button to generate an HTTPS password. +#. After account created, set either "Full name" or "Display name", it is used by Gerrit + for code review emails. .. _pushing-a-patch: -- To view, visit https://review.coreboot.org/c/flashrom/+/79250?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: I69b6f5c1c56a2798dd156582cb5fa246b2396ab3 Gerrit-Change-Number: 79250 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

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

23 Nov '23

Attention is currently required from: Anastasia Klimchuk. Stanislav Ponomarev 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 4: (10 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/79112/comment/153babef_74cfa5ff : PS2, Line 9: I was working on an > During the development of Done https://review.coreboot.org/c/flashrom/+/79112/comment/3afdb5d1_727a4c53 : PS2, Line 9: pet > maybe remove "pet" :) Done https://review.coreboot.org/c/flashrom/+/79112/comment/09b8ad96_e0551d0a : PS2, Line 10: when i implemented > implementation of Done https://review.coreboot.org/c/flashrom/+/79112/comment/59db9e38_34f88060 : PS2, Line 11: i > uppercase Done https://review.coreboot.org/c/flashrom/+/79112/comment/a34947ea_e9af1c5b : PS2, Line 14: I added > This patch adds Done https://review.coreboot.org/c/flashrom/+/79112/comment/e195c582_de881ae5 : PS2, Line 18: After that > TESTED= Done Patchset: PS2: > Thank you for you patch! I added some comments. […] Thank you for your detailed review! Not sure what happened with the email - most likely i should have set all the names in Gerrit before sending the patch File serial.c: https://review.coreboot.org/c/flashrom/+/79112/comment/f9527939_5863938b : PS2, Line 380: int ret = tcflush(sp_fd, TCIFLUSH); : if (ret == 0) : return; > We typically use more compact variation. […] Done https://review.coreboot.org/c/flashrom/+/79112/comment/bc2ee925_41203f48 : PS2, Line 392: ret > 0 > A question about return codes: `serialport_read_nonblock` can return positive or negative error code […] Done https://review.coreboot.org/c/flashrom/+/79112/comment/615df0f6_09424ad9 : PS2, Line 401: return; > I am thinking, maybe change slightly with less returns. […] yeah, i'm a bit uncomfortable with this too, but i actually have no idea what to do if something else occurs especially since there is no return value to indicate that i considered leaving the original FIXME: error handling comment here too the only thing i'm pretty sure about - any error other than ENOTTY means sp_fd is in unrecoverable state and any following read/write operation will fail too i have made the changes you suggested and added some new comments -- 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: 4 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 23 Nov 2023 16:04:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[L] Change in flashrom[main]: tests: Unit tests for erase function selection algo
by Hsuan-ting Chen (Code Review) 23 Nov '23

23 Nov '23

Attention is currently required from: Anastasia Klimchuk. Hsuan-ting Chen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67535?usp=email ) Change subject: tests: Unit tests for erase function selection algo ...................................................................... Patch Set 22: (1 comment) File tests/erase_func_algo.c: https://review.coreboot.org/c/flashrom/+/67535/comment/02c0c605_60895505 : PS22, Line 270: 0xff > Hi Anastasia: […] Thanks for the reply in https://review.coreboot.org/c/flashrom/+/79229/comments/455533e3_7f82ee37 -- To view, visit https://review.coreboot.org/c/flashrom/+/67535?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: I8f3fdefb76e71f6f8dc295d9dead5f38642aace7 Gerrit-Change-Number: 67535 Gerrit-PatchSet: 22 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alexander Goncharov <chat(a)joursoir.net> Gerrit-CC: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 23 Nov 2023 10:58:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: erase_func_algo: Avoid putting 0xff into buffers
by Hsuan-ting Chen (Code Review) 23 Nov '23

23 Nov '23

Hsuan-ting Chen has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/79229?usp=email ) Change subject: erase_func_algo: Avoid putting 0xff into buffers ...................................................................... Abandoned Not correct -- To view, visit https://review.coreboot.org/c/flashrom/+/79229?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: Ibe159f9f7cf35688cfdbe18245fea2d870386412 Gerrit-Change-Number: 79229 Gerrit-PatchSet: 1 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: abandon

1 0

[M] Change in flashrom[main]: erase_func_algo: Avoid putting 0xff into buffers
by Hsuan-ting Chen (Code Review) 23 Nov '23

23 Nov '23

Attention is currently required from: Anastasia Klimchuk. Hsuan-ting Chen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79229?usp=email ) Change subject: erase_func_algo: Avoid putting 0xff into buffers ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > In theory, there could be more than one way to erase. […] Thanks for the detailed explaination! Yes, I found this behaviour inconsistant while porting this to chromium flashrom. I would be grateful if there's any related performance materials that could share with me. I'll abandon this change. -- To view, visit https://review.coreboot.org/c/flashrom/+/79229?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: Ibe159f9f7cf35688cfdbe18245fea2d870386412 Gerrit-Change-Number: 79229 Gerrit-PatchSet: 1 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 23 Nov 2023 10:57:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hsuan-ting Chen <roccochen(a)google.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: erase_func_algo: Avoid putting 0xff into buffers
by Anastasia Klimchuk (Code Review) 23 Nov '23

23 Nov '23

Attention is currently required from: Hsuan-ting Chen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79229?usp=email ) Change subject: erase_func_algo: Avoid putting 0xff into buffers ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > Hi Anastasia: […] In theory, there could be more than one way to erase. In practice, the test is relying on the current implementation of erase/write which does it in one exact way. If you are trying to adapt the test to chromium flashrom: last time I looked it had a different algorithm for erase/write. So don't be surprised if you see different erasers invocations for some of test cases. There was a plan for this year to merge two algos and get the best of both worlds. I measured the performance a year ago, and depending on the test case, one or other performed better. The plans did not happen, and now I guess it's up to you if you want to do that. -- To view, visit https://review.coreboot.org/c/flashrom/+/79229?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: Ibe159f9f7cf35688cfdbe18245fea2d870386412 Gerrit-Change-Number: 79229 Gerrit-PatchSet: 1 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Comment-Date: Thu, 23 Nov 2023 10:44:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hsuan-ting Chen <roccochen(a)google.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: erase_func_algo: Avoid putting 0xff into buffers
by Hsuan-ting Chen (Code Review) 23 Nov '23

23 Nov '23

Attention is currently required from: Anastasia Klimchuk. Hsuan-ting Chen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79229?usp=email ) Change subject: erase_func_algo: Avoid putting 0xff into buffers ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > Hsuan-ting, I appreciate your effort to create a patch, but I don't understand what is the issue you […] Hi Anastasia: Here's my previous note: https://review.coreboot.org/c/flashrom/+/67535/comment/5f7ed5b9_3f1e6037/ Thanks for the prompt reply. Yes, I think I am not really clear about this test. I am not very clear about the "golden example" should be. For example, ``` .initial_buf = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, .written_buf = {0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf}, ``` Should we erase all or should we aware that the initial buf is already 0xFFs so skip erasure? Please let me know if there's anything I need to study or misunderstood. -- To view, visit https://review.coreboot.org/c/flashrom/+/79229?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: Ibe159f9f7cf35688cfdbe18245fea2d870386412 Gerrit-Change-Number: 79229 Gerrit-PatchSet: 1 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 23 Nov 2023 10:07:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit