Thomas Heijligen has submitted this change. ( https://review.coreboot.org/c/flashrom/+/67480 )
Change subject: spi: Make 'default_spi_send_command' the default unless defined
......................................................................
spi: Make 'default_spi_send_command' the default unless defined
A NULL func pointer is necessary and sufficient for the
condition `NULL func pointer => default_spi_send_command' as to not
need this explicit specification of 'default'.
Therefore drop the explicit need to specify the 'default_spi_send_command'
callback function pointer in the spi_master struct. This is a reasonable default for every other driver in the tree with only a few exceptions.
This simplifies the code and driver development.
Change-Id: I63abcb8c64f233cdbf58a149a31051fa648305a2
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/67480
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Thomas Heijligen <src(a)posteo.de>
---
M ft2232_spi.c
M spi.c
2 files changed, 27 insertions(+), 5 deletions(-)
Approvals:
build bot (Jenkins): Verified
Thomas Heijligen: Looks good to me, approved
diff --git a/ft2232_spi.c b/ft2232_spi.c
index dfec1a0..16bcfe3 100644
--- a/ft2232_spi.c
+++ b/ft2232_spi.c
@@ -295,7 +295,6 @@
.features = SPI_MASTER_4BA,
.max_data_read = 64 * 1024,
.max_data_write = 256,
- .command = default_spi_send_command,
.multicommand = ft2232_spi_send_multicommand,
.read = default_spi_read,
.write_256 = default_spi_write_256,
diff --git a/spi.c b/spi.c
index 6c2c4c4..ca27cbb 100644
--- a/spi.c
+++ b/spi.c
@@ -30,8 +30,9 @@
unsigned int readcnt, const unsigned char *writearr,
unsigned char *readarr)
{
- return flash->mst->spi.command(flash, writecnt, readcnt, writearr,
- readarr);
+ if (flash->mst->spi.command)
+ return flash->mst->spi.command(flash, writecnt, readcnt, writearr, readarr);
+ return default_spi_send_command(flash, writecnt, readcnt, writearr, readarr);
}
int spi_send_multicommand(const struct flashctx *flash, struct spi_command *cmds)
@@ -152,9 +153,9 @@
}
}
- if (!mst->write_256 || !mst->read || !mst->command ||
+ if (!mst->write_256 || !mst->read ||
!mst->multicommand || !mst->probe_opcode ||
- ((mst->command == default_spi_send_command) &&
+ ((mst->command == default_spi_send_command || !mst->command) &&
(mst->multicommand == default_spi_send_multicommand))) {
msg_perr("%s called with incomplete master definition. "
"Please report a bug at flashrom(a)flashrom.org\n",
--
To view, visit https://review.coreboot.org/c/flashrom/+/67480
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I63abcb8c64f233cdbf58a149a31051fa648305a2
Gerrit-Change-Number: 67480
Gerrit-PatchSet: 5
Gerrit-Owner: 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-MessageType: merged
Attention is currently required from: Edward O'Callaghan, Aarya, Anastasia Klimchuk.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70264 )
Change subject: tree/: Drop default_spi_probe_opcode for NULL case
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS5:
Needs a manual rebase due to new programmers
--
To view, visit https://review.coreboot.org/c/flashrom/+/70264
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id502c5d2596ad1db52faf05723083620e4c52c12
Gerrit-Change-Number: 70264
Gerrit-PatchSet: 5
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
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: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Mon, 27 Feb 2023 10:27:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Thomas Heijligen has submitted this change. ( https://review.coreboot.org/c/flashrom/+/73285 )
(
1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: jedec.c: Provide better lexical scope to itermediates
......................................................................
jedec.c: Provide better lexical scope to itermediates
Change-Id: I8e01d471bb33a933b80760df2c69a4bf3589ba76
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/73285
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Reviewed-by: Thomas Heijligen <src(a)posteo.de>
---
M jedec.c
1 file changed, 20 insertions(+), 10 deletions(-)
Approvals:
build bot (Jenkins): Verified
Stefan Reinauer: Looks good to me, approved
Thomas Heijligen: Looks good to me, approved
diff --git a/jedec.c b/jedec.c
index 01c401f..db2e3e8 100644
--- a/jedec.c
+++ b/jedec.c
@@ -38,13 +38,11 @@
static void toggle_ready_jedec_common(const struct flashctx *flash, chipaddr dst, unsigned int delay)
{
unsigned int i = 0;
- uint8_t tmp1, tmp2;
-
- tmp1 = chip_readb(flash, dst) & 0x40;
+ uint8_t tmp1 = chip_readb(flash, dst) & 0x40;
while (i++ < 0xFFFFFFF) {
programmer_delay(flash, delay);
- tmp2 = chip_readb(flash, dst) & 0x40;
+ uint8_t tmp2 = chip_readb(flash, dst) & 0x40;
if (tmp1 == tmp2) {
break;
}
@@ -76,12 +74,11 @@
uint8_t data)
{
unsigned int i = 0;
- uint8_t tmp;
data &= 0x80;
while (i++ < 0xFFFFFFF) {
- tmp = chip_readb(flash, dst) & 0x80;
+ uint8_t tmp = chip_readb(flash, dst) & 0x80;
if (tmp == data) {
break;
}
@@ -377,12 +374,11 @@
int write_jedec_1(struct flashctx *flash, const uint8_t *src, unsigned int start,
unsigned int len)
{
- unsigned int i;
int failed = 0;
chipaddr dst = flash->virtual_memory + start;
const chipaddr olddst = dst;
- for (i = 0; i < len; i++) {
+ for (unsigned int i = 0; i < len; i++) {
if (write_byte_program_jedec_common(flash, src, dst))
failed = 1;
dst++, src++;
@@ -444,7 +440,7 @@
int write_jedec(struct flashctx *flash, const uint8_t *buf, unsigned int start,
int unsigned len)
{
- unsigned int i, starthere, lenhere;
+ unsigned int starthere, lenhere;
/* FIXME: page_size is the wrong variable. We need max_writechunk_size
* in struct flashctx to do this properly. All chips using
* write_jedec have page_size set to max_writechunk_size, so
@@ -462,7 +458,7 @@
* (start + len - 1) / page_size. Since we want to include that last
* page as well, the loop condition uses <=.
*/
- for (i = start / page_size; i <= nwrites; i++) {
+ for (unsigned int i = start / page_size; i <= nwrites; i++) {
/* Byte position of the first byte in the range in this page. */
/* starthere is an offset to the base address of the chip. */
starthere = max(start, i * page_size);
--
To view, visit https://review.coreboot.org/c/flashrom/+/73285
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I8e01d471bb33a933b80760df2c69a4bf3589ba76
Gerrit-Change-Number: 73285
Gerrit-PatchSet: 3
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Edward O'Callaghan.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73285 )
Change subject: jedec.c: Provide better lexical scope to itermediates
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/73285
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I8e01d471bb33a933b80760df2c69a4bf3589ba76
Gerrit-Change-Number: 73285
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
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-Comment-Date: Mon, 27 Feb 2023 10:25:13 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Thomas Heijligen has submitted this change. ( https://review.coreboot.org/c/flashrom/+/73284 )
(
1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: jedec.c: Tidy up whitespace and line wraps
......................................................................
jedec.c: Tidy up whitespace and line wraps
Change-Id: I3f18609709e59cdfc5a853c6e1ae5f53aecacc1a
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/73284
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Reviewed-by: Thomas Heijligen <src(a)posteo.de>
---
M jedec.c
1 file changed, 20 insertions(+), 7 deletions(-)
Approvals:
build bot (Jenkins): Verified
Stefan Reinauer: Looks good to me, approved
Thomas Heijligen: Looks good to me, approved
diff --git a/jedec.c b/jedec.c
index bb617e0..01c401f 100644
--- a/jedec.c
+++ b/jedec.c
@@ -305,11 +305,11 @@
int erase_sector_jedec(struct flashctx *flash, unsigned int page, unsigned int size)
{
- /* Issue the Sector Erase command */
+ /* Issue the Sector Erase command */
issuecmd(flash, 0x80, 0);
issuecmd(flash, 0x30, page);
- /* wait for Toggle bit ready */
+ /* Wait for Toggle bit ready */
toggle_ready_jedec_slow(flash);
/* FIXME: Check the status register for errors. */
@@ -318,11 +318,11 @@
int erase_block_jedec(struct flashctx *flash, unsigned int block, unsigned int size)
{
- /* Issue the Sector Erase command */
+ /* Issue the Block Erase command */
issuecmd(flash, 0x80, 0);
issuecmd(flash, 0x50, block);
- /* wait for Toggle bit ready */
+ /* Wait for Toggle bit ready */
toggle_ready_jedec_slow(flash);
/* FIXME: Check the status register for errors. */
@@ -333,12 +333,11 @@
int erase_chip_block_jedec(struct flashctx *flash, unsigned int addr, unsigned int blocksize)
{
if ((addr != 0) || (blocksize != flash->chip->total_size * 1024)) {
- msg_cerr("%s called with incorrect arguments\n",
- __func__);
+ msg_cerr("%s called with incorrect arguments\n", __func__);
return -1;
}
- /* Issue the JEDEC Chip Erase command */
+ /* Issue the JEDEC Chip Erase command */
issuecmd(flash, 0x80, 0);
issuecmd(flash, 0x10, 0);
--
To view, visit https://review.coreboot.org/c/flashrom/+/73284
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3f18609709e59cdfc5a853c6e1ae5f53aecacc1a
Gerrit-Change-Number: 73284
Gerrit-PatchSet: 3
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Edward O'Callaghan.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73284 )
Change subject: jedec.c: Tidy up whitespace and line wraps
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/73284
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3f18609709e59cdfc5a853c6e1ae5f53aecacc1a
Gerrit-Change-Number: 73284
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
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-Comment-Date: Mon, 27 Feb 2023 10:24:09 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Thomas Heijligen has submitted this change. ( https://review.coreboot.org/c/flashrom/+/73283 )
(
1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: jedec.c: Add a little more const correctness
......................................................................
jedec.c: Add a little more const correctness
Change-Id: Ic9a76ce3734bd83399c95478a7c0bfc081211124
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/73283
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Reviewed-by: Thomas Heijligen <src(a)posteo.de>
---
M jedec.c
1 file changed, 18 insertions(+), 5 deletions(-)
Approvals:
build bot (Jenkins): Verified
Stefan Reinauer: Looks good to me, approved
Thomas Heijligen: Looks good to me, approved
diff --git a/jedec.c b/jedec.c
index 2cdde61..bb617e0 100644
--- a/jedec.c
+++ b/jedec.c
@@ -123,7 +123,7 @@
int probe_jedec_29gl(struct flashctx *flash)
{
const unsigned int mask = getaddrmask(flash->chip);
- chipaddr bios = flash->virtual_memory;
+ const chipaddr bios = flash->virtual_memory;
const struct flashchip *chip = flash->chip;
/* Reset chip to a clean slate */
@@ -381,9 +381,8 @@
unsigned int i;
int failed = 0;
chipaddr dst = flash->virtual_memory + start;
- chipaddr olddst;
+ const chipaddr olddst = dst;
- olddst = dst;
for (i = 0; i < len; i++) {
if (write_byte_program_jedec_common(flash, src, dst))
failed = 1;
@@ -452,8 +451,8 @@
* write_jedec have page_size set to max_writechunk_size, so
* we're OK for now.
*/
- unsigned int page_size = flash->chip->page_size;
- unsigned int nwrites = (start + len - 1) / page_size;
+ const unsigned int page_size = flash->chip->page_size;
+ const unsigned int nwrites = (start + len - 1) / page_size;
/* Warning: This loop has a very unusual condition and body.
* The loop needs to go through each page with at least one affected
--
To view, visit https://review.coreboot.org/c/flashrom/+/73283
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ic9a76ce3734bd83399c95478a7c0bfc081211124
Gerrit-Change-Number: 73283
Gerrit-PatchSet: 3
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Edward O'Callaghan.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73283 )
Change subject: jedec.c: Add a little more const correctness
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/73283
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ic9a76ce3734bd83399c95478a7c0bfc081211124
Gerrit-Change-Number: 73283
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
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-Comment-Date: Mon, 27 Feb 2023 10:23:18 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Thomas Heijligen has submitted this change. ( https://review.coreboot.org/c/flashrom/+/73282 )
(
1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: jedec.c: Rename func to jedec_write_page()
......................................................................
jedec.c: Rename func to jedec_write_page()
Change-Id: I1be83d5974e305bddceaa34b64e982b774ade0d2
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/73282
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Reviewed-by: Thomas Heijligen <src(a)posteo.de>
---
M jedec.c
1 file changed, 16 insertions(+), 2 deletions(-)
Approvals:
build bot (Jenkins): Verified
Stefan Reinauer: Looks good to me, approved
Thomas Heijligen: Looks good to me, approved
diff --git a/jedec.c b/jedec.c
index f9442ec..2cdde61 100644
--- a/jedec.c
+++ b/jedec.c
@@ -396,7 +396,7 @@
return failed;
}
-static int write_page_write_jedec_common(struct flashctx *flash, const uint8_t *src,
+static int jedec_write_page(struct flashctx *flash, const uint8_t *src,
unsigned int start, unsigned int page_size)
{
int tries = 0, failed;
@@ -471,7 +471,7 @@
/* Length of bytes in the range in this page. */
lenhere = min(start + len, (i + 1) * page_size) - starthere;
- if (write_page_write_jedec_common(flash, buf + starthere - start, starthere, lenhere))
+ if (jedec_write_page(flash, buf + starthere - start, starthere, lenhere))
return 1;
update_progress(flash, FLASHROM_PROGRESS_WRITE, i + 1, nwrites + 1);
}
--
To view, visit https://review.coreboot.org/c/flashrom/+/73282
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1be83d5974e305bddceaa34b64e982b774ade0d2
Gerrit-Change-Number: 73282
Gerrit-PatchSet: 3
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Edward O'Callaghan.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73282 )
Change subject: jedec.c: Rename func to jedec_write_page()
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/73282
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1be83d5974e305bddceaa34b64e982b774ade0d2
Gerrit-Change-Number: 73282
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
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-Comment-Date: Mon, 27 Feb 2023 10:20:57 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment