[PATCH] Revert "mtd: spi-nor: Add 4B_OPCODES flag to w25q256"

[PATCH] Revert "mtd: spi-nor: Add 4B_OPCODES flag to w25q256"

[Chuanhong Guo]

This reverts commit 10050a02f7d508fa88f70fcfceefbacd13488ca7.

Winbond W25Q256FV and W25Q256JV both uses 0xef4019 as JEDEC ID,
but only the latter has proper 4B_OPCODES support.
W25Q256FV has all 4B read instructions but it lacks a 4B page program
instruction, causing the entire flash to be read-only.
Disable 4B_OPCODES for W25Q256 completely.
Users can use broken-flash-reset as a temporary workaround.

Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
---
"line over 80 characters" warning produced by checkpatch.pl isn't
fixed because I think a revert commit should bring a file back to
what it was before.
I don't have a w25q256jv available and can't compare SFDP table
to create a fix similar to mx25l25635 one.

 drivers/mtd/spi-nor/winbond.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
index 17deabad57e1..9673ec7fa003 100644
--- a/drivers/mtd/spi-nor/winbond.c
+++ b/drivers/mtd/spi-nor/winbond.c
@@ -52,9 +52,7 @@ static const struct flash_info winbond_parts[] = {
 	{ "w25q80", INFO(0xef5014, 0, 64 * 1024,  16, SECT_4K) },
 	{ "w25q80bl", INFO(0xef4014, 0, 64 * 1024,  16, SECT_4K) },
 	{ "w25q128", INFO(0xef4018, 0, 64 * 1024, 256, SECT_4K) },
-	{ "w25q256", INFO(0xef4019, 0, 64 * 1024, 512,
-			  SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
-			  SPI_NOR_4B_OPCODES) },
+	{ "w25q256", INFO(0xef4019, 0, 64 * 1024, 512, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 	{ "w25q256jvm", INFO(0xef7019, 0, 64 * 1024, 512,
 			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 	{ "w25q256jw", INFO(0xef6019, 0, 64 * 1024, 512,
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] Revert "mtd: spi-nor: Add 4B_OPCODES flag to w25q256"

[Chuanhong Guo]

Hi Robert!

On Sat, Apr 4, 2020 at 9:01 PM Chuanhong Guo <gch981213@gmail.com> wrote:
> "line over 80 characters" warning produced by checkpatch.pl isn't
> fixed because I think a revert commit should bring a file back to
> what it was before.
> I don't have a w25q256jv available and can't compare SFDP table
> to create a fix similar to mx25l25635 one.

I just tried and unable to dump SFDP on my W25Q256FV,
probably because my chip is too old to have one.
Could you check if your W25Q256JV has this and dump it?
Just add some prints in spi_nor_read_sfdp.
If a 4-byte address instruction table is present, current kernel
should be able to discover 4B_OPCODES support automatically.
Even if that's not the case we may still be able to distinguish
W25Q256FV and W25Q256JV using SFDP table.

-- 
Regards,
Chuanhong Guo

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] Revert "mtd: spi-nor: Add 4B_OPCODES flag to w25q256"

[Robert Marko]

[-- Attachment #1: Type: text/plain, Size: 1580 bytes --]

On Mon, 6 Apr 2020 at 07:18, Chuanhong Guo <gch981213@gmail.com> wrote:
>
> Hi Robert!
>
> On Sat, Apr 4, 2020 at 9:01 PM Chuanhong Guo <gch981213@gmail.com> wrote:
> > "line over 80 characters" warning produced by checkpatch.pl isn't
> > fixed because I think a revert commit should bring a file back to
> > what it was before.
> > I don't have a w25q256jv available and can't compare SFDP table
> > to create a fix similar to mx25l25635 one.
>
> I just tried and unable to dump SFDP on my W25Q256FV,
> probably because my chip is too old to have one.
> Could you check if your W25Q256JV has this and dump it?
> Just add some prints in spi_nor_read_sfdp.
> If a 4-byte address instruction table is present, current kernel
> should be able to discover 4B_OPCODES support automatically.
> Even if that's not the case we may still be able to distinguish
> W25Q256FV and W25Q256JV using SFDP table.

It appears that W25Q256JV has an SFDP table and in it advertises 3B or 4B modes.
>
> [    1.957903] spi_qup 78b5000.spi: IN:block:16, fifo:64, OUT:block:16, fifo:64
> [    1.962185] SFDP advertises 3B or 4B
> [    1.977393] spi-nor spi0.0: w25q256 (32768 Kbytes)
>
I have used the attached patch to check what does the SFDP DWORD 1 advertises.
If FV version has or does not advertise 4B support than that can be
used to differentiate them.
Can you apply this patch and check what the FV version advertises as I
don't have a device using that revision.
FV version also should have SFDP as datasheet for it clearly advertises is.

Best regards
Robert
>
> --
> Regards,
> Chuanhong Guo

[-- Attachment #2: 999-spi-nor-w25q256-sfdp.patch --]
[-- Type: text/x-patch, Size: 2684 bytes --]

From b2992b8ec607dc6704eb9b8da9a937894a406d85 Mon Sep 17 00:00:00 2001
From: Robert Marko <robert.marko@sartura.hr>
Date: Mon, 6 Apr 2020 13:52:07 +0200
Subject: [PATCH] spi-nor: w25q256 sfdp

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
 drivers/mtd/spi-nor/spi-nor.c | 37 ++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index f417fb680cd8..09a4a8bce07f 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2143,6 +2143,40 @@ static struct spi_nor_fixups gd25q256_fixups = {
 	.default_init = gd25q256_default_init,
 };
 
+static int
+w25q256_post_bfpt_fixups(struct spi_nor *nor,
+			    const struct sfdp_parameter_header *bfpt_header,
+			    const struct sfdp_bfpt *bfpt,
+			    struct spi_nor_flash_parameter *params)
+{
+	/*
+	 * W25Q256JV fully supports 4B opcodes but W25Q256FV 4B page program
+	 * instruction, causing the entire flash to be read-only.
+	 * Unfortunately, Winbond has re-used the same JEDEC ID for both
+	 * variants which prevents us from defining a new entry in the parts
+	 * table.
+	 * We need a way to differentiate W25Q256JV and W25Q256FV.
+	 */
+
+	if ((bfpt->dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) ==
+		BFPT_DWORD1_ADDRESS_BYTES_3_ONLY)
+		pr_warn("SFDP advertises 3B only\n");
+
+	if ((bfpt->dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) ==
+		BFPT_DWORD1_ADDRESS_BYTES_3_OR_4)
+		pr_warn("SFDP advertises 3B or 4B\n");
+
+	if ((bfpt->dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) ==
+		BFPT_DWORD1_ADDRESS_BYTES_4_ONLY)
+		pr_warn("SFDP advertises 4B only\n");
+
+	return 0;
+}
+
+static struct spi_nor_fixups w25q256_fixups = {
+	.post_bfpt = w25q256_post_bfpt_fixups,
+};
+
 /* NOTE: double check command sets and memory organization when you add
  * more nor chips.  This current list focusses on newer chips, which
  * have been converging on command sets which including JEDEC ID.
@@ -2480,7 +2514,8 @@ static const struct flash_info spi_nor_ids[] = {
 	{ "w25q80", INFO(0xef5014, 0, 64 * 1024,  16, SECT_4K) },
 	{ "w25q80bl", INFO(0xef4014, 0, 64 * 1024,  16, SECT_4K) },
 	{ "w25q128", INFO(0xef4018, 0, 64 * 1024, 256, SECT_4K) },
-	{ "w25q256", INFO(0xef4019, 0, 64 * 1024, 512, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+	{ "w25q256", INFO(0xef4019, 0, 64 * 1024, 512, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
+				.fixups = &w25q256_fixups, },
 	{ "w25q256jvm", INFO(0xef7019, 0, 64 * 1024, 512,
 			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 	{ "w25m512jv", INFO(0xef7119, 0, 64 * 1024, 1024,
-- 
2.26.0


[-- Attachment #3: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] Revert "mtd: spi-nor: Add 4B_OPCODES flag to w25q256"

[Chuanhong Guo]

[-- Attachment #1: Type: text/plain, Size: 1086 bytes --]

Hi!

On Mon, Apr 6, 2020 at 8:23 PM Robert Marko <robimarko@gmail.com> wrote:
>
> It appears that W25Q256JV has an SFDP table and in it advertises 3B or 4B modes.
> >
> > [    1.957903] spi_qup 78b5000.spi: IN:block:16, fifo:64, OUT:block:16, fifo:64
> > [    1.962185] SFDP advertises 3B or 4B
> > [    1.977393] spi-nor spi0.0: w25q256 (32768 Kbytes)
> >
> I have used the attached patch to check what does the SFDP DWORD 1 advertises.
> If FV version has or does not advertise 4B support than that can be
> used to differentiate them.

My old w25q256fv spits all 0xFF to 0x5a read sfdp instruction.
I've asked someone with a newer w25q256fv to dump the entire SFDP
for me and it's in the attachment. You could do a comparison between
w25q256jv with this dump.

> Can you apply this patch and check what the FV version advertises as I
> don't have a device using that revision.
> FV version also should have SFDP as datasheet for it clearly advertises is.

I've checked the sfdp dump in the attachment and it's also advertising
3B or 4B in 1st BFPT dword.

-- 
Regards,
Chuanhong Guo

[-- Attachment #2: sfdp-w25q256fvem-wson8.bin --]
[-- Type: application/octet-stream, Size: 256 bytes --]

[-- Attachment #3: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] Revert "mtd: spi-nor: Add 4B_OPCODES flag to w25q256"

[Tudor.Ambarus]

Hi, Robert, Chuanhong,

On Monday, April 6, 2020 3:23:44 PM EEST Robert Marko wrote:
> > > I don't have a w25q256jv available and can't compare SFDP table
> > > to create a fix similar to mx25l25635 one.
> > 
> > I just tried and unable to dump SFDP on my W25Q256FV,
> > probably because my chip is too old to have one.
> > Could you check if your W25Q256JV has this and dump it?
> > Just add some prints in spi_nor_read_sfdp.
> > If a 4-byte address instruction table is present, current kernel
> > should be able to discover 4B_OPCODES support automatically.
> > Even if that's not the case we may still be able to distinguish
> > W25Q256FV and W25Q256JV using SFDP table.
> 
> It appears that W25Q256JV has an SFDP table and in it advertises 3B or 4B
> modes.

Mantas tried a fix for this, see it at:
https://patchwork.ozlabs.org/project/linux-mtd/patch/1586958510-24012-1-git-send-email-mantas@8devices.com/

Would you please check the thread?

Cheers,
ta



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/