Change in flashrom[master]: WIP: s25f.c: import file from cros flashrom
Nikolai Artemiev has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/46140 ) Change subject: WIP: s25f.c: import file from cros flashrom ...................................................................... WIP: s25f.c: import file from cros flashrom Imported from cros flashrom at hash `9c4c9a56b6a0370b383df9c75d71b3bd469e672d` Signed-off-by: Nikolai Artemiev <nartemiev@google.com> Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 --- M Makefile M chipdrivers.h M flash.h M flashrom.c A s25f.c M spi.h 6 files changed, 585 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/46140/1 diff --git a/Makefile b/Makefile index 8af7042..2de7131 100644 --- a/Makefile +++ b/Makefile @@ -638,7 +638,7 @@ CHIP_OBJS = jedec.o stm50.o w39.o w29ee011.o \ sst28sf040.o 82802ab.o \ sst49lfxxxc.o sst_fwhub.o edi.o flashchips.o spi.o spi25.o spi25_statusreg.o \ - spi95.o opaque.o sfdp.o en29lv640b.o at45db.o writeprotect.o + spi95.o opaque.o sfdp.o en29lv640b.o at45db.o writeprotect.o s25f.o ############################################################################### # Library code. diff --git a/chipdrivers.h b/chipdrivers.h index cf03811..225dce7 100644 --- a/chipdrivers.h +++ b/chipdrivers.h @@ -20,7 +20,8 @@ #ifndef __CHIPDRIVERS_H__ #define __CHIPDRIVERS_H__ 1 -#include "flash.h" /* for chipaddr and flashctx */ +#include "flash.h" /* for chipaddr and flashctx */ +#include "writeprotect.h" /* for modifier_bits */ /* spi.c */ int spi_aai_write(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); @@ -170,6 +171,15 @@ int printlock_sst_fwhub(struct flashctx *flash); int unlock_sst_fwhub(struct flashctx *flash); +/* s25fl.c */ +int probe_spi_big_spansion(struct flashctx *flash); +int s25fl_block_erase(struct flashctx *flash, unsigned int addr, unsigned int blocklen); + +/* s25f.c */ +int s25f_get_modifier_bits(const struct flashctx *flash, struct modifier_bits *m); +int s25f_set_modifier_bits(const struct flashctx *flash, struct modifier_bits *m); +int s25fs_block_erase_d8(struct flashctx *flash, unsigned int addr, unsigned int blocklen); + /* w39.c */ int printlock_w39f010(struct flashctx * flash); int printlock_w39l010(struct flashctx * flash); diff --git a/flash.h b/flash.h index fefca9d..db64966 100644 --- a/flash.h +++ b/flash.h @@ -49,11 +49,17 @@ #define ERROR_OOM -100 #define TIMEOUT_ERROR -101 +struct flashrom_flashctx; +#define flashctx flashrom_flashctx /* TODO: Agree on a name and convert all occurences. */ + /* TODO: check using code for correct usage of types */ typedef uintptr_t chipaddr; #define PRIxPTR_WIDTH ((int)(sizeof(uintptr_t)*2)) int register_shutdown(int (*function) (void *data), void *data); +#define CHIP_RESTORE_CALLBACK int (*func) (struct flashctx *flash, uint8_t status) + +int register_chip_restore(CHIP_RESTORE_CALLBACK, struct 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); @@ -163,8 +169,6 @@ #define TEST_BAD_PRE (struct tested){ .probe = BAD, .read = BAD, .erase = BAD, .write = NT } #define TEST_BAD_PREW (struct tested){ .probe = BAD, .read = BAD, .erase = BAD, .write = BAD } -struct flashrom_flashctx; -#define flashctx flashrom_flashctx /* TODO: Agree on a name and convert all occurences. */ typedef int (erasefunc_t)(struct flashctx *flash, unsigned int addr, unsigned int blocklen); struct flashchip { diff --git a/flashrom.c b/flashrom.c index c18a04f..6ed863b 100644 --- a/flashrom.c +++ b/flashrom.c @@ -536,6 +536,15 @@ {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 */ @@ -586,6 +595,23 @@ 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/s25f.c b/s25f.c new file mode 100644 index 0000000..a30618c --- /dev/null +++ b/s25f.c @@ -0,0 +1,535 @@ +/* + * This file is part of the flashrom project. + * + * Copyright (C) 2014 Google Inc. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * Neither the name of Google or the names of contributors or + * licensors may be used to endorse or promote products derived from this + * software without specific prior written permission. + * + * This software is provided "AS IS," without a warranty of any kind. + * ALL EXPRESS OR IMPLIED CONDITIONS, REPRESENTATIONS AND WARRANTIES, + * INCLUDING ANY IMPLIED WARRANTY OF MERCHANTABILITY, FITNESS FOR A + * PARTICULAR PURPOSE OR NON-INFRINGEMENT, ARE HEREBY EXCLUDED. + * GOOGLE INC AND ITS LICENSORS SHALL NOT BE LIABLE + * FOR ANY DAMAGES SUFFERED BY LICENSEE AS A RESULT OF USING, MODIFYING + * OR DISTRIBUTING THIS SOFTWARE OR ITS DERIVATIVES. IN NO EVENT WILL + * GOOGLE OR ITS LICENSORS BE LIABLE FOR ANY LOST REVENUE, PROFIT OR DATA, + * OR FOR DIRECT, INDIRECT, SPECIAL, CONSEQUENTIAL, INCIDENTAL OR + * PUNITIVE DAMAGES, HOWEVER CAUSED AND REGARDLESS OF THE THEORY OF + * LIABILITY, ARISING OUT OF THE USE OF OR INABILITY TO USE THIS SOFTWARE, + * EVEN IF GOOGLE HAS BEEN ADVISED OF THE POSSIBILITY OF SUCH DAMAGES. + * + * s25f.c - Helper functions for Spansion S25FL and S25FS SPI flash chips. + * Uses 24 bit addressing for the FS chips and 32 bit addressing for the FL + * chips (which is required by the overlayed sector size devices). + * TODO: Implement fancy hybrid sector architecture helpers. + */ + +#include <string.h> + +#include "chipdrivers.h" +#include "hwaccess.h" +#include "spi.h" +#include "writeprotect.h" + +/* + * RDAR and WRAR are supported on chips which have more than one set of status + * and control registers and take an address of the register to read/write. + * WRR, RDSR2, and RDCR are used on chips with a more limited set of control/ + * status registers. + * + * WRR is somewhat peculiar. It shares the same opcode as JEDEC_WRSR, and if + * given one data byte (following the opcode) it acts the same way. If it's + * given two data bytes, the first data byte overwrites status register 1 + * and the second data byte overwrites config register 1. + */ +#define CMD_WRR 0x01 +#define CMD_WRDI 0x04 +#define CMD_RDSR2 0x07 /* note: read SR1 with JEDEC RDSR opcode */ +#define CMD_RDCR 0x35 +#define CMD_RDAR 0x65 +#define CMD_WRAR 0x71 + +/* TODO: For now, commands which use an address assume 24-bit addressing */ +#define CMD_WRR_LEN 3 +#define CMD_WRDI_LEN 1 +#define CMD_RDAR_LEN 4 +#define CMD_WRAR_LEN 5 + +#define CMD_RSTEN 0x66 +#define CMD_RST 0x99 + +#define CR1NV_ADDR 0x000002 +#define CR1_BPNV_O (1 << 3) +#define CR1_TBPROT_O (1 << 5) +#define CR3NV_ADDR 0x000004 +#define CR3NV_20H_NV (1 << 3) + +/* See "Embedded Algorithm Performance Tables for additional timing specs. */ +#define T_W 145 * 1000 /* NV register write time (145ms) */ +#define T_RPH 35 /* Reset pulse hold time (35us) */ +#define S25FS_T_SE 145 * 1000 /* Sector Erase Time (145ms) */ +#define S25FL_T_SE 130 * 1000 /* Sector Erase Time (130ms) */ + +static int s25f_legacy_software_reset(const 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; + } + + /* Allow time for reset command to execute. The datasheet specifies + * Trph = 35us, double that to be safe. */ + programmer_delay(T_RPH * 2); + + return 0; +} + +/* "Legacy software reset" is disabled by default on S25FS, use this instead. */ +static 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, + }}; + + result = spi_send_multicommand(flash, cmds); + if (result) { + msg_cerr("%s failed during command execution\n", __func__); + return result; + } + + /* Allow time for reset command to execute. Double tRPH to be safe. */ + programmer_delay(T_RPH * 2); + + return 0; +} + +static int s25f_poll_status(const struct flashctx *flash) +{ + uint8_t tmp = spi_read_status_register(flash); + + while (tmp & 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 (tmp & SPI_SR_ERA_ERR) { + msg_cerr("Erase error occurred\n"); + s25f_legacy_software_reset(flash); + return -1; + } + + if (tmp & (1 << 6)) { + msg_cerr("Programming error occurred\n"); + s25f_legacy_software_reset(flash); + return -1; + } + + programmer_delay(1000 * 10); + tmp = spi_read_status_register(flash); + } + + return 0; +} + +/* "Read Any Register" instruction only supported on S25FS */ +static int s25fs_read_cr(const struct flashctx *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; +} + +static int s25f_read_cr1(const struct flashctx *flash) +{ + int result; + uint8_t cfg; + unsigned char read_cr_cmd[] = { CMD_RDCR }; + + result = spi_send_command(flash, sizeof(read_cr_cmd), 1, read_cr_cmd, &cfg); + if (result) { + msg_cerr("%s failed during command execution\n", __func__); + return -1; + } + + return cfg; +} + +/* "Write Any Register" instruction only supported on S25FS */ +static int s25fs_write_cr(const struct flashctx *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 s25f_poll_status(flash); +} + +static int s25f_write_cr1(const struct flashctx *flash, 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_WRR_LEN, + .writearr = (const unsigned char[]){ + CMD_WRR, + spi_read_status_register(flash), + 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\n", __func__); + return -1; + } + + programmer_delay(T_W); + return s25f_poll_status(flash); +} + +static int s25fs_restore_cr3nv(struct flashctx *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; +} + +/* returns state of top/bottom block protection, or <0 to indicate error */ +static int s25f_get_tbprot_o(const struct flashctx *flash) +{ + int cr1 = s25f_read_cr1(flash); + + if (cr1 < 0) + return -1; + + /* + * 1 = BP starts at bottom (low address) + * 0 = BP start at top (high address) + */ + return cr1 & CR1_TBPROT_O ? 1 : 0; +} + +/* fills modifier_bits struct, returns 0 to indicate success */ +int s25f_get_modifier_bits(const struct flashctx *flash, + struct modifier_bits *m) +{ + int tmp; + + memset(m, 0, sizeof(*m)); + + tmp = s25f_get_tbprot_o(flash); + if (tmp < 0) + return -1; + m->tb = tmp; + + return 0; +} + +int s25f_set_modifier_bits(const struct flashctx *flash, + struct modifier_bits *m) +{ + int cr1, cr1_orig; + + cr1 = cr1_orig = s25f_read_cr1(flash); + if (cr1 < 0) + return -1; + + /* + * Clear BPNV so that setting BP2-0 in status register gets + * written to non-volatile memory. + * + * For TBPROT: + * 1 = BP starts at bottom (low address) + * 0 = BP start at top (high address) + */ + cr1 &= ~(CR1_BPNV_O | CR1_TBPROT_O); + cr1 |= m->tb ? CR1_TBPROT_O : 0; + + if (cr1 != cr1_orig) { + msg_cdbg("%s: setting cr1 bits to 0x%02x\n", __func__, cr1); + if (s25f_write_cr1(flash, cr1) < 0) + return -1; + if (s25f_read_cr1(flash) != cr1) { + msg_cerr("%s: failed to set CR1 value\n", __func__); + return -1; + } + } else { + msg_cdbg("%s: cr1 bits already match desired value: " + "0x%02x\n", __func__, cr1); + } + + return 0; +} + +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(S25FS_T_SE); + return s25f_poll_status(flash); +} + +int s25fl_block_erase(struct flashctx *flash, + unsigned int addr, unsigned int blocklen) +{ + int result; + + struct spi_command erase_cmds[] = { + { + .writecnt = JEDEC_WREN_OUTSIZE, + .writearr = (const unsigned char[]){ + JEDEC_WREN + }, + .readcnt = 0, + .readarr = NULL, + }, { + .writecnt = JEDEC_BE_DC_OUTSIZE, + .writearr = (const unsigned char[]){ + JEDEC_BE_DC, + (addr >> 24) & 0xff, + (addr >> 16) & 0xff, + (addr >> 8) & 0xff, + (addr & 0xff) + }, + .readcnt = 0, + .readarr = NULL, + }, { + .writecnt = 0, + .readcnt = 0, + } + }; + + 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(S25FL_T_SE); + return s25f_poll_status(flash); +} + + +int probe_spi_big_spansion(struct flashctx *flash) +{ + 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) { + for (size_t 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; +} diff --git a/spi.h b/spi.h index c1ca46a..09da579 100644 --- a/spi.h +++ b/spi.h @@ -111,6 +111,11 @@ #define JEDEC_BE_D7_OUTSIZE 0x04 #define JEDEC_BE_D7_INSIZE 0x00 +/* Block Erase 0xdc is supported by Spansion chips, takes 4 byte address */ +#define JEDEC_BE_DC 0xdc +#define JEDEC_BE_DC_OUTSIZE 0x05 +#define JEDEC_BE_DC_INSIZE 0x00 + /* Sector Erase 0x20 is supported by Macronix/SST chips. */ #define JEDEC_SE 0x20 #define JEDEC_SE_OUTSIZE 0x04 @@ -129,6 +134,7 @@ /* 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 */ -- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-MessageType: newchange
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46140 ) Change subject: WIP: s25f.c: import file from cros flashrom ...................................................................... Patch Set 1: (7 comments) https://review.coreboot.org/c/flashrom/+/46140/1/Makefile File Makefile: https://review.coreboot.org/c/flashrom/+/46140/1/Makefile@641 PS1, Line 641: s25f.o Make adding the file and putting it into the makefile its own patch. Also don't forget to add it to meson as well. https://review.coreboot.org/c/flashrom/+/46140/1/chipdrivers.h File chipdrivers.h: https://review.coreboot.org/c/flashrom/+/46140/1/chipdrivers.h@24 PS1, Line 24: #include "writeprotect.h" /* for modifier_bits */ unrelated. https://review.coreboot.org/c/flashrom/+/46140/1/flash.h File flash.h: https://review.coreboot.org/c/flashrom/+/46140/1/flash.h@52 PS1, Line 52: struct flashrom_flashctx; : #define flashctx flashrom_flashctx /* TODO: Agree on a name and convert all occurences. */ can you make this move its own patch, nice and simple one. https://review.coreboot.org/c/flashrom/+/46140/1/flash.h@60 PS1, Line 60: #define CHIP_RESTORE_CALLBACK int (*func) (struct flashctx *flash, uint8_t status) : : int register_chip_restore(CHIP_RESTORE_CALLBACK, struct flashctx *flash, uint8_t status); add this chip_restore support into its own patch. https://review.coreboot.org/c/flashrom/+/46140/1/flashrom.c File flashrom.c: https://review.coreboot.org/c/flashrom/+/46140/1/flashrom.c@539 PS1, Line 539: #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]; chip restore patch https://review.coreboot.org/c/flashrom/+/46140/1/flashrom.c@598 PS1, Line 598: //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; : } chip restore patch https://review.coreboot.org/c/flashrom/+/46140/1/flashrom.c@2319 PS1, Line 2319: { probably need to call chip restore here? -- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Thu, 15 Oct 2020 06:36:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Hello build bot (Jenkins), Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/46140 to look at the new patch set (#2). Change subject: WIP: s25f.c: import file from cros flashrom ...................................................................... WIP: s25f.c: import file from cros flashrom Imported from cros flashrom at hash `9c4c9a56b6a0370b383df9c75d71b3bd469e672d` Signed-off-by: Nikolai Artemiev <nartemiev@google.com> Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 --- M Makefile M chipdrivers.h A s25f.c M spi.h 4 files changed, 553 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/46140/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 2 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: newpatchset
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46140 ) Change subject: WIP: s25f.c: import file from cros flashrom ...................................................................... Patch Set 2: (7 comments) https://review.coreboot.org/c/flashrom/+/46140/1/Makefile File Makefile: https://review.coreboot.org/c/flashrom/+/46140/1/Makefile@641 PS1, Line 641: s25f.o
Make adding the file and putting it into the makefile its own patch. […] I think this is done now that chip restore has been extracted to a separate patch - or do you want me to separate out the addition of s25f.c from the header changes in this patch?
https://review.coreboot.org/c/flashrom/+/46140/1/chipdrivers.h File chipdrivers.h: https://review.coreboot.org/c/flashrom/+/46140/1/chipdrivers.h@24 PS1, Line 24: #include "writeprotect.h" /* for modifier_bits */
unrelated. It provides the `struct modifier_bits` type used in the s25f prototypes.
https://review.coreboot.org/c/flashrom/+/46140/1/flash.h File flash.h: https://review.coreboot.org/c/flashrom/+/46140/1/flash.h@52 PS1, Line 52: struct flashrom_flashctx; : #define flashctx flashrom_flashctx /* TODO: Agree on a name and convert all occurences. */
can you make this move its own patch, nice and simple one. Done - I think it's actually better to leave the typedef where it was since it was close to the struct definition. I have moved the function prototype that used it instead (in the chip restore patch).
https://review.coreboot.org/c/flashrom/+/46140/1/flash.h@60 PS1, Line 60: #define CHIP_RESTORE_CALLBACK int (*func) (struct flashctx *flash, uint8_t status) : : int register_chip_restore(CHIP_RESTORE_CALLBACK, struct flashctx *flash, uint8_t status);
add this chip_restore support into its own patch. Done
https://review.coreboot.org/c/flashrom/+/46140/1/flashrom.c File flashrom.c: https://review.coreboot.org/c/flashrom/+/46140/1/flashrom.c@539 PS1, Line 539: #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];
chip restore patch Done
https://review.coreboot.org/c/flashrom/+/46140/1/flashrom.c@598 PS1, Line 598: //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; : }
chip restore patch Done
https://review.coreboot.org/c/flashrom/+/46140/1/flashrom.c@2319 PS1, Line 2319: {
probably need to call chip restore here? Done
-- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 2 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Fri, 06 Nov 2020 01:09:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec@chromium.org> Gerrit-MessageType: comment
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46140 ) Change subject: WIP: s25f.c: import file from cros flashrom ...................................................................... Patch Set 2: (10 comments) https://review.coreboot.org/c/flashrom/+/46140/2/chipdrivers.h File chipdrivers.h: https://review.coreboot.org/c/flashrom/+/46140/2/chipdrivers.h@24 PS2, Line 24: #include "writeprotect.h" /* for modifier_bits */ I don't think this is needed in this commit for adding the chip initial support. https://review.coreboot.org/c/flashrom/+/46140/2/s25f.c File s25f.c: https://review.coreboot.org/c/flashrom/+/46140/2/s25f.c@34 PS2, Line 34: * s25f.c - Helper functions for Spansion S25FL and S25FS SPI flash chips. : * Uses 24 bit addressing for the FS chips and 32 bit addressing for the FL : * chips (which is required by the overlayed sector size devices). : * TODO: Implement fancy hybrid sector architecture helpers. separate out comment from license header. https://review.coreboot.org/c/flashrom/+/46140/2/s25f.c@43 PS2, Line 43: #include "hwaccess.h" is this needed? https://review.coreboot.org/c/flashrom/+/46140/2/s25f.c@45 PS2, Line 45: #include "writeprotect.h" ditto. https://review.coreboot.org/c/flashrom/+/46140/2/s25f.c@192 PS2, Line 192: unsigned char read_cr_cmd[] = { : CMD_RDAR, : (addr >> 16) & 0xff, : (addr >> 8) & 0xff, : (addr & 0xff), : 0x00, 0x00, 0x00, 0x00, : 0x00, 0x00, 0x00, 0x00, : }; looks like these various cmd blocks though the file are badly formatted. Please make it consistent with the rest of flashrom such as in spi25.c as an example I guess. https://review.coreboot.org/c/flashrom/+/46140/2/s25f.c@489 PS2, Line 489: et; = 0; https://review.coreboot.org/c/flashrom/+/46140/2/s25f.c@494 PS2, Line 494: if (!ret) { invert the ret check logic to avoid such a big block under a if branch. ``` if (ret) return ret; ``` https://review.coreboot.org/c/flashrom/+/46140/2/s25f.c@531 PS2, Line 531: return ret = https://review.coreboot.org/c/flashrom/+/46140/2/s25f.c@531 PS2, Line 531: return ret = https://review.coreboot.org/c/flashrom/+/46140/2/s25f.c@534 PS2, Line 534: 0; ret -- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 2 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Fri, 06 Nov 2020 03:22:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Hello build bot (Jenkins), Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/46140 to look at the new patch set (#3). Change subject: WIP: s25f.c: import file from cros flashrom ...................................................................... WIP: s25f.c: import file from cros flashrom Imported from cros flashrom at hash `9c4c9a56b6a0370b383df9c75d71b3bd469e672d` Signed-off-by: Nikolai Artemiev <nartemiev@google.com> Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 --- M Makefile M chipdrivers.h A s25f.c M spi.h 4 files changed, 555 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/46140/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 3 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: newpatchset
Hello build bot (Jenkins), Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/46140 to look at the new patch set (#4). Change subject: WIP: s25f.c: import file from cros flashrom ...................................................................... WIP: s25f.c: import file from cros flashrom Imported from cros flashrom at hash `9c4c9a56b6a0370b383df9c75d71b3bd469e672d` Signed-off-by: Nikolai Artemiev <nartemiev@google.com> Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 --- M Makefile M chipdrivers.h A s25f.c M spi.h 4 files changed, 552 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/46140/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 4 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: newpatchset
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46140 ) Change subject: WIP: s25f.c: import file from cros flashrom ...................................................................... Patch Set 4: (11 comments) https://review.coreboot.org/c/flashrom/+/46140/2/chipdrivers.h File chipdrivers.h: https://review.coreboot.org/c/flashrom/+/46140/2/chipdrivers.h@24 PS2, Line 24: #include "writeprotect.h" /* for modifier_bits */
I don't think this is needed in this commit for adding the chip initial support. It provides the modifier_bits struct, used by the s25f_{set,get}_modifier_bits() prototypes. We could leave them out, but I think it's worth adding them now.
https://review.coreboot.org/c/flashrom/+/46140/2/s25f.c File s25f.c: https://review.coreboot.org/c/flashrom/+/46140/2/s25f.c@34 PS2, Line 34: * s25f.c - Helper functions for Spansion S25FL and S25FS SPI flash chips. : * Uses 24 bit addressing for the FS chips and 32 bit addressing for the FL : * chips (which is required by the overlayed sector size devices). : * TODO: Implement fancy hybrid sector architecture helpers.
separate out comment from license header. Done
https://review.coreboot.org/c/flashrom/+/46140/2/s25f.c@43 PS2, Line 43: #include "hwaccess.h"
is this needed? It was needed for the `be_to_cpu32` macro, but I have changed the code that used it now.
https://review.coreboot.org/c/flashrom/+/46140/2/s25f.c@45 PS2, Line 45: #include "writeprotect.h"
ditto. It provides the `struct modifier_bits` type.
https://review.coreboot.org/c/flashrom/+/46140/2/s25f.c@192 PS2, Line 192: unsigned char read_cr_cmd[] = { : CMD_RDAR, : (addr >> 16) & 0xff, : (addr >> 8) & 0xff, : (addr & 0xff), : 0x00, 0x00, 0x00, 0x00, : 0x00, 0x00, 0x00, 0x00, : };
looks like these various cmd blocks though the file are badly formatted. […] I've updated this one. I'm not sure what the indent levels should be for the others (e.g. in s25fs_write_cr), I can't find a comparable example in spi25.c. It seems like a weird mix of tabulation and regular indentation rules.
https://review.coreboot.org/c/flashrom/+/46140/2/s25f.c@489 PS2, Line 489: et;
= 0; This was never really used to store a return value, only to store the return code from spi_send_command(). I've changed it accordingly.
https://review.coreboot.org/c/flashrom/+/46140/2/s25f.c@494 PS2, Line 494: if (!ret) {
invert the ret check logic to avoid such a big block under a if branch. […] Done
https://review.coreboot.org/c/flashrom/+/46140/2/s25f.c@531 PS2, Line 531: return
ret = Restructured, no longer have ret variable.
https://review.coreboot.org/c/flashrom/+/46140/2/s25f.c@531 PS2, Line 531: return
ret = ditto
https://review.coreboot.org/c/flashrom/+/46140/2/s25f.c@534 PS2, Line 534: 0;
ret ditto
https://review.coreboot.org/c/flashrom/+/46140/4/s25f.c File s25f.c: https://review.coreboot.org/c/flashrom/+/46140/4/s25f.c@487 PS4, Line 487: int probe_spi_big_spansion(struct flashctx *flash) n.b. I have changed this function quite a bit: - Added context to msg_gdbg() debugging calls - Replaced type punned union + memcpy + endianness conversion with bitshift + bitwise or - Flattened control flow - Changed unsigned char -> uint8_t Compare at patchset 2 to patchset 4 to see the changes. -- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 4 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Fri, 06 Nov 2020 08:16:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec@chromium.org> Gerrit-MessageType: comment
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46140 ) Change subject: WIP: s25f.c: import file from cros flashrom ...................................................................... Patch Set 5: (3 comments) https://review.coreboot.org/c/flashrom/+/46140/2//COMMIT_MSG Commit Message: https://review.coreboot.org/c/flashrom/+/46140/2//COMMIT_MSG@9 PS2, Line 9: Imported "Adapted" I think you are at the stage to flesh out this commit message with details now. https://review.coreboot.org/c/flashrom/+/46140/5/Makefile File Makefile: https://review.coreboot.org/c/flashrom/+/46140/5/Makefile@641 PS5, Line 641: s25f.o add this to the meson build system as well https://review.coreboot.org/c/flashrom/+/46140/2/chipdrivers.h File chipdrivers.h: https://review.coreboot.org/c/flashrom/+/46140/2/chipdrivers.h@24 PS2, Line 24: #include "writeprotect.h" /* for modifier_bits */
It provides the modifier_bits struct, used by the s25f_{set,get}_modifier_bits() prototypes. […] I hear you however I think it's worth separating out into another commit. write-protect support is its own thing over the base level chip support. It makes the commits easier to review as well. just unstage the commit and make it a follow up commit, should be easy enough to separate out the hunks for this.
-- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 5 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Sam McNally <sammc@google.com> Gerrit-Comment-Date: Fri, 13 Nov 2020 04:22:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec@chromium.org> Comment-In-Reply-To: Nikolai Artemiev <nartemiev@google.com> Gerrit-MessageType: comment
Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/46140 to look at the new patch set (#8). Change subject: WIP: s25f.c: bring over basic support from cros flashrom ...................................................................... WIP: s25f.c: bring over basic support from cros flashrom Adapted from from cros flashrom. Signed-off-by: Nikolai Artemiev <nartemiev@google.com> Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 --- M Makefile M chipdrivers.h M meson.build A s25f.c M spi.h 5 files changed, 434 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/46140/8 -- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 8 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Sam McNally <sammc@google.com> Gerrit-MessageType: newpatchset
Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/46140 to look at the new patch set (#9). Change subject: s25f.c: import basic support from cros flashrom ...................................................................... s25f.c: import basic support from cros flashrom Adapted from cros flashrom at hash, brings over basic support but does not include writeprotect code. BUG=b:153800073 TEST=builds Signed-off-by: Nikolai Artemiev <nartemiev@google.com> Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 --- M Makefile M chipdrivers.h M meson.build A s25f.c M spi.h 5 files changed, 433 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/46140/9 -- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 9 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Sam McNally <sammc@google.com> Gerrit-MessageType: newpatchset
Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/46140 to look at the new patch set (#10). Change subject: s25f.c: import basic support from cros flashrom ...................................................................... s25f.c: import basic support from cros flashrom Adapted from cros flashrom, brings over basic support but does not include writeprotect code. BUG=b:153800073 TEST=builds Signed-off-by: Nikolai Artemiev <nartemiev@google.com> Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 --- M Makefile M chipdrivers.h M meson.build A s25f.c M spi.h 5 files changed, 433 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/46140/10 -- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 10 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Sam McNally <sammc@google.com> Gerrit-MessageType: newpatchset
Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/46140 to look at the new patch set (#11). Change subject: s25f.c: import from cros flashrom ...................................................................... s25f.c: import from cros flashrom Adapted from cros flashrom at hash `9c4c9a56b6a0370b383df9c75d71b3bd469e672d`. Brings over basic support for s25f chips, but does not include writeprotect support which will be added in a following patch. BUG=b:153800073 TEST=builds Signed-off-by: Nikolai Artemiev <nartemiev@google.com> Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 --- M Makefile M chipdrivers.h M meson.build A s25f.c M spi.h 5 files changed, 433 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/46140/11 -- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 11 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Sam McNally <sammc@google.com> Gerrit-MessageType: newpatchset
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46140 ) Change subject: s25f.c: import from cros flashrom ...................................................................... Patch Set 11: (3 comments) https://review.coreboot.org/c/flashrom/+/46140/2//COMMIT_MSG Commit Message: https://review.coreboot.org/c/flashrom/+/46140/2//COMMIT_MSG@9 PS2, Line 9: Imported
"Adapted" […] Done
https://review.coreboot.org/c/flashrom/+/46140/5/Makefile File Makefile: https://review.coreboot.org/c/flashrom/+/46140/5/Makefile@641 PS5, Line 641: s25f.o
add this to the meson build system as well Done
https://review.coreboot.org/c/flashrom/+/46140/2/chipdrivers.h File chipdrivers.h: https://review.coreboot.org/c/flashrom/+/46140/2/chipdrivers.h@24 PS2, Line 24: #include "writeprotect.h" /* for modifier_bits */
I hear you however I think it's worth separating out into another commit. […] Done
-- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 11 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Sam McNally <sammc@google.com> Gerrit-Comment-Date: Mon, 23 Nov 2020 03:01:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec@chromium.org> Comment-In-Reply-To: Nikolai Artemiev <nartemiev@google.com> Gerrit-MessageType: comment
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46140 ) Change subject: s25f.c: import from cros flashrom ...................................................................... Patch Set 11: (4 comments) https://review.coreboot.org/c/flashrom/+/46140/11//COMMIT_MSG Commit Message: https://review.coreboot.org/c/flashrom/+/46140/11//COMMIT_MSG@7 PS11, Line 7: import from cros flashrom still says "import". What about a commit short msg that actually says what this does? `s25f.c: Implement probe and block erasure logic for Spansion` https://review.coreboot.org/c/flashrom/+/46140/11//COMMIT_MSG@12 PS11, Line 12: Brings over basic support for s25f chips Can you please elaborate here with details. The following implements Spansion probe logic which is different because .. and block erasures for Spansion that do .. https://review.coreboot.org/c/flashrom/+/46140/11//COMMIT_MSG@12 PS11, Line 12: but does not include : writeprotect support which will be added in a following patch. That is irrelevant here. https://review.coreboot.org/c/flashrom/+/46140/11/chipdrivers.h File chipdrivers.h: https://review.coreboot.org/c/flashrom/+/46140/11/chipdrivers.h@23 PS11, Line 23: /* unrelated change -- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 11 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Sam McNally <sammc@google.com> Gerrit-Comment-Date: Mon, 23 Nov 2020 04:09:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/46140 to look at the new patch set (#12). Change subject: s25f.c: implement probing and block erasers for Spansion ...................................................................... s25f.c: implement probing and block erasers for Spansion This adds support for Spansion 25FLxxxx chips. These chips require their own probing logic because the first 6 bytes returned by RDID must be examined to identify the chip. New erase functions are required as the chips support multiple sector layouts, and the default layout must be changed to be able to erase the entire flash. Adapted from cros flashrom at `9c4c9a56b6a0370b383df9c75d71b3bd469e672d`. BUG=b:153800073 TEST=builds Signed-off-by: Nikolai Artemiev <nartemiev@google.com> Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 --- M Makefile M chipdrivers.h M meson.build A s25f.c M spi.h 5 files changed, 433 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/46140/12 -- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 12 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Sam McNally <sammc@google.com> Gerrit-MessageType: newpatchset
Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/46140 to look at the new patch set (#13). Change subject: s25f.c: implement probing and block erasers for Spansion ...................................................................... s25f.c: implement probing and block erasers for Spansion This adds support for Spansion 25Fxxxxx chips. These chips require their own probing logic because the first 6 bytes returned by RDID must be examined to identify the chip. New erase functions are required as the chips support multiple sector layouts, and the default layout must be changed to be able to erase the entire flash. Adapted from cros flashrom at `9c4c9a56b6a0370b383df9c75d71b3bd469e672d`. BUG=b:153800073 TEST=builds Signed-off-by: Nikolai Artemiev <nartemiev@google.com> Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 --- M Makefile M chipdrivers.h M meson.build A s25f.c M spi.h 5 files changed, 433 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/46140/13 -- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 13 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Sam McNally <sammc@google.com> Gerrit-MessageType: newpatchset
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46140 ) Change subject: s25f.c: implement probing and block erasers for Spansion ...................................................................... Patch Set 13: (3 comments) https://review.coreboot.org/c/flashrom/+/46140/11//COMMIT_MSG Commit Message: https://review.coreboot.org/c/flashrom/+/46140/11//COMMIT_MSG@7 PS11, Line 7: import from cros flashrom
still says "import". What about a commit short msg that actually says what this does? […] Done
https://review.coreboot.org/c/flashrom/+/46140/11//COMMIT_MSG@12 PS11, Line 12: Brings over basic support for s25f chips
Can you please elaborate here with details. […] Done
https://review.coreboot.org/c/flashrom/+/46140/11//COMMIT_MSG@12 PS11, Line 12: but does not include : writeprotect support which will be added in a following patch.
That is irrelevant here. Done
-- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 13 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Sam McNally <sammc@google.com> Gerrit-Comment-Date: Mon, 23 Nov 2020 06:25:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec@chromium.org> Gerrit-MessageType: comment
Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/46140 to look at the new patch set (#14). Change subject: s25f.c: implement probing and block erasers for Spansion ...................................................................... s25f.c: implement probing and block erasers for Spansion This adds support for Spansion 25Fxxxxx chips. These chips require their own probing logic because the first 6 bytes returned by RDID must be examined to identify the chip. New erase functions are required as the chips support multiple sector layouts, and the default layout must be changed to be able to erase the entire flash. Adapted from cros flashrom at `9c4c9a56b6a0370b383df9c75d71b3bd469e672d`. BUG=b:153800073 TEST=builds Signed-off-by: Nikolai Artemiev <nartemiev@google.com> Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 --- M Makefile M chipdrivers.h M meson.build A s25f.c M spi.h 5 files changed, 433 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/46140/14 -- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 14 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Sam McNally <sammc@google.com> Gerrit-MessageType: newpatchset
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46140 ) Change subject: s25f.c: implement probing and block erasers for Spansion ...................................................................... Patch Set 14: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 14 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Sam McNally <sammc@google.com> Gerrit-Comment-Date: Mon, 23 Nov 2020 07:05:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46140 ) Change subject: s25f.c: implement probing and block erasers for Spansion ...................................................................... Patch Set 14: (2 comments) https://review.coreboot.org/c/flashrom/+/46140/14/s25f.c File s25f.c: https://review.coreboot.org/c/flashrom/+/46140/14/s25f.c@1 PS14, Line 1: /* : * This file is part of the flashrom project. : * : * Copyright (C) 2014 Google Inc. : * : * Redistribution and use in source and binary forms, with or without : * modification, are permitted provided that the following conditions : * are met: : * : * Redistributions of source code must retain the above copyright : * notice, this list of conditions and the following disclaimer. : * : * Redistributions in binary form must reproduce the above copyright : * notice, this list of conditions and the following disclaimer in the : * documentation and/or other materials provided with the distribution. : * : * Neither the name of Google or the names of contributors or : * licensors may be used to endorse or promote products derived from this : * software without specific prior written permission. : * : * This software is provided "AS IS," without a warranty of any kind. : * ALL EXPRESS OR IMPLIED CONDITIONS, REPRESENTATIONS AND WARRANTIES, : * INCLUDING ANY IMPLIED WARRANTY OF MERCHANTABILITY, FITNESS FOR A : * PARTICULAR PURPOSE OR NON-INFRINGEMENT, ARE HEREBY EXCLUDED. : * GOOGLE INC AND ITS LICENSORS SHALL NOT BE LIABLE : * FOR ANY DAMAGES SUFFERED BY LICENSEE AS A RESULT OF USING, MODIFYING : * OR DISTRIBUTING THIS SOFTWARE OR ITS DERIVATIVES. IN NO EVENT WILL : * GOOGLE OR ITS LICENSORS BE LIABLE FOR ANY LOST REVENUE, PROFIT OR DATA, : * OR FOR DIRECT, INDIRECT, SPECIAL, CONSEQUENTIAL, INCIDENTAL OR : * PUNITIVE DAMAGES, HOWEVER CAUSED AND REGARDLESS OF THE THEORY OF : * LIABILITY, ARISING OUT OF THE USE OF OR INABILITY TO USE THIS SOFTWARE, : * EVEN IF GOOGLE HAS BEEN ADVISED OF THE POSSIBILITY OF SUCH DAMAGES. : */ Use, ``` /* * This file is part of the flashrom project. * * Copyright (C) 2020 The Chromium OS Authors * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation; either version 2 of the License, or * (at your option) any later version. * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. */ ``` and send a patch to our tree to fix it up for us. https://review.coreboot.org/c/flashrom/+/46140/14/s25f.c@370 PS14, Line 370: int probe_spi_big_spansion(struct flashctx *flash) : { : uint8_t cmd = JEDEC_RDID; : uint8_t dev_id[6]; /* We care only about 6 first bytes */ : : if (spi_send_command(flash, sizeof(cmd), sizeof(dev_id), &cmd, dev_id)) : return 0; : : msg_gdbg("Read id bytes: "); : for (size_t i = 0; i < sizeof(dev_id); i++) : msg_gdbg(" 0x%02x", dev_id[i]); : msg_gdbg(".\n"); : : /* : * 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 : * : */ : : uint32_t model_id = : dev_id[1] << 24 | : dev_id[2] << 16 | : dev_id[4] << 8 | : dev_id[5] << 0; : : if (dev_id[0] == flash->chip->manufacture_id && model_id == flash->chip->model_id) : return 1; : : return 0; : } : I think this duplicates logic that is in spi25.c ``` static void rdid_get_ids(unsigned char *readarr, int bytes, uint32_t *id1, uint32_t *id2) `` I'll allow it for now but I would like to see a follow up if possible to consolidate it into the canonical rdid paths from spi25.c as I cannot see anything too bespoke going on here. -- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 14 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Sam McNally <sammc@google.com> Gerrit-Comment-Date: Mon, 23 Nov 2020 07:12:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46140 ) Change subject: s25f.c: implement probing and block erasers for Spansion ...................................................................... Patch Set 14: (1 comment) https://review.coreboot.org/c/flashrom/+/46140/14/chipdrivers.h File chipdrivers.h: https://review.coreboot.org/c/flashrom/+/46140/14/chipdrivers.h@173 PS14, Line 173: /* s25fl.c */ I don't see a s25fl.c. I guess it was merged into s25f.c at some point. -- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 14 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Sam McNally <sammc@google.com> Gerrit-Comment-Date: Mon, 23 Nov 2020 07:14:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46140 ) Change subject: s25f.c: implement probing and block erasers for Spansion ...................................................................... Patch Set 14: -Code-Review (1 comment) https://review.coreboot.org/c/flashrom/+/46140/14/chipdrivers.h File chipdrivers.h: https://review.coreboot.org/c/flashrom/+/46140/14/chipdrivers.h@23 PS14, Line 23: /* for chipaddr and flashctx */ unrelated change, why click Done? -- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 14 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Sam McNally <sammc@google.com> Gerrit-Comment-Date: Mon, 23 Nov 2020 07:28:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/46140 to look at the new patch set (#15). Change subject: s25f.c: implement probing and block erasers for Spansion ...................................................................... s25f.c: implement probing and block erasers for Spansion This adds support for Spansion 25Fxxxxx chips. These chips require their own probing logic because the first 6 bytes returned by RDID must be examined to identify the chip. New erase functions are required as the chips support multiple sector layouts, and the default layout must be changed to be able to erase the entire flash. Adapted from cros flashrom at `9c4c9a56b6a0370b383df9c75d71b3bd469e672d`. BUG=b:153800073 TEST=builds Signed-off-by: Nikolai Artemiev <nartemiev@google.com> Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 --- M Makefile M chipdrivers.h M meson.build A s25f.c M spi.h 5 files changed, 404 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/46140/15 -- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 15 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Sam McNally <sammc@google.com> Gerrit-MessageType: newpatchset
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46140 ) Change subject: s25f.c: implement probing and block erasers for Spansion ...................................................................... Patch Set 15: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 15 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Sam McNally <sammc@google.com> Gerrit-Comment-Date: Mon, 30 Nov 2020 23:50:15 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46140 ) Change subject: s25f.c: implement probing and block erasers for Spansion ...................................................................... Patch Set 15: (4 comments) https://review.coreboot.org/c/flashrom/+/46140/14/chipdrivers.h File chipdrivers.h: https://review.coreboot.org/c/flashrom/+/46140/14/chipdrivers.h@23 PS14, Line 23: /* for chipaddr and flashctx */
unrelated change, why click Done? This is just an extra tab that got left behind, I clicked done because I had deleted the write protect code that comment was referring to. I've reverted the tab now.
https://review.coreboot.org/c/flashrom/+/46140/14/chipdrivers.h@173 PS14, Line 173: /* s25fl.c */
I don't see a s25fl.c. I guess it was merged into s25f.c at some point. Done
https://review.coreboot.org/c/flashrom/+/46140/14/s25f.c File s25f.c: https://review.coreboot.org/c/flashrom/+/46140/14/s25f.c@1 PS14, Line 1: /* : * This file is part of the flashrom project. : * : * Copyright (C) 2014 Google Inc. : * : * Redistribution and use in source and binary forms, with or without : * modification, are permitted provided that the following conditions : * are met: : * : * Redistributions of source code must retain the above copyright : * notice, this list of conditions and the following disclaimer. : * : * Redistributions in binary form must reproduce the above copyright : * notice, this list of conditions and the following disclaimer in the : * documentation and/or other materials provided with the distribution. : * : * Neither the name of Google or the names of contributors or : * licensors may be used to endorse or promote products derived from this : * software without specific prior written permission. : * : * This software is provided "AS IS," without a warranty of any kind. : * ALL EXPRESS OR IMPLIED CONDITIONS, REPRESENTATIONS AND WARRANTIES, : * INCLUDING ANY IMPLIED WARRANTY OF MERCHANTABILITY, FITNESS FOR A : * PARTICULAR PURPOSE OR NON-INFRINGEMENT, ARE HEREBY EXCLUDED. : * GOOGLE INC AND ITS LICENSORS SHALL NOT BE LIABLE : * FOR ANY DAMAGES SUFFERED BY LICENSEE AS A RESULT OF USING, MODIFYING : * OR DISTRIBUTING THIS SOFTWARE OR ITS DERIVATIVES. IN NO EVENT WILL : * GOOGLE OR ITS LICENSORS BE LIABLE FOR ANY LOST REVENUE, PROFIT OR DATA, : * OR FOR DIRECT, INDIRECT, SPECIAL, CONSEQUENTIAL, INCIDENTAL OR : * PUNITIVE DAMAGES, HOWEVER CAUSED AND REGARDLESS OF THE THEORY OF : * LIABILITY, ARISING OUT OF THE USE OF OR INABILITY TO USE THIS SOFTWARE, : * EVEN IF GOOGLE HAS BEEN ADVISED OF THE POSSIBILITY OF SUCH DAMAGES. : */
Use, […] Done - except I've kept the original copyright year & name.
https://review.coreboot.org/c/flashrom/+/46140/14/s25f.c@370 PS14, Line 370: int probe_spi_big_spansion(struct flashctx *flash) : { : uint8_t cmd = JEDEC_RDID; : uint8_t dev_id[6]; /* We care only about 6 first bytes */ : : if (spi_send_command(flash, sizeof(cmd), sizeof(dev_id), &cmd, dev_id)) : return 0; : : msg_gdbg("Read id bytes: "); : for (size_t i = 0; i < sizeof(dev_id); i++) : msg_gdbg(" 0x%02x", dev_id[i]); : msg_gdbg(".\n"); : : /* : * 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 : * : */ : : uint32_t model_id = : dev_id[1] << 24 | : dev_id[2] << 16 | : dev_id[4] << 8 | : dev_id[5] << 0; : : if (dev_id[0] == flash->chip->manufacture_id && model_id == flash->chip->model_id) : return 1; : : return 0; : } :
I think this duplicates logic that is in spi25.c […] I've looked into rdid_get_ids() and I don't think it is the same as what's going on here - they examine different bytes in the RDID response. Maybe we could combine them somehow, but as you say that can be done later.
-- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 15 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Sam McNally <sammc@google.com> Gerrit-Comment-Date: Mon, 30 Nov 2020 23:52:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sam McNally <sammc@google.com> Comment-In-Reply-To: Edward O'Callaghan <quasisec@chromium.org> Gerrit-MessageType: comment
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46140 ) Change subject: s25f.c: implement probing and block erasers for Spansion ...................................................................... Patch Set 16: (5 comments) https://review.coreboot.org/c/flashrom/+/46140/1/Makefile File Makefile: https://review.coreboot.org/c/flashrom/+/46140/1/Makefile@641 PS1, Line 641: s25f.o
I think this is done now that chip restore has been extracted to a separate patch - or do you want m […] Ack
https://review.coreboot.org/c/flashrom/+/46140/11/chipdrivers.h File chipdrivers.h: https://review.coreboot.org/c/flashrom/+/46140/11/chipdrivers.h@23 PS11, Line 23: /*
unrelated change Ack. Please remember to resolve your comments next time!
https://review.coreboot.org/c/flashrom/+/46140/1/chipdrivers.h File chipdrivers.h: https://review.coreboot.org/c/flashrom/+/46140/1/chipdrivers.h@24 PS1, Line 24: #include "writeprotect.h" /* for modifier_bits */
It provides the `struct modifier_bits` type used in the s25f prototypes. Ack
https://review.coreboot.org/c/flashrom/+/46140/2/s25f.c File s25f.c: https://review.coreboot.org/c/flashrom/+/46140/2/s25f.c@192 PS2, Line 192: unsigned char read_cr_cmd[] = { : CMD_RDAR, : (addr >> 16) & 0xff, : (addr >> 8) & 0xff, : (addr & 0xff), : 0x00, 0x00, 0x00, 0x00, : 0x00, 0x00, 0x00, 0x00, : };
I've updated this one. I'm not sure what the indent levels should be for the others (e.g. […] Ack
https://review.coreboot.org/c/flashrom/+/46140/4/s25f.c File s25f.c: https://review.coreboot.org/c/flashrom/+/46140/4/s25f.c@487 PS4, Line 487: int probe_spi_big_spansion(struct flashctx *flash)
n.b. I have changed this function quite a bit: […] Ack
-- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 16 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Sam McNally <sammc@google.com> Gerrit-Comment-Date: Thu, 03 Dec 2020 12:31:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec@chromium.org> Comment-In-Reply-To: Nikolai Artemiev <nartemiev@google.com> Gerrit-MessageType: comment
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/46140 ) Change subject: s25f.c: implement probing and block erasers for Spansion ...................................................................... s25f.c: implement probing and block erasers for Spansion This adds support for Spansion 25Fxxxxx chips. These chips require their own probing logic because the first 6 bytes returned by RDID must be examined to identify the chip. New erase functions are required as the chips support multiple sector layouts, and the default layout must be changed to be able to erase the entire flash. Adapted from cros flashrom at `9c4c9a56b6a0370b383df9c75d71b3bd469e672d`. BUG=b:153800073 TEST=builds Signed-off-by: Nikolai Artemiev <nartemiev@google.com> Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Reviewed-on: https://review.coreboot.org/c/flashrom/+/46140 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec@chromium.org> --- M Makefile M chipdrivers.h M meson.build A s25f.c M spi.h 5 files changed, 404 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved diff --git a/Makefile b/Makefile index 327d43b..1f7ea46 100644 --- a/Makefile +++ b/Makefile @@ -648,7 +648,7 @@ CHIP_OBJS = jedec.o stm50.o w39.o w29ee011.o \ sst28sf040.o 82802ab.o \ sst49lfxxxc.o sst_fwhub.o edi.o flashchips.o spi.o spi25.o spi25_statusreg.o \ - spi95.o opaque.o sfdp.o en29lv640b.o at45db.o writeprotect.o + spi95.o opaque.o sfdp.o en29lv640b.o at45db.o writeprotect.o s25f.o ############################################################################### # Library code. diff --git a/chipdrivers.h b/chipdrivers.h index cf03811..b42c363 100644 --- a/chipdrivers.h +++ b/chipdrivers.h @@ -170,6 +170,11 @@ int printlock_sst_fwhub(struct flashctx *flash); int unlock_sst_fwhub(struct flashctx *flash); +/* s25f.c */ +int probe_spi_big_spansion(struct flashctx *flash); +int s25fl_block_erase(struct flashctx *flash, unsigned int addr, unsigned int blocklen); +int s25fs_block_erase_d8(struct flashctx *flash, unsigned int addr, unsigned int blocklen); + /* w39.c */ int printlock_w39f010(struct flashctx * flash); int printlock_w39l010(struct flashctx * flash); diff --git a/meson.build b/meson.build index 1acbdb3..fd5a5ef 100644 --- a/meson.build +++ b/meson.build @@ -354,6 +354,7 @@ srcs += 'opaque.c' srcs += 'print.c' srcs += 'programmer.c' +srcs += 's25f.c' srcs += 'sfdp.c' srcs += 'spi25.c' srcs += 'spi25_statusreg.c' diff --git a/s25f.c b/s25f.c new file mode 100644 index 0000000..32af53e --- /dev/null +++ b/s25f.c @@ -0,0 +1,391 @@ +/* + * This file is part of the flashrom project. + * + * Copyright (C) 2014 Google LLC. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +/* + * s25f.c - Helper functions for Spansion S25FL and S25FS SPI flash chips. + * Uses 24 bit addressing for the FS chips and 32 bit addressing for the FL + * chips (which is required by the overlayed sector size devices). + * TODO: Implement fancy hybrid sector architecture helpers. + */ + +#include <string.h> + +#include "chipdrivers.h" +#include "spi.h" +#include "writeprotect.h" + +/* + * RDAR and WRAR are supported on chips which have more than one set of status + * and control registers and take an address of the register to read/write. + * WRR, RDSR2, and RDCR are used on chips with a more limited set of control/ + * status registers. + * + * WRR is somewhat peculiar. It shares the same opcode as JEDEC_WRSR, and if + * given one data byte (following the opcode) it acts the same way. If it's + * given two data bytes, the first data byte overwrites status register 1 + * and the second data byte overwrites config register 1. + */ +#define CMD_WRR 0x01 +#define CMD_WRDI 0x04 +#define CMD_RDSR2 0x07 /* note: read SR1 with JEDEC RDSR opcode */ +#define CMD_RDCR 0x35 +#define CMD_RDAR 0x65 +#define CMD_WRAR 0x71 + +/* TODO: For now, commands which use an address assume 24-bit addressing */ +#define CMD_WRR_LEN 3 +#define CMD_WRDI_LEN 1 +#define CMD_RDAR_LEN 4 +#define CMD_WRAR_LEN 5 + +#define CMD_RSTEN 0x66 +#define CMD_RST 0x99 + +#define CR1NV_ADDR 0x000002 +#define CR1_BPNV_O (1 << 3) +#define CR1_TBPROT_O (1 << 5) +#define CR3NV_ADDR 0x000004 +#define CR3NV_20H_NV (1 << 3) + +/* See "Embedded Algorithm Performance Tables for additional timing specs. */ +#define T_W 145 * 1000 /* NV register write time (145ms) */ +#define T_RPH 35 /* Reset pulse hold time (35us) */ +#define S25FS_T_SE 145 * 1000 /* Sector Erase Time (145ms) */ +#define S25FL_T_SE 130 * 1000 /* Sector Erase Time (130ms) */ + +static int s25f_legacy_software_reset(const struct flashctx *flash) +{ + struct spi_command cmds[] = { + { + .writecnt = 1, + .writearr = (const uint8_t[]){ CMD_RSTEN }, + .readcnt = 0, + .readarr = NULL, + }, { + .writecnt = 1, + .writearr = (const uint8_t[]){ 0xf0 }, + .readcnt = 0, + .readarr = NULL, + }, { + .writecnt = 0, + .writearr = NULL, + .readcnt = 0, + .readarr = NULL, + }}; + + int result = spi_send_multicommand(flash, cmds); + if (result) { + msg_cerr("%s failed during command execution\n", __func__); + return result; + } + + /* Allow time for reset command to execute. The datasheet specifies + * Trph = 35us, double that to be safe. */ + programmer_delay(T_RPH * 2); + + return 0; +} + +/* "Legacy software reset" is disabled by default on S25FS, use this instead. */ +static int s25fs_software_reset(struct flashctx *flash) +{ + struct spi_command cmds[] = { + { + .writecnt = 1, + .writearr = (const uint8_t[]){ CMD_RSTEN }, + .readcnt = 0, + .readarr = NULL, + }, { + .writecnt = 1, + .writearr = (const uint8_t[]){ CMD_RST }, + .readcnt = 0, + .readarr = NULL, + }, { + .writecnt = 0, + .writearr = NULL, + .readcnt = 0, + .readarr = NULL, + }}; + + int result = spi_send_multicommand(flash, cmds); + if (result) { + msg_cerr("%s failed during command execution\n", __func__); + return result; + } + + /* Allow time for reset command to execute. Double tRPH to be safe. */ + programmer_delay(T_RPH * 2); + + return 0; +} + +static int s25f_poll_status(const struct flashctx *flash) +{ + uint8_t tmp = spi_read_status_register(flash); + + while (tmp & 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 (tmp & SPI_SR_ERA_ERR) { + msg_cerr("Erase error occurred\n"); + s25f_legacy_software_reset(flash); + return -1; + } + + if (tmp & (1 << 6)) { + msg_cerr("Programming error occurred\n"); + s25f_legacy_software_reset(flash); + return -1; + } + + programmer_delay(1000 * 10); + tmp = spi_read_status_register(flash); + } + + return 0; +} + +/* "Read Any Register" instruction only supported on S25FS */ +static int s25fs_read_cr(const struct flashctx *flash, uint32_t addr) +{ + uint8_t cfg; + /* By default, 8 dummy cycles are necessary for variable-latency + commands such as RDAR (see CR2NV[3:0]). */ + uint8_t read_cr_cmd[] = { + CMD_RDAR, + (addr >> 16) & 0xff, + (addr >> 8) & 0xff, + (addr & 0xff), + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + }; + + int 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" instruction only supported on S25FS */ +static int s25fs_write_cr(const struct flashctx *flash, + uint32_t addr, uint8_t data) +{ + struct spi_command cmds[] = { + { + .writecnt = JEDEC_WREN_OUTSIZE, + .writearr = (const uint8_t[]){ JEDEC_WREN }, + .readcnt = 0, + .readarr = NULL, + }, { + .writecnt = CMD_WRAR_LEN, + .writearr = (const uint8_t[]){ + CMD_WRAR, + (addr >> 16) & 0xff, + (addr >> 8) & 0xff, + (addr & 0xff), + data + }, + .readcnt = 0, + .readarr = NULL, + }, { + .writecnt = 0, + .writearr = NULL, + .readcnt = 0, + .readarr = NULL, + }}; + + int 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 s25f_poll_status(flash); +} + +static int s25fs_restore_cr3nv(struct flashctx *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; +} + +int s25fs_block_erase_d8(struct flashctx *flash, + uint32_t addr, uint32_t blocklen) +{ + static int cr3nv_checked = 0; + + struct spi_command erase_cmds[] = { + { + .writecnt = JEDEC_WREN_OUTSIZE, + .writearr = (const uint8_t[]){ JEDEC_WREN }, + .readcnt = 0, + .readarr = NULL, + }, { + .writecnt = JEDEC_BE_D8_OUTSIZE, + .writearr = (const uint8_t[]){ + 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) { + uint8_t 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; + } + + int 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(S25FS_T_SE); + return s25f_poll_status(flash); +} + +int s25fl_block_erase(struct flashctx *flash, + uint32_t addr, uint32_t blocklen) +{ + struct spi_command erase_cmds[] = { + { + .writecnt = JEDEC_WREN_OUTSIZE, + .writearr = (const uint8_t[]){ + JEDEC_WREN + }, + .readcnt = 0, + .readarr = NULL, + }, { + .writecnt = JEDEC_BE_DC_OUTSIZE, + .writearr = (const uint8_t[]){ + JEDEC_BE_DC, + (addr >> 24) & 0xff, + (addr >> 16) & 0xff, + (addr >> 8) & 0xff, + (addr & 0xff) + }, + .readcnt = 0, + .readarr = NULL, + }, { + .writecnt = 0, + .readcnt = 0, + } + }; + + int 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(S25FL_T_SE); + return s25f_poll_status(flash); +} + + +int probe_spi_big_spansion(struct flashctx *flash) +{ + uint8_t cmd = JEDEC_RDID; + uint8_t dev_id[6]; /* We care only about 6 first bytes */ + + if (spi_send_command(flash, sizeof(cmd), sizeof(dev_id), &cmd, dev_id)) + return 0; + + msg_gdbg("Read id bytes: "); + for (size_t i = 0; i < sizeof(dev_id); i++) + msg_gdbg(" 0x%02x", dev_id[i]); + msg_gdbg(".\n"); + + /* + * 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 + * + */ + + uint32_t model_id = + dev_id[1] << 24 | + dev_id[2] << 16 | + dev_id[4] << 8 | + dev_id[5] << 0; + + if (dev_id[0] == flash->chip->manufacture_id && model_id == flash->chip->model_id) + return 1; + + return 0; +} diff --git a/spi.h b/spi.h index c1ca46a..09da579 100644 --- a/spi.h +++ b/spi.h @@ -111,6 +111,11 @@ #define JEDEC_BE_D7_OUTSIZE 0x04 #define JEDEC_BE_D7_INSIZE 0x00 +/* Block Erase 0xdc is supported by Spansion chips, takes 4 byte address */ +#define JEDEC_BE_DC 0xdc +#define JEDEC_BE_DC_OUTSIZE 0x05 +#define JEDEC_BE_DC_INSIZE 0x00 + /* Sector Erase 0x20 is supported by Macronix/SST chips. */ #define JEDEC_SE 0x20 #define JEDEC_SE_OUTSIZE 0x04 @@ -129,6 +134,7 @@ /* 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 */ -- To view, visit https://review.coreboot.org/c/flashrom/+/46140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d23f9c36ce8b2959807fbeee7f60e02444e3763 Gerrit-Change-Number: 46140 Gerrit-PatchSet: 17 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Sam McNally <sammc@google.com> Gerrit-MessageType: merged
participants (3)
-
Edward O'Callaghan (Code Review) -
Nikolai Artemiev (Code Review)
-
Sam McNally (Code Review)