Discussion:
[PATCH 1/1] driver:mtd:spi-nor: Add Micron quad I/O support
Marek Vasut
2014-09-25 10:11:57 UTC
Permalink
For Micron spi norflash,you can enable Quad spi transfer
by clear EVCR(Enhanced Volatile Configuration Register)
Quad I/O protocol bit.
---
drivers/mtd/spi-nor/spi-nor.c | 45
+++++++++++++++++++++++++++++++++++++++++ include/linux/mtd/spi-nor.h |
6 ++++++
2 files changed, 51 insertions(+)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index b5ad6be..e72894f 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -878,6 +878,44 @@ static int spansion_quad_enable(struct spi_nor *nor)
return 0;
}
+static int micron_quad_enable(struct spi_nor *nor)
+{
+ int ret, val;
+
+ ret = nor->read_reg(nor, SPINOR_OP_RD_EVCR, &val, 1);
+ if (ret < 0) {
+ dev_err(nor->dev, "error %d reading EVCR\n", ret);
+ return -EINVAL;
+ }
+
+ write_enable(nor);
+
+ /* set EVCR ,enable quad I/O */
+ nor->cmd_buf[0] = val & ~EVCR_QUAD_EN_MICRON;
+ ret = nor->write_reg(nor, SPINOR_OP_WD_EVCR, nor->cmd_buf, 1, 0);
+ if (ret < 0) {
+ dev_err(nor->dev,
+ "error while writing EVCR register\n");
+ return -EINVAL;
+ }
+
+ if (wait_till_ready(nor))
+ return 1;
Why does this not return proper error code or even better, return value from
wait_till_ready() ?

Other than that, there's nothing wrong with the patch I think.

Best regards,
Marek Vasut
bpqw
2014-09-26 08:39:38 UTC
Permalink
Post by Marek Vasut
+ /* set EVCR ,enable quad I/O */
+ nor->cmd_buf[0] = val & ~EVCR_QUAD_EN_MICRON;
+ ret = nor->write_reg(nor, SPINOR_OP_WD_EVCR, nor->cmd_buf, 1, 0);
+ if (ret < 0) {
+ dev_err(nor->dev,
+ "error while writing EVCR register\n");
+ return -EINVAL;
+ }
+
+ if (wait_till_ready(nor))
+ return 1;
Why does this not return proper error code or even better, return value from
wait_till_ready() ?
Other than that, there's nothing wrong with the patch I think.
Hi,Marek
Thanks for your review,you can find the same usage in the spi-nor.c.
Below method is OK? Or you can give me some suggestion.

if (wait_till_ready(nor))
return - EINVAL;
Marek Vasut
2014-09-26 08:46:07 UTC
Permalink
Post by bpqw
Post by Marek Vasut
+ /* set EVCR ,enable quad I/O */
+ nor->cmd_buf[0] = val & ~EVCR_QUAD_EN_MICRON;
+ ret = nor->write_reg(nor, SPINOR_OP_WD_EVCR, nor->cmd_buf, 1, 0);
+ if (ret < 0) {
+ dev_err(nor->dev,
+ "error while writing EVCR register\n");
+ return -EINVAL;
+ }
+
+ if (wait_till_ready(nor))
+ return 1;
Why does this not return proper error code or even better, return value
from wait_till_ready() ?
Other than that, there's nothing wrong with the patch I think.
Hi,Marek
Thanks for your review,you can find the same usage in the spi-nor.c.
Below method is OK? Or you can give me some suggestion.
if (wait_till_ready(nor))
return - EINVAL;
ret = wait_till_readynor()
if (ret)
return ret;

But all right, this means the subsystem isn't perfect. Well, others, what do you
think ?

Best regards,
Marek Vasut
bpqw
2014-09-28 01:59:42 UTC
Permalink
For Micron spi norflash,you can enable
Quad spi transfer by clear EVCR(Enhanced
Volatile Configuration Register) Quad I/O
protocol bit.

Signed-off-by: bean huo <***@micron.com>
---
v1-v2:modified to that capture wait_till_ready()
return value,if error,directly return its
the value.

drivers/mtd/spi-nor/spi-nor.c | 46 +++++++++++++++++++++++++++++++++++++++++
include/linux/mtd/spi-nor.h | 6 ++++++
2 files changed, 52 insertions(+)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index b5ad6be..0c3b4fd 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -878,6 +878,45 @@ static int spansion_quad_enable(struct spi_nor *nor)
return 0;
}

+static int micron_quad_enable(struct spi_nor *nor)
+{
+ int ret, val;
+
+ ret = nor->read_reg(nor, SPINOR_OP_RD_EVCR, &val, 1);
+ if (ret < 0) {
+ dev_err(nor->dev, "error %d reading EVCR\n", ret);
+ return -EINVAL;
+ }
+
+ write_enable(nor);
+
+ /* set EVCR ,enable quad I/O */
+ nor->cmd_buf[0] = val & ~EVCR_QUAD_EN_MICRON;
+ ret = nor->write_reg(nor, SPINOR_OP_WD_EVCR, nor->cmd_buf, 1, 0);
+ if (ret < 0) {
+ dev_err(nor->dev,
+ "error while writing EVCR register\n");
+ return -EINVAL;
+ }
+
+ ret = wait_till_ready(nor);
+ if (ret)
+ return ret;
+
+ /* read EVCR and check it */
+ ret = nor->read_reg(nor, SPINOR_OP_RD_EVCR, &val, 1);
+ if (ret < 0) {
+ dev_err(nor->dev, "error %d reading EVCR\n", ret);
+ return -EINVAL;
+ }
+ if (val & EVCR_QUAD_EN_MICRON) {
+ dev_err(nor->dev, "Micron EVCR Quad bit not clear\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int set_quad_mode(struct spi_nor *nor, u32 jedec_id)
{
int status;
@@ -890,6 +929,13 @@ static int set_quad_mode(struct spi_nor *nor, u32 jedec_id)
return -EINVAL;
}
return status;
+ case CFI_MFR_ST:
+ status = micron_quad_enable(nor);
+ if (status) {
+ dev_err(nor->dev, "Micron quad-read not enabled\n");
+ return -EINVAL;
+ }
+ return status;
default:
status = spansion_quad_enable(nor);
if (status) {
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 9e6294f..d71b659 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -56,6 +56,10 @@
/* Used for Spansion flashes only. */
#define SPINOR_OP_BRWR 0x17 /* Bank register write */

+/* Used for Micron flashes only. */
+#define SPINOR_OP_RD_EVCR 0x65 /* Read EVCR register */
+#define SPINOR_OP_WD_EVCR 0x61 /* Write EVCR register */
+
/* Status Register bits. */
#define SR_WIP 1 /* Write in progress */
#define SR_WEL 2 /* Write enable latch */
@@ -67,6 +71,8 @@

#define SR_QUAD_EN_MX 0x40 /* Macronix Quad I/O */

+#define EVCR_QUAD_EN_MICRON 0x80 /* Micron Quad I/O */
+
/* Flag Status Register bits */
#define FSR_READY 0x80
--
1.7.9.5
Marek Vasut
2014-09-28 22:43:43 UTC
Permalink
For Micron spi norflash,you can enable
Quad spi transfer by clear EVCR(Enhanced
Volatile Configuration Register) Quad I/O
protocol bit.
OK, this information is nice and all, but what does this patch do? I can't learn
this information from the commit message as it is, can I ? And , the purpose of
the commit message is exactly to summarize the change the patch implements.
---
v1-v2:modified to that capture wait_till_ready()
return value,if error,directly return its
the value.
drivers/mtd/spi-nor/spi-nor.c | 46
+++++++++++++++++++++++++++++++++++++++++ include/linux/mtd/spi-nor.h |
6 ++++++
2 files changed, 52 insertions(+)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index b5ad6be..0c3b4fd 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -878,6 +878,45 @@ static int spansion_quad_enable(struct spi_nor *nor)
return 0;
}
+static int micron_quad_enable(struct spi_nor *nor)
+{
+ int ret, val;
+
+ ret = nor->read_reg(nor, SPINOR_OP_RD_EVCR, &val, 1);
+ if (ret < 0) {
+ dev_err(nor->dev, "error %d reading EVCR\n", ret);
+ return -EINVAL;
+ }
+
+ write_enable(nor);
+
+ /* set EVCR ,enable quad I/O */
+ nor->cmd_buf[0] = val & ~EVCR_QUAD_EN_MICRON;
+ ret = nor->write_reg(nor, SPINOR_OP_WD_EVCR, nor->cmd_buf, 1, 0);
+ if (ret < 0) {
+ dev_err(nor->dev,
+ "error while writing EVCR register\n");
+ return -EINVAL;
Why not just "return ret;" ?
[...]
bpqw
2014-09-29 00:30:04 UTC
Permalink
For Micron spi norflash,you can enable Quad spi transfer by clear
EVCR(Enhanced Volatile Configuration Register) Quad I/O protocol bit.
OK, this information is nice and all, but what does this patch do? I can't learn this information from the commit message as it is, can I ?
And , the purpose of the commit message is exactly to summarize the change the patch implements.
you don't understand what purpose of this patch! just as subject and commit message described,
it is for enable Micron Quad spi transfer mode.do you read the spi-nor.c file? please pay attention
to the set_quad_mode() function.by the way,I can add more commit message for it,but I think
it is redundant,don't need.
+static int micron_quad_enable(struct spi_nor *nor) {
+ int ret, val;
+
+ ret = nor->read_reg(nor, SPINOR_OP_RD_EVCR, &val, 1);
+ if (ret < 0) {
+ dev_err(nor->dev, "error %d reading EVCR\n", ret);
+ return -EINVAL;
+ }
+
+ write_enable(nor);
+
+ /* set EVCR ,enable quad I/O */
+ nor->cmd_buf[0] = val & ~EVCR_QUAD_EN_MICRON;
+ ret = nor->write_reg(nor, SPINOR_OP_WD_EVCR, nor->cmd_buf, 1, 0);
+ if (ret < 0) {
+ dev_err(nor->dev,
+ "error while writing EVCR register\n");
+ return -EINVAL;
Why not just "return ret;" ?
[...]
Ok,this is good,I will modify it in the next patch version.thanks.
Marek Vasut
2014-09-29 18:57:31 UTC
Permalink
Post by bpqw
Post by Marek Vasut
For Micron spi norflash,you can enable Quad spi transfer by clear
EVCR(Enhanced Volatile Configuration Register) Quad I/O protocol bit.
OK, this information is nice and all, but what does this patch do? I can't
learn this information from the commit message as it is, can I ? And ,
the purpose of the commit message is exactly to summarize the change the
patch implements.
you don't understand what purpose of this patch!
Well, I dare to say, reacting to feedback like you just did won't make you many
allies around here.
Post by bpqw
just as subject and commit
message described, it is for enable Micron Quad spi transfer mode.
I understand the subject part. The commit message, on the other hand, just
states that it is possible to frob with a certain register to achieve a certain
effect ; the commit message does not state what this patch does or how is the
patch useful.

Does this patch enable the bit or does it disable the bit ? I cannot tell
without looking into the code , I really have no clue just by reading the
subject and the commit message.
Post by bpqw
do you
read the spi-nor.c file?
No, I didn't even look at the code.
Post by bpqw
please pay attention to the set_quad_mode()
function.
No, what set_quad_mode_function() are you talking about ...
Post by bpqw
by the way,I can add more commit message for it,but I think it is
redundant,don't need.
The commit message shall state what the patch does in the first place, what the
hardware can do is ortogonal to that. The commit message can be as short as:

The hardware supports 4-bit I/O when bit FOO is set in register BAR. This patch
adds function that sets bit FOO in register BAR to enable 4-bit I/O if condition
BAZ and QUUX are met.

Then I do not even have to look at the code if I want to just get the high-level
overview of what the patch does. If I want to know the details, I will look into
the code.

Do you know what I'm getting at ?

[...]

Best regards,
Marek Vasut
Bean Huo 霍斌斌 (beanhuo)
2014-09-30 02:47:39 UTC
Permalink
For Micron spi norflash,enables or disables quad I/O
protocol ,which controled by EVCR(Enhanced
Volatile Configuration Register) Quad I/O
protocol bit 7.When EVCR bit 7 is reset to 0,
the spi norflash will operate in quad I/O following
the next WRITE ENHANCED VOLATILE CONFIGURATION
command.

Signed-off-by: bean huo <***@micron.com>
---
v1-v2:modified to that capture wait_till_ready()
return value,if error,directly return its
the value.
v2-v3:directly use the reurning error value of
read_reg and write_reg,instead of -EINVAL.

drivers/mtd/spi-nor/spi-nor.c | 46 +++++++++++++++++++++++++++++++++++++++++
include/linux/mtd/spi-nor.h | 6 ++++++
2 files changed, 52 insertions(+)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index b5ad6be..486b167 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -878,6 +878,45 @@ static int spansion_quad_enable(struct spi_nor *nor)
return 0;
}

+static int micron_quad_enable(struct spi_nor *nor)
+{
+ int ret, val;
+
+ ret = nor->read_reg(nor, SPINOR_OP_RD_EVCR, &val, 1);
+ if (ret < 0) {
+ dev_err(nor->dev, "error %d reading EVCR\n", ret);
+ return ret;
+ }
+
+ write_enable(nor);
+
+ /* set EVCR ,enable quad I/O */
+ nor->cmd_buf[0] = val & ~EVCR_QUAD_EN_MICRON;
+ ret = nor->write_reg(nor, SPINOR_OP_WD_EVCR, nor->cmd_buf, 1, 0);
+ if (ret < 0) {
+ dev_err(nor->dev,
+ "error while writing EVCR register\n");
+ return ret;
+ }
+
+ ret = wait_till_ready(nor);
+ if (ret)
+ return ret;
+
+ /* read EVCR and check it */
+ ret = nor->read_reg(nor, SPINOR_OP_RD_EVCR, &val, 1);
+ if (ret < 0) {
+ dev_err(nor->dev, "error %d reading EVCR\n", ret);
+ return ret;
+ }
+ if (val & EVCR_QUAD_EN_MICRON) {
+ dev_err(nor->dev, "Micron EVCR Quad bit not clear\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int set_quad_mode(struct spi_nor *nor, u32 jedec_id)
{
int status;
@@ -890,6 +929,13 @@ static int set_quad_mode(struct spi_nor *nor, u32 jedec_id)
return -EINVAL;
}
return status;
+ case CFI_MFR_ST:
+ status = micron_quad_enable(nor);
+ if (status) {
+ dev_err(nor->dev, "Micron quad-read not enabled\n");
+ return -EINVAL;
+ }
+ return status;
default:
status = spansion_quad_enable(nor);
if (status) {
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 9e6294f..d71b659 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -56,6 +56,10 @@
/* Used for Spansion flashes only. */
#define SPINOR_OP_BRWR 0x17 /* Bank register write */

+/* Used for Micron flashes only. */
+#define SPINOR_OP_RD_EVCR 0x65 /* Read EVCR register */
+#define SPINOR_OP_WD_EVCR 0x61 /* Write EVCR register */
+
/* Status Register bits. */
#define SR_WIP 1 /* Write in progress */
#define SR_WEL 2 /* Write enable latch */
@@ -67,6 +71,8 @@

#define SR_QUAD_EN_MX 0x40 /* Macronix Quad I/O */

+#define EVCR_QUAD_EN_MICRON 0x80 /* Micron Quad I/O */
+
/* Flag Status Register bits */
#define FSR_READY 0x80
--
1.7.9.5
Marek Vasut
2014-09-30 13:38:40 UTC
Permalink
On Tuesday, September 30, 2014 at 04:47:39 AM, Bean Huo =E9=9C=8D=E6=96=
Post by Bean Huo 霍斌斌 (beanhuo)
For Micron spi norflash,enables or disables quad I/O
protocol ,which controled by EVCR(Enhanced
Volatile Configuration Register) Quad I/O
protocol bit 7.When EVCR bit 7 is reset to 0,
the spi norflash will operate in quad I/O following
the next WRITE ENHANCED VOLATILE CONFIGURATION
command.
You only do one WRITE ENHANCED VOLATILE CONFIGURATION command in the pa=
tch, so
this text doesn't add up.

Try something like this:
-->8--
This patch adds code which enables Quad I/O mode on Micron SPI NOR
flashes.

=46or Micron SPI NOR flash, enabling or disabling quad I/O protocol
is controled by EVCR (Enhanced Volatile Configuration Register),
Quad I/O protocol bit 7. When EVCR bit 7 is reset to 0, the SPI
NOR flash will operate in quad I/O mode.
--8<--

What do you think ?

Brian, am I bitching too much about pointless things ? Please stop me i=
f you=20
think I do.

[...]

Best regards,
Marek Vasut
Bean Huo 霍斌斌 (beanhuo)
2014-10-01 14:24:41 UTC
Permalink
For Micron spi norflash,enables or disables quad I/O protocol ,which
controled by EVCR(Enhanced Volatile Configuration Register) Quad I/O
protocol bit 7.When EVCR bit 7 is reset to 0, the spi norflash will
operate in quad I/O following the next WRITE ENHANCED VOLATILE
CONFIGURATION command.
You only do one WRITE ENHANCED VOLATILE CONFIGURATION command in the patch, so this text doesn't add up.
-->8--
This patch adds code which enables Quad I/O mode on Micron SPI NOR flashes.
For Micron SPI NOR flash, enabling or disabling quad I/O protocol is controled by EVCR (Enhanced Volatile Configuration Register), Quad I/O protocol bit 7. When EVCR bit 7 is reset to 0, the SPI NOR flash will operate in quad I/O mode.
--8<--
What do you think ?
Perfect,I will modify my commit message a
Marek Vasut
2014-10-01 14:32:02 UTC
Permalink
On Wednesday, October 01, 2014 at 04:24:41 PM, Bean Huo =E9=9C=8D=E6=96=
Post by Marek Vasut
For Micron spi norflash,enables or disables quad I/O protocol ,whi=
ch
Post by Marek Vasut
controled by EVCR(Enhanced Volatile Configuration Register) Quad I=
/O
Post by Marek Vasut
protocol bit 7.When EVCR bit 7 is reset to 0, the spi norflash wil=
l
Post by Marek Vasut
operate in quad I/O following the next WRITE ENHANCED VOLATILE
CONFIGURATION command.
You only do one WRITE ENHANCED VOLATILE CONFIGURATION command in the
patch, so this text doesn't add up.
-->8--
This patch adds code which enables Quad I/O mode on Micron SPI NOR flashes.
For Micron SPI NOR flash, enabling or disabling quad I/O protocol is
controled by EVCR (Enhanced Volatile Configuration Register), Quad I=
/O
Post by Marek Vasut
protocol bit 7. When EVCR bit 7 is reset to 0, the SPI NOR flash wil=
l
Post by Marek Vasut
operate in quad I/O mode. --8<--
What do you think ?
=20
Perfect,I will modify my commit message and sumbit it again.thanks.
Thank you. I didn't mean to grind you unnecessarily or be outright bitc=
h, sorry=20
if it did sound like so.

Best regards,
Marek Vasut
bpqw
2014-10-01 14:28:17 UTC
Permalink
This patch adds code which enables Quad I/O mode on Micron SPI NOR flashes.

For Micron SPI NOR flash, enabling or disabling quad I/O protocol is controlled
by EVCR (Enhanced Volatile Configuration Register), Quad I/O protocol bit 7.
When EVCR bit 7 is reset to 0, the SPI NOR flash will operate in quad I/O mode.

Signed-off-by: bean huo <***@micron.com>
---
v1-v2:modified to that capture wait_till_ready()
return value,if error,directly return its
the value.
v2-v3:directly use the reurning error value of
read_reg and write_reg,instead of -EINVAL.

drivers/mtd/spi-nor/spi-nor.c | 46 +++++++++++++++++++++++++++++++++++++++++
include/linux/mtd/spi-nor.h | 6 ++++++
2 files changed, 52 insertions(+)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index b5ad6be..486b167 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -878,6 +878,45 @@ static int spansion_quad_enable(struct spi_nor *nor)
return 0;
}

+static int micron_quad_enable(struct spi_nor *nor) {
+ int ret, val;
+
+ ret = nor->read_reg(nor, SPINOR_OP_RD_EVCR, &val, 1);
+ if (ret < 0) {
+ dev_err(nor->dev, "error %d reading EVCR\n", ret);
+ return ret;
+ }
+
+ write_enable(nor);
+
+ /* set EVCR ,enable quad I/O */
+ nor->cmd_buf[0] = val & ~EVCR_QUAD_EN_MICRON;
+ ret = nor->write_reg(nor, SPINOR_OP_WD_EVCR, nor->cmd_buf, 1, 0);
+ if (ret < 0) {
+ dev_err(nor->dev,
+ "error while writing EVCR register\n");
+ return ret;
+ }
+
+ ret = wait_till_ready(nor);
+ if (ret)
+ return ret;
+
+ /* read EVCR and check it */
+ ret = nor->read_reg(nor, SPINOR_OP_RD_EVCR, &val, 1);
+ if (ret < 0) {
+ dev_err(nor->dev, "error %d reading EVCR\n", ret);
+ return ret;
+ }
+ if (val & EVCR_QUAD_EN_MICRON) {
+ dev_err(nor->dev, "Micron EVCR Quad bit not clear\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int set_quad_mode(struct spi_nor *nor, u32 jedec_id) {
int status;
@@ -890,6 +929,13 @@ static int set_quad_mode(struct spi_nor *nor, u32 jedec_id)
return -EINVAL;
}
return status;
+ case CFI_MFR_ST:
+ status = micron_quad_enable(nor);
+ if (status) {
+ dev_err(nor->dev, "Micron quad-read not enabled\n");
+ return -EINVAL;
+ }
+ return status;
default:
status = spansion_quad_enable(nor);
if (status) {
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 9e6294f..d71b659 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -56,6 +56,10 @@
/* Used for Spansion flashes only. */
#define SPINOR_OP_BRWR 0x17 /* Bank register write */

+/* Used for Micron flashes only. */
+#define SPINOR_OP_RD_EVCR 0x65 /* Read EVCR register */
+#define SPINOR_OP_WD_EVCR 0x61 /* Write EVCR register */
+
/* Status Register bits. */
#define SR_WIP 1 /* Write in progress */
#define SR_WEL 2 /* Write enable latch */
@@ -67,6 +71,8 @@

#define SR_QUAD_EN_MX 0x40 /* Macronix Quad I/O */

+#define EVCR_QUAD_EN_MICRON 0x80 /* Micron Quad I/O */
+
/* Flag Status Register bits */
#define FSR_READY 0x80

--
1.7.9.5
Marek Vasut
2014-10-01 14:33:18 UTC
Permalink
This patch adds code which enables Quad I/O mode on Micron SPI NOR flashes.
For Micron SPI NOR flash, enabling or disabling quad I/O protocol is
controlled by EVCR (Enhanced Volatile Configuration Register), Quad I/O
protocol bit 7. When EVCR bit 7 is reset to 0, the SPI NOR flash will
operate in quad I/O mode.
I don't see anything obviously wrong.

Acked-by: Marek Vasut <***@denx.de>

Best regards,
Marek Vasut
bpqw
2014-10-04 05:55:47 UTC
Permalink
Post by Marek Vasut
I don't see anything obviously wrong.
Hi,Brian

How do you think about this patch?
bpqw
2014-10-16 01:53:09 UTC
Permalink
Hi,brian
How about this patch? And can be accepted by linux-mtd?
Bean Huo 霍斌斌 (beanhuo)
2014-10-17 00:37:26 UTC
Permalink
This patch adds code which enables Quad I/O mode on Micron SPI NOR flashes.

For Micron SPI NOR flash, enabling or disabling quad I/O protocol is controlled
by EVCR (Enhanced Volatile Configuration Register), Quad I/O protocol bit 7.
When EVCR bit 7 is reset to 0, the SPI NOR flash will operate in quad I/O mode.

Signed-off-by: bean huo <***@micron.com>
Acked-by: Marek Vasut <***@denx.de>
---
v1-v2:modified to that capture wait_till_ready()
return value,if error,directly return its
the value.
v2-v3:directly use the reurning error value of
read_reg and write_reg,instead of -EINVAL.

drivers/mtd/spi-nor/spi-nor.c | 46 +++++++++++++++++++++++++++++++++++++++++
include/linux/mtd/spi-nor.h | 6 ++++++
2 files changed, 52 insertions(+)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index b5ad6be..486b167 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -878,6 +878,45 @@ static int spansion_quad_enable(struct spi_nor *nor)
return 0;
}

+static int micron_quad_enable(struct spi_nor *nor) {
+ int ret, val;
+
+ ret = nor->read_reg(nor, SPINOR_OP_RD_EVCR, &val, 1);
+ if (ret < 0) {
+ dev_err(nor->dev, "error %d reading EVCR\n", ret);
+ return ret;
+ }
+
+ write_enable(nor);
+
+ /* set EVCR ,enable quad I/O */
+ nor->cmd_buf[0] = val & ~EVCR_QUAD_EN_MICRON;
+ ret = nor->write_reg(nor, SPINOR_OP_WD_EVCR, nor->cmd_buf, 1, 0);
+ if (ret < 0) {
+ dev_err(nor->dev,
+ "error while writing EVCR register\n");
+ return ret;
+ }
+
+ ret = wait_till_ready(nor);
+ if (ret)
+ return ret;
+
+ /* read EVCR and check it */
+ ret = nor->read_reg(nor, SPINOR_OP_RD_EVCR, &val, 1);
+ if (ret < 0) {
+ dev_err(nor->dev, "error %d reading EVCR\n", ret);
+ return ret;
+ }
+ if (val & EVCR_QUAD_EN_MICRON) {
+ dev_err(nor->dev, "Micron EVCR Quad bit not clear\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int set_quad_mode(struct spi_nor *nor, u32 jedec_id) {
int status;
@@ -890,6 +929,13 @@ static int set_quad_mode(struct spi_nor *nor, u32 jedec_id)
return -EINVAL;
}
return status;
+ case CFI_MFR_ST:
+ status = micron_quad_enable(nor);
+ if (status) {
+ dev_err(nor->dev, "Micron quad-read not enabled\n");
+ return -EINVAL;
+ }
+ return status;
default:
status = spansion_quad_enable(nor);
if (status) {
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 9e6294f..d71b659 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -56,6 +56,10 @@
/* Used for Spansion flashes only. */
#define SPINOR_OP_BRWR 0x17 /* Bank register write */

+/* Used for Micron flashes only. */
+#define SPINOR_OP_RD_EVCR 0x65 /* Read EVCR register */
+#define SPINOR_OP_WD_EVCR 0x61 /* Write EVCR register */
+
/* Status Register bits. */
#define SR_WIP 1 /* Write in progress */
#define SR_WEL 2 /* Write enable latch */
@@ -67,6 +71,8 @@

#define SR_QUAD_EN_MX 0x40 /* Macronix Quad I/O */

+#define EVCR_QUAD_EN_MICRON 0x80 /* Micron Quad I/O */
+
/* Flag Status Register bits */
#define FSR_READY 0x80

--
1.7.9.5
bpqw
2014-10-20 01:24:23 UTC
Permalink
This patch adds code which enables Quad I/O mode on Micron SPI NOR flashes.

For Micron SPI NOR flash, enabling or disabling quad I/O protocol is controlled
by EVCR (Enhanced Volatile Configuration Register), Quad I/O protocol bit 7.
When EVCR bit 7 is reset to 0, the SPI NOR flash will operate in quad I/O mode.

Signed-off-by: bean huo <***@micron.com>
Acked-by: Marek Vasut <***@denx.de>
---
v1-v2:modified to that capture wait_till_ready()
return value,if error,directly return its
the value.
v2-v3:directly use the reurning error value of
read_reg and write_reg,instead of -EINVAL.

drivers/mtd/spi-nor/spi-nor.c | 46 +++++++++++++++++++++++++++++++++++++++++
include/linux/mtd/spi-nor.h | 6 ++++++
2 files changed, 52 insertions(+)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index b5ad6be..486b167 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -878,6 +878,45 @@ static int spansion_quad_enable(struct spi_nor *nor)
return 0;
}

+static int micron_quad_enable(struct spi_nor *nor) {
+ int ret, val;
+
+ ret = nor->read_reg(nor, SPINOR_OP_RD_EVCR, &val, 1);
+ if (ret < 0) {
+ dev_err(nor->dev, "error %d reading EVCR\n", ret);
+ return ret;
+ }
+
+ write_enable(nor);
+
+ /* set EVCR ,enable quad I/O */
+ nor->cmd_buf[0] = val & ~EVCR_QUAD_EN_MICRON;
+ ret = nor->write_reg(nor, SPINOR_OP_WD_EVCR, nor->cmd_buf, 1, 0);
+ if (ret < 0) {
+ dev_err(nor->dev,
+ "error while writing EVCR register\n");
+ return ret;
+ }
+
+ ret = wait_till_ready(nor);
+ if (ret)
+ return ret;
+
+ /* read EVCR and check it */
+ ret = nor->read_reg(nor, SPINOR_OP_RD_EVCR, &val, 1);
+ if (ret < 0) {
+ dev_err(nor->dev, "error %d reading EVCR\n", ret);
+ return ret;
+ }
+ if (val & EVCR_QUAD_EN_MICRON) {
+ dev_err(nor->dev, "Micron EVCR Quad bit not clear\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int set_quad_mode(struct spi_nor *nor, u32 jedec_id) {
int status;
@@ -890,6 +929,13 @@ static int set_quad_mode(struct spi_nor *nor, u32 jedec_id)
return -EINVAL;
}
return status;
+ case CFI_MFR_ST:
+ status = micron_quad_enable(nor);
+ if (status) {
+ dev_err(nor->dev, "Micron quad-read not enabled\n");
+ return -EINVAL;
+ }
+ return status;
default:
status = spansion_quad_enable(nor);
if (status) {
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 9e6294f..d71b659 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -56,6 +56,10 @@
/* Used for Spansion flashes only. */
#define SPINOR_OP_BRWR 0x17 /* Bank register write */

+/* Used for Micron flashes only. */
+#define SPINOR_OP_RD_EVCR 0x65 /* Read EVCR register */
+#define SPINOR_OP_WD_EVCR 0x61 /* Write EVCR register */
+
/* Status Register bits. */
#define SR_WIP 1 /* Write in progress */
#define SR_WEL 2 /* Write enable latch */
@@ -67,6 +71,8 @@

#define SR_QUAD_EN_MX 0x40 /* Macronix Quad I/O */

+#define EVCR_QUAD_EN_MICRON 0x80 /* Micron Quad I/O */
+
/* Flag Status Register bits */
#define FSR_READY 0x80

--
1.7.9.5

Loading...