Messages in this thread | | | From | Han Xu <> | Subject | Re: [PATCH] mtd: gpmi-nand: do not fail setting ONFI timing mode if available | Date | Fri, 14 Jul 2017 14:53:39 +0000 |
| |
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. */
| |