Re: [EXT] [PATCH] mtd: spinand: micron: add support for MT29F1G01AAADD

[Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>]

On Mon, 18 Nov 2019 15:09:51 +0100
Marco Felsch <m.felsch@xxxxxxxxxxxxxx> wrote:

> Hi Shiva,
> 
> On 19-11-18 10:16, Shivamurthy Shastri (sshivamurthy) wrote:
> > Hi Marco,
> >   
> > > 
> > > The MT29F1G01AAADD is a single die, SLC based SPI NAND. It has a
> > > capacity of 1Gb and supports 4-bit ECC. The datasheet can be found [1].
> > > 
> > > Unfortunatly the linked device is marked as EoL, but I will expect that
> > > the MT29F1G01AAADDH4-ITX behaves the same way.
> > > 
> > > [1]
> > > https://nam01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdata
> > > sheet.octopart.com%2FMT29F1G01AAADDH4-IT%3AD-Micron-datasheet-
> > > 11572380.pdf&amp;data=02%7C01%7Csshivamurthy%40micron.com%7C21a
> > > da5347828461980a408d7642021a9%7Cf38a5ecd28134862b11bac1d563c806f%
> > > 7C0%7C1%7C637087961499818902&amp;sdata=%2Fh%2FHfUoSnl8qqSVClVfp
> > > ykvi3UiDEZFTn%2BVCsAf9IaM%3D&amp;reserved=0
> > > 
> > > Cc: Peter Pan <peterpandong@xxxxxxxxxx>
> > > Cc: sshivamurthy@xxxxxxxxxx
> > > Signed-off-by: Marco Felsch <m.felsch@xxxxxxxxxxxxxx>
> > > ---
> > > v2:
> > > - Convert 0x10 into 16 for ooblayout description
> > > - Don't break web link within commit message
> > > 
> > >  drivers/mtd/nand/spi/micron.c | 68
> > > +++++++++++++++++++++++++++++++++++
> > >  1 file changed, 68 insertions(+)
> > > 
> > > diff --git a/drivers/mtd/nand/spi/micron.c b/drivers/mtd/nand/spi/micron.c
> > > index 7d7b1f7fcf71..70e278759bd3 100644
> > > --- a/drivers/mtd/nand/spi/micron.c
> > > +++ b/drivers/mtd/nand/spi/micron.c
> > > @@ -34,6 +34,18 @@ static
> > > SPINAND_OP_VARIANTS(update_cache_variants,
> > >  		SPINAND_PROG_LOAD_X4(false, 0, NULL, 0),
> > >  		SPINAND_PROG_LOAD(false, 0, NULL, 0));
> > > 
> > > +static SPINAND_OP_VARIANTS(read_cache_variants_mt29f1g01aaadd,
> > > +		SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
> > > +		SPINAND_PAGE_READ_FROM_CACHE_X2_OP(0, 1, NULL, 0),
> > > +		SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL,
> > > 0),
> > > +		SPINAND_PAGE_READ_FROM_CACHE_OP(false, 0, 1, NULL,
> > > 0));
> > > +
> > > +static SPINAND_OP_VARIANTS(write_cache_variants_mt29f1g01aaadd,
> > > +		SPINAND_PROG_LOAD(true, 0, NULL, 0));
> > > +
> > > +static SPINAND_OP_VARIANTS(update_cache_variants_mt29f1g01aaadd,
> > > +		SPINAND_PROG_LOAD(false, 0, NULL, 0));
> > > +
> > >  static int mt29f2g01abagd_ooblayout_ecc(struct mtd_info *mtd, int section,
> > >  					struct mtd_oob_region *region)
> > >  {
> > > @@ -90,6 +102,52 @@ static int mt29f2g01abagd_ecc_get_status(struct
> > > spinand_device *spinand,
> > >  	return -EINVAL;
> > >  }
> > > 
> > > +static int mt29f1g01aaadd_ooblayout_ecc(struct mtd_info *mtd, int
> > > section,
> > > +					struct mtd_oob_region *region)
> > > +{
> > > +	if (section > 3)
> > > +		return -ERANGE;
> > > +
> > > +	region->offset = (section * 16) + 8;
> > > +	region->length = 8;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int mt29f1g01aaadd_ooblayout_free(struct mtd_info *mtd, int
> > > section,
> > > +					 struct mtd_oob_region *region)
> > > +{
> > > +	if (section > 3)
> > > +		return -ERANGE;
> > > +
> > > +	/* 2 bytes for the BBM + 2 bytes to skip non-ecc memory */
> > > +	region->offset = (section * 16) + 4;
> > > +	region->length = 4;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static const struct mtd_ooblayout_ops mt29f1g01aaadd_ooblayout = {
> > > +	.ecc = mt29f1g01aaadd_ooblayout_ecc,
> > > +	.free = mt29f1g01aaadd_ooblayout_free,
> > > +};
> > > +
> > > +static int mt29f1g01aaadd_ecc_get_status(struct spinand_device *spinand,
> > > +					 u8 status)
> > > +{
> > > +	switch (status & STATUS_ECC_MASK) {
> > > +	case STATUS_ECC_NO_BITFLIPS:
> > > +		return 0;
> > > +	case STATUS_ECC_HAS_BITFLIPS:
> > > +		/* 1 to 4-bit error detected and corrected */
> > > +		return 4;
> > > +	case STATUS_ECC_UNCOR_ERROR:
> > > +		return -EBADMSG;
> > > +	default:
> > > +		return -EINVAL;
> > > +	}
> > > +}
> > > +
> > >  static const struct spinand_info micron_spinand_table[] = {
> > >  	SPINAND_INFO("MT29F2G01ABAGD", 0x24,
> > >  		     NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 2, 1, 1),
> > > @@ -100,6 +158,16 @@ static const struct spinand_info
> > > micron_spinand_table[] = {
> > >  		     0,
> > >  		     SPINAND_ECCINFO(&mt29f2g01abagd_ooblayout,
> > >  				     mt29f2g01abagd_ecc_get_status)),
> > > +	SPINAND_INFO("MT29F1G01AAADD", 0x12,
> > > +		     NAND_MEMORG(1, 2048, 64, 64, 1024, 20, 2, 1, 1),
> > > +		     NAND_ECCREQ(4, 2048),  
> > 
> > I think, this should be NAND_ECCREQ(4, 512).  
> 
> I don't thinks so, according the datasheet [1], section ECC Protection:
> 
> 8<--------------------------------------
> During a PROGRAM operation, the device calculates an ECC code on the 2k
> page in the cache register, before the page is written to the NAND
> Flash array. The ECC code is stored in the spare area of the page.
> 8<--------------------------------------

Looking at "Table 11: ECC Protection" it really seems to be 4bit/512. I
think the sentence you quoted just means the ECC is calculated for each
512 bytes block in the page and written at once (no subpage write).
BTW, there's an easy way to know who's right => nandbiterrs.

> 
> [1] https://datasheet.octopart.com/MT29F1G01AAADDH4-IT:D-Micron-datasheet-11572380.pdf
> 
> Regards,
>   Marco
> 
> >   
> > > +		     SPINAND_INFO_OP_VARIANTS(
> > > +
> > > 	&read_cache_variants_mt29f1g01aaadd,
> > > +
> > > 	&write_cache_variants_mt29f1g01aaadd,
> > > +
> > > 	&update_cache_variants_mt29f1g01aaadd),
> > > +		     0,
> > > +		     SPINAND_ECCINFO(&mt29f1g01aaadd_ooblayout,
> > > +				     mt29f1g01aaadd_ecc_get_status)),
> > >  };
> > > 
> > >  static int micron_spinand_detect(struct spinand_device *spinand)
> > > --
> > > 2.20.1  
> > 
> > Thanks,
> > Shiva
> >   
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/