November 2022 - flashrom-gerrit

[S] Change in flashrom[1.3.x]: linux_mtd: Mark Opaque chip as tested for WP
by Anastasia Klimchuk (Code Review) 28 Nov '22

28 Nov '22

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Nikolai Artemiev. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69996 ) Change subject: linux_mtd: Mark Opaque chip as tested for WP ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/69996 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.3.x Gerrit-Change-Id: Icc0521c28555a93f26ce66bdbeaa68590f10c358 Gerrit-Change-Number: 69996 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 28 Nov 2022 01:18:18 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[1.3.x]: cli_classic.c: Be consistent with pointer types
by Anastasia Klimchuk (Code Review) 28 Nov '22

28 Nov '22

Attention is currently required from: Felix Singer, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69995 ) Change subject: cli_classic.c: Be consistent with pointer types ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/69995 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.3.x Gerrit-Change-Id: Ie8f5bc0d9296f7c6b8f8a351b53052f5fe86b09d Gerrit-Change-Number: 69995 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 28 Nov 2022 01:18:12 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tests: Add selfcheck to unit tests
by Anastasia Klimchuk (Code Review) 28 Nov '22

28 Nov '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Evan Benn. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69620 ) Change subject: tests: Add selfcheck to unit tests ...................................................................... Patch Set 7: (15 comments) File tests/selfcheck.c: https://review.coreboot.org/c/flashrom/+/69620/comment/d6d7adef_7cdced17 PS7, Line 40: programmer row programmer entry https://review.coreboot.org/c/flashrom/+/69620/comment/ba1faf3e_3c1c2f79 PS7, Line 68: assert_table(chip->bustype != BUS_NONE, "chip bustype is BUS_NONE", i, : chip->name); make one line https://review.coreboot.org/c/flashrom/+/69620/comment/01f43ced_4c329c3f PS7, Line 74: { You can add `(void)state; /* unused */` in the beginning here. And don't forget new line after :) https://review.coreboot.org/c/flashrom/+/69620/comment/a4882238_416078f2 PS7, Line 87: /* Blocks with zero size are bugs in flashchips.c. */ : /* Blocks with zero size are bugs in flashchips.c. */ Same line repeats twice? https://review.coreboot.org/c/flashrom/+/69620/comment/a4e9c405_ed9b7c65 PS7, Line 89: if (eraser.eraseblocks[i].count : && !eraser.eraseblocks[i].size) { make if condition on one line https://review.coreboot.org/c/flashrom/+/69620/comment/82a7942d_3a554945 PS7, Line 91: fail_msg( : "Flash chip %s erase function " : "%zu region %zu has size 0", make one line (out of 91-93) https://review.coreboot.org/c/flashrom/+/69620/comment/7b901ec3_fce91ab2 PS7, Line 97: if (!eraser.eraseblocks[i].count : && eraser.eraseblocks[i].size) { make if condition on one line https://review.coreboot.org/c/flashrom/+/69620/comment/2db58e2e_deba01a6 PS7, Line 99: fail_msg( : "Flash chip %s erase function " : "%zu region %zu has count 0", make one line (out of 99-101) https://review.coreboot.org/c/flashrom/+/69620/comment/2858275b_5457302b PS7, Line 135: remove empty new line https://review.coreboot.org/c/flashrom/+/69620/comment/644ebc9e_bc2215fa PS7, Line 136: fail_msg( : "Flash chip %s erase function " : "%zu and %zu are identical.", make one line (out of 136-138) https://review.coreboot.org/c/flashrom/+/69620/comment/0b4f067d_59c49825 PS7, Line 143: fail_msg( : "Flash chip %s erase function %zu is not " : "in order", make one line (out of 143-145) https://review.coreboot.org/c/flashrom/+/69620/comment/0721dbe9_6cd0eb7c PS7, Line 170: fail_msg("Board enable for %s %s is misdefined.\n", b->vendor_name, : b->board_name); make one line File tests/tests.h: https://review.coreboot.org/c/flashrom/+/69620/comment/bdad05c0_58858fc3 PS7, Line 90: _tables The file name is `selfcheck.c` without `_tables` suffix File tests/well_formed_tables.c: https://review.coreboot.org/c/flashrom/+/69620/comment/a9e6ae8b_7edd2282 PS6, Line 24: do { \ : if (!(assertion)) \ : fail_msg(#assertion " is false for index:%zu name:%s", (index), \ : (name) ? (name) : "unknown"); \ : } while (0) > I agree that would be the preferred style but a macro is used so that the __LINE__ format used by fa […] Having correct __LINE__ is important, helps a lot with debugging tests. So I am happy to keep as it is. Felix would you agree? https://review.coreboot.org/c/flashrom/+/69620/comment/12ce4e84_8a724dc2 PS6, Line 83: SKIP_TEST > Done not done :P -- To view, visit https://review.coreboot.org/c/flashrom/+/69620 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I41cd014d9bf909296b6c28e3e00548e6883ff41a Gerrit-Change-Number: 69620 Gerrit-PatchSet: 7 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Mon, 28 Nov 2022 00:40:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: internal.c: laptop_ok global state can become stale
by Sam McNally (Code Review) 28 Nov '22

28 Nov '22

Attention is currently required from: Edward O'Callaghan. Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70026 ) Change subject: internal.c: laptop_ok global state can become stale ...................................................................... Patch Set 1: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/70026/comment/373be7cd_708db168 PS1, Line 30: BUG=b:260518132,b:260518132 These are both the same. -- To view, visit https://review.coreboot.org/c/flashrom/+/70026 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2c00c351904307eeb1488c5dfaffc91d6468ee25 Gerrit-Change-Number: 70026 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 28 Nov 2022 00:37:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: bindings: Add FLASHROM_WP_ERR_UNSUPPORTED_STATE
by Evan Benn (Code Review) 28 Nov '22

28 Nov '22

Attention is currently required from: Edward O'Callaghan. Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69864 ) Change subject: bindings: Add FLASHROM_WP_ERR_UNSUPPORTED_STATE ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: please submit when able, thanks -- To view, visit https://review.coreboot.org/c/flashrom/+/69864 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2adaad6552a601147e779c85a2a51d2e39b91c28 Gerrit-Change-Number: 69864 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 28 Nov 2022 00:25:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: internal.c: laptop_ok global state can become stale
by Edward O'Callaghan (Code Review) 28 Nov '22

28 Nov '22

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/70026 ) Change subject: internal.c: laptop_ok global state can become stale ...................................................................... internal.c: laptop_ok global state can become stale Craask and similar DUT's are erroneously probing random second chips. ``` Found chipset "Intel Alder Lake-N". Enabling flash write... SPI Configuration is locked down. FREG0: Flash Descriptor region (0x00000000-0x00000fff) is read-write. FREG1: BIOS region (0x003a0000-0x00ffffff) is read-write. FREG2: Management Engine region (0x00001000-0x0039ffff) is read-write. OK. Found Winbond flash chip "W25Q128.V..M" (16384 kB, Programmer-specific) on host. Warning: Setting BIOS Control at 0xdc from 0x8b to 0x89 failed. New value is 0x8b. Found MoselVitelic flash chip "V29C51000T" (64 kB, Parallel) mapped at physical address 0x00000000ffff0000. ``` This seems to be due to `laptop_ok` becoming a stale global state after the first operation leading to probing on unrelated buses. Therefore unconditionally reset the global state upon entry into the internal driver. BUG=b:260518132,b:260518132 TEST=Craask reportly no longer finds duplicate chip. Change-Id: I2c00c351904307eeb1488c5dfaffc91d6468ee25 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M internal.c 1 file changed, 37 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/26/70026/1 diff --git a/internal.c b/internal.c index 4f68456..9b80f37 100644 --- a/internal.c +++ b/internal.c @@ -211,6 +211,9 @@ if (ret) return ret; + /* Unconditionally reset global state from previous operation. */ + laptop_ok = false; + /* Default to Parallel/LPC/FWH flash devices. If a known host controller * is found, the host controller init routine sets the * internal_buses_supported bitfield. -- To view, visit https://review.coreboot.org/c/flashrom/+/70026 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2c00c351904307eeb1488c5dfaffc91d6468ee25 Gerrit-Change-Number: 70026 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: cli, manpage: Clean up occurrences of old image argument
by Anastasia Klimchuk (Code Review) 27 Nov '22

27 Nov '22

Attention is currently required from: Felix Singer, Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70003 ) Change subject: cli, manpage: Clean up occurrences of old image argument ...................................................................... Patch Set 3: (2 comments) File cli_classic.c: https://review.coreboot.org/c/flashrom/+/70003/comment/5e636031_8fd240f2 PS1, Line 621: include > Oh no... did I just do the blasphemy of changing the public API! how could I... %( […] Done File cli_classic.c: https://review.coreboot.org/c/flashrom/+/70003/comment/8c68c31e_beb280ae PS2, Line 622: {"image", 1, NULL, 'i'}, > Add a comment here that this is deprecated Done -- To view, visit https://review.coreboot.org/c/flashrom/+/70003 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If457e2b8548034868105b515125d7e8b4d5f6134 Gerrit-Change-Number: 70003 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Sun, 27 Nov 2022 23:57:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: cli, manpage: Clean up occurrences of old image argument
by Anastasia Klimchuk (Code Review) 27 Nov '22

27 Nov '22

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. 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/+/70003 to look at the new patch set (#3). Change subject: cli, manpage: Clean up occurrences of old image argument ...................................................................... cli, manpage: Clean up occurrences of old image argument --include argument was introduced and replaced --image argument in commit 45d50a101e8073191e6d88143990ed91d3bfe815 This patch cleans up remaining few places where old `--image` argument was mentioned so that now all the documentation has `--include`. --image is deprecated. Both old --image and new --include have the same short version -i and it remains the same. The code remains the same since the code handles --include already. Tested by running flashrom -h man ./flashrom.8.tmpl Ticket: https://ticket.coreboot.org/issues/372 Change-Id: If457e2b8548034868105b515125d7e8b4d5f6134 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M cli_classic.c M flashrom.8.tmpl 2 files changed, 32 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/03/70003/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/70003 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If457e2b8548034868105b515125d7e8b4d5f6134 Gerrit-Change-Number: 70003 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: Felix Singer <felixsinger(a)posteo.net> 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-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: flashchips.c: Add 4BA write to XM25Qx256C
by Anastasia Klimchuk (Code Review) 27 Nov '22

27 Nov '22

Attention is currently required from: Liam Flaherty, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69713 ) Change subject: flashchips.c: Add 4BA write to XM25Qx256C ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/69713 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I96c80762fcda2af6028c7a53d8c545b0c6565cbd Gerrit-Change-Number: 69713 Gerrit-PatchSet: 4 Gerrit-Owner: Liam Flaherty <liamflaherty(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Liam Flaherty <liamflaherty(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 27 Nov 2022 23:43:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashchips.c: Add 4BA write to XM25Qx256C
by Felix Singer (Code Review) 27 Nov '22

27 Nov '22

Attention is currently required from: Liam Flaherty, Angel Pons. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69713 ) Change subject: flashchips.c: Add 4BA write to XM25Qx256C ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/69713 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I96c80762fcda2af6028c7a53d8c545b0c6565cbd Gerrit-Change-Number: 69713 Gerrit-PatchSet: 4 Gerrit-Owner: Liam Flaherty <liamflaherty(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Liam Flaherty <liamflaherty(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 27 Nov 2022 23:19:40 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2022