Attention is currently required from: Nikolai Artemiev, Stefan Reinauer.
Anton Samsonov has posted comments on this change by Anton Samsonov. ( https://review.coreboot.org/c/flashrom/+/85004?usp=email )
Change subject: flashchips: Update test status for Spansion S25FL256L
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> how recent is the source you tested on?
According to saved logs, the original (unpatched) version was 1.4.0-65-g4bf0eb3d, while the version with this patch applied was 1.4.0-76-gcbdb853.
--
To view, visit https://review.coreboot.org/c/flashrom/+/85004?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: I9e934791cd8d96c2cb926fd310ec48ec2ab2d1e3
Gerrit-Change-Number: 85004
Gerrit-PatchSet: 1
Gerrit-Owner: Anton Samsonov <avscomputing(a)gmail.com>
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Fri, 08 Nov 2024 17:14:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
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 9:
(1 comment)
Patchset:
PS9:
> Since it's all approved, and will be submitted in few days (the guidelines are here https://flashrom […]
Yes I was already hopeful that it might get done in time when I read about the new release.
I will definitly make the announcement once everything is merged.
Glad to be a part of flashrom :)
--
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: 9
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-Comment-Date: Fri, 08 Nov 2024 14:03:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Attention is currently required from: Matti Finder.
Anastasia Klimchuk 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 9:
(1 comment)
Patchset:
PS9:
Since it's all approved, and will be submitted in few days (the guidelines are here https://flashrom.org/dev_guide/development_guide.html#merging-patches), I wanted to say:
I am so glad that you made it in the perfect timing: in a week we cut an v1.5 rc1, so the new cool feature goes in! Congrats! ;)
It would be great if, after the patches are submitted, Matti, if you can make a post on the mailing list, to tell the people about new feature. It can be short post, but important things to mention are:
- RPMC support will be included in the upcoming release v1.5
- Currently the only way to use it is via SFDP, so please add `-c "SFDP-capable chip"`
- Add link to manpage-webpage (it might take few hours after submitting the patch, but then website will be updated)
If you are not subscribed to the mailing list, the info is here: https://flashrom.org/contact.html#mailing-list-1
Thank you so much for all your work!
--
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: 9
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-Comment-Date: Fri, 08 Nov 2024 13:23:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Matti Finder, Peter Marheine.
Anastasia Klimchuk 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 14: Code-Review+2
--
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: 14
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>
Gerrit-Comment-Date: Fri, 08 Nov 2024 12:53:43 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Anton Samsonov, Nikolai Artemiev, Stefan Reinauer.
Anastasia Klimchuk has posted comments on this change by Anton Samsonov. ( https://review.coreboot.org/c/flashrom/+/85004?usp=email )
Change subject: flashchips: Update test status for Spansion S25FL256L
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
Patchset:
PS1:
Hello Anton, thanks for testing! Just for my information, how recent is the source you tested on?
--
To view, visit https://review.coreboot.org/c/flashrom/+/85004?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: I9e934791cd8d96c2cb926fd310ec48ec2ab2d1e3
Gerrit-Change-Number: 85004
Gerrit-PatchSet: 1
Gerrit-Owner: Anton Samsonov <avscomputing(a)gmail.com>
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Anton Samsonov <avscomputing(a)gmail.com>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Fri, 08 Nov 2024 12:22:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
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 14:
(2 comments)
File doc/classic_cli_manpage.rst:
https://review.coreboot.org/c/flashrom/+/84840/comment/e78f6460_d3ed8678?us… :
PS13, Line 24: | [-n] [-N] [-f]
> This is duplicate of line 22, remove this one
Done
https://review.coreboot.org/c/flashrom/+/84840/comment/36d5932c_2fdfe1b4?us… :
PS13, Line 29: | [-V[V[V]]] [-o <logfile>] [--progress]
> This is a duplicate of line 23, I assume you were planning to add new options at the end of the list […]
No they were supposed to be in the same block as the other rpmc commands, similar to how the wp options were added.
Thank you for noticing, I missed that during the rebase.
--
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: 14
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: Fri, 08 Nov 2024 12:14:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
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 (#14).
The following approvals got outdated and were removed:
Code-Review+2 by Peter Marheine, 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/dev_guide/building_from_source.rst
M doc/release_notes/devel.rst
4 files changed, 307 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/84840/14
--
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: 14
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: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Attention: Matti Finder <matti.finder(a)gmail.com>
Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/84982?usp=email )
Change subject: Extract usbdev declarations to a separate header.
......................................................................
Extract 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>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/84982
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Miklós Márton <martonmiklosqdev(a)gmail.com>
Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org>
---
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(-)
Approvals:
Anastasia Klimchuk: Looks good to me, approved
build bot (Jenkins): Verified
Miklós Márton: Looks good to me, approved
diff --git a/dediprog.c b/dediprog.c
index ba1f9d6..05031fd 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: merged
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I9d819ea1c5bd51289d02189c1dff367ce6d25617
Gerrit-Change-Number: 84982
Gerrit-PatchSet: 5
Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Attention is currently required from: Antonio Vázquez Blanco.
Anastasia Klimchuk has posted comments on this change by Antonio Vázquez Blanco. ( https://review.coreboot.org/c/flashrom/+/84985?usp=email )
Change subject: Extract cli_output declarations to a separate header.
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
The same comment as on the CB:84983 , but also, one more thing.
I see that cli_output.c is not the latest. If you could rebase the patch on the latest head?
Also, given that the latest changes to cli_output.c were related to progress indicator, if you could run few tests after rebasing, for example:
> flashrom -p dummy:emulate=W25Q128FV,freq=64mhz -r dump.rom --progress
> flashrom -p dummy:emulate=W25Q128FV,freq=64mhz -E --progress
--
To view, visit https://review.coreboot.org/c/flashrom/+/84985?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: I45d62bd219bdbed919788ae17a64aeb119a8aac4
Gerrit-Change-Number: 84985
Gerrit-PatchSet: 2
Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Comment-Date: Fri, 08 Nov 2024 11:57:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Antonio Vázquez Blanco.
Anastasia Klimchuk has posted comments on this change by Antonio Vázquez Blanco. ( https://review.coreboot.org/c/flashrom/+/84983?usp=email )
Change subject: Rename cli_classic.h to a more adequate cli_getop.h
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
I agree with renaming cli_classic.h to cli_getop.h.
But there is also one more thing in this patch, changing the license header text, and I don't think it should be in the same commit.
I actually also agree with using SPDX tags, if we do it for all the source files, it's a great improvement, so much less lines of code. There was even a discussion at some point recently about that, and it's one of the things to do on my list.
But before starting changing all the headers there are few things I would want to check, and also in any case it would be separate commits.
So, long story short, would you mind to return back the old license headers, and keep only the renaming cli_classic.h -> cli_getop.h in this commit?
And I hope that we will do license headers later.
Thank you!
--
To view, visit https://review.coreboot.org/c/flashrom/+/84983?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: I5986828f3bdec583af6f7b02cbe1a9c45ed2000f
Gerrit-Change-Number: 84983
Gerrit-PatchSet: 2
Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Comment-Date: Fri, 08 Nov 2024 11:30:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No