Hi!
I'm attaching a patch which adds support for Spansion S25FL127S flash chip in 64KiB and 256KiB mode. I also tested 64KiB mode with FT4232H Mini Module which works fine (please see attached logs).
Best regards, Jernej Skrabec
On Mon, 24 Nov 2014 23:59:15 +0100 (CET) Jernej Škrabec jernej.skrabec@planet.si wrote:
Hi!
I'm attaching a patch which adds support for Spansion S25FL127S flash chip in 64KiB and 256KiB mode. I also tested 64KiB mode with FT4232H Mini Module which works fine (please see attached logs).
Hello Jernej,
thanks for your patch! Please note that before we can include your patch you need to sign the Developer's Certificate of Origin by stating so in the patch/email. We can not include your patch as is! Please read http://flashrom.org/Development_Guidelines#Sign-off_Procedure for details, thanks.
Apart from that the naming of the chip definitions does not make much sense. The model number of the S25FL127S does not denote the sector layout in any way (unlike other Spansion chips). It would be best to write a custom probing function that checks for the flags in the configuration register and alters struct flashchip accordingly (similar to what we do for at45db chips). But for now I would accept a simple suffix to the "S25FL127S" name and propose S25FL127S-64kB and S25FL127S-256kB.
Also, the comment in the patch is not correct AFAICS. Quote from the datasheet "A P4E command applied to a sector that is larger than 4 kbytes will not be executed and will not set the E_ERR status." This contradicts the comment that states that a bit is changed. There are some minor other things I'd like to change but I can make these changes myself. Thank you very much for your effort. Please reply with your sign-off so that I can integrate the patch soon.
Hello Stefan,
sorry for late response, I've been busy these days. I hope that this new patch is acceptable for now.
Best regards, Jernej Škrabec
----- Izvorno sporočilo ----- Od: "Stefan Tauner" stefan.tauner@alumni.tuwien.ac.at Za: "Jernej Škrabec" jernej.skrabec@planet.si Kp: flashrom@flashrom.org Poslano: sobota, 29. november 2014 21:03:26 Zadeva: Re: [flashrom] [PATCH] Support for S25FL127S
On Mon, 24 Nov 2014 23:59:15 +0100 (CET) Jernej Škrabec jernej.skrabec@planet.si wrote:
Hi!
I'm attaching a patch which adds support for Spansion S25FL127S flash chip in 64KiB and 256KiB mode. I also tested 64KiB mode with FT4232H Mini Module which works fine (please see attached logs).
Hello Jernej,
thanks for your patch! Please note that before we can include your patch you need to sign the Developer's Certificate of Origin by stating so in the patch/email. We can not include your patch as is! Please read http://flashrom.org/Development_Guidelines#Sign-off_Procedure for details, thanks.
Apart from that the naming of the chip definitions does not make much sense. The model number of the S25FL127S does not denote the sector layout in any way (unlike other Spansion chips). It would be best to write a custom probing function that checks for the flags in the configuration register and alters struct flashchip accordingly (similar to what we do for at45db chips). But for now I would accept a simple suffix to the "S25FL127S" name and propose S25FL127S-64kB and S25FL127S-256kB.
Also, the comment in the patch is not correct AFAICS. Quote from the datasheet "A P4E command applied to a sector that is larger than 4 kbytes will not be executed and will not set the E_ERR status." This contradicts the comment that states that a bit is changed. There are some minor other things I'd like to change but I can make these changes myself. Thank you very much for your effort. Please reply with your sign-off so that I can integrate the patch soon.
On Mon, 1 Dec 2014 23:22:41 +0100 (CET) Jernej Škrabec jernej.skrabec@planet.si wrote:
Hello Stefan,
sorry for late response, I've been busy these days. I hope that this new patch is acceptable for now.
Hi,
you replied way faster than we did so... :) Your patch is still missing the most important part AFAICS: the signed-off-by line as described here: http://flashrom.org/Development_Guidelines#Sign-off_Procedure
Hi!
I put "signed-off-by" in the first line of the new patch. Did I miss something? Or it would be better to put patch in the mail directly?
Regards, Jernej Škrabec
----- Izvorno sporočilo ----- Od: "Stefan Tauner" stefan.tauner@alumni.tuwien.ac.at Za: "Jernej Škrabec" jernej.skrabec@planet.si Kp: flashrom@flashrom.org Poslano: ponedeljek, 1. december 2014 23:44:36 Zadeva: Re: [flashrom] [PATCH] Support for S25FL127S
On Mon, 1 Dec 2014 23:22:41 +0100 (CET) Jernej Škrabec jernej.skrabec@planet.si wrote:
Hello Stefan,
sorry for late response, I've been busy these days. I hope that this new patch is acceptable for now.
Hi,
you replied way faster than we did so... :) Your patch is still missing the most important part AFAICS: the signed-off-by line as described here: http://flashrom.org/Development_Guidelines#Sign-off_Procedure
On Mon, 1 Dec 2014 23:53:26 +0100 (CET) Jernej Škrabec jernej.skrabec@planet.si wrote:
Hi!
I put "signed-off-by" in the first line of the new patch. Did I miss something? Or it would be better to put patch in the mail directly?
No, sorry... too late already :) Thank you very much.
This flash chip can be configured (one time) to use 64 KiB or 256 KiB sectors. Additionally, in the 64 KiB mode it supports 16 4 KiB sub-sectors that can be (one time) programmed to be on the top or bottom of the device. The sub-sectors can be erased with the 0x20 opcode but because this opcode does not work with the remaining sectors and flashrom can not cope with that the 0x20 opcode is not supported yet.
This patch adds two definitions, one for the 64 KiB and 256 KiB configuration respectively. The device also shares the RDID with the various S25FL128 devices so we have to increase the maximum number of successfully probed chips to 8.
The 64 KiB mode was tested on real hardware.
Signed-off-by: Jernej Škrabec jernej.skrabec@planet.si Signed-off-by: Stefan Tauner stefan.tauner@alumni.tuwien.ac.at ---
Hi Jernej,
I have refined the patch a little bit. Can you please retest at least probing without the -c parameter? It should print all flash parts that share the same ID with your chip.
cli_classic.c | 4 ++-- flashchips.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ flashchips.h | 2 +- 3 files changed, 80 insertions(+), 3 deletions(-)
diff --git a/cli_classic.c b/cli_classic.c index 8588881..a2c2014 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -91,9 +91,9 @@ static int check_filename(char *filename, char *type)
int main(int argc, char *argv[]) { - /* Probe for up to three flash chips. */ const struct flashchip *chip = NULL; - struct flashctx flashes[6] = {{0}}; + /* Probe for up to eight flash chips. */ + struct flashctx flashes[8] = {{0}}; struct flashctx *fill_flash; const char *name; int namelen, opt, i, j; diff --git a/flashchips.c b/flashchips.c index 39fd671..7305abc 100644 --- a/flashchips.c +++ b/flashchips.c @@ -10437,6 +10437,83 @@ const struct flashchip flashchips[] = {
{ .vendor = "Spansion", + .name = "S25FL127S-64kB", /* hybrid: 32 (top or bottom) 4 kB sub-sectors + 64 kB sectors */ + .bustype = BUS_SPI, + .manufacture_id = SPANSION_ID, + .model_id = SPANSION_S25FL128, + .total_size = 16384, + .page_size = 256, + /* supports 4B addressing */ + /* OTP: 1024B total, 32B reserved; read 0x4B; write 0x42 */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP, + .tested = TEST_OK_PREW, + /* FIXME: we should distinguish the configuration on probing time like we do for AT45DB chips */ + .probe = probe_spi_rdid, + .probe_timing = TIMING_ZERO, + .block_erasers = { + { + /* This chip supports erasing of 32 so-called "parameter sectors" with + * opcode 0x20 which may be configured to be on top or bottom of the address + * space. Trying to access an address outside these 4kB blocks does have no + * effect on the memory contents, e.g. + .eraseblocks = { + {4 * 1024, 32}, + {64 * 1024, 254} // inaccessible + }, + .block_erase = spi_block_erase_20, + }, { */ + .eraseblocks = { { 64 * 1024, 256} }, + .block_erase = spi_block_erase_d8, + }, { + .eraseblocks = { { 16384 * 1024, 1} }, + .block_erase = spi_block_erase_60, + }, { + .eraseblocks = { { 16384 * 1024, 1} }, + .block_erase = spi_block_erase_c7, + } + }, + .printlock = spi_prettyprint_status_register_bp2_srwd, + .unlock = spi_disable_blockprotect_bp2_srwd, /* #WP pin write-protects SRWP bit. */ + .write = spi_chip_write_256, /* Multi I/O supported */ + .read = spi_chip_read, /* Fast read (0x0B) and multi I/O supported */ + .voltage = {2700, 3600}, + }, + + { + .vendor = "Spansion", + .name = "S25FL127S-256kB", /* uniform 256kB sectors */ + .bustype = BUS_SPI, + .manufacture_id = SPANSION_ID, + .model_id = SPANSION_S25FL128, + .total_size = 16384, + .page_size = 512, + /* supports 4B addressing */ + /* OTP: 1024B total, 32B reserved; read 0x4B; write 0x42 */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP, + .tested = TEST_UNTESTED, + .probe = probe_spi_rdid, + .probe_timing = TIMING_ZERO, + .block_erasers = { + { + .eraseblocks = { {256 * 1024, 64} }, + .block_erase = spi_block_erase_d8, + }, { + .eraseblocks = { { 16384 * 1024, 1} }, + .block_erase = spi_block_erase_60, + }, { + .eraseblocks = { { 16384 * 1024, 1} }, + .block_erase = spi_block_erase_c7, + } + }, + .printlock = spi_prettyprint_status_register_bp2_srwd, + .unlock = spi_disable_blockprotect_bp2_srwd, /* #WP pin write-protects SRWP bit. */ + .write = spi_chip_write_256, /* Multi I/O supported */ + .read = spi_chip_read, /* Fast read (0x0B) and multi I/O supported */ + .voltage = {2700, 3600}, + }, + + { + .vendor = "Spansion", .name = "S25FL128P......0", /* uniform 64 kB sectors */ .bustype = BUS_SPI, .manufacture_id = SPANSION_ID, diff --git a/flashchips.h b/flashchips.h index a850e4c..8a5ec5b 100644 --- a/flashchips.h +++ b/flashchips.h @@ -600,7 +600,7 @@ #define SPANSION_S25FL016A 0x0214 #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 S25FL128P, S25FL128S and S25FL129P (including dual-die S70FL256P) variants (EDI supported) */ +#define SPANSION_S25FL128 0x2018 /* Same ID for various S25FL127S, S25FL128P, S25FL128S and S25FL129P (including dual-die S70FL256P) variants (EDI supported) */ #define SPANSION_S25FL256 0x0219 #define SPANSION_S25FL512 0x0220 #define SPANSION_S25FL204 0x4013
-
Jernej Škrabec -
Stefan Tauner