Re: [PATCH v3 3/4] mtd: rawnand: micron: add fixup for ONFI revision

[Boris Brezillon <boris.brezillon@bootlin.com>]

On Wed, 20 Jun 2018 21:12:02 +0000
Chris Packham <Chris.Packham@alliedtelesis.co.nz> wrote:

> On 20/06/18 19:54, Boris Brezillon wrote:
> > On Wed, 20 Jun 2018 17:05:43 +1200
> > Chris Packham <chris.packham@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.brezillon@bootlin.com>
> >> Signed-off-by: Chris Packham <chris.packham@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?

It can be part of your v4, but yes, the idea is to first introduce the
macros and patch existing users, and then use it in this patch.