Attention is currently required from: Thomas Heijligen.
Hello Thomas Heijligen, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/80631?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: Clarify that adding docs follows dev process and add into howtos
......................................................................
doc: Clarify that adding docs follows dev process and add into howtos
Also add link to "How to add new doc" into a collection of
Contributors howtos.
The doc stays in root of docs, and also in the main left-side
menu on the website, so that it is very easy to discover. Everyone
who has even the slightest motivation to update the docs, should
be able to find instructions easily.
Change-Id: I882de0614ab76b8e83b0fafa67296526fecd8a16
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
---
M doc/contrib_howtos/index.rst
M doc/how_to_add_docs.rst
2 files changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/31/80631/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/80631?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I882de0614ab76b8e83b0fafa67296526fecd8a16
Gerrit-Change-Number: 80631
Gerrit-PatchSet: 2
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
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-MessageType: newpatchset
Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/80631?usp=email )
Change subject: doc: Clarify that adding docs follows dev process
......................................................................
doc: Clarify that adding docs follows dev process
Change-Id: I882de0614ab76b8e83b0fafa67296526fecd8a16
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
---
M doc/how_to_add_docs.rst
1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/31/80631/1
diff --git a/doc/how_to_add_docs.rst b/doc/how_to_add_docs.rst
index 0b32acb..ee99ce7 100644
--- a/doc/how_to_add_docs.rst
+++ b/doc/how_to_add_docs.rst
@@ -1,6 +1,11 @@
How to add or update docs
=========================
+Documentation files live in ``/doc`` directory in the source tree, so
+adding or updating documentation follows the same process as changing
+the code. If you've never done it before, start by carefully
+reading the :doc:`/dev_guide/development_guide`.
+
To add or update a documentation page, you need to create or modify
an ``.rst`` file in the ``/doc`` directory and send a patch for
review.
--
To view, visit https://review.coreboot.org/c/flashrom/+/80631?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I882de0614ab76b8e83b0fafa67296526fecd8a16
Gerrit-Change-Number: 80631
Gerrit-PatchSet: 1
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newchange
Attention is currently required from: Riku Viitanen.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/80499?usp=email )
Change subject: serprog: clean up documentation
......................................................................
Patch Set 1:
(3 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/80499/comment/f03725b2_df58cabc :
PS1, Line 14: Reviewed-on: https://review.sourcearcade.org/c/flashprog/+/52
: Reviewed-by: Nico Huber <nico.h(a)gmx.de>
: Tested-by: Nico Huber <nico.h(a)gmx.de>
These tags seem to come from another project, you don't need to copy them here, just remove. At the end of review, Gerrit will add the info with the user(s) who done review in the repo.
The only thing which can be useful to keep, is the link to another code review. You can add it in commit message without a tag, like you did in your other patch. This way if anyone is interested, they can click the link and see the info.
Patchset:
PS1:
Thank you so much for clean up documentation!
There are few more things that I wanted to say about it.
I am happy for this patch to get finished and merged (see my other two small comments).
But the next thing I would do is create a serprog page in the actual documentation directory, `/doc`.
We are in process of moving documentation from wiki to `/doc` in the tree, so that's very relevant. I can do the boring part of converting content of this txt file into rst format, adding page to index etc. Would you review the page? Thank you! It would be the same content as it is txt file now (and txt file will be deleted after that).
The only thing I am still deciding, where is the best place for Programmers docs, whether it's inside Users documentation or separate item in the root menu. What do you think, which place better? :)
All docs are generated into htmls and displayed here https://www.flashrom.org/index.html so you can see how it will look eventually.
File Documentation/serprog-protocol.txt:
https://review.coreboot.org/c/flashrom/+/80499/comment/97e95cbc_1a229adf :
PS1, Line 106: serprog.h
Maybe change to `serprog.c` ? Since .h does not exist, more info is in the source code itself.
--
To view, visit https://review.coreboot.org/c/flashrom/+/80499?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Ic83e7fd80840f2db0b006935a964721da0388068
Gerrit-Change-Number: 80499
Gerrit-PatchSet: 1
Gerrit-Owner: Riku Viitanen <riku.viitanen(a)protonmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Riku Viitanen <riku.viitanen(a)protonmail.com>
Gerrit-Comment-Date: Sun, 18 Feb 2024 12:46:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Riku Viitanen.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/80498?usp=email )
Change subject: serprog: Add support for multiple SPI chip selects
......................................................................
Patch Set 2:
(3 comments)
Patchset:
PS2:
Riku, thank you for your patch! I have two small comments.
File serprog.c:
https://review.coreboot.org/c/flashrom/+/80498/comment/3b6802cf_d96d22fa :
PS2, Line 827: cs = extract_programmer_param_str(cfg, "cs");
Since you are adding new programmer param, you need to update manpage as well.
Specifically, manpage is generated from the file `doc/classic_cli_manpage.rst`, look for the section called `serprog programmer` and you can add a paragraph about `cs` param.
May be useful to mention where it is supported (from your code, there are error paths, seems like it's not supported everywhere).
Thank you!
https://review.coreboot.org/c/flashrom/+/80498/comment/bfbb8cc9_2cc8bc0d :
PS2, Line 833: msg_perr("Error: Invalid chip select requested! "
: "Only 0-255 are valid.\n");
Not critical comment, but: this can be one line (and same for another error message before).
Our guidelines for line length are here: https://www.flashrom.org/dev_guide/development_guide.html#coding-style
--
To view, visit https://review.coreboot.org/c/flashrom/+/80498?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: If8052bc6f5c314dcc493bc083bb8270723efaae7
Gerrit-Change-Number: 80498
Gerrit-PatchSet: 2
Gerrit-Owner: Riku Viitanen <riku.viitanen(a)protonmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Riku Viitanen <riku.viitanen(a)protonmail.com>
Gerrit-Comment-Date: Sun, 18 Feb 2024 12:19:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Anton Samsonov, Thomas Heijligen.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79152?usp=email )
Change subject: Makefile: Fix version string for non-Git builds
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS5:
Anton, do you remember about this patch, let's maybe finish it? I have one comment, and I am guessing it would be easy for you to answer. I don't want your work to be lost. Thank you!
--
To view, visit https://review.coreboot.org/c/flashrom/+/79152?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I8694e618878823a9e96b1f2bcfa63f6c71d3c2ed
Gerrit-Change-Number: 79152
Gerrit-PatchSet: 5
Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Anton Samsonov <contact-launchpad(a)zxlab.ru>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru>
Gerrit-Comment-Date: Sat, 17 Feb 2024 09:45:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Anton Samsonov, Anton Samsonov, Thomas Heijligen.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77778?usp=email )
Change subject: Makefile,meson.build: Add support for Sphinx versions prior to 4.x
......................................................................
Patch Set 7: Code-Review+2
(2 comments)
Patchset:
PS7:
I downloaded patch locally, and it works as expected (which is, as before) for version 4.3.2
File doc/sphinx-wrapper.sh:
https://review.coreboot.org/c/flashrom/+/77778/comment/04cda7a7_db156890 :
PS5, Line 14:
> Done
That's a really good comment!
--
To view, visit https://review.coreboot.org/c/flashrom/+/77778?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I9cd280551a1ba4d17edb2e857d56f80431b61e1b
Gerrit-Change-Number: 77778
Gerrit-PatchSet: 7
Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anton Samsonov <avscomputing(a)gmail.com>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Anton Samsonov <avscomputing(a)gmail.com>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru>
Gerrit-Comment-Date: Fri, 16 Feb 2024 07:14:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Anton Samsonov <avscomputing(a)gmail.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Riku Viitanen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/80499?usp=email )
Change subject: serprog: clean up documentation
......................................................................
serprog: clean up documentation
* serprog.h doesn't exist, at least not in this repo
* in the doc, no other command has S_CMD_ prefix either
Change-Id: Ic83e7fd80840f2db0b006935a964721da0388068
Signed-off-by: Riku Viitanen <riku.viitanen(a)protonmail.com>
Reviewed-on: https://review.sourcearcade.org/c/flashprog/+/52
Reviewed-by: Nico Huber <nico.h(a)gmx.de>
Tested-by: Nico Huber <nico.h(a)gmx.de>
---
M Documentation/serprog-protocol.txt
1 file changed, 1 insertion(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/99/80499/1
diff --git a/Documentation/serprog-protocol.txt b/Documentation/serprog-protocol.txt
index d58b8e9..b3d4fd0 100644
--- a/Documentation/serprog-protocol.txt
+++ b/Documentation/serprog-protocol.txt
@@ -84,7 +84,7 @@
lower than the one requested. If there is no lower frequency
available the lowest possible should be used. The value
chosen is sent back in the reply with an ACK.
- 0x15 (S_CMD_S_PIN_STATE):
+ 0x15 (S_PIN_STATE):
Sets the state of the pin drivers connected to the flash chip. Disabling them allows other
devices (e.g. a mainboard's chipset) to access the chip. This way the serprog controller can
remain attached to the flash chip even when the board is running. The user is responsible to
@@ -102,5 +102,3 @@
S_CMD_O_DELAY, S_CMD_O_EXEC.
In addition, support for these commands is recommended:
S_CMD_Q_PGMNAME, S_CMD_Q_BUSTYPE, S_CMD_Q_CHIPSIZE (if parallel).
-
-See also serprog.h.
--
To view, visit https://review.coreboot.org/c/flashrom/+/80499?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Ic83e7fd80840f2db0b006935a964721da0388068
Gerrit-Change-Number: 80499
Gerrit-PatchSet: 1
Gerrit-Owner: Riku Viitanen <riku.viitanen(a)protonmail.com>
Gerrit-MessageType: newchange
Attention is currently required from: Riku Viitanen.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/80498?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: serprog: Add support for multiple SPI chip selects
......................................................................
serprog: Add support for multiple SPI chip selects
Tested with an EliteBook 8560w, Pi Pico, and my serprog firmware:
https://codeberg.org/Riku_V/pico-serprog/
As seen on Flashprog: https://review.sourcearcade.org/c/flashprog/+/51
Change-Id: If8052bc6f5c314dcc493bc083bb8270723efaae7
Signed-off-by: Riku Viitanen <riku.viitanen(a)protonmail.com>
---
M Documentation/serprog-protocol.txt
M serprog.c
2 files changed, 31 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/98/80498/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/80498?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: If8052bc6f5c314dcc493bc083bb8270723efaae7
Gerrit-Change-Number: 80498
Gerrit-PatchSet: 2
Gerrit-Owner: Riku Viitanen <riku.viitanen(a)protonmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Riku Viitanen <riku.viitanen(a)protonmail.com>
Gerrit-MessageType: newpatchset
Riku Viitanen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/80498?usp=email )
Change subject: serprog: Add support for multiple SPI chip selects
......................................................................
serprog: Add support for multiple SPI chip selects
Tested with an EN25F80, Pi Pico, and this serprog firmware:
https://codeberg.org/Riku_V/pico-serprog/commits/branch/multi-cs
Change-Id: If8052bc6f5c314dcc493bc083bb8270723efaae7
Signed-off-by: Riku Viitanen <riku.viitanen(a)protonmail.com>
Reviewed-on: https://review.sourcearcade.org/c/flashprog/+/51
Reviewed-by: Nico Huber <nico.h(a)gmx.de>
Tested-by: Nico Huber <nico.h(a)gmx.de>
---
M Documentation/serprog-protocol.txt
M serprog.c
2 files changed, 31 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/98/80498/1
diff --git a/Documentation/serprog-protocol.txt b/Documentation/serprog-protocol.txt
index 6b7e7e3..d58b8e9 100644
--- a/Documentation/serprog-protocol.txt
+++ b/Documentation/serprog-protocol.txt
@@ -35,6 +35,7 @@
+ slen bytes of data
0x14 Set SPI clock frequency in Hz 32-bit requested frequency ACK + 32-bit set frequency / NAK
0x15 Toggle flash chip pin drivers 8-bit (0 disable, else enable) ACK / NAK
+0x16 Set SPI Chip Select 8-bit ACK / NAK
0x?? unimplemented command - invalid.
@@ -89,6 +90,9 @@
remain attached to the flash chip even when the board is running. The user is responsible to
NOT connect VCC and other permanently externally driven signals to the programmer as needed.
If the value is 0, then the drivers should be disabled, otherwise they should be enabled.
+ 0x16 (S_SPI_CS):
+ Set which SPI Chip Select pin to use. This operation is immediate,
+ meaning it doesn't use the operation buffer.
About mandatory commands:
The only truly mandatory commands for any device are 0x00, 0x01, 0x02 and 0x10,
but one can't really do anything with these commands.
diff --git a/serprog.c b/serprog.c
index b51f0cf..750538a 100644
--- a/serprog.c
+++ b/serprog.c
@@ -3,6 +3,7 @@
*
* Copyright (C) 2009, 2011 Urja Rannikko <urjaman(a)gmail.com>
* Copyright (C) 2009 Carl-Daniel Hailfinger
+ * Copyright (C) 2024 Riku Viitanen <riku.viitanen(a)protonmail.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
@@ -63,6 +64,7 @@
#define S_CMD_O_SPIOP 0x13 /* Perform SPI operation. */
#define S_CMD_S_SPI_FREQ 0x14 /* Set SPI clock frequency */
#define S_CMD_S_PIN_STATE 0x15 /* Enable/disable output drivers */
+#define S_CMD_S_SPI_CS 0x16 /* Set SPI chip select to use */
#define MSGHEADER "serprog: "
@@ -742,7 +744,7 @@
/* Check for the minimum operational set of commands. */
if (serprog_buses_supported & BUS_SPI) {
uint8_t bt = BUS_SPI;
- char *spispeed;
+ char *spispeed, *cs;
if (sp_check_commandavail(S_CMD_O_SPIOP) == 0) {
msg_perr("Error: SPI operation not supported while the "
"bustype is SPI\n");
@@ -822,6 +824,30 @@
}
}
free(spispeed);
+ cs = extract_programmer_param("cs");
+ if (cs) {
+ char *endptr = NULL;
+ errno = 0;
+ unsigned long cs_num = strtoul(cs, &endptr, 0);
+ if (!*cs || errno || *endptr || cs_num > 255) {
+ msg_perr("Error: Invalid chip select requested! "
+ "Only 0-255 are valid.\n");
+ free(cs);
+ goto init_err_cleanup_exit;
+ }
+ free(cs);
+ if (!sp_check_commandavail(S_CMD_S_SPI_CS)) {
+ msg_perr("Error: Setting SPI chip select is not supported!\n");
+ goto init_err_cleanup_exit;
+ }
+ msg_pdbg(MSGHEADER "Requested to use chip select %lu.\n", cs_num);
+ uint8_t cs_num8 = cs_num;
+ if (sp_docommand(S_CMD_S_SPI_CS, 1, &cs_num8, 0, NULL)) {
+ msg_perr("Error: Chip select %u not supported "
+ "by programmer!\n", cs_num8);
+ goto init_err_cleanup_exit;
+ }
+ }
bt = serprog_buses_supported;
if (sp_docommand(S_CMD_S_BUSTYPE, 1, &bt, 0, NULL))
goto init_err_cleanup_exit;
--
To view, visit https://review.coreboot.org/c/flashrom/+/80498?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: If8052bc6f5c314dcc493bc083bb8270723efaae7
Gerrit-Change-Number: 80498
Gerrit-PatchSet: 1
Gerrit-Owner: Riku Viitanen <riku.viitanen(a)protonmail.com>
Gerrit-MessageType: newchange
Attention is currently required from: Anastasia Klimchuk, Anton Samsonov, Thomas Heijligen.
Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77778?usp=email )
Change subject: Makefile,meson.build: Add support for Sphinx versions prior to 4.x
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/77778?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I9cd280551a1ba4d17edb2e857d56f80431b61e1b
Gerrit-Change-Number: 77778
Gerrit-PatchSet: 7
Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anton Samsonov <avscomputing(a)gmail.com>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru>
Gerrit-Comment-Date: Mon, 12 Feb 2024 22:28:57 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment