Attention is currently required from: Antonio Vázquez Blanco, David Reguera Garcia (Dreg), Matti Finder, Miklós Márton, Nikolai Artemiev, Peter Marheine, Sergii Dmytruk.
Anastasia Klimchuk has posted comments on this change by Antonio Vázquez Blanco. ( https://review.coreboot.org/c/flashrom/+/85539?usp=email )
Change subject: Move SPI declarations from flash.h to spi.h
......................................................................
Patch Set 7: Code-Review+2
(1 comment)
Patchset:
PS7:
looks good to me! I will give some time for others to have a look too
--
To view, visit https://review.coreboot.org/c/flashrom/+/85539?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I794a71536a3b85fde39f83c802fa0f5dd8d428e0
Gerrit-Change-Number: 85539
Gerrit-PatchSet: 7
Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: David Reguera Garcia (Dreg) <regueragarciadavid(a)gmail.com>
Gerrit-Reviewer: Matti Finder <matti.finder(a)gmail.com>
Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: David Reguera Garcia (Dreg) <regueragarciadavid(a)gmail.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Attention: Matti Finder <matti.finder(a)gmail.com>
Gerrit-Comment-Date: Tue, 18 Feb 2025 05:24:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: Anastasia Klimchuk, David Reguera Garcia (Dreg), Matti Finder, Miklós Márton, Nikolai Artemiev, Peter Marheine, Sergii Dmytruk.
Antonio Vázquez Blanco has posted comments on this change by Antonio Vázquez Blanco. ( https://review.coreboot.org/c/flashrom/+/85539?usp=email )
Change subject: Move SPI declarations from flash.h to spi.h
......................................................................
Patch Set 7:
(4 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/85539/comment/cf803667_ebd64052?us… :
PS2, Line 7: Extract SPI declarations to the correct header
> Nit: "extract" usually means creating something new in the process of move (header in this case), so […]
Message was not very descriptive. I hope is better now!
https://review.coreboot.org/c/flashrom/+/85539/comment/8153cfe9_dcbf0b0e?us… :
PS2, Line 8:
> My only comment here is that now that the patch goes across the whole tree, maybe you can add to a c […]
I also tried to improve the message! Done!
File edi.c:
https://review.coreboot.org/c/flashrom/+/85539/comment/9e0b9de5_c2a4c1ac?us… :
PS2, Line 18: #include <spi.h>
> These are not system includes, so the newly-added lines should still be quoted (`"spi.h"`). […]
Thanks for the pointer! I will try to keep this consistent. Done :)
File include/spi.h:
https://review.coreboot.org/c/flashrom/+/85539/comment/632f4f1c_4c4a7c01?us… :
PS2, Line 19: #include <flash.h>
> Same as for `spi.h`, quote this instead of using angle brackets.
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/85539?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I794a71536a3b85fde39f83c802fa0f5dd8d428e0
Gerrit-Change-Number: 85539
Gerrit-PatchSet: 7
Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: David Reguera Garcia (Dreg) <regueragarciadavid(a)gmail.com>
Gerrit-Reviewer: Matti Finder <matti.finder(a)gmail.com>
Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: David Reguera Garcia (Dreg) <regueragarciadavid(a)gmail.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Attention: Matti Finder <matti.finder(a)gmail.com>
Gerrit-Comment-Date: Mon, 17 Feb 2025 21:43:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>
Attention is currently required from: Antonio Vázquez Blanco, David Reguera Garcia (Dreg), Matti Finder, Miklós Márton, Nikolai Artemiev.
Hello Anastasia Klimchuk, David Reguera Garcia (Dreg), Matti Finder, Miklós Márton, Nikolai Artemiev, Peter Marheine, Sergii Dmytruk, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/85539?usp=email
to look at the new patch set (#7).
Change subject: Move SPI declarations from flash.h to spi.h
......................................................................
Move SPI declarations from flash.h to spi.h
As a consecuence, some of the files that used to include flash.h no
longer need to do so. For this reason, flash.h includes are also deleted
in this commit.
Change-Id: I794a71536a3b85fde39f83c802fa0f5dd8d428e0
Signed-off-by: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
---
M amd_imc.c
M at45db.c
M bitbang_spi.c
M buspirate_spi.c
M dediprog.c
M dummyflasher.c
M edi.c
M ft2232_spi.c
M ichspi.c
M include/cli_output.h
M include/flash.h
M include/programmer.h
M include/spi.h
M it87spi.c
M jlink_spi.c
M linux_spi.c
M mediatek_i2c_spi.c
M mstarddc_spi.c
M ni845x_spi.c
M nicintel_eeprom.c
M pickit2_spi.c
M rpmc.c
M sb600spi.c
M sfdp.c
M spi.c
M spi25.c
M spi25_statusreg.c
M stlinkv3_spi.c
M usbblaster_spi.c
M wbsio_spi.c
M writeprotect.c
31 files changed, 18 insertions(+), 42 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/39/85539/7
--
To view, visit https://review.coreboot.org/c/flashrom/+/85539?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I794a71536a3b85fde39f83c802fa0f5dd8d428e0
Gerrit-Change-Number: 85539
Gerrit-PatchSet: 7
Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: David Reguera Garcia (Dreg) <regueragarciadavid(a)gmail.com>
Gerrit-Reviewer: Matti Finder <matti.finder(a)gmail.com>
Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: David Reguera Garcia (Dreg) <regueragarciadavid(a)gmail.com>
Gerrit-Attention: Matti Finder <matti.finder(a)gmail.com>
Attention is currently required from: Antonio Vázquez Blanco, David Reguera Garcia (Dreg), Matti Finder, Miklós Márton, Nikolai Artemiev.
Hello Anastasia Klimchuk, David Reguera Garcia (Dreg), Matti Finder, Miklós Márton, Nikolai Artemiev, Peter Marheine, Sergii Dmytruk, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/85539?usp=email
to look at the new patch set (#6).
Change subject: Extract SPI declarations to the correct header
......................................................................
Extract SPI declarations to the correct header
Change-Id: I794a71536a3b85fde39f83c802fa0f5dd8d428e0
Signed-off-by: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
---
M amd_imc.c
M at45db.c
M bitbang_spi.c
M buspirate_spi.c
M dediprog.c
M dummyflasher.c
M edi.c
M ft2232_spi.c
M ichspi.c
M include/cli_output.h
M include/flash.h
M include/programmer.h
M include/spi.h
M it87spi.c
M jlink_spi.c
M linux_spi.c
M mediatek_i2c_spi.c
M mstarddc_spi.c
M ni845x_spi.c
M nicintel_eeprom.c
M pickit2_spi.c
M rpmc.c
M sb600spi.c
M sfdp.c
M spi.c
M spi25.c
M spi25_statusreg.c
M stlinkv3_spi.c
M usbblaster_spi.c
M wbsio_spi.c
M writeprotect.c
31 files changed, 18 insertions(+), 42 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/39/85539/6
--
To view, visit https://review.coreboot.org/c/flashrom/+/85539?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I794a71536a3b85fde39f83c802fa0f5dd8d428e0
Gerrit-Change-Number: 85539
Gerrit-PatchSet: 6
Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: David Reguera Garcia (Dreg) <regueragarciadavid(a)gmail.com>
Gerrit-Reviewer: Matti Finder <matti.finder(a)gmail.com>
Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: David Reguera Garcia (Dreg) <regueragarciadavid(a)gmail.com>
Gerrit-Attention: Matti Finder <matti.finder(a)gmail.com>
Attention is currently required from: Antonio Vázquez Blanco, David Reguera Garcia (Dreg), Matti Finder, Miklós Márton, Nikolai Artemiev.
Hello Anastasia Klimchuk, David Reguera Garcia (Dreg), Matti Finder, Miklós Márton, Nikolai Artemiev, Peter Marheine, Sergii Dmytruk, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/85539?usp=email
to look at the new patch set (#5).
Change subject: Extract SPI declarations to the correct header
......................................................................
Extract SPI declarations to the correct header
Change-Id: I794a71536a3b85fde39f83c802fa0f5dd8d428e0
Signed-off-by: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
---
M amd_imc.c
M at45db.c
M bitbang_spi.c
M buspirate_spi.c
M dediprog.c
M dummyflasher.c
M edi.c
M ft2232_spi.c
M ichspi.c
M include/cli_output.h
M include/flash.h
M include/programmer.h
M include/spi.h
M it87spi.c
M jlink_spi.c
M linux_spi.c
M mediatek_i2c_spi.c
M mstarddc_spi.c
M ni845x_spi.c
M nicintel_eeprom.c
M pickit2_spi.c
M rpmc.c
M sb600spi.c
M sfdp.c
M spi.c
M spi25.c
M spi25_statusreg.c
M stlinkv3_spi.c
M usbblaster_spi.c
M wbsio_spi.c
M writeprotect.c
31 files changed, 18 insertions(+), 42 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/39/85539/5
--
To view, visit https://review.coreboot.org/c/flashrom/+/85539?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I794a71536a3b85fde39f83c802fa0f5dd8d428e0
Gerrit-Change-Number: 85539
Gerrit-PatchSet: 5
Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: David Reguera Garcia (Dreg) <regueragarciadavid(a)gmail.com>
Gerrit-Reviewer: Matti Finder <matti.finder(a)gmail.com>
Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: David Reguera Garcia (Dreg) <regueragarciadavid(a)gmail.com>
Gerrit-Attention: Matti Finder <matti.finder(a)gmail.com>
Attention is currently required from: Antonio Vázquez Blanco, David Reguera Garcia (Dreg), Matti Finder, Miklós Márton, Nikolai Artemiev.
Hello Anastasia Klimchuk, David Reguera Garcia (Dreg), Matti Finder, Miklós Márton, Nikolai Artemiev, Peter Marheine, Sergii Dmytruk, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/85539?usp=email
to look at the new patch set (#4).
Change subject: Extract SPI declarations to the correct header
......................................................................
Extract SPI declarations to the correct header
Change-Id: I794a71536a3b85fde39f83c802fa0f5dd8d428e0
Signed-off-by: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
---
M amd_imc.c
M at45db.c
M bitbang_spi.c
M buspirate_spi.c
M dediprog.c
M dummyflasher.c
M edi.c
M ft2232_spi.c
M ichspi.c
M include/cli_output.h
M include/flash.h
M include/programmer.h
M include/spi.h
M it87spi.c
M jlink_spi.c
M linux_spi.c
M mediatek_i2c_spi.c
M mstarddc_spi.c
M ni845x_spi.c
M nicintel_eeprom.c
M pickit2_spi.c
M rpmc.c
M sb600spi.c
M sfdp.c
M spi.c
M spi25.c
M spi25_statusreg.c
M stlinkv3_spi.c
M usbblaster_spi.c
M wbsio_spi.c
M writeprotect.c
31 files changed, 18 insertions(+), 42 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/39/85539/4
--
To view, visit https://review.coreboot.org/c/flashrom/+/85539?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I794a71536a3b85fde39f83c802fa0f5dd8d428e0
Gerrit-Change-Number: 85539
Gerrit-PatchSet: 4
Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: David Reguera Garcia (Dreg) <regueragarciadavid(a)gmail.com>
Gerrit-Reviewer: Matti Finder <matti.finder(a)gmail.com>
Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: David Reguera Garcia (Dreg) <regueragarciadavid(a)gmail.com>
Gerrit-Attention: Matti Finder <matti.finder(a)gmail.com>
Attention is currently required from: Antonio Vázquez Blanco, David Reguera Garcia (Dreg), Matti Finder, Miklós Márton, Nikolai Artemiev.
Hello Anastasia Klimchuk, David Reguera Garcia (Dreg), Matti Finder, Miklós Márton, Nikolai Artemiev, Peter Marheine, Sergii Dmytruk, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/85539?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: Extract SPI declarations to the correct header
......................................................................
Extract SPI declarations to the correct header
Change-Id: I794a71536a3b85fde39f83c802fa0f5dd8d428e0
Signed-off-by: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
---
M amd_imc.c
M at45db.c
M bitbang_spi.c
M buspirate_spi.c
M dediprog.c
M dummyflasher.c
M edi.c
M ft2232_spi.c
M ichspi.c
M include/flash.h
M include/programmer.h
M include/spi.h
M it87spi.c
M jlink_spi.c
M linux_spi.c
M mediatek_i2c_spi.c
M mstarddc_spi.c
M ni845x_spi.c
M nicintel_eeprom.c
M pickit2_spi.c
M rpmc.c
M sb600spi.c
M sfdp.c
M spi.c
M spi25.c
M spi25_statusreg.c
M stlinkv3_spi.c
M usbblaster_spi.c
M wbsio_spi.c
M writeprotect.c
30 files changed, 17 insertions(+), 41 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/39/85539/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/85539?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I794a71536a3b85fde39f83c802fa0f5dd8d428e0
Gerrit-Change-Number: 85539
Gerrit-PatchSet: 3
Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: David Reguera Garcia (Dreg) <regueragarciadavid(a)gmail.com>
Gerrit-Reviewer: Matti Finder <matti.finder(a)gmail.com>
Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: David Reguera Garcia (Dreg) <regueragarciadavid(a)gmail.com>
Gerrit-Attention: Matti Finder <matti.finder(a)gmail.com>
Attention is currently required from: Peter Marheine, Simon Arlott.
Anastasia Klimchuk has posted comments on this change by Simon Arlott. ( https://review.coreboot.org/c/flashrom/+/86411?usp=email )
Change subject: spidriver: Add support for the Excamera Labs SPIDriver programmer
......................................................................
Patch Set 2:
(11 comments)
Patchset:
PS2:
Simon, nice to meet you, thank you for the patch! This is really cool :)
I have added comment to the files, and I have two more comments to the patch in general:
1) We have a document where all the things that are added since the latest release, are accumulated. It helps to write release notes, and also for people who are interested.
The doc is here https://www.flashrom.org/release_notes/devel.html
Could you please add a section to that doc about your new programmer? It doesn't have to be long text, and you can link manpage (especially if you do my other comment about manpage)
source code here: `/doc/release_notes/devel.rst`
2) Would you agree to be a maintainer for your programmer? This means if someone sends a patch which modifies your programmer, you are automatically added as a reviewer (you can always add more reviewers, not a problem).
Duties are explained here, you will be in "flashrom reviewers" group: https://www.flashrom.org/about_flashrom/team.html
If you agree, add yourself to MAINTAINERS file, the file path is the c file of the programmer. You can include it in this patch (I mean, if you agree :) )
Thank you!
Commit Message:
https://review.coreboot.org/c/flashrom/+/86411/comment/8ddf5676_b55ef4c3?us… :
PS2, Line 9: This is a SPI hardware interface with a display (https://spidriver.com/),
: connected as an FT230X USB serial device at a fixed baud rate of 460800.
:
: Firmware: https://github.com/jamesbowman/spidriver
: Protocol: https://github.com/jamesbowman/spidriver/blob/master/protocol.md
These lines would be very good to add in the beginning of manpage entry that you added.
But also keep them here too.
File doc/classic_cli_manpage.rst:
https://review.coreboot.org/c/flashrom/+/86411/comment/fc6345bb_e55af17b?us… :
PS2, Line 359: spidriver
I am thinking about naming, maybe excamera_spidriver will be better? I understand that SPIDriver is literally the name of the device, but it might be too generic. Lots of other programmers names include vendor names.
What do you think? Is SPIDriver well known as a name?
File include/programmer.h:
https://review.coreboot.org/c/flashrom/+/86411/comment/feb3451c_91645dc4?us… :
PS2, Line 101: extern const struct programmer_entry programmer_spidriver;
You need to swap lines 100 and 101 to be in nice alphabetical order.
(I am realising after looking through your patch that half of the places is not in the abc order :\ but at least this file is )
File spidriver.c:
https://review.coreboot.org/c/flashrom/+/86411/comment/27173a0b_d5ea5158?us… :
PS2, Line 19: */
Maybe add link to firmware here as well? (from commit message)
https://review.coreboot.org/c/flashrom/+/86411/comment/25215dca_d891decd?us… :
PS2, Line 81: {
Why do you need this bracket (and closing on line 101)? Is there a reason for this block?
https://review.coreboot.org/c/flashrom/+/86411/comment/caf5e834_ef6690e2?us… :
PS2, Line 136: /* Read and write */
I am just curious, there is a way to write, or the way to read and write, but no way to only read? I see it's also in the protocol, so it's not a question to your implementation. But I am wondering why there is not read?
https://review.coreboot.org/c/flashrom/+/86411/comment/d6bc7ebd_c81f6442?us… :
PS2, Line 179: .max_data_read = MAX_DATA_READ_UNLIMITED,
: .max_data_write = MAX_DATA_WRITE_UNLIMITED,
The indentation here needs to be aligned with the rest of `=`
https://review.coreboot.org/c/flashrom/+/86411/comment/1abed391_c3e0ba26?us… :
PS2, Line 211: Invalid SPI mode %ld
Lets add the second part to the error message: "Valid values are <...>"
https://review.coreboot.org/c/flashrom/+/86411/comment/d4d71c26_92cdf9bc?us… :
PS2, Line 212: return 1;
So invalid `mode` param returns an error, but invalid values for `a` and `b` below continue running with default values.
Why is this difference in behaviour? Can it be the same?
I would lean to throwing an error in case of invalid value, and only use default in param is absent completely. It feels safer, so that the user won't accidentally run when they provided invalid value and maybe they meant something else.
https://review.coreboot.org/c/flashrom/+/86411/comment/2163f0b2_a788c993?us… :
PS2, Line 309: version 1
Is it useful to print `fw_version` here instead, as an in addition to ?
--
To view, visit https://review.coreboot.org/c/flashrom/+/86411?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I07b23c1146d4ad3606b54a1e8dc8030cf4ebf57b
Gerrit-Change-Number: 86411
Gerrit-PatchSet: 2
Gerrit-Owner: Simon Arlott <flashrom.simon(a)arlott.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Simon Arlott <flashrom.simon(a)arlott.org>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Mon, 17 Feb 2025 11:59:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Anastasia Klimchuk, Peter Marheine, Richard Hughes.
Sergii Dmytruk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/86031?usp=email )
Change subject: libflashrom: Update the API for progress callback
......................................................................
Patch Set 8: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/86031?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Ia8cc0461c449b7e65888a64cdc594c55b81eae7a
Gerrit-Change-Number: 86031
Gerrit-PatchSet: 8
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Richard Hughes <richard(a)hughsie.com>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Richard Hughes <richard(a)hughsie.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Sun, 16 Feb 2025 11:00:13 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Peter Marheine, Richard Hughes, Sergii Dmytruk.
Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/86031?usp=email )
Change subject: libflashrom: Update the API for progress callback
......................................................................
Patch Set 8:
(3 comments)
File include/libflashrom.h:
https://review.coreboot.org/c/flashrom/+/86031/comment/cfb97eca_910e3b47?us… :
PS7, Line 112: */
> The comment should probably have a note saying that this has priority over `flashrom_set_progress_ca […]
Done
File libflashrom.c:
https://review.coreboot.org/c/flashrom/+/86031/comment/984ecd1a_74d3d268?us… :
PS7, Line 76: " ignoring this call since %s is deprecated\n",
> nit: there is already a space on the previous line
Done
https://review.coreboot.org/c/flashrom/+/86031/comment/b589c1f6_4f0a52dc?us… :
PS7, Line 134: sizeof(flashctx->progress_state.user_data));
> nit: Why use `memcpy()`? Types match, so simple assignments should do and will break if types chang […]
user_data was complaining on dereferencing void pointer so I changed to `memcpy()` and then I changed progress_state copying to memcpy so that they look the same.
For progress_state it wasn't needed, I changed it back to be an assignment.
--
To view, visit https://review.coreboot.org/c/flashrom/+/86031?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Ia8cc0461c449b7e65888a64cdc594c55b81eae7a
Gerrit-Change-Number: 86031
Gerrit-PatchSet: 8
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Richard Hughes <richard(a)hughsie.com>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Richard Hughes <richard(a)hughsie.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Sun, 16 Feb 2025 08:20:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>