Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Chuanhong Guo <gch981213@gmail.com>
To:     linux-mtd@lists.infradead.org
Cc:     Chuanhong Guo <gch981213@gmail.com>,
        Jeff Kletsky <git-commits@allycomm.com>,
        Miquel Raynal <miquel.raynal@bootlin.com>,
        Richard Weinberger <richard@nod.at>,
        David Woodhouse <dwmw2@infradead.org>,
        Brian Norris <computersforpeace@gmail.com>,
        Vignesh Raghavendra <vigneshr@ti.com>,
        Frieder Schrempf <frieder.schrempf@kontron.de>,
        Boris Brezillon <bbrezillon@kernel.org>,
        Stefan Roese <sr@denx.de>, linux-kernel@vger.kernel.org
Subject: [PATCH][RFC] mtd: spinand: fix detection of GD5FxGQ4xA flash
Date:   Wed, 16 Oct 2019 09:38:24 +0800
Message-Id: <20191016013845.23508-1-gch981213@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

GD5FxGQ4xA didn't follow the SPI spec to keep MISO low while slave is
reading, and instead MISO is kept high. As a result, the first byte
of id becomes 0xFF.
Since the first byte isn't supposed to be checked at all, this patch
just removed that check.

While at it, redo the comment above to better explain what's happening.

Fixes: cfd93d7c908e ("mtd: spinand: Add support for GigaDevice GD5F1GQ4UFxxG")
Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
CC: Jeff Kletsky <git-commits@allycomm.com>
---
RFC:
I doubt whether this patch is a proper fix for the underlying problem:
The actual problem is that we have two different implementation of read id
command: One replies immediately after master sending 0x9f and the other
need to send 0x9f and an offset byte (found in winbond and early GD flashes.)
Current code only works if SPI master is properly implemented (i.e. keep MOSI
low while reading.)
I'm wondering if it worths to split the implementation of read_id into two
variants and assign corresponding ID tables to each variant, or we could
trust all SPI controllers and this fix is sufficient.

 drivers/mtd/nand/spi/gigadevice.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/spi/gigadevice.c b/drivers/mtd/nand/spi/gigadevice.c
index e99d425aa93f..ab0e53b09f0c 100644
--- a/drivers/mtd/nand/spi/gigadevice.c
+++ b/drivers/mtd/nand/spi/gigadevice.c
@@ -249,13 +249,14 @@ static int gigadevice_spinand_detect(struct spinand_device *spinand)
 	int ret;
 
 	/*
-	 * Earlier GDF5-series devices (A,E) return [0][MID][DID]
-	 * Later (F) devices return [MID][DID1][DID2]
+	 * Earlier GDF5-series devices (A,E) need sending an extra offset
+	 * byte before replying flash ID, so the first byte is undetermined.
+	 * Later (F) devices don't need that.
 	 */
 
 	if (id[0] == SPINAND_MFR_GIGADEVICE)
 		did = (id[1] << 8) + id[2];
-	else if (id[0] == 0 && id[1] == SPINAND_MFR_GIGADEVICE)
+	else if (id[1] == SPINAND_MFR_GIGADEVICE)
 		did = id[2];
 	else
 		return 0;
-- 
2.21.0