From: Han Xu <han.xu@nxp.com>
To: Boris Brezillon <boris.brezillon@free-electrons.com>,
        Miquel Raynal <miquel.raynal@free-electrons.com>
CC: "richard@nod.at" <richard@nod.at>,
        "linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
        "dwmw2@infradead.org" <dwmw2@infradead.org>,
        "computersforpeace@gmail.com" <computersforpeace@gmail.com>,
        "marek.vasut@gmail.com" <marek.vasut@gmail.com>,
        "cyrille.pitchen@wedev4u.fr" <cyrille.pitchen@wedev4u.fr>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] mtd: gpmi-nand: do not fail setting ONFI timing mode if
 available
Thread-Topic: [PATCH] mtd: gpmi-nand: do not fail setting ONFI timing mode if
 available
Thread-Index: AQHS/A1OBO77IVOOUEmMNKEWTMCw7KJSMWUAgAE3OAA=
Date: Fri, 14 Jul 2017 14:53:39 +0000
Message-ID: <178298b1-b482-e636-2daa-1a6595b8c139@nxp.com>
References: <20170713192030.22177-1-miquel.raynal@free-electrons.com>
 <20170713221520.268bc554@bbrezillon>
In-Reply-To: <20170713221520.268bc554@bbrezillon>
Accept-Language: en-US
Content-Language: en-US
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="utf-8"
Content-ID: <4FA9F6137B667E47859BC3267E6A73D5@eurprd04.prod.outlook.com>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Jul 2017 14:53:39.8780
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0401MB2224
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from base64 to 8bit by nfs id v6EErpm0019471
Content-Length: 2246
Lines: 58


On 07/13/2017 03:15 PM, Boris Brezillon wrote:
> Hi Miquel,
>
> Le Thu, 13 Jul 2017 21:20:30 +0200,
> Miquel Raynal <miquel.raynal@free-electrons.com> a écrit :
>
>> GPMI NFC driver fails to apply timing mode if the ->onfi_get_features()
>> does not return the mode that was previously applied.
>>
>> We can assume that a nand chip supports a timing as long as it is
>> read from the ONFI parameter page. Reading back a different mode than
>> the one previously applied does not mean the mode is unsupported but
>> that the nand chip does not implement the ONFI feature because it
>> probably does not need to.
>>
>> The output of ->onfi_get_feature() is irrelevant so delete it.
> Having the NAND part that is not supporting the get/set(timing_mode)
> feature explicitly mentioned in the commit message would help reviewers
> understand why this patch is needed.
>
> Also mention that, even though the SET/GET_FEATURES(timing_mode) is
> marked as required in the ONFI spec, this Macronix chip does not
> support it which could be considered as a bug.
>
> Regards,
>
> Boris

Yes, this is a Macronix chip bug and I have reproduced on my side, 
ignoring the GET_FEATURE checking is a workaround and the chip will 
still works in EDO mode 5, but I don't accept to remove the reasonable 
checking code for a chip bug.

>> Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
>> ---
>>   drivers/mtd/nand/gpmi-nand/gpmi-lib.c | 7 -------
>>   1 file changed, 7 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
>> index 141bd70a49c2..4d137145439c 100644
>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
>> @@ -939,13 +939,6 @@ 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. */