- flashrom-gerrit - mail.coreboot.org

[S] Change in flashrom[master]: jedec.c: Drop branching non-zero programmer_delay() operands
by Edward O'Callaghan (Code Review) 31 Jan '23

31 Jan '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/72692 ) Change subject: jedec.c: Drop branching non-zero programmer_delay() operands ...................................................................... jedec.c: Drop branching non-zero programmer_delay() operands The programmer_delay() function is already tolerant upon zero delay values and will simply just return with a NOP. Therefore there is no need to branch. Change-Id: Ic547669bb16e6ace4fe283e07345fc2d7075d63e Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M jedec.c 1 file changed, 25 insertions(+), 22 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/92/72692/1 diff --git a/jedec.c b/jedec.c index b44bfb7..d35bc8e 100644 --- a/jedec.c +++ b/jedec.c @@ -44,8 +44,7 @@ tmp1 = chip_readb(flash, dst) & 0x40; while (i++ < 0xFFFFFFF) { - if (delay) - programmer_delay(flash, delay); + programmer_delay(flash, delay); tmp2 = chip_readb(flash, dst) & 0x40; if (tmp1 == tmp2) { break; @@ -204,31 +203,24 @@ * mode completely. Allow the chip to finish this before seeing a * reset command. */ - if (probe_timing_enter) - programmer_delay(flash, probe_timing_enter); + programmer_delay(flash, probe_timing_enter); /* Reset chip to a clean slate */ if ((chip->feature_bits & FEATURE_RESET_MASK) == FEATURE_LONG_RESET) { chip_writeb(flash, 0xAA, bios + ((shifted ? 0x2AAA : 0x5555) & mask)); - if (probe_timing_exit) - programmer_delay(flash, 10); + programmer_delay(flash, 10); chip_writeb(flash, 0x55, bios + ((shifted ? 0x5555 : 0x2AAA) & mask)); - if (probe_timing_exit) - programmer_delay(flash, 10); + programmer_delay(flash, 10); } chip_writeb(flash, 0xF0, bios + ((shifted ? 0x2AAA : 0x5555) & mask)); - if (probe_timing_exit) - programmer_delay(flash, probe_timing_exit); + programmer_delay(flash, probe_timing_exit); /* Issue JEDEC Product ID Entry command */ chip_writeb(flash, 0xAA, bios + ((shifted ? 0x2AAA : 0x5555) & mask)); - if (probe_timing_enter) - programmer_delay(flash, 10); + programmer_delay(flash, 10); chip_writeb(flash, 0x55, bios + ((shifted ? 0x5555 : 0x2AAA) & mask)); - if (probe_timing_enter) - programmer_delay(flash, 10); + programmer_delay(flash, 10); chip_writeb(flash, JEDEC_REMS, bios + ((shifted ? 0x2AAA : 0x5555) & mask)); - if (probe_timing_enter) - programmer_delay(flash, probe_timing_enter); + programmer_delay(flash, probe_timing_enter); /* Read product ID */ id1 = chip_readb(flash, bios + (0x00 << shifted)); @@ -251,15 +243,12 @@ /* Issue JEDEC Product ID Exit command */ if ((chip->feature_bits & FEATURE_RESET_MASK) == FEATURE_LONG_RESET) { chip_writeb(flash, 0xAA, bios + ((shifted ? 0x2AAA : 0x5555) & mask)); - if (probe_timing_exit) - programmer_delay(flash, 10); + programmer_delay(flash, 10); chip_writeb(flash, 0x55, bios + ((shifted ? 0x5555 : 0x2AAA) & mask)); - if (probe_timing_exit) - programmer_delay(flash, 10); + programmer_delay(flash, 10); } chip_writeb(flash, 0xF0, bios + ((shifted ? 0x2AAA : 0x5555) & mask)); - if (probe_timing_exit) - programmer_delay(flash, probe_timing_exit); + programmer_delay(flash, probe_timing_exit); msg_cdbg("%s: id1 0x%02x, id2 0x%02x", __func__, largeid1, largeid2); if (!oddparity(id1)) -- To view, visit https://review.coreboot.org/c/flashrom/+/72692 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic547669bb16e6ace4fe283e07345fc2d7075d63e Gerrit-Change-Number: 72692 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: jedec.c: Fold up dst into toggle_ready_jedec()
by Edward O'Callaghan (Code Review) 31 Jan '23

31 Jan '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/72691 ) Change subject: jedec.c: Fold up dst into toggle_ready_jedec() ...................................................................... jedec.c: Fold up dst into toggle_ready_jedec() Change-Id: Ib7a3fdbc6e0a888093dc8da6f5567a7301ec5040 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M jedec.c 1 file changed, 15 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/91/72691/1 diff --git a/jedec.c b/jedec.c index 3f27337..e33ce93 100644 --- a/jedec.c +++ b/jedec.c @@ -67,8 +67,9 @@ * Given that erase is slow on all chips, it is recommended to use * toggle_ready_jedec_slow in erase functions. */ -static void toggle_ready_jedec_slow(const struct flashctx *flash, chipaddr dst) +static void toggle_ready_jedec_slow(const struct flashctx *flash) { + const chipaddr dst = flash->virtual_memory; toggle_ready_jedec_common(flash, dst, 8 * 1000); } @@ -300,14 +301,12 @@ int erase_sector_jedec(struct flashctx *flash, unsigned int page, unsigned int size) { - const chipaddr bios = flash->virtual_memory; - /* Issue the Sector Erase command */ issuecmd(flash, 0x80, 0); issuecmd(flash, 0x30, page); /* wait for Toggle bit ready */ - toggle_ready_jedec_slow(flash, bios); + toggle_ready_jedec_slow(flash); /* FIXME: Check the status register for errors. */ return 0; @@ -315,14 +314,12 @@ int erase_block_jedec(struct flashctx *flash, unsigned int block, unsigned int size) { - const chipaddr bios = flash->virtual_memory; - /* Issue the Sector Erase command */ issuecmd(flash, 0x80, 0); issuecmd(flash, 0x50, block); /* wait for Toggle bit ready */ - toggle_ready_jedec_slow(flash, bios); + toggle_ready_jedec_slow(flash); /* FIXME: Check the status register for errors. */ return 0; @@ -331,8 +328,6 @@ /* erase chip with block_erase() prototype */ int erase_chip_block_jedec(struct flashctx *flash, unsigned int addr, unsigned int blocksize) { - const chipaddr bios = flash->virtual_memory; - if ((addr != 0) || (blocksize != flash->chip->total_size * 1024)) { msg_cerr("%s called with incorrect arguments\n", __func__); @@ -343,7 +338,7 @@ issuecmd(flash, 0x80, 0); issuecmd(flash, 0x10, 0); - toggle_ready_jedec_slow(flash, bios); + toggle_ready_jedec_slow(flash); /* FIXME: Check the status register for errors. */ return 0; -- To view, visit https://review.coreboot.org/c/flashrom/+/72691 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib7a3fdbc6e0a888093dc8da6f5567a7301ec5040 Gerrit-Change-Number: 72691 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: jedec.c: Make programer_delay() usage symetrical
by Edward O'Callaghan (Code Review) 31 Jan '23

31 Jan '23

Attention is currently required from: Edward O'Callaghan. Hello build bot (Jenkins), Stefan Reinauer, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/72630 to look at the new patch set (#4). Change subject: jedec.c: Make programer_delay() usage symetrical ...................................................................... jedec.c: Make programer_delay() usage symetrical Change-Id: Ia8c22914bc14d39716fb2f82f0b8394edc1cae7f Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M jedec.c 1 file changed, 17 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/30/72630/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/72630 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia8c22914bc14d39716fb2f82f0b8394edc1cae7f Gerrit-Change-Number: 72630 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: jedec.c: Use JEDEC_ define from spi.h over magic
by Edward O'Callaghan (Code Review) 31 Jan '23

31 Jan '23

Attention is currently required from: Edward O'Callaghan. Hello build bot (Jenkins), Stefan Reinauer, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/72632 to look at the new patch set (#2). Change subject: jedec.c: Use JEDEC_ define from spi.h over magic ...................................................................... jedec.c: Use JEDEC_ define from spi.h over magic Change-Id: Id0c98ee900e37c26dc06f96a9950713f332240fd Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M jedec.c 1 file changed, 13 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/72632/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/72632 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id0c98ee900e37c26dc06f96a9950713f332240fd Gerrit-Change-Number: 72632 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: jedec.c: Make programer_delay() usage symetrical
by Stefan Reinauer (Code Review) 31 Jan '23

31 Jan '23

Attention is currently required from: Edward O'Callaghan. Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72630 ) Change subject: jedec.c: Make programer_delay() usage symetrical ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/72630/comment/a6c69480_7c6f064f PS3, Line 7: Make programer_delay() usage symetrical What does that mean? -- To view, visit https://review.coreboot.org/c/flashrom/+/72630 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia8c22914bc14d39716fb2f82f0b8394edc1cae7f Gerrit-Change-Number: 72630 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 31 Jan 2023 22:41:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: jedec.c: Use JEDEC_ define from spi.h over magic
by Stefan Reinauer (Code Review) 31 Jan '23

31 Jan '23

Attention is currently required from: Edward O'Callaghan. Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72632 ) Change subject: jedec.c: Use JEDEC_ define from spi.h over magic ...................................................................... Patch Set 1: (1 comment) File jedec.c: https://review.coreboot.org/c/flashrom/+/72632/comment/1a40f12f_7e7f64d4 PS1, Line 135: JEDEC_REMS Are we sure that this is more readable than the "magic" (given that two out of the three values remain "magic") -- To view, visit https://review.coreboot.org/c/flashrom/+/72632 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id0c98ee900e37c26dc06f96a9950713f332240fd Gerrit-Change-Number: 72632 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 31 Jan 2023 22:38:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: jedec.c: Use JEDEC_ define from spi.h over magic
by Stefan Reinauer (Code Review) 31 Jan '23

31 Jan '23

Attention is currently required from: Edward O'Callaghan. Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72632 ) Change subject: jedec.c: Use JEDEC_ define from spi.h over magic ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: ``` jedec.c: In function 'probe_jedec_29gl': jedec.c:135:28: error: 'JEDEC_REMS' undeclared (first use in this function) 135 | chip_writeb(flash, JEDEC_REMS, bios + (0x5555 & mask)); | ^~~~~~~~~~ jedec.c:135:28: note: each undeclared identifier is reported only once for each function it appears in jedec.c: In function 'probe_jedec': jedec.c:219:28: error: 'JEDEC_REMS' undeclared (first use in this function) 219 | chip_writeb(flash, JEDEC_REMS, bios + ((shifted ? 0x2AAA : 0x5555) & mask)); | ^~~~~~~~~~ ``` -- To view, visit https://review.coreboot.org/c/flashrom/+/72632 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id0c98ee900e37c26dc06f96a9950713f332240fd Gerrit-Change-Number: 72632 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 31 Jan 2023 22:37:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: jedec.c: Fold up mask param into func
by Stefan Reinauer (Code Review) 31 Jan '23

31 Jan '23

Attention is currently required from: Edward O'Callaghan. Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72631 ) Change subject: jedec.c: Fold up mask param into func ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/72631 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iee4b1d5bc850cbeec0de9a1603fe04a3a1855330 Gerrit-Change-Number: 72631 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 31 Jan 2023 22:36:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: jedec.c: Rewrite control flow procedurally
by Stefan Reinauer (Code Review) 31 Jan '23

31 Jan '23

Attention is currently required from: Edward O'Callaghan. Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72608 ) Change subject: jedec.c: Rewrite control flow procedurally ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/72608 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0927ed40e54cc7e114a57dc40e3614f4825a0ca9 Gerrit-Change-Number: 72608 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 31 Jan 2023 22:35:31 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: jedec.c: Move printlock stuff into printlock.c
by Stefan Reinauer (Code Review) 31 Jan '23

31 Jan '23

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan. Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72607 ) Change subject: jedec.c: Move printlock stuff into printlock.c ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/72607 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iacaa16c81e141aac30feb6871700c4fdc9eec8e9 Gerrit-Change-Number: 72607 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 31 Jan 2023 22:33:52 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit