Patchwork Macronix MX25L6445E

login
register
about
Submitter Wagner, Helge (GE Intelligent Platforms)
Date 2011-06-27 15:06:42
Message ID <966E8B72E12C6C469AF763C7C8D09F51024EEB1D@BUDMLVEM06.e2k.ad.ge.com>
Download mbox | patch
Permalink /patch/3218/
State Superseded
Headers show

Comments

Wagner, Helge (GE Intelligent Platforms) - 2011-06-27 15:06:42
Hello Stefan,

>i have not looked at the datasheet(s), but maybe there is a way to distinguish the two devices (with another id command for example)?
I have asked Macronix, and they suggest to use the REMS2 function, which is supported on the MX25L6445E (returns 0xC216) and is not supported on the MX25L6405 (according to Macronix, it returns 0x0000 or 0xFFFF for this command).

So now there is no longer a need to re-order the erase functions for the MX25L6405.

Here is my new patch.

Signed-off-by: Helge Wagner <helge.wagner@ge.com>

Regards,
Helge Wagner



-----Original Message-----
From: Stefan Tauner [mailto:stefan.tauner@student.tuwien.ac.at] 
Sent: Freitag, 17. Juni 2011 15:49
To: Wagner, Helge (GE Intelligent Platforms)
Cc: flashrom@flashrom.org; Carl-Daniel Hailfinger
Subject: Re: [flashrom] [PATCH] Macronix MX25L6445E

On Mon, 30 May 2011 14:08:09 +0200
"Wagner, Helge (GE Intelligent Platforms)" <Helge.Wagner@ge.com> wrote:

> Hi,
> 
> i have tried to reprogram a Macronix MX25L6445E flash chip. This worked, but gave me an error message:
> 
> ERASE FAILED at 0x0001107f! Expected=0xff, Read=0x00, failed byte 
> count from 0x00010000-0x0001ffff: 0x9c15 ERASE FAILED!
> 
> Looking deeper into the datasheet, i saw that the MX25L6445E has the same ID as the MX25L6405, but with (at least) one difference:
> The MX25L6405 has a block size of 64KByte for erase function 20h, while the MX25L6445E is using 4KByte sector size for the same erase function 20h. As a result the spi_block_erase_20() will fail for the MX25L6445E (while the spi_block_erase_d8() which is used then will succeed).
> 
> I have now re-ordered the erase functions for the MX25L6405/MX25L6445E:
> spi_block_erase_d8      - first
> spi_block_erase_60       - second
> spi_block_erase_c7       - third
> spi_block_erase_20       - last
> So if one of the d8/60/c7 erase functions are successfull, the spi_block_erase_20 will never be called.
> 
> To be sure that the spi_block_erase_20 works for both the MX25L6405 and the MX25L6445E, we could change the block erase size from 64K to 4K. This should work even for the MX25L6405, but with the side effect of the erase taking longer than needed.
> 
> Any comments?
> 
> Please find my patches included.
> Signed-off-by: Helge Wagner <Helge.Wagner@ge.com>

hello helge

thanks for your patch!
i have not looked at the datasheet(s), but maybe there is a way to distinguish the two devices (with another id command for example)?
if we change the order of eraser functions as your patch suggests we should comment the reason precisely in the code. else we may introduce the previous behavior sometime later e.g. when we refine partial writes.
i personally don't see a problem apart from that, but i am not sure if it is safe either and wont break pending patches. so others have to ack it.
--
Kind regards/Mit freundlichen Grüßen, Stefan Tauner
Stefan Tauner - 2011-06-27 15:50:13
On Mon, 27 Jun 2011 17:06:42 +0200
"Wagner, Helge (GE Intelligent Platforms)" <Helge.Wagner@ge.com> wrote:

> Hello Stefan,
> 
> >i have not looked at the datasheet(s), but maybe there is a way to distinguish the two devices (with another id command for example)?
> I have asked Macronix, and they suggest to use the REMS2 function, which is supported on the MX25L6445E (returns 0xC216) and is not supported on the MX25L6405 (according to Macronix, it returns 0x0000 or 0xFFFF for this command).
> 
> So now there is no longer a need to re-order the erase functions for the MX25L6405.
> 
> Here is my new patch.
> 
> Signed-off-by: Helge Wagner <helge.wagner@ge.com>
> 
> Regards,
> Helge Wagner
> 
> diff -u flashrom/chipdrivers.h flashrom-patched/chipdrivers.h
> --- flashrom/chipdrivers.h	2011-06-27 15:21:57.000000000 +0200
> +++ flashrom-patched/chipdrivers.h	2011-06-27 16:29:25.000000000 +0200
> @@ -31,6 +31,7 @@
>  int probe_spi_rems(struct flashchip *flash);
>  int probe_spi_res1(struct flashchip *flash);
>  int probe_spi_res2(struct flashchip *flash);
> +int probe_spi_mx_2017(struct flashchip *flash);
>  int spi_write_enable(void);
>  int spi_write_disable(void);
>  int spi_block_erase_20(struct flashchip *flash, unsigned int addr, unsigned int blocklen);
> diff -u flashrom/flashchips.c flashrom-patched/flashchips.c
> --- flashrom/flashchips.c	2011-06-27 15:21:57.000000000 +0200
> +++ flashrom-patched/flashchips.c	2011-06-27 16:26:34.000000000 +0200
> @@ -4131,8 +4131,8 @@
>  		.total_size	= 8192,
>  		.page_size	= 256,
>  		.feature_bits	= FEATURE_WRSR_WREN,
> -		.tested		= TEST_OK_PROBE,
> -		.probe		= probe_spi_rdid,
> +		.tested		= TEST_UNTESTED,
> +		.probe		= probe_spi_mx_2017,
>  		.probe_timing	= TIMING_ZERO,
>  		.block_erasers	=
>  		{
> @@ -4141,6 +4141,40 @@
>  				.block_erase = spi_block_erase_20,
>  			}, {
>  				.eraseblocks = { {64 * 1024, 128} },
> +				.block_erase = spi_block_erase_d8,
> +			}, {
> +				.eraseblocks = { {8 * 1024 * 1024, 1} },
> +				.block_erase = spi_block_erase_60,
> +			}, {
> +				.eraseblocks = { {8 * 1024 * 1024, 1} },
> +				.block_erase = spi_block_erase_c7,
> +			}
> +		},
> +		.unlock		= spi_disable_blockprotect,
> +		.write		= spi_chip_write_256,
> +		.read		= spi_chip_read,
> +		.voltage	= {2700, 3600},
> +	},
> +
> +	{
> +		.vendor		= "Macronix",
> +		.name		= "MX25L6445E",
> +		.bustype	= CHIP_BUSTYPE_SPI,
> +		.manufacture_id	= MACRONIX_ID,
> +		.model_id	= MACRONIX_MX25L6445E,
> +		.total_size	= 8192,
> +		.page_size	= 256,
> +		.feature_bits	= FEATURE_WRSR_WREN,
> +		.tested		= TEST_OK_PREW,
> +		.probe		= probe_spi_mx_2017,
> +		.probe_timing	= TIMING_ZERO,
> +		.block_erasers	=
> +		{
> +			{
> +				.eraseblocks = { {4 * 1024, 2048} },
> +				.block_erase = spi_block_erase_20,
> +			}, {
> +				.eraseblocks = { {64 * 1024, 128} },
>  				.block_erase = spi_block_erase_d8,
>  			}, {
>  				.eraseblocks = { {8 * 1024 * 1024, 1} },
> diff -u flashrom/flashchips.h flashrom-patched/flashchips.h
> --- flashrom/flashchips.h	2011-06-27 15:21:57.000000000 +0200
> +++ flashrom-patched/flashchips.h	2011-06-27 15:46:22.000000000 +0200
> @@ -356,7 +356,8 @@
>  #define MACRONIX_MX25L8005	0x2014	/* Same as MX25V8005 */
>  #define MACRONIX_MX25L1605	0x2015	/* MX25L1605{,A,D} */
>  #define MACRONIX_MX25L3205	0x2016	/* MX25L3205{,A} */
> -#define MACRONIX_MX25L6405	0x2017	/* MX25L3205{,D} */
> +#define MACRONIX_MX25L6405	0x2017	/* MX25L6405{,D} */ 
> +#define MACRONIX_MX25L6445E	0x16	/* MX25L6445E, ID = REMS2 answer */
>  #define MACRONIX_MX25L12805	0x2018	/* MX25L12805 */
>  #define MACRONIX_MX25L1635D	0x2415
>  #define MACRONIX_MX25L1635E	0x2515	/* MX25L1635{E} */
> diff -u flashrom/spi.h flashrom-patched/spi.h
> --- flashrom/spi.h	2011-06-27 15:21:57.000000000 +0200
> +++ flashrom-patched/spi.h	2011-06-27 16:29:38.000000000 +0200
> @@ -40,6 +40,11 @@
>  #define JEDEC_REMS_OUTSIZE	0x04
>  #define JEDEC_REMS_INSIZE	0x02
>  
> +/* Read Electronic Manufacturer Signature 2 */
> +#define JEDEC_REMS2		0xEF
> +#define JEDEC_REMS2_OUTSIZE	0x04
> +#define JEDEC_REMS2_INSIZE	0x02
> +
>  /* Read Electronic Signature */
>  #define JEDEC_RES		0xab
>  #define JEDEC_RES_OUTSIZE	0x04
> diff -u flashrom/spi25.c flashrom-patched/spi25.c
> --- flashrom/spi25.c	2011-06-27 15:21:57.000000000 +0200
> +++ flashrom-patched/spi25.c	2011-06-27 16:30:33.000000000 +0200
> @@ -66,6 +66,27 @@
>  	return 0;
>  }
>  
> +static int spi_rems2(unsigned char *readarr)
> +{
> +	unsigned char cmd[JEDEC_REMS2_OUTSIZE] = { JEDEC_REMS2, 0, 0, 0 };
> +	uint32_t readaddr;
> +	int ret;
> +
> +	ret = spi_send_command(sizeof(cmd), JEDEC_REMS2_INSIZE, cmd, readarr);
> +	if (ret == SPI_INVALID_ADDRESS) {
> +		/* Find the lowest even address allowed for reads. */
> +		readaddr = (spi_get_valid_read_addr() + 1) & ~1;
> +		cmd[1] = (readaddr >> 16) & 0xff,
> +		cmd[2] = (readaddr >> 8) & 0xff,
> +		cmd[3] = (readaddr >> 0) & 0xff,
> +		ret = spi_send_command(sizeof(cmd), JEDEC_REMS2_INSIZE, cmd, readarr);
> +	}
> +	if (ret)
> +		return ret;
> +	msg_cspew("REMS2 returned %02x %02x. ", readarr[0], readarr[1]);
> +	return 0;
> +}
> +
>  static int spi_res(unsigned char *readarr, int bytes)
>  {
>  	unsigned char cmd[JEDEC_RES_OUTSIZE] = { JEDEC_RES, 0, 0, 0 };
> @@ -173,6 +194,49 @@
>  	return probe_spi_rdid_generic(flash, 3);
>  }
>  
> +int probe_spi_mx_2017(struct flashchip *flash)
> +{
> +	unsigned char readarr[4];
> +	uint32_t id1;
> +	uint32_t id2;
> +
> +	if (spi_rdid(readarr, 3)) {
> +		msg_cdbg("\n");
> +		return 0;
> +	}
> +	id1 = readarr[0];
> +	id2 = (readarr[1] << 8) | readarr[2];
> +
> +	if (id1 == MACRONIX_ID && id2 == MACRONIX_MX25L6405) {
> +	/* We have either MACRONIX_MX25L6405 or MACRONIX_MX25L6445E.
> +	 * MACRONIX_MX25L6405 has 64KB for erase function 20h and does not support REMS2 function,
> +	 * MACRONIX_MX25L6445E has 4KB for erase function 20h and does support REMS2 function.
> +	 * So to differentiate both, we execute REMS2 function (this is suggested by macronix).
> +	 */
> +		if (spi_rems2(readarr)) {
> +			msg_cdbg("\n");
> +			return 0;
> +		}
> +		id1 = readarr[0];
> +		id2 = readarr[1];
> +		if (id1 == MACRONIX_ID && id2 == MACRONIX_MX25L6445E) {
> +		/* MACRONIX_MX25L6445E */
> +			if (flash->model_id == MACRONIX_MX25L6445E)
> +				return 1;
> +			else
> +				return 0;
> +		} else {
> +		/* MACRONIX_MX25L6405 */
> +			if (flash->model_id == MACRONIX_MX25L6405)
> +				return 1;
> +			else
> +				return 0;	
> +		}
> +	}
> +	/* neither MACRONIX_MX25L6405 nor MACRONIX_MX25L6445E */
> +	return 0;
> +}
> +
>  int probe_spi_rdid4(struct flashchip *flash)
>  {
>  	/* Some SPI controllers do not support commands with writecnt=1 and
> 
> 

hello again and thanks for all you have done.

without looking at your patch in too much detail i would like to
mention another possibility to implement this. as i have done in my
sfdp and intel hardware sequencing patch you can alter the struct
flashchip *flash the probing function gets as parameter.
you can then change the erase block size (and name) according to the id
instead of using two chips and returning 0/1 respectively.

i dont know if that alternative is any better. one can argue that these
really are two independent chips and deserve their own flashchip
struct. OTOH are they almost identical. but that's probably true for
many chip (variants). so i think the scheme you have used is the right
one and this mail is mostly for documenting/educating (read:
blahblahblah :)

the other thing that sprang to my mind is the question if there are
other mx chips that need this kind of differentiation and if the scheme
should be more generalized. what about 0x2015, 0x2016, 0x2018? do they
also have such problems?
Carl-Daniel Hailfinger - 2011-06-27 18:28:10
Hello Helge,

thanks for your patch. It looks good to me on a first glance, and I hope
to review it in detail soon. A short comment (in reply to Stefan's
review) is at the end of this mail.

Am 27.06.2011 17:50 schrieb Stefan Tauner:
> On Mon, 27 Jun 2011 17:06:42 +0200
> "Wagner, Helge (GE Intelligent Platforms)" <Helge.Wagner@ge.com> wrote:
>
>   
>> Hello Stefan,
>>
>>     
>>> i have not looked at the datasheet(s), but maybe there is a way to distinguish the two devices (with another id command for example)?
>>>       
>> I have asked Macronix, and they suggest to use the REMS2 function, which is supported on the MX25L6445E (returns 0xC216) and is not supported on the MX25L6405 (according to Macronix, it returns 0x0000 or 0xFFFF for this command).
>>
>> So now there is no longer a need to re-order the erase functions for the MX25L6405.
>>
>> Here is my new patch.
>>
>> Signed-off-by: Helge Wagner <helge.wagner@ge.com>
>>
>> Regards,
>> Helge Wagner
>>
>> diff -u flashrom/chipdrivers.h flashrom-patched/chipdrivers.h
>> --- flashrom/chipdrivers.h	2011-06-27 15:21:57.000000000 +0200
>> +++ flashrom-patched/chipdrivers.h	2011-06-27 16:29:25.000000000 +0200
>> @@ -31,6 +31,7 @@
>>  int probe_spi_rems(struct flashchip *flash);
>>  int probe_spi_res1(struct flashchip *flash);
>>  int probe_spi_res2(struct flashchip *flash);
>> +int probe_spi_mx_2017(struct flashchip *flash);
>>  int spi_write_enable(void);
>>  int spi_write_disable(void);
>>  int spi_block_erase_20(struct flashchip *flash, unsigned int addr, unsigned int blocklen);
>> diff -u flashrom/flashchips.c flashrom-patched/flashchips.c
>> --- flashrom/flashchips.c	2011-06-27 15:21:57.000000000 +0200
>> +++ flashrom-patched/flashchips.c	2011-06-27 16:26:34.000000000 +0200
>> @@ -4131,8 +4131,8 @@
>>  		.total_size	= 8192,
>>  		.page_size	= 256,
>>  		.feature_bits	= FEATURE_WRSR_WREN,
>> -		.tested		= TEST_OK_PROBE,
>> -		.probe		= probe_spi_rdid,
>> +		.tested		= TEST_UNTESTED,
>> +		.probe		= probe_spi_mx_2017,
>>  		.probe_timing	= TIMING_ZERO,
>>  		.block_erasers	=
>>  		{
>> @@ -4141,6 +4141,40 @@
>>  				.block_erase = spi_block_erase_20,
>>  			}, {
>>  				.eraseblocks = { {64 * 1024, 128} },
>> +				.block_erase = spi_block_erase_d8,
>> +			}, {
>> +				.eraseblocks = { {8 * 1024 * 1024, 1} },
>> +				.block_erase = spi_block_erase_60,
>> +			}, {
>> +				.eraseblocks = { {8 * 1024 * 1024, 1} },
>> +				.block_erase = spi_block_erase_c7,
>> +			}
>> +		},
>> +		.unlock		= spi_disable_blockprotect,
>> +		.write		= spi_chip_write_256,
>> +		.read		= spi_chip_read,
>> +		.voltage	= {2700, 3600},
>> +	},
>> +
>> +	{
>> +		.vendor		= "Macronix",
>> +		.name		= "MX25L6445E",
>> +		.bustype	= CHIP_BUSTYPE_SPI,
>> +		.manufacture_id	= MACRONIX_ID,
>> +		.model_id	= MACRONIX_MX25L6445E,
>> +		.total_size	= 8192,
>> +		.page_size	= 256,
>> +		.feature_bits	= FEATURE_WRSR_WREN,
>> +		.tested		= TEST_OK_PREW,
>> +		.probe		= probe_spi_mx_2017,
>> +		.probe_timing	= TIMING_ZERO,
>> +		.block_erasers	=
>> +		{
>> +			{
>> +				.eraseblocks = { {4 * 1024, 2048} },
>> +				.block_erase = spi_block_erase_20,
>> +			}, {
>> +				.eraseblocks = { {64 * 1024, 128} },
>>  				.block_erase = spi_block_erase_d8,
>>  			}, {
>>  				.eraseblocks = { {8 * 1024 * 1024, 1} },
>> diff -u flashrom/flashchips.h flashrom-patched/flashchips.h
>> --- flashrom/flashchips.h	2011-06-27 15:21:57.000000000 +0200
>> +++ flashrom-patched/flashchips.h	2011-06-27 15:46:22.000000000 +0200
>> @@ -356,7 +356,8 @@
>>  #define MACRONIX_MX25L8005	0x2014	/* Same as MX25V8005 */
>>  #define MACRONIX_MX25L1605	0x2015	/* MX25L1605{,A,D} */
>>  #define MACRONIX_MX25L3205	0x2016	/* MX25L3205{,A} */
>> -#define MACRONIX_MX25L6405	0x2017	/* MX25L3205{,D} */
>> +#define MACRONIX_MX25L6405	0x2017	/* MX25L6405{,D} */ 
>> +#define MACRONIX_MX25L6445E	0x16	/* MX25L6445E, ID = REMS2 answer */
>>  #define MACRONIX_MX25L12805	0x2018	/* MX25L12805 */
>>  #define MACRONIX_MX25L1635D	0x2415
>>  #define MACRONIX_MX25L1635E	0x2515	/* MX25L1635{E} */
>> diff -u flashrom/spi.h flashrom-patched/spi.h
>> --- flashrom/spi.h	2011-06-27 15:21:57.000000000 +0200
>> +++ flashrom-patched/spi.h	2011-06-27 16:29:38.000000000 +0200
>> @@ -40,6 +40,11 @@
>>  #define JEDEC_REMS_OUTSIZE	0x04
>>  #define JEDEC_REMS_INSIZE	0x02
>>  
>> +/* Read Electronic Manufacturer Signature 2 */
>> +#define JEDEC_REMS2		0xEF
>> +#define JEDEC_REMS2_OUTSIZE	0x04
>> +#define JEDEC_REMS2_INSIZE	0x02
>> +
>>  /* Read Electronic Signature */
>>  #define JEDEC_RES		0xab
>>  #define JEDEC_RES_OUTSIZE	0x04
>> diff -u flashrom/spi25.c flashrom-patched/spi25.c
>> --- flashrom/spi25.c	2011-06-27 15:21:57.000000000 +0200
>> +++ flashrom-patched/spi25.c	2011-06-27 16:30:33.000000000 +0200
>> @@ -66,6 +66,27 @@
>>  	return 0;
>>  }
>>  
>> +static int spi_rems2(unsigned char *readarr)
>> +{
>> +	unsigned char cmd[JEDEC_REMS2_OUTSIZE] = { JEDEC_REMS2, 0, 0, 0 };
>> +	uint32_t readaddr;
>> +	int ret;
>> +
>> +	ret = spi_send_command(sizeof(cmd), JEDEC_REMS2_INSIZE, cmd, readarr);
>> +	if (ret == SPI_INVALID_ADDRESS) {
>> +		/* Find the lowest even address allowed for reads. */
>> +		readaddr = (spi_get_valid_read_addr() + 1) & ~1;
>> +		cmd[1] = (readaddr >> 16) & 0xff,
>> +		cmd[2] = (readaddr >> 8) & 0xff,
>> +		cmd[3] = (readaddr >> 0) & 0xff,
>> +		ret = spi_send_command(sizeof(cmd), JEDEC_REMS2_INSIZE, cmd, readarr);
>> +	}
>> +	if (ret)
>> +		return ret;
>> +	msg_cspew("REMS2 returned %02x %02x. ", readarr[0], readarr[1]);
>> +	return 0;
>> +}
>> +
>>  static int spi_res(unsigned char *readarr, int bytes)
>>  {
>>  	unsigned char cmd[JEDEC_RES_OUTSIZE] = { JEDEC_RES, 0, 0, 0 };
>> @@ -173,6 +194,49 @@
>>  	return probe_spi_rdid_generic(flash, 3);
>>  }
>>  
>> +int probe_spi_mx_2017(struct flashchip *flash)
>> +{
>> +	unsigned char readarr[4];
>> +	uint32_t id1;
>> +	uint32_t id2;
>> +
>> +	if (spi_rdid(readarr, 3)) {
>> +		msg_cdbg("\n");
>> +		return 0;
>> +	}
>> +	id1 = readarr[0];
>> +	id2 = (readarr[1] << 8) | readarr[2];
>> +
>> +	if (id1 == MACRONIX_ID && id2 == MACRONIX_MX25L6405) {
>> +	/* We have either MACRONIX_MX25L6405 or MACRONIX_MX25L6445E.
>> +	 * MACRONIX_MX25L6405 has 64KB for erase function 20h and does not support REMS2 function,
>> +	 * MACRONIX_MX25L6445E has 4KB for erase function 20h and does support REMS2 function.
>> +	 * So to differentiate both, we execute REMS2 function (this is suggested by macronix).
>> +	 */
>> +		if (spi_rems2(readarr)) {
>> +			msg_cdbg("\n");
>> +			return 0;
>> +		}
>> +		id1 = readarr[0];
>> +		id2 = readarr[1];
>> +		if (id1 == MACRONIX_ID && id2 == MACRONIX_MX25L6445E) {
>> +		/* MACRONIX_MX25L6445E */
>> +			if (flash->model_id == MACRONIX_MX25L6445E)
>> +				return 1;
>> +			else
>> +				return 0;
>> +		} else {
>> +		/* MACRONIX_MX25L6405 */
>> +			if (flash->model_id == MACRONIX_MX25L6405)
>> +				return 1;
>> +			else
>> +				return 0;	
>> +		}
>> +	}
>> +	/* neither MACRONIX_MX25L6405 nor MACRONIX_MX25L6445E */
>> +	return 0;
>> +}
>> +
>>  int probe_spi_rdid4(struct flashchip *flash)
>>  {
>>  	/* Some SPI controllers do not support commands with writecnt=1 and
>>
>>
>>     
> hello again and thanks for all you have done.
>
> without looking at your patch in too much detail i would like to
> mention another possibility to implement this. as i have done in my
> sfdp and intel hardware sequencing patch you can alter the struct
> flashchip *flash the probing function gets as parameter.
> you can then change the erase block size (and name) according to the id
> instead of using two chips and returning 0/1 respectively.
>   

Please avoid changing struct flashchip *flash in the probe function
unless it is absolutely necessary. The hack needed for SFDP should not
spread elsewhere, especially if we can handle this nicely with two
separate flashchip entries.

Regards,
Carl-Daniel
Wagner, Helge (GE Intelligent Platforms) - 2011-06-29 11:56:40
Hello all,

as Stefan suggested ("what about 0x2015, 0x2016, 0x2018? do they also
have such problems?"), I was looking at some more Macronix data sheets.
While doing this, I found out that my solution is NOT sufficient for
some of the Macronix 0x2017 flash chips.
I am currently working on a solution for this problem. Please stand by.

Regards,
Helge

-----Original Message-----
From: flashrom-bounces+helge.wagner=ge.com@flashrom.org
[mailto:flashrom-bounces+helge.wagner=ge.com@flashrom.org] On Behalf Of
Carl-Daniel Hailfinger
Sent: Montag, 27. Juni 2011 20:28
To: flashrom@flashrom.org
Subject: Re: [flashrom] [PATCH] Macronix MX25L6445E

Hello Helge,

thanks for your patch. It looks good to me on a first glance, and I hope
to review it in detail soon. A short comment (in reply to Stefan's
review) is at the end of this mail.

Am 27.06.2011 17:50 schrieb Stefan Tauner:
> On Mon, 27 Jun 2011 17:06:42 +0200
> "Wagner, Helge (GE Intelligent Platforms)" <Helge.Wagner@ge.com>
wrote:
>
>   
>> Hello Stefan,
>>
>>     
>>> i have not looked at the datasheet(s), but maybe there is a way to
distinguish the two devices (with another id command for example)?
>>>       
>> I have asked Macronix, and they suggest to use the REMS2 function,
which is supported on the MX25L6445E (returns 0xC216) and is not
supported on the MX25L6405 (according to Macronix, it returns 0x0000 or
0xFFFF for this command).
>>
>> So now there is no longer a need to re-order the erase functions for
the MX25L6405.
>>
>> Here is my new patch.
>>
>> Signed-off-by: Helge Wagner <helge.wagner@ge.com>
>>
>> Regards,
>> Helge Wagner
>>
>> diff -u flashrom/chipdrivers.h flashrom-patched/chipdrivers.h
>> --- flashrom/chipdrivers.h	2011-06-27 15:21:57.000000000 +0200
>> +++ flashrom-patched/chipdrivers.h	2011-06-27 16:29:25.000000000
+0200
>> @@ -31,6 +31,7 @@
>>  int probe_spi_rems(struct flashchip *flash);  int 
>> probe_spi_res1(struct flashchip *flash);  int probe_spi_res2(struct 
>> flashchip *flash);
>> +int probe_spi_mx_2017(struct flashchip *flash);
>>  int spi_write_enable(void);
>>  int spi_write_disable(void);
>>  int spi_block_erase_20(struct flashchip *flash, unsigned int addr, 
>> unsigned int blocklen); diff -u flashrom/flashchips.c
flashrom-patched/flashchips.c
>> --- flashrom/flashchips.c	2011-06-27 15:21:57.000000000 +0200
>> +++ flashrom-patched/flashchips.c	2011-06-27 16:26:34.000000000
+0200
>> @@ -4131,8 +4131,8 @@
>>  		.total_size	= 8192,
>>  		.page_size	= 256,
>>  		.feature_bits	= FEATURE_WRSR_WREN,
>> -		.tested		= TEST_OK_PROBE,
>> -		.probe		= probe_spi_rdid,
>> +		.tested		= TEST_UNTESTED,
>> +		.probe		= probe_spi_mx_2017,
>>  		.probe_timing	= TIMING_ZERO,
>>  		.block_erasers	=
>>  		{
>> @@ -4141,6 +4141,40 @@
>>  				.block_erase = spi_block_erase_20,
>>  			}, {
>>  				.eraseblocks = { {64 * 1024, 128} },
>> +				.block_erase = spi_block_erase_d8,
>> +			}, {
>> +				.eraseblocks = { {8 * 1024 * 1024, 1} },
>> +				.block_erase = spi_block_erase_60,
>> +			}, {
>> +				.eraseblocks = { {8 * 1024 * 1024, 1} },
>> +				.block_erase = spi_block_erase_c7,
>> +			}
>> +		},
>> +		.unlock		= spi_disable_blockprotect,
>> +		.write		= spi_chip_write_256,
>> +		.read		= spi_chip_read,
>> +		.voltage	= {2700, 3600},
>> +	},
>> +
>> +	{
>> +		.vendor		= "Macronix",
>> +		.name		= "MX25L6445E",
>> +		.bustype	= CHIP_BUSTYPE_SPI,
>> +		.manufacture_id	= MACRONIX_ID,
>> +		.model_id	= MACRONIX_MX25L6445E,
>> +		.total_size	= 8192,
>> +		.page_size	= 256,
>> +		.feature_bits	= FEATURE_WRSR_WREN,
>> +		.tested		= TEST_OK_PREW,
>> +		.probe		= probe_spi_mx_2017,
>> +		.probe_timing	= TIMING_ZERO,
>> +		.block_erasers	=
>> +		{
>> +			{
>> +				.eraseblocks = { {4 * 1024, 2048} },
>> +				.block_erase = spi_block_erase_20,
>> +			}, {
>> +				.eraseblocks = { {64 * 1024, 128} },
>>  				.block_erase = spi_block_erase_d8,
>>  			}, {
>>  				.eraseblocks = { {8 * 1024 * 1024, 1} },
diff -u 
>> flashrom/flashchips.h flashrom-patched/flashchips.h
>> --- flashrom/flashchips.h	2011-06-27 15:21:57.000000000 +0200
>> +++ flashrom-patched/flashchips.h	2011-06-27 15:46:22.000000000
+0200
>> @@ -356,7 +356,8 @@
>>  #define MACRONIX_MX25L8005	0x2014	/* Same as MX25V8005 */
>>  #define MACRONIX_MX25L1605	0x2015	/* MX25L1605{,A,D} */
>>  #define MACRONIX_MX25L3205	0x2016	/* MX25L3205{,A} */
>> -#define MACRONIX_MX25L6405	0x2017	/* MX25L3205{,D} */
>> +#define MACRONIX_MX25L6405	0x2017	/* MX25L6405{,D} */ 
>> +#define MACRONIX_MX25L6445E	0x16	/* MX25L6445E, ID = REMS2 answer
*/
>>  #define MACRONIX_MX25L12805	0x2018	/* MX25L12805 */
>>  #define MACRONIX_MX25L1635D	0x2415
>>  #define MACRONIX_MX25L1635E	0x2515	/* MX25L1635{E} */
>> diff -u flashrom/spi.h flashrom-patched/spi.h
>> --- flashrom/spi.h	2011-06-27 15:21:57.000000000 +0200
>> +++ flashrom-patched/spi.h	2011-06-27 16:29:38.000000000 +0200
>> @@ -40,6 +40,11 @@
>>  #define JEDEC_REMS_OUTSIZE	0x04
>>  #define JEDEC_REMS_INSIZE	0x02
>>  
>> +/* Read Electronic Manufacturer Signature 2 */
>> +#define JEDEC_REMS2		0xEF
>> +#define JEDEC_REMS2_OUTSIZE	0x04
>> +#define JEDEC_REMS2_INSIZE	0x02
>> +
>>  /* Read Electronic Signature */
>>  #define JEDEC_RES		0xab
>>  #define JEDEC_RES_OUTSIZE	0x04
>> diff -u flashrom/spi25.c flashrom-patched/spi25.c
>> --- flashrom/spi25.c	2011-06-27 15:21:57.000000000 +0200
>> +++ flashrom-patched/spi25.c	2011-06-27 16:30:33.000000000 +0200
>> @@ -66,6 +66,27 @@
>>  	return 0;
>>  }
>>  
>> +static int spi_rems2(unsigned char *readarr) {
>> +	unsigned char cmd[JEDEC_REMS2_OUTSIZE] = { JEDEC_REMS2, 0, 0, 0
};
>> +	uint32_t readaddr;
>> +	int ret;
>> +
>> +	ret = spi_send_command(sizeof(cmd), JEDEC_REMS2_INSIZE, cmd,
readarr);
>> +	if (ret == SPI_INVALID_ADDRESS) {
>> +		/* Find the lowest even address allowed for reads. */
>> +		readaddr = (spi_get_valid_read_addr() + 1) & ~1;
>> +		cmd[1] = (readaddr >> 16) & 0xff,
>> +		cmd[2] = (readaddr >> 8) & 0xff,
>> +		cmd[3] = (readaddr >> 0) & 0xff,
>> +		ret = spi_send_command(sizeof(cmd), JEDEC_REMS2_INSIZE,
cmd, readarr);
>> +	}
>> +	if (ret)
>> +		return ret;
>> +	msg_cspew("REMS2 returned %02x %02x. ", readarr[0], readarr[1]);
>> +	return 0;
>> +}
>> +
>>  static int spi_res(unsigned char *readarr, int bytes)  {
>>  	unsigned char cmd[JEDEC_RES_OUTSIZE] = { JEDEC_RES, 0, 0, 0 };
@@ 
>> -173,6 +194,49 @@
>>  	return probe_spi_rdid_generic(flash, 3);  }
>>  
>> +int probe_spi_mx_2017(struct flashchip *flash) {
>> +	unsigned char readarr[4];
>> +	uint32_t id1;
>> +	uint32_t id2;
>> +
>> +	if (spi_rdid(readarr, 3)) {
>> +		msg_cdbg("\n");
>> +		return 0;
>> +	}
>> +	id1 = readarr[0];
>> +	id2 = (readarr[1] << 8) | readarr[2];
>> +
>> +	if (id1 == MACRONIX_ID && id2 == MACRONIX_MX25L6405) {
>> +	/* We have either MACRONIX_MX25L6405 or MACRONIX_MX25L6445E.
>> +	 * MACRONIX_MX25L6405 has 64KB for erase function 20h and does
not support REMS2 function,
>> +	 * MACRONIX_MX25L6445E has 4KB for erase function 20h and does
support REMS2 function.
>> +	 * So to differentiate both, we execute REMS2 function (this is
suggested by macronix).
>> +	 */
>> +		if (spi_rems2(readarr)) {
>> +			msg_cdbg("\n");
>> +			return 0;
>> +		}
>> +		id1 = readarr[0];
>> +		id2 = readarr[1];
>> +		if (id1 == MACRONIX_ID && id2 == MACRONIX_MX25L6445E) {
>> +		/* MACRONIX_MX25L6445E */
>> +			if (flash->model_id == MACRONIX_MX25L6445E)
>> +				return 1;
>> +			else
>> +				return 0;
>> +		} else {
>> +		/* MACRONIX_MX25L6405 */
>> +			if (flash->model_id == MACRONIX_MX25L6405)
>> +				return 1;
>> +			else
>> +				return 0;	
>> +		}
>> +	}
>> +	/* neither MACRONIX_MX25L6405 nor MACRONIX_MX25L6445E */
>> +	return 0;
>> +}
>> +
>>  int probe_spi_rdid4(struct flashchip *flash)  {
>>  	/* Some SPI controllers do not support commands with writecnt=1
and
>>
>>
>>     
> hello again and thanks for all you have done.
>
> without looking at your patch in too much detail i would like to
> mention another possibility to implement this. as i have done in my
> sfdp and intel hardware sequencing patch you can alter the struct
> flashchip *flash the probing function gets as parameter.
> you can then change the erase block size (and name) according to the
id
> instead of using two chips and returning 0/1 respectively.
>   

Please avoid changing struct flashchip *flash in the probe function
unless it is absolutely necessary. The hack needed for SFDP should not
spread elsewhere, especially if we can handle this nicely with two
separate flashchip entries.

Regards,
Carl-Daniel
Wagner, Helge (GE Intelligent Platforms) - 2011-06-29 13:55:50
Hello all,

i have now re-worked my patch. This is the full description of the
complete patch:

-Fixed spi_block_erase_d8() block size for MX25L3205 was wrong.
-Now using spi_block_erase_20_4or64k() erase function instead of
spi_block_erase_20() which can handle both 4KB and 64KB block sizes for
those flash chips that might have same id but different erase block
sizes.
-New probe function added (probe_spi_mx_2017()) to differentiate
MX25L6445E and MX25L6405.

Signed-off-by: Helge Wagner <helge.wagner@ge.com>

I have also included log files for the MX25L6445E.

Regards,
Helge Wagner

diff -u flashrom/chipdrivers.h flashrom-patched/chipdrivers.h
--- flashrom/chipdrivers.h	2011-06-27 15:21:57.000000000 +0200
+++ flashrom-patched/chipdrivers.h	2011-06-28 14:38:54.000000000
+0200
@@ -31,9 +31,11 @@
 int probe_spi_rems(struct flashchip *flash);
 int probe_spi_res1(struct flashchip *flash);
 int probe_spi_res2(struct flashchip *flash);
+int probe_spi_mx_2017(struct flashchip *flash);
 int spi_write_enable(void);
 int spi_write_disable(void);
 int spi_block_erase_20(struct flashchip *flash, unsigned int addr,
unsigned int blocklen);
+int spi_block_erase_20_4or64k(struct flashchip *flash, unsigned int
addr, unsigned int blocklen);
 int spi_block_erase_52(struct flashchip *flash, unsigned int addr,
unsigned int blocklen);
 int spi_block_erase_d7(struct flashchip *flash, unsigned int addr,
unsigned int blocklen);
 int spi_block_erase_d8(struct flashchip *flash, unsigned int addr,
unsigned int blocklen);
diff -u flashrom/flashchips.c flashrom-patched/flashchips.c
--- flashrom/flashchips.c	2011-06-27 15:21:57.000000000 +0200
+++ flashrom-patched/flashchips.c	2011-06-29 13:22:51.000000000
+0200
@@ -3959,14 +3959,14 @@
 		.total_size	= 2048,
 		.page_size	= 256,
 		.feature_bits	= FEATURE_WRSR_WREN,
-		.tested		= TEST_OK_PREW,
+		.tested		= TEST_OK_PRW,
 		.probe		= probe_spi_rdid,
 		.probe_timing	= TIMING_ZERO,
 		.block_erasers	=
 		{
 			{
-				.eraseblocks = { {4 * 1024, 512} },
-				.block_erase = spi_block_erase_20,
/* This erase function has 64k blocksize for eLiteFlash */
+				.eraseblocks = { {64 * 1024, 32} },
+				.block_erase =
spi_block_erase_20_4or64k,	/* This erase function has 64k blocksize
for eLiteFlash */
 			}, {
 				.eraseblocks = { {64 * 1024, 32} },
/* Not supported in MX25L1605 (eLiteFlash) and MX25L1605D */
 				.block_erase = spi_block_erase_52,
@@ -4063,16 +4063,16 @@
 		.total_size	= 4096,
 		.page_size	= 256,
 		.feature_bits	= FEATURE_WRSR_WREN,
-		.tested		= TEST_OK_PREW,
+		.tested		= TEST_OK_PRW,
 		.probe		= probe_spi_rdid,
 		.probe_timing	= TIMING_ZERO,
 		.block_erasers	=
 		{
 			{
-				.eraseblocks = { {4 * 1024, 1024} },
-				.block_erase = spi_block_erase_20,
+				.eraseblocks = { {64 * 1024, 64} },
+				.block_erase =
spi_block_erase_20_4or64k,
 			}, {
-				.eraseblocks = { {4 * 1024, 1024} },
+				.eraseblocks = { {64 * 1024, 64} },
 				.block_erase = spi_block_erase_d8,
 			}, {
 				.eraseblocks = { {4 * 1024 * 1024, 1} },
@@ -4132,12 +4132,46 @@
 		.page_size	= 256,
 		.feature_bits	= FEATURE_WRSR_WREN,
 		.tested		= TEST_OK_PROBE,
-		.probe		= probe_spi_rdid,
+		.probe		= probe_spi_mx_2017,
 		.probe_timing	= TIMING_ZERO,
 		.block_erasers	=
 		{
 			{
 				.eraseblocks = { {64 * 1024, 128} },
+				.block_erase =
spi_block_erase_20_4or64k,
+			}, {
+				.eraseblocks = { {64 * 1024, 128} },
+				.block_erase = spi_block_erase_d8,
+			}, {
+				.eraseblocks = { {8 * 1024 * 1024, 1} },
+				.block_erase = spi_block_erase_60,
+			}, {
+				.eraseblocks = { {8 * 1024 * 1024, 1} },
+				.block_erase = spi_block_erase_c7,
+			}
+		},
+		.unlock		= spi_disable_blockprotect,
+		.write		= spi_chip_write_256,
+		.read		= spi_chip_read,
+		.voltage	= {2700, 3600},
+	},
+
+	{
+		.vendor		= "Macronix",
+		.name		= "MX25L6445E",
+		.bustype	= CHIP_BUSTYPE_SPI,
+		.manufacture_id	= MACRONIX_ID,
+		.model_id	= MACRONIX_MX25L6445E,
+		.total_size	= 8192,
+		.page_size	= 256,
+		.feature_bits	= FEATURE_WRSR_WREN,
+		.tested		= TEST_OK_PREW,
+		.probe		= probe_spi_mx_2017,
+		.probe_timing	= TIMING_ZERO,
+		.block_erasers	=
+		{
+			{
+				.eraseblocks = { {4 * 1024, 2048} },
 				.block_erase = spi_block_erase_20,
 			}, {
 				.eraseblocks = { {64 * 1024, 128} },
diff -u flashrom/flashchips.h flashrom-patched/flashchips.h
--- flashrom/flashchips.h	2011-06-27 15:21:57.000000000 +0200
+++ flashrom-patched/flashchips.h	2011-06-28 16:24:07.000000000
+0200
@@ -356,7 +356,8 @@
 #define MACRONIX_MX25L8005	0x2014	/* Same as MX25V8005 */
 #define MACRONIX_MX25L1605	0x2015	/* MX25L1605{,A,D} */
 #define MACRONIX_MX25L3205	0x2016	/* MX25L3205{,A} */
-#define MACRONIX_MX25L6405	0x2017	/* MX25L3205{,D} */
+#define MACRONIX_MX25L6405	0x2017	/* MX25L6405, MX25L6406E */ 
+#define MACRONIX_MX25L6445E	0x16	/* MX25L6445E, MX25L6405D ID =
REMS2 answer */
 #define MACRONIX_MX25L12805	0x2018	/* MX25L12805 */
 #define MACRONIX_MX25L1635D	0x2415
 #define MACRONIX_MX25L1635E	0x2515	/* MX25L1635{E} */
 static int spi_res(unsigned char *readarr, int bytes)
 {
 	unsigned char cmd[JEDEC_RES_OUTSIZE] = { JEDEC_RES, 0, 0, 0 };
@@ -173,6 +195,58 @@
 	return probe_spi_rdid_generic(flash, 3);
 }
 
+int probe_spi_mx_2017(struct flashchip *flash)
+{
+	unsigned char readarr[4];
+	uint32_t id1;
+	uint32_t id2;
+	uint32_t id3;
+	uint32_t id4;
+
+	if (spi_rdid(readarr, 3)) {
+		msg_cdbg("\n");
+		return 0;
+	}
+	id1 = readarr[0];
+	id2 = (readarr[1] << 8) | readarr[2];
+
+	if (id1 == MACRONIX_ID && id2 == MACRONIX_MX25L6405) {
+	/* Now we have detected the 2017h class of Macronix flash
devices.
+	 * We can further differentiate two classes:
+	 * -those who support the REMS2 function do always have 4KB
sector size
+	 *  (at least i have not found one with 64KB yet),
+	 * and
+	 * -those who don't support the REMS2 function might have 4KB or
64KB
+	 *  (some have 4KB, e.g. MX25L6406E, some have 64KB, e.g.
MX25L6405 (not D))
+	 *  for erase function 20h.
+	 */
+		if (spi_rems2(readarr)) {
+			msg_cdbg("\n");
+			return 0;
+		}
+		id3 = readarr[0];
+		id4 = readarr[1];
+		msg_cdbg("%s: id1 0x%02x, id2 0x%02x, id3 0x%02x, id4
0x%02x\n", __func__, id1, id2, id3, id4);
+		if (id3 == MACRONIX_ID && id4 == MACRONIX_MX25L6445E) {
+		/* MACRONIX_MX25L6445E class, always 4KB sector */
+			if (flash->model_id == MACRONIX_MX25L6445E)
+				return 1;
+			else
+				return 0;
+		} else {
+		/* MACRONIX_MX25L6405 class, might have 4KB or 64KB
sector */
+			if (flash->model_id == MACRONIX_MX25L6405)
+				return 1;
+			else
+				return 0;
+		}
+	} else {
+		msg_cdbg("%s: id1 0x%02x, id2 0x%02x\n", __func__, id1,
id2);
+	}
+	/* neither MACRONIX_MX25L6405 nor MACRONIX_MX25L6445E */
+	return 0;
+}
+
 int probe_spi_rdid4(struct flashchip *flash)
 {
 	/* Some SPI controllers do not support commands with writecnt=1
and
@@ -694,6 +768,79 @@
 	return 0;
 }
 
+/* Sector size might be 4KB or 64KB, but on call to this function this
is unknown.
+ * So we do a first erase command and then check if mem in the complete
64KB sector is erased.
+ * If not we assume 4KB and so we repeat the erase for each 4KB sector
in the 64KB block
+ * (which is not erased already).
+ * PLEASE NOTE: the .eraseblocks block size must be set to 64KB if you
use this function.
+ */
+int spi_block_erase_20_4or64k(struct flashchip *flash, unsigned int
addr, unsigned int blocklen)
+{
+	int result, i, j, fully_erased, sector_erased;
+	unsigned int curraddr, currblocklen;
+	uint8_t *readbuf;
+
+	if (blocklen != (64 * 1024)) {
+		msg_gerr("%s: blocklen not 64KB! Please report a bug at
"
+			 "flashrom@flashrom.org\n", __func__);
+		return 1;
+	}
+
+	readbuf = malloc(blocklen); 
+	if (!readbuf) {
+		msg_gerr("Out of memory!\n");
+		exit(1);
+	} 
+
+        for (i = 0; i < 16; i++) {	/* there are 16 4KB sectors in
64KB */
+
+		curraddr = addr + (i * (4 * 1024));
+
+		if (i > 0) {
+			/* Skip the sector erase if the sector is
already erased */
+			sector_erased = 1;
+			result = flash->read(flash, readbuf, curraddr,
(4 * 1024));
+			if (result) {
+				msg_gerr("Erase impossible because read
failed "
+					 "at 0x%x (len 0x%x)\n",
curraddr, (4 * 1024));
+				break;
+			}
+			for (j = 0; j < currblocklen; j++)
+				if (readbuf[j] != 0xFF)
+					sector_erased = 0;
+			if (sector_erased)
+				continue;	/* Sector is already
erased */
+		}
+
+		result = spi_block_erase_20 (flash, curraddr, (4 *
1024)); /* last parameter (blocklen) is unused */
+		if (result) break;
+
+		if (0 == i) {
+			curraddr += 4 * 1024;
+			currblocklen = (64*1024)-(4*1024);
+			fully_erased = 1;
+			result = flash->read(flash, readbuf, curraddr,
currblocklen);
+			if (result) {
+				msg_gerr("Erase impossible because read
failed "
+					 "at 0x%x (len 0x%x)\n",
curraddr, currblocklen);
+				break;
+			}
+			for (j = 0; j < currblocklen; j++)
+				if (readbuf[j] != 0xFF)
+					fully_erased = 0;
+			if (fully_erased) {
+			/* This means either that the SE command has
erased the full 64KB
+			 * or that the remaining 64KB sector was already
erased before.
+			 * Anyhow, we can now exit without calling the
SE command again.
+			 */
+				break;
+			}
+		}
+        }
+	free(readbuf);
+        return result;
+}
+
 int spi_block_erase_60(struct flashchip *flash, unsigned int addr,
unsigned int blocklen)
 {
 	if ((addr != 0) || (blocklen != flash->total_size * 1024)) {

-----Original Message-----
From: flashrom-bounces@flashrom.org
[mailto:flashrom-bounces@flashrom.org] On Behalf Of Wagner, Helge (GE
Intelligent Platforms)
Sent: Mittwoch, 29. Juni 2011 13:57
To: flashrom@flashrom.org
Subject: Re: [flashrom] [PATCH] Macronix MX25L6445E

Hello all,

as Stefan suggested ("what about 0x2015, 0x2016, 0x2018? do they also
have such problems?"), I was looking at some more Macronix data sheets.
While doing this, I found out that my solution is NOT sufficient for
some of the Macronix 0x2017 flash chips.
I am currently working on a solution for this problem. Please stand by.

Regards,
Helge

-----Original Message-----
From: flashrom-bounces+helge.wagner=ge.com@flashrom.org
[mailto:flashrom-bounces+helge.wagner=ge.com@flashrom.org] On Behalf Of
Carl-Daniel Hailfinger
Sent: Montag, 27. Juni 2011 20:28
To: flashrom@flashrom.org
Subject: Re: [flashrom] [PATCH] Macronix MX25L6445E

Hello Helge,

thanks for your patch. It looks good to me on a first glance, and I hope
to review it in detail soon. A short comment (in reply to Stefan's
review) is at the end of this mail.

Am 27.06.2011 17:50 schrieb Stefan Tauner:
> On Mon, 27 Jun 2011 17:06:42 +0200
> "Wagner, Helge (GE Intelligent Platforms)" <Helge.Wagner@ge.com>
wrote:
>
>   
>> Hello Stefan,
>>
>>     
>>> i have not looked at the datasheet(s), but maybe there is a way to
distinguish the two devices (with another id command for example)?
>>>       
>> I have asked Macronix, and they suggest to use the REMS2 function,
which is supported on the MX25L6445E (returns 0xC216) and is not
supported on the MX25L6405 (according to Macronix, it returns 0x0000 or
0xFFFF for this command).
>>
>> So now there is no longer a need to re-order the erase functions for
the MX25L6405.
>>
>> Here is my new patch.
>>
>> Signed-off-by: Helge Wagner <helge.wagner@ge.com>
>>
>> Regards,
>> Helge Wagner
>>
>> diff -u flashrom/chipdrivers.h flashrom-patched/chipdrivers.h
>> --- flashrom/chipdrivers.h	2011-06-27 15:21:57.000000000 +0200
>> +++ flashrom-patched/chipdrivers.h	2011-06-27 16:29:25.000000000
+0200
>> @@ -31,6 +31,7 @@
>>  int probe_spi_rems(struct flashchip *flash);  int 
>> probe_spi_res1(struct flashchip *flash);  int probe_spi_res2(struct 
>> flashchip *flash);
>> +int probe_spi_mx_2017(struct flashchip *flash);
>>  int spi_write_enable(void);
>>  int spi_write_disable(void);
>>  int spi_block_erase_20(struct flashchip *flash, unsigned int addr, 
>> unsigned int blocklen); diff -u flashrom/flashchips.c
flashrom-patched/flashchips.c
>> --- flashrom/flashchips.c	2011-06-27 15:21:57.000000000 +0200
>> +++ flashrom-patched/flashchips.c	2011-06-27 16:26:34.000000000
+0200
>> @@ -4131,8 +4131,8 @@
>>  		.total_size	= 8192,
>>  		.page_size	= 256,
>>  		.feature_bits	= FEATURE_WRSR_WREN,
>> -		.tested		= TEST_OK_PROBE,
>> -		.probe		= probe_spi_rdid,
>> +		.tested		= TEST_UNTESTED,
>> +		.probe		= probe_spi_mx_2017,
>>  		.probe_timing	= TIMING_ZERO,
>>  		.block_erasers	=
>>  		{
>> @@ -4141,6 +4141,40 @@
>>  				.block_erase = spi_block_erase_20,
>>  			}, {
>>  				.eraseblocks = { {64 * 1024, 128} },
>> +				.block_erase = spi_block_erase_d8,
>> +			}, {
>> +				.eraseblocks = { {8 * 1024 * 1024, 1} },
>> +				.block_erase = spi_block_erase_60,
>> +			}, {
>> +				.eraseblocks = { {8 * 1024 * 1024, 1} },
>> +				.block_erase = spi_block_erase_c7,
>> +			}
>> +		},
>> +		.unlock		= spi_disable_blockprotect,
>> +		.write		= spi_chip_write_256,
>> +		.read		= spi_chip_read,
>> +		.voltage	= {2700, 3600},
>> +	},
>> +
>> +	{
>> +		.vendor		= "Macronix",
>> +		.name		= "MX25L6445E",
>> +		.bustype	= CHIP_BUSTYPE_SPI,
>> +		.manufacture_id	= MACRONIX_ID,
>> +		.model_id	= MACRONIX_MX25L6445E,
>> +		.total_size	= 8192,
>> +		.page_size	= 256,
>> +		.feature_bits	= FEATURE_WRSR_WREN,
>> +		.tested		= TEST_OK_PREW,
>> +		.probe		= probe_spi_mx_2017,
>> +		.probe_timing	= TIMING_ZERO,
>> +		.block_erasers	=
>> +		{
>> +			{
>> +				.eraseblocks = { {4 * 1024, 2048} },
>> +				.block_erase = spi_block_erase_20,
>> +			}, {
>> +				.eraseblocks = { {64 * 1024, 128} },
>>  				.block_erase = spi_block_erase_d8,
>>  			}, {
>>  				.eraseblocks = { {8 * 1024 * 1024, 1} },
diff -u 
>> flashrom/flashchips.h flashrom-patched/flashchips.h
>> --- flashrom/flashchips.h	2011-06-27 15:21:57.000000000 +0200
>> +++ flashrom-patched/flashchips.h	2011-06-27 15:46:22.000000000
+0200
>> @@ -356,7 +356,8 @@
>>  #define MACRONIX_MX25L8005	0x2014	/* Same as MX25V8005 */
>>  #define MACRONIX_MX25L1605	0x2015	/* MX25L1605{,A,D} */
>>  #define MACRONIX_MX25L3205	0x2016	/* MX25L3205{,A} */
>> -#define MACRONIX_MX25L6405	0x2017	/* MX25L3205{,D} */
>> +#define MACRONIX_MX25L6405	0x2017	/* MX25L6405{,D} */ 
>> +#define MACRONIX_MX25L6445E	0x16	/* MX25L6445E, ID = REMS2 answer
*/
>>  #define MACRONIX_MX25L12805	0x2018	/* MX25L12805 */
>>  #define MACRONIX_MX25L1635D	0x2415
>>  #define MACRONIX_MX25L1635E	0x2515	/* MX25L1635{E} */
>> diff -u flashrom/spi.h flashrom-patched/spi.h
>> --- flashrom/spi.h	2011-06-27 15:21:57.000000000 +0200
>> +++ flashrom-patched/spi.h	2011-06-27 16:29:38.000000000 +0200
>> @@ -40,6 +40,11 @@
>>  #define JEDEC_REMS_OUTSIZE	0x04
>>  #define JEDEC_REMS_INSIZE	0x02
>>  
>> +/* Read Electronic Manufacturer Signature 2 */
>> +#define JEDEC_REMS2		0xEF
>> +#define JEDEC_REMS2_OUTSIZE	0x04
>> +#define JEDEC_REMS2_INSIZE	0x02
>> +
>>  /* Read Electronic Signature */
>>  #define JEDEC_RES		0xab
>>  #define JEDEC_RES_OUTSIZE	0x04
>> diff -u flashrom/spi25.c flashrom-patched/spi25.c
>> --- flashrom/spi25.c	2011-06-27 15:21:57.000000000 +0200
>> +++ flashrom-patched/spi25.c	2011-06-27 16:30:33.000000000 +0200
>> @@ -66,6 +66,27 @@
>>  	return 0;
>>  }
>>  
>> +static int spi_rems2(unsigned char *readarr) {
>> +	unsigned char cmd[JEDEC_REMS2_OUTSIZE] = { JEDEC_REMS2, 0, 0, 0
};
>> +	uint32_t readaddr;
>> +	int ret;
>> +
>> +	ret = spi_send_command(sizeof(cmd), JEDEC_REMS2_INSIZE, cmd,
readarr);
>> +	if (ret == SPI_INVALID_ADDRESS) {
>> +		/* Find the lowest even address allowed for reads. */
>> +		readaddr = (spi_get_valid_read_addr() + 1) & ~1;
>> +		cmd[1] = (readaddr >> 16) & 0xff,
>> +		cmd[2] = (readaddr >> 8) & 0xff,
>> +		cmd[3] = (readaddr >> 0) & 0xff,
>> +		ret = spi_send_command(sizeof(cmd), JEDEC_REMS2_INSIZE,
cmd, readarr);
>> +	}
>> +	if (ret)
>> +		return ret;
>> +	msg_cspew("REMS2 returned %02x %02x. ", readarr[0], readarr[1]);
>> +	return 0;
>> +}
>> +
>>  static int spi_res(unsigned char *readarr, int bytes)  {
>>  	unsigned char cmd[JEDEC_RES_OUTSIZE] = { JEDEC_RES, 0, 0, 0 };
@@ 
>> -173,6 +194,49 @@
>>  	return probe_spi_rdid_generic(flash, 3);  }
>>  
>> +int probe_spi_mx_2017(struct flashchip *flash) {
>> +	unsigned char readarr[4];
>> +	uint32_t id1;
>> +	uint32_t id2;
>> +
>> +	if (spi_rdid(readarr, 3)) {
>> +		msg_cdbg("\n");
>> +		return 0;
>> +	}
>> +	id1 = readarr[0];
>> +	id2 = (readarr[1] << 8) | readarr[2];
>> +
>> +	if (id1 == MACRONIX_ID && id2 == MACRONIX_MX25L6405) {
>> +	/* We have either MACRONIX_MX25L6405 or MACRONIX_MX25L6445E.
>> +	 * MACRONIX_MX25L6405 has 64KB for erase function 20h and does
not support REMS2 function,
>> +	 * MACRONIX_MX25L6445E has 4KB for erase function 20h and does
support REMS2 function.
>> +	 * So to differentiate both, we execute REMS2 function (this is
suggested by macronix).
>> +	 */
>> +		if (spi_rems2(readarr)) {
>> +			msg_cdbg("\n");
>> +			return 0;
>> +		}
>> +		id1 = readarr[0];
>> +		id2 = readarr[1];
>> +		if (id1 == MACRONIX_ID && id2 == MACRONIX_MX25L6445E) {
>> +		/* MACRONIX_MX25L6445E */
>> +			if (flash->model_id == MACRONIX_MX25L6445E)
>> +				return 1;
>> +			else
>> +				return 0;
>> +		} else {
>> +		/* MACRONIX_MX25L6405 */
>> +			if (flash->model_id == MACRONIX_MX25L6405)
>> +				return 1;
>> +			else
>> +				return 0;	
>> +		}
>> +	}
>> +	/* neither MACRONIX_MX25L6405 nor MACRONIX_MX25L6445E */
>> +	return 0;
>> +}
>> +
>>  int probe_spi_rdid4(struct flashchip *flash)  {
>>  	/* Some SPI controllers do not support commands with writecnt=1
and
>>
>>
>>     
> hello again and thanks for all you have done.
>
> without looking at your patch in too much detail i would like to 
> mention another possibility to implement this. as i have done in my 
> sfdp and intel hardware sequencing patch you can alter the struct 
> flashchip *flash the probing function gets as parameter.
> you can then change the erase block size (and name) according to the
id
> instead of using two chips and returning 0/1 respectively.
>   

Please avoid changing struct flashchip *flash in the probe function
unless it is absolutely necessary. The hack needed for SFDP should not
spread elsewhere, especially if we can handle this nicely with two
separate flashchip entries.

Regards,
Carl-Daniel

--
http://www.hailfinger.org/

Patch

diff -u flashrom/chipdrivers.h flashrom-patched/chipdrivers.h
--- flashrom/chipdrivers.h	2011-06-27 15:21:57.000000000 +0200
+++ flashrom-patched/chipdrivers.h	2011-06-27 16:29:25.000000000 +0200
@@ -31,6 +31,7 @@ 
 int probe_spi_rems(struct flashchip *flash);
 int probe_spi_res1(struct flashchip *flash);
 int probe_spi_res2(struct flashchip *flash);
+int probe_spi_mx_2017(struct flashchip *flash);
 int spi_write_enable(void);
 int spi_write_disable(void);
 int spi_block_erase_20(struct flashchip *flash, unsigned int addr, unsigned int blocklen);
diff -u flashrom/flashchips.c flashrom-patched/flashchips.c
--- flashrom/flashchips.c	2011-06-27 15:21:57.000000000 +0200
+++ flashrom-patched/flashchips.c	2011-06-27 16:26:34.000000000 +0200
@@ -4131,8 +4131,8 @@ 
 		.total_size	= 8192,
 		.page_size	= 256,
 		.feature_bits	= FEATURE_WRSR_WREN,
-		.tested		= TEST_OK_PROBE,
-		.probe		= probe_spi_rdid,
+		.tested		= TEST_UNTESTED,
+		.probe		= probe_spi_mx_2017,
 		.probe_timing	= TIMING_ZERO,
 		.block_erasers	=
 		{
@@ -4141,6 +4141,40 @@ 
 				.block_erase = spi_block_erase_20,
 			}, {
 				.eraseblocks = { {64 * 1024, 128} },
+				.block_erase = spi_block_erase_d8,
+			}, {
+				.eraseblocks = { {8 * 1024 * 1024, 1} },
+				.block_erase = spi_block_erase_60,
+			}, {
+				.eraseblocks = { {8 * 1024 * 1024, 1} },
+				.block_erase = spi_block_erase_c7,
+			}
+		},
+		.unlock		= spi_disable_blockprotect,
+		.write		= spi_chip_write_256,
+		.read		= spi_chip_read,
+		.voltage	= {2700, 3600},
+	},
+
+	{
+		.vendor		= "Macronix",
+		.name		= "MX25L6445E",
+		.bustype	= CHIP_BUSTYPE_SPI,
+		.manufacture_id	= MACRONIX_ID,
+		.model_id	= MACRONIX_MX25L6445E,
+		.total_size	= 8192,
+		.page_size	= 256,
+		.feature_bits	= FEATURE_WRSR_WREN,
+		.tested		= TEST_OK_PREW,
+		.probe		= probe_spi_mx_2017,
+		.probe_timing	= TIMING_ZERO,
+		.block_erasers	=
+		{
+			{
+				.eraseblocks = { {4 * 1024, 2048} },
+				.block_erase = spi_block_erase_20,
+			}, {
+				.eraseblocks = { {64 * 1024, 128} },
 				.block_erase = spi_block_erase_d8,
 			}, {
 				.eraseblocks = { {8 * 1024 * 1024, 1} },
diff -u flashrom/flashchips.h flashrom-patched/flashchips.h
--- flashrom/flashchips.h	2011-06-27 15:21:57.000000000 +0200
+++ flashrom-patched/flashchips.h	2011-06-27 15:46:22.000000000 +0200
@@ -356,7 +356,8 @@ 
 #define MACRONIX_MX25L8005	0x2014	/* Same as MX25V8005 */
 #define MACRONIX_MX25L1605	0x2015	/* MX25L1605{,A,D} */
 #define MACRONIX_MX25L3205	0x2016	/* MX25L3205{,A} */
-#define MACRONIX_MX25L6405	0x2017	/* MX25L3205{,D} */
+#define MACRONIX_MX25L6405	0x2017	/* MX25L6405{,D} */ 
+#define MACRONIX_MX25L6445E	0x16	/* MX25L6445E, ID = REMS2 answer */
 #define MACRONIX_MX25L12805	0x2018	/* MX25L12805 */
 #define MACRONIX_MX25L1635D	0x2415
 #define MACRONIX_MX25L1635E	0x2515	/* MX25L1635{E} */
diff -u flashrom/spi.h flashrom-patched/spi.h
--- flashrom/spi.h	2011-06-27 15:21:57.000000000 +0200
+++ flashrom-patched/spi.h	2011-06-27 16:29:38.000000000 +0200
@@ -40,6 +40,11 @@ 
 #define JEDEC_REMS_OUTSIZE	0x04
 #define JEDEC_REMS_INSIZE	0x02
 
+/* Read Electronic Manufacturer Signature 2 */
+#define JEDEC_REMS2		0xEF
+#define JEDEC_REMS2_OUTSIZE	0x04
+#define JEDEC_REMS2_INSIZE	0x02
+
 /* Read Electronic Signature */
 #define JEDEC_RES		0xab
 #define JEDEC_RES_OUTSIZE	0x04
diff -u flashrom/spi25.c flashrom-patched/spi25.c
--- flashrom/spi25.c	2011-06-27 15:21:57.000000000 +0200
+++ flashrom-patched/spi25.c	2011-06-27 16:30:33.000000000 +0200
@@ -66,6 +66,27 @@ 
 	return 0;
 }
 
+static int spi_rems2(unsigned char *readarr)
+{
+	unsigned char cmd[JEDEC_REMS2_OUTSIZE] = { JEDEC_REMS2, 0, 0, 0 };
+	uint32_t readaddr;
+	int ret;
+
+	ret = spi_send_command(sizeof(cmd), JEDEC_REMS2_INSIZE, cmd, readarr);
+	if (ret == SPI_INVALID_ADDRESS) {
+		/* Find the lowest even address allowed for reads. */
+		readaddr = (spi_get_valid_read_addr() + 1) & ~1;
+		cmd[1] = (readaddr >> 16) & 0xff,
+		cmd[2] = (readaddr >> 8) & 0xff,
+		cmd[3] = (readaddr >> 0) & 0xff,
+		ret = spi_send_command(sizeof(cmd), JEDEC_REMS2_INSIZE, cmd, readarr);
+	}
+	if (ret)
+		return ret;
+	msg_cspew("REMS2 returned %02x %02x. ", readarr[0], readarr[1]);
+	return 0;
+}
+
 static int spi_res(unsigned char *readarr, int bytes)
 {
 	unsigned char cmd[JEDEC_RES_OUTSIZE] = { JEDEC_RES, 0, 0, 0 };
@@ -173,6 +194,49 @@ 
 	return probe_spi_rdid_generic(flash, 3);
 }
 
+int probe_spi_mx_2017(struct flashchip *flash)
+{
+	unsigned char readarr[4];
+	uint32_t id1;
+	uint32_t id2;
+
+	if (spi_rdid(readarr, 3)) {
+		msg_cdbg("\n");
+		return 0;
+	}
+	id1 = readarr[0];
+	id2 = (readarr[1] << 8) | readarr[2];
+
+	if (id1 == MACRONIX_ID && id2 == MACRONIX_MX25L6405) {
+	/* We have either MACRONIX_MX25L6405 or MACRONIX_MX25L6445E.
+	 * MACRONIX_MX25L6405 has 64KB for erase function 20h and does not support REMS2 function,
+	 * MACRONIX_MX25L6445E has 4KB for erase function 20h and does support REMS2 function.
+	 * So to differentiate both, we execute REMS2 function (this is suggested by macronix).
+	 */
+		if (spi_rems2(readarr)) {
+			msg_cdbg("\n");
+			return 0;
+		}
+		id1 = readarr[0];
+		id2 = readarr[1];
+		if (id1 == MACRONIX_ID && id2 == MACRONIX_MX25L6445E) {
+		/* MACRONIX_MX25L6445E */
+			if (flash->model_id == MACRONIX_MX25L6445E)
+				return 1;
+			else
+				return 0;
+		} else {
+		/* MACRONIX_MX25L6405 */
+			if (flash->model_id == MACRONIX_MX25L6405)
+				return 1;
+			else
+				return 0;	
+		}
+	}
+	/* neither MACRONIX_MX25L6405 nor MACRONIX_MX25L6445E */
+	return 0;
+}
+
 int probe_spi_rdid4(struct flashchip *flash)
 {
 	/* Some SPI controllers do not support commands with writecnt=1 and