On 20/06/18 19:54, Boris Brezillon wrote: > On Wed, 20 Jun 2018 17:05:43 +1200 > Chris Packham <chris.pack...@alliedtelesis.co.nz> wrote: > >> Some Micron NAND chips (MT29F1G08ABAFAWP-ITE:F) report 00 00 for the >> revision number field of the ONFI parameter page. Rather than rejecting >> these outright assume ONFI version 1.0 if the revision number is 00 00. >> >> Reviewed-by: Boris Brezillon <boris.brezil...@bootlin.com> >> Signed-off-by: Chris Packham <chris.pack...@alliedtelesis.co.nz> >> --- >> This is now qualified on vendor == MICRON. I haven't qualified this >> based on specific chips the ABAFA (id=d1) and ABBFA (id=a1) variants are >> documented to have this behaviour. >> >> Changes in v2: >> - use fixup_onfi_param_page >> Changes in v3: >> - add code comment next to workaround >> >> drivers/mtd/nand/raw/nand_micron.c | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/drivers/mtd/nand/raw/nand_micron.c >> b/drivers/mtd/nand/raw/nand_micron.c >> index 5ec4c90a637d..5cec79372181 100644 >> --- a/drivers/mtd/nand/raw/nand_micron.c >> +++ b/drivers/mtd/nand/raw/nand_micron.c >> @@ -289,6 +289,19 @@ static int micron_nand_init(struct nand_chip *chip) >> return 0; >> } >> >> +static void micron_fixup_onfi_param_page(struct nand_chip *chip, >> + struct nand_onfi_params *p) >> +{ >> + /* >> + * MT29F1G08ABAFAWP-ITE:F and possibly others report 00 00 for the >> + * revision number field of the ONFI parameter page. Assume ONFI >> + * version 1.0 if the revision number is 00 00. >> + */ >> + if (le16_to_cpu(p->revision) == 0) >> + p->revision = cpu_to_le16(1 << 1); > > Would be better to have macros defining all version numbers in rawnand.h > > #define ONFI_VERSION_1_0 BIT(1) > #define ONFI_VERSION_2_0 BIT(2) > #define ONFI_VERSION_2_1 BIT(3) > #define ONFI_VERSION_2_2 BIT(4) > #define ONFI_VERSION_2_3 BIT(5) > #define ONFI_VERSION_3_0 BIT(6) > #define ONFI_VERSION_3_1 BIT(7) > #define ONFI_VERSION_3_2 BIT(8) > #define ONFI_VERSION_4_0 BIT(9) >
Make sense. Do you want me to do so in a v4 or send a new patch and clean up the other uses? >> +} >> + >> const struct nand_manufacturer_ops micron_nand_manuf_ops = { >> .init = micron_nand_init, >> + .fixup_onfi_param_page = micron_fixup_onfi_param_page, >> }; > >