Attention is currently required from: Thomas Heijligen.
Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/82198?usp=email )
Change subject: doc: Add doc for supported chipsets
......................................................................
doc: Add doc for supported chipsets
Change-Id: I9c9edc7deeeb7a783e2ba2fc6b372edb9c61609e
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
---
M doc/supported_hw/index.rst
A doc/supported_hw/supported_chipsets.rst
2 files changed, 18 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/98/82198/1
diff --git a/doc/supported_hw/index.rst b/doc/supported_hw/index.rst
index cc0f9de..18f3d26 100644
--- a/doc/supported_hw/index.rst
+++ b/doc/supported_hw/index.rst
@@ -8,6 +8,8 @@
supported_flashchips
supported_prog/index
+ supported_chipsets
* :doc:`supported_flashchips`
* :doc:`supported_prog/index`
+* :doc:`supported_chipsets`
diff --git a/doc/supported_hw/supported_chipsets.rst b/doc/supported_hw/supported_chipsets.rst
new file mode 100644
index 0000000..ba4de72
--- /dev/null
+++ b/doc/supported_hw/supported_chipsets.rst
@@ -0,0 +1,16 @@
+==================
+Supported chipsets
+==================
+
+To see the list of all supported chipsets, check the ``const struct penable chipset_enables[]`` in ``chipset_enable.c`` file in the source tree.
+If you have a flashrom repo cloned locally, you can look at the file in your repo, alternatively inspect
+the file `on the web UI of our GitHub mirror <https://github.com/flashrom/flashrom/blob/main/chipset_enable.c#L1768>`_.
+
+Each chipset entry is described by ``struct penable`` in ``include/programmer.h`` which you can inspect in the same way, either in the local source tree or
+`in the GitHub web UI <https://github.com/flashrom/flashrom/blob/main/include/programmer.h#L149>`_.
+
+Note the ``enum test_state status`` of the chipset. ``OK`` means chipset is tested, ``NT`` means not tested, to see all possible
+test states check the ``enum test_state`` definition in ``include/flash.h``.
+
+Also note that macros for supported buses are defined in ``chipset_enable.c`` right before ``chipset_enables[]`` array.
+For example ``B_S`` means ``BUS_SPI``, check the ``chipset_enable.c`` source code for the rest of macro definitions.
--
To view, visit https://review.coreboot.org/c/flashrom/+/82198?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: I9c9edc7deeeb7a783e2ba2fc6b372edb9c61609e
Gerrit-Change-Number: 82198
Gerrit-PatchSet: 1
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: newchange
Attention is currently required from: Thomas Heijligen.
Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/82197?usp=email )
Change subject: doc: Add doc for supported flash chips
......................................................................
doc: Add doc for supported flash chips
Change-Id: I05fb60a4caf2cfb30586fa482687b10638996395
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
---
M doc/supported_hw/index.rst
A doc/supported_hw/supported_flashchips.rst
2 files changed, 22 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/97/82197/1
diff --git a/doc/supported_hw/index.rst b/doc/supported_hw/index.rst
index 5b3305c..cc0f9de 100644
--- a/doc/supported_hw/index.rst
+++ b/doc/supported_hw/index.rst
@@ -6,6 +6,8 @@
:maxdepth: 3
:hidden:
+ supported_flashchips
supported_prog/index
+* :doc:`supported_flashchips`
* :doc:`supported_prog/index`
diff --git a/doc/supported_hw/supported_flashchips.rst b/doc/supported_hw/supported_flashchips.rst
new file mode 100644
index 0000000..d7128e0
--- /dev/null
+++ b/doc/supported_hw/supported_flashchips.rst
@@ -0,0 +1,20 @@
+=====================
+Supported flash chips
+=====================
+
+The list of all supported flash chips is in ``flashchips.c`` file in the source tree.
+If you have a flashrom repo cloned locally, you can look at the file in your repo,
+alternatively inspect the file on the `web UI of our GitHub mirror <https://github.com/flashrom/flashrom/blob/main/flashchips.c>`_.
+
+If you want to check whether a flash chip is supported in the given release, you can rebase your local
+repo at the release tag, alternatively select a tag/branch in GitHub web UI (dropdown on the top-left).
+
+Each chip definition is described by a ``struct flashchip`` in ``include/flash.h``, which you can inspect in the same way,
+either in local source tree or on GitHub web UI.
+
+Note the ``.tested`` status of the chip. If the status is ``TEST_UNTESTED`` this means chip definition has been added purely based on
+datasheet, but without testing on real hardware.
+
+Instructions how to update tested status of the chip are here: :doc:`/contrib_howtos/how_to_mark_chip_tested`.
+
+Instructions how to add support for a chip are here: :doc:`/contrib_howtos/how_to_add_new_chip`.
--
To view, visit https://review.coreboot.org/c/flashrom/+/82197?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: I05fb60a4caf2cfb30586fa482687b10638996395
Gerrit-Change-Number: 82197
Gerrit-PatchSet: 1
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: newchange
Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/82196?usp=email )
Change subject: doc: Fix index files in Supported HW section
......................................................................
doc: Fix index files in Supported HW section
By default toctree in the index file displays full tree of docs
with all the nested levels, and it's too much detail. Besides, left
side menu displays the tree anyway, so duplication is not needed.
Supported hardware section has the deepest nesting out of all other
docs.
This patch changes high-level index files to only display flat list
of next level subtree. On deeper level, full index is displayed.
Change-Id: Ia15e9766cce6f19be1e69fbb1236a327ae3d57b3
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
---
M doc/supported_hw/index.rst
M doc/supported_hw/supported_prog/index.rst
2 files changed, 11 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/96/82196/1
diff --git a/doc/supported_hw/index.rst b/doc/supported_hw/index.rst
index 5552326..5b3305c 100644
--- a/doc/supported_hw/index.rst
+++ b/doc/supported_hw/index.rst
@@ -4,5 +4,8 @@
.. toctree::
:maxdepth: 3
+ :hidden:
supported_prog/index
+
+* :doc:`supported_prog/index`
diff --git a/doc/supported_hw/supported_prog/index.rst b/doc/supported_hw/supported_prog/index.rst
index 9d71578..1dfabd5 100644
--- a/doc/supported_hw/supported_prog/index.rst
+++ b/doc/supported_hw/supported_prog/index.rst
@@ -3,5 +3,13 @@
.. toctree::
:maxdepth: 2
+ :hidden:
serprog/index
+
+flashrom supports many different programmers, for the full list you can look into `programmer_table.c <https://github.com/flashrom/flashrom/blob/main/programmer_table.c>`_
+in the source tree. Some of the programmers have their own documentation pages, see below.
+
+Patches to add/update documentation, or migrate docs from `old wiki website <https://wiki.flashrom.org/Supported_programmers>`_ are very much appreciated.
+
+* :doc:`serprog/index`
--
To view, visit https://review.coreboot.org/c/flashrom/+/82196?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: Ia15e9766cce6f19be1e69fbb1236a327ae3d57b3
Gerrit-Change-Number: 82196
Gerrit-PatchSet: 1
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newchange
Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer.
Georg Gottleuber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82101?usp=email )
Change subject: flashchips.c: Add support for XM25RU256C
......................................................................
Patch Set 2:
(3 comments)
Patchset:
PS1:
> Thank you! Do you have a link to the datasheet for the chip?
I have personally asked for the datasheet, but I don't know if it is okay to publish it. XM25RU256C is very similar to XM25QU256C.
Patchset:
PS2:
Thank you for the quick review.
File flashchips.c:
https://review.coreboot.org/c/flashrom/+/82101/comment/82916e6e_ac7f0dcf :
PS1, Line 21731: TEST_UNTESTED
> You tested it, from the logs your provided you did probe, read, write, so this should be `TEST_OK_PR […]
OK, I was unsure how this review process works and didn't want to force two steps at once. See patchset v2...
--
To view, visit https://review.coreboot.org/c/flashrom/+/82101?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: I431474a662304d09438e274706d3fc9cfbbe0bd6
Gerrit-Change-Number: 82101
Gerrit-PatchSet: 2
Gerrit-Owner: Georg Gottleuber <ggo(a)tuxedo.de>
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: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Mon, 06 May 2024 08:25:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nicholas Chin.
Hello Nicholas Chin, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/82193?usp=email
to look at the new patch set (#5).
Change subject: ch347_spi: Add driver support for CH347F packaging.
......................................................................
ch347_spi: Add driver support for CH347F packaging.
Bug fix: fix SPI clock settings errors.
CH347F can work simultaneously with SPI, I2C and other signals.
CH347 introduce is available at the following URL:
https://www.wch-ic.com/products/CH347.html
Change-Id: I693baf1a0d9dc20757f56fba626b5f5ad20f71dd
Signed-off-by: ZhiYuanNJ <871238103(a)qq.com>
---
M ch347_spi.c
1 file changed, 66 insertions(+), 23 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/93/82193/5
--
To view, visit https://review.coreboot.org/c/flashrom/+/82193?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: I693baf1a0d9dc20757f56fba626b5f5ad20f71dd
Gerrit-Change-Number: 82193
Gerrit-PatchSet: 5
Gerrit-Owner: ZhiYuanNJ
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nicholas Chin.
ZhiYuanNJ has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82193?usp=email )
Change subject: ch347_spi: Fix CH347T PID, interface, and clock settings errors
......................................................................
Patch Set 4:
(13 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/82193/comment/67c31ce5_f0f8db70 :
PS2, Line 7: Bug fix: Fix CH347T PID, interface, and clock settings errors
> The typical commit message style is `<component>: Short description (up to 72 characters)` (see http […]
Done
https://review.coreboot.org/c/flashrom/+/82193/comment/f52a18e7_811f32e0 :
PS2, Line 10: The latest package with F as 347, compared to T, does not require active mode switching and can simultaneously meet communication requirements such as USB to SPI/I2C
> Please reflow to 72 characters per line
Done
https://review.coreboot.org/c/flashrom/+/82193/comment/3f0781e7_f90e257e :
PS2, Line 10: The latest package with F as 347, compared to T, does not require active mode switching and can simultaneously meet communication requirements such as USB to SPI/I2C
> Please reflow to 72 characters per line
Done
https://review.coreboot.org/c/flashrom/+/82193/comment/839a99e2_9f45ebf0 :
PS2, Line 12: ?from=search&wd=eyJpdiI6Im4zdVRJdHdpNks1WUFLSUFDNnRsYnc9PSIsInZhbHVlIjoicHFsU3EwQlgzWmgzM1ZIYnBpZXFiUT09IiwibWFjIjoiNjZjNjc5ZDQ4Yzg2ZDdhNzc5YzM4OTA0MTlkM2FiYWQ1Yzg0ZTZkNWNiZDczZDM1NjM1ODIwN2NjZDhlOTNmOCJ9
> Everything from the `?` onwards isn't necessary to navigate to the product page
Done
https://review.coreboot.org/c/flashrom/+/82193/comment/d28deb28_ee982ff6 :
PS2, Line 12: ?from=search&wd=eyJpdiI6Im4zdVRJdHdpNks1WUFLSUFDNnRsYnc9PSIsInZhbHVlIjoicHFsU3EwQlgzWmgzM1ZIYnBpZXFiUT09IiwibWFjIjoiNjZjNjc5ZDQ4Yzg2ZDdhNzc5YzM4OTA0MTlkM2FiYWQ1Yzg0ZTZkNWNiZDczZDM1NjM1ODIwN2NjZDhlOTNmOCJ9
> Everything from the `?` onwards isn't necessary to navigate to the product page
Done
File ch347_spi.c:
PS2:
> I would split the spispeed changes into a separate patch so that this one is just about adding the C […]
Do I need to open a separate submission for the issue of device speed settings in this submission?
PS2:
> Please use the `/* comment */` comment style rather than `//coment` for consistency with the rest of […]
Done
https://review.coreboot.org/c/flashrom/+/82193/comment/0f24d584_716d43e1 :
PS2, Line 10: *
: * CH347 is a high-speed USB bus converter chip that provides UART, I2C
: * and SPI synchronous serial ports and JTAG interface through USB bus.
: *
: * The SPI interface by CH347 can supports transmission frequency
: * configuration up to 60MHz.
: *
: * The USB2.0 to spi scheme based on CH347 can be used to build
: * customized USB high-speed spi debugger and other products.
: *
: * _____________
: * | |____SPI (MISO,MOSI,SCK,CSC0,CSC1)
: * USB__| CH347T/F |
: * |_____________|____(UART/I2C/JTAG/SWD/GPIO)
: * ______|______
: * | |
: * | 8 MHz XTAL |
: * |_____________|
: *
> This should be placed after the licence header, not in the middle of it. […]
Done
https://review.coreboot.org/c/flashrom/+/82193/comment/bf99344c_d7c39384 :
PS2, Line 79: 0x55DD
> This should be left at 0x55DB, which corresponds to mode 1. […]
Thank you for the reminder, Corrected.
https://review.coreboot.org/c/flashrom/+/82193/comment/f8ba5ba6_63cbd15e :
PS2, Line 97: 2, //CH347T interface number
: 4, //CH347F interface number
> Use the `CH347*_IFACE` defines from above? That also removes the need for the comments
Done
https://review.coreboot.org/c/flashrom/+/82193/comment/746e374c_59713052 :
PS2, Line 344: Couldn't open device %04x:%04x.\n
> This block of code would only run if flashrom has gone through all the IDs in `devs_ch347_spi` and w […]
Done
https://review.coreboot.org/c/flashrom/+/82193/comment/e1d9326d_fb4eea64 :
PS2, Line 347:
> Remove trailing whiespace
Done
https://review.coreboot.org/c/flashrom/+/82193/comment/00c2db12_1d74593c :
PS2, Line 356: Failed to claim interface 2
> Update this to reflect the actual interface number being used
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/82193?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: I693baf1a0d9dc20757f56fba626b5f5ad20f71dd
Gerrit-Change-Number: 82193
Gerrit-PatchSet: 4
Gerrit-Owner: ZhiYuanNJ
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Mon, 06 May 2024 06:23:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: ZhiYuanNJ.
Hello Nicholas Chin, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/82193?usp=email
to look at the new patch set (#4).
Change subject: ch347_spi: Fix CH347T PID, interface, and clock settings errors
......................................................................
ch347_spi: Fix CH347T PID, interface, and clock settings errors
Add functionality : Add driver support for CH347F packaging.
CH347F can work simultaneously with SPI, I2C and other signals.
CH347 introduce is available at the following URL:
https://www.wch-ic.com/products/CH347.html
Change-Id: I693baf1a0d9dc20757f56fba626b5f5ad20f71dd
Signed-off-by: ZhiYuanNJ <871238103(a)qq.com>
---
M ch347_spi.c
1 file changed, 66 insertions(+), 23 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/93/82193/4
--
To view, visit https://review.coreboot.org/c/flashrom/+/82193?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: I693baf1a0d9dc20757f56fba626b5f5ad20f71dd
Gerrit-Change-Number: 82193
Gerrit-PatchSet: 4
Gerrit-Owner: ZhiYuanNJ
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: ZhiYuanNJ
Gerrit-MessageType: newpatchset
Attention is currently required from: ZhiYuanNJ.
Nicholas Chin has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82193?usp=email )
Change subject: Bug fix: Fix CH347T PID, interface, and clock settings errors
......................................................................
Patch Set 2:
(12 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/82193/comment/2e932e8c_c09e281c :
PS2, Line 7: Bug fix: Fix CH347T PID, interface, and clock settings errors
The typical commit message style is `<component>: Short description (up to 72 characters)` (see https://www.flashrom.org/dev_guide/development_guide.html#commit-message-1)
In this case, the component would be `ch347_spi`, and the description could be something like `Add support for QFN28 package (CH347F)`. Then the next line about `Add functionality` can be dropped. As I note later, I would move the spispeed changes to a separate patch.
https://review.coreboot.org/c/flashrom/+/82193/comment/5bfef957_13d3e12e :
PS2, Line 10: The latest package with F as 347, compared to T, does not require active mode switching and can simultaneously meet communication requirements such as USB to SPI/I2C
Please reflow to 72 characters per line
https://review.coreboot.org/c/flashrom/+/82193/comment/db3dcde1_71e49d49 :
PS2, Line 12: ?from=search&wd=eyJpdiI6Im4zdVRJdHdpNks1WUFLSUFDNnRsYnc9PSIsInZhbHVlIjoicHFsU3EwQlgzWmgzM1ZIYnBpZXFiUT09IiwibWFjIjoiNjZjNjc5ZDQ4Yzg2ZDdhNzc5YzM4OTA0MTlkM2FiYWQ1Yzg0ZTZkNWNiZDczZDM1NjM1ODIwN2NjZDhlOTNmOCJ9
Everything from the `?` onwards isn't necessary to navigate to the product page
Patchset:
PS2:
Hello! Thanks for the patch!
File ch347_spi.c:
PS2:
I would split the spispeed changes into a separate patch so that this one is just about adding the CH347F variant.
PS2:
Please use the `/* comment */` comment style rather than `//coment` for consistency with the rest of the file
https://review.coreboot.org/c/flashrom/+/82193/comment/a30520cf_e5b36fc0 :
PS2, Line 10: *
: * CH347 is a high-speed USB bus converter chip that provides UART, I2C
: * and SPI synchronous serial ports and JTAG interface through USB bus.
: *
: * The SPI interface by CH347 can supports transmission frequency
: * configuration up to 60MHz.
: *
: * The USB2.0 to spi scheme based on CH347 can be used to build
: * customized USB high-speed spi debugger and other products.
: *
: * _____________
: * | |____SPI (MISO,MOSI,SCK,CSC0,CSC1)
: * USB__| CH347T/F |
: * |_____________|____(UART/I2C/JTAG/SWD/GPIO)
: * ______|______
: * | |
: * | 8 MHz XTAL |
: * |_____________|
: *
This should be placed after the licence header, not in the middle of it. Also, I'm not sure this text really adds any value in the first place.
https://review.coreboot.org/c/flashrom/+/82193/comment/6f0d657b_5ad5d4c6 :
PS2, Line 79: 0x55DD
This should be left at 0x55DB, which corresponds to mode 1. 0x55DD corresponds to mode 3, which does not support SPI
https://review.coreboot.org/c/flashrom/+/82193/comment/f2521d3c_87af01f6 :
PS2, Line 97: 2, //CH347T interface number
: 4, //CH347F interface number
Use the `CH347*_IFACE` defines from above? That also removes the need for the comments
https://review.coreboot.org/c/flashrom/+/82193/comment/ae96164a_82bf0b50 :
PS2, Line 344: Couldn't open device %04x:%04x.\n
This block of code would only run if flashrom has gone through all the IDs in `devs_ch347_spi` and wasn't able to open any of them. So a generic message saying it couldn't find a CH347 would probably be more appropriate compared to saying it couldn't open a particular ID (which in this case would be whatever is the last vid and pid in the list)
https://review.coreboot.org/c/flashrom/+/82193/comment/807eb57f_4a629f18 :
PS2, Line 347:
Remove trailing whiespace
https://review.coreboot.org/c/flashrom/+/82193/comment/22f87e53_3a0f7ace :
PS2, Line 356: Failed to claim interface 2
Update this to reflect the actual interface number being used
--
To view, visit https://review.coreboot.org/c/flashrom/+/82193?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: I693baf1a0d9dc20757f56fba626b5f5ad20f71dd
Gerrit-Change-Number: 82193
Gerrit-PatchSet: 2
Gerrit-Owner: ZhiYuanNJ
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: ZhiYuanNJ
Gerrit-Comment-Date: Mon, 06 May 2024 04:42:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Anastasia Klimchuk, Brian Norris.
Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/80807?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: flashrom: Don't throw around "delay 1 second" so lightly
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS1:
> > I think that recommendation should be captured in some kind of changelog so there's "real" documen […]
Angel's suggestion (on the list) to continue using the delay for `!BUS_NONSPI` seems like a reasonable compromise between safety and wastefulness (and should work for all the programmers that ChromeOS cares about). Do you plan to make that change?
--
To view, visit https://review.coreboot.org/c/flashrom/+/80807?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: Ie09651fede3f9f03425244c94a2da8bae00315fc
Gerrit-Change-Number: 80807
Gerrit-PatchSet: 5
Gerrit-Owner: Brian Norris <briannorris(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Attention: Brian Norris <briannorris(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Mon, 06 May 2024 03:57:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Brian Norris <briannorris(a)chromium.org>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com>
Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-MessageType: comment