[PATCH] mtd: nand: gpmi: fix edo mode for non fully ONFI compliant flashes

[PATCH] mtd: nand: gpmi: fix edo mode for non fully ONFI compliant flashes

[Manfred Schlaegl]

In enable_edo_mode the timing mode feature is set according to previously
read capabilities of the parameter page ("Timing mode support"). After
the value was set, it is read back to provide a "double-check".
If the "double check" fails, the whole function returns with an error,
which leads to a very slow (non-edo) fallback timing.

The problem here is, that there seem to be some NAND flashes, which are
not fully ONFI 1.0 compliant.
One of these is Winbond W29N04GV. According to datasheet and parameter
page, the flash supports timing mode 4 (edo), but the timing mode feature
is simply missing.

It seems that setting a non-existing feature is simply ignored. The real
problem occurs, when the feature is read back: W29N04GV always delivers
zero, which causes the "double-check" to fail. This leads to very slow
timing and therefore to poor performance.

To solve this, we simply remove the double-check, which is a paranoia
check anyways.

The modification was intensively tested on i.MX6 with linux-4.1, Winbond
W29N04GV and Micron MT29F4G08ABADAH4.

Signed-off-by: Manfred Schlaegl <manfred.schlaegl@ginzinger.com>
---
 drivers/mtd/nand/raw/gpmi-nand/gpmi-lib.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-lib.c
index 97787246af41..40fba96df215 100644
--- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-lib.c
+++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-lib.c
@@ -939,16 +939,9 @@ static int enable_edo_mode(struct gpmi_nand_data *this, int mode)
 	if (ret)
 		goto err_out;
 
-	/* [2] send GET FEATURE command to double-check the timing mode */
-	memset(feature, 0, ONFI_SUBFEATURE_PARAM_LEN);
-	ret = nand->onfi_get_features(mtd, nand,
-				ONFI_FEATURE_ADDR_TIMING_MODE, feature);
-	if (ret || feature[0] != mode)
-		goto err_out;
-
 	nand->select_chip(mtd, -1);
 
-	/* [3] set the main IO clock, 100MHz for mode 5, 80MHz for mode 4. */
+	/* [2] set the main IO clock, 100MHz for mode 5, 80MHz for mode 4. */
 	rate = (mode == 5) ? 100000000 : 80000000;
 	clk_set_rate(r->clock[0], rate);
 
-- 
2.11.0

Re: [PATCH] mtd: nand: gpmi: fix edo mode for non fully ONFI compliant flashes

[Miquel Raynal]

Hi Manfred,

On Tue, 20 Feb 2018 09:15:33 +0100, Manfred Schlaegl
<manfred.schlaegl@ginzinger.com> wrote:

> In enable_edo_mode the timing mode feature is set according to previously
> read capabilities of the parameter page ("Timing mode support"). After
> the value was set, it is read back to provide a "double-check".
> If the "double check" fails, the whole function returns with an error,
> which leads to a very slow (non-edo) fallback timing.
> 
> The problem here is, that there seem to be some NAND flashes, which are
> not fully ONFI 1.0 compliant.
> One of these is Winbond W29N04GV. According to datasheet and parameter
> page, the flash supports timing mode 4 (edo), but the timing mode feature
> is simply missing.
> 
> It seems that setting a non-existing feature is simply ignored. The real
> problem occurs, when the feature is read back: W29N04GV always delivers
> zero, which causes the "double-check" to fail. This leads to very slow
> timing and therefore to poor performance.
> 
> To solve this, we simply remove the double-check, which is a paranoia
> check anyways.

I have been in troubles about this already and sent a first patch
to dot the exact same thing, which was (rightly) nacked by Han.

So I moved to another solution which is:
1/ Move the GPMI driver to use the standardized
->setup_data_interface() hook. This way, use the logic already laying
in the core (controller drivers should not care about that anyway).
2/ Handle differently the ONFI/JEDEC parameter page so that we only
keep the parameters we actually use (instead of the whole page)
3/ Add the possibility in vendor code to nack a feature which is
declared supported (I guess it is your case too) by editing the new
structure replacing the parameter page. This way the check won't be
trustable and thus ignored.

You can find the first versions here [1] [2] and I am currently working
on a next version right now. Look at the last patch in Macronix NAND
code, I guess you could do a similar patch for your NAND (you may wait
for the next version for that, it is subject to slightly change).

Cheers,
Miquèl

[1]
http://lists.infradead.org/pipermail/linux-mtd/2018-January/078920.html
[2]
http://lists.infradead.org/pipermail/linux-mtd/2018-February/079048.html

-- 
Miquel Raynal, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com