All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Add support for flag status register on Micron chips
@ 2014-04-08 16:12 ` grmoore
  0 siblings, 0 replies; 20+ messages in thread
From: grmoore @ 2014-04-08 16:12 UTC (permalink / raw)
  To: ggrahammoore
  Cc: David Woodhouse, Brian Norris, Marek Vasut, Artem Bityutskiy,
	Sourav Poddar, Sascha Hauer, Geert Uytterhoeven, Jingoo Han,
	Insop Song, Graham Moore, linux-mtd, linux-kernel, Alan Tull,
	Dinh Nguyen, Yves Vandervennet

From: Graham Moore <grmoore@altera.com>

This is a slightly different version of the patch that Insop Song
submitted (http://marc.info/?i=201403012022.10111.marex%20()%20denx%20!%20de).

I talked to Insop, and he agreed I should submit this patch as a follow-on to his.

This patch uses a flag in the m25p_ids[] array to determine which chips need
to use the FSR (Flag Status Register).

Rationale for using the FSR:

The Micron data sheets say we have to do this, at least for the multi-die 512M 
and 1G parts (n25q512 and n25q00).  In practice, if we don't check the FSR 
for program/erase status, and we rely solely on the status register (SR), 
then we get corrupted data in the flash.

Micron told us (Altera) that for multi-die chips based on the 65nm 256MB
die, we need to check the SR first, then check the FSR, which is why the 
wait_for_fsr_ready function does that.  Future chips based on 45 nm 512MB die 
will use the FSR only.              

Thanks.

Graham Moore (1):
  Add support for flag status register on Micron chips.

 drivers/mtd/devices/m25p80.c |   94 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 80 insertions(+), 14 deletions(-)

-- 
1.7.10.4


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH] Add support for flag status register on Micron chips
@ 2014-04-08 16:12 ` grmoore
  0 siblings, 0 replies; 20+ messages in thread
From: grmoore @ 2014-04-08 16:12 UTC (permalink / raw)
  To: ggrahammoore
  Cc: Marek Vasut, Geert Uytterhoeven, Graham Moore, Artem Bityutskiy,
	Sascha Hauer, Jingoo Han, linux-kernel, Yves Vandervennet,
	linux-mtd, Insop Song, Alan Tull, Sourav Poddar, Brian Norris,
	David Woodhouse, Dinh Nguyen

From: Graham Moore <grmoore@altera.com>

This is a slightly different version of the patch that Insop Song
submitted (http://marc.info/?i=201403012022.10111.marex%20()%20denx%20!%20de).

I talked to Insop, and he agreed I should submit this patch as a follow-on to his.

This patch uses a flag in the m25p_ids[] array to determine which chips need
to use the FSR (Flag Status Register).

Rationale for using the FSR:

The Micron data sheets say we have to do this, at least for the multi-die 512M 
and 1G parts (n25q512 and n25q00).  In practice, if we don't check the FSR 
for program/erase status, and we rely solely on the status register (SR), 
then we get corrupted data in the flash.

Micron told us (Altera) that for multi-die chips based on the 65nm 256MB
die, we need to check the SR first, then check the FSR, which is why the 
wait_for_fsr_ready function does that.  Future chips based on 45 nm 512MB die 
will use the FSR only.              

Thanks.

Graham Moore (1):
  Add support for flag status register on Micron chips.

 drivers/mtd/devices/m25p80.c |   94 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 80 insertions(+), 14 deletions(-)

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH] Add support for flag status register on Micron chips.
  2014-04-08 16:12 ` grmoore
@ 2014-04-08 16:12   ` grmoore
  -1 siblings, 0 replies; 20+ messages in thread
From: grmoore @ 2014-04-08 16:12 UTC (permalink / raw)
  To: ggrahammoore
  Cc: David Woodhouse, Brian Norris, Marek Vasut, Artem Bityutskiy,
	Sourav Poddar, Sascha Hauer, Geert Uytterhoeven, Jingoo Han,
	Insop Song, Graham Moore, linux-mtd, linux-kernel, Alan Tull,
	Dinh Nguyen, Yves Vandervennet

From: Graham Moore <grmoore@altera.com>

Some new Micron flash chips require reading the flag
status register to determine when operations have completed.

Furthermore, chips with multi-die stacks of the 65nm 256Mb QSPI also
require reading the status register before reading the flag status register.

This patch adds support for the flag status register in the n25q512a1 and n25q00
Micron QSPI flash chips.

Signed-off-by: Graham Moore <grmoore@altera.com>
---
 drivers/mtd/devices/m25p80.c |   94 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 80 insertions(+), 14 deletions(-)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index ad19139..38306aa 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -39,6 +39,7 @@
 #define	OPCODE_WREN		0x06	/* Write enable */
 #define	OPCODE_RDSR		0x05	/* Read status register */
 #define	OPCODE_WRSR		0x01	/* Write status register 1 byte */
+#define	OPCODE_RDFSR		0x70  /* read flag status register */
 #define	OPCODE_NORM_READ	0x03	/* Read data bytes (low frequency) */
 #define	OPCODE_FAST_READ	0x0b	/* Read data bytes (high frequency) */
 #define	OPCODE_QUAD_READ        0x6b    /* Read data bytes */
@@ -81,6 +82,9 @@
 
 #define SR_QUAD_EN_MX           0x40    /* Macronix Quad I/O */
 
+/* Flag Status Register bits */
+#define FSR_READY               0x80    /* FSR ready */
+
 /* Configuration Register bits. */
 #define CR_QUAD_EN_SPAN		0x2     /* Spansion Quad I/O */
 
@@ -108,6 +112,7 @@ struct m25p {
 	u8			read_opcode;
 	u8			program_opcode;
 	u8			*command;
+	int (*wait_till_ready)(struct m25p *flash);
 	enum read_type		flash_read;
 };
 
@@ -145,6 +150,27 @@ static int read_sr(struct m25p *flash)
 }
 
 /*
+ * Read the flag status register, returning its value in the location
+ * Return the status register value.
+ * Returns negative if error occurred.
+ */
+static int read_fsr(struct m25p *flash)
+{
+	ssize_t retval;
+	u8 code = OPCODE_RDFSR;
+	u8 val;
+
+	retval = spi_write_then_read(flash->spi, &code, 1, &val, 1);
+
+	if (retval < 0) {
+		dev_err(&flash->spi->dev, "error %d reading FSR\n",
+				(int) retval);
+		return retval;
+	}
+
+	return val;
+}
+/*
  * Read configuration register, returning its value in the
  * location. Return the configuration register value.
  * Returns negative if error occured.
@@ -233,7 +259,7 @@ static inline int set_4byte(struct m25p *flash, u32 jedec_id, int enable)
  * Service routine to read status register until ready, or timeout occurs.
  * Returns non-zero if error.
  */
-static int wait_till_ready(struct m25p *flash)
+static int _wait_till_ready(struct m25p *flash)
 {
 	unsigned long deadline;
 	int sr;
@@ -254,6 +280,37 @@ static int wait_till_ready(struct m25p *flash)
 }
 
 /*
+ * Service routine to read flag status register until ready, or timeout occurs.
+ * Returns non-zero if error.
+ */
+static int _wait_till_fsr_ready(struct m25p *flash)
+{
+	unsigned long deadline;
+	int fsr;
+	int sr;
+
+	deadline = jiffies + MAX_READY_WAIT_JIFFIES;
+
+	do {
+		sr = read_sr(flash);
+		if (sr < 0)
+			break;
+		/* only check fsr if sr not busy */
+		if (!(sr & SR_WIP)) {
+			fsr = read_fsr(flash);
+			if (fsr < 0)
+				break;
+			if (fsr & FSR_READY)
+				return 0;
+		}
+
+		cond_resched();
+
+	} while (!time_after_eq(jiffies, deadline));
+
+	return 1;
+}
+/*
  * Write status Register and configuration register with 2 bytes
  * The first byte will be written to the status register, while the
  * second byte will be written to the configuration register.
@@ -280,7 +337,7 @@ static int macronix_quad_enable(struct m25p *flash)
 
 	spi_write(flash->spi, &cmd, 2);
 
-	if (wait_till_ready(flash))
+	if (flash->wait_till_ready(flash))
 		return 1;
 
 	ret = read_sr(flash);
@@ -351,7 +408,7 @@ static int erase_chip(struct m25p *flash)
 			(long long)(flash->mtd.size >> 10));
 
 	/* Wait until finished previous write command. */
-	if (wait_till_ready(flash))
+	if (flash->wait_till_ready(flash))
 		return 1;
 
 	/* Send write enable, then erase commands. */
@@ -391,7 +448,7 @@ static int erase_sector(struct m25p *flash, u32 offset)
 			__func__, flash->mtd.erasesize / 1024, offset);
 
 	/* Wait until finished previous write command. */
-	if (wait_till_ready(flash))
+	if (flash->wait_till_ready(flash))
 		return 1;
 
 	/* Send write enable, then erase commands. */
@@ -536,7 +593,7 @@ static int m25p80_read(struct mtd_info *mtd, loff_t from, size_t len,
 	mutex_lock(&flash->lock);
 
 	/* Wait till previous write/erase is done. */
-	if (wait_till_ready(flash)) {
+	if (flash->wait_till_ready(flash)) {
 		/* REVISIT status return?? */
 		mutex_unlock(&flash->lock);
 		return 1;
@@ -585,7 +642,7 @@ static int m25p80_write(struct mtd_info *mtd, loff_t to, size_t len,
 	mutex_lock(&flash->lock);
 
 	/* Wait until finished previous write command. */
-	if (wait_till_ready(flash)) {
+	if (flash->wait_till_ready(flash)) {
 		mutex_unlock(&flash->lock);
 		return 1;
 	}
@@ -628,7 +685,7 @@ static int m25p80_write(struct mtd_info *mtd, loff_t to, size_t len,
 			t[1].tx_buf = buf + i;
 			t[1].len = page_size;
 
-			wait_till_ready(flash);
+			flash->wait_till_ready(flash);
 
 			write_enable(flash);
 
@@ -668,7 +725,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
 	mutex_lock(&flash->lock);
 
 	/* Wait until finished previous write command. */
-	ret = wait_till_ready(flash);
+	ret = flash->wait_till_ready(flash);
 	if (ret)
 		goto time_out;
 
@@ -683,7 +740,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
 		/* write one byte. */
 		t[1].len = 1;
 		spi_sync(flash->spi, &m);
-		ret = wait_till_ready(flash);
+		ret = flash->wait_till_ready(flash);
 		if (ret)
 			goto time_out;
 		*retlen += m.actual_length - m25p_cmdsz(flash);
@@ -702,7 +759,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
 		t[1].tx_buf = buf + actual;
 
 		spi_sync(flash->spi, &m);
-		ret = wait_till_ready(flash);
+		ret = flash->wait_till_ready(flash);
 		if (ret)
 			goto time_out;
 		*retlen += m.actual_length - cmd_sz;
@@ -710,7 +767,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
 		to += 2;
 	}
 	write_disable(flash);
-	ret = wait_till_ready(flash);
+	ret = flash->wait_till_ready(flash);
 	if (ret)
 		goto time_out;
 
@@ -724,7 +781,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
 		t[1].tx_buf = buf + actual;
 
 		spi_sync(flash->spi, &m);
-		ret = wait_till_ready(flash);
+		ret = flash->wait_till_ready(flash);
 		if (ret)
 			goto time_out;
 		*retlen += m.actual_length - m25p_cmdsz(flash);
@@ -745,7 +802,7 @@ static int m25p80_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
 
 	mutex_lock(&flash->lock);
 	/* Wait until finished previous command */
-	if (wait_till_ready(flash)) {
+	if (flash->wait_till_ready(flash)) {
 		res = 1;
 		goto err;
 	}
@@ -790,7 +847,7 @@ static int m25p80_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
 
 	mutex_lock(&flash->lock);
 	/* Wait until finished previous command */
-	if (wait_till_ready(flash)) {
+	if (flash->wait_till_ready(flash)) {
 		res = 1;
 		goto err;
 	}
@@ -856,6 +913,7 @@ struct flash_info {
 #define	M25P_NO_FR	0x08		/* Can't do fastread */
 #define	SECT_4K_PMC	0x10		/* OPCODE_BE_4K_PMC works uniformly */
 #define	M25P80_QUAD_READ	0x20    /* Flash supports Quad Read */
+#define	USE_FSR		0x40		/* use flag status register */
 };
 
 #define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)	\
@@ -941,6 +999,8 @@ static const struct spi_device_id m25p_ids[] = {
 	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, 0) },
 	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K) },
 	{ "n25q512a",    INFO(0x20bb20, 0, 64 * 1024, 1024, SECT_4K) },
+	{ "n25q512a1",   INFO(0x20ba20, 0, 64 * 1024, 1024, USE_FSR) },
+	{ "n25q00",      INFO(0x20ba21, 0, 64 * 1024, 2048, USE_FSR) },
 
 	/* PMC */
 	{ "pm25lv512",   INFO(0,        0, 32 * 1024,    2, SECT_4K_PMC) },
@@ -1206,6 +1266,12 @@ static int m25p_probe(struct spi_device *spi)
 	if (info->flags & M25P_NO_ERASE)
 		flash->mtd.flags |= MTD_NO_ERASE;
 
+	if (info->flags & USE_FSR)
+		flash->wait_till_ready = &_wait_till_fsr_ready;
+	else
+		flash->wait_till_ready = &_wait_till_ready;
+
+
 	ppdata.of_node = spi->dev.of_node;
 	flash->mtd.dev.parent = &spi->dev;
 	flash->page_size = info->page_size;
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH] Add support for flag status register on Micron chips.
@ 2014-04-08 16:12   ` grmoore
  0 siblings, 0 replies; 20+ messages in thread
From: grmoore @ 2014-04-08 16:12 UTC (permalink / raw)
  To: ggrahammoore
  Cc: Marek Vasut, Geert Uytterhoeven, Graham Moore, Artem Bityutskiy,
	Sascha Hauer, Jingoo Han, linux-kernel, Yves Vandervennet,
	linux-mtd, Insop Song, Alan Tull, Sourav Poddar, Brian Norris,
	David Woodhouse, Dinh Nguyen

From: Graham Moore <grmoore@altera.com>

Some new Micron flash chips require reading the flag
status register to determine when operations have completed.

Furthermore, chips with multi-die stacks of the 65nm 256Mb QSPI also
require reading the status register before reading the flag status register.

This patch adds support for the flag status register in the n25q512a1 and n25q00
Micron QSPI flash chips.

Signed-off-by: Graham Moore <grmoore@altera.com>
---
 drivers/mtd/devices/m25p80.c |   94 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 80 insertions(+), 14 deletions(-)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index ad19139..38306aa 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -39,6 +39,7 @@
 #define	OPCODE_WREN		0x06	/* Write enable */
 #define	OPCODE_RDSR		0x05	/* Read status register */
 #define	OPCODE_WRSR		0x01	/* Write status register 1 byte */
+#define	OPCODE_RDFSR		0x70  /* read flag status register */
 #define	OPCODE_NORM_READ	0x03	/* Read data bytes (low frequency) */
 #define	OPCODE_FAST_READ	0x0b	/* Read data bytes (high frequency) */
 #define	OPCODE_QUAD_READ        0x6b    /* Read data bytes */
@@ -81,6 +82,9 @@
 
 #define SR_QUAD_EN_MX           0x40    /* Macronix Quad I/O */
 
+/* Flag Status Register bits */
+#define FSR_READY               0x80    /* FSR ready */
+
 /* Configuration Register bits. */
 #define CR_QUAD_EN_SPAN		0x2     /* Spansion Quad I/O */
 
@@ -108,6 +112,7 @@ struct m25p {
 	u8			read_opcode;
 	u8			program_opcode;
 	u8			*command;
+	int (*wait_till_ready)(struct m25p *flash);
 	enum read_type		flash_read;
 };
 
@@ -145,6 +150,27 @@ static int read_sr(struct m25p *flash)
 }
 
 /*
+ * Read the flag status register, returning its value in the location
+ * Return the status register value.
+ * Returns negative if error occurred.
+ */
+static int read_fsr(struct m25p *flash)
+{
+	ssize_t retval;
+	u8 code = OPCODE_RDFSR;
+	u8 val;
+
+	retval = spi_write_then_read(flash->spi, &code, 1, &val, 1);
+
+	if (retval < 0) {
+		dev_err(&flash->spi->dev, "error %d reading FSR\n",
+				(int) retval);
+		return retval;
+	}
+
+	return val;
+}
+/*
  * Read configuration register, returning its value in the
  * location. Return the configuration register value.
  * Returns negative if error occured.
@@ -233,7 +259,7 @@ static inline int set_4byte(struct m25p *flash, u32 jedec_id, int enable)
  * Service routine to read status register until ready, or timeout occurs.
  * Returns non-zero if error.
  */
-static int wait_till_ready(struct m25p *flash)
+static int _wait_till_ready(struct m25p *flash)
 {
 	unsigned long deadline;
 	int sr;
@@ -254,6 +280,37 @@ static int wait_till_ready(struct m25p *flash)
 }
 
 /*
+ * Service routine to read flag status register until ready, or timeout occurs.
+ * Returns non-zero if error.
+ */
+static int _wait_till_fsr_ready(struct m25p *flash)
+{
+	unsigned long deadline;
+	int fsr;
+	int sr;
+
+	deadline = jiffies + MAX_READY_WAIT_JIFFIES;
+
+	do {
+		sr = read_sr(flash);
+		if (sr < 0)
+			break;
+		/* only check fsr if sr not busy */
+		if (!(sr & SR_WIP)) {
+			fsr = read_fsr(flash);
+			if (fsr < 0)
+				break;
+			if (fsr & FSR_READY)
+				return 0;
+		}
+
+		cond_resched();
+
+	} while (!time_after_eq(jiffies, deadline));
+
+	return 1;
+}
+/*
  * Write status Register and configuration register with 2 bytes
  * The first byte will be written to the status register, while the
  * second byte will be written to the configuration register.
@@ -280,7 +337,7 @@ static int macronix_quad_enable(struct m25p *flash)
 
 	spi_write(flash->spi, &cmd, 2);
 
-	if (wait_till_ready(flash))
+	if (flash->wait_till_ready(flash))
 		return 1;
 
 	ret = read_sr(flash);
@@ -351,7 +408,7 @@ static int erase_chip(struct m25p *flash)
 			(long long)(flash->mtd.size >> 10));
 
 	/* Wait until finished previous write command. */
-	if (wait_till_ready(flash))
+	if (flash->wait_till_ready(flash))
 		return 1;
 
 	/* Send write enable, then erase commands. */
@@ -391,7 +448,7 @@ static int erase_sector(struct m25p *flash, u32 offset)
 			__func__, flash->mtd.erasesize / 1024, offset);
 
 	/* Wait until finished previous write command. */
-	if (wait_till_ready(flash))
+	if (flash->wait_till_ready(flash))
 		return 1;
 
 	/* Send write enable, then erase commands. */
@@ -536,7 +593,7 @@ static int m25p80_read(struct mtd_info *mtd, loff_t from, size_t len,
 	mutex_lock(&flash->lock);
 
 	/* Wait till previous write/erase is done. */
-	if (wait_till_ready(flash)) {
+	if (flash->wait_till_ready(flash)) {
 		/* REVISIT status return?? */
 		mutex_unlock(&flash->lock);
 		return 1;
@@ -585,7 +642,7 @@ static int m25p80_write(struct mtd_info *mtd, loff_t to, size_t len,
 	mutex_lock(&flash->lock);
 
 	/* Wait until finished previous write command. */
-	if (wait_till_ready(flash)) {
+	if (flash->wait_till_ready(flash)) {
 		mutex_unlock(&flash->lock);
 		return 1;
 	}
@@ -628,7 +685,7 @@ static int m25p80_write(struct mtd_info *mtd, loff_t to, size_t len,
 			t[1].tx_buf = buf + i;
 			t[1].len = page_size;
 
-			wait_till_ready(flash);
+			flash->wait_till_ready(flash);
 
 			write_enable(flash);
 
@@ -668,7 +725,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
 	mutex_lock(&flash->lock);
 
 	/* Wait until finished previous write command. */
-	ret = wait_till_ready(flash);
+	ret = flash->wait_till_ready(flash);
 	if (ret)
 		goto time_out;
 
@@ -683,7 +740,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
 		/* write one byte. */
 		t[1].len = 1;
 		spi_sync(flash->spi, &m);
-		ret = wait_till_ready(flash);
+		ret = flash->wait_till_ready(flash);
 		if (ret)
 			goto time_out;
 		*retlen += m.actual_length - m25p_cmdsz(flash);
@@ -702,7 +759,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
 		t[1].tx_buf = buf + actual;
 
 		spi_sync(flash->spi, &m);
-		ret = wait_till_ready(flash);
+		ret = flash->wait_till_ready(flash);
 		if (ret)
 			goto time_out;
 		*retlen += m.actual_length - cmd_sz;
@@ -710,7 +767,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
 		to += 2;
 	}
 	write_disable(flash);
-	ret = wait_till_ready(flash);
+	ret = flash->wait_till_ready(flash);
 	if (ret)
 		goto time_out;
 
@@ -724,7 +781,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
 		t[1].tx_buf = buf + actual;
 
 		spi_sync(flash->spi, &m);
-		ret = wait_till_ready(flash);
+		ret = flash->wait_till_ready(flash);
 		if (ret)
 			goto time_out;
 		*retlen += m.actual_length - m25p_cmdsz(flash);
@@ -745,7 +802,7 @@ static int m25p80_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
 
 	mutex_lock(&flash->lock);
 	/* Wait until finished previous command */
-	if (wait_till_ready(flash)) {
+	if (flash->wait_till_ready(flash)) {
 		res = 1;
 		goto err;
 	}
@@ -790,7 +847,7 @@ static int m25p80_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
 
 	mutex_lock(&flash->lock);
 	/* Wait until finished previous command */
-	if (wait_till_ready(flash)) {
+	if (flash->wait_till_ready(flash)) {
 		res = 1;
 		goto err;
 	}
@@ -856,6 +913,7 @@ struct flash_info {
 #define	M25P_NO_FR	0x08		/* Can't do fastread */
 #define	SECT_4K_PMC	0x10		/* OPCODE_BE_4K_PMC works uniformly */
 #define	M25P80_QUAD_READ	0x20    /* Flash supports Quad Read */
+#define	USE_FSR		0x40		/* use flag status register */
 };
 
 #define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)	\
@@ -941,6 +999,8 @@ static const struct spi_device_id m25p_ids[] = {
 	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, 0) },
 	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K) },
 	{ "n25q512a",    INFO(0x20bb20, 0, 64 * 1024, 1024, SECT_4K) },
+	{ "n25q512a1",   INFO(0x20ba20, 0, 64 * 1024, 1024, USE_FSR) },
+	{ "n25q00",      INFO(0x20ba21, 0, 64 * 1024, 2048, USE_FSR) },
 
 	/* PMC */
 	{ "pm25lv512",   INFO(0,        0, 32 * 1024,    2, SECT_4K_PMC) },
@@ -1206,6 +1266,12 @@ static int m25p_probe(struct spi_device *spi)
 	if (info->flags & M25P_NO_ERASE)
 		flash->mtd.flags |= MTD_NO_ERASE;
 
+	if (info->flags & USE_FSR)
+		flash->wait_till_ready = &_wait_till_fsr_ready;
+	else
+		flash->wait_till_ready = &_wait_till_ready;
+
+
 	ppdata.of_node = spi->dev.of_node;
 	flash->mtd.dev.parent = &spi->dev;
 	flash->page_size = info->page_size;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* RE: [PATCH] Add support for flag status register on Micron chips.
  2014-04-08 16:12   ` grmoore
@ 2014-04-08 16:52     ` Insop Song
  -1 siblings, 0 replies; 20+ messages in thread
From: Insop Song @ 2014-04-08 16:52 UTC (permalink / raw)
  To: grmoore, ggrahammoore
  Cc: David Woodhouse, Brian Norris, Marek Vasut, Artem Bityutskiy,
	Sourav Poddar, Sascha Hauer, Geert Uytterhoeven, Jingoo Han,
	linux-mtd, linux-kernel, Alan Tull, Dinh Nguyen,
	Yves Vandervennet


On Tuesday, April 08, 2014 9:13 AM, Graham Moore <grmoore@altera.com>
> 
> From: Graham Moore <grmoore@altera.com>
> 
> Some new Micron flash chips require reading the flag status register to
> determine when operations have completed.
> 
> Furthermore, chips with multi-die stacks of the 65nm 256Mb QSPI also
> require reading the status register before reading the flag status register.
> 
> This patch adds support for the flag status register in the n25q512a1 and
> n25q00 Micron QSPI flash chips.
> 

Reviewed his change, and it is more generic then my previous patch.

Reviewed-by: Insop Song <insop.song@gainspeed.com>

> Signed-off-by: Graham Moore <grmoore@altera.com>
> ---
>  drivers/mtd/devices/m25p80.c |   94
> +++++++++++++++++++++++++++++++++++-------
>  1 file changed, 80 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index ad19139..38306aa 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -39,6 +39,7 @@
>  #define	OPCODE_WREN		0x06	/* Write enable */
>  #define	OPCODE_RDSR		0x05	/* Read status register */
>  #define	OPCODE_WRSR		0x01	/* Write status
> register 1 byte */
> +#define	OPCODE_RDFSR		0x70  /* read flag status
> register */
>  #define	OPCODE_NORM_READ	0x03	/* Read data bytes (low
> frequency) */
>  #define	OPCODE_FAST_READ	0x0b	/* Read data bytes (high
> frequency) */
>  #define	OPCODE_QUAD_READ        0x6b    /* Read data bytes */
> @@ -81,6 +82,9 @@
> 
>  #define SR_QUAD_EN_MX           0x40    /* Macronix Quad I/O */
> 
> +/* Flag Status Register bits */
> +#define FSR_READY               0x80    /* FSR ready */
> +
>  /* Configuration Register bits. */
>  #define CR_QUAD_EN_SPAN		0x2     /* Spansion Quad I/O */
> 
> @@ -108,6 +112,7 @@ struct m25p {
>  	u8			read_opcode;
>  	u8			program_opcode;
>  	u8			*command;
> +	int (*wait_till_ready)(struct m25p *flash);
>  	enum read_type		flash_read;
>  };
> 
> @@ -145,6 +150,27 @@ static int read_sr(struct m25p *flash)  }
> 
>  /*
> + * Read the flag status register, returning its value in the location
> + * Return the status register value.
> + * Returns negative if error occurred.
> + */
> +static int read_fsr(struct m25p *flash) {
> +	ssize_t retval;
> +	u8 code = OPCODE_RDFSR;
> +	u8 val;
> +
> +	retval = spi_write_then_read(flash->spi, &code, 1, &val, 1);
> +
> +	if (retval < 0) {
> +		dev_err(&flash->spi->dev, "error %d reading FSR\n",
> +				(int) retval);
> +		return retval;
> +	}
> +
> +	return val;
> +}
> +/*
>   * Read configuration register, returning its value in the
>   * location. Return the configuration register value.
>   * Returns negative if error occured.
> @@ -233,7 +259,7 @@ static inline int set_4byte(struct m25p *flash, u32
> jedec_id, int enable)
>   * Service routine to read status register until ready, or timeout occurs.
>   * Returns non-zero if error.
>   */
> -static int wait_till_ready(struct m25p *flash)
> +static int _wait_till_ready(struct m25p *flash)
>  {
>  	unsigned long deadline;
>  	int sr;
> @@ -254,6 +280,37 @@ static int wait_till_ready(struct m25p *flash)  }
> 
>  /*
> + * Service routine to read flag status register until ready, or timeout occurs.
> + * Returns non-zero if error.
> + */
> +static int _wait_till_fsr_ready(struct m25p *flash) {
> +	unsigned long deadline;
> +	int fsr;
> +	int sr;
> +
> +	deadline = jiffies + MAX_READY_WAIT_JIFFIES;
> +
> +	do {
> +		sr = read_sr(flash);
> +		if (sr < 0)
> +			break;
> +		/* only check fsr if sr not busy */
> +		if (!(sr & SR_WIP)) {
> +			fsr = read_fsr(flash);
> +			if (fsr < 0)
> +				break;
> +			if (fsr & FSR_READY)
> +				return 0;
> +		}
> +
> +		cond_resched();
> +
> +	} while (!time_after_eq(jiffies, deadline));
> +
> +	return 1;
> +}
> +/*
>   * Write status Register and configuration register with 2 bytes
>   * The first byte will be written to the status register, while the
>   * second byte will be written to the configuration register.
> @@ -280,7 +337,7 @@ static int macronix_quad_enable(struct m25p *flash)
> 
>  	spi_write(flash->spi, &cmd, 2);
> 
> -	if (wait_till_ready(flash))
> +	if (flash->wait_till_ready(flash))
>  		return 1;
> 
>  	ret = read_sr(flash);
> @@ -351,7 +408,7 @@ static int erase_chip(struct m25p *flash)
>  			(long long)(flash->mtd.size >> 10));
> 
>  	/* Wait until finished previous write command. */
> -	if (wait_till_ready(flash))
> +	if (flash->wait_till_ready(flash))
>  		return 1;
> 
>  	/* Send write enable, then erase commands. */ @@ -391,7 +448,7
> @@ static int erase_sector(struct m25p *flash, u32 offset)
>  			__func__, flash->mtd.erasesize / 1024, offset);
> 
>  	/* Wait until finished previous write command. */
> -	if (wait_till_ready(flash))
> +	if (flash->wait_till_ready(flash))
>  		return 1;
> 
>  	/* Send write enable, then erase commands. */ @@ -536,7 +593,7
> @@ static int m25p80_read(struct mtd_info *mtd, loff_t from, size_t len,
>  	mutex_lock(&flash->lock);
> 
>  	/* Wait till previous write/erase is done. */
> -	if (wait_till_ready(flash)) {
> +	if (flash->wait_till_ready(flash)) {
>  		/* REVISIT status return?? */
>  		mutex_unlock(&flash->lock);
>  		return 1;
> @@ -585,7 +642,7 @@ static int m25p80_write(struct mtd_info *mtd, loff_t
> to, size_t len,
>  	mutex_lock(&flash->lock);
> 
>  	/* Wait until finished previous write command. */
> -	if (wait_till_ready(flash)) {
> +	if (flash->wait_till_ready(flash)) {
>  		mutex_unlock(&flash->lock);
>  		return 1;
>  	}
> @@ -628,7 +685,7 @@ static int m25p80_write(struct mtd_info *mtd, loff_t
> to, size_t len,
>  			t[1].tx_buf = buf + i;
>  			t[1].len = page_size;
> 
> -			wait_till_ready(flash);
> +			flash->wait_till_ready(flash);
> 
>  			write_enable(flash);
> 
> @@ -668,7 +725,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to,
> size_t len,
>  	mutex_lock(&flash->lock);
> 
>  	/* Wait until finished previous write command. */
> -	ret = wait_till_ready(flash);
> +	ret = flash->wait_till_ready(flash);
>  	if (ret)
>  		goto time_out;
> 
> @@ -683,7 +740,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to,
> size_t len,
>  		/* write one byte. */
>  		t[1].len = 1;
>  		spi_sync(flash->spi, &m);
> -		ret = wait_till_ready(flash);
> +		ret = flash->wait_till_ready(flash);
>  		if (ret)
>  			goto time_out;
>  		*retlen += m.actual_length - m25p_cmdsz(flash); @@ -702,7
> +759,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
>  		t[1].tx_buf = buf + actual;
> 
>  		spi_sync(flash->spi, &m);
> -		ret = wait_till_ready(flash);
> +		ret = flash->wait_till_ready(flash);
>  		if (ret)
>  			goto time_out;
>  		*retlen += m.actual_length - cmd_sz;
> @@ -710,7 +767,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to,
> size_t len,
>  		to += 2;
>  	}
>  	write_disable(flash);
> -	ret = wait_till_ready(flash);
> +	ret = flash->wait_till_ready(flash);
>  	if (ret)
>  		goto time_out;
> 
> @@ -724,7 +781,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to,
> size_t len,
>  		t[1].tx_buf = buf + actual;
> 
>  		spi_sync(flash->spi, &m);
> -		ret = wait_till_ready(flash);
> +		ret = flash->wait_till_ready(flash);
>  		if (ret)
>  			goto time_out;
>  		*retlen += m.actual_length - m25p_cmdsz(flash); @@ -745,7
> +802,7 @@ static int m25p80_lock(struct mtd_info *mtd, loff_t ofs, uint64_t
> len)
> 
>  	mutex_lock(&flash->lock);
>  	/* Wait until finished previous command */
> -	if (wait_till_ready(flash)) {
> +	if (flash->wait_till_ready(flash)) {
>  		res = 1;
>  		goto err;
>  	}
> @@ -790,7 +847,7 @@ static int m25p80_unlock(struct mtd_info *mtd, loff_t
> ofs, uint64_t len)
> 
>  	mutex_lock(&flash->lock);
>  	/* Wait until finished previous command */
> -	if (wait_till_ready(flash)) {
> +	if (flash->wait_till_ready(flash)) {
>  		res = 1;
>  		goto err;
>  	}
> @@ -856,6 +913,7 @@ struct flash_info {
>  #define	M25P_NO_FR	0x08		/* Can't do fastread */
>  #define	SECT_4K_PMC	0x10		/* OPCODE_BE_4K_PMC
> works uniformly */
>  #define	M25P80_QUAD_READ	0x20    /* Flash supports Quad Read
> */
> +#define	USE_FSR		0x40		/* use flag status
> register */
>  };
> 
>  #define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)	\
> @@ -941,6 +999,8 @@ static const struct spi_device_id m25p_ids[] = {
>  	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, 0) },
>  	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K) },
>  	{ "n25q512a",    INFO(0x20bb20, 0, 64 * 1024, 1024, SECT_4K) },
> +	{ "n25q512a1",   INFO(0x20ba20, 0, 64 * 1024, 1024, USE_FSR) },
> +	{ "n25q00",      INFO(0x20ba21, 0, 64 * 1024, 2048, USE_FSR) },
> 
>  	/* PMC */
>  	{ "pm25lv512",   INFO(0,        0, 32 * 1024,    2, SECT_4K_PMC) },
> @@ -1206,6 +1266,12 @@ static int m25p_probe(struct spi_device *spi)
>  	if (info->flags & M25P_NO_ERASE)
>  		flash->mtd.flags |= MTD_NO_ERASE;
> 
> +	if (info->flags & USE_FSR)
> +		flash->wait_till_ready = &_wait_till_fsr_ready;
> +	else
> +		flash->wait_till_ready = &_wait_till_ready;
> +
> +
>  	ppdata.of_node = spi->dev.of_node;
>  	flash->mtd.dev.parent = &spi->dev;
>  	flash->page_size = info->page_size;
> --
> 1.7.10.4


^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH] Add support for flag status register on Micron chips.
@ 2014-04-08 16:52     ` Insop Song
  0 siblings, 0 replies; 20+ messages in thread
From: Insop Song @ 2014-04-08 16:52 UTC (permalink / raw)
  To: grmoore, ggrahammoore
  Cc: Marek Vasut, Geert Uytterhoeven, Artem Bityutskiy, Sascha Hauer,
	Jingoo Han, linux-kernel, Yves Vandervennet, linux-mtd,
	Alan Tull, Sourav Poddar, Brian Norris, David Woodhouse,
	Dinh Nguyen


On Tuesday, April 08, 2014 9:13 AM, Graham Moore <grmoore@altera.com>
> 
> From: Graham Moore <grmoore@altera.com>
> 
> Some new Micron flash chips require reading the flag status register to
> determine when operations have completed.
> 
> Furthermore, chips with multi-die stacks of the 65nm 256Mb QSPI also
> require reading the status register before reading the flag status register.
> 
> This patch adds support for the flag status register in the n25q512a1 and
> n25q00 Micron QSPI flash chips.
> 

Reviewed his change, and it is more generic then my previous patch.

Reviewed-by: Insop Song <insop.song@gainspeed.com>

> Signed-off-by: Graham Moore <grmoore@altera.com>
> ---
>  drivers/mtd/devices/m25p80.c |   94
> +++++++++++++++++++++++++++++++++++-------
>  1 file changed, 80 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index ad19139..38306aa 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -39,6 +39,7 @@
>  #define	OPCODE_WREN		0x06	/* Write enable */
>  #define	OPCODE_RDSR		0x05	/* Read status register */
>  #define	OPCODE_WRSR		0x01	/* Write status
> register 1 byte */
> +#define	OPCODE_RDFSR		0x70  /* read flag status
> register */
>  #define	OPCODE_NORM_READ	0x03	/* Read data bytes (low
> frequency) */
>  #define	OPCODE_FAST_READ	0x0b	/* Read data bytes (high
> frequency) */
>  #define	OPCODE_QUAD_READ        0x6b    /* Read data bytes */
> @@ -81,6 +82,9 @@
> 
>  #define SR_QUAD_EN_MX           0x40    /* Macronix Quad I/O */
> 
> +/* Flag Status Register bits */
> +#define FSR_READY               0x80    /* FSR ready */
> +
>  /* Configuration Register bits. */
>  #define CR_QUAD_EN_SPAN		0x2     /* Spansion Quad I/O */
> 
> @@ -108,6 +112,7 @@ struct m25p {
>  	u8			read_opcode;
>  	u8			program_opcode;
>  	u8			*command;
> +	int (*wait_till_ready)(struct m25p *flash);
>  	enum read_type		flash_read;
>  };
> 
> @@ -145,6 +150,27 @@ static int read_sr(struct m25p *flash)  }
> 
>  /*
> + * Read the flag status register, returning its value in the location
> + * Return the status register value.
> + * Returns negative if error occurred.
> + */
> +static int read_fsr(struct m25p *flash) {
> +	ssize_t retval;
> +	u8 code = OPCODE_RDFSR;
> +	u8 val;
> +
> +	retval = spi_write_then_read(flash->spi, &code, 1, &val, 1);
> +
> +	if (retval < 0) {
> +		dev_err(&flash->spi->dev, "error %d reading FSR\n",
> +				(int) retval);
> +		return retval;
> +	}
> +
> +	return val;
> +}
> +/*
>   * Read configuration register, returning its value in the
>   * location. Return the configuration register value.
>   * Returns negative if error occured.
> @@ -233,7 +259,7 @@ static inline int set_4byte(struct m25p *flash, u32
> jedec_id, int enable)
>   * Service routine to read status register until ready, or timeout occurs.
>   * Returns non-zero if error.
>   */
> -static int wait_till_ready(struct m25p *flash)
> +static int _wait_till_ready(struct m25p *flash)
>  {
>  	unsigned long deadline;
>  	int sr;
> @@ -254,6 +280,37 @@ static int wait_till_ready(struct m25p *flash)  }
> 
>  /*
> + * Service routine to read flag status register until ready, or timeout occurs.
> + * Returns non-zero if error.
> + */
> +static int _wait_till_fsr_ready(struct m25p *flash) {
> +	unsigned long deadline;
> +	int fsr;
> +	int sr;
> +
> +	deadline = jiffies + MAX_READY_WAIT_JIFFIES;
> +
> +	do {
> +		sr = read_sr(flash);
> +		if (sr < 0)
> +			break;
> +		/* only check fsr if sr not busy */
> +		if (!(sr & SR_WIP)) {
> +			fsr = read_fsr(flash);
> +			if (fsr < 0)
> +				break;
> +			if (fsr & FSR_READY)
> +				return 0;
> +		}
> +
> +		cond_resched();
> +
> +	} while (!time_after_eq(jiffies, deadline));
> +
> +	return 1;
> +}
> +/*
>   * Write status Register and configuration register with 2 bytes
>   * The first byte will be written to the status register, while the
>   * second byte will be written to the configuration register.
> @@ -280,7 +337,7 @@ static int macronix_quad_enable(struct m25p *flash)
> 
>  	spi_write(flash->spi, &cmd, 2);
> 
> -	if (wait_till_ready(flash))
> +	if (flash->wait_till_ready(flash))
>  		return 1;
> 
>  	ret = read_sr(flash);
> @@ -351,7 +408,7 @@ static int erase_chip(struct m25p *flash)
>  			(long long)(flash->mtd.size >> 10));
> 
>  	/* Wait until finished previous write command. */
> -	if (wait_till_ready(flash))
> +	if (flash->wait_till_ready(flash))
>  		return 1;
> 
>  	/* Send write enable, then erase commands. */ @@ -391,7 +448,7
> @@ static int erase_sector(struct m25p *flash, u32 offset)
>  			__func__, flash->mtd.erasesize / 1024, offset);
> 
>  	/* Wait until finished previous write command. */
> -	if (wait_till_ready(flash))
> +	if (flash->wait_till_ready(flash))
>  		return 1;
> 
>  	/* Send write enable, then erase commands. */ @@ -536,7 +593,7
> @@ static int m25p80_read(struct mtd_info *mtd, loff_t from, size_t len,
>  	mutex_lock(&flash->lock);
> 
>  	/* Wait till previous write/erase is done. */
> -	if (wait_till_ready(flash)) {
> +	if (flash->wait_till_ready(flash)) {
>  		/* REVISIT status return?? */
>  		mutex_unlock(&flash->lock);
>  		return 1;
> @@ -585,7 +642,7 @@ static int m25p80_write(struct mtd_info *mtd, loff_t
> to, size_t len,
>  	mutex_lock(&flash->lock);
> 
>  	/* Wait until finished previous write command. */
> -	if (wait_till_ready(flash)) {
> +	if (flash->wait_till_ready(flash)) {
>  		mutex_unlock(&flash->lock);
>  		return 1;
>  	}
> @@ -628,7 +685,7 @@ static int m25p80_write(struct mtd_info *mtd, loff_t
> to, size_t len,
>  			t[1].tx_buf = buf + i;
>  			t[1].len = page_size;
> 
> -			wait_till_ready(flash);
> +			flash->wait_till_ready(flash);
> 
>  			write_enable(flash);
> 
> @@ -668,7 +725,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to,
> size_t len,
>  	mutex_lock(&flash->lock);
> 
>  	/* Wait until finished previous write command. */
> -	ret = wait_till_ready(flash);
> +	ret = flash->wait_till_ready(flash);
>  	if (ret)
>  		goto time_out;
> 
> @@ -683,7 +740,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to,
> size_t len,
>  		/* write one byte. */
>  		t[1].len = 1;
>  		spi_sync(flash->spi, &m);
> -		ret = wait_till_ready(flash);
> +		ret = flash->wait_till_ready(flash);
>  		if (ret)
>  			goto time_out;
>  		*retlen += m.actual_length - m25p_cmdsz(flash); @@ -702,7
> +759,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
>  		t[1].tx_buf = buf + actual;
> 
>  		spi_sync(flash->spi, &m);
> -		ret = wait_till_ready(flash);
> +		ret = flash->wait_till_ready(flash);
>  		if (ret)
>  			goto time_out;
>  		*retlen += m.actual_length - cmd_sz;
> @@ -710,7 +767,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to,
> size_t len,
>  		to += 2;
>  	}
>  	write_disable(flash);
> -	ret = wait_till_ready(flash);
> +	ret = flash->wait_till_ready(flash);
>  	if (ret)
>  		goto time_out;
> 
> @@ -724,7 +781,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to,
> size_t len,
>  		t[1].tx_buf = buf + actual;
> 
>  		spi_sync(flash->spi, &m);
> -		ret = wait_till_ready(flash);
> +		ret = flash->wait_till_ready(flash);
>  		if (ret)
>  			goto time_out;
>  		*retlen += m.actual_length - m25p_cmdsz(flash); @@ -745,7
> +802,7 @@ static int m25p80_lock(struct mtd_info *mtd, loff_t ofs, uint64_t
> len)
> 
>  	mutex_lock(&flash->lock);
>  	/* Wait until finished previous command */
> -	if (wait_till_ready(flash)) {
> +	if (flash->wait_till_ready(flash)) {
>  		res = 1;
>  		goto err;
>  	}
> @@ -790,7 +847,7 @@ static int m25p80_unlock(struct mtd_info *mtd, loff_t
> ofs, uint64_t len)
> 
>  	mutex_lock(&flash->lock);
>  	/* Wait until finished previous command */
> -	if (wait_till_ready(flash)) {
> +	if (flash->wait_till_ready(flash)) {
>  		res = 1;
>  		goto err;
>  	}
> @@ -856,6 +913,7 @@ struct flash_info {
>  #define	M25P_NO_FR	0x08		/* Can't do fastread */
>  #define	SECT_4K_PMC	0x10		/* OPCODE_BE_4K_PMC
> works uniformly */
>  #define	M25P80_QUAD_READ	0x20    /* Flash supports Quad Read
> */
> +#define	USE_FSR		0x40		/* use flag status
> register */
>  };
> 
>  #define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)	\
> @@ -941,6 +999,8 @@ static const struct spi_device_id m25p_ids[] = {
>  	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, 0) },
>  	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K) },
>  	{ "n25q512a",    INFO(0x20bb20, 0, 64 * 1024, 1024, SECT_4K) },
> +	{ "n25q512a1",   INFO(0x20ba20, 0, 64 * 1024, 1024, USE_FSR) },
> +	{ "n25q00",      INFO(0x20ba21, 0, 64 * 1024, 2048, USE_FSR) },
> 
>  	/* PMC */
>  	{ "pm25lv512",   INFO(0,        0, 32 * 1024,    2, SECT_4K_PMC) },
> @@ -1206,6 +1266,12 @@ static int m25p_probe(struct spi_device *spi)
>  	if (info->flags & M25P_NO_ERASE)
>  		flash->mtd.flags |= MTD_NO_ERASE;
> 
> +	if (info->flags & USE_FSR)
> +		flash->wait_till_ready = &_wait_till_fsr_ready;
> +	else
> +		flash->wait_till_ready = &_wait_till_ready;
> +
> +
>  	ppdata.of_node = spi->dev.of_node;
>  	flash->mtd.dev.parent = &spi->dev;
>  	flash->page_size = info->page_size;
> --
> 1.7.10.4

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Add support for flag status register on Micron chips
  2014-04-08 16:12 ` grmoore
@ 2014-04-09 10:03   ` Marek Vasut
  -1 siblings, 0 replies; 20+ messages in thread
From: Marek Vasut @ 2014-04-09 10:03 UTC (permalink / raw)
  To: grmoore
  Cc: ggrahammoore, David Woodhouse, Brian Norris, Artem Bityutskiy,
	Sourav Poddar, Sascha Hauer, Geert Uytterhoeven, Jingoo Han,
	Insop Song, linux-mtd, linux-kernel, Alan Tull, Dinh Nguyen,
	Yves Vandervennet, Gerhard Sittig

On Tuesday, April 08, 2014 at 06:12:49 PM, grmoore@altera.com wrote:
> From: Graham Moore <grmoore@altera.com>
> 
> This is a slightly different version of the patch that Insop Song
> submitted
> (http://marc.info/?i=201403012022.10111.marex%20()%20denx%20!%20de).
> 
> I talked to Insop, and he agreed I should submit this patch as a follow-on
> to his.
> 
> This patch uses a flag in the m25p_ids[] array to determine which chips
> need to use the FSR (Flag Status Register).
> 
> Rationale for using the FSR:
> 
> The Micron data sheets say we have to do this, at least for the multi-die
> 512M and 1G parts (n25q512 and n25q00).  In practice, if we don't check
> the FSR for program/erase status, and we rely solely on the status
> register (SR), then we get corrupted data in the flash.

I talked to Gerhard yesterday and he told me there is something like that on 
ONFI NAND. I think I now understand why that new register is in-place. 
Apparently, in the ONFI NAND case, there is a READY and TRUE-READY signal and 
one of those reflects that _all_ the dies have finished their operation. This is 
in my opinion seriously misdesigned as it breaks any kind of backward 
compatibility.

> Micron told us (Altera) that for multi-die chips based on the 65nm 256MB
> die, we need to check the SR first, then check the FSR, which is why the
> wait_for_fsr_ready function does that.  Future chips based on 45 nm 512MB
> die will use the FSR only.

Can these SPI flash makers screw the design even more? OT: Why don't we have a 
single standard for all the SF chips which won't need all these crappy quirks 
:-(

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Add support for flag status register on Micron chips
@ 2014-04-09 10:03   ` Marek Vasut
  0 siblings, 0 replies; 20+ messages in thread
From: Marek Vasut @ 2014-04-09 10:03 UTC (permalink / raw)
  To: grmoore
  Cc: ggrahammoore, Artem Bityutskiy, Sascha Hauer, Jingoo Han,
	Geert Uytterhoeven, linux-kernel, Yves Vandervennet, linux-mtd,
	Insop Song, Alan Tull, Sourav Poddar, Brian Norris,
	David Woodhouse, Gerhard Sittig, Dinh Nguyen

On Tuesday, April 08, 2014 at 06:12:49 PM, grmoore@altera.com wrote:
> From: Graham Moore <grmoore@altera.com>
> 
> This is a slightly different version of the patch that Insop Song
> submitted
> (http://marc.info/?i=201403012022.10111.marex%20()%20denx%20!%20de).
> 
> I talked to Insop, and he agreed I should submit this patch as a follow-on
> to his.
> 
> This patch uses a flag in the m25p_ids[] array to determine which chips
> need to use the FSR (Flag Status Register).
> 
> Rationale for using the FSR:
> 
> The Micron data sheets say we have to do this, at least for the multi-die
> 512M and 1G parts (n25q512 and n25q00).  In practice, if we don't check
> the FSR for program/erase status, and we rely solely on the status
> register (SR), then we get corrupted data in the flash.

I talked to Gerhard yesterday and he told me there is something like that on 
ONFI NAND. I think I now understand why that new register is in-place. 
Apparently, in the ONFI NAND case, there is a READY and TRUE-READY signal and 
one of those reflects that _all_ the dies have finished their operation. This is 
in my opinion seriously misdesigned as it breaks any kind of backward 
compatibility.

> Micron told us (Altera) that for multi-die chips based on the 65nm 256MB
> die, we need to check the SR first, then check the FSR, which is why the
> wait_for_fsr_ready function does that.  Future chips based on 45 nm 512MB
> die will use the FSR only.

Can these SPI flash makers screw the design even more? OT: Why don't we have a 
single standard for all the SF chips which won't need all these crappy quirks 
:-(

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Add support for flag status register on Micron chips.
  2014-04-08 16:12   ` grmoore
@ 2014-04-09 10:06     ` Marek Vasut
  -1 siblings, 0 replies; 20+ messages in thread
From: Marek Vasut @ 2014-04-09 10:06 UTC (permalink / raw)
  To: grmoore
  Cc: ggrahammoore, David Woodhouse, Brian Norris, Artem Bityutskiy,
	Sourav Poddar, Sascha Hauer, Geert Uytterhoeven, Jingoo Han,
	Insop Song, linux-mtd, linux-kernel, Alan Tull, Dinh Nguyen,
	Yves Vandervennet

On Tuesday, April 08, 2014 at 06:12:50 PM, grmoore@altera.com wrote:
> From: Graham Moore <grmoore@altera.com>
> 
> Some new Micron flash chips require reading the flag
> status register to determine when operations have completed.
> 
> Furthermore, chips with multi-die stacks of the 65nm 256Mb QSPI also
> require reading the status register before reading the flag status
> register.
> 
> This patch adds support for the flag status register in the n25q512a1 and
> n25q00 Micron QSPI flash chips.

[...]

> +static int read_fsr(struct m25p *flash)
> +{
> +	ssize_t retval;
> +	u8 code = OPCODE_RDFSR;
> +	u8 val;
> +
> +	retval = spi_write_then_read(flash->spi, &code, 1, &val, 1);
> +
> +	if (retval < 0) {
> +		dev_err(&flash->spi->dev, "error %d reading FSR\n",
> +				(int) retval);

Is the type-cast really needed here? Why ?

> +		return retval;
> +	}
> +
> +	return val;
> +}
> +/*
>   * Read configuration register, returning its value in the
>   * location. Return the configuration register value.
>   * Returns negative if error occured.
> @@ -233,7 +259,7 @@ static inline int set_4byte(struct m25p *flash, u32
> jedec_id, int enable) * Service routine to read status register until
> ready, or timeout occurs. * Returns non-zero if error.
>   */
> -static int wait_till_ready(struct m25p *flash)
> +static int _wait_till_ready(struct m25p *flash)

Please avoid using function names that start with underscore .
[...]

Thanks!

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Add support for flag status register on Micron chips.
@ 2014-04-09 10:06     ` Marek Vasut
  0 siblings, 0 replies; 20+ messages in thread
From: Marek Vasut @ 2014-04-09 10:06 UTC (permalink / raw)
  To: grmoore
  Cc: ggrahammoore, Artem Bityutskiy, Sascha Hauer, Jingoo Han,
	Geert Uytterhoeven, linux-kernel, Yves Vandervennet, linux-mtd,
	Insop Song, Alan Tull, Sourav Poddar, Brian Norris,
	David Woodhouse, Dinh Nguyen

On Tuesday, April 08, 2014 at 06:12:50 PM, grmoore@altera.com wrote:
> From: Graham Moore <grmoore@altera.com>
> 
> Some new Micron flash chips require reading the flag
> status register to determine when operations have completed.
> 
> Furthermore, chips with multi-die stacks of the 65nm 256Mb QSPI also
> require reading the status register before reading the flag status
> register.
> 
> This patch adds support for the flag status register in the n25q512a1 and
> n25q00 Micron QSPI flash chips.

[...]

> +static int read_fsr(struct m25p *flash)
> +{
> +	ssize_t retval;
> +	u8 code = OPCODE_RDFSR;
> +	u8 val;
> +
> +	retval = spi_write_then_read(flash->spi, &code, 1, &val, 1);
> +
> +	if (retval < 0) {
> +		dev_err(&flash->spi->dev, "error %d reading FSR\n",
> +				(int) retval);

Is the type-cast really needed here? Why ?

> +		return retval;
> +	}
> +
> +	return val;
> +}
> +/*
>   * Read configuration register, returning its value in the
>   * location. Return the configuration register value.
>   * Returns negative if error occured.
> @@ -233,7 +259,7 @@ static inline int set_4byte(struct m25p *flash, u32
> jedec_id, int enable) * Service routine to read status register until
> ready, or timeout occurs. * Returns non-zero if error.
>   */
> -static int wait_till_ready(struct m25p *flash)
> +static int _wait_till_ready(struct m25p *flash)

Please avoid using function names that start with underscore .
[...]

Thanks!

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Add support for flag status register on Micron chips.
  2014-04-09 10:06     ` Marek Vasut
@ 2014-04-09 10:16       ` Jingoo Han
  -1 siblings, 0 replies; 20+ messages in thread
From: Jingoo Han @ 2014-04-09 10:16 UTC (permalink / raw)
  To: 'Marek Vasut', 'Graham Moore'
  Cc: ggrahammoore, 'David Woodhouse', 'Brian Norris',
	'Artem Bityutskiy', 'Sourav Poddar',
	'Sascha Hauer', 'Geert Uytterhoeven',
	'Insop Song',
	linux-mtd, linux-kernel, 'Alan Tull',
	'Dinh Nguyen', 'Yves Vandervennet',
	'Jingoo Han'

On Wednesday, April 09, 2014 7:07 PM, Marek Vasut wrote:
> On Tuesday, April 08, 2014 at 06:12:50 PM, grmoore@altera.com wrote:
> > From: Graham Moore <grmoore@altera.com>
> >
> > Some new Micron flash chips require reading the flag
> > status register to determine when operations have completed.
> >
> > Furthermore, chips with multi-die stacks of the 65nm 256Mb QSPI also
> > require reading the status register before reading the flag status
> > register.
> >
> > This patch adds support for the flag status register in the n25q512a1 and
> > n25q00 Micron QSPI flash chips.
> 
> [...]
> 
> > +static int read_fsr(struct m25p *flash)
> > +{
> > +	ssize_t retval;
> > +	u8 code = OPCODE_RDFSR;
> > +	u8 val;
> > +
> > +	retval = spi_write_then_read(flash->spi, &code, 1, &val, 1);
> > +
> > +	if (retval < 0) {
> > +		dev_err(&flash->spi->dev, "error %d reading FSR\n",
> > +				(int) retval);
> 
> Is the type-cast really needed here? Why ?

The type-cast looks clumsy.
The type of 'retval' is retval; thus, '%zd' can be used,
instead of '%d', as below.

 +		dev_err(&flash->spi->dev, "error %zd reading FSR\n",
 +				retval);


> 
> > +		return retval;
> > +	}
> > +
> > +	return val;
> > +}
> > +/*
> >   * Read configuration register, returning its value in the
> >   * location. Return the configuration register value.
> >   * Returns negative if error occured.
> > @@ -233,7 +259,7 @@ static inline int set_4byte(struct m25p *flash, u32
> > jedec_id, int enable) * Service routine to read status register until
> > ready, or timeout occurs. * Returns non-zero if error.
> >   */
> > -static int wait_till_ready(struct m25p *flash)
> > +static int _wait_till_ready(struct m25p *flash)
> 
> Please avoid using function names that start with underscore .

+1

I agree with Marek Vasut's opinion.
If there is no reason, please don't use it.

Best regards,
Jingoo Han

> [...]
> 
> Thanks!
> 
> Best regards,
> Marek Vasut


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Add support for flag status register on Micron chips.
@ 2014-04-09 10:16       ` Jingoo Han
  0 siblings, 0 replies; 20+ messages in thread
From: Jingoo Han @ 2014-04-09 10:16 UTC (permalink / raw)
  To: 'Marek Vasut', 'Graham Moore'
  Cc: ggrahammoore, 'Artem Bityutskiy', 'Sascha Hauer',
	'Jingoo Han', 'Geert Uytterhoeven',
	linux-kernel, 'Yves Vandervennet',
	linux-mtd, 'Insop Song', 'Alan Tull',
	'Sourav Poddar', 'Brian Norris',
	'David Woodhouse', 'Dinh Nguyen'

On Wednesday, April 09, 2014 7:07 PM, Marek Vasut wrote:
> On Tuesday, April 08, 2014 at 06:12:50 PM, grmoore@altera.com wrote:
> > From: Graham Moore <grmoore@altera.com>
> >
> > Some new Micron flash chips require reading the flag
> > status register to determine when operations have completed.
> >
> > Furthermore, chips with multi-die stacks of the 65nm 256Mb QSPI also
> > require reading the status register before reading the flag status
> > register.
> >
> > This patch adds support for the flag status register in the n25q512a1 and
> > n25q00 Micron QSPI flash chips.
> 
> [...]
> 
> > +static int read_fsr(struct m25p *flash)
> > +{
> > +	ssize_t retval;
> > +	u8 code = OPCODE_RDFSR;
> > +	u8 val;
> > +
> > +	retval = spi_write_then_read(flash->spi, &code, 1, &val, 1);
> > +
> > +	if (retval < 0) {
> > +		dev_err(&flash->spi->dev, "error %d reading FSR\n",
> > +				(int) retval);
> 
> Is the type-cast really needed here? Why ?

The type-cast looks clumsy.
The type of 'retval' is retval; thus, '%zd' can be used,
instead of '%d', as below.

 +		dev_err(&flash->spi->dev, "error %zd reading FSR\n",
 +				retval);


> 
> > +		return retval;
> > +	}
> > +
> > +	return val;
> > +}
> > +/*
> >   * Read configuration register, returning its value in the
> >   * location. Return the configuration register value.
> >   * Returns negative if error occured.
> > @@ -233,7 +259,7 @@ static inline int set_4byte(struct m25p *flash, u32
> > jedec_id, int enable) * Service routine to read status register until
> > ready, or timeout occurs. * Returns non-zero if error.
> >   */
> > -static int wait_till_ready(struct m25p *flash)
> > +static int _wait_till_ready(struct m25p *flash)
> 
> Please avoid using function names that start with underscore .

+1

I agree with Marek Vasut's opinion.
If there is no reason, please don't use it.

Best regards,
Jingoo Han

> [...]
> 
> Thanks!
> 
> Best regards,
> Marek Vasut

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Add support for flag status register on Micron chips.
  2014-04-09 10:16       ` Jingoo Han
@ 2014-04-09 10:21         ` Jingoo Han
  -1 siblings, 0 replies; 20+ messages in thread
From: Jingoo Han @ 2014-04-09 10:21 UTC (permalink / raw)
  To: 'Marek Vasut', 'Graham Moore'
  Cc: ggrahammoore, 'David Woodhouse', 'Brian Norris',
	'Artem Bityutskiy', 'Sourav Poddar',
	'Sascha Hauer', 'Geert Uytterhoeven',
	'Insop Song',
	linux-mtd, linux-kernel, 'Alan Tull',
	'Dinh Nguyen', 'Yves Vandervennet',
	'Jingoo Han'

On Wednesday, April 09, 2014 7:16 PM, Jingoo Han wrote:
> On Wednesday, April 09, 2014 7:07 PM, Marek Vasut wrote:
> > On Tuesday, April 08, 2014 at 06:12:50 PM, grmoore@altera.com wrote:
> > > From: Graham Moore <grmoore@altera.com>
> > >
> > > Some new Micron flash chips require reading the flag
> > > status register to determine when operations have completed.
> > >
> > > Furthermore, chips with multi-die stacks of the 65nm 256Mb QSPI also
> > > require reading the status register before reading the flag status
> > > register.
> > >
> > > This patch adds support for the flag status register in the n25q512a1 and
> > > n25q00 Micron QSPI flash chips.
> >
> > [...]
> >
> > > +static int read_fsr(struct m25p *flash)
> > > +{
> > > +	ssize_t retval;
> > > +	u8 code = OPCODE_RDFSR;
> > > +	u8 val;
> > > +
> > > +	retval = spi_write_then_read(flash->spi, &code, 1, &val, 1);
> > > +
> > > +	if (retval < 0) {
> > > +		dev_err(&flash->spi->dev, "error %d reading FSR\n",
> > > +				(int) retval);
> >
> > Is the type-cast really needed here? Why ?
> 
> The type-cast looks clumsy.
> The type of 'retval' is retval; thus, '%zd' can be used,

Sorry, there is a typo!
s/retval/ssize_t

The type of 'retval' is "ssize_t"; thus, '%zd' can be used,
instead of '%d', as below.

 +		dev_err(&flash->spi->dev, "error %zd reading FSR\n",
 +				retval);

Best regards,
Jingoo Han


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Add support for flag status register on Micron chips.
@ 2014-04-09 10:21         ` Jingoo Han
  0 siblings, 0 replies; 20+ messages in thread
From: Jingoo Han @ 2014-04-09 10:21 UTC (permalink / raw)
  To: 'Marek Vasut', 'Graham Moore'
  Cc: ggrahammoore, 'Artem Bityutskiy', 'Sascha Hauer',
	'Jingoo Han', 'Geert Uytterhoeven',
	linux-kernel, 'Yves Vandervennet',
	linux-mtd, 'Insop Song', 'Alan Tull',
	'Sourav Poddar', 'Brian Norris',
	'David Woodhouse', 'Dinh Nguyen'

On Wednesday, April 09, 2014 7:16 PM, Jingoo Han wrote:
> On Wednesday, April 09, 2014 7:07 PM, Marek Vasut wrote:
> > On Tuesday, April 08, 2014 at 06:12:50 PM, grmoore@altera.com wrote:
> > > From: Graham Moore <grmoore@altera.com>
> > >
> > > Some new Micron flash chips require reading the flag
> > > status register to determine when operations have completed.
> > >
> > > Furthermore, chips with multi-die stacks of the 65nm 256Mb QSPI also
> > > require reading the status register before reading the flag status
> > > register.
> > >
> > > This patch adds support for the flag status register in the n25q512a1 and
> > > n25q00 Micron QSPI flash chips.
> >
> > [...]
> >
> > > +static int read_fsr(struct m25p *flash)
> > > +{
> > > +	ssize_t retval;
> > > +	u8 code = OPCODE_RDFSR;
> > > +	u8 val;
> > > +
> > > +	retval = spi_write_then_read(flash->spi, &code, 1, &val, 1);
> > > +
> > > +	if (retval < 0) {
> > > +		dev_err(&flash->spi->dev, "error %d reading FSR\n",
> > > +				(int) retval);
> >
> > Is the type-cast really needed here? Why ?
> 
> The type-cast looks clumsy.
> The type of 'retval' is retval; thus, '%zd' can be used,

Sorry, there is a typo!
s/retval/ssize_t

The type of 'retval' is "ssize_t"; thus, '%zd' can be used,
instead of '%d', as below.

 +		dev_err(&flash->spi->dev, "error %zd reading FSR\n",
 +				retval);

Best regards,
Jingoo Han

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Add support for flag status register on Micron chips
  2014-04-09 10:03   ` Marek Vasut
@ 2014-04-09 11:09     ` Gerhard Sittig
  -1 siblings, 0 replies; 20+ messages in thread
From: Gerhard Sittig @ 2014-04-09 11:09 UTC (permalink / raw)
  To: Marek Vasut
  Cc: grmoore, ggrahammoore, David Woodhouse, Brian Norris,
	Artem Bityutskiy, Sourav Poddar, Sascha Hauer,
	Geert Uytterhoeven, Jingoo Han, Insop Song, linux-mtd,
	linux-kernel, Alan Tull, Dinh Nguyen, Yves Vandervennet

On Wed, 2014-04-09 at 12:03 +0200, Marek Vasut wrote:
> 
> On Tuesday, April 08, 2014 at 06:12:49 PM, grmoore@altera.com wrote:
> > From: Graham Moore <grmoore@altera.com>
> > 
> > This is a slightly different version of the patch that Insop Song
> > submitted
> > (http://marc.info/?i=201403012022.10111.marex%20()%20denx%20!%20de).
> > 
> > I talked to Insop, and he agreed I should submit this patch as a follow-on
> > to his.
> > 
> > This patch uses a flag in the m25p_ids[] array to determine which chips
> > need to use the FSR (Flag Status Register).
> > 
> > Rationale for using the FSR:
> > 
> > The Micron data sheets say we have to do this, at least for the multi-die
> > 512M and 1G parts (n25q512 and n25q00).  In practice, if we don't check
> > the FSR for program/erase status, and we rely solely on the status
> > register (SR), then we get corrupted data in the flash.
> 
> I talked to Gerhard yesterday and he told me there is something like that on 
> ONFI NAND. I think I now understand why that new register is in-place. 
> Apparently, in the ONFI NAND case, there is a READY and TRUE-READY signal and 
> one of those reflects that _all_ the dies have finished their operation. This is 
> in my opinion seriously misdesigned as it breaks any kind of backward 
> compatibility.

There's a little more to it, I think.

The "ready" and "true ready" are terms from Linux header files.
In Micron datasheets, both of the bits 5 and 6 in the READ_STATUS
response are referred to as "ready/busy", with a few footnotes to
them depending on the mode of operation or the command that is
being executed.

The other thing is that there is the READ_STATUS command, which
_might_ yield responses from different dies upon subsequent
repeated reads.  So you may have to determine how many dies are
in the package, to repeat the STATUS query as many times, and
logically combine these results in the chip's overall status.  Or
use the extended status query where you can address an individual
die, but which is not supported by every chip.  Pick your poison.

The hardware R/B pins are wired-OR, such that any busy die will
pull the summary pin level low.  But this only tells you whether
the operation is still pending, after it's complete you have to
get the status and will face the situation described above.

Can't tell how this NAND approach maps to the NOR subject
described here.  They might have a similar motivation, yet
implement different approaches to the issue.

> > Micron told us (Altera) that for multi-die chips based on the 65nm 256MB
> > die, we need to check the SR first, then check the FSR, which is why the
> > wait_for_fsr_ready function does that.  Future chips based on 45 nm 512MB
> > die will use the FSR only.

This sounds to me similar to polling the NAND's R/B pin until the
operation has completed, to then fetch the STATUS byte to
determine the execution's result.  Does this sound plausible?
For NOR, do you poll for the "busy" condition to deassert, and
check for success then?


virtually yours
Gerhard Sittig
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office@denx.de

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Add support for flag status register on Micron chips
@ 2014-04-09 11:09     ` Gerhard Sittig
  0 siblings, 0 replies; 20+ messages in thread
From: Gerhard Sittig @ 2014-04-09 11:09 UTC (permalink / raw)
  To: Marek Vasut
  Cc: ggrahammoore, Insop Song, grmoore, Sascha Hauer, Jingoo Han,
	Geert Uytterhoeven, linux-kernel, Yves Vandervennet, linux-mtd,
	Artem Bityutskiy, Alan Tull, Sourav Poddar, Brian Norris,
	David Woodhouse, Dinh Nguyen

On Wed, 2014-04-09 at 12:03 +0200, Marek Vasut wrote:
> 
> On Tuesday, April 08, 2014 at 06:12:49 PM, grmoore@altera.com wrote:
> > From: Graham Moore <grmoore@altera.com>
> > 
> > This is a slightly different version of the patch that Insop Song
> > submitted
> > (http://marc.info/?i=201403012022.10111.marex%20()%20denx%20!%20de).
> > 
> > I talked to Insop, and he agreed I should submit this patch as a follow-on
> > to his.
> > 
> > This patch uses a flag in the m25p_ids[] array to determine which chips
> > need to use the FSR (Flag Status Register).
> > 
> > Rationale for using the FSR:
> > 
> > The Micron data sheets say we have to do this, at least for the multi-die
> > 512M and 1G parts (n25q512 and n25q00).  In practice, if we don't check
> > the FSR for program/erase status, and we rely solely on the status
> > register (SR), then we get corrupted data in the flash.
> 
> I talked to Gerhard yesterday and he told me there is something like that on 
> ONFI NAND. I think I now understand why that new register is in-place. 
> Apparently, in the ONFI NAND case, there is a READY and TRUE-READY signal and 
> one of those reflects that _all_ the dies have finished their operation. This is 
> in my opinion seriously misdesigned as it breaks any kind of backward 
> compatibility.

There's a little more to it, I think.

The "ready" and "true ready" are terms from Linux header files.
In Micron datasheets, both of the bits 5 and 6 in the READ_STATUS
response are referred to as "ready/busy", with a few footnotes to
them depending on the mode of operation or the command that is
being executed.

The other thing is that there is the READ_STATUS command, which
_might_ yield responses from different dies upon subsequent
repeated reads.  So you may have to determine how many dies are
in the package, to repeat the STATUS query as many times, and
logically combine these results in the chip's overall status.  Or
use the extended status query where you can address an individual
die, but which is not supported by every chip.  Pick your poison.

The hardware R/B pins are wired-OR, such that any busy die will
pull the summary pin level low.  But this only tells you whether
the operation is still pending, after it's complete you have to
get the status and will face the situation described above.

Can't tell how this NAND approach maps to the NOR subject
described here.  They might have a similar motivation, yet
implement different approaches to the issue.

> > Micron told us (Altera) that for multi-die chips based on the 65nm 256MB
> > die, we need to check the SR first, then check the FSR, which is why the
> > wait_for_fsr_ready function does that.  Future chips based on 45 nm 512MB
> > die will use the FSR only.

This sounds to me similar to polling the NAND's R/B pin until the
operation has completed, to then fetch the STATUS byte to
determine the execution's result.  Does this sound plausible?
For NOR, do you poll for the "busy" condition to deassert, and
check for success then?


virtually yours
Gerhard Sittig
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office@denx.de

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Add support for flag status register on Micron chips
  2014-04-09 11:09     ` Gerhard Sittig
@ 2014-04-09 18:14       ` Graham Moore
  -1 siblings, 0 replies; 20+ messages in thread
From: Graham Moore @ 2014-04-09 18:14 UTC (permalink / raw)
  To: Gerhard Sittig
  Cc: Marek Vasut, grmoore, David Woodhouse, Brian Norris,
	Artem Bityutskiy, Sourav Poddar, Sascha Hauer,
	Geert Uytterhoeven, Jingoo Han, Insop Song, linux-mtd,
	linux-kernel, Alan Tull, Dinh Nguyen, Yves Vandervennet

On Wed, Apr 9, 2014 at 6:09 AM, Gerhard Sittig <gsi@denx.de> wrote:
> On Wed, 2014-04-09 at 12:03 +0200, Marek Vasut wrote:
>>
>> On Tuesday, April 08, 2014 at 06:12:49 PM, grmoore@altera.com wrote:
>> > From: Graham Moore <grmoore@altera.com>
>> >
>> > This is a slightly different version of the patch that Insop Song
>> > submitted
>> > (http://marc.info/?i=201403012022.10111.marex%20()%20denx%20!%20de).
>> >
>> > I talked to Insop, and he agreed I should submit this patch as a follow-on
>> > to his.
>> >
>> > This patch uses a flag in the m25p_ids[] array to determine which chips
>> > need to use the FSR (Flag Status Register).
>> >
>> > Rationale for using the FSR:
>> >
>> > The Micron data sheets say we have to do this, at least for the multi-die
>> > 512M and 1G parts (n25q512 and n25q00).  In practice, if we don't check
>> > the FSR for program/erase status, and we rely solely on the status
>> > register (SR), then we get corrupted data in the flash.
[...]
>> > Micron told us (Altera) that for multi-die chips based on the 65nm 256MB
>> > die, we need to check the SR first, then check the FSR, which is why the
>> > wait_for_fsr_ready function does that.  Future chips based on 45 nm 512MB
>> > die will use the FSR only.
>
> This sounds to me similar to polling the NAND's R/B pin until the
> operation has completed, to then fetch the STATUS byte to
> determine the execution's result.  Does this sound plausible?
> For NOR, do you poll for the "busy" condition to deassert, and
> check for success then?

Sounds plausible to me.  We poll the SR until not busy, then poll the
FSR until it's not busy.  Success is when FSR busy is deasserted
within the timeout.

Micron said we have to read the FSR "at least once", so we don't
read it once for every die or anything like that.  I ran a quick
test, and for both the 2-die and 4-die parts, the FSR shows not busy
on the first read after SR not busy.

-Graham

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Add support for flag status register on Micron chips
@ 2014-04-09 18:14       ` Graham Moore
  0 siblings, 0 replies; 20+ messages in thread
From: Graham Moore @ 2014-04-09 18:14 UTC (permalink / raw)
  To: Gerhard Sittig
  Cc: Marek Vasut, Geert Uytterhoeven, Insop Song, grmoore,
	Sascha Hauer, Jingoo Han, linux-kernel, Yves Vandervennet,
	linux-mtd, Artem Bityutskiy, Alan Tull, Sourav Poddar,
	Brian Norris, David Woodhouse, Dinh Nguyen

On Wed, Apr 9, 2014 at 6:09 AM, Gerhard Sittig <gsi@denx.de> wrote:
> On Wed, 2014-04-09 at 12:03 +0200, Marek Vasut wrote:
>>
>> On Tuesday, April 08, 2014 at 06:12:49 PM, grmoore@altera.com wrote:
>> > From: Graham Moore <grmoore@altera.com>
>> >
>> > This is a slightly different version of the patch that Insop Song
>> > submitted
>> > (http://marc.info/?i=201403012022.10111.marex%20()%20denx%20!%20de).
>> >
>> > I talked to Insop, and he agreed I should submit this patch as a follow-on
>> > to his.
>> >
>> > This patch uses a flag in the m25p_ids[] array to determine which chips
>> > need to use the FSR (Flag Status Register).
>> >
>> > Rationale for using the FSR:
>> >
>> > The Micron data sheets say we have to do this, at least for the multi-die
>> > 512M and 1G parts (n25q512 and n25q00).  In practice, if we don't check
>> > the FSR for program/erase status, and we rely solely on the status
>> > register (SR), then we get corrupted data in the flash.
[...]
>> > Micron told us (Altera) that for multi-die chips based on the 65nm 256MB
>> > die, we need to check the SR first, then check the FSR, which is why the
>> > wait_for_fsr_ready function does that.  Future chips based on 45 nm 512MB
>> > die will use the FSR only.
>
> This sounds to me similar to polling the NAND's R/B pin until the
> operation has completed, to then fetch the STATUS byte to
> determine the execution's result.  Does this sound plausible?
> For NOR, do you poll for the "busy" condition to deassert, and
> check for success then?

Sounds plausible to me.  We poll the SR until not busy, then poll the
FSR until it's not busy.  Success is when FSR busy is deasserted
within the timeout.

Micron said we have to read the FSR "at least once", so we don't
read it once for every die or anything like that.  I ran a quick
test, and for both the 2-die and 4-die parts, the FSR shows not busy
on the first read after SR not busy.

-Graham

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Add support for flag status register on Micron chips
  2014-04-09 18:14       ` Graham Moore
@ 2014-04-09 18:31         ` Marek Vasut
  -1 siblings, 0 replies; 20+ messages in thread
From: Marek Vasut @ 2014-04-09 18:31 UTC (permalink / raw)
  To: Graham Moore
  Cc: Gerhard Sittig, grmoore, David Woodhouse, Brian Norris,
	Artem Bityutskiy, Sourav Poddar, Sascha Hauer,
	Geert Uytterhoeven, Jingoo Han, Insop Song, linux-mtd,
	linux-kernel, Alan Tull, Dinh Nguyen, Yves Vandervennet

On Wednesday, April 09, 2014 at 08:14:49 PM, Graham Moore wrote:
> On Wed, Apr 9, 2014 at 6:09 AM, Gerhard Sittig <gsi@denx.de> wrote:
> > On Wed, 2014-04-09 at 12:03 +0200, Marek Vasut wrote:
> >> On Tuesday, April 08, 2014 at 06:12:49 PM, grmoore@altera.com wrote:
> >> > From: Graham Moore <grmoore@altera.com>
> >> > 
> >> > This is a slightly different version of the patch that Insop Song
> >> > submitted
> >> > (http://marc.info/?i=201403012022.10111.marex%20()%20denx%20!%20de).
> >> > 
> >> > I talked to Insop, and he agreed I should submit this patch as a
> >> > follow-on to his.
> >> > 
> >> > This patch uses a flag in the m25p_ids[] array to determine which
> >> > chips need to use the FSR (Flag Status Register).
> >> > 
> >> > Rationale for using the FSR:
> >> > 
> >> > The Micron data sheets say we have to do this, at least for the
> >> > multi-die 512M and 1G parts (n25q512 and n25q00).  In practice, if we
> >> > don't check the FSR for program/erase status, and we rely solely on
> >> > the status register (SR), then we get corrupted data in the flash.
> 
> [...]
> 
> >> > Micron told us (Altera) that for multi-die chips based on the 65nm
> >> > 256MB die, we need to check the SR first, then check the FSR, which
> >> > is why the wait_for_fsr_ready function does that.  Future chips based
> >> > on 45 nm 512MB die will use the FSR only.
> > 
> > This sounds to me similar to polling the NAND's R/B pin until the
> > operation has completed, to then fetch the STATUS byte to
> > determine the execution's result.  Does this sound plausible?
> > For NOR, do you poll for the "busy" condition to deassert, and
> > check for success then?
> 
> Sounds plausible to me.  We poll the SR until not busy, then poll the
> FSR until it's not busy.  Success is when FSR busy is deasserted
> within the timeout.
> 
> Micron said we have to read the FSR "at least once", so we don't
> read it once for every die or anything like that.  I ran a quick
> test, and for both the 2-die and 4-die parts, the FSR shows not busy
> on the first read after SR not busy.

I'd love to know how this FSR-not-busy is exactly related to SR-not-busy, but I 
guess only Micron can tell :-/

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Add support for flag status register on Micron chips
@ 2014-04-09 18:31         ` Marek Vasut
  0 siblings, 0 replies; 20+ messages in thread
From: Marek Vasut @ 2014-04-09 18:31 UTC (permalink / raw)
  To: Graham Moore
  Cc: Geert Uytterhoeven, Insop Song, grmoore, Gerhard Sittig,
	Jingoo Han, linux-kernel, Yves Vandervennet, linux-mtd,
	Artem Bityutskiy, Alan Tull, Sourav Poddar, Brian Norris,
	David Woodhouse, Sascha Hauer, Dinh Nguyen

On Wednesday, April 09, 2014 at 08:14:49 PM, Graham Moore wrote:
> On Wed, Apr 9, 2014 at 6:09 AM, Gerhard Sittig <gsi@denx.de> wrote:
> > On Wed, 2014-04-09 at 12:03 +0200, Marek Vasut wrote:
> >> On Tuesday, April 08, 2014 at 06:12:49 PM, grmoore@altera.com wrote:
> >> > From: Graham Moore <grmoore@altera.com>
> >> > 
> >> > This is a slightly different version of the patch that Insop Song
> >> > submitted
> >> > (http://marc.info/?i=201403012022.10111.marex%20()%20denx%20!%20de).
> >> > 
> >> > I talked to Insop, and he agreed I should submit this patch as a
> >> > follow-on to his.
> >> > 
> >> > This patch uses a flag in the m25p_ids[] array to determine which
> >> > chips need to use the FSR (Flag Status Register).
> >> > 
> >> > Rationale for using the FSR:
> >> > 
> >> > The Micron data sheets say we have to do this, at least for the
> >> > multi-die 512M and 1G parts (n25q512 and n25q00).  In practice, if we
> >> > don't check the FSR for program/erase status, and we rely solely on
> >> > the status register (SR), then we get corrupted data in the flash.
> 
> [...]
> 
> >> > Micron told us (Altera) that for multi-die chips based on the 65nm
> >> > 256MB die, we need to check the SR first, then check the FSR, which
> >> > is why the wait_for_fsr_ready function does that.  Future chips based
> >> > on 45 nm 512MB die will use the FSR only.
> > 
> > This sounds to me similar to polling the NAND's R/B pin until the
> > operation has completed, to then fetch the STATUS byte to
> > determine the execution's result.  Does this sound plausible?
> > For NOR, do you poll for the "busy" condition to deassert, and
> > check for success then?
> 
> Sounds plausible to me.  We poll the SR until not busy, then poll the
> FSR until it's not busy.  Success is when FSR busy is deasserted
> within the timeout.
> 
> Micron said we have to read the FSR "at least once", so we don't
> read it once for every die or anything like that.  I ran a quick
> test, and for both the 2-die and 4-die parts, the FSR shows not busy
> on the first read after SR not busy.

I'd love to know how this FSR-not-busy is exactly related to SR-not-busy, but I 
guess only Micron can tell :-/

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2014-04-09 21:11 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-08 16:12 [PATCH] Add support for flag status register on Micron chips grmoore
2014-04-08 16:12 ` grmoore
2014-04-08 16:12 ` grmoore
2014-04-08 16:12   ` grmoore
2014-04-08 16:52   ` Insop Song
2014-04-08 16:52     ` Insop Song
2014-04-09 10:06   ` Marek Vasut
2014-04-09 10:06     ` Marek Vasut
2014-04-09 10:16     ` Jingoo Han
2014-04-09 10:16       ` Jingoo Han
2014-04-09 10:21       ` Jingoo Han
2014-04-09 10:21         ` Jingoo Han
2014-04-09 10:03 ` Marek Vasut
2014-04-09 10:03   ` Marek Vasut
2014-04-09 11:09   ` Gerhard Sittig
2014-04-09 11:09     ` Gerhard Sittig
2014-04-09 18:14     ` Graham Moore
2014-04-09 18:14       ` Graham Moore
2014-04-09 18:31       ` Marek Vasut
2014-04-09 18:31         ` Marek Vasut

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.