August 2023 - flashrom-gerrit - mail.coreboot.org

[S] Change in flashrom[master]: flashchips.c: Add support for IS25WQ040
by Vasily Galkin (Code Review) 27 Aug '23

27 Aug '23

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer, Thomas Heijligen, roman.stingler(a)gmail.com. Vasily Galkin has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58179?usp=email ) Change subject: flashchips.c: Add support for IS25WQ040 ...................................................................... Patch Set 6: (3 comments) Patchset: PS6: Thanks for review! Seems-to-be-incorreect comments in the code are removed; rebase done too. I have no this chip at hand by now, so the current version is not tested, but ~1y ago I used previous variant for flashing such chip, so I hope it is still fine after rebasing. File flashchips.c: https://review.coreboot.org/c/flashrom/+/58179/comment/00ecbc7a_fc780a28 : PS5, Line 7792: 1024B > Page 47 says OTP is 256Bytes ? Also are you sure about read and write? This and next-line comments were the parts of the first version of a patch by Roman Stingler that I used as a basis. While using that patch as a base I verified the constants in the code, but completely foget to look in the comments. No I see that they didn't correspond to the datasheet. So I just remove those comments. https://review.coreboot.org/c/flashrom/+/58179/comment/e035bcdf_a27858c5 : PS5, Line 7793: /* QPI enable 0x35, disable 0xF5 */ > QPI is not in the datasheet? Removed this commands 0x35, 0xF5 are not mentioned in datasheet. However the QPI I/O mode itself is mentioned, so the FEATURE_QPI flag on the line below is kept -- To view, visit https://review.coreboot.org/c/flashrom/+/58179?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I072c6b94d7931637d1c2721c3316205f2d57320e Gerrit-Change-Number: 58179 Gerrit-PatchSet: 6 Gerrit-Owner: Vasily Galkin Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> 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-Reviewer: roman.stingler(a)gmail.com Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: roman.stingler(a)gmail.com Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Sat, 26 Aug 2023 23:34:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashchips.c: Add support for IS25WQ040
by Vasily Galkin (Code Review) 27 Aug '23

27 Aug '23

Attention is currently required from: Thomas Heijligen, Vasily Galkin, roman.stingler(a)gmail.com. Hello Anastasia Klimchuk, Thomas Heijligen, build bot (Jenkins), roman.stingler(a)gmail.com, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/58179?usp=email to look at the new patch set (#6). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: flashchips.c: Add support for IS25WQ040 ...................................................................... flashchips.c: Add support for IS25WQ040 Based on https://github.com/flashrom/flashrom/pull/204 squashed with fixes of IS25WQ040 size: it is 4Mbits, not 4MBytes, see https://www.issi.com/WW/pdf/25WQ020-040.pdf Tested read, write and erase with ft2232_spi-based "Tigard" programmer. Change-Id: I072c6b94d7931637d1c2721c3316205f2d57320e Signed-off-by: Roman Stingler <roman.stingler(a)gmail.com> Signed-off-by: Vasily Galkin <galkin-vv(a)ya.ru> --- M flashchips.c M include/flashchips.h 2 files changed, 41 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/58179/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/58179?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I072c6b94d7931637d1c2721c3316205f2d57320e Gerrit-Change-Number: 58179 Gerrit-PatchSet: 6 Gerrit-Owner: Vasily Galkin Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: roman.stingler(a)gmail.com Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Vasily Galkin Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: roman.stingler(a)gmail.com Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: flashchips: add WP features for W25X* analogous to tested W25X20
by Vasily Galkin (Code Review) 27 Aug '23

27 Aug '23

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer. Vasily Galkin has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/77531?usp=email ) Change subject: flashchips: add WP features for W25X* analogous to tested W25X20 ...................................................................... flashchips: add WP features for W25X* analogous to tested W25X20 These chips were not tested physically, just added since they are very similar to just added and tested W25X20. However basic logic test were done via running --wp-list with dummy emulating those IDs While there are per-chip datasheets like https://www.winbond.com/resource-files/W25X05CL_G%2008012019.pdf the combined datasheets that shows differences between WP modes of different chips are more useful: https://www.winbond.com/resource-files/w25x10a-20a-40a-80a%20revf%20080709.… https://media.digikey.com/pdf/data%20sheets/winbond%20pdfs/w25x16,16a,32,64… Signed-off-by: Vasily Galkin <galkin-vv(a)ya.ru> Change-Id: Ie69660a6f69e3cac31c5565792f401e69d43f8b8 --- M flashchips.c 1 file changed, 48 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/31/77531/1 diff --git a/flashchips.c b/flashchips.c index 4edceb0..d64a03a 100644 --- a/flashchips.c +++ b/flashchips.c @@ -19682,6 +19682,12 @@ .write = SPI_CHIP_WRITE256, .read = SPI_CHIP_READ, .voltage = {2300, 3600}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}}, + }, + .decode_range = DECODE_RANGE_SPI25, }, { @@ -19714,6 +19720,13 @@ .write = SPI_CHIP_WRITE256, .read = SPI_CHIP_READ, .voltage = {2700, 3600}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}}, + .tb = {STATUS1, 5, RW}, + }, + .decode_range = DECODE_RANGE_SPI25, }, { @@ -19752,6 +19765,13 @@ .write = SPI_CHIP_WRITE256, .read = SPI_CHIP_READ, .voltage = {2700, 3600}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}}, + .tb = {STATUS1, 5, RW}, + }, + .decode_range = DECODE_RANGE_SPI25, }, { @@ -19829,6 +19849,13 @@ .write = SPI_CHIP_WRITE256, .read = SPI_CHIP_READ, .voltage = {2700, 3600}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}}, + .tb = {STATUS1, 5, RW}, + }, + .decode_range = DECODE_RANGE_SPI25, }, { @@ -19861,6 +19888,13 @@ .write = SPI_CHIP_WRITE256, .read = SPI_CHIP_READ, .voltage = {2700, 3600}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}}, + .tb = {STATUS1, 5, RW}, + }, + .decode_range = DECODE_RANGE_SPI25, }, { @@ -19899,6 +19933,13 @@ .write = SPI_CHIP_WRITE256, .read = SPI_CHIP_READ, .voltage = {2700, 3600}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}}, + .tb = {STATUS1, 5, RW}, + }, + .decode_range = DECODE_RANGE_SPI25, }, { @@ -19931,6 +19972,13 @@ .write = SPI_CHIP_WRITE256, .read = SPI_CHIP_READ, .voltage = {2700, 3600}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}}, + .tb = {STATUS1, 5, RW}, + }, + .decode_range = DECODE_RANGE_SPI25, }, /* W29EE011, W29EE012, W29C010M, W29C011A do not support probe_jedec according to the datasheet, but it works for newer(?) steppings. */ -- To view, visit https://review.coreboot.org/c/flashrom/+/77531?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie69660a6f69e3cac31c5565792f401e69d43f8b8 Gerrit-Change-Number: 77531 Gerrit-PatchSet: 1 Gerrit-Owner: Vasily Galkin Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

[XS] Change in flashrom[master]: flashchips: Add WP features for Winbond W25X20
by Vasily Galkin (Code Review) 27 Aug '23

27 Aug '23

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer. Vasily Galkin has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/77530?usp=email ) Change subject: flashchips: Add WP features for Winbond W25X20 ...................................................................... flashchips: Add WP features for Winbond W25X20 WP-related registers list from official datasheet https://www.winbond.com/resource-files/w25x20cl_revf%2020150806.pdf Commandline options tested with ft2232_spi-based "Tigard" programmer: wp-disable wp-enable wp-list wp-status wp-range=0,0 wp-range=0,0x00040000 Signed-off-by: Vasily Galkin <galkin-vv(a)ya.ru> Change-Id: I82c0cc52ca2a78d27f513234cc12d3e09d8905a5 --- M flashchips.c 1 file changed, 8 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/30/77530/1 diff --git a/flashchips.c b/flashchips.c index 6920c03..4edceb0 100644 --- a/flashchips.c +++ b/flashchips.c @@ -19763,7 +19763,7 @@ .total_size = 256, .page_size = 256, .feature_bits = FEATURE_WRSR_WREN, - .tested = TEST_OK_PREW, + .tested = TEST_OK_PREWB, .probe = PROBE_SPI_RDID, .probe_timing = TIMING_ZERO, .block_erasers = @@ -19784,6 +19784,13 @@ .write = SPI_CHIP_WRITE256, .read = SPI_CHIP_READ, .voltage = {2700, 3600}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}}, + .tb = {STATUS1, 5, RW}, + }, + .decode_range = DECODE_RANGE_SPI25, }, { -- To view, visit https://review.coreboot.org/c/flashrom/+/77530?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I82c0cc52ca2a78d27f513234cc12d3e09d8905a5 Gerrit-Change-Number: 77530 Gerrit-PatchSet: 1 Gerrit-Owner: Vasily Galkin Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

[XS] Change in flashrom[master]: spi25_statusreg: add verbose output on status registers read results
by Vasily Galkin (Code Review) 27 Aug '23

27 Aug '23

Vasily Galkin has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/77529?usp=email ) Change subject: spi25_statusreg: add verbose output on status registers read results ...................................................................... spi25_statusreg: add verbose output on status registers read results Useful during enabling/debugging WP operations on various SPI chips Signed-off-by: Vasily Galkin <galkin-vv(a)ya.ru> Change-Id: Ibc8e9229ab5d6578479564d11cc7aff9442e24ad --- M spi25_statusreg.c 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/29/77529/1 diff --git a/spi25_statusreg.c b/spi25_statusreg.c index fa276f7..bb56ee1 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -262,6 +262,7 @@ } *value = readarr[0]; + msg_cspew("%s: read_cmd 0x%02x returned 0x%02x\n", __func__, read_cmd, readarr[0]); return 0; } -- To view, visit https://review.coreboot.org/c/flashrom/+/77529?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibc8e9229ab5d6578479564d11cc7aff9442e24ad Gerrit-Change-Number: 77529 Gerrit-PatchSet: 1 Gerrit-Owner: Vasily Galkin Gerrit-MessageType: newchange

1 0

[XS] Change in flashrom[master]: Makefile: Remove a bashism when searching for sphinx-build
by Patrick Georgi (Code Review) 25 Aug '23

25 Aug '23

Attention is currently required from: Bart De Schuymer, Peter Marheine, Thomas Heijligen. Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77288?usp=email ) Change subject: Makefile: Remove a bashism when searching for sphinx-build ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Patchset: PS2: As the resident portability nut, I approve of this change! -- To view, visit https://review.coreboot.org/c/flashrom/+/77288?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4d89e24ec3401446acec857eae134928bc3064d2 Gerrit-Change-Number: 77288 Gerrit-PatchSet: 2 Gerrit-Owner: Bart De Schuymer <bdschuym(a)artinalgorithms.be> Gerrit-Reviewer: Jan Samek <jan.samek(a)siemens.com> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> 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-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Bart De Schuymer <bdschuym(a)artinalgorithms.be> Gerrit-Comment-Date: Fri, 25 Aug 2023 13:40:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[master]: doc: Add html_logo and html_title to sphinx config
by Anastasia Klimchuk (Code Review) 25 Aug '23

25 Aug '23

Attention is currently required from: Alexander Goncharov, Stefan Reinauer, Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77438?usp=email ) Change subject: doc: Add html_logo and html_title to sphinx config ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: I missed the elephant in the room, there is a way to add logo in sphinx config. This one will be shown on every page (unlike CB:77250 which only shows on home page). Maybe it's fine to have both? If it turns out both are too much, we can remove the one from CB:77250. This one should stay because it is displayed on every page. -- To view, visit https://review.coreboot.org/c/flashrom/+/77438?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9a530f7143880393fffc2a97327416f2cd688586 Gerrit-Change-Number: 77438 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Fri, 25 Aug 2023 12:28:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[master]: doc: Add html_logo and html_title to sphinx config
by Anastasia Klimchuk (Code Review) 25 Aug '23

25 Aug '23

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/77438?usp=email ) Change subject: doc: Add html_logo and html_title to sphinx config ...................................................................... doc: Add html_logo and html_title to sphinx config Logo set in config will be shown at the top of the sidebar, which means it will be shown on every page. Change-Id: I9a530f7143880393fffc2a97327416f2cd688586 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/conf.py 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/38/77438/1 diff --git a/doc/conf.py b/doc/conf.py index be9cc01..df60be7 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -40,6 +40,9 @@ html_favicon = 'logo/flashrom_icon_color-32x32.ico' +html_logo = 'logo/flashrom_icon_color.svg' + +html_title = 'flashrom documentation' # -- Options for manual page output -------------------------------------------- man_make_section_directory = True -- To view, visit https://review.coreboot.org/c/flashrom/+/77438?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9a530f7143880393fffc2a97327416f2cd688586 Gerrit-Change-Number: 77438 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

[XS] Change in flashrom[master]: Makefile: Remove a bashism when searching for sphinx-build
by Anastasia Klimchuk (Code Review) 25 Aug '23

25 Aug '23

Attention is currently required from: Bart De Schuymer, Peter Marheine, Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77288?usp=email ) Change subject: Makefile: Remove a bashism when searching for sphinx-build ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: Bart, sorry I forgot to ask previously, did you have a chance to test run with both dash and bash? Otherwise that's it, that's only question I have left. -- To view, visit https://review.coreboot.org/c/flashrom/+/77288?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4d89e24ec3401446acec857eae134928bc3064d2 Gerrit-Change-Number: 77288 Gerrit-PatchSet: 2 Gerrit-Owner: Bart De Schuymer <bdschuym(a)artinalgorithms.be> Gerrit-Reviewer: Jan Samek <jan.samek(a)siemens.com> 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-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Bart De Schuymer <bdschuym(a)artinalgorithms.be> Gerrit-Comment-Date: Fri, 25 Aug 2023 11:54:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashchips: Add support for MXIC MX2525643G It is similar to the MX25...
by Anastasia Klimchuk (Code Review) 25 Aug '23

25 Aug '23

Attention is currently required from: Name of user not set #1005084. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/76853?usp=email ) Change subject: flashchips: Add support for MXIC MX2525643G It is similar to the MX2525635F and shares its RDID. ...................................................................... Patch Set 1: (10 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/76853/comment/5b7e47b2_fa06df8f : PS1, Line 7: MX2525643G Let's use exact name MX25U25643G , can you please update? https://review.coreboot.org/c/flashrom/+/76853/comment/c63ecb56_34d773ce : PS1, Line 8: MX2525635F Same here, probably MX25U25635F https://review.coreboot.org/c/flashrom/+/76853/comment/978d2742_ecc2afdf : PS1, Line 10: Tested: read, write and erase. Could you please add info which programmer you ran? https://review.coreboot.org/c/flashrom/+/76853/comment/82c36f2b_9b710bbd : PS1, Line 12: Datasheet is available at the following URL: : https://www.mxic.com.tw/Lists/Datasheet/Attachments/8766/MX2525643G,%201.8V… You need to update the link (the new one you gave me in the comment works) Patchset: PS1: > Dear Anastasia Klimchuk […] Thank you, new link works, I opened datasheet successfully. I added some comments, you will need to make changes to your patch and then upload new version and respond to comments. If you have questions, feel free to ask! File flashchips.c: https://review.coreboot.org/c/flashrom/+/76853/comment/1788f041_bcb07613 : PS1, Line 10094: MACRONIX_MX25U25643G After you fix my other comment about id, you will change this to MACRONIX_MX25U25635F https://review.coreboot.org/c/flashrom/+/76853/comment/56f7cf37_7ba56c55 : PS1, Line 10104: { : .eraseblocks = { {4 * 1024, 8192} }, : .block_erase = SPI_BLOCK_ERASE_21, : }, Maybe I am missing something, I don't see this in the datasheet. I can see 20h for sector erase but not 21? https://review.coreboot.org/c/flashrom/+/76853/comment/ee93c4dd_8d63d827 : PS1, Line 10113: { : .eraseblocks = { {32 * 1024, 1024} }, : .block_erase = SPI_BLOCK_ERASE_52, : }, same https://review.coreboot.org/c/flashrom/+/76853/comment/aff5d7a2_e211f672 : PS1, Line 10117: .eraseblocks = { {64 * 1024, 512} }, : .block_erase = SPI_BLOCK_ERASE_DC, : }, { same File include/flashchips.h: https://review.coreboot.org/c/flashrom/+/76853/comment/15ae0d87_479973b2 : PS1, Line 530: #define MACRONIX_MX25U25643G 0x2539 Don't add the same id, add a comment on existing id (MACRONIX_MX25U25635F), like that: /* Same as MACRONIX_MX25U25635F */ You can look at examples of the same situation with same ids just few lines above in this file. -- To view, visit https://review.coreboot.org/c/flashrom/+/76853?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie04a5e2325aab94bffb276675be3fa4a88c6e134 Gerrit-Change-Number: 76853 Gerrit-PatchSet: 1 Gerrit-Owner: Name of user not set #1005084 Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Name of user not set #1005084 Gerrit-Comment-Date: Fri, 25 Aug 2023 10:14:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Name of user not set #1005084 Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit August 2023