December 2022 - flashrom-gerrit

[M] Change in flashrom[master]: ichspi.c: Read chip ID and use it to populate `flash->chip`
by Edward O'Callaghan (Code Review) 08 Dec '22

08 Dec '22

Attention is currently required from: Nico Huber, Nikolai Artemiev. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69195 ) Change subject: ichspi.c: Read chip ID and use it to populate `flash->chip` ...................................................................... Patch Set 14: (2 comments) File ichspi.c: https://review.coreboot.org/c/flashrom/+/69195/comment/c21eff14_fe3c807f PS14, Line 144: spurious https://review.coreboot.org/c/flashrom/+/69195/comment/60f1a2d2_c2ba5b60 PS14, Line 1495: ich_generation `ich_gen` -- To view, visit https://review.coreboot.org/c/flashrom/+/69195 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia408e1e45dc6f53c0934afd6558e301abfa48ee6 Gerrit-Change-Number: 69195 Gerrit-PatchSet: 14 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Thu, 08 Dec 2022 08:31:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: ichspi.c: Clean up HSFC FCYCLE definitions
by Edward O'Callaghan (Code Review) 08 Dec '22

08 Dec '22

Attention is currently required from: Subrata Banik, Nikolai Artemiev. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70458 ) Change subject: ichspi.c: Clean up HSFC FCYCLE definitions ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/70458 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0996c5331837276049241600e0ffac21a47ec3af Gerrit-Change-Number: 70458 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Thu, 08 Dec 2022 08:29:52 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom.c: Supplement `chip->unlock()` calls with wp unlocking
by Edward O'Callaghan (Code Review) 08 Dec '22

08 Dec '22

Attention is currently required from: Nikolai Artemiev. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69517 ) Change subject: flashrom.c: Supplement `chip->unlock()` calls with wp unlocking ...................................................................... Patch Set 8: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/69517 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I74b3f5d3a17749ea60485b916b2d87467a5d8b2f Gerrit-Change-Number: 69517 Gerrit-PatchSet: 8 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Thu, 08 Dec 2022 08:27:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: layout: Check return values for strdup in register_include_arg
by David Hendricks (Code Review) 08 Dec '22

08 Dec '22

Attention is currently required from: Angel Pons, Anastasia Klimchuk. David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70006 ) Change subject: layout: Check return values for strdup in register_include_arg ...................................................................... Patch Set 7: Code-Review+2 (1 comment) File layout.c: https://review.coreboot.org/c/flashrom/+/70006/comment/53572c7d_4cc76d63 PS7, Line 123: char *file = NULL; /* file is optional, so defaults to NULL */ (consider this comment to be a non-blocking nit since the code is fine as you've written it and Edward already suggested a better refactoring idea) Without this initialization `file` could get free()'d with a garbage value. For extra safety on top of the initialization, you might try using different labels so the program flow will skip freeing pointers that are simply unused, initialized or not. For example: ``` if (colon) { name = strndup(...); if (!name) { ... goto error_1; } file = strndup(...); if (!file) { ... goto error_2; } } ... error_2: free(file); error_1: free(name); return 1; ``` -- To view, visit https://review.coreboot.org/c/flashrom/+/70006 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6c22196be6847a8c9704f1de936604a51b4b8a28 Gerrit-Change-Number: 70006 Gerrit-PatchSet: 7 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 08 Dec 2022 07:14:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: ichspi.c: Read chip ID and use it to populate `flash->chip`
by Nikolai Artemiev (Code Review) 08 Dec '22

08 Dec '22

Attention is currently required from: Nico Huber, Edward O'Callaghan. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69195 ) Change subject: ichspi.c: Read chip ID and use it to populate `flash->chip` ...................................................................... Patch Set 14: (2 comments) File ichspi.c: https://review.coreboot.org/c/flashrom/+/69195/comment/db66db58_39fbbaf2 PS12, Line 135: #define HSFC_CYCLE_RDID HSFC_FCYCLE_MASK(6) > Unresolving so I don't forget this. Done: https://review.coreboot.org/c/flashrom/+/70458 https://review.coreboot.org/c/flashrom/+/69195/comment/9e63c87c_61acbdfa PS12, Line 1474: if (ich_exec_sync_hwseq_xfer(flash, HSFC_CYCLE_RDID, 1, len, ich_generation, > Unresolving so I don't forget this. Done -- To view, visit https://review.coreboot.org/c/flashrom/+/69195 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia408e1e45dc6f53c0934afd6558e301abfa48ee6 Gerrit-Change-Number: 69195 Gerrit-PatchSet: 14 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Thu, 08 Dec 2022 06:59:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: ichspi.c: Clean up HSFC FCYCLE definitions
by Nikolai Artemiev (Code Review) 08 Dec '22

08 Dec '22

Nikolai Artemiev has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/70458 ) Change subject: ichspi.c: Clean up HSFC FCYCLE definitions ...................................................................... ichspi.c: Clean up HSFC FCYCLE definitions Move the FCYCLE bit definitions out of the ICH9 definitions and into their own section because they are used by PCH100 as well. Rename HSFC_FCYCLE to ICH9_HSFC_FCYCLE because it is specific to ICH9. BUG=b:253715389,b:253713774 BRANCH=none TEST=builds Change-Id: I0996c5331837276049241600e0ffac21a47ec3af Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M ichspi.c 1 file changed, 48 insertions(+), 22 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/58/70458/1 diff --git a/ichspi.c b/ichspi.c index 2b6c7ae..b1e3f54 100644 --- a/ichspi.c +++ b/ichspi.c @@ -46,9 +46,15 @@ * however we still treat them separately in order to reuse code. */ -/* Changed HSFC Control bits */ /* - * 4 bits to represents the FCYCLE operation for PCH as: + * HSFC Control bits + * + * FCYCLE is a 2 bit field (HSFC bits 1-2) on ICH9 and 4 bit field + * (HSFC bits 1-4) on PCH100. + * + * ICH9 and PCH100 use the same FCYCLE values for flash operations, + * however FCYCLE values above 3 are only supported by PCH100. + * * 0: SPI Read * 2: SPI Write * 3: SPI Erase 4K @@ -56,9 +62,21 @@ * 6: SPI RDID * 7: SPI Write Status * 8: SPI Read Status - */ + */ +#define HSFC_FGO_OFF 0 +#define HSFC_FGO (0x1 << HSFC_FGO_OFF) +#define HSFC_FCYCLE_MASK(n) ((n) << HSFC_FCYCLE_OFF) +#define HSFC_FCYCLE_OFF 1 +#define HSFC_CYCLE_READ HSFC_FCYCLE_MASK(0x0) +#define HSFC_CYCLE_WRITE HSFC_FCYCLE_MASK(0x2) +#define HSFC_CYCLE_BLOCK_ERASE HSFC_FCYCLE_MASK(0x3) +#define HSFC_CYCLE_RDID HSFC_FCYCLE_MASK(0x6) +#define HSFC_CYCLE_WR_STATUS HSFC_FCYCLE_MASK(0x7) +#define HSFC_CYCLE_RD_STATUS HSFC_FCYCLE_MASK(0x8) + +/* PCH100 controller register definition */ +#define PCH100_HSFC_FCYCLE_OFF 1 #define PCH100_HSFC_FCYCLE_BIT_WIDTH 0xf -#define PCH100_HSFC_FCYCLE_OFF (17 - 16) /* 1-4: FLASH Cycle */ #define PCH100_HSFC_FCYCLE HSFC_FCYCLE_MASK(PCH100_HSFC_FCYCLE_BIT_WIDTH) /* New HSFC Control bit */ #define PCH100_HSFC_WET_OFF (21 - 16) /* 5: Write Enable Type */ @@ -117,23 +135,12 @@ #define HSFS_FLOCKDN (0x1 << HSFS_FLOCKDN_OFF) #define ICH9_REG_HSFC 0x06 /* 16 Bits Hardware Sequencing Flash Control */ -#define HSFC_FGO_OFF 0 /* 0: Flash Cycle Go */ -#define HSFC_FGO (0x1 << HSFC_FGO_OFF) -/* - * 2 bits to represents the FCYCLE operation for ICH9 as: - * 0: SPI Read - * 2: SPI Write - * 3: SPI Block Erase - */ + + /* 0: Flash Cycle Go */ + /* 1-2: FLASH Cycle */ +#define ICH9_HSFC_FCYCLE_OFF 1 #define ICH9_HSFC_FCYCLE_BIT_WIDTH 3 -#define HSFC_FCYCLE_OFF 1 /* 1-2: FLASH Cycle */ -#define HSFC_FCYCLE_MASK(n) ((n) << HSFC_FCYCLE_OFF) -#define HSFC_FCYCLE HSFC_FCYCLE_MASK(ICH9_HSFC_FCYCLE_BIT_WIDTH) -#define HSFC_CYCLE_READ HSFC_FCYCLE_MASK(0) -#define HSFC_CYCLE_WRITE HSFC_FCYCLE_MASK(2) -#define HSFC_CYCLE_BLOCK_ERASE HSFC_FCYCLE_MASK(3) -#define HSFC_CYCLE_WR_STATUS HSFC_FCYCLE_MASK(7) -#define HSFC_CYCLE_RD_STATUS HSFC_FCYCLE_MASK(8) +#define ICH9_HSFC_FCYCLE HSFC_FCYCLE_MASK(ICH9_HSFC_FCYCLE_BIT_WIDTH) /* 3-7: reserved */ #define HSFC_FDBC_OFF 8 /* 8-13: Flash Data Byte Count */ #define HSFC_FDBC (0x3f << HSFC_FDBC_OFF) @@ -492,7 +499,7 @@ pprint_reg(PCH100_HSFC, WET, reg_val, ", "); break; default: - pprint_reg(HSFC, FCYCLE, reg_val, ", "); + pprint_reg(ICH9_HSFC, FCYCLE, reg_val, ", "); break; } pprint_reg(HSFC, FDBC, reg_val, ", "); @@ -1927,7 +1934,7 @@ swseq->reg_opmenu = ICH9_REG_OPMENU; hwseq->addr_mask = ICH9_FADDR_FLA; hwseq->only_4k = false; - hwseq->hsfc_fcycle = HSFC_FCYCLE; + hwseq->hsfc_fcycle = ICH9_HSFC_FCYCLE; break; } -- To view, visit https://review.coreboot.org/c/flashrom/+/70458 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0996c5331837276049241600e0ffac21a47ec3af Gerrit-Change-Number: 70458 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: ichspi.c: Read chip ID and use it to populate `flash->chip`
by Nikolai Artemiev (Code Review) 08 Dec '22

08 Dec '22

Attention is currently required from: Nico Huber, Edward O'Callaghan. Hello build bot (Jenkins), Subrata Banik, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/69195 to look at the new patch set (#14). Change subject: ichspi.c: Read chip ID and use it to populate `flash->chip` ...................................................................... ichspi.c: Read chip ID and use it to populate `flash->chip` Read the flash chip vendor/device ID using hardware sequencing, find the corresponding flashchip entry, and copy it over to `flash->chip`. Identifying the chip was not previously required as ICH hardware sequencing handles chip-level details related to read/write/erase ops. However writeprotect operations require the chip entry to be identified so that chip->reg_bits can be used to compute status register values. BUG=b:253715389,b:253713774 BRANCH=none TEST=flashrom on dedede (JSL) identifies "W25Q128.V..M" chip TEST=flashrom -{r,v} on dedede TEST=write/erase bios region on dedede: flashrom -{E,w} --layout <(echo '0x381000:0xffffff bios') -i bios Change-Id: Ia408e1e45dc6f53c0934afd6558e301abfa48ee6 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M ichspi.c 1 file changed, 102 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/95/69195/14 -- To view, visit https://review.coreboot.org/c/flashrom/+/69195 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia408e1e45dc6f53c0934afd6558e301abfa48ee6 Gerrit-Change-Number: 69195 Gerrit-PatchSet: 14 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: README: Add information about meson and link build instructions
by David Hendricks (Code Review) 08 Dec '22

08 Dec '22

Attention is currently required from: Anastasia Klimchuk. David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70176 ) Change subject: README: Add information about meson and link build instructions ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/70176 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2a27d8f2ba42e18be2485ae95bec1b4c874bb4f7 Gerrit-Change-Number: 70176 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 08 Dec 2022 06:37:17 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tree/: Change chip restore data type from uint8_t to void ptr
by Edward O'Callaghan (Code Review) 08 Dec '22

08 Dec '22

Attention is currently required from: Nikolai Artemiev. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70349 ) Change subject: tree/: Change chip restore data type from uint8_t to void ptr ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/70349 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I311b468a4b0349f4da9584c12b36af6ec2394527 Gerrit-Change-Number: 70349 Gerrit-PatchSet: 5 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Thu, 08 Dec 2022 03:33:17 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: serprog.c: Dispense with forward decl by reshuffle
by Anastasia Klimchuk (Code Review) 08 Dec '22

08 Dec '22

Attention is currently required from: Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70443 ) Change subject: serprog.c: Dispense with forward decl by reshuffle ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/70443/comment/63fb1df0_7b272e88 PS1, Line 8: Could you please test that binary did not change before and after, and add the info into commit message? Thank you! -- To view, visit https://review.coreboot.org/c/flashrom/+/70443 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0ffcd8bd03fae0d905887a88c9b2e06f61577fd4 Gerrit-Change-Number: 70443 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Thu, 08 Dec 2022 03:30:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit December 2022