May 2022 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: flashrom.c: wrap chip->write() with a helper
by Thomas Heijligen (Code Review) 16 May '22

16 May '22

Attention is currently required from: Edward O'Callaghan. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64374 ) Change subject: flashrom.c: wrap chip->write() with a helper ...................................................................... Patch Set 1: (4 comments) Patchset: PS1: With the current state of the patch I does not see any benefit. This may change with a follow up patch File flashrom.c: https://review.coreboot.org/c/flashrom/+/64374/comment/45178087_5a9e8236 PS1, Line 1050: * @flash flash chip We have different documentation styles in the code. @flash @param flash We should unify this. I think `@param ...` is the one Doxygen and other tools recognize. https://review.coreboot.org/c/flashrom/+/64374/comment/5e6ea690_1f953be4 PS1, Line 1054: * Document the return code https://review.coreboot.org/c/flashrom/+/64374/comment/420379cf_b4a47dbb PS1, Line 1055: * TODO: Look up regions that are write-protected and avoid attempt to write Resolve the TODO? Maybe publish a follow up for it. -- To view, visit https://review.coreboot.org/c/flashrom/+/64374 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I46490eba664f96df37310972141ee5db13e48877 Gerrit-Change-Number: 64374 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 16 May 2022 08:04:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: include/flash.h: Drop dead struct members
by Thomas Heijligen (Code Review) 16 May '22

16 May '22

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk, Nikolai Artemiev. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64373 ) Change subject: include/flash.h: Drop dead struct members ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/64373 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I43b25175c6ff833b822a93c4e752a28cf97d64b8 Gerrit-Change-Number: 64373 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> 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-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 16 May 2022 07:28:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: Make need_erase() helper static local
by Thomas Heijligen (Code Review) 16 May '22

16 May '22

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64369 ) Change subject: flashrom.c: Make need_erase() helper static local ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/64369 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic0946bb109fca2fc18e15eefa11cccea284ded0b Gerrit-Change-Number: 64369 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> 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-Comment-Date: Mon, 16 May 2022 07:23:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: wrap chip->write() with a helper
by Arthur Heymans (Code Review) 16 May '22

16 May '22

Attention is currently required from: Edward O'Callaghan. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64374 ) Change subject: flashrom.c: wrap chip->write() with a helper ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/64374 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I46490eba664f96df37310972141ee5db13e48877 Gerrit-Change-Number: 64374 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 16 May 2022 06:02:27 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: include/flash.h: Drop dead struct members
by Arthur Heymans (Code Review) 16 May '22

16 May '22

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk, Nikolai Artemiev. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64373 ) Change subject: include/flash.h: Drop dead struct members ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/64373 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I43b25175c6ff833b822a93c4e752a28cf97d64b8 Gerrit-Change-Number: 64373 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> 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-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 16 May 2022 06:02:07 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: Make need_erase() helper static local
by Arthur Heymans (Code Review) 16 May '22

16 May '22

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64369 ) Change subject: flashrom.c: Make need_erase() helper static local ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/64369 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic0946bb109fca2fc18e15eefa11cccea284ded0b Gerrit-Change-Number: 64369 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 16 May 2022 06:01:58 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: spi25_statusreg.c: Allow opaque masters to hook spi_{rw}_register()
by Edward O'Callaghan (Code Review) 16 May '22

16 May '22

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/64375 ) Change subject: spi25_statusreg.c: Allow opaque masters to hook spi_{rw}_register() ...................................................................... spi25_statusreg.c: Allow opaque masters to hook spi_{rw}_register() Change-Id: I3ab0d7f5f48338c8ecb118a69651c203fbc516ac Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M include/programmer.h M spi25_statusreg.c 2 files changed, 11 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/64375/1 diff --git a/include/programmer.h b/include/programmer.h index 5fb0626..4afaddd 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -405,6 +405,8 @@ enum flashrom_wp_result (*wp_write_cfg)(struct flashctx *, const struct flashrom_wp_cfg *); enum flashrom_wp_result (*wp_read_cfg)(struct flashrom_wp_cfg *, struct flashctx *); enum flashrom_wp_result (*wp_get_ranges)(struct flashrom_wp_ranges **, struct flashctx *); + uint8_t (*read_status) (const struct flashctx *flash); + int (*write_status) (const struct flashctx *flash, int status); int (*shutdown)(void *data); void *data; }; diff --git a/spi25_statusreg.c b/spi25_statusreg.c index 0f43056..0b90716 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -19,6 +19,7 @@ #include "flash.h" #include "chipdrivers.h" +#include "programmer.h" #include "spi.h" /* === Generic functions === */ @@ -35,6 +36,9 @@ */ switch (reg) { case STATUS1: + if ((flash->mst->buses_supported & BUS_PROG) && flash->mst->opaque.write_status) + return flash->mst->opaque.write_status(flash, value); + write_cmd[0] = JEDEC_WRSR; write_cmd[1] = value; write_cmd_len = JEDEC_WRSR_OUTSIZE; @@ -149,6 +153,11 @@ switch (reg) { case STATUS1: + if ((flash->mst->buses_supported & BUS_PROG) && flash->mst->opaque.read_status) { + *value = flash->mst->opaque.read_status(flash); + return 0; + } + read_cmd = JEDEC_RDSR; break; case STATUS2: -- To view, visit https://review.coreboot.org/c/flashrom/+/64375 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3ab0d7f5f48338c8ecb118a69651c203fbc516ac Gerrit-Change-Number: 64375 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: flashrom.c: wrap chip->write() with a helper
by Edward O'Callaghan (Code Review) 16 May '22

16 May '22

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/64374 ) Change subject: flashrom.c: wrap chip->write() with a helper ...................................................................... flashrom.c: wrap chip->write() with a helper Change-Id: I46490eba664f96df37310972141ee5db13e48877 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M flashrom.c 1 file changed, 22 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/74/64374/1 diff --git a/flashrom.c b/flashrom.c index 7f54488..b2c60fe 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1044,6 +1044,26 @@ return 0; } +/* + * write_flash - wrapper for flash->write() with additional high-level policy + * + * @flash flash chip + * @buf buffer to write to flash + * @start start address in flash + * @len number of bytes to write + * + * TODO: Look up regions that are write-protected and avoid attempt to write + * to them at all. + */ +static int write_flash(struct flashctx *flash, const uint8_t *buf, + unsigned int start, unsigned int len) +{ + if (!flash || !flash->chip->write) + return -1; + + return flash->chip->write(flash, buf, start, len); +} + /** * @brief Reads the included layout regions into a buffer. * @@ -1318,7 +1338,7 @@ if (!writecount++) msg_cdbg("W"); /* Needs the partial write function signature. */ - if (flashctx->chip->write(flashctx, backup_contents + starthere, + if (write_flash(flashctx, backup_contents + starthere, info->erase_start + starthere, lenhere)) goto _free_ret; starthere += lenhere; @@ -1422,7 +1442,7 @@ if (!writecount++) msg_cdbg("W"); /* Needs the partial write function signature. */ - if (flashctx->chip->write(flashctx, newcontents + starthere, + if (write_flash(flashctx, newcontents + starthere, info->erase_start + starthere, lenhere)) goto _free_ret; starthere += lenhere; -- To view, visit https://review.coreboot.org/c/flashrom/+/64374 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I46490eba664f96df37310972141ee5db13e48877 Gerrit-Change-Number: 64374 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: include/flash.h: Drop dead struct members
by Edward O'Callaghan (Code Review) 16 May '22

16 May '22

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/64373 ) Change subject: include/flash.h: Drop dead struct members ...................................................................... include/flash.h: Drop dead struct members These were part of the original wp implementation, now dead code left over. Change-Id: I43b25175c6ff833b822a93c4e752a28cf97d64b8 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M include/flash.h 1 file changed, 0 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/73/64373/1 diff --git a/include/flash.h b/include/flash.h index 89d5737..27ade09 100644 --- a/include/flash.h +++ b/include/flash.h @@ -266,8 +266,6 @@ int (*unlock) (struct flashctx *flash); int (*write) (struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); int (*read) (struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len); - uint8_t (*read_status) (const struct flashctx *flash); - int (*write_status) (const struct flashctx *flash, int status); struct voltage { uint16_t min; uint16_t max; -- To view, visit https://review.coreboot.org/c/flashrom/+/64373 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I43b25175c6ff833b822a93c4e752a28cf97d64b8 Gerrit-Change-Number: 64373 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: ichspi: Factor out common hwseq_xfer logic into helpers
by Subrata Banik (Code Review) 16 May '22

16 May '22

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62869 ) Change subject: ichspi: Factor out common hwseq_xfer logic into helpers ...................................................................... Patch Set 16: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/62869/comment/60caebc9_42167bab PS15, Line 7: Refactor Flashrom HW Sequencing Operation > How about 'ichspi: Factor out common hwseq_xfer logic into helpers' Ack -- To view, visit https://review.coreboot.org/c/flashrom/+/62869 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic9fd50841449e02f476a8834f4642d6ecad36dc3 Gerrit-Change-Number: 62869 Gerrit-PatchSet: 16 Gerrit-Owner: Subrata Banik <subratabanik(a)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: 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: Mon, 16 May 2022 05:24:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2022