November 2024 - flashrom-gerrit

[S] Change in flashrom[main]: Split usbdev declarations to a separate header.
by Antonio Vázquez Blanco (Code Review) 03 Nov '24

03 Nov '24

Attention is currently required from: Antonio Vázquez Blanco. Hello Miklós Márton, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/84982?usp=email to look at the new patch set (#2). Change subject: Split usbdev declarations to a separate header. ...................................................................... Split usbdev declarations to a separate header. This is a simple refactor that aims to simplify maintenance and to clarify file dependency inside the project. Currently, most of the declarations reside in programmer.h making it difficult to really understand file dependency. Change-Id: I9d819ea1c5bd51289d02189c1dff367ce6d25617 Signed-off-by: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> --- M dediprog.c M developerbox_spi.c M include/programmer.h A include/usbdev.h M stlinkv3_spi.c M usbdev.c 6 files changed, 34 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/82/84982/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/84982?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: I9d819ea1c5bd51289d02189c1dff367ce6d25617 Gerrit-Change-Number: 84982 Gerrit-PatchSet: 2 Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>

1 0

[S] Change in flashrom[main]: Split usbvev declarations to a separate header.
by Miklós Márton (Code Review) 03 Nov '24

03 Nov '24

Attention is currently required from: Antonio Vázquez Blanco. Miklós Márton has posted comments on this change by Antonio Vázquez Blanco. ( https://review.coreboot.org/c/flashrom/+/84982?usp=email ) Change subject: Split usbvev declarations to a separate header. ...................................................................... Patch Set 1: (2 comments) Patchset: PS1: Apart from the typo LGTM. Commit Message: https://review.coreboot.org/c/flashrom/+/84982/comment/cbc316aa_ce9c0c22?us… : PS1, Line 7: usbvev usbdev typo? -- To view, visit https://review.coreboot.org/c/flashrom/+/84982?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: I9d819ea1c5bd51289d02189c1dff367ce6d25617 Gerrit-Change-Number: 84982 Gerrit-PatchSet: 1 Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> Gerrit-Comment-Date: Sun, 03 Nov 2024 17:31:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in flashrom[main]: Split usbvev declarations to a separate header.
by Antonio Vázquez Blanco (Code Review) 03 Nov '24

03 Nov '24

Antonio Vázquez Blanco has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/84982?usp=email ) Change subject: Split usbvev declarations to a separate header. ...................................................................... Split usbvev declarations to a separate header. This is a simple refactor that aims to simplify maintenance and to clarify file dependency inside the project. Currently, most of the declarations reside in programmer.h making it difficult to really understand file dependency. Change-Id: I9d819ea1c5bd51289d02189c1dff367ce6d25617 Signed-off-by: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> --- M dediprog.c M developerbox_spi.c M include/programmer.h A include/usbdev.h M stlinkv3_spi.c M usbdev.c 6 files changed, 34 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/82/84982/1 diff --git a/dediprog.c b/dediprog.c index aa3a1cf..11e7d6c 100644 --- a/dediprog.c +++ b/dediprog.c @@ -26,6 +26,7 @@ #include "chipdrivers.h" #include "programmer.h" #include "spi.h" +#include "usbdev.h" /* LIBUSB_CALL ensures the right calling conventions on libusb callbacks. * However, the macro is not defined everywhere. m( diff --git a/developerbox_spi.c b/developerbox_spi.c index 64b7e8a..67d3d68 100644 --- a/developerbox_spi.c +++ b/developerbox_spi.c @@ -35,6 +35,7 @@ #include <libusb.h> #include "programmer.h" #include "spi.h" +#include "usbdev.h" /* Bit positions for each pin. */ #define DEVELOPERBOX_SPI_SCK 0 diff --git a/include/programmer.h b/include/programmer.h index 5ed9c8a..babb0bc 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -526,13 +526,4 @@ (flash->chip->feature_bits & (FEATURE_4BA_ENTER | FEATURE_4BA_ENTER_WREN | FEATURE_4BA_ENTER_EAR7)); } -/* usbdev.c */ -struct libusb_device_handle; -struct libusb_context; -struct libusb_device_handle *usb_dev_get_by_vid_pid_serial( - struct libusb_context *usb_ctx, uint16_t vid, uint16_t pid, const char *serialno); -struct libusb_device_handle *usb_dev_get_by_vid_pid_number( - struct libusb_context *usb_ctx, uint16_t vid, uint16_t pid, unsigned int num); - - #endif /* !__PROGRAMMER_H__ */ diff --git a/include/usbdev.h b/include/usbdev.h new file mode 100644 index 0000000..3bd64b3 --- /dev/null +++ b/include/usbdev.h @@ -0,0 +1,27 @@ +/* + * This file is part of the flashrom project. + * + * Copyright (C) 2024 Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef __USBDEV_H__ +#define __USBDEV_H__ 1 + +#include <libusb.h> + +struct libusb_device_handle *usb_dev_get_by_vid_pid_serial( + struct libusb_context *usb_ctx, uint16_t vid, uint16_t pid, const char *serialno); +struct libusb_device_handle *usb_dev_get_by_vid_pid_number( + struct libusb_context *usb_ctx, uint16_t vid, uint16_t pid, unsigned int num); + +#endif /* __USBDEV_H__ */ diff --git a/stlinkv3_spi.c b/stlinkv3_spi.c index 29aa8d3..a5c2289 100644 --- a/stlinkv3_spi.c +++ b/stlinkv3_spi.c @@ -26,6 +26,7 @@ #include "flash.h" #include "programmer.h" #include "spi.h" +#include "usbdev.h" #include <libusb.h> #include <limits.h> diff --git a/usbdev.c b/usbdev.c index 846ed58..02627d8 100644 --- a/usbdev.c +++ b/usbdev.c @@ -15,11 +15,13 @@ * GNU General Public License for more details. */ + +#include "usbdev.h" + +#include "flash.h" // msg_perr, msg_pdbg... #include <inttypes.h> #include <stdbool.h> #include <string.h> -#include <libusb.h> -#include "programmer.h" /* * Check whether we should filter the current device. -- To view, visit https://review.coreboot.org/c/flashrom/+/84982?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I9d819ea1c5bd51289d02189c1dff367ce6d25617 Gerrit-Change-Number: 84982 Gerrit-PatchSet: 1 Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>

1 0

[XS] Change in flashrom[main]: doc: Change link from gitiles to github
by Anastasia Klimchuk (Code Review) 03 Nov '24

03 Nov '24

Attention is currently required from: Peter Marheine. Hello Peter Marheine, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/84960?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: doc: Change link from gitiles to github ...................................................................... doc: Change link from gitiles to github gitiles are not always available, so the link was not always working, which could make readers confused. While we are here, add missing link to Dev Guide. Change-Id: I9103e5199bdf1b65fa3136aa01259a85e788a251 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/about_flashrom/team.rst 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/60/84960/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/84960?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: I9103e5199bdf1b65fa3136aa01259a85e788a251 Gerrit-Change-Number: 84960 Gerrit-PatchSet: 2 Gerrit-Owner: 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: Peter Marheine <pmarheine(a)chromium.org>

1 0

[L] Change in flashrom[main]: cli_client: Add rpmc command support
by Matti Finder (Code Review) 02 Nov '24

02 Nov '24

Attention is currently required from: Anastasia Klimchuk, Peter Marheine. Matti Finder has posted comments on this change by Matti Finder. ( https://review.coreboot.org/c/flashrom/+/84840?usp=email ) Change subject: cli_client: Add rpmc command support ...................................................................... Patch Set 8: (2 comments) File doc/classic_cli_manpage.rst: https://review.coreboot.org/c/flashrom/+/84840/comment/d67204e8_05fca319?us… : PS7, Line 329: : RPMC commands : ^^^^^^^^^^^^^ > Perhaps you can add a little intro which mentions that commands are available only if chip model sup […] Done File include/flash.h: https://review.coreboot.org/c/flashrom/+/84840/comment/fc5dc377_535149a4?us… : PS4, Line 581: rpmc_ctx > Ah, I think I understand now (also after reading CB:84934 with implementation). […] Yeah as of now the only way to use the rpmc comands is to set `-c "SFDP-capable chip"` manually. That is also what I did for testing. I think for vendors that still have enough ids to give special ones to rpmc capable devices, we can just or the FEATURE_FLASH_HARDENING bit and set the struct rpmc_ctx values accordingly. In the beginning I also thought about extending the probe function to also probe for rpmc capabilites or adding a new capability check to set feature bits. That seemed like a lot of hard to test changes, but it might be worth it for the future. But I'm not sure how the future of flashrom should look like. If maintaining a complicated flashchips.c is worth it, when most of the information can also be parsed from sfdp pages (for newer flashchips). -- To view, visit https://review.coreboot.org/c/flashrom/+/84840?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: I36c823bbee65f256eb6edabe6f058321c9a0cfa1 Gerrit-Change-Number: 84840 Gerrit-PatchSet: 8 Gerrit-Owner: Matti Finder <matti.finder(a)gmail.com> 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Sat, 02 Nov 2024 16:35:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Matti Finder <matti.finder(a)gmail.com>

1 0

[L] Change in flashrom[main]: rpmc: add rpmc commands feature
by Matti Finder (Code Review) 02 Nov '24

02 Nov '24

Attention is currently required from: Anastasia Klimchuk, Peter Marheine. Matti Finder has posted comments on this change by Matti Finder. ( https://review.coreboot.org/c/flashrom/+/84934?usp=email ) Change subject: rpmc: add rpmc commands feature ...................................................................... Patch Set 4: (7 comments) Patchset: PS3: > Given that you agreed in the other comment, you can add yourself in MAINTAINERS file and include in […] Done File rpmc.c: https://review.coreboot.org/c/flashrom/+/84934/comment/1d16add1_cfaf8bb3?us… : PS3, Line 131: Unsupported > Maybe `Unknown` instead of `Unsupported`, because it's not even in our enum. Done https://review.coreboot.org/c/flashrom/+/84934/comment/1d4bee90_291806c2?us… : PS3, Line 223: { > You don't need { } when the body of if conditional is only one line. […] Done https://review.coreboot.org/c/flashrom/+/84934/comment/d9dba094_29df7b76?us… : PS3, Line 224: return -1; > I am adding comment here, as this is an example, but it applies to other code in this file. […] My original of how to handle the return values wasn't that good, and just lead to a lot of questions. I have now switched it over to using a rpmc_result enum. Let me know if that makes the return values better to understand File sfdp.c: https://review.coreboot.org/c/flashrom/+/84934/comment/b066ede5_2b8a2ec1?us… : PS3, Line 446: hdrs[i].id > Can these header IDs ever repeat from one header to the other? (probably not?) […] No these ids are meant to be unique. From how I understand the original code. We query the device which maps these tables somewhere in the flash memory. Since page 0 is mandatory it is always the first one that is read. The ones after that are device specific. So there is only a relation for the first one. https://review.coreboot.org/c/flashrom/+/84934/comment/064aff5a_cd306c1a?us… : PS3, Line 449: msg_cdbg("The chip contains an unknown " : "version of the JEDEC flash " : "parameters table, skipping it.\n"); > I know it was like this before, but while you are here... maybe you can add `hdrs[i]. […] Done https://review.coreboot.org/c/flashrom/+/84934/comment/4aded69f_151b7de2?us… : PS3, Line 461: 1 > It's in hex few lines above (0x01 on line 448) and here is decimal. […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/84934?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: I6ab3d0446e9fd674b20550fdbfaf499b8d4a9b38 Gerrit-Change-Number: 84934 Gerrit-PatchSet: 4 Gerrit-Owner: Matti Finder <matti.finder(a)gmail.com> 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Sat, 02 Nov 2024 16:32:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[L] Change in flashrom[main]: cli_client: Add rpmc command support
by Matti Finder (Code Review) 02 Nov '24

02 Nov '24

Attention is currently required from: Matti Finder, Peter Marheine. Hello Anastasia Klimchuk, Peter Marheine, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/84840?usp=email to look at the new patch set (#8). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: cli_client: Add rpmc command support ...................................................................... cli_client: Add rpmc command support This commit adds uses the new rpmc command implementation to add them as a new feature to the cli_client. Also adds the necessary documentation for this new feature. Tested on the Winbond W25R128JV as a 'SFDP-capable chip'. This patch was done to add rpmc command support to flashrom. This enables users to write root keys to their flash chips while they flash data on the chip. This might become useful in the future as rpmc support is extended in coreboot. Also adds debug tools to flashrom, which might be useful in implementing coreboots rpmc support. Change-Id: I36c823bbee65f256eb6edabe6f058321c9a0cfa1 Signed-off-by: Matti Finder <matti.finder(a)gmail.com> --- M cli_classic.c M doc/classic_cli_manpage.rst M doc/release_notes/devel.rst 3 files changed, 257 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/84840/8 -- To view, visit https://review.coreboot.org/c/flashrom/+/84840?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: I36c823bbee65f256eb6edabe6f058321c9a0cfa1 Gerrit-Change-Number: 84840 Gerrit-PatchSet: 8 Gerrit-Owner: Matti Finder <matti.finder(a)gmail.com> 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: Matti Finder <matti.finder(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>

1 0

[L] Change in flashrom[main]: rpmc: add rpmc commands feature
by Matti Finder (Code Review) 02 Nov '24

02 Nov '24

Attention is currently required from: Matti Finder, Peter Marheine. Hello Anastasia Klimchuk, Peter Marheine, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/84934?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: rpmc: add rpmc commands feature ...................................................................... rpmc: add rpmc commands feature Added optional support for all the commands specified in JESD260. Added a new optional dependency to openssls libcrypto. Added parsing for the rpmc parameter sfdp table. Added necessary rpmc parameter information to flashchips struct and the flash hardening feature to enable rpmc commands. Enables future use of these commands in the cli_client and also libflashrom. Change-Id: I6ab3d0446e9fd674b20550fdbfaf499b8d4a9b38 Signed-off-by: Matti Finder <matti.finder(a)gmail.com> --- M MAINTAINERS M include/flash.h A include/rpmc.h M meson.build M meson_options.txt A rpmc.c M sfdp.c 7 files changed, 823 insertions(+), 15 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/34/84934/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/84934?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: I6ab3d0446e9fd674b20550fdbfaf499b8d4a9b38 Gerrit-Change-Number: 84934 Gerrit-PatchSet: 4 Gerrit-Owner: Matti Finder <matti.finder(a)gmail.com> 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: Matti Finder <matti.finder(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>

1 0

[S] Change in flashrom[main]: [TESTONLY]flashchips: add S25FL256L
by Anastasia Klimchuk (Code Review) 02 Nov '24

02 Nov '24

Anastasia Klimchuk has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/58223?usp=email ) Change subject: [TESTONLY]flashchips: add S25FL256L ...................................................................... Abandoned It seems, no one is working on this anymore -- To view, visit https://review.coreboot.org/c/flashrom/+/58223?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: abandon Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ia2e2c3b2b5e4ef510e2e39f0521318769234c278 Gerrit-Change-Number: 58223 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>

1 0

[XS] Change in flashrom[staging]: Constify arguments in flashrom_image_* prototypes
by Anastasia Klimchuk (Code Review) 02 Nov '24

02 Nov '24

Anastasia Klimchuk has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/20812?usp=email ) Change subject: Constify arguments in flashrom_image_* prototypes ...................................................................... Abandoned The patch is on the staging branch (which we don't use anymore), and has not been touched for >5 years. If anyone is interested to resurrect and finish this work, the patch can be restored. -- To view, visit https://review.coreboot.org/c/flashrom/+/20812?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: abandon Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Change-Id: I5840b95f9f9254e20d99bd01d0b1009c4b2252c4 Gerrit-Change-Number: 20812 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> 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>

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2024