November 2017 - flashrom-gerrit

Change in flashrom[master]: spi_master: Introduce SPI_MASTER_4BA feature flag
by build bot (Jenkins) (Code Review) 10 Nov '17

10 Nov '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/22421 ) Change subject: spi_master: Introduce SPI_MASTER_4BA feature flag ...................................................................... Patch Set 2: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom_gerrit/747/ : SUCCESS https://qa.coreboot.org/job/flashrom-customrules/857/ : SUCCESS -- To view, visit https://review.coreboot.org/22421 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife66e3fc49b9716f9c99cad957095b528135ec2c Gerrit-Change-Number: 22421 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 10 Nov 2017 20:29:35 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[master]: spi25: Revise decision when to enter/exit 4BA mode
by build bot (Jenkins) (Code Review) 10 Nov '17

10 Nov '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/22422 ) Change subject: spi25: Revise decision when to enter/exit 4BA mode ...................................................................... Patch Set 1: Verified-1 Build Failed https://qa.coreboot.org/job/flashrom_gerrit/748/ : FAILURE https://qa.coreboot.org/job/flashrom-customrules/858/ : SUCCESS -- To view, visit https://review.coreboot.org/22422 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I877fe817f801fc54bd0ee2ce4e3ead324cbb3673 Gerrit-Change-Number: 22422 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 10 Nov 2017 20:29:34 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[master]: spi25: Revise decision when to enter/exit 4BA mode
by Nico Huber (Code Review) 10 Nov '17

10 Nov '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/22422 Change subject: spi25: Revise decision when to enter/exit 4BA mode ...................................................................... spi25: Revise decision when to enter/exit 4BA mode Instead of arbitrarily deciding whether to enter 4BA mode in the flash chip's declaration, advertise that entering 4BA mode is supported and only enter it if the SPI master supports 4-byte addresses. If not, exit 4BA mode (as we don't know if the chip is in its reset state). Two new feature flags are introduced for that: * FEATURE_4BA_ENTER: Can enter/exit 4BA mode with instructions 0xb7/0xe9 w/o WREN. * FEATURE_4BA_ENTER_WREN Can enter/exit 4BA mode with instructions 0xb7/0xe9 after WREN. FEATURE_4BA_SUPPORT was dropped, it's completely implicit now. Change-Id: I877fe817f801fc54bd0ee2ce4e3ead324cbb3673 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M chipdrivers.h M flash.h M flashchips.c M flashrom.c M spi25.c 5 files changed, 36 insertions(+), 50 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/22/22422/1 diff --git a/chipdrivers.h b/chipdrivers.h index d7a4aa9..8405220 100644 --- a/chipdrivers.h +++ b/chipdrivers.h @@ -61,10 +61,8 @@ int spi_nbyte_read(struct flashctx *flash, unsigned int addr, uint8_t *bytes, unsigned int len); int spi_read_chunked(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len, unsigned int chunksize); int spi_write_chunked(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len, unsigned int chunksize); -int spi_enter_4ba_b7(struct flashctx *flash); -int spi_enter_4ba_b7_we(struct flashctx *flash); -int spi_exit_4ba_e9(struct flashctx *flash); -int spi_exit_4ba_e9_we(struct flashctx *flash); +int spi_enter_4ba(struct flashctx *flash); +int spi_exit_4ba(struct flashctx *flash); /* spi25_statusreg.c */ diff --git a/flash.h b/flash.h index bab4873..bf7ebd0 100644 --- a/flash.h +++ b/flash.h @@ -119,11 +119,12 @@ #define FEATURE_WRSR_EITHER (FEATURE_WRSR_EWSR | FEATURE_WRSR_WREN) #define FEATURE_OTP (1 << 8) #define FEATURE_QPI (1 << 9) -#define FEATURE_4BA_SUPPORT (1 << 10) -#define FEATURE_4BA_EXT_ADDR (1 << 11) /**< Regular 3-byte operations can be used by writing the most +#define FEATURE_4BA_ENTER (1 << 10) /**< Can enter/exit 4BA mode with instructions 0xb7/0xe9 w/o WREN */ +#define FEATURE_4BA_ENTER_WREN (1 << 11) /**< Can enter/exit 4BA mode with instructions 0xb7/0xe9 after WREN */ +#define FEATURE_4BA_EXT_ADDR (1 << 12) /**< Regular 3-byte operations can be used by writing the most signifcant address byte into an extended address register. */ -#define FEATURE_4BA_READ (1 << 12) /**< Native 4BA read instruction (0x13) is supported. */ -#define FEATURE_4BA_WRITE (1 << 13) /**< Native 4BA byte program (0x12) is supported. */ +#define FEATURE_4BA_READ (1 << 13) /**< Native 4BA read instruction (0x13) is supported. */ +#define FEATURE_4BA_WRITE (1 << 14) /**< Native 4BA byte program (0x12) is supported. */ enum test_state { OK = 0, @@ -205,7 +206,6 @@ int (*unlock) (struct flashctx *flash); int (*write) (struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); int (*read) (struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len); - int (*set_4ba) (struct flashctx *flash); struct voltage { uint16_t min; uint16_t max; diff --git a/flashchips.c b/flashchips.c index 06e659d..0da048a 100644 --- a/flashchips.c +++ b/flashchips.c @@ -8052,7 +8052,7 @@ .total_size = 32768, .page_size = 256, /* OTP: 512B total; enter 0xB1, exit 0xC1 */ - .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_SUPPORT, + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_ENTER, .tested = TEST_OK_PREW, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, @@ -8080,7 +8080,6 @@ .unlock = spi_disable_blockprotect_bp3_srwd, .write = spi_chip_write_256, .read = spi_chip_read, /* Fast read (0x0B) supported */ - .set_4ba = spi_enter_4ba_b7, .voltage = {2700, 3600}, }, @@ -8093,7 +8092,7 @@ .total_size = 65536, .page_size = 256, /* OTP: 512B total; enter 0xB1, exit 0xC1 */ - .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_SUPPORT, + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_ENTER, .tested = TEST_OK_PREW, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, @@ -8121,7 +8120,6 @@ .unlock = spi_disable_blockprotect_bp3_srwd, .write = spi_chip_write_256, .read = spi_chip_read, /* Fast read (0x0B) supported */ - .set_4ba = spi_enter_4ba_b7, .voltage = {2700, 3600}, }, @@ -9834,7 +9832,8 @@ .page_size = 256, /* supports SFDP */ /* OTP: 64B total; read 0x4B, write 0x42 */ - .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_SUPPORT | FEATURE_4BA_READ | FEATURE_4BA_WRITE, + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP + | FEATURE_4BA_ENTER_WREN | FEATURE_4BA_READ | FEATURE_4BA_WRITE, .tested = TEST_OK_PREW, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, @@ -9867,7 +9866,8 @@ .page_size = 256, /* supports SFDP */ /* OTP: 64B total; read 0x4B, write 0x42 */ - .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_SUPPORT | FEATURE_4BA_READ | FEATURE_4BA_WRITE, + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP + | FEATURE_4BA_ENTER_WREN | FEATURE_4BA_READ | FEATURE_4BA_WRITE, .tested = TEST_OK_PREW, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, @@ -14745,7 +14745,7 @@ /* supports SFDP */ /* OTP: 1024B total, 256B reserved; read 0x48; write 0x42, erase 0x44, read ID 0x4B */ /* FOUR_BYTE_ADDR: supports 4-bytes addressing mode */ - .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_SUPPORT | FEATURE_4BA_READ, + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_ENTER_WREN | FEATURE_4BA_READ, .tested = TEST_OK_PREW, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, @@ -14772,7 +14772,6 @@ .unlock = spi_disable_blockprotect, .write = spi_chip_write_256, .read = spi_chip_read, - .set_4ba = spi_enter_4ba_b7_we, .voltage = {2700, 3600}, }, diff --git a/flashrom.c b/flashrom.c index 6944907..9522f25 100644 --- a/flashrom.c +++ b/flashrom.c @@ -2219,11 +2219,15 @@ flash->in_4ba_mode = false; /* Enable/disable 4-byte addressing mode if flash chip supports it */ - if ((flash->chip->feature_bits & FEATURE_4BA_SUPPORT) && flash->chip->set_4ba) { - if (flash->chip->set_4ba(flash)) { - msg_cerr("Enabling/disabling 4-byte addressing mode failed!\n"); - return 1; - } + if (flash->chip->feature_bits & (FEATURE_4BA_ENTER | FEATURE_4BA_ENTER_WREN)) { + int ret; + if (flash->mst->buses_supported & BUS_SPI && flash->mst->spi.features & SPI_MASTER_4BA) + ret = spi_enter_4ba(flash); + else + ret = spi_exit_4ba(flash); + if (ret) + msg_cwarn("Enabling/disabling 4-byte addressing mode failed!\n" + "Assuming chip is in 3-byte addressing mode.\n"); } return 0; diff --git a/spi25.c b/spi25.c index 4671907..196d9f5 100644 --- a/spi25.c +++ b/spi25.c @@ -839,42 +839,27 @@ return SPI_GENERIC_ERROR; } -/* Enter 4-bytes addressing mode (without sending WREN before) */ -int spi_enter_4ba_b7(struct flashctx *flash) +static int spi_enter_exit_4ba(struct flashctx *const flash, const bool enter) { - const unsigned char cmd = JEDEC_ENTER_4_BYTE_ADDR_MODE; + const unsigned char cmd = enter ? JEDEC_ENTER_4_BYTE_ADDR_MODE : JEDEC_EXIT_4_BYTE_ADDR_MODE; + int ret = 1; - const int ret = spi_send_command(flash, sizeof(cmd), 0, &cmd, NULL); + if (flash->chip->feature_bits & FEATURE_4BA_ENTER) + ret = spi_send_command(flash, sizeof(cmd), 0, &cmd, NULL); + else if (flash->chip->feature_bits & FEATURE_4BA_ENTER_WREN) + ret = spi_simple_write_cmd(flash, cmd, 0); + if (!ret) - flash->in_4ba_mode = true; + flash->in_4ba_mode = enter; return ret; } -/* Enter 4-bytes addressing mode with sending WREN before */ -int spi_enter_4ba_b7_we(struct flashctx *flash) +int spi_enter_4ba(struct flashctx *const flash) { - const int ret = spi_simple_write_cmd(flash, JEDEC_ENTER_4_BYTE_ADDR_MODE, 0); - if (!ret) - flash->in_4ba_mode = true; - return ret; + return spi_enter_exit_4ba(flash, true); } -/* Exit 4-bytes addressing mode (without sending WREN before) */ -int spi_exit_4ba_e9(struct flashctx *flash) +int spi_exit_4ba(struct flashctx *flash) { - const unsigned char cmd = JEDEC_EXIT_4_BYTE_ADDR_MODE; - - const int ret = spi_send_command(flash, sizeof(cmd), 0, &cmd, NULL); - if (!ret) - flash->in_4ba_mode = false; - return ret; -} - -/* Exit 4-bytes addressing mode with sending WREN before */ -int spi_exit_4ba_e9_we(struct flashctx *flash) -{ - const int ret = spi_simple_write_cmd(flash, JEDEC_EXIT_4_BYTE_ADDR_MODE, 0); - if (!ret) - flash->in_4ba_mode = false; - return ret; + return spi_enter_exit_4ba(flash, false); } -- To view, visit https://review.coreboot.org/22422 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I877fe817f801fc54bd0ee2ce4e3ead324cbb3673 Gerrit-Change-Number: 22422 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[master]: spi_master: Introduce SPI_MASTER_4BA feature flag
by Nico Huber (Code Review) 10 Nov '17

10 Nov '17

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/22421 to look at the new patch set (#2). Change subject: spi_master: Introduce SPI_MASTER_4BA feature flag ...................................................................... spi_master: Introduce SPI_MASTER_4BA feature flag Add a feature flag SPI_MASTER_4BA to `struct spi_master` that advertises programmer-side support for 4-byte addresses in generic commands (and read/write commands if the master uses the default implementations). Set it for all masters that handle commands address-agnostic. Don't prefer native 4BA instructions if the master doesn't support them. Change-Id: Ife66e3fc49b9716f9c99cad957095b528135ec2c Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M bitbang_spi.c M buspirate_spi.c M ch341a_spi.c M dummyflasher.c M ft2232_spi.c M linux_spi.c M programmer.h M serprog.c M spi25.c 9 files changed, 23 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/22421/2 -- To view, visit https://review.coreboot.org/22421 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ife66e3fc49b9716f9c99cad957095b528135ec2c Gerrit-Change-Number: 22421 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[master]: spi_master: Introduce SPI_MASTER_4BA feature flag
by build bot (Jenkins) (Code Review) 10 Nov '17

10 Nov '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/22421 ) Change subject: spi_master: Introduce SPI_MASTER_4BA feature flag ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom_gerrit/746/ : SUCCESS https://qa.coreboot.org/job/flashrom-customrules/856/ : SUCCESS -- To view, visit https://review.coreboot.org/22421 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife66e3fc49b9716f9c99cad957095b528135ec2c Gerrit-Change-Number: 22421 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 10 Nov 2017 19:34:38 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[master]: spi_master: Introduce SPI_MASTER_4BA feature flag
by Nico Huber (Code Review) 10 Nov '17

10 Nov '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/22421 Change subject: spi_master: Introduce SPI_MASTER_4BA feature flag ...................................................................... spi_master: Introduce SPI_MASTER_4BA feature flag Add a feature flag SPI_MASTER_4BA to `struct spi_master` that advertises programmer-side support for 4-byte addresses in generic commands (and read/write commands if the master uses the default implementations). Set it for all masters that handle commands address-agnostic. Change-Id: Ife66e3fc49b9716f9c99cad957095b528135ec2c Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M bitbang_spi.c M buspirate_spi.c M ch341a_spi.c M dummyflasher.c M ft2232_spi.c M linux_spi.c M programmer.h M serprog.c 8 files changed, 14 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/22421/1 diff --git a/bitbang_spi.c b/bitbang_spi.c index 0b27a67..5af2370 100644 --- a/bitbang_spi.c +++ b/bitbang_spi.c @@ -65,6 +65,7 @@ static const struct spi_master spi_master_bitbang = { .type = SPI_CONTROLLER_BITBANG, + .features = SPI_MASTER_4BA, .max_data_read = MAX_DATA_READ_UNLIMITED, .max_data_write = MAX_DATA_WRITE_UNLIMITED, .command = bitbang_spi_send_command, diff --git a/buspirate_spi.c b/buspirate_spi.c index b6554ac..6f4c6d0 100644 --- a/buspirate_spi.c +++ b/buspirate_spi.c @@ -136,6 +136,7 @@ static struct spi_master spi_master_buspirate = { .type = SPI_CONTROLLER_BUSPIRATE, + .features = SPI_MASTER_4BA, .max_data_read = MAX_DATA_UNSPECIFIED, .max_data_write = MAX_DATA_UNSPECIFIED, .command = NULL, diff --git a/ch341a_spi.c b/ch341a_spi.c index 6eb2804..2253e43 100644 --- a/ch341a_spi.c +++ b/ch341a_spi.c @@ -399,6 +399,7 @@ static const struct spi_master spi_master_ch341a_spi = { .type = SPI_CONTROLLER_CH341A_SPI, + .features = SPI_MASTER_4BA, /* flashrom's current maximum is 256 B. CH341A was tested on Linux and Windows to accept atleast * 128 kB. Basically there should be no hard limit because transfers are broken up into USB packets * sent to the device and most of their payload streamed via SPI. */ diff --git a/dummyflasher.c b/dummyflasher.c index f171128..1c0c85b 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -111,6 +111,7 @@ static const struct spi_master spi_master_dummyflasher = { .type = SPI_CONTROLLER_DUMMY, + .features = SPI_MASTER_4BA, .max_data_read = MAX_DATA_READ_UNLIMITED, .max_data_write = MAX_DATA_UNSPECIFIED, .command = dummy_spi_send_command, diff --git a/ft2232_spi.c b/ft2232_spi.c index be60837..3889eed 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -159,6 +159,7 @@ static const struct spi_master spi_master_ft2232 = { .type = SPI_CONTROLLER_FT2232, + .features = SPI_MASTER_4BA, .max_data_read = 64 * 1024, .max_data_write = 256, .command = ft2232_spi_send_command, diff --git a/linux_spi.c b/linux_spi.c index e51fbc4..16eb133 100644 --- a/linux_spi.c +++ b/linux_spi.c @@ -55,6 +55,7 @@ static const struct spi_master spi_master_linux = { .type = SPI_CONTROLLER_LINUX, + .features = SPI_MASTER_4BA, .max_data_read = MAX_DATA_UNSPECIFIED, /* TODO? */ .max_data_write = MAX_DATA_UNSPECIFIED, /* TODO? */ .command = linux_spi_send_command, @@ -189,7 +190,7 @@ static int linux_spi_write_256(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len) { return spi_write_chunked(flash, buf, start, len, - ((unsigned int)getpagesize()) - 4); + ((unsigned int)getpagesize()) - 5); } #endif // CONFIG_LINUX_SPI == 1 diff --git a/programmer.h b/programmer.h index b390a53..3525aa6 100644 --- a/programmer.h +++ b/programmer.h @@ -24,6 +24,8 @@ #ifndef __PROGRAMMER_H__ #define __PROGRAMMER_H__ 1 +#include <stdint.h> + #include "flash.h" /* for chipaddr and flashctx */ enum programmer { @@ -610,8 +612,12 @@ #define MAX_DATA_UNSPECIFIED 0 #define MAX_DATA_READ_UNLIMITED 64 * 1024 #define MAX_DATA_WRITE_UNLIMITED 256 + +#define SPI_MASTER_4BA (1U << 0) + struct spi_master { enum spi_controller type; + uint32_t features; unsigned int max_data_read; // (Ideally,) maximum data read size in one go (excluding opcode+address). unsigned int max_data_write; // (Ideally,) maximum data write size in one go (excluding opcode+address). int (*command)(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, diff --git a/serprog.c b/serprog.c index 25c9944..62f2820 100644 --- a/serprog.c +++ b/serprog.c @@ -305,6 +305,7 @@ unsigned char *readarr); static struct spi_master spi_master_serprog = { .type = SPI_CONTROLLER_SERPROG, + .features = SPI_MASTER_4BA, .max_data_read = MAX_DATA_READ_UNLIMITED, .max_data_write = MAX_DATA_WRITE_UNLIMITED, .command = serprog_spi_send_command, -- To view, visit https://review.coreboot.org/22421 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ife66e3fc49b9716f9c99cad957095b528135ec2c Gerrit-Change-Number: 22421 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[master]: spi: Move ICH BBAR quirk out of the way
by build bot (Jenkins) (Code Review) 09 Nov '17

09 Nov '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/22396 ) Change subject: spi: Move ICH BBAR quirk out of the way ...................................................................... Patch Set 2: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom_gerrit/745/ : SUCCESS https://qa.coreboot.org/job/flashrom-customrules/855/ : SUCCESS -- To view, visit https://review.coreboot.org/22396 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icbac513c5339e8aff624870252133284ef85ab73 Gerrit-Change-Number: 22396 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 09 Nov 2017 20:22:00 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[master]: spi: Move ICH BBAR quirk out of the way
by Nico Huber (Code Review) 09 Nov '17

09 Nov '17

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/22396 to look at the new patch set (#2). Change subject: spi: Move ICH BBAR quirk out of the way ...................................................................... spi: Move ICH BBAR quirk out of the way Get rid of the layering violations around ICH's BBAR. Move all the weird address handling into (surprise, surprise) `ichspi.c`. Might fix writes for the `BBAR != 0` case by accident. Background: Some ICHs have a BBAR (BIOS Base Address Configuration Register) that, if set, limits the valid address range to [BBAR, 2^24). Current code lifted addresses for REMS, RES and READ operations by BBAR, now we do it for all addresses in ichspi. Special care has to be taken if the BBAR is not aligned by the flash chip's size. In this case, the lower part of the chip (from BBAR aligned down, up to BBAR) is inacces- sible (this seems to be the original intend behind BBAR) and has to be left out in the address offset calculation. Change-Id: Icbac513c5339e8aff624870252133284ef85ab73 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M flash.h M ichspi.c M programmer.h M spi.c M spi25.c 5 files changed, 28 insertions(+), 83 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/96/22396/2 -- To view, visit https://review.coreboot.org/22396 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icbac513c5339e8aff624870252133284ef85ab73 Gerrit-Change-Number: 22396 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[master]: spi: Move ICH BBAR quirk out of the way
by build bot (Jenkins) (Code Review) 09 Nov '17

09 Nov '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/22396 ) Change subject: spi: Move ICH BBAR quirk out of the way ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom_gerrit/744/ : SUCCESS https://qa.coreboot.org/job/flashrom-customrules/854/ : SUCCESS -- To view, visit https://review.coreboot.org/22396 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icbac513c5339e8aff624870252133284ef85ab73 Gerrit-Change-Number: 22396 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 09 Nov 2017 17:54:30 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[master]: spi: Move ICH BBAR quirk out of the way
by Nico Huber (Code Review) 09 Nov '17

09 Nov '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/22396 Change subject: spi: Move ICH BBAR quirk out of the way ...................................................................... spi: Move ICH BBAR quirk out of the way Get rid of the layering violations around ICH's BBAR. Move all the weird address handling into (surprise, surprise) `ichspi.c`. Might fix writes for the `BBAR != 0` case by accident. Drop the weird alignment fixup in spi_chip_read(). It looks broken (always sets `addrbase` below the valid base address) and there is no indication in the CL that it was ever tested. Background: Some ICHs have a BBAR (BIOS Base Address Configuration Register) that, if set, limits the valid address range to [BBAR, 2^24). Current code lifted addresses for REMS, RES and READ operations by BBAR, now we do it for all addresses in ichspi. Change-Id: Icbac513c5339e8aff624870252133284ef85ab73 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M flash.h M ichspi.c M programmer.h M spi.c M spi25.c 5 files changed, 18 insertions(+), 83 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/96/22396/1 diff --git a/flash.h b/flash.h index 4f0ab15..bab4873 100644 --- a/flash.h +++ b/flash.h @@ -371,7 +371,6 @@ #define NULL_SPI_CMD { 0, 0, NULL, NULL, } int spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int spi_send_multicommand(struct flashctx *flash, struct spi_command *cmds); -uint32_t spi_get_valid_read_addr(struct flashctx *flash); enum chipbustype get_buses_supported(void); #endif /* !__FLASH_H__ */ diff --git a/ichspi.c b/ichspi.c index 859d55f..9a4cf5d 100644 --- a/ichspi.c +++ b/ichspi.c @@ -229,7 +229,7 @@ static int ichspi_lock = 0; static enum ich_chipset ich_generation = CHIPSET_ICH_UNKNOWN; -uint32_t ichspi_bbar = 0; +static uint32_t ichspi_bbar; static void *ich_spibar = NULL; @@ -1150,19 +1150,6 @@ return SPI_INVALID_LENGTH; } - /* if opcode-type requires an address */ - if (opcode->spi_type == SPI_OPCODE_TYPE_READ_WITH_ADDRESS || - opcode->spi_type == SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS) { - addr = (writearr[1] << 16) | - (writearr[2] << 8) | (writearr[3] << 0); - if (addr < ichspi_bbar) { - msg_perr("%s: Address 0x%06x below allowed " - "range 0x%06x-0xffffff\n", __func__, - addr, ichspi_bbar); - return SPI_INVALID_ADDRESS; - } - } - /* Translate read/write array/count. * The maximum data length is identical for the maximum read length and * for the maximum write length excluding opcode and address. Opcode and @@ -1181,6 +1168,20 @@ count = readcnt; } + /* if opcode-type requires an address */ + if (opcode->spi_type == SPI_OPCODE_TYPE_READ_WITH_ADDRESS || + opcode->spi_type == SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS) { + addr = writearr[1] << 16 | writearr[2] << 8 | writearr[3]; + if (count > UINT32_MAX - addr || /* just an integer overflow check */ + ichspi_bbar > UINT32_MAX - addr - count || /* another integer overflow check */ + ichspi_bbar + addr + count > 1 << 24) { /* last address must be in 24-bit space */ + msg_perr("%s: Addresses 0x%06x-0x%06x not in allowed range 0x000000-0x%06x\n", + __func__, addr, addr + count - 1, (1 << 24) - ichspi_bbar - 1); + return SPI_INVALID_ADDRESS; + } + addr += ichspi_bbar; + } + result = run_opcode(flash, *opcode, addr, count, data); if (result) { msg_pdbg("Running OPCODE 0x%02x failed ", opcode->opcode); diff --git a/programmer.h b/programmer.h index a98b713..b390a53 100644 --- a/programmer.h +++ b/programmer.h @@ -661,7 +661,6 @@ /* ichspi.c */ #if CONFIG_INTERNAL == 1 -extern uint32_t ichspi_bbar; int ich_init_spi(void *spibar, enum ich_chipset ich_generation); int via_init_spi(uint32_t mmio_base); diff --git a/spi.c b/spi.c index 0a4a618..56f1fdf 100644 --- a/spi.c +++ b/spi.c @@ -103,31 +103,7 @@ int spi_chip_read(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len) { - unsigned int addrbase = 0; - - /* Check if the chip fits between lowest valid and highest possible - * address. Highest possible address with the current SPI implementation - * means 0xffffff, the highest unsigned 24bit number. - */ - addrbase = spi_get_valid_read_addr(flash); - /* Show flash chip size warning if flash chip doesn't support - 4-Bytes Addressing mode and last address excedes 24 bits */ - if (!(flash->chip->feature_bits & FEATURE_4BA_SUPPORT) && - addrbase + flash->chip->total_size * 1024 > (1 << 24)) { - msg_perr("Flash chip size exceeds the allowed access window. "); - msg_perr("Read will probably fail.\n"); - /* Try to get the best alignment subject to constraints. */ - addrbase = (1 << 24) - flash->chip->total_size * 1024; - } - /* Check if alignment is native (at least the largest power of two which - * is a factor of the mapped size of the chip). - */ - if (ffs(flash->chip->total_size * 1024) > (ffs(addrbase) ? : 33)) { - msg_perr("Flash chip is not aligned natively in the allowed " - "access window.\n"); - msg_perr("Read will probably return garbage.\n"); - } - return flash->mst->spi.read(flash, buf, addrbase + start, len); + return flash->mst->spi.read(flash, buf, start, len); } /* @@ -140,27 +116,6 @@ int spi_chip_write_256(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len) { return flash->mst->spi.write_256(flash, buf, start, len); -} - -/* - * Get the lowest allowed address for read accesses. This often happens to - * be the lowest allowed address for all commands which take an address. - * This is a master limitation. - */ -uint32_t spi_get_valid_read_addr(struct flashctx *flash) -{ - switch (flash->mst->spi.type) { -#if CONFIG_INTERNAL == 1 -#if defined(__i386__) || defined(__x86_64__) - case SPI_CONTROLLER_ICH7: - case SPI_CONTROLLER_ICH9: - /* Return BBAR for ICH chipsets. */ - return ichspi_bbar; -#endif -#endif - default: - return 0; - } } int spi_aai_write(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len) diff --git a/spi25.c b/spi25.c index df0ebbe..06eea9f 100644 --- a/spi25.c +++ b/spi25.c @@ -49,21 +49,11 @@ static int spi_rems(struct flashctx *flash, unsigned char *readarr) { - unsigned char cmd[JEDEC_REMS_OUTSIZE] = { JEDEC_REMS, 0, 0, 0 }; - uint32_t readaddr; + const unsigned char cmd[JEDEC_REMS_OUTSIZE] = { JEDEC_REMS, 0, 0, 0 }; int ret; ret = spi_send_command(flash, sizeof(cmd), JEDEC_REMS_INSIZE, cmd, readarr); - if (ret == SPI_INVALID_ADDRESS) { - /* Find the lowest even address allowed for reads. */ - readaddr = (spi_get_valid_read_addr(flash) + 1) & ~1; - cmd[1] = (readaddr >> 16) & 0xff, - cmd[2] = (readaddr >> 8) & 0xff, - cmd[3] = (readaddr >> 0) & 0xff, - ret = spi_send_command(flash, sizeof(cmd), JEDEC_REMS_INSIZE, - cmd, readarr); - } if (ret) return ret; msg_cspew("REMS returned 0x%02x 0x%02x. ", readarr[0], readarr[1]); @@ -72,20 +62,11 @@ static int spi_res(struct flashctx *flash, unsigned char *readarr, int bytes) { - unsigned char cmd[JEDEC_RES_OUTSIZE] = { JEDEC_RES, 0, 0, 0 }; - uint32_t readaddr; + const unsigned char cmd[JEDEC_RES_OUTSIZE] = { JEDEC_RES, 0, 0, 0 }; int ret; int i; ret = spi_send_command(flash, sizeof(cmd), bytes, cmd, readarr); - if (ret == SPI_INVALID_ADDRESS) { - /* Find the lowest even address allowed for reads. */ - readaddr = (spi_get_valid_read_addr(flash) + 1) & ~1; - cmd[1] = (readaddr >> 16) & 0xff, - cmd[2] = (readaddr >> 8) & 0xff, - cmd[3] = (readaddr >> 0) & 0xff, - ret = spi_send_command(flash, sizeof(cmd), bytes, cmd, readarr); - } if (ret) return ret; msg_cspew("RES returned"); -- To view, visit https://review.coreboot.org/22396 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Icbac513c5339e8aff624870252133284ef85ab73 Gerrit-Change-Number: 22396 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2017