Re: [flashrom] [PATCH] Macronix MX25L6445E

[Wagner, Helge (GE Intelligent Platforms)]

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=[e-mail]
[mailto:flashrom-bounces+helge.wagner=[e-mail]] On Behalf Of
Carl-Daniel Hailfinger
Sent: Montag, 27. Juni 2011 20:28
To: [e-mail]
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)"

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
> >
> > 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