Re: mtd: nand: atmel: probe of Spansion S34ML02G1 fails

[Boris Brezillon <boris.brezillon@free-electrons.com>]

Hi Alexander,

Le Tue, 25 Jul 2017 11:26:33 +0200,
Alexander Dahl <ada@thorsis.com> a écrit :

> Hello,
> 
> when testing the recent v4.13-rc2 on our at91sam9g20 based platform I 
> discovered one of the NAND flash chips we use can not be setup correctly 
> anymore. I guess this may be due to the reworked atmel nand driver after 
> v4.9, but it may be another problem.
> 
> The board layout is similar to the at91sam9g20ek for the NAND flash part 
> and we're using different flash chips. With an hynix HY27UF082G2B 
> everything works, the console output on boot is like this:
> 
>   nand: Could not find valid ONFI parameter page; aborting
>   nand: device found, Manufacturer ID: 0xad, Chip ID: 0xda
>   nand: Hynix NAND 256MiB 3,3V 8-bit
>   nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
> 
> The same board with another pin compatible flash chip, the Spansion 
> S34ML02G100TF100 however fails. I added some debug print statements to a 
> otherwise clean 4.13-rc2 kernel and get this:
> 
>   nand: device found, Manufacturer ID: 0x01, Chip ID: 0xda
>   nand: AMD/Spansion S34ML02G1
>   nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
>   nand: onfi_version: 10, onfi_params.opt_cmd: 0x001B
>   nand: chip->onfi_set_features() failed: -22
>   nand: nand_setup_data_interface() failed: -22
>   atmel-nand-controller 10000000.ebi:nand-controller: nand_scan_ident() 
> failed: -22
>   atmel-nand-controller: probe of 10000000.ebi:nand-controller failed 
> with error -22
> 
> So in the ONFI parameter page of the flash chip the value for "optional 
> commands supported" in the features block is 0x001B which is exactly 
> what the datasheet says. This means this flash chip does not support 
> "Get Features" and "Set Features".
> 
> Now nand_onfi_set_features() correctly fails, because the nand chip can 
> not set features. It's called in nand_setup_data_interface() in 
> nand_base.c which fails itself because of the return value of the failed 
> set features call and so the whole chain of fails starts.

Oh crap! We didn't consider ONFI NANDs which are supporting the SET/GET
FEATURES operations when working on the timing selection logic.

> 
> The nand chip actually works fine in U-Boot, and with kernels up to 
> v4.9.35, I didn't test v4.10, v4.11, but IIRC the board booted from nand 
> flash with v4.12.
> 
> Can anyone help me to fix this?

Can you try with the following fix? 

Thanks,

Boris

> 
> Greets
> Alex
> 
--->8---
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 5fa5ddc94834..fc897f799def 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -1125,7 +1125,9 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
         * Ensure the timing mode has been changed on the chip side
         * before changing timings on the controller side.
         */
-       if (chip->onfi_version) {
+       if (chip->onfi_version &&
+           (le16_to_cpu(chip->onfi_params.opt_cmd) &
+            ONFI_OPT_CMD_SET_GET_FEATURES)) {
                u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = {
                        chip->onfi_timing_mode_default,
                };