October 2022 - flashrom-gerrit

[S] Change in flashrom[master]: ichspi.c: plumb flashctx through hwseq xfer helper
by Subrata Banik (Code Review) 30 Oct '22

30 Oct '22

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68772 ) Change subject: ichspi.c: plumb flashctx through hwseq xfer helper ...................................................................... Patch Set 2: (1 comment) File ichspi.c: https://review.coreboot.org/c/flashrom/+/68772/comment/67dbf97f_e237efce PS2, Line 1348: const struct flashctx *flash > Wondering if we are planning to use it in future ? in current code, i don't see any usage of this ne […] Looks like https://review.coreboot.org/c/flashrom/+/68773/1/ichspi.c#1353 is the purpose -- To view, visit https://review.coreboot.org/c/flashrom/+/68772 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I67b5aa6350930d912e5036473ac3e792debac0bd Gerrit-Change-Number: 68772 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sun, 30 Oct 2022 17:57:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: ichspi.c: plumb flashctx through hwseq xfer helper
by Subrata Banik (Code Review) 30 Oct '22

30 Oct '22

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68772 ) Change subject: ichspi.c: plumb flashctx through hwseq xfer helper ...................................................................... Patch Set 2: (1 comment) File ichspi.c: https://review.coreboot.org/c/flashrom/+/68772/comment/4d477e6a_ac90dd46 PS2, Line 1348: const struct flashctx *flash Wondering if we are planning to use it in future ? in current code, i don't see any usage of this new argument ? -- To view, visit https://review.coreboot.org/c/flashrom/+/68772 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I67b5aa6350930d912e5036473ac3e792debac0bd Gerrit-Change-Number: 68772 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sun, 30 Oct 2022 17:56:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[1.2.x]: ichspi: Unify timeouts across all SPI operations to 30s
by Felix Singer (Code Review) 30 Oct '22

30 Oct '22

Felix Singer has submitted this change. ( https://review.coreboot.org/c/flashrom/+/68691 ) 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.2.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/+/68691 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 12ee126..725bfb3 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1282,20 +1282,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", @@ -1378,7 +1382,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) { @@ -1418,7 +1421,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; } @@ -1427,7 +1430,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) { @@ -1455,7 +1457,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; @@ -1468,7 +1470,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) { @@ -1497,7 +1498,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/+/68691 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.2.x Gerrit-Change-Id: Ifa910dea794175d8ee2ad277549e5a0d69cba45b Gerrit-Change-Number: 68691 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

1 0

[M] Change in flashrom[1.1.x]: ichspi: Unify timeouts across all SPI operations to 30s
by Felix Singer (Code Review) 30 Oct '22

30 Oct '22

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

1 0

[M] Change in flashrom[1.0.x]: ichspi: Unify timeouts across all SPI operations to 30s
by Felix Singer (Code Review) 30 Oct '22

30 Oct '22

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

1 0

[M] Change in flashrom[1.1.x]: ichspi: Unify timeouts across all SPI operations to 30s
by Michael Niewöhner (Code Review) 30 Oct '22

30 Oct '22

Attention is currently required from: Felix Singer, Nico Huber, Angel Pons. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68690 ) Change subject: ichspi: Unify timeouts across all SPI operations to 30s ...................................................................... Patch Set 1: Code-Review+2 -- 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: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> 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-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 30 Oct 2022 08:27:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[1.2.x]: ichspi: Unify timeouts across all SPI operations to 30s
by Michael Niewöhner (Code Review) 30 Oct '22

30 Oct '22

Attention is currently required from: Felix Singer, Nico Huber, Angel Pons. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68691 ) Change subject: ichspi: Unify timeouts across all SPI operations to 30s ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/68691 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.2.x Gerrit-Change-Id: Ifa910dea794175d8ee2ad277549e5a0d69cba45b Gerrit-Change-Number: 68691 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> 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-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 30 Oct 2022 08:27:29 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[1.0.x]: ichspi: Unify timeouts across all SPI operations to 30s
by Michael Niewöhner (Code Review) 30 Oct '22

30 Oct '22

Attention is currently required from: Felix Singer, Nico Huber, Angel Pons. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68689 ) Change subject: ichspi: Unify timeouts across all SPI operations to 30s ...................................................................... Patch Set 1: Code-Review+2 -- 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: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> 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-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 30 Oct 2022 08:27:23 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: programmer: Drop dead fallback_map() boilerplate
by Felix Singer (Code Review) 30 Oct '22

30 Oct '22

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

1 0

[S] Change in flashrom[master]: programmer: Drop dead fallback_map() boilerplate
by Edward O'Callaghan (Code Review) 30 Oct '22

30 Oct '22

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

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit October 2022