Patchwork Support for S25FL127S

login
register
about
Submitter Jernej Škrabec
Date 2014-12-01 22:22:41
Message ID <327714832.82931.1417472561837.JavaMail.root@planet.si>
Download mbox | patch
Permalink /patch/4252/
State Superseded
Headers show

Comments

Jernej Škrabec - 2014-12-01 22:22:41
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.
Stefan Tauner - 2014-12-01 22:44:36
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
Jernej Škrabec - 2014-12-01 22:53:26
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
Stefan Tauner - 2014-12-01 23:08:59
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.

Patch

Signed-off-by: Jernej Škrabec <jernej.skrabec@planet.si>
Index: flashchips.c
===================================================================
diff --git a/trunk/flashchips.c b/trunk/flashchips.c
--- a/trunk/flashchips.c	(revision 1855)
+++ b/trunk/flashchips.c	(working copy)
@@ -10434,6 +10434,72 @@ 
 		.read		= spi_chip_read, /* Fast read (0x0B) and multi I/O supported */
 		.voltage	= {2700, 3600},
 	},
+	
+	{
+		.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,
+		.probe		= probe_spi_rdid,
+		.probe_timing	= TIMING_ZERO,
+		.block_erasers	= {
+			{
+				.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,
+		.read		= spi_chip_read, /* Fast read (0x0B) 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,
+		.read		= spi_chip_read, /* Fast read (0x0B) supported */
+		.voltage	= {2700, 3600},
+	},
 
 	{
 		.vendor		= "Spansion",