Attention is currently required from: Thomas Heijligen, Nicholas Chin.
ChrisEric1 CECL has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72057 )
Change subject: Add support for VIA VL805 USB 3 XHCI flashing
......................................................................
Patch Set 11:
(1 comment)
File vl805.c:
https://review.coreboot.org/c/flashrom/+/72057/comment/eea78dab_2ffdb838
PS3, Line 38: static struct pci_dev *dev = NULL;
> Yes, you have to define an custom data struct which is passed along. […]
@nic3-14159#0913 on coreboot discord helped me out a lot, it has been fixed (still needs testing)
--
To view, visit https://review.coreboot.org/c/flashrom/+/72057
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I71435afcacdf97e14d627e35bce3d29de9657f38
Gerrit-Change-Number: 72057
Gerrit-PatchSet: 11
Gerrit-Owner: ChrisEric1 CECL <christopherericlentocha(a)gmail.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Tue, 14 Feb 2023 17:10:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: ChrisEric1 CECL <christopherericlentocha(a)gmail.com>
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: comment
Attention is currently required from: ChrisEric1 CECL, Nicholas Chin.
Hello build bot (Jenkins), Thomas Heijligen,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/72057
to look at the new patch set (#12).
Change subject: Add support for VIA VL805 USB 3 XHCI flashing
......................................................................
Add support for VIA VL805 USB 3 XHCI flashing
It works fine for me on my Raspberry Pi 4 Model B. Was able read read,
erase, write, verify. Only thing is the WP with the W25X10 gives a
untested warning.
Change-Id: I71435afcacdf97e14d627e35bce3d29de9657f38
Signed-off-by: Christopher Lentocha <christopherericlentocha(a)gmail.com>
---
M Makefile
M flashrom.8.tmpl
M include/programmer.h
M meson.build
M meson_options.txt
M programmer_table.c
A vl805.c
7 files changed, 226 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/57/72057/12
--
To view, visit https://review.coreboot.org/c/flashrom/+/72057
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I71435afcacdf97e14d627e35bce3d29de9657f38
Gerrit-Change-Number: 72057
Gerrit-PatchSet: 12
Gerrit-Owner: ChrisEric1 CECL <christopherericlentocha(a)gmail.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Attention: ChrisEric1 CECL <christopherericlentocha(a)gmail.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Edward O'Callaghan.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72821 )
Change subject: programmer_table.c: Allow name_to_entry() to return default when set
......................................................................
Patch Set 1: -Code-Review
(1 comment)
File programmer_table.c:
https://review.coreboot.org/c/flashrom/+/72821/comment/5922fc8f_f5d7a262
PS1, Line 184: if (strlen(name) == 0) {
Same here, when passing a NULL pointer this function dies.
--
To view, visit https://review.coreboot.org/c/flashrom/+/72821
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I75fd24a76d0f6f25072428a8d992a1977c4ad1f8
Gerrit-Change-Number: 72821
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Tue, 14 Feb 2023 17:03:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72820 )
Change subject: programmer_table.c: Move name to entry to own function
......................................................................
Patch Set 1: -Code-Review
(1 comment)
Patchset:
PS1:
In later patchsets name is passed as NULL which ends in a segmentation fault
CONFIG_DEFAULT_PROGRAMMER_NAME is given && ./flashrom # without args
--
To view, visit https://review.coreboot.org/c/flashrom/+/72820
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I5da3861b47d0d0b527e7cde5dd61bb696de6fedb
Gerrit-Change-Number: 72820
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Tue, 14 Feb 2023 17:01:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72821 )
Change subject: programmer_table.c: Allow name_to_entry() to return default when set
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/72821/comment/9b449354_e1bedde5
PS1, Line 8:
When CONFIG_DEFAULT_PROGRAMMER_NAME is set by the build system, programmer_name_to_entry() returns this if an empty string is given as name.
--
To view, visit https://review.coreboot.org/c/flashrom/+/72821
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I75fd24a76d0f6f25072428a8d992a1977c4ad1f8
Gerrit-Change-Number: 72821
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Tue, 14 Feb 2023 16:40:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72821 )
Change subject: programmer_table.c: Allow name_to_entry() to return default when set
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
Patchset:
PS1:
You know my intentions for CONFIG_DEFAULT_PROGRAMMER..., but as long as we keep it this is a good move
--
To view, visit https://review.coreboot.org/c/flashrom/+/72821
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I75fd24a76d0f6f25072428a8d992a1977c4ad1f8
Gerrit-Change-Number: 72821
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Tue, 14 Feb 2023 16:37:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72820 )
Change subject: programmer_table.c: Move name to entry to own function
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/72820/comment/dcad95ef_f8db1a67
PS1, Line 7: programmer_table.c: Move name to entry to own function
...: Move programmer name to entry ...
--
To view, visit https://review.coreboot.org/c/flashrom/+/72820
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I5da3861b47d0d0b527e7cde5dd61bb696de6fedb
Gerrit-Change-Number: 72820
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Tue, 14 Feb 2023 16:34:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72656 )
Change subject: meson: use cfg_data for to evaluate cflags
......................................................................
Patch Set 1:
(1 comment)
File meson.build:
https://review.coreboot.org/c/flashrom/+/72656/comment/d7d2df39_9e7cd33a
PS1, Line 481:
: config.set('CONFIG_DEFAULT_PROGRAMMER_NAME', get_option('default_programmer_name') != '' ? '&programmer_' + get_option('default_programmer_name') : 'NULL')
> not sure I like the embedded ternary operator use in this case, could it be assigned to a intermedia […]
You know, I would like to remove the `CONFIG_DEFAULT_PROGRAMMER`. It accesses parts which should not be accessible by the cli and also alters the default behavior based on compile time decisions.
In an automated job the programmer should given in the same way as the operation to perform.
I don't hang on the syntax and can change it.
--
To view, visit https://review.coreboot.org/c/flashrom/+/72656
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie91b4e1377b9d6d347511d4727c0a26a314b1e57
Gerrit-Change-Number: 72656
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Tue, 14 Feb 2023 12:03:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: comment
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/71578 )
Change subject: internal.c: Factor out laptop alerts into helper func
......................................................................
internal.c: Factor out laptop alerts into helper func
Minor however a unfortunate '_' suffix is temporarily needed
to skirt around global variable shadowing.
Change-Id: I8eea91012e6539b4fdf5d49a75a9cb48bb8a57ca
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/71578
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Alexander Goncharov <chat(a)joursoir.net>
Reviewed-by: Angel Pons <th3fanbus(a)gmail.com>
Reviewed-by: Thomas Heijligen <src(a)posteo.de>
---
M internal.c
1 file changed, 49 insertions(+), 25 deletions(-)
Approvals:
build bot (Jenkins): Verified
Thomas Heijligen: Looks good to me, approved
Angel Pons: Looks good to me, approved
Alexander Goncharov: Looks good to me, but someone else must approve
diff --git a/internal.c b/internal.c
index c5fdb74..07126bf 100644
--- a/internal.c
+++ b/internal.c
@@ -162,6 +162,36 @@
return 0;
}
+// FIXME: remove '_' suffix from parameters once global shadowing is fixed.
+static void report_nonwl_laptop_detected(int is_laptop_, bool laptop_ok_)
+{
+ if (is_laptop_ && !laptop_ok_) {
+ msg_pinfo("========================================================================\n");
+ if (is_laptop_ == 1) {
+ msg_pinfo("You seem to be running flashrom on an unknown laptop. Some\n"
+ "internal buses have been disabled for safety reasons.\n\n");
+ } else {
+ msg_pinfo("You may be running flashrom on an unknown laptop. We could not\n"
+ "detect this for sure because your vendor has not set up the SMBIOS\n"
+ "tables correctly. Some internal buses have been disabled for\n"
+ "safety reasons. You can enforce using all buses by adding\n"
+ " -p internal:laptop=this_is_not_a_laptop\n"
+ "to the command line, but please read the following warning if you\n"
+ "are not sure.\n\n");
+ }
+ msg_perr("Laptops, notebooks and netbooks are difficult to support and we\n"
+ "recommend to use the vendor flashing utility. The embedded controller\n"
+ "(EC) in these machines often interacts badly with flashing.\n"
+ "See the manpage and https://flashrom.org/Laptops for details.\n\n"
+ "If flash is shared with the EC, erase is guaranteed to brick your laptop\n"
+ "and write may brick your laptop.\n"
+ "Read and probe may irritate your EC and cause fan failure, backlight\n"
+ "failure and sudden poweroff.\n"
+ "You have been warned.\n"
+ "========================================================================\n");
+ }
+}
+
static int internal_init(const struct programmer_cfg *cfg)
{
int ret = 0;
@@ -283,31 +313,7 @@
register_par_master(&par_master_internal, internal_buses_supported, NULL);
/* Report if a non-whitelisted laptop is detected that likely uses a legacy bus. */
- if (is_laptop && !laptop_ok) {
- msg_pinfo("========================================================================\n");
- if (is_laptop == 1) {
- msg_pinfo("You seem to be running flashrom on an unknown laptop. Some\n"
- "internal buses have been disabled for safety reasons.\n\n");
- } else {
- msg_pinfo("You may be running flashrom on an unknown laptop. We could not\n"
- "detect this for sure because your vendor has not set up the SMBIOS\n"
- "tables correctly. Some internal buses have been disabled for\n"
- "safety reasons. You can enforce using all buses by adding\n"
- " -p internal:laptop=this_is_not_a_laptop\n"
- "to the command line, but please read the following warning if you\n"
- "are not sure.\n\n");
- }
- msg_perr("Laptops, notebooks and netbooks are difficult to support and we\n"
- "recommend to use the vendor flashing utility. The embedded controller\n"
- "(EC) in these machines often interacts badly with flashing.\n"
- "See the manpage and https://flashrom.org/Laptops for details.\n\n"
- "If flash is shared with the EC, erase is guaranteed to brick your laptop\n"
- "and write may brick your laptop.\n"
- "Read and probe may irritate your EC and cause fan failure, backlight\n"
- "failure and sudden poweroff.\n"
- "You have been warned.\n"
- "========================================================================\n");
- }
+ report_nonwl_laptop_detected(is_laptop, laptop_ok);
ret = 0;
--
To view, visit https://review.coreboot.org/c/flashrom/+/71578
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I8eea91012e6539b4fdf5d49a75a9cb48bb8a57ca
Gerrit-Change-Number: 71578
Gerrit-PatchSet: 4
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged