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

Change in flashrom[master]: programmer.h,chipset_enable.c: Make penable cb more descript
by Anastasia Klimchuk (Code Review) 27 Jul '21

27 Jul '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52364 ) Change subject: programmer.h,chipset_enable.c: Make penable cb more descript ...................................................................... Patch Set 3: Code-Review+2 (1 comment) Patchset: PS3: I apologise, this patch has been forgotten it seems. Looking at the latest version, I think renaming doit() into enable_flash() makes code easier to read. -- To view, visit https://review.coreboot.org/c/flashrom/+/52364 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8b3a4ec51ea488c40c20ccafea883f8dea93577d Gerrit-Change-Number: 52364 Gerrit-PatchSet: 3 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: 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 27 Jul 2021 07:08:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add init-shutdown test for nicrealtek
by Anastasia Klimchuk (Code Review) 27 Jul '21

27 Jul '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56470 ) Change subject: tests: Add init-shutdown test for nicrealtek ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: I rebased this chain to the latest -- To view, visit https://review.coreboot.org/c/flashrom/+/56470 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iaed14fe1d83c8eb45ec185ebd3f1c97cb81941f4 Gerrit-Change-Number: 56470 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 27 Jul 2021 05:54:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add tests to erase a chip
by Anastasia Klimchuk (Code Review) 27 Jul '21

27 Jul '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56501 ) Change subject: tests: Add tests to erase a chip ...................................................................... Patch Set 4: (2 comments) File tests/chip.c: https://review.coreboot.org/c/flashrom/+/56501/comment/6d59786e_a1590474 PS3, Line 26: chip_state > g_chip_state Done https://review.coreboot.org/c/flashrom/+/56501/comment/6ad66b93_689b209b PS3, Line 37: /* : * Populate buf with the value used for erase operation, this allows to pass : * verification checks and also emulates successful erase. : */ : buf = memset(buf, 0xff, len); > this should be coming from the chip state now Done. I am sorry I misunderstood your comment before. Thought it was done after adding second test which uses dummy's emulation capabilities, because dummy keeps state etc. But of course, improving first test is a good idea. Thank you for helping me with this! -- To view, visit https://review.coreboot.org/c/flashrom/+/56501 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6f74bfe4e02244d24d6c837cc3d551251e7b4898 Gerrit-Change-Number: 56501 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 27 Jul 2021 04:27: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]: tests: Add tests to erase a chip
by Anastasia Klimchuk (Code Review) 27 Jul '21

27 Jul '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/56501 to look at the new patch set (#4). Change subject: tests: Add tests to erase a chip ...................................................................... tests: Add tests to erase a chip Two tests cover the code which parforms do_erase operation. First one works with fake chip and dummy programmer. Fake chip has all operations defined, they all log and return 0. Read operation always initialises buffer with one and the same value. Second one uses the chip which is closer to the real one, because read/write/unlock/erase operations are real. The tests takes the advantage of dummyflasher's capability of emulating a W25Q128.V chip. BUG=b:181803212 TEST=builds and ninja test Change-Id: I6f74bfe4e02244d24d6c837cc3d551251e7b4898 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M meson.build A tests/chip.c M tests/meson.build M tests/tests.c M tests/tests.h A tests/wrap_send_tests.c A tests/wrap_send_tests.h 7 files changed, 332 insertions(+), 28 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/01/56501/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/56501 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6f74bfe4e02244d24d6c837cc3d551251e7b4898 Gerrit-Change-Number: 56501 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: flashchips: Add MX25L12833F
by Tao Xia (Code Review) 27 Jul '21

27 Jul '21

Attention is currently required from: Nico Huber, Henry Sun, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev, Karthik Ramasubramanian. Tao Xia has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56546 ) Change subject: flashchips: Add MX25L12833F ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > Haven't looked at the datasheet. I have uploaded the datasheet in the issue https://partnerissuetracker.corp.google.com/issues/190574697 -- To view, visit https://review.coreboot.org/c/flashrom/+/56546 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I14ab7e04f5209d2bcf34b0d2de9da2c01bf32d00 Gerrit-Change-Number: 56546 Gerrit-PatchSet: 1 Gerrit-Owner: Tao Xia <xiatao5(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Henry Sun <henrysun(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Ginger Zhang <zhangqingchun(a)huaqin.corp-partner.google.com> Gerrit-CC: Krystal Han <hanlijing(a)huaqin.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Weimin Wu <wuweimin(a)huaqin.corp-partner.google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Henry Sun <henrysun(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Tue, 27 Jul 2021 03:30:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer.h: Minor tidy up
by Edward O'Callaghan (Code Review) 27 Jul '21

27 Jul '21

Attention is currently required from: Nico Huber. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56466 ) Change subject: programmer.h: Minor tidy up ...................................................................... Patch Set 2: (2 comments) Patchset: PS2: > Do we need any of the #ifdef at all? Very likely not in most cases. I rather we just let the linker do its job which I think you are suggesting here? Do we want to tear them out in this same patch as some pre-process is needed here? File programmer.h: https://review.coreboot.org/c/flashrom/+/56466/comment/1012c8d0_91fa4f85 PS2, Line 477: remove extra \n -- To view, visit https://review.coreboot.org/c/flashrom/+/56466 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7ceda32b66d17318d42b94a1fab621a9a926fa77 Gerrit-Change-Number: 56466 Gerrit-PatchSet: 2 Gerrit-Owner: 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Tue, 27 Jul 2021 02:37:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add tests to erase a chip
by Edward O'Callaghan (Code Review) 27 Jul '21

27 Jul '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56501 ) Change subject: tests: Add tests to erase a chip ...................................................................... Patch Set 3: (2 comments) File tests/chip.c: https://review.coreboot.org/c/flashrom/+/56501/comment/36d9ef4b_32b325ed PS3, Line 26: chip_state g_chip_state https://review.coreboot.org/c/flashrom/+/56501/comment/a58b6776_86b87f03 PS3, Line 37: /* : * Populate buf with the value used for erase operation, this allows to pass : * verification checks and also emulates successful erase. : */ : buf = memset(buf, 0xff, len); this should be coming from the chip state now -- To view, visit https://review.coreboot.org/c/flashrom/+/56501 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6f74bfe4e02244d24d6c837cc3d551251e7b4898 Gerrit-Change-Number: 56501 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 27 Jul 2021 02:31:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: treewide: Use calloc() where applicable
by Anastasia Klimchuk (Code Review) 27 Jul '21

27 Jul '21

Attention is currently required from: Miklós Márton, Edward O'Callaghan, Angel Pons, Arthur Heymans. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55269 ) Change subject: treewide: Use calloc() where applicable ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: Since I am one of reviewers here, what are our plans for this patch? I thought it's all good, but I read all Nico's comments and think that there can be more caveats than I can see... -- To view, visit https://review.coreboot.org/c/flashrom/+/55269 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id6ef0d6d02c7b63baabc9a0087275cab22a48736 Gerrit-Change-Number: 55269 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(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-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Tue, 27 Jul 2021 01:30:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add tests to erase a chip
by Anastasia Klimchuk (Code Review) 27 Jul '21

27 Jul '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56501 ) Change subject: tests: Add tests to erase a chip ...................................................................... Patch Set 3: (2 comments) File tests/chip.c: https://review.coreboot.org/c/flashrom/+/56501/comment/8973261b_f6f7777a PS1, Line 42: printf("Unlock chip called\n"); > If you're emulating a chip, I'd also mock and verify locking to increase test coverage I added chip state which counts how many times unlock was called, and then read/write/erase check that unlock has been called. This is all added for fake chip test. https://review.coreboot.org/c/flashrom/+/56501/comment/17177d48_13e309c9 PS1, Line 57: char *param_dup = strdup(""); > Hmmm, I guess the `memmove` inside that function changes the haystack. If so... […] Yes, from my memory it is memmove. I remember spending a while on this when I was writing the very first test, memmove did segfault because it was trying to modify read-only memory. Eventually I added strdup to workaround that. -- To view, visit https://review.coreboot.org/c/flashrom/+/56501 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6f74bfe4e02244d24d6c837cc3d551251e7b4898 Gerrit-Change-Number: 56501 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 27 Jul 2021 01:03:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add tests to erase a chip
by Anastasia Klimchuk (Code Review) 27 Jul '21

27 Jul '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/56501 to look at the new patch set (#3). Change subject: tests: Add tests to erase a chip ...................................................................... tests: Add tests to erase a chip Two tests cover the code which parforms do_erase operation. First one works with fake chip and dummy programmer. Fake chip has all operations defined, they all log and return 0. Read operation always initialises buffer with one and the same value. Second one uses the chip which is closer to the real one, because read/write/unlock/erase operations are real. The tests takes the advantage of dummyflasher's capability of emulating a W25Q128.V chip. BUG=b:181803212 TEST=builds and ninja test Change-Id: I6f74bfe4e02244d24d6c837cc3d551251e7b4898 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M meson.build A tests/chip.c M tests/meson.build M tests/tests.c M tests/tests.h A tests/wrap_send_tests.c A tests/wrap_send_tests.h 7 files changed, 329 insertions(+), 28 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/01/56501/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/56501 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6f74bfe4e02244d24d6c837cc3d551251e7b4898 Gerrit-Change-Number: 56501 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> 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