March 2022 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: helpers.c: use unsigned int for bit shifts (ASAN)
by Anastasia Klimchuk (Code Review) 17 Mar '22

17 Mar '22

Attention is currently required from: Paul Menzel, Edward O'Callaghan, Daniel Campello. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62862 ) Change subject: helpers.c: use unsigned int for bit shifts (ASAN) ...................................................................... Patch Set 2: Code-Review+1 (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/62862/comment/467687af_d294a6b0 PS2, Line 11: error detected Lets say "error detected in chromium tree", that would explain where flashrom-9999 is coming from. https://review.coreboot.org/c/flashrom/+/62862/comment/0653997f_05258a15 PS2, Line 18: TEST=FEATURES=test emerge-amd64-generic flashrom You probably ran tests in upstream tree too (before sending the patch)? (meson tests). I would add this as first line in TEST tag, commit needs to have some info how the patch was tested on upstream tree. And then emerge with tests can come as second line. -- To view, visit https://review.coreboot.org/c/flashrom/+/62862 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib595f13c29dd5c0775e074801756e4f920b4daaf Gerrit-Change-Number: 62862 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-Comment-Date: Thu, 17 Mar 2022 23:06:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add run_probe_lifecycle and add dummyflasher probe test
by Anastasia Klimchuk (Code Review) 17 Mar '22

17 Mar '22

Attention is currently required from: Nico Huber. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59741 ) Change subject: tests: Add run_probe_lifecycle and add dummyflasher probe test ...................................................................... Patch Set 5: (1 comment) Patchset: PS5: Just checking, is everything fine in this patch? Or is there anything else to do? The rest of the chain has +2. Thank you! -- To view, visit https://review.coreboot.org/c/flashrom/+/59741 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9eb7fe3a436fbba5e70db957139fd26e00efec36 Gerrit-Change-Number: 59741 Gerrit-PatchSet: 5 Gerrit-Owner: 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Thu, 17 Mar 2022 22:30:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: raiden_debug_spi: Add more informative error message when WP is enabled
by Karthik Ramasubramanian (Code Review) 17 Mar '22

17 Mar '22

Attention is currently required from: Robert Zieba, Nico Huber, Diana Zigterman, Jon Murphy, Edward O'Callaghan, Anastasia Klimchuk. Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62909 ) Change subject: raiden_debug_spi: Add more informative error message when WP is enabled ...................................................................... Patch Set 1: (1 comment) File raiden_debug_spi.c: https://review.coreboot.org/c/flashrom/+/62909/comment/ebe8a676_bc8a3a30 PS1, Line 987: Nit: Space. -- To view, visit https://review.coreboot.org/c/flashrom/+/62909 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib1e8383baa9c3ea41ab1079af12e3dc8cdff90ae Gerrit-Change-Number: 62909 Gerrit-PatchSet: 1 Gerrit-Owner: Robert Zieba <robertzieba(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Diana Zigterman <dzigterman(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Peers <epeers(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Robert Zieba <robertzieba(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Diana Zigterman <dzigterman(a)google.com> Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 17 Mar 2022 22:16:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Makefile: build shared usb code only when needed by programmer
by Thomas Heijligen (Code Review) 17 Mar '22

17 Mar '22

Attention is currently required from: Nico Huber, Angel Pons. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62832 ) Change subject: Makefile: build shared usb code only when needed by programmer ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > Hmm, is it worth the additional effort to edit Makefile again if […] This makes it clear which files are needed for the individual programmer. IMO these files belong to the peogrammer(s), not the dependency. -- To view, visit https://review.coreboot.org/c/flashrom/+/62832 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9a91c5801a573e7021686a183a1bd874122e35cc Gerrit-Change-Number: 62832 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 17 Mar 2022 21:57:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: print_buildinfo: remove unreachable print of libpci version
by Nico Huber (Code Review) 17 Mar '22

17 Mar '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62833 ) Change subject: print_buildinfo: remove unreachable print of libpci version ...................................................................... Patch Set 3: Code-Review+1 (1 comment) Patchset: PS1: > It would make the build more complex. […] How about we define something in the Makefile that can be used literally? roughly: LIBPCI_VERSION := "no PCI support" ifeq ($(USE_LIBPCI),yes) LIBPCI_VERSION := "libpci $(shell pkg-config --modversion libpci 2>/dev/null)" endif CFLAGS += -DLIBPCI_VERSION='$(LIBPCI_VERSION)' -- To view, visit https://review.coreboot.org/c/flashrom/+/62833 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0b5dbf3bd82a2ffe64b73881383e92f7dad4c382 Gerrit-Change-Number: 62833 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 17 Mar 2022 21:43:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Makefile: build shared usb code only when needed by programmer
by Nico Huber (Code Review) 17 Mar '22

17 Mar '22

Attention is currently required from: Thomas Heijligen, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62832 ) Change subject: Makefile: build shared usb code only when needed by programmer ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Hmm, is it worth the additional effort to edit Makefile again if the dependencies change in the future? Also more typing effort when a new programmer gets added. Is there a technical reason to do this? -- To view, visit https://review.coreboot.org/c/flashrom/+/62832 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9a91c5801a573e7021686a183a1bd874122e35cc Gerrit-Change-Number: 62832 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 17 Mar 2022 21:37:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: hwaccess: replace macros by C code
by Nico Huber (Code Review) 17 Mar '22

17 Mar '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62898 ) Change subject: hwaccess: replace macros by C code ...................................................................... Patch Set 5: (1 comment) Patchset: PS5: Does anybody have a preference regarding `ULL` vs. `ull`? I also wonder why we have all the casts around. -- To view, visit https://review.coreboot.org/c/flashrom/+/62898 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I86d38d816b37c283279c485fac8027f8fb94364a Gerrit-Change-Number: 62898 Gerrit-PatchSet: 5 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 17 Mar 2022 21:31:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: hwaccess: replace macros by C code
by Nico Huber (Code Review) 17 Mar '22

17 Mar '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62898 ) Change subject: hwaccess: replace macros by C code ...................................................................... Patch Set 5: Code-Review+1 (6 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/62898/comment/f103a0d4_9781ef2a PS5, Line 9: seperate sep*a*rate https://review.coreboot.org/c/flashrom/+/62898/comment/9989fb7b_e4bea970 PS5, Line 11: endiannes one more `s` https://review.coreboot.org/c/flashrom/+/62898/comment/e896baf1_f2a4162c PS5, Line 12: swaped another `p`, i.e. `swapped` also a lot in the code. Patchset: PS5: Looks ok to me. Maybe give people some time to comment before the next patch set :) File hwaccess.h: https://review.coreboot.org/c/flashrom/+/62898/comment/35da6b13_52c02bf7 PS5, Line 30: return ((value & (uint16_t)0x00ffU) << 8) | Maybe add another space after `return` for alignment? https://review.coreboot.org/c/flashrom/+/62898/comment/2570e05a_faf5c6d6 PS5, Line 64: gets or `becomes` or in macro notion `expands to`? I would also add a lot of whitespace, e.g. blank lines around this one. Somehow I managed to read the code to understand the comment :D -- To view, visit https://review.coreboot.org/c/flashrom/+/62898 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I86d38d816b37c283279c485fac8027f8fb94364a Gerrit-Change-Number: 62898 Gerrit-PatchSet: 5 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 17 Mar 2022 21:30:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: raiden_debug_spi: Add more informative error message when WP is enabled
by Robert Zieba (Code Review) 17 Mar '22

17 Mar '22

Robert Zieba has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/62909 ) Change subject: raiden_debug_spi: Add more informative error message when WP is enabled ...................................................................... raiden_debug_spi: Add more informative error message when WP is enabled Current error messages are not very helpful when attempting to flash a target that has WP enabled. This change checks for the USB_SPI_DISABLED error that occurs in this case and gives a more informative error message. BUG=b:210645611 TEST=Tested with WP enabled and disable to verify that error message is displayed properly Signed-off-by: Robert Zieba <robertzieba(a)google.com> Change-Id: Ib1e8383baa9c3ea41ab1079af12e3dc8cdff90ae --- M raiden_debug_spi.c 1 file changed, 19 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/09/62909/1 diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index 3f35552..ea85984 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -343,6 +343,7 @@ #include "usb_device.h" #include <libusb.h> +#include <stdint.h> #include <stdio.h> #include <stdlib.h> #include <string.h> @@ -983,6 +984,24 @@ return status; } + /* Check if we received an error from the device, 4 is the size of a + packet id and error code. */ + if (rsp_config.packet_size == 4 && + rsp_config.packet_v2.rsp_start.status_code != USB_SPI_SUCCESS) { + status = rsp_config.packet_v2.rsp_start.status_code; + switch (status) { + case USB_SPI_DISABLED: + msg_perr("Raiden: Target SPI bridge is disabled (is WP enabled?)\n"); + break; + + default: + msg_perr("Raiden: Received error 0x%x from device\n", status); + break; + } + + return status; + } + msg_perr("Raiden: Packet is not a valid config\n" " config attempt = %d\n" " packet id = %u\n" -- To view, visit https://review.coreboot.org/c/flashrom/+/62909 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib1e8383baa9c3ea41ab1079af12e3dc8cdff90ae Gerrit-Change-Number: 62909 Gerrit-PatchSet: 1 Gerrit-Owner: Robert Zieba <robertzieba(a)google.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: Endian conversion: move to platform.h and platform/endian*.c
by Thomas Heijligen (Code Review) 17 Mar '22

17 Mar '22

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Nico Huber, Angel Pons, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/62899 to look at the new patch set (#5). Change subject: Endian conversion: move to platform.h and platform/endian*.c ...................................................................... Endian conversion: move to platform.h and platform/endian*.c Starting to move the platform dependent code to platform/ and provide the abstraction through the platform.h header. Change-Id: I35640282d451960f2a329ae24339ec05dbae6d30 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M Makefile M hwaccess_physmap.c M meson.build R platform.h R platform/endian_big.c R platform/endian_little.c 6 files changed, 12 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/99/62899/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/62899 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I35640282d451960f2a329ae24339ec05dbae6d30 Gerrit-Change-Number: 62899 Gerrit-PatchSet: 5 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2022