January 2018 - flashrom-gerrit

Change in flashrom[master]: flashchips: Revise all 4BA chips
by Nico Huber (Code Review) 02 Jan '18

02 Jan '18

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/22423 ) Change subject: flashchips: Revise all 4BA chips ...................................................................... flashchips: Revise all 4BA chips Advertise all 4BA features that are currently supported by flashrom, plus add a new feature flag for the 4BA fast-read instruction. Also, list all supported 3BA and 4BA erase-block functions. As this adds a lot of new code paths that could be taken for these chips, mark them all as untested again. Change-Id: I0598496ee7058e3b170684d366f58e4014e0e871 Signed-off-by: Nico Huber <nico.h(a)gmx.de> Reviewed-on: https://review.coreboot.org/22423 Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> --- M flash.h M flashchips.c 2 files changed, 50 insertions(+), 16 deletions(-) Approvals: Stefan Reinauer: Looks good to me, approved build bot (Jenkins): Verified David Hendricks: Looks good to me, approved diff --git a/flash.h b/flash.h index 6e86c35..d31b256 100644 --- a/flash.h +++ b/flash.h @@ -98,9 +98,9 @@ /* * How many different erase functions do we have per chip? - * Atmel AT25FS010 has 6 different functions. + * Macronix MX25L25635F has 8 different functions. */ -#define NUM_ERASEFUNCTIONS 6 +#define NUM_ERASEFUNCTIONS 8 /* Feature bits used for non-SPI only */ #define FEATURE_REGISTERMAP (1 << 0) @@ -124,7 +124,12 @@ #define FEATURE_4BA_EXT_ADDR (1 << 12) /**< Regular 3-byte operations can be used by writing the most significant address byte into an extended address register. */ #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. */ +#define FEATURE_4BA_FAST_READ (1 << 14) /**< Native 4BA fast read instruction (0x0c) is supported. */ +#define FEATURE_4BA_WRITE (1 << 15) /**< Native 4BA byte program (0x12) is supported. */ +/* 4BA Shorthands */ +#define FEATURE_4BA_NATIVE (FEATURE_4BA_READ | FEATURE_4BA_FAST_READ | FEATURE_4BA_WRITE) +#define FEATURE_4BA (FEATURE_4BA_ENTER | FEATURE_4BA_EXT_ADDR | FEATURE_4BA_NATIVE) +#define FEATURE_4BA_WREN (FEATURE_4BA_ENTER_WREN | FEATURE_4BA_EXT_ADDR | FEATURE_4BA_NATIVE) enum test_state { OK = 0, diff --git a/flashchips.c b/flashchips.c index 0e16a48..e389af6 100644 --- a/flashchips.c +++ b/flashchips.c @@ -8127,27 +8127,36 @@ { .vendor = "Macronix", - .name = "MX25L25635F/MX25L25645E/MX25L25665E", + .name = "MX25L25635F", .bustype = BUS_SPI, .manufacture_id = MACRONIX_ID, .model_id = MACRONIX_MX25L25635F, .total_size = 32768, .page_size = 256, /* OTP: 512B total; enter 0xB1, exit 0xC1 */ - .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_ENTER, - .tested = TEST_OK_PREW, + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA, + .tested = TEST_UNTESTED, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, .block_erasers = { { .eraseblocks = { {4 * 1024, 8192} }, + .block_erase = spi_block_erase_21, + }, { + .eraseblocks = { {4 * 1024, 8192} }, .block_erase = spi_block_erase_20, }, { .eraseblocks = { {32 * 1024, 1024} }, + .block_erase = spi_block_erase_5c, + }, { + .eraseblocks = { {32 * 1024, 1024} }, .block_erase = spi_block_erase_52, }, { .eraseblocks = { {64 * 1024, 512} }, + .block_erase = spi_block_erase_dc, + }, { + .eraseblocks = { {64 * 1024, 512} }, .block_erase = spi_block_erase_d8, }, { .eraseblocks = { {32 * 1024 * 1024, 1} }, @@ -8174,20 +8183,29 @@ .total_size = 65536, .page_size = 256, /* OTP: 512B total; enter 0xB1, exit 0xC1 */ - .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_ENTER, - .tested = TEST_OK_PREW, + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA, + .tested = TEST_UNTESTED, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, .block_erasers = { { .eraseblocks = { {4 * 1024, 16384} }, + .block_erase = spi_block_erase_21, + }, { + .eraseblocks = { {4 * 1024, 16384} }, .block_erase = spi_block_erase_20, }, { .eraseblocks = { {32 * 1024, 2048} }, + .block_erase = spi_block_erase_5c, + }, { + .eraseblocks = { {32 * 1024, 2048} }, .block_erase = spi_block_erase_52, }, { .eraseblocks = { {64 * 1024, 1024} }, + .block_erase = spi_block_erase_dc, + }, { + .eraseblocks = { {64 * 1024, 1024} }, .block_erase = spi_block_erase_d8, }, { .eraseblocks = { {64 * 1024 * 1024, 1} }, @@ -9914,9 +9932,8 @@ .page_size = 256, /* supports SFDP */ /* OTP: 64B total; read 0x4B, write 0x42 */ - .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP - | FEATURE_4BA_ENTER_WREN | FEATURE_4BA_READ | FEATURE_4BA_WRITE, - .tested = TEST_OK_PREW, + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_WREN, + .tested = TEST_UNTESTED, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, .block_erasers = { @@ -9924,9 +9941,15 @@ .eraseblocks = { {4 * 1024, 8192} }, .block_erase = spi_block_erase_21, }, { + .eraseblocks = { {4 * 1024, 8192} }, + .block_erase = spi_block_erase_20, + }, { .eraseblocks = { {64 * 1024, 512} }, .block_erase = spi_block_erase_dc, }, { + .eraseblocks = { {64 * 1024, 512} }, + .block_erase = spi_block_erase_d8, + }, { .eraseblocks = { {32768 * 1024, 1} }, .block_erase = spi_block_erase_c7, } @@ -9948,9 +9971,8 @@ .page_size = 256, /* supports SFDP */ /* OTP: 64B total; read 0x4B, write 0x42 */ - .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP - | FEATURE_4BA_ENTER_WREN | FEATURE_4BA_READ | FEATURE_4BA_WRITE, - .tested = TEST_OK_PREW, + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_WREN, + .tested = TEST_UNTESTED, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, .block_erasers = { @@ -9958,9 +9980,15 @@ .eraseblocks = { {4 * 1024, 16384} }, .block_erase = spi_block_erase_21, }, { + .eraseblocks = { {4 * 1024, 16384} }, + .block_erase = spi_block_erase_20, + }, { .eraseblocks = { {64 * 1024, 1024} }, .block_erase = spi_block_erase_dc, }, { + .eraseblocks = { {64 * 1024, 1024} }, + .block_erase = spi_block_erase_d8, + }, { .eraseblocks = { {65536 * 1024, 1} }, .block_erase = spi_block_erase_c7, } @@ -14827,8 +14855,9 @@ /* 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_ENTER_WREN | FEATURE_4BA_READ, - .tested = TEST_OK_PREW, + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_ENTER_WREN + | FEATURE_4BA_EXT_ADDR | FEATURE_4BA_READ | FEATURE_4BA_FAST_READ, + .tested = TEST_UNTESTED, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, .block_erasers = -- To view, visit https://review.coreboot.org/22423 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I0598496ee7058e3b170684d366f58e4014e0e871 Gerrit-Change-Number: 22423 Gerrit-PatchSet: 7 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

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

02 Jan '18

Nico Huber has submitted this change and it was merged. ( 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 (the chip might be in 4BA mode after reset). If we can't assure the state of 4BA mode, we bail out to simplify the code (we'd have to ensure that we don't run any instructions that can usually be switched to 4BA mode otherwise). Two new feature flags are introduced: * 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 is dropped, it's completely implicit now. Also, draw the with/without WREN distinction into the enter/exit functions to reduce code redundancy. Change-Id: I877fe817f801fc54bd0ee2ce4e3ead324cbb3673 Signed-off-by: Nico Huber <nico.h(a)gmx.de> Reviewed-on: https://review.coreboot.org/22422 Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> --- M chipdrivers.h M flash.h M flashchips.c M flashrom.c M spi25.c 5 files changed, 36 insertions(+), 48 deletions(-) Approvals: Stefan Reinauer: Looks good to me, approved build bot (Jenkins): Verified David Hendricks: Looks good to me, approved 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 a14c302..6e86c35 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 significant 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 c8ea5eb..0e16a48 100644 --- a/flashchips.c +++ b/flashchips.c @@ -8134,7 +8134,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, @@ -8162,7 +8162,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}, }, @@ -8175,7 +8174,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, @@ -8203,7 +8202,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}, }, @@ -9916,7 +9914,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, @@ -9949,7 +9948,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, @@ -14827,7 +14827,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, @@ -14854,7 +14854,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 9cc1be6..ac987fd 100644 --- a/flashrom.c +++ b/flashrom.c @@ -42,6 +42,7 @@ #include "flashchips.h" #include "programmer.h" #include "hwaccess.h" +#include "chipdrivers.h" const char flashrom_version[] = FLASHROM_VERSION; const char *chip_to_probe = NULL; @@ -2219,9 +2220,14 @@ 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"); + if (flash->chip->feature_bits & (FEATURE_4BA_ENTER | FEATURE_4BA_ENTER_WREN)) { + int ret; + if (spi_master_4ba(flash)) + ret = spi_enter_4ba(flash); + else + ret = spi_exit_4ba(flash); + if (ret) { + msg_cerr("Failed to set correct 4BA mode! Aborting.\n"); return 1; } } diff --git a/spi25.c b/spi25.c index 787f62e..00e0992 100644 --- a/spi25.c +++ b/spi25.c @@ -843,42 +843,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, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I877fe817f801fc54bd0ee2ce4e3ead324cbb3673 Gerrit-Change-Number: 22422 Gerrit-PatchSet: 7 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> 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) 02 Jan '18

02 Jan '18

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 8: Build Successful https://qa.coreboot.org/job/flashrom-customrules/994/ : SUCCESS -- To view, visit https://review.coreboot.org/22421 To unsubscribe, or for help writing mail filters, 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: 8 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 02 Jan 2018 20:14:47 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in flashrom[master]: spi_master: Introduce SPI_MASTER_4BA feature flag
by Nico Huber (Code Review) 02 Jan '18

02 Jan '18

Nico Huber has submitted this change and it was merged. ( 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. 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> Reviewed-on: https://review.coreboot.org/22421 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> --- 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, 30 insertions(+), 5 deletions(-) Approvals: build bot (Jenkins): Verified David Hendricks: Looks good to me, approved 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 611559a..1d3605a 100644 --- a/linux_spi.c +++ b/linux_spi.c @@ -57,6 +57,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, diff --git a/programmer.h b/programmer.h index b390a53..8736449 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) /**< Can handle 4-byte addresses */ + 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, @@ -786,4 +792,11 @@ void sp_set_pin(enum SP_PIN pin, int val); int sp_get_pin(enum SP_PIN pin); +/* spi_master feature checks */ +static inline bool spi_master_4ba(const struct flashctx *const flash) +{ + return flash->mst->buses_supported & BUS_SPI && + flash->mst->spi.features & SPI_MASTER_4BA; +} + #endif /* !__PROGRAMMER_H__ */ 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, diff --git a/spi25.c b/spi25.c index dcc7641..787f62e 100644 --- a/spi25.c +++ b/spi25.c @@ -375,6 +375,10 @@ const bool native_4ba, const unsigned int addr) { if (native_4ba || flash->in_4ba_mode) { + if (!spi_master_4ba(flash)) { + msg_cwarn("4-byte address requested but master can't handle 4-byte addresses.\n"); + return -1; + } cmd_buf[1] = (addr >> 24) & 0xff; cmd_buf[2] = (addr >> 16) & 0xff; cmd_buf[3] = (addr >> 8) & 0xff; @@ -384,9 +388,10 @@ if (flash->chip->feature_bits & FEATURE_4BA_EXT_ADDR) { if (spi_set_extended_address(flash, addr >> 24)) return -1; - } else { - if (addr >> 24) - return -1; + } else if (addr >> 24) { + msg_cerr("Can't handle 4-byte address for opcode '0x%02x'\n" + "with this chip/programmer combination.\n", cmd_buf[0]); + return -1; } cmd_buf[1] = (addr >> 16) & 0xff; cmd_buf[2] = (addr >> 8) & 0xff; @@ -628,7 +633,7 @@ static int spi_nbyte_program(struct flashctx *flash, unsigned int addr, const uint8_t *bytes, unsigned int len) { - const bool native_4ba = !!(flash->chip->feature_bits & FEATURE_4BA_WRITE); + const bool native_4ba = flash->chip->feature_bits & FEATURE_4BA_WRITE && spi_master_4ba(flash); const uint8_t op = native_4ba ? JEDEC_BYTE_PROGRAM_4BA : JEDEC_BYTE_PROGRAM; return spi_write_cmd(flash, op, native_4ba, addr, bytes, len, 10); } @@ -636,7 +641,7 @@ int spi_nbyte_read(struct flashctx *flash, unsigned int address, uint8_t *bytes, unsigned int len) { - const bool native_4ba = !!(flash->chip->feature_bits & FEATURE_4BA_READ); + const bool native_4ba = flash->chip->feature_bits & FEATURE_4BA_READ && spi_master_4ba(flash); uint8_t cmd[1 + JEDEC_MAX_ADDR_LEN] = { native_4ba ? JEDEC_READ_4BA : JEDEC_READ, }; const int addr_len = spi_prepare_address(flash, cmd, native_4ba, address); -- To view, visit https://review.coreboot.org/22421 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ife66e3fc49b9716f9c99cad957095b528135ec2c Gerrit-Change-Number: 22421 Gerrit-PatchSet: 8 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[master]: chipset_enable: Mark VX900 as tested
by build bot (Jenkins) (Code Review) 02 Jan '18

02 Jan '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/23030 ) Change subject: chipset_enable: Mark VX900 as tested ...................................................................... Patch Set 3: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/993/ : SUCCESS -- To view, visit https://review.coreboot.org/23030 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I190253b0c1920747b710ed7155e78191cce139eb Gerrit-Change-Number: 23030 Gerrit-PatchSet: 3 Gerrit-Owner: Lubomir Rintel <lkundrak(a)v3.sk> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 02 Jan 2018 17:28:11 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: chipset_enable: Mark VX900 as tested
by Lubomir Rintel (Code Review) 02 Jan '18

02 Jan '18

Hello Paul Menzel, build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/23030 to look at the new patch set (#3). Change subject: chipset_enable: Mark VX900 as tested ...................................................................... chipset_enable: Mark VX900 as tested I can confirm a successful reading and writing of MX25L8005 (SPI) on a HP t5550 Thin Client (AMI BIOS 786R9 v1.04). Change-Id: I190253b0c1920747b710ed7155e78191cce139eb Signed-off-by: Lubomir Rintel <lkundrak(a)v3.sk> --- M chipset_enable.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/30/23030/3 -- To view, visit https://review.coreboot.org/23030 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I190253b0c1920747b710ed7155e78191cce139eb Gerrit-Change-Number: 23030 Gerrit-PatchSet: 3 Gerrit-Owner: Lubomir Rintel <lkundrak(a)v3.sk> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[master]: chipset_enable: Marg VX900 as tested
by build bot (Jenkins) (Code Review) 02 Jan '18

02 Jan '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/23030 ) Change subject: chipset_enable: Marg VX900 as tested ...................................................................... Patch Set 2: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/992/ : SUCCESS -- To view, visit https://review.coreboot.org/23030 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I190253b0c1920747b710ed7155e78191cce139eb Gerrit-Change-Number: 23030 Gerrit-PatchSet: 2 Gerrit-Owner: Lubomir Rintel <lkundrak(a)v3.sk> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 02 Jan 2018 17:27:43 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: chipset_enable: Marg VX900 as tested
by Lubomir Rintel (Code Review) 02 Jan '18

02 Jan '18

Hello Paul Menzel, build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/23030 to look at the new patch set (#2). Change subject: chipset_enable: Marg VX900 as tested ...................................................................... chipset_enable: Marg VX900 as tested I can confirm a successful reading and writing of MX25L8005 (SPI) on a HP t5550 Thin Client (AMI BIOS 786R9 v1.04). Change-Id: I190253b0c1920747b710ed7155e78191cce139eb Signed-off-by: Lubomir Rintel <lkundrak(a)v3.sk> --- M chipset_enable.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/30/23030/2 -- To view, visit https://review.coreboot.org/23030 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I190253b0c1920747b710ed7155e78191cce139eb Gerrit-Change-Number: 23030 Gerrit-PatchSet: 2 Gerrit-Owner: Lubomir Rintel <lkundrak(a)v3.sk> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[master]: buspirate_spi: Add support for variable serial speeds
by David Hendricks (Code Review) 02 Jan '18

02 Jan '18

David Hendricks has posted comments on this change. ( https://review.coreboot.org/23057 ) Change subject: buspirate_spi: Add support for variable serial speeds ...................................................................... Patch Set 1: Code-Review+1 (2 comments) Looks pretty good. I don't have a bus pirate though, but I pointed out this patch on IRC so hopefully somebody more familiar with the hardware can chime in here. https://review.coreboot.org/#/c/23057/1/buspirate_spi.c File buspirate_spi.c: https://review.coreboot.org/#/c/23057/1/buspirate_spi.c@201 PS1, Line 201: if ((ret = serialport_config(sp_fd, 115200))) nit: extra parens https://review.coreboot.org/#/c/23057/1/buspirate_spi.c@262 PS1, Line 262: serialspeeds[i].name You could also use ARRAY_SIZE(serialspeeds) as your limit so that you don't need the terminator at the end of serialspeeds. -- To view, visit https://review.coreboot.org/23057 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3706f17a94fdf056063f2ad4a5f0a219665cdcbf Gerrit-Change-Number: 23057 Gerrit-PatchSet: 1 Gerrit-Owner: Shawn Anastasio <shawnanastasio(a)yahoo.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 02 Jan 2018 04:08:28 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: vt_vx: check whether the chipset's MMIO range is configured
by David Hendricks (Code Review) 02 Jan '18

02 Jan '18

David Hendricks has posted comments on this change. ( https://review.coreboot.org/23029 ) Change subject: vt_vx: check whether the chipset's MMIO range is configured ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/23029 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5d3a1ba695153e854e0979ae634f8ed97e6b6293 Gerrit-Change-Number: 23029 Gerrit-PatchSet: 1 Gerrit-Owner: Lubomir Rintel <lkundrak(a)v3.sk> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 02 Jan 2018 03:52:27 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit January 2018