4byte addressing mode on Micron n25q256a vs write enable

[Elie De Brauwer]

Hello,

I pulled a the " m25p80: Micron SPI uses Macronix-style 4-byte
addressing" from Linux-next (see (1) and (2) ) and applied it on my
3.9 tree in order to get a Micron n25q256a (N25Q256A13ESF40F to be
complete) functional on custom board with an i.mx28. The patch came
close to get this combination functional but it lacked something. If
we look at the n25q256a datasheet (3) page 68 contains in paragraph
"ENTER or EXIT 4-BYTE ADDRESS MODE Command" the following:

<quote>
To enter or exit the 4-byte address mode, the WRITE ENABLE command
must be executed to set the write enable latch bit to 1. (Note: The
WRITE ENABLE command must NOT be executed on the N25Q256A83ESF40x and
N25Q256A83E1240x devices.) S# must be driven LOW. The effect of the
command is immediate; after the command has been executed, the write
enable latch bit is cleared to 0.
</quote>

So with just your patch (1), (2) applied:

switch (JEDEC_MFR(jedec_id)) {
case CFI_MFR_MACRONIX:
+ case CFI_MFR_ST: /* Micron, actually */
case 0xEF /* winbond */:

The result is that the SPI flash does not enter 4byte mode. If I
however rework this patch as:

switch (JEDEC_MFR(jedec_id)) {
+ case CFI_MFR_ST: /* Micron, actually */
+ write_enable(flash);
case CFI_MFR_MACRONIX:
case 0xEF /* winbond */:

It just-seems-to-work, but I have no idea how general this patch is
for other Micron devices which support the 4byte mode.

my 2 cents.
E.

References:
(1) https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/mtd/devices/m25p80.c?id=7b9497fcaaceb84acbb62e80e04eba2e36ee5ed4
(2) http://patchwork.ozlabs.org/patch/266130/
(3) http://www.micron.com/~/media/Documents/Products/Data%20Sheet/NOR%20Flash/Serial%20NOR/N25Q/n25q_256mb_3v_65nm.pdf

[Brian Norris]

Hi Elie,

Post by Elie De Brauwer
Hello,
I pulled a the " m25p80: Micron SPI uses Macronix-style 4-byte
addressing" from Linux-next (see (1) and (2) ) and applied it on my
3.9 tree in order to get a Micron n25q256a (N25Q256A13ESF40F to be
complete) functional on custom board with an i.mx28. The patch came
close to get this combination functional but it lacked something. If
we look at the n25q256a datasheet (3) page 68 contains in paragraph
<quote>
To enter or exit the 4-byte address mode, the WRITE ENABLE command
must be executed to set the write enable latch bit to 1. (Note: The
WRITE ENABLE command must NOT be executed on the N25Q256A83ESF40x and
N25Q256A83E1240x devices.) S# must be driven LOW. The effect of the
command is immediate; after the command has been executed, the write
enable latch bit is cleared to 0.
</quote>
switch (JEDEC_MFR(jedec_id)) {
+ case CFI_MFR_ST: /* Micron, actually */
The result is that the SPI flash does not enter 4byte mode. If I
switch (JEDEC_MFR(jedec_id)) {
+ case CFI_MFR_ST: /* Micron, actually */
+ write_enable(flash);
It just-seems-to-work, but I have no idea how general this patch is
for other Micron devices which support the 4byte mode.

Hmm, that's interesting. I overlooked the note about WRITE ENABLE, and
it so happens I am working on the N25Q256A83ESF40x, so I am happily
oblivious.

Heeding the note in the quote there, though, I would hesitate to apply
WRITE ENABLE to all Micron 4-byte addressed flash. I am contacting
Micron about this unusual inconsistency, since it's not very easy to
programmatically discover which Micron flash need WRITE ENABLE and which
don't.

I'll try to get around to testing your patch on my N25Q256A83ESF40x
sometime, just to see what happens when I disobey the datasheet.

Post by Elie De Brauwer
my 2 cents.
E.
(1) https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/mtd/devices/m25p80.c?id=7b9497fcaaceb84acbb62e80e04eba2e36ee5ed4
(2) http://patchwork.ozlabs.org/patch/266130/
(3) http://www.micron.com/~/media/Documents/Products/Data%20Sheet/NOR%20Flash/Serial%20NOR/N25Q/n25q_256mb_3v_65nm.pdf

Thanks for the detailed report.

Brian

[Brian Norris]

Hi Elie,

Post by Brian Norris

Post by Elie De Brauwer
I pulled a the " m25p80: Micron SPI uses Macronix-style 4-byte
addressing" from Linux-next (see (1) and (2) ) and applied it on my
3.9 tree in order to get a Micron n25q256a (N25Q256A13ESF40F to be
complete) functional on custom board with an i.mx28. The patch came
close to get this combination functional but it lacked something. If
we look at the n25q256a datasheet (3) page 68 contains in paragraph
<quote>
To enter or exit the 4-byte address mode, the WRITE ENABLE command
must be executed to set the write enable latch bit to 1. (Note: The
WRITE ENABLE command must NOT be executed on the N25Q256A83ESF40x and
N25Q256A83E1240x devices.) S# must be driven LOW. The effect of the
command is immediate; after the command has been executed, the write
enable latch bit is cleared to 0.
</quote>
switch (JEDEC_MFR(jedec_id)) {
+ case CFI_MFR_ST: /* Micron, actually */
The result is that the SPI flash does not enter 4byte mode. If I
switch (JEDEC_MFR(jedec_id)) {
+ case CFI_MFR_ST: /* Micron, actually */
+ write_enable(flash);
It just-seems-to-work, but I have no idea how general this patch is
for other Micron devices which support the 4byte mode.

Hmm, that's interesting. I overlooked the note about WRITE ENABLE, and
it so happens I am working on the N25Q256A83ESF40x, so I am happily
oblivious.
Heeding the note in the quote there, though, I would hesitate to apply
WRITE ENABLE to all Micron 4-byte addressed flash. I am contacting
Micron about this unusual inconsistency, since it's not very easy to
programmatically discover which Micron flash need WRITE ENABLE and which
don't.

I have some word back from Micron. Apparently the A8 parts were made to
be compatible with Macronix, which didn't need the WREN command. They
will still accept the command fine, but it's not required.

They recommended that if we want to be generic to all types of Micron
flash, we should do the following sequence (to avoid leaving the flash
in a write-able state):

WREN
EN4B
WRDI

So feel free to send a patch that does this sequence for all Micron SPI
flash.

Thanks,
Brian

[Elie De Brauwer]

According to the datasheet for Micron n25q256a (N25Q256A13ESF40F) 4-byte
addressing mode should be entered as follows:

<quote>
To enter or exit the 4-byte address mode, the WRITE ENABLE command
must be executed to set the write enable latch bit to 1. (Note: The
WRITE ENABLE command must NOT be executed on the N25Q256A83ESF40x and
N25Q256A83E1240x devices.) S# must be driven LOW. The effect of the
command is immediate; after the command has been executed, the write
enable latch bit is cleared to 0.
</quote>

Micron's portable way to perform this for all types of Micron flash
is to first issue a write enable, then switch the addressing mode
followed by a write disable to avoid leaving the flash in a write-
able state.
---
drivers/mtd/devices/m25p80.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 26b14f9..272d483 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -169,8 +169,16 @@ static inline int write_disable(struct m25p *flash)
static inline int set_4byte(struct m25p *flash, u32 jedec_id, int enable)
{
switch (JEDEC_MFR(jedec_id)) {
- case CFI_MFR_MACRONIX:
case CFI_MFR_ST: /* Micron, actually */
+ {
+ int status;
+ write_enable(flash);
+ flash->command[0] = enable ? OPCODE_EN4B : OPCODE_EX4B;
+ status = spi_write(flash->spi, flash->command, 1);
+ write_disable(flash);
+ return status;
+ }
+ case CFI_MFR_MACRONIX:
case 0xEF /* winbond */:
flash->command[0] = enable ? OPCODE_EN4B : OPCODE_EX4B;
return spi_write(flash->spi, flash->command, 1);

[Brian Norris]

Post by Elie De Brauwer
According to the datasheet for Micron n25q256a (N25Q256A13ESF40F) 4-byte
<quote>
To enter or exit the 4-byte address mode, the WRITE ENABLE command
must be executed to set the write enable latch bit to 1. (Note: The
WRITE ENABLE command must NOT be executed on the N25Q256A83ESF40x and
N25Q256A83E1240x devices.) S# must be driven LOW. The effect of the
command is immediate; after the command has been executed, the write
enable latch bit is cleared to 0.
</quote>
Micron's portable way to perform this for all types of Micron flash
is to first issue a write enable, then switch the addressing mode
followed by a write disable to avoid leaving the flash in a write-
able state.

I just noticed, you're missing a

"Signed-off-by: Your Name <your at email.com>"

If you reply with a line like that, I can past it into the commit.

Post by Elie De Brauwer
---
drivers/mtd/devices/m25p80.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 26b14f9..272d483 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -169,8 +169,16 @@ static inline int write_disable(struct m25p *flash)
static inline int set_4byte(struct m25p *flash, u32 jedec_id, int enable)
{
switch (JEDEC_MFR(jedec_id)) {
case CFI_MFR_ST: /* Micron, actually */
+ {
+ int status;
+ write_enable(flash);
+ flash->command[0] = enable ? OPCODE_EN4B : OPCODE_EX4B;
+ status = spi_write(flash->spi, flash->command, 1);
+ write_disable(flash);
+ return status;
+ }
flash->command[0] = enable ? OPCODE_EN4B : OPCODE_EX4B;
return spi_write(flash->spi, flash->command, 1);

I personally don't like the code duplication from the Macronix and
Winbond cases, and the extra context braces may not be needed. How about
the following? If it's OK, I'll just squash your description and my
patch.

Brian

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 26b14f9..6bc9618 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -168,12 +168,25 @@ static inline int write_disable(struct m25p *flash)
*/
static inline int set_4byte(struct m25p *flash, u32 jedec_id, int enable)
{
+ int status;
+ bool need_wren = false;
+
switch (JEDEC_MFR(jedec_id)) {
- case CFI_MFR_MACRONIX:
case CFI_MFR_ST: /* Micron, actually */
+ /* Some Micron need WREN command; all will accept it */
+ need_wren = true;
+ case CFI_MFR_MACRONIX:
case 0xEF /* winbond */:
+ if (need_wren)
+ write_enable(flash);
+
flash->command[0] = enable ? OPCODE_EN4B : OPCODE_EX4B;
- return spi_write(flash->spi, flash->command, 1);
+ status = spi_write(flash->spi, flash->command, 1);
+
+ if (need_wren)
+ write_disable(flash);
+
+ return status;
default:
/* Spansion style */
flash->command[0] = OPCODE_BRWR;

[Brian Norris]

Re-include linux-mtd

Whoops, forgot to actually include linux-mtd!

On Thu, Sep 19, 2013 at 7:25 AM, Brian Norris

Post by Brian Norris

Post by Elie De Brauwer
According to the datasheet for Micron n25q256a (N25Q256A13ESF40F) 4-byte
<quote>
To enter or exit the 4-byte address mode, the WRITE ENABLE command
must be executed to set the write enable latch bit to 1. (Note: The
WRITE ENABLE command must NOT be executed on the N25Q256A83ESF40x and
N25Q256A83E1240x devices.) S# must be driven LOW. The effect of the
command is immediate; after the command has been executed, the write
enable latch bit is cleared to 0.
</quote>
Micron's portable way to perform this for all types of Micron flash
is to first issue a write enable, then switch the addressing mode
followed by a write disable to avoid leaving the flash in a write-
able state.

BTW, I just tested this myself on my N25Q256A83ESF40F, and the extra
write-enable/write-disable doesn't seem to cause problems.

Post by Brian Norris
I just noticed, you're missing a
"Signed-off-by: Your Name <your at email.com>"

As did your Cc: linux-mtd at infradead.org ;)

Signed-off-by: Elie De Brauwer <eliedebrauwer at email.com>

Post by Brian Norris
If you reply with a line like that, I can past it into the commit.

Post by Elie De Brauwer
---
drivers/mtd/devices/m25p80.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 26b14f9..272d483 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -169,8 +169,16 @@ static inline int write_disable(struct m25p *flash)
static inline int set_4byte(struct m25p *flash, u32 jedec_id, int enable)
{
switch (JEDEC_MFR(jedec_id)) {
case CFI_MFR_ST: /* Micron, actually */
+ {
+ int status;
+ write_enable(flash);
+ flash->command[0] = enable ? OPCODE_EN4B : OPCODE_EX4B;
+ status = spi_write(flash->spi, flash->command, 1);
+ write_disable(flash);
+ return status;
+ }
flash->command[0] = enable ? OPCODE_EN4B : OPCODE_EX4B;
return spi_write(flash->spi, flash->command, 1);

I personally don't like the code duplication from the Macronix and
Winbond cases, and the extra context braces may not be needed. How about
the following? If it's OK, I'll just squash your description and my
patch.

Fine with me, squash em !.

Squashed and pushed to l2-mtd.git. Thanks!
Brian