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

[Robert Marko]

On Mon, 6 Apr 2020 at 07:18, Chuanhong Guo <gch981213@xxxxxxxxx> wrote:
>
> Hi Robert!
>
> On Sat, Apr 4, 2020 at 9:01 PM Chuanhong Guo <gch981213@xxxxxxxxx> 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
From b2992b8ec607dc6704eb9b8da9a937894a406d85 Mon Sep 17 00:00:00 2001
From: Robert Marko <robert.marko@xxxxxxxxxx>
Date: Mon, 6 Apr 2020 13:52:07 +0200
Subject: [PATCH] spi-nor: w25q256 sfdp

Signed-off-by: Robert Marko <robert.marko@xxxxxxxxxx>
---
 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