Felix Singer has submitted this change. ( https://review.coreboot.org/c/flashrom/+/68690 )
Change subject: ichspi: Unify timeouts across all SPI operations to 30s
......................................................................
ichspi: Unify timeouts across all SPI operations to 30s
Note: This patch was backported from the master branch and it was
modified so that it can be applied on the 1.1.x branch.
`ich_hwseq_wait_for_cycle_complete()` drops taking `timeout` as argument
in favor of a fixed timeout of `30 seconds` for any given SPI operation
as recommended by the SPI programming guide.
Document: Alder Lake-P Client Platform SPI Programming Guide
Rev 1.30 (supporting document for multi-master accessing the
SPI Flash device.)
Refer to below section to understand the problem in more detail and SPI
operation timeout recommendation from Intel in multi-master
scenarios.
On Intel Chipsets that support multi-mastering access of the SPI flash
may run into a timeout failure when the operation initiated from a
single master just follows the SPI operational timeout recommendation
as per the vendor datasheet (example: winbond spiflash W25Q256JV-DTR
specification, table 9.7).
In the multi-master SPI accessing scenario using hardware sequencing
operation, it's impossible to know the actual status of the SPI bus
prior to individual master starting the operation (SPI Cycle In Progress
a.k.a SCIP bit represents the status of SPI operation on individual
master).
Thus, any SPI operation triggered in multi-master environment might need
to account a worst case scenario where the most time consuming operation
might have occupied the SPI bus from a master and an operation initiated
by another master just timed out.
Here is the timeout calculation for any hardware sequencing operation:
Worst Case Operational Delay =
(Maximum Time consumed by a SPI operation + Any marginal
adjustment)
Timeout Recommendation for Hardware Sequencing Operation =
((Worst Case Operational Delay) * (#No. Of SPI Master - 1) +
Current Operational latency)
Assume, on Intel platform with 6 SPI master like, Host CPU, CSE, EC,
GbE and other reserved etc, hence, the Timeout Calculation for SPI
erase Operation would look like as below:
Maximum Time consumed by a SPI Operation = 5 seconds
Worst Case Operational Delay = 5 seconds
Timeout Recommendation for Hardware Seq Operation =
5 seconds * (6 - 1) + 5 seconds = 30 seconds
BUG=b:223630977
TEST=Able to perform read/write/erase operation on PCH 600 series
chipset (board name: Brya).
Original-Signed-off-by: Subrata Banik <subratabanik(a)google.com>
Original-Reviewed-on: https://review.coreboot.org/c/flashrom/+/62867
Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Original-Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org>
Original-Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz>
Original-Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org>
Change-Id: Ifa910dea794175d8ee2ad277549e5a0d69cba45b
Signed-off-by: Felix Singer <felixsinger(a)posteo.net>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/68690
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Michael Niewöhner <foss(a)mniewoehner.de>
---
M ichspi.c
1 file changed, 87 insertions(+), 11 deletions(-)
Approvals:
build bot (Jenkins): Verified
Michael Niewöhner: Looks good to me, approved
diff --git a/ichspi.c b/ichspi.c
index 0f1470d..6cf6b5f 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -1265,20 +1265,24 @@
Resets all error flags in HSFS.
Returns 0 if the cycle completes successfully without errors within
timeout us, 1 on errors. */
-static int ich_hwseq_wait_for_cycle_complete(unsigned int timeout,
- unsigned int len)
+static int ich_hwseq_wait_for_cycle_complete(unsigned int len)
{
+ /*
+ * The SPI bus may be busy due to performing operations from other masters, hence
+ * introduce the long timeout of 30s to cover the worst case scenarios as well.
+ */
+ unsigned int timeout_us = 30 * 1000 * 1000;
uint16_t hsfs;
uint32_t addr;
- timeout /= 8; /* scale timeout duration to counter */
+ timeout_us /= 8; /* scale timeout duration to counter */
while ((((hsfs = REGREAD16(ICH9_REG_HSFS)) &
(HSFS_FDONE | HSFS_FCERR)) == 0) &&
- --timeout) {
+ --timeout_us) {
programmer_delay(8);
}
REGWRITE16(ICH9_REG_HSFS, REGREAD16(ICH9_REG_HSFS));
- if (!timeout) {
+ if (!timeout_us) {
addr = REGREAD32(ICH9_REG_FADDR) & hwseq_data.addr_mask;
msg_perr("Timeout error between offset 0x%08x and "
"0x%08x (= 0x%08x + %d)!\n",
@@ -1361,7 +1365,6 @@
{
uint32_t erase_block;
uint16_t hsfc;
- uint32_t timeout = 5000 * 1000; /* 5 s for max 64 kB */
erase_block = ich_hwseq_get_erase_block_size(addr);
if (len != erase_block) {
@@ -1401,7 +1404,7 @@
prettyprint_ich9_reg_hsfc(hsfc);
REGWRITE16(ICH9_REG_HSFC, hsfc);
- if (ich_hwseq_wait_for_cycle_complete(timeout, len))
+ if (ich_hwseq_wait_for_cycle_complete(len))
return -1;
return 0;
}
@@ -1410,7 +1413,6 @@
unsigned int addr, unsigned int len)
{
uint16_t hsfc;
- uint16_t timeout = 100 * 60;
uint8_t block_len;
if (addr + len > flash->chip->total_size * 1024) {
@@ -1438,7 +1440,7 @@
hsfc |= HSFC_FGO; /* start */
REGWRITE16(ICH9_REG_HSFC, hsfc);
- if (ich_hwseq_wait_for_cycle_complete(timeout, block_len))
+ if (ich_hwseq_wait_for_cycle_complete(block_len))
return 1;
ich_read_data(buf, block_len, ICH9_REG_FDATA0);
addr += block_len;
@@ -1451,7 +1453,6 @@
static int ich_hwseq_write(struct flashctx *flash, const uint8_t *buf, unsigned int addr, unsigned int len)
{
uint16_t hsfc;
- uint16_t timeout = 100 * 60;
uint8_t block_len;
if (addr + len > flash->chip->total_size * 1024) {
@@ -1480,7 +1481,7 @@
hsfc |= HSFC_FGO; /* start */
REGWRITE16(ICH9_REG_HSFC, hsfc);
- if (ich_hwseq_wait_for_cycle_complete(timeout, block_len))
+ if (ich_hwseq_wait_for_cycle_complete(block_len))
return -1;
addr += block_len;
buf += block_len;
--
To view, visit https://review.coreboot.org/c/flashrom/+/68690
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: 1.1.x
Gerrit-Change-Id: Ifa910dea794175d8ee2ad277549e5a0d69cba45b
Gerrit-Change-Number: 68690
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Subrata Banik <subratabanik(a)google.com>
Gerrit-MessageType: merged
Felix Singer has submitted this change. ( https://review.coreboot.org/c/flashrom/+/68689 )
Change subject: ichspi: Unify timeouts across all SPI operations to 30s
......................................................................
ichspi: Unify timeouts across all SPI operations to 30s
Note: This patch was backported from the master branch and it was
modified so that it can be applied on the 1.0.x branch.
`ich_hwseq_wait_for_cycle_complete()` drops taking `timeout` as argument
in favor of a fixed timeout of `30 seconds` for any given SPI operation
as recommended by the SPI programming guide.
Document: Alder Lake-P Client Platform SPI Programming Guide
Rev 1.30 (supporting document for multi-master accessing the
SPI Flash device.)
Refer to below section to understand the problem in more detail and SPI
operation timeout recommendation from Intel in multi-master
scenarios.
On Intel Chipsets that support multi-mastering access of the SPI flash
may run into a timeout failure when the operation initiated from a
single master just follows the SPI operational timeout recommendation
as per the vendor datasheet (example: winbond spiflash W25Q256JV-DTR
specification, table 9.7).
In the multi-master SPI accessing scenario using hardware sequencing
operation, it's impossible to know the actual status of the SPI bus
prior to individual master starting the operation (SPI Cycle In Progress
a.k.a SCIP bit represents the status of SPI operation on individual
master).
Thus, any SPI operation triggered in multi-master environment might need
to account a worst case scenario where the most time consuming operation
might have occupied the SPI bus from a master and an operation initiated
by another master just timed out.
Here is the timeout calculation for any hardware sequencing operation:
Worst Case Operational Delay =
(Maximum Time consumed by a SPI operation + Any marginal
adjustment)
Timeout Recommendation for Hardware Sequencing Operation =
((Worst Case Operational Delay) * (#No. Of SPI Master - 1) +
Current Operational latency)
Assume, on Intel platform with 6 SPI master like, Host CPU, CSE, EC,
GbE and other reserved etc, hence, the Timeout Calculation for SPI
erase Operation would look like as below:
Maximum Time consumed by a SPI Operation = 5 seconds
Worst Case Operational Delay = 5 seconds
Timeout Recommendation for Hardware Seq Operation =
5 seconds * (6 - 1) + 5 seconds = 30 seconds
BUG=b:223630977
TEST=Able to perform read/write/erase operation on PCH 600 series
chipset (board name: Brya).
Original-Signed-off-by: Subrata Banik <subratabanik(a)google.com>
Original-Reviewed-on: https://review.coreboot.org/c/flashrom/+/62867
Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Original-Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org>
Original-Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz>
Original-Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org>
Change-Id: Ifa910dea794175d8ee2ad277549e5a0d69cba45b
Signed-off-by: Felix Singer <felixsinger(a)posteo.net>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/68689
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Michael Niewöhner <foss(a)mniewoehner.de>
---
M ichspi.c
1 file changed, 87 insertions(+), 11 deletions(-)
Approvals:
build bot (Jenkins): Verified
Michael Niewöhner: Looks good to me, approved
diff --git a/ichspi.c b/ichspi.c
index c7bda92..af201a4 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -1251,20 +1251,24 @@
Resets all error flags in HSFS.
Returns 0 if the cycle completes successfully without errors within
timeout us, 1 on errors. */
-static int ich_hwseq_wait_for_cycle_complete(unsigned int timeout,
- unsigned int len)
+static int ich_hwseq_wait_for_cycle_complete(unsigned int len)
{
+ /*
+ * The SPI bus may be busy due to performing operations from other masters, hence
+ * introduce the long timeout of 30s to cover the worst case scenarios as well.
+ */
+ unsigned int timeout_us = 30 * 1000 * 1000;
uint16_t hsfs;
uint32_t addr;
- timeout /= 8; /* scale timeout duration to counter */
+ timeout_us /= 8; /* scale timeout duration to counter */
while ((((hsfs = REGREAD16(ICH9_REG_HSFS)) &
(HSFS_FDONE | HSFS_FCERR)) == 0) &&
- --timeout) {
+ --timeout_us) {
programmer_delay(8);
}
REGWRITE16(ICH9_REG_HSFS, REGREAD16(ICH9_REG_HSFS));
- if (!timeout) {
+ if (!timeout_us) {
addr = REGREAD32(ICH9_REG_FADDR) & hwseq_data.addr_mask;
msg_perr("Timeout error between offset 0x%08x and "
"0x%08x (= 0x%08x + %d)!\n",
@@ -1347,7 +1351,6 @@
{
uint32_t erase_block;
uint16_t hsfc;
- uint32_t timeout = 5000 * 1000; /* 5 s for max 64 kB */
erase_block = ich_hwseq_get_erase_block_size(addr);
if (len != erase_block) {
@@ -1387,7 +1390,7 @@
prettyprint_ich9_reg_hsfc(hsfc);
REGWRITE16(ICH9_REG_HSFC, hsfc);
- if (ich_hwseq_wait_for_cycle_complete(timeout, len))
+ if (ich_hwseq_wait_for_cycle_complete(len))
return -1;
return 0;
}
@@ -1396,7 +1399,6 @@
unsigned int addr, unsigned int len)
{
uint16_t hsfc;
- uint16_t timeout = 100 * 60;
uint8_t block_len;
if (addr + len > flash->chip->total_size * 1024) {
@@ -1424,7 +1426,7 @@
hsfc |= HSFC_FGO; /* start */
REGWRITE16(ICH9_REG_HSFC, hsfc);
- if (ich_hwseq_wait_for_cycle_complete(timeout, block_len))
+ if (ich_hwseq_wait_for_cycle_complete(block_len))
return 1;
ich_read_data(buf, block_len, ICH9_REG_FDATA0);
addr += block_len;
@@ -1437,7 +1439,6 @@
static int ich_hwseq_write(struct flashctx *flash, const uint8_t *buf, unsigned int addr, unsigned int len)
{
uint16_t hsfc;
- uint16_t timeout = 100 * 60;
uint8_t block_len;
if (addr + len > flash->chip->total_size * 1024) {
@@ -1466,7 +1467,7 @@
hsfc |= HSFC_FGO; /* start */
REGWRITE16(ICH9_REG_HSFC, hsfc);
- if (ich_hwseq_wait_for_cycle_complete(timeout, block_len))
+ if (ich_hwseq_wait_for_cycle_complete(block_len))
return -1;
addr += block_len;
buf += block_len;
--
To view, visit https://review.coreboot.org/c/flashrom/+/68689
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: 1.0.x
Gerrit-Change-Id: Ifa910dea794175d8ee2ad277549e5a0d69cba45b
Gerrit-Change-Number: 68689
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Subrata Banik <subratabanik(a)google.com>
Gerrit-MessageType: merged
Attention is currently required from: Thomas Heijligen, Edward O'Callaghan.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68963 )
Change subject: programmer: Drop dead fallback_map() boilerplate
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/68963
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ibb7760f807fae040416cef2797a7dbf6572f7df9
Gerrit-Change-Number: 68963
Gerrit-PatchSet: 1
Gerrit-Owner: 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-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Sun, 30 Oct 2022 05:05:48 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/68963 )
Change subject: programmer: Drop dead fallback_map() boilerplate
......................................................................
programmer: Drop dead fallback_map() boilerplate
The fallback_{un}map() boilerplate code doesn't do anything,
merly distracts away from otherwise linear control flow. Just
drop it as anything in the future that could need such a thing
is free to implement it when required.
Change-Id: Ibb7760f807fae040416cef2797a7dbf6572f7df9
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
---
M flashrom.c
M include/programmer.h
M parallel.c
M programmer.c
4 files changed, 23 insertions(+), 25 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/68963/1
diff --git a/flashrom.c b/flashrom.c
index 624c463..2517716 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -216,11 +216,15 @@
else if (mst->buses_supported & BUS_NONSPI)
map_flash_region = mst->par.map_flash_region;
- void *ret;
+ /* A result of NULL causes mapped addresses to be chip physical
+ * addresses, assuming only a single region is mapped (the entire flash
+ * space). Chips with a second region (like a register map) require a
+ * real memory mapping to distinguish the different ranges. Those chips
+ * are FWH/LPC, so the bus master provides a real mapping.
+ */
+ void *ret = NULL;
if (map_flash_region)
ret = map_flash_region(descr, phys_addr, len);
- else
- ret = fallback_map(descr, phys_addr, len);
msg_gspew("%s: mapping %s from 0x%0*" PRIxPTR " to 0x%0*" PRIxPTR "\n",
__func__, descr, PRIxPTR_WIDTH, phys_addr, PRIxPTR_WIDTH, (uintptr_t) ret);
return ret;
@@ -237,8 +241,6 @@
if (unmap_flash_region)
unmap_flash_region(virt_addr, len);
- else
- fallback_unmap(virt_addr, len);
msg_gspew("%s: unmapped 0x%0*" PRIxPTR "\n", __func__, PRIxPTR_WIDTH, (uintptr_t)virt_addr);
}
diff --git a/include/programmer.h b/include/programmer.h
index 9603443..d5ad172 100644
--- a/include/programmer.h
+++ b/include/programmer.h
@@ -440,8 +440,6 @@
int register_par_master(const struct par_master *mst, const enum chipbustype buses, void *data);
/* programmer.c */
-void *fallback_map(const char *descr, uintptr_t phys_addr, size_t len);
-void fallback_unmap(void *virt_addr, size_t len);
void fallback_chip_writew(const struct flashctx *flash, uint16_t val, chipaddr addr);
void fallback_chip_writel(const struct flashctx *flash, uint32_t val, chipaddr addr);
void fallback_chip_writen(const struct flashctx *flash, const uint8_t *buf, chipaddr addr, size_t len);
diff --git a/parallel.c b/parallel.c
index e635088..cea9e7a 100644
--- a/parallel.c
+++ b/parallel.c
@@ -76,7 +76,7 @@
}
}
- /* Bus masters supporting FWH/LPC cannot use fallback_map(), distinct
+ /* Bus masters supporting FWH/LPC cannot use chip physical maps, distinct
* mappings are needed to support chips with FEATURE_REGISTERMAP
*/
if ((buses & (BUS_FWH | BUS_LPC)) && !mst->map_flash_region) {
diff --git a/programmer.c b/programmer.c
index 939d8c2..d4f15a1 100644
--- a/programmer.c
+++ b/programmer.c
@@ -17,23 +17,6 @@
#include "flash.h"
#include "programmer.h"
-/* Fallback map() for programmers which don't need special handling */
-void *fallback_map(const char *descr, uintptr_t phys_addr, size_t len)
-{
- /* A result of NULL causes mapped addresses to be chip physical
- * addresses, assuming only a single region is mapped (the entire flash
- * space). Chips with a second region (like a register map) require a
- * real memory mapping to distinguish the different ranges. Those chips
- * are FWH/LPC, so the bus master provides a real mapping.
- */
- return NULL;
-}
-
-/* No-op/fallback unmap() for programmers which don't need special handling */
-void fallback_unmap(void *virt_addr, size_t len)
-{
-}
-
/* Little-endian fallback for drivers not supporting 16 bit accesses */
void fallback_chip_writew(const struct flashctx *flash, uint16_t val,
chipaddr addr)
--
To view, visit https://review.coreboot.org/c/flashrom/+/68963
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ibb7760f807fae040416cef2797a7dbf6572f7df9
Gerrit-Change-Number: 68963
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: newchange
Attention is currently required from: Simon Buhrow, Nico Huber, Thomas Heijligen, Anastasia Klimchuk.
Aarya has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66104 )
Change subject: flashrom.c: Add wrapper function to use the erase algorithm
......................................................................
Patch Set 66:
(1 comment)
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/66104/comment/af0bd04d_062621e0
PS66, Line 1204: msg_cerr("Write failed at %#lx, Abort.\n", i);
> I got an error on this line when building for arm board (ignore the line number): […]
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/66104
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I29e3f2bd796759794184b125741a5abaac6f3ce8
Gerrit-Change-Number: 66104
Gerrit-PatchSet: 66
Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Simon Buhrow
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Simon Buhrow
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Fri, 28 Oct 2022 15:40:19 +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: Simon Buhrow, Nico Huber, Thomas Heijligen, Aarya.
Hello build bot (Jenkins), Simon Buhrow, Thomas Heijligen,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/66104
to look at the new patch set (#67).
Change subject: flashrom.c: Add wrapper function to use the erase algorithm
......................................................................
flashrom.c: Add wrapper function to use the erase algorithm
Add a function to call the erase algorithm.
Change-Id: I29e3f2bd796759794184b125741a5abaac6f3ce8
Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com>
---
M flashrom.c
M tests/chip_wp.c
2 files changed, 144 insertions(+), 289 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/04/66104/67
--
To view, visit https://review.coreboot.org/c/flashrom/+/66104
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I29e3f2bd796759794184b125741a5abaac6f3ce8
Gerrit-Change-Number: 66104
Gerrit-PatchSet: 67
Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Simon Buhrow
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Simon Buhrow
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Alexander Goncharov.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68755 )
Change subject: tests: add probe lifecycle test for ch341a_spi
......................................................................
Patch Set 2:
(1 comment)
File tests/ch341a_spi.c:
https://review.coreboot.org/c/flashrom/+/68755/comment/c289bf39_85f5bf89
PS2, Line 26: struct libusb_transfer *transfer_in;
I see the programmer has an array of these, but io_state has one. Is that because only one iteration of the loop is needed in `usb_transfer`? (I am guessing). So programmer reserves an array, but to run this unit test only one item of `transfer_in` is needed?
If yes, all good, just please add a comment.
--
To view, visit https://review.coreboot.org/c/flashrom/+/68755
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0a2d5591d097435fc69719e1d9bd153433425821
Gerrit-Change-Number: 68755
Gerrit-PatchSet: 2
Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Comment-Date: Fri, 28 Oct 2022 07:39:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment