Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/40626 )
Change subject: Revert "flashchips: port S25FS(128S) chip from chromiumos" ......................................................................
Revert "flashchips: port S25FS(128S) chip from chromiumos"
This reverts commit a3519561bd0fb44153bb376322b799000657576f.
Breaks support for most SPI flash chips. It's too big and too invasive to be reviewed as a single commit.
The changes to `spi_poll_wip():spi25.c` were not noticed in the original review that were from the similarly named function and file `s25f_poll_status():s25f.c` in the downstream Chromium fork.
V.2: Rebase and rephrase commit msg to reflect how the issue slipped in.
Change-Id: Id2a4593bdb654f8a26957d69d52189ce61621d93 Signed-off-by: Edward O'Callaghan quasisec@google.com Reviewed-on: https://review.coreboot.org/c/flashrom/+/40626 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Shiyu Sun sshiyu@google.com Reviewed-by: Angel Pons th3fanbus@gmail.com --- M chipdrivers.h M flash.h M flashchips.c M flashchips.h M flashrom.c M spi.h M spi25.c M spi25_statusreg.c 8 files changed, 6 insertions(+), 415 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Shiyu Sun: Looks good to me, but someone else must approve
diff --git a/chipdrivers.h b/chipdrivers.h index 04963c2..3c7d146 100644 --- a/chipdrivers.h +++ b/chipdrivers.h @@ -35,9 +35,6 @@ int probe_spi_res2(struct flashctx *flash); int probe_spi_res3(struct flashctx *flash); int probe_spi_at25f(struct flashctx *flash); -int probe_spi_big_spansion(struct flashctx *flash); -int s25fs_software_reset(struct flashctx *flash); -int spi_poll_wip(struct flashctx *const flash, const unsigned int poll_delay); int spi_write_enable(struct flashctx *flash); int spi_write_disable(struct flashctx *flash); int spi_block_erase_20(struct flashctx *flash, unsigned int addr, unsigned int blocklen); @@ -52,7 +49,6 @@ int spi_block_erase_c7(struct flashctx *flash, unsigned int addr, unsigned int blocklen); int spi_block_erase_d7(struct flashctx *flash, unsigned int addr, unsigned int blocklen); int spi_block_erase_d8(struct flashctx *flash, unsigned int addr, unsigned int blocklen); -int s25fs_block_erase_d8(struct flashctx *flash, unsigned int addr, unsigned int blocklen); int spi_block_erase_db(struct flashctx *flash, unsigned int addr, unsigned int blocklen); int spi_block_erase_dc(struct flashctx *flash, unsigned int addr, unsigned int blocklen); erasefunc_t *spi_get_erasefn_from_opcode(uint8_t opcode); @@ -64,12 +60,10 @@ int spi_exit_4ba(struct flashctx *flash); int spi_set_extended_address(struct flashctx *, uint8_t addr_high);
+ /* spi25_statusreg.c */ uint8_t spi_read_status_register(const struct flashctx *flash); int spi_write_status_register(const struct flashctx *flash, int status); -int s25fs_read_cr(struct flashctx *const flash, uint32_t addr); -int s25fs_write_cr(struct flashctx *const flash, uint32_t addr, uint8_t data); -int s25fs_restore_cr3nv(struct flashctx *const flash, uint8_t cfg); void spi_prettyprint_status_register_bit(uint8_t status, int bit); int spi_prettyprint_status_register_plain(struct flashctx *flash); int spi_prettyprint_status_register_default_welwip(struct flashctx *flash); diff --git a/flash.h b/flash.h index 1278e86..2f0143b 100644 --- a/flash.h +++ b/flash.h @@ -54,8 +54,6 @@ #define PRIxPTR_WIDTH ((int)(sizeof(uintptr_t)*2))
int register_shutdown(int (*function) (void *data), void *data); -#define CHIP_RESTORE_CALLBACK int (*func) (struct flashrom_flashctx *flash, uint8_t status) -int register_chip_restore(CHIP_RESTORE_CALLBACK, struct flashrom_flashctx *flash, uint8_t status); int shutdown_free(void *data); void *programmer_map_flash_region(const char *descr, uintptr_t phys_addr, size_t len); void programmer_unmap_flash_region(void *virt_addr, size_t len); @@ -209,7 +207,6 @@ SPI_EDI = 1, } spi_cmd_set;
- int (*reset) (struct flashctx *flash); int (*probe) (struct flashctx *flash);
/* Delay after "enter/exit ID mode" commands in microseconds. diff --git a/flashchips.c b/flashchips.c index 974ba81..b4006df 100644 --- a/flashchips.c +++ b/flashchips.c @@ -49,7 +49,6 @@ * .eraseblocks[] = Array of { blocksize, blockcount } * .block_erase = Block erase function * } - * .reset = Reset Chip * .printlock = Chip lock status function * .unlock = Chip unlock function * .write = Chip write function @@ -15826,69 +15825,6 @@
{ .vendor = "Spansion", - .name = "S25FS128S Large Sectors", - .bustype = BUS_SPI, - .manufacture_id = SPANSION_ID, - .model_id = SPANSION_S25FS128S_L, - .total_size = 16384, - .page_size = 256, - .feature_bits = FEATURE_WRSR_WREN, - .tested = TEST_UNTESTED, - .probe = probe_spi_big_spansion, - .probe_timing = TIMING_ZERO, - .block_erasers = - { - { - .eraseblocks = { {64 * 1024, 256} }, - .block_erase = s25fs_block_erase_d8, - }, { - .eraseblocks = { {16 * 1024 * 1024, 1} }, - .block_erase = spi_block_erase_60, - }, { - .eraseblocks = { {16 * 1024 * 1024, 1} }, - .block_erase = spi_block_erase_c7, - }, - }, - .unlock = spi_disable_blockprotect, - .write = spi_chip_write_256, - .read = spi_chip_read, - .voltage = {1700, 2000}, - }, - - { - .vendor = "Spansion", - .name = "S25FS128S Small Sectors", - .bustype = BUS_SPI, - .manufacture_id = SPANSION_ID, - .model_id = SPANSION_S25FS128S_S, - .total_size = 16384, - .page_size = 256, - .feature_bits = FEATURE_WRSR_WREN, - .tested = TEST_OK_PREW, - .probe = probe_spi_big_spansion, - .probe_timing = TIMING_ZERO, - .block_erasers = - { - { - .eraseblocks = { {64 * 1024, 256} }, - .block_erase = s25fs_block_erase_d8, - }, { - .eraseblocks = { {16 * 1024 * 1024, 1} }, - .block_erase = spi_block_erase_60, - }, { - .eraseblocks = { {16 * 1024 * 1024, 1} }, - .block_erase = spi_block_erase_c7, - }, - }, - .reset = s25fs_software_reset, - .unlock = spi_disable_blockprotect, - .write = spi_chip_write_256, - .read = spi_chip_read, - .voltage = {1700, 2000}, - }, - - { - .vendor = "Spansion", .name = "S25FL129P......1", /* uniform 256 kB sectors */ .bustype = BUS_SPI, .manufacture_id = SPANSION_ID, diff --git a/flashchips.h b/flashchips.h index 0c77d1d..e5ef390 100644 --- a/flashchips.h +++ b/flashchips.h @@ -642,8 +642,6 @@ #define SPANSION_S25FL032A 0x0215 /* Same as S25FL032P, but the latter supports EDI and CFI */ #define SPANSION_S25FL064A 0x0216 /* Same as S25FL064P, but the latter supports EDI and CFI */ #define SPANSION_S25FL128 0x2018 /* Same ID for various S25FL127S, S25FL128P, S25FL128S and S25FL129P (including dual-die S70FL256P) variants (EDI supported) */ -#define SPANSION_S25FS128S_L 0x20180081 /* Large sectors. */ -#define SPANSION_S25FS128S_S 0x20180181 /* Small sectors. */ #define SPANSION_S25FL256 0x0219 #define SPANSION_S25FL512 0x0220 #define SPANSION_S25FL204 0x4013 diff --git a/flashrom.c b/flashrom.c index 4224637..07ce734 100644 --- a/flashrom.c +++ b/flashrom.c @@ -500,14 +500,6 @@ {0}, /* This entry corresponds to PROGRAMMER_INVALID. */ };
-#define CHIP_RESTORE_MAXFN 4 -static int chip_restore_fn_count = 0; -static struct chip_restore_func_data { - CHIP_RESTORE_CALLBACK; - struct flashctx *flash; - uint8_t status; -} chip_restore_fn[CHIP_RESTORE_MAXFN]; - #define SHUTDOWN_MAXFN 32 static int shutdown_fn_count = 0; /** @private */ @@ -558,23 +550,6 @@ return 0; }
-//int register_chip_restore(int (*function) (void *data), void *data) -int register_chip_restore(CHIP_RESTORE_CALLBACK, - struct flashctx *flash, uint8_t status) -{ - if (chip_restore_fn_count >= CHIP_RESTORE_MAXFN) { - msg_perr("Tried to register more than %i chip restore" - " functions.\n", CHIP_RESTORE_MAXFN); - return 1; - } - chip_restore_fn[chip_restore_fn_count].func = func; /* from macro */ - chip_restore_fn[chip_restore_fn_count].flash = flash; - chip_restore_fn[chip_restore_fn_count].status = status; - chip_restore_fn_count++; - - return 0; -} - int programmer_init(enum programmer prog, const char *param) { int ret; diff --git a/spi.h b/spi.h index 12063d7..3f45038 100644 --- a/spi.h +++ b/spi.h @@ -101,7 +101,7 @@ #define JEDEC_BE_C4_OUTSIZE 0x04 #define JEDEC_BE_C4_INSIZE 0x00
-/* Block Erase 0xd8 is supported by EON/Macronix/Spansion chips. */ +/* Block Erase 0xd8 is supported by EON/Macronix chips. */ #define JEDEC_BE_D8 0xd8 #define JEDEC_BE_D8_OUTSIZE 0x04 #define JEDEC_BE_D8_INSIZE 0x00 @@ -116,18 +116,6 @@ #define JEDEC_SE_OUTSIZE 0x04 #define JEDEC_SE_INSIZE 0x00
-/* RADR, WRAR, RSTEN, RST & CR3NV OPs and timers on Spansion S25FS chips */ -#define CMD_RDAR 0x65 -#define CMD_WRAR 0x71 -#define CMD_WRAR_LEN 5 -#define CMD_RSTEN 0x66 -#define CMD_RST 0x99 -#define CR3NV_ADDR 0x000004 -#define CR3NV_20H_NV (1 << 3) -#define T_W 145 * 1000 /* NV register write time */ -#define T_RPH 35 /* Reset pulse hold time */ -#define T_SE 145 * 1000 /* Sector Erase Time */ - /* Page Erase 0xDB */ #define JEDEC_PE 0xDB #define JEDEC_PE_OUTSIZE 0x04 @@ -141,7 +129,6 @@ /* Status Register Bits */ #define SPI_SR_WIP (0x01 << 0) #define SPI_SR_WEL (0x01 << 1) -#define SPI_SR_ERA_ERR (0x01 << 5) #define SPI_SR_AAI (0x01 << 6)
/* Write Status Enable */ diff --git a/spi25.c b/spi25.c index 16d7f1b..0a939e7 100644 --- a/spi25.c +++ b/spi25.c @@ -25,7 +25,6 @@ #include "flashchips.h" #include "chipdrivers.h" #include "programmer.h" -#include "hwaccess.h" #include "spi.h"
static int spi_rdid(struct flashctx *flash, unsigned char *readarr, int bytes) @@ -285,155 +284,13 @@ return 0; }
-/* Used for probing 'big' Spansion/Cypress S25FS chips */ -int probe_spi_big_spansion(struct flashctx *flash) +static int spi_poll_wip(struct flashctx *const flash, const unsigned int poll_delay) { - static const unsigned char cmd = JEDEC_RDID; - int ret; - unsigned char dev_id[6]; /* We care only about 6 first bytes */ - - ret = spi_send_command(flash, sizeof(cmd), sizeof(dev_id), &cmd, dev_id); - - if (!ret) { - unsigned long i; - - for (i = 0; i < sizeof(dev_id); i++) - msg_gdbg(" 0x%02x", dev_id[i]); - msg_gdbg(".\n"); - - if (dev_id[0] == flash->chip->manufacture_id) { - union { - uint8_t array[4]; - uint32_t whole; - } model_id; - - /* - * The structure of the RDID output is as follows: - * - * offset value meaning - * 00h 01h Manufacturer ID for Spansion - * 01h 20h 128 Mb capacity - * 01h 02h 256 Mb capacity - * 02h 18h 128 Mb capacity - * 02h 19h 256 Mb capacity - * 03h 4Dh Full size of the RDID output (ignored) - * 04h 00h FS: 256-kB physical sectors - * 04h 01h FS: 64-kB physical sectors - * 04h 00h FL: 256-kB physical sectors - * 04h 01h FL: Mix of 64-kB and 4KB overlayed sectors - * 05h 80h FL family - * 05h 81h FS family - * - * Need to use bytes 1, 2, 4, and 5 to properly identify one of eight - * possible chips: - * - * 2 types * 2 possible sizes * 2 possible sector layouts - * - */ - memcpy(model_id.array, dev_id + 1, 2); - memcpy(model_id.array + 2, dev_id + 4, 2); - if (be_to_cpu32(model_id.whole) == flash->chip->model_id) - return 1; - } - } - - return 0; -} - -/* Used for Spansion/Cypress S25F chips */ -static int s25f_legacy_software_reset(struct flashctx *flash) -{ - int result; - struct spi_command cmds[] = { - { - .writecnt = 1, - .writearr = (const unsigned char[]){ CMD_RSTEN }, - .readcnt = 0, - .readarr = NULL, - }, { - .writecnt = 1, - .writearr = (const unsigned char[]){ 0xf0 }, - .readcnt = 0, - .readarr = NULL, - }, { - .writecnt = 0, - .writearr = NULL, - .readcnt = 0, - .readarr = NULL, - }}; - - result = spi_send_multicommand(flash, cmds); - if (result) { - msg_cerr("%s failed during command execution\n", __func__); - return result; - } - /* Reset takes 35us according to data-sheet, double that for safety */ - programmer_delay(T_RPH * 2); - - return 0; -} - -/* Only for Spansion S25FS chips, where legacy reset is disabled by default */ -int s25fs_software_reset(struct flashctx *flash) -{ - int result; - struct spi_command cmds[] = { - { - .writecnt = 1, - .writearr = (const unsigned char[]){ CMD_RSTEN }, - .readcnt = 0, - .readarr = NULL, - }, { - .writecnt = 1, - .writearr = (const unsigned char[]){ CMD_RST }, - .readcnt = 0, - .readarr = NULL, - }, { - .writecnt = 0, - .writearr = NULL, - .readcnt = 0, - .readarr = NULL, - }}; - - msg_cdbg("Force resetting SPI chip.\n"); - result = spi_send_multicommand(flash, cmds); - if (result) { - msg_cerr("%s failed during command execution\n", __func__); - return result; - } - - programmer_delay(T_RPH * 2); - - return 0; -} - -int spi_poll_wip(struct flashctx *const flash, const unsigned int poll_delay) -{ - uint8_t status_reg = spi_read_status_register(flash); - + /* FIXME: We can't tell if spi_read_status_register() failed. */ /* FIXME: We don't time out. */ - while (status_reg & SPI_SR_WIP) { - /* - * The WIP bit on S25F chips remains set to 1 if erase or - * programming errors occur, so we must check for those - * errors here. If an error is encountered, do a software - * reset to clear WIP and other volatile bits, otherwise - * the chip will be unresponsive to further commands. - */ - if (status_reg & SPI_SR_ERA_ERR) { - msg_cerr("Erase error occurred\n"); - s25f_legacy_software_reset(flash); - return -1; - } - if (status_reg & (1 << 6)) { - msg_cerr("Programming error occurred\n"); - s25f_legacy_software_reset(flash); - return -1; - } + while (spi_read_status_register(flash) & SPI_SR_WIP) programmer_delay(poll_delay); - status_reg = spi_read_status_register(flash); - } - + /* FIXME: Check the status register for errors. */ return 0; }
@@ -631,73 +488,6 @@ return spi_write_cmd(flash, 0xd8, false, addr, NULL, 0, 100 * 1000); }
-/* Used on Spansion/Cypress S25FS chips */ -int s25fs_block_erase_d8(struct flashctx *flash, - unsigned int addr, unsigned int blocklen) -{ - unsigned char cfg; - int result; - static int cr3nv_checked = 0; - - struct spi_command erase_cmds[] = { - { - .writecnt = JEDEC_WREN_OUTSIZE, - .writearr = (const unsigned char[]){ JEDEC_WREN }, - .readcnt = 0, - .readarr = NULL, - }, { - .writecnt = JEDEC_BE_D8_OUTSIZE, - .writearr = (const unsigned char[]){ - JEDEC_BE_D8, - (addr >> 16) & 0xff, - (addr >> 8) & 0xff, - (addr & 0xff) - }, - .readcnt = 0, - .readarr = NULL, - }, { - .writecnt = 0, - .writearr = NULL, - .readcnt = 0, - .readarr = NULL, - }}; - - /* Check if hybrid sector architecture is in use and, if so, - * switch to uniform sectors. */ - if (!cr3nv_checked) { - cfg = s25fs_read_cr(flash, CR3NV_ADDR); - if (!(cfg & CR3NV_20H_NV)) { - s25fs_write_cr(flash, CR3NV_ADDR, cfg | CR3NV_20H_NV); - s25fs_software_reset(flash); - - cfg = s25fs_read_cr(flash, CR3NV_ADDR); - if (!(cfg & CR3NV_20H_NV)) { - msg_cerr("%s: Unable to enable uniform " - "block sizes.\n", __func__); - return 1; - } - - msg_cdbg("\n%s: CR3NV updated (0x%02x -> 0x%02x)\n", - __func__, cfg, - s25fs_read_cr(flash, CR3NV_ADDR)); - /* Restore CR3V when flashrom exits */ - register_chip_restore(s25fs_restore_cr3nv, flash, cfg); - } - - cr3nv_checked = 1; - } - - result = spi_send_multicommand(flash, erase_cmds); - if (result) { - msg_cerr("%s failed during command execution at address 0x%x\n", - __func__, addr); - return result; - } - - programmer_delay(T_SE); - return spi_poll_wip(flash, 1000 * 10); -} - /* Block size is usually * 4k for PMC */ diff --git a/spi25_statusreg.c b/spi25_statusreg.c index 083a832..4b9b2a9 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -108,89 +108,6 @@ return readarr[0]; }
-static int spi_restore_status(struct flashctx *flash, uint8_t status) -{ - msg_cdbg("restoring chip status (0x%02x)\n", status); - return spi_write_status_register(flash, status); -} - -/* 'Read Any Register' used on Spansion/Cypress S25FS chips */ -int s25fs_read_cr(struct flashctx *const flash, uint32_t addr) -{ - int result; - uint8_t cfg; - /* By default, 8 dummy cycles are necessary for variable-latency - commands such as RDAR (see CR2NV[3:0]). */ - unsigned char read_cr_cmd[] = { - CMD_RDAR, - (addr >> 16) & 0xff, - (addr >> 8) & 0xff, - (addr & 0xff), - 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - }; - - result = spi_send_command(flash, sizeof(read_cr_cmd), 1, read_cr_cmd, &cfg); - if (result) { - msg_cerr("%s failed during command execution at address 0x%x\n", - __func__, addr); - return -1; - } - - return cfg; -} - -/* 'Write Any Register' used on Spansion/Cypress S25FS chips */ -int s25fs_write_cr(struct flashctx *const flash, - uint32_t addr, uint8_t data) -{ - int result; - struct spi_command cmds[] = { - { - .writecnt = JEDEC_WREN_OUTSIZE, - .writearr = (const unsigned char[]){ JEDEC_WREN }, - .readcnt = 0, - .readarr = NULL, - }, { - .writecnt = CMD_WRAR_LEN, - .writearr = (const unsigned char[]){ - CMD_WRAR, - (addr >> 16) & 0xff, - (addr >> 8) & 0xff, - (addr & 0xff), - data - }, - .readcnt = 0, - .readarr = NULL, - }, { - .writecnt = 0, - .writearr = NULL, - .readcnt = 0, - .readarr = NULL, - }}; - - result = spi_send_multicommand(flash, cmds); - if (result) { - msg_cerr("%s failed during command execution at address 0x%x\n", - __func__, addr); - return -1; - } - - programmer_delay(T_W); - return spi_poll_wip(flash, 1000 * 10); -} - -/* Used on Spansion/Cypress S25FS chips */ -int s25fs_restore_cr3nv(struct flashctx *const flash, uint8_t cfg) -{ - int ret = 0; - - msg_cdbg("Restoring CR3NV value to 0x%02x\n", cfg); - ret |= s25fs_write_cr(flash, CR3NV_ADDR, cfg); - ret |= s25fs_software_reset(flash); - return ret; -} - /* A generic block protection disable. * Tests if a protection is enabled with the block protection mask (bp_mask) and returns success otherwise. * Tests if the register bits are locked with the lock_mask (lock_mask). @@ -222,9 +139,6 @@ return 0; }
- /* restore status register content upon exit */ - register_chip_restore(spi_restore_status, flash, status); - msg_cdbg("Some block protection in effect, disabling... "); if ((status & lock_mask) != 0) { msg_cdbg("\n\tNeed to disable the register lock first... ");