Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/70176 )
Change subject: README: Add information about meson and link build instructions
......................................................................
README: Add information about meson and link build instructions
The patch adds one paragraph of information about meson into the
README file. This meant to be the minimum required to unblock
release candidate. README file will have a more substantial
upgrade soon.
Ticket: https://ticket.coreboot.org/issues/354
Change-Id: I2a27d8f2ba42e18be2485ae95bec1b4c874bb4f7
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/70176
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Angel Pons <th3fanbus(a)gmail.com>
---
M README
1 file changed, 32 insertions(+), 0 deletions(-)
Approvals:
build bot (Jenkins): Verified
Angel Pons: Looks good to me, approved
diff --git a/README b/README
index 49af09e..d00524e 100644
--- a/README
+++ b/README
@@ -46,6 +46,18 @@
Build Instructions
------------------
+flashrom supports building with make and meson.
+
+Meson build system supports almost all the environments, although not exactly
+all of them. Full meson support is on the roadmap in the nearest future.
+To build flashrom with meson, follow the instruction and information in
+/Documentation/building.md
+
+If you are unsure which build system to use, and/or don't know what's the
+difference, use make for now.
+
+The rest of Build Instructions below refers to building flashrom with make.
+
To build flashrom you need to install the following software:
* C compiler (GCC / clang)
--
To view, visit https://review.coreboot.org/c/flashrom/+/70176
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I2a27d8f2ba42e18be2485ae95bec1b4c874bb4f7
Gerrit-Change-Number: 70176
Gerrit-PatchSet: 3
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: 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-MessageType: merged
Attention is currently required from: Thomas Heijligen, Angel Pons, Anastasia Klimchuk.
Nicholas Chin has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70573 )
Change subject: ch347_spi.c: Add initial support for the WCH CH347
......................................................................
Patch Set 3:
(1 comment)
File ch347_spi.c:
https://review.coreboot.org/c/flashrom/+/70573/comment/32ad09a6_424d52d4
PS2, Line 24: #define CH347_CMD_SPI_CFG 0xC0
: #define CH347_CMD_SPI_CS_CTRL 0xC1
: #define CH347_CMD_SPI_OUT_IN 0xC2
: #define CH347_CMD_SPI_IN 0xC3
: #define CH347_CMD_SPI_OUT 0xC4
> Done. […]
Or should I just assume 8 character tabs from the kernel coding style which flashrom generally follows?
--
To view, visit https://review.coreboot.org/c/flashrom/+/70573
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I31b86c41076cc45d4a416a73fa1131350fb745ba
Gerrit-Change-Number: 70573
Gerrit-PatchSet: 3
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.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-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Sun, 11 Dec 2022 20:03:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Angel Pons, Anastasia Klimchuk.
Nicholas Chin has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70573 )
Change subject: ch347_spi.c: Add initial support for the WCH CH347
......................................................................
Patch Set 3:
(6 comments)
File ch347_spi.c:
https://review.coreboot.org/c/flashrom/+/70573/comment/7bde1c88_74ca3b6b
PS2, Line 24: #define CH347_CMD_SPI_CFG 0xC0
: #define CH347_CMD_SPI_CS_CTRL 0xC1
: #define CH347_CMD_SPI_OUT_IN 0xC2
: #define CH347_CMD_SPI_IN 0xC3
: #define CH347_CMD_SPI_OUT 0xC4
> Suggestion: use tabs to align the hex values? […]
Done. I used mixed spaces and tabs to get it to align properly even when using different tab widths in a text editor, not sure if that's the best way to do it.
https://review.coreboot.org/c/flashrom/+/70573/comment/13686991_a4e02a57
PS2, Line 48: static struct libusb_device_handle *handle = NULL;
> There have been changes to other programmers to pass this kind of data around through the flashctx. […]
Done
https://review.coreboot.org/c/flashrom/+/70573/comment/b0a2bac7_b9e3ef53
PS2, Line 51: 1
> Do you need to specify the array size? Or is this hardcoded because the code below only tries openin […]
Done. Removed the array size and added a terminating {0} entry which other programmers include.
https://review.coreboot.org/c/flashrom/+/70573/comment/4ee00a36_504b7a33
PS2, Line 71: uint8_t cmd[13] = {0};
: cmd[0] = CH347_CMD_SPI_CS_CTRL;
: /* payload length, uint16 LSB: 10 */
: cmd[1] = 10;
:
: cmd[3] = cs1;
: cmd[8] = cs2;
> How about: […]
Done. I forgot about this syntax, thanks!
https://review.coreboot.org/c/flashrom/+/70573/comment/8516c7c7_03fec2d2
PS2, Line 221:
> nit: trailing space
Done
https://review.coreboot.org/c/flashrom/+/70573/comment/ce33b794_89ee63d2
PS2, Line 287: uint16_t vid = devs_ch347_spi[0].vendor_id;
: uint16_t pid = devs_ch347_spi[0].device_id;
: handle = libusb_open_device_with_vid_pid(NULL, vid, pid);
: if (handle == NULL) {
: msg_perr("Couldn't open device %04x:%04x.\n", vid, pid);
: return -1;
: }
> Does the CH341A code do the same? It should be fine as long as there's only one VID/DID in the list.
Yeah, it does. I took a look at other libusb programmers and most of them do basically the same thing as most only match a single VID/DID. It looks like stlinkv3_spi is the only libusb programmer which matches multiple IDs. I'll take a look at how that works when I add support for the CH347 HID mode which uses a different DID.
--
To view, visit https://review.coreboot.org/c/flashrom/+/70573
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I31b86c41076cc45d4a416a73fa1131350fb745ba
Gerrit-Change-Number: 70573
Gerrit-PatchSet: 3
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.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-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Sun, 11 Dec 2022 19:52:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk, Nicholas Chin.
Hello build bot (Jenkins), Thomas Heijligen, Angel Pons, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/70573
to look at the new patch set (#3).
Change subject: ch347_spi.c: Add initial support for the WCH CH347
......................................................................
ch347_spi.c: Add initial support for the WCH CH347
Add support for the WCH CH347, a high-speed USB to bus converter
supporting multiple protocols interfaces including SPI. Currently only
mode 1 (vendor defined communication interface) is supported, mode 2
(USB HID communication interface) support will be added later. The code
is currently hard coded to use a CS1 and a SPI clock of 60 MHz, though
there are 2 CS lines and 6 other GPIO lines available, as well as a
configurable clock divisor. Support for these will be exposed through
programmer parameters in later commits.
This currently uses the synchronous libusb API. Performance seems to be
alright so far, if it becomes an issue I may switch to the asynchronous
API.
Tested with a MX25L1606E flash chip and a hard coded divisor of 2 for a
SPI clock speed of 15 MHz, as I was having signal integrity issues at
higher clock speeds.
Signed-off-by: Nicholas Chin <nic.c3.14(a)gmail.com>
Change-Id: I31b86c41076cc45d4a416a73fa1131350fb745ba
---
M Makefile
A ch347_spi.c
M include/programmer.h
M programmer_table.c
4 files changed, 393 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/73/70573/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/70573
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I31b86c41076cc45d4a416a73fa1131350fb745ba
Gerrit-Change-Number: 70573
Gerrit-PatchSet: 3
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Thomas Heijligen, Name of user not set #1004601.
Hello build bot (Jenkins), Thomas Heijligen, Name of user not set #1004601,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/70571
to look at the new patch set (#4).
Change subject: serial: Add Darwin/macOS support for custom and >230400 baudrates
......................................................................
serial: Add Darwin/macOS support for custom and >230400 baudrates
This change is based on the patch proposed by Denis Ahrens in
https://review.coreboot.org/c/flashrom/+/67822
Change-Id: I3e6b88d2b4c2a130b16456752681fd9f807bf6f0
Signed-off-by: Peter Stuge <peter(a)stuge.se>
---
M Makefile
A custom_baud_darwin.c
M meson.build
M serial.c
4 files changed, 80 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/71/70571/4
--
To view, visit https://review.coreboot.org/c/flashrom/+/70571
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3e6b88d2b4c2a130b16456752681fd9f807bf6f0
Gerrit-Change-Number: 70571
Gerrit-PatchSet: 4
Gerrit-Owner: Peter Stuge <peter(a)stuge.se>
Gerrit-Reviewer: Name of user not set #1004601
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Name of user not set #1004601
Gerrit-MessageType: newpatchset
Attention is currently required from: Thomas Heijligen, Name of user not set #1004601.
Hello build bot (Jenkins), Thomas Heijligen, Name of user not set #1004601,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/70571
to look at the new patch set (#3).
Change subject: serial: Add Darwin/macOS support for custom and >230400 baudrates
......................................................................
serial: Add Darwin/macOS support for custom and >230400 baudrates
This change is based on the patch proposed by Denis Ahrens in
https://review.coreboot.org/c/flashrom/+/67822
Change-Id: I3e6b88d2b4c2a130b16456752681fd9f807bf6f0
Signed-off-by: Peter Stuge <peter(a)stuge.se>
---
M Makefile
A custom_baud_darwin.c
M meson.build
M serial.c
4 files changed, 87 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/71/70571/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/70571
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3e6b88d2b4c2a130b16456752681fd9f807bf6f0
Gerrit-Change-Number: 70571
Gerrit-PatchSet: 3
Gerrit-Owner: Peter Stuge <peter(a)stuge.se>
Gerrit-Reviewer: Name of user not set #1004601
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Name of user not set #1004601
Gerrit-MessageType: newpatchset
Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk, Evan Benn.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70554 )
Change subject: tests/selfcheck.c: Fix on non-x86 machines
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
```
In file included from ../../tests/include/test.h:28,
from ../../tests/selfcheck.c:19:
../../tests/selfcheck.c: In function 'selfcheck_board_matches_table':
../../tests/selfcheck.c:142:21: error: 'board_matches_size' undeclared (first use in this function)
142 | assert_true(board_matches_size > 1);
| ^~~~~~~~~~~~~~~~~~
../../tests/selfcheck.c:142:21: note: each undeclared identifier is reported only once for each function it appears in
../../tests/selfcheck.c:143:21: error: 'board_matches' undeclared (first use in this function)
143 | assert_true(board_matches[board_matches_size - 1].vendor_name == NULL);
```
Looks like some more things need to be guarded.
--
To view, visit https://review.coreboot.org/c/flashrom/+/70554
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Icbe677d3ef164e998daf898ddbea34f96246677f
Gerrit-Change-Number: 70554
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Evan Benn <evanbenn(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Evan Benn <evanbenn(a)google.com>
Gerrit-Comment-Date: Sun, 11 Dec 2022 11:05:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68153 )
Change subject: libflashrom.c: Invert if conditions to improve the readability
......................................................................
Patch Set 6:
(1 comment)
File libflashrom.c:
https://review.coreboot.org/c/flashrom/+/68153/comment/db280cff_c4fe1e5a
PS3, Line 107: (enum flashrom_test_state)
> So yes, the cast is needed for now. I'm setting this to resolved since it's unrelated.
No idea why we have 2 enums for the same thing. We shouldn't.
--
To view, visit https://review.coreboot.org/c/flashrom/+/68153
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I4021d8802cd041dcca29a226af0798ebd9c5a81b
Gerrit-Change-Number: 68153
Gerrit-PatchSet: 6
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Comment-Date: Sun, 11 Dec 2022 11:04:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68153 )
Change subject: libflashrom.c: Invert if conditions to improve the readability
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/68153
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I4021d8802cd041dcca29a226af0798ebd9c5a81b
Gerrit-Change-Number: 68153
Gerrit-PatchSet: 6
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Comment-Date: Sun, 11 Dec 2022 11:02:32 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment