Patchwork 4 byte address mode for Macronix MX25L25735F

login
register
about
Submitter Tim Chick
Date 2016-04-05 07:18:04
Message ID <9E2123AAD013EC4D81A8EA2DAF53BDC46D926EEB@MTKMBS61N1.mediatek.inc>
Download mbox | patch
Permalink /patch/4437/
State New
Headers show

Comments

Tim Chick - 2016-04-05 07:18:04
Hi David,

There was a mistake in the logic, which I have corrected.

I was also asked by someone else on the list if it worked with the MX25L25635F, which is 32Mbytes, but uses 3-byte addressing by default.

So I made the attached changes, which switch the chip to 4-byte mode. It also has some dedicated 4-byte commands, and a BAR register, but it seemed easiest to just use what I had tested for the MX25L25735F. I can’t actually test the MX25L25635F though, as I don’t have one.

Thanks,
Tim


From: David Hendricks [mailto:dhendrix@google.com]

Sent: 04 April 2016 23:21
To: Tim Chick
Cc: flashrom@flashrom.org
Subject: Re: [flashrom] [PATCH] 4 byte address mode for Macronix MX25L25735F

On Thu, Mar 31, 2016 at 8:21 AM, Tim Chick <Tim.Chick@mediatek.com<mailto:Tim.Chick@mediatek.com>> wrote:
Hi List,

Flashrom would not detect this chip. When the definition was added, everything failed as the chip only supports 4 byte address operation.

Interesting - I didn't know such chips existed. The ones I've used have backwards-compatible commands that support 3-byte addresses. FYI - Some other high-capacity chips have 4-byte address enable bit in a config register that will make the usual read/write/erase instructions accept 4 byte addresses. And yet other large chips have alternative instructions that function the same but only accept a 4-byte address.

The attached patch adds 4 byte address support for 4 byte only chips, as determined by the JEDEC flash parameter table, and support for this chip specifically.

I’ve only allowed it to work with the SPI_CONTROLLER_FT2232 controller, as that is the only one I have to test.

I’ve also only ported spi_block_erase_20 – the other block erase functions will fail.

Please let me know what you think!

Good stuff! FWIW, I have a work-in-progress patch on chromium.org<http://chromium.org> (https://chromium-review.googlesource.com/#/c/323359/) for the other types of high-capacity flash chips. I've tested on a Spansion S25FS256 using linux_spi and ft2232. It needs a lot of clean-up, but might be of help. Most of the changes were to convert read/write/erase functions to use allocated buffers whose length depends on whether we're using a 3- or 4-byte address.

I'll borrow some ideas from your patch as well to support the "4-byte address only" chips.

--
David Hendricks (dhendrix)
Systems Software Engineer, Google Inc.
Peter Martini - 2016-04-05 09:06:56
Hi all,

I'd asked about support for the MX25L25635E earlier on this list [
https://www.flashrom.org/pipermail/flashrom/2015-September/013855.html],
and have confirmed with the patches referenced there that I can read and
flash my chip.  I'll try this patch tonight when I get home and report
back; I'd love to see this support main-lined.

Peter

On Tue, Apr 5, 2016 at 3:18 AM, Tim Chick <Tim.Chick@mediatek.com> wrote:

> Hi David,
>
>
>
> There was a mistake in the logic, which I have corrected.
>
>
>
> I was also asked by someone else on the list if it worked with the
> MX25L25635F, which is 32Mbytes, but uses 3-byte addressing by default.
>
>
>
> So I made the attached changes, which switch the chip to 4-byte mode. It
> also has some dedicated 4-byte commands, and a BAR register, but it seemed
> easiest to just use what I had tested for the MX25L25735F. I can’t actually
> test the MX25L25635F though, as I don’t have one.
>
>
>
> Thanks,
> Tim
>
>
>
>
>
> *From:* David Hendricks [mailto:dhendrix@google.com]
> *Sent:* 04 April 2016 23:21
> *To:* Tim Chick
> *Cc:* flashrom@flashrom.org
> *Subject:* Re: [flashrom] [PATCH] 4 byte address mode for Macronix
> MX25L25735F
>
>
>
> On Thu, Mar 31, 2016 at 8:21 AM, Tim Chick <Tim.Chick@mediatek.com> wrote:
>
> Hi List,
>
>
>
> Flashrom would not detect this chip. When the definition was added,
> everything failed as the chip only supports 4 byte address operation.
>
>
>
> Interesting - I didn't know such chips existed. The ones I've used have
> backwards-compatible commands that support 3-byte addresses. FYI - Some
> other high-capacity chips have 4-byte address enable bit in a config
> register that will make the usual read/write/erase instructions accept 4
> byte addresses. And yet other large chips have alternative instructions
> that function the same but only accept a 4-byte address.
>
>
>
> The attached patch adds 4 byte address support for 4 byte only chips, as
> determined by the JEDEC flash parameter table, and support for this chip
> specifically.
>
>
>
> I’ve only allowed it to work with the SPI_CONTROLLER_FT2232 controller, as
> that is the only one I have to test.
>
>
>
> I’ve also only ported spi_block_erase_20 – the other block erase functions
> will fail.
>
>
>
> Please let me know what you think!
>
>
>
> Good stuff! FWIW, I have a work-in-progress patch on chromium.org (
> https://chromium-review.googlesource.com/#/c/323359/) for the other types
> of high-capacity flash chips. I've tested on a Spansion S25FS256 using
> linux_spi and ft2232. It needs a lot of clean-up, but might be of help.
> Most of the changes were to convert read/write/erase functions to use
> allocated buffers whose length depends on whether we're using a 3- or
> 4-byte address.
>
>
>
> I'll borrow some ideas from your patch as well to support the "4-byte
> address only" chips.
>
>
>
> --
>
> David Hendricks (dhendrix)
> Systems Software Engineer, Google Inc.
>
> ************* Email Confidentiality Notice ********************
> The information contained in this e-mail message (including any
> attachments) may be confidential, proprietary, privileged, or otherwise
> exempt from disclosure under applicable laws. It is intended to be
> conveyed only to the designated recipient(s). Any use, dissemination,
> distribution, printing, retaining or copying of this e-mail (including its
> attachments) by unintended recipient(s) is strictly prohibited and may
> be unlawful. If you are not an intended recipient of this e-mail, or believe
> that you have received this e-mail in error, please notify the sender
> immediately (by replying to this e-mail), delete any and all copies of
> this e-mail (including any attachments) from your system, and do not
> disclose the content of this e-mail to any other person. Thank you!
>
>
> _______________________________________________
> flashrom mailing list
> flashrom@flashrom.org
> https://www.flashrom.org/mailman/listinfo/flashrom
>

Patch

Index: chipdrivers.h

===================================================================
--- chipdrivers.h	(revision 1955)

+++ chipdrivers.h	(working copy)

@@ -34,6 +34,7 @@ 

 
 /* spi25.c */
 int probe_spi_rdid(struct flashctx *flash);
+int probe_spi_rdid_jedec_addr_len(struct flashctx *flash);

 int probe_spi_rdid4(struct flashctx *flash);
 int probe_spi_rems(struct flashctx *flash);
 int probe_spi_res1(struct flashctx *flash);
Index: flash.h

===================================================================
--- flash.h	(revision 1955)

+++ flash.h	(working copy)

@@ -208,6 +208,7 @@ 

 		uint16_t max;
 	} voltage;
 	enum write_granularity gran;
+	bool is_4byte_addr;

 };
 
 struct flashctx {
Index: flashchips.c

===================================================================
--- flashchips.c	(revision 1955)

+++ flashchips.c	(working copy)

@@ -7632,6 +7632,33 @@ 

 
 	{
 		.vendor		= "Macronix",
+			.name		= "MX25L25635F/MX25L25735F",

+			.bustype	= BUS_SPI,

+			.manufacture_id	= MACRONIX_ID,

+			.model_id	= MACRONIX_MX25L25635F,

+			.total_size	= 32768,

+			.page_size	= 256,

+			/* OTP: 512B total; enter 0xB1, exit 0xC1 */

+			.feature_bits	= FEATURE_WRSR_WREN | FEATURE_OTP,

+			.tested		= TEST_OK_PREW,

+			.probe		= probe_spi_rdid_jedec_addr_len,

+			.probe_timing	= TIMING_ZERO,

+			.block_erasers	=

+		{

+			{

+				.eraseblocks = { {4 * 1024, 8192} },

+				.block_erase = spi_block_erase_20,

+			},

+		},

+			.printlock	= spi_prettyprint_status_register_bp3_srwd, /* bit6 is quad enable */

+			.unlock		= spi_disable_blockprotect_bp3_srwd,

+			.write		= spi_chip_write_256,

+			.read		= spi_chip_read, /* Fast read (0x0B) supported */

+			.voltage	= {2700, 3600},

+	},

+	

+	{

+		.vendor		= "Macronix",

 		.name		= "MX25L3205(A)",
 		.bustype	= BUS_SPI,
 		.manufacture_id	= MACRONIX_ID,
Index: sfdp.c

===================================================================
--- sfdp.c	(revision 1955)

+++ sfdp.c	(working copy)

@@ -158,6 +158,7 @@ 

 	case 0x2:
 		msg_cdbg("  4-Byte only addressing (not supported by "
 			 "flashrom).\n");
+		chip->is_4byte_addr = 1;

 		return 1;
 	default:
 		msg_cdbg("  Required addressing mode (0x%x) not supported.\n",
Index: spi.c

===================================================================
--- spi.c	(revision 1955)

+++ spi.c	(working copy)

@@ -110,20 +110,23 @@ 

 	 * means 0xffffff, the highest unsigned 24bit number.
 	 */
 	addrbase = spi_get_valid_read_addr(flash);
-	if (addrbase + flash->chip->total_size * 1024 > (1 << 24)) {

-		msg_perr("Flash chip size exceeds the allowed access window. ");

-		msg_perr("Read will probably fail.\n");

-		/* Try to get the best alignment subject to constraints. */

-		addrbase = (1 << 24) - flash->chip->total_size * 1024;

+	if( !(flash->chip->is_4byte_addr) )

+	{

+		if (addrbase + flash->chip->total_size * 1024 > (1 << 24)) {

+			msg_perr("Flash chip size exceeds the allowed access window. ");

+			msg_perr("Read will probably fail.\n");

+			/* Try to get the best alignment subject to constraints. */

+			addrbase = (1 << 24) - flash->chip->total_size * 1024;

+		}

+		/* Check if alignment is native (at least the largest power of two which

+		 * is a factor of the mapped size of the chip).

+		 */

+		if (ffs(flash->chip->total_size * 1024) > (ffs(addrbase) ? : 33)) {

+			msg_perr("Flash chip is not aligned natively in the allowed "

+					 "access window.\n");

+			msg_perr("Read will probably return garbage.\n");

+		}

 	}
-	/* Check if alignment is native (at least the largest power of two which

-	 * is a factor of the mapped size of the chip).

-	 */

-	if (ffs(flash->chip->total_size * 1024) > (ffs(addrbase) ? : 33)) {

-		msg_perr("Flash chip is not aligned natively in the allowed "

-			 "access window.\n");

-		msg_perr("Read will probably return garbage.\n");

-	}

 	return flash->mst->spi.read(flash, buf, addrbase + start, len);
 }
 
Index: spi25.c

===================================================================
--- spi25.c	(revision 1955)

+++ spi25.c	(working copy)

@@ -29,6 +29,13 @@ 

 #include "programmer.h"
 #include "spi.h"
 
+#define FOUR_BYTE_ADDR_NOT_SUPPORTED \

+if( flash->chip->is_4byte_addr ) { \

+	msg_cerr("%s 4 byte address not supported\n", __func__); \

+	return(SPI_INVALID_LENGTH); \

+}

+	

+

 static int spi_rdid(struct flashctx *flash, unsigned char *readarr, int bytes)
 {
 	static const unsigned char cmd[JEDEC_RDID_OUTSIZE] = { JEDEC_RDID };
@@ -167,6 +174,69 @@ 

 	return probe_spi_rdid_generic(flash, 3);
 }
 
+int probe_spi_rdid_jedec_addr_len(struct flashctx *flash)

+{

+	int rc;

+	

+	rc = probe_spi_rdid_generic(flash, 3);

+	

+	if(!rc)

+	{

+		return(rc);

+	}

+	

+	rc = probe_spi_sfdp(flash);

+	

+	if( (!rc) && flash->chip->is_4byte_addr )

+	{

+		/* Probe failed due to 4-byte only,

+		 * that's OK

+		 */

+		rc = 1;

+		

+		switch(flash->mst->spi.type)

+		{

+			case SPI_CONTROLLER_FT2232:

+				break;

+			default:

+				msg_cerr("%s 4 byte address only chip, but controller does not support 4 byte mode\n", __func__);

+				return(0);

+		}

+		msg_cinfo("%s: Using 4 byte address mode\n", __func__ );

+	}

+	else

+	{

+		/* If the chip is bigger then 16MBytes, and using SPI_CONTROLLER_FT2232

+		 * put into 4-byte mode

+		 */

+		if( flash->chip->total_size > ( 16 * 1024 ) )

+		{

+			static const unsigned char cmd[] = { 0xB7 };

+			int result;

+

+			switch(flash->mst->spi.type)

+			{

+				case SPI_CONTROLLER_FT2232:

+					break;

+				default:

+					msg_cerr("%s 4 byte address mode needed chip, but controller does not support 4 byte mode\n", __func__);

+					return(0);

+			}

+			

+			result = spi_send_command(flash, sizeof(cmd), 0, cmd, NULL);

+

+			if (result)

+			{

+				msg_cerr("%s Send EN4B failed\n", __func__);

+				return(0);

+			}

+			msg_cinfo("%s: Switched to 4 byte address mode\n", __func__ );

+		}

+	}

+	

+	return(rc);

+}

+

 int probe_spi_rdid4(struct flashctx *flash)
 {
 	/* Some SPI controllers do not support commands with writecnt=1 and
@@ -458,6 +528,8 @@ 

 		.readarr	= NULL,
 	}};
 
+	FOUR_BYTE_ADDR_NOT_SUPPORTED;

+	

 	result = spi_send_multicommand(flash, cmds);
 	if (result) {
 		msg_cerr("%s failed during command execution at address 0x%x\n",
@@ -502,6 +574,8 @@ 

 		.readarr	= NULL,
 	}};
 
+	FOUR_BYTE_ADDR_NOT_SUPPORTED;

+

 	result = spi_send_multicommand(flash, cmds);
 	if (result) {
 		msg_cerr("%s failed during command execution at address 0x%x\n", __func__, addr);
@@ -548,6 +622,8 @@ 

 		.readarr	= NULL,
 	}};
 
+	FOUR_BYTE_ADDR_NOT_SUPPORTED;

+

 	result = spi_send_multicommand(flash, cmds);
 	if (result) {
 		msg_cerr("%s failed during command execution at address 0x%x\n",
@@ -593,6 +669,8 @@ 

 		.readarr	= NULL,
 	}};
 
+	FOUR_BYTE_ADDR_NOT_SUPPORTED;

+

 	result = spi_send_multicommand(flash, cmds);
 	if (result) {
 		msg_cerr("%s failed during command execution at address 0x%x\n",
@@ -635,6 +713,8 @@ 

 		.readarr	= NULL,
 	} };
 
+	FOUR_BYTE_ADDR_NOT_SUPPORTED;

+

 	result = spi_send_multicommand(flash, cmds);
 	if (result) {
 		msg_cerr("%s failed during command execution at address 0x%x\n", __func__, addr);
@@ -654,6 +734,9 @@ 

 		       unsigned int blocklen)
 {
 	int result;
+	int i = 1;

+	unsigned char cmd[JEDEC_SE_OUTSIZE+1] = {

+		JEDEC_SE };

 	struct spi_command cmds[] = {
 	{
 		.writecnt	= JEDEC_WREN_OUTSIZE,
@@ -662,12 +745,7 @@ 

 		.readarr	= NULL,
 	}, {
 		.writecnt	= JEDEC_SE_OUTSIZE,
-		.writearr	= (const unsigned char[]){

-					JEDEC_SE,

-					(addr >> 16) & 0xff,

-					(addr >> 8) & 0xff,

-					(addr & 0xff)

-				},

+		.writearr	= cmd,

 		.readcnt	= 0,
 		.readarr	= NULL,
 	}, {
@@ -677,6 +755,15 @@ 

 		.readarr	= NULL,
 	}};
 
+	if( flash->chip->is_4byte_addr )

+	{

+		cmds[1].writecnt++;

+		cmd[i++] = (addr >> 24) & 0xff;

+	}

+	cmd[i++] = (addr >> 16) & 0xff;

+	cmd[i++] = (addr >>  8) & 0xff;

+	cmd[i++] = (addr >>  0) & 0xff;	

+	

 	result = spi_send_multicommand(flash, cmds);
 	if (result) {
 		msg_cerr("%s failed during command execution at address 0x%x\n",
@@ -718,6 +805,8 @@ 

 		.readarr	= NULL,
 	}};
 
+	FOUR_BYTE_ADDR_NOT_SUPPORTED;

+

 	result = spi_send_multicommand(flash, cmds);
 	if (result) {
 		msg_cerr("%s failed during command execution at address 0x%x\n", __func__, addr);
@@ -758,6 +847,8 @@ 

 		.readarr	= NULL,
 	}};
 
+	FOUR_BYTE_ADDR_NOT_SUPPORTED;

+

 	result = spi_send_multicommand(flash, cmds);
 	if (result) {
 		msg_cerr("%s failed during command execution at address 0x%x\n", __func__, addr);
@@ -868,6 +959,8 @@ 

 		.readarr	= NULL,
 	}};
 
+	FOUR_BYTE_ADDR_NOT_SUPPORTED;

+

 	result = spi_send_multicommand(flash, cmds);
 	if (result) {
 		msg_cerr("%s failed during command execution at address 0x%x\n",
@@ -879,13 +972,10 @@ 

 int spi_nbyte_program(struct flashctx *flash, unsigned int addr, const uint8_t *bytes, unsigned int len)
 {
 	int result;
+	int i = 1;

 	/* FIXME: Switch to malloc based on len unless that kills speed. */
 	unsigned char cmd[JEDEC_BYTE_PROGRAM_OUTSIZE - 1 + 256] = {
-		JEDEC_BYTE_PROGRAM,

-		(addr >> 16) & 0xff,

-		(addr >> 8) & 0xff,

-		(addr >> 0) & 0xff,

-	};

+		JEDEC_BYTE_PROGRAM };

 	struct spi_command cmds[] = {
 	{
 		.writecnt	= JEDEC_WREN_OUTSIZE,
@@ -904,6 +994,15 @@ 

 		.readarr	= NULL,
 	}};
 
+	if( flash->chip->is_4byte_addr )

+	{

+		cmds[1].writecnt++;

+		cmd[i++] = (addr >> 24) & 0xff;

+	}

+	cmd[i++] = (addr >> 16) & 0xff;

+	cmd[i++] = (addr >>  8) & 0xff;

+	cmd[i++] = (addr >>  0) & 0xff;	

+	

 	if (!len) {
 		msg_cerr("%s called for zero-length write\n", __func__);
 		return 1;
@@ -913,7 +1012,7 @@ 

 		return 1;
 	}
 
-	memcpy(&cmd[4], bytes, len);

+	memcpy(&cmd[i], bytes, len);

 
 	result = spi_send_multicommand(flash, cmds);
 	if (result) {
@@ -926,15 +1025,23 @@ 

 int spi_nbyte_read(struct flashctx *flash, unsigned int address, uint8_t *bytes,
 		   unsigned int len)
 {
-	const unsigned char cmd[JEDEC_READ_OUTSIZE] = {

-		JEDEC_READ,

-		(address >> 16) & 0xff,

-		(address >> 8) & 0xff,

-		(address >> 0) & 0xff,

-	};

+	int i = 1;

+	unsigned int cmd_len = JEDEC_READ_OUTSIZE;

+	unsigned char cmd[JEDEC_READ_OUTSIZE+1];

+	

+	cmd[0] = JEDEC_READ;

+	

+	if( flash->chip->is_4byte_addr )

+	{

+		cmd_len++;

+		cmd[i++] = (address >> 24) & 0xff;

+	}

+	cmd[i++] = (address >> 16) & 0xff;

+	cmd[i++] = (address >>  8) & 0xff;

+	cmd[i++] = (address >>  0) & 0xff;

 
 	/* Send Read */
-	return spi_send_command(flash, sizeof(cmd), len, cmd, bytes);

+	return spi_send_command(flash, cmd_len, len, cmd, bytes);

 }
 
 /*