| Message ID | 20190814082232.2119-1-m.felsch@pengutronix.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Miquel Raynal |
| Headers | show |
| Series | mtd: spinand: micron: add support for MT29F1G01AAADD | expand |
Hi Marco, Marco Felsch <m.felsch@pengutronix.de> wrote on Wed, 14 Aug 2019 10:22:32 +0200: > 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://datasheet.octopart.com/ \ > MT29F1G01AAADDH4-IT:D-Micron-datasheet-11572380.pdf > > Cc: Peter Pan <peterpandong@micron.com> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > --- > 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..9d63450afc69 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 * 0x10) + 8; Any reason to use hex here? ^ If not I would prefer decimal numbers. Otherwise looks fine. Thanks, Miquèl
Hi Miquel, On 19-08-19 10:17, Miquel Raynal wrote: > Hi Marco, > > Marco Felsch <m.felsch@pengutronix.de> wrote on Wed, 14 Aug 2019 > 10:22:32 +0200: > > > 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://datasheet.octopart.com/ \ > > MT29F1G01AAADDH4-IT:D-Micron-datasheet-11572380.pdf > > > > Cc: Peter Pan <peterpandong@micron.com> > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > --- > > 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..9d63450afc69 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 * 0x10) + 8; > > Any reason to use hex here? ^ > > If not I would prefer decimal numbers. Since the datasheet describe it in hex to. Can you have a look on [1] table 11? May we do something like: region->offset = (section * 0x10) + 0x8; [1] https://datasheet.octopart.com/MT29F1G01AAADDH4-IT:D-Micron-datasheet-11572380.pdf > > Otherwise looks fine. Anyway I can change the above code to use only decimal values if you like it more. Regards, Marco > > Thanks, > Miquèl >
Hi Marco, Marco Felsch <m.felsch@pengutronix.de> wrote on Mon, 19 Aug 2019 15:30:42 +0200: > Hi Miquel, > > On 19-08-19 10:17, Miquel Raynal wrote: > > Hi Marco, > > > > Marco Felsch <m.felsch@pengutronix.de> wrote on Wed, 14 Aug 2019 > > 10:22:32 +0200: > > > > > 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://datasheet.octopart.com/ \ > > > MT29F1G01AAADDH4-IT:D-Micron-datasheet-11572380.pdf > > > > > > Cc: Peter Pan <peterpandong@micron.com> > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > > --- > > > 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..9d63450afc69 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 * 0x10) + 8; > > > > Any reason to use hex here? ^ > > > > If not I would prefer decimal numbers. > > Since the datasheet describe it in hex to. > > Can you have a look on [1] table 11? May we do something like: > > region->offset = (section * 0x10) + 0x8; > > [1] https://datasheet.octopart.com/MT29F1G01AAADDH4-IT:D-Micron-datasheet-11572380.pdf > > > > > Otherwise looks fine. > > Anyway I can change the above code to use only decimal values if you > like it more. I think it is better to reserve hexadecimal values to register operations. Please translate into decimal. Thanks, Miquèl
Hi Miquel, On 19-08-19 16:34, Miquel Raynal wrote: > Hi Marco, > > Marco Felsch <m.felsch@pengutronix.de> wrote on Mon, 19 Aug 2019 > 15:30:42 +0200: > > > Hi Miquel, > > > > On 19-08-19 10:17, Miquel Raynal wrote: > > > Hi Marco, > > > > > > Marco Felsch <m.felsch@pengutronix.de> wrote on Wed, 14 Aug 2019 > > > 10:22:32 +0200: > > > > > > > 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://datasheet.octopart.com/ \ > > > > MT29F1G01AAADDH4-IT:D-Micron-datasheet-11572380.pdf > > > > > > > > Cc: Peter Pan <peterpandong@micron.com> > > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > > > --- > > > > 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..9d63450afc69 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 * 0x10) + 8; > > > > > > Any reason to use hex here? ^ > > > > > > If not I would prefer decimal numbers. > > > > Since the datasheet describe it in hex to. > > > > Can you have a look on [1] table 11? May we do something like: > > > > region->offset = (section * 0x10) + 0x8; > > > > [1] https://datasheet.octopart.com/MT29F1G01AAADDH4-IT:D-Micron-datasheet-11572380.pdf > > > > > > > > Otherwise looks fine. > > > > Anyway I can change the above code to use only decimal values if you > > like it more. > > I think it is better to reserve hexadecimal values to register > operations. Please translate into decimal. Okay. Just one last question. What is the common way to go to specify the free area? By this I mean that the NAND has two areas to store the user metadata calling it 'user metadata I' and 'user metadata II'. 'user metadata II' isn't ecc protected so I skip them. But the current supported chip does not skip the user metadata area which isn't protected [1] table 10. [1] https://www.micron.com/~/media/documents/products/data-sheet/nand-flash/70-series/m79a_2gb_3v_nand_spi.pdf Regards, Marco > > Thanks, > Miquèl > >
Hello Miquèl, On Mon, Aug 19, 2019 at 10:17:18AM +0200, Miquel Raynal wrote: > Marco Felsch <m.felsch@pengutronix.de> wrote on Wed, 14 Aug 2019 > 10:22:32 +0200: > > +static int mt29f1g01aaadd_ooblayout_ecc(struct mtd_info *mtd, int section, > > + struct mtd_oob_region *region) > > +{ > > + if (section > 3) > > + return -ERANGE; > > + > > + region->offset = (section * 0x10) + 8; > > Any reason to use hex here? ^ > > If not I would prefer decimal numbers. IMHO it is quite common to use hexadecimal also for register offsets, not only for register values. I checked a few drivers (drivers/mtd/nand/raw/mxc_nand.c, drivers/clk/meson/g12a.c, drivers/gpio/gpio-sch.c) and they all use hex also for the offsets, so it seems to be at least usual. Also as offsets for repeating registers are usually powers of two, hexadecimal constants are more suitable IMHO. Best regards Uwe
Hi Uwe, Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote on Tue, 20 Aug 2019 10:28:37 +0200: > Hello Miquèl, > > On Mon, Aug 19, 2019 at 10:17:18AM +0200, Miquel Raynal wrote: > > Marco Felsch <m.felsch@pengutronix.de> wrote on Wed, 14 Aug 2019 > > 10:22:32 +0200: > > > +static int mt29f1g01aaadd_ooblayout_ecc(struct mtd_info *mtd, int section, > > > + struct mtd_oob_region *region) > > > +{ > > > + if (section > 3) > > > + return -ERANGE; > > > + > > > + region->offset = (section * 0x10) + 8; > > > > Any reason to use hex here? ^ > > > > If not I would prefer decimal numbers. > > IMHO it is quite common to use hexadecimal also for register offsets, > not only for register values. > > I checked a few drivers (drivers/mtd/nand/raw/mxc_nand.c, > drivers/clk/meson/g12a.c, drivers/gpio/gpio-sch.c) and they all use hex > also for the offsets, so it seems to be at least usual. Also as offsets > for repeating registers are usually powers of two, hexadecimal constants > are more suitable IMHO. Absolutely. But here region->offset is not a register offset at all, it is an indication for the upper layers of the position of an area within a bigger buffer (here: where are the ECC bytes in my buffer in RAM). I don't think hexadecimal numbers have any interest here. Thanks, Miquèl
Hi Marco, > > Hi Miquel, > > On 19-08-19 16:34, Miquel Raynal wrote: > > Hi Marco, > > > > Marco Felsch <m.felsch@pengutronix.de> wrote on Mon, 19 Aug 2019 > > 15:30:42 +0200: > > > > > Hi Miquel, > > > > > > On 19-08-19 10:17, Miquel Raynal wrote: > > > > Hi Marco, > > > > > > > > Marco Felsch <m.felsch@pengutronix.de> wrote on Wed, 14 Aug 2019 > > > > 10:22:32 +0200: > > > > > > > > > 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%2F&data=02%7C01%7Csshivamurthy%40micron.co > m%7C420c4296ddc9420ba7da08d7253924ce%7Cf38a5ecd28134862b11bac1d5 > 63c806f%7C0%7C1%7C637018799689823280&sdata=wZHbyU68pOT%2Bs > 3lrcuEk2FqG0DDggzLVpKpMDcYink0%3D&reserved=0 \ > > > > > MT29F1G01AAADDH4-IT:D-Micron-datasheet-11572380.pdf > > > > > > > > > > Cc: Peter Pan <peterpandong@micron.com> > > > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > > > > --- > > > > > 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..9d63450afc69 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 * 0x10) + 8; > > > > > > > > Any reason to use hex here? ^ > > > > > > > > If not I would prefer decimal numbers. > > > > > > Since the datasheet describe it in hex to. > > > > > > Can you have a look on [1] table 11? May we do something like: > > > > > > region->offset = (section * 0x10) + 0x8; > > > > > > [1] > https://nam01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdata > sheet.octopart.com%2FMT29F1G01AAADDH4-IT%3AD-Micron-datasheet- > 11572380.pdf&data=02%7C01%7Csshivamurthy%40micron.com%7C420c > 4296ddc9420ba7da08d7253924ce%7Cf38a5ecd28134862b11bac1d563c806f%7 > C0%7C1%7C637018799689823280&sdata=XaTab%2BxmXRmz7jxwINT2B > BqAV0aRlyR1EGDz%2BktS%2BQs%3D&reserved=0 > > > > > > > > > > > Otherwise looks fine. > > > > > > Anyway I can change the above code to use only decimal values if you > > > like it more. > > > > I think it is better to reserve hexadecimal values to register > > operations. Please translate into decimal. > > Okay. Just one last question. What is the common way to go to specify > the free area? By this I mean that the NAND has two areas to store the > user metadata calling it 'user metadata I' and 'user metadata II'. 'user > metadata II' isn't ecc protected so I skip them. But the current > supported chip does not skip the user metadata area which isn't > protected [1] table 10. > > [1] https://www.micron.com/~/media/documents/products/data- > sheet/nand-flash/70-series/m79a_2gb_3v_nand_spi.pdf > I have written patch to make helpers to be more generic. They work for Micron's M78A, M79A and M70A series SPI NANDs. Regards, Shiva > Regards, > Marco > > > > > Thanks, > > Miquèl > > > > > > -- > Pengutronix e.K. | | > Industrial Linux Solutions | > https://nam01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww > .pengutronix.de%2F&data=02%7C01%7Csshivamurthy%40micron.com% > 7C420c4296ddc9420ba7da08d7253924ce%7Cf38a5ecd28134862b11bac1d563c > 806f%7C0%7C1%7C637018799689823280&sdata=k%2BwLO84bN9Dt02% > 2FJ%2BLLboEx8t29T8my7oKrchrV6bMw%3D&reserved=0 | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | > > ______________________________________________________ > Linux MTD discussion mailing list > https://nam01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.i > nfradead.org%2Fmailman%2Flistinfo%2Flinux- > mtd%2F&data=02%7C01%7Csshivamurthy%40micron.com%7C420c4296 > ddc9420ba7da08d7253924ce%7Cf38a5ecd28134862b11bac1d563c806f%7C0% > 7C1%7C637018799689823280&sdata=03Qz9zc098PqOiGOIALy1PkgVNGB > NYqDPuctarAddGg%3D&reserved=0
Hi Marco, > > > > > Hi Miquel, > > > > On 19-08-19 16:34, Miquel Raynal wrote: > > > Hi Marco, > > > > > > Marco Felsch <m.felsch@pengutronix.de> wrote on Mon, 19 Aug 2019 > > > 15:30:42 +0200: > > > > > > > Hi Miquel, > > > > > > > > On 19-08-19 10:17, Miquel Raynal wrote: > > > > > Hi Marco, > > > > > > > > > > Marco Felsch <m.felsch@pengutronix.de> wrote on Wed, 14 Aug > 2019 > > > > > 10:22:32 +0200: > > > > > > > > > > > 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%2F&data=02%7C01%7Csshivamurthy%40micron.co > > > m%7C420c4296ddc9420ba7da08d7253924ce%7Cf38a5ecd28134862b11bac1d5 > > > 63c806f%7C0%7C1%7C637018799689823280&sdata=wZHbyU68pOT%2Bs > > 3lrcuEk2FqG0DDggzLVpKpMDcYink0%3D&reserved=0 \ > > > > > > MT29F1G01AAADDH4-IT:D-Micron-datasheet-11572380.pdf > > > > > > > > > > > > Cc: Peter Pan <peterpandong@micron.com> > > > > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > > > > > --- > > > > > > 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..9d63450afc69 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 * 0x10) + 8; > > > > > > > > > > Any reason to use hex here? ^ > > > > > > > > > > If not I would prefer decimal numbers. > > > > > > > > Since the datasheet describe it in hex to. > > > > > > > > Can you have a look on [1] table 11? May we do something like: > > > > > > > > region->offset = (section * 0x10) + 0x8; > > > > > > > > [1] > > > https://nam01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdata > > sheet.octopart.com%2FMT29F1G01AAADDH4-IT%3AD-Micron-datasheet- > > > 11572380.pdf&data=02%7C01%7Csshivamurthy%40micron.com%7C420c > > > 4296ddc9420ba7da08d7253924ce%7Cf38a5ecd28134862b11bac1d563c806f%7 > > > C0%7C1%7C637018799689823280&sdata=XaTab%2BxmXRmz7jxwINT2B > > BqAV0aRlyR1EGDz%2BktS%2BQs%3D&reserved=0 > > > > > > > > > > > > > > Otherwise looks fine. > > > > > > > > Anyway I can change the above code to use only decimal values if you > > > > like it more. > > > > > > I think it is better to reserve hexadecimal values to register > > > operations. Please translate into decimal. > > > > Okay. Just one last question. What is the common way to go to specify > > the free area? By this I mean that the NAND has two areas to store the > > user metadata calling it 'user metadata I' and 'user metadata II'. 'user > > metadata II' isn't ecc protected so I skip them. But the current > > supported chip does not skip the user metadata area which isn't > > protected [1] table 10. > > > > [1] https://www.micron.com/~/media/documents/products/data- > > sheet/nand-flash/70-series/m79a_2gb_3v_nand_spi.pdf > > > > I have written patch to make helpers to be more generic. > They work for Micron's M78A, M79A and M70A series SPI NANDs. > I missed link in last email, here it is. http://patchwork.ozlabs.org/patch/1134724/ > > Regards, > Shiva > > > Regards, > > Marco > > > > > > > > Thanks, > > > Miquèl > > > > > > > > > > -- > > Pengutronix e.K. | | > > Industrial Linux Solutions | > > > https://nam01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww > > > .pengutronix.de%2F&data=02%7C01%7Csshivamurthy%40micron.com% > > > 7C420c4296ddc9420ba7da08d7253924ce%7Cf38a5ecd28134862b11bac1d563c > > > 806f%7C0%7C1%7C637018799689823280&sdata=k%2BwLO84bN9Dt02% > > 2FJ%2BLLboEx8t29T8my7oKrchrV6bMw%3D&reserved=0 | > > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | > > > > ______________________________________________________ > > Linux MTD discussion mailing list > > > https://nam01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.i > > nfradead.org%2Fmailman%2Flistinfo%2Flinux- > > > mtd%2F&data=02%7C01%7Csshivamurthy%40micron.com%7C420c4296 > > > ddc9420ba7da08d7253924ce%7Cf38a5ecd28134862b11bac1d563c806f%7C0% > > > 7C1%7C637018799689823280&sdata=03Qz9zc098PqOiGOIALy1PkgVNGB > > NYqDPuctarAddGg%3D&reserved=0
Hi Shivamurthy, On 19-08-20 11:31, Shivamurthy Shastri (sshivamurthy) wrote: > Hi Marco, > > > > > Hi Miquel, > > > > On 19-08-19 16:34, Miquel Raynal wrote: > > > Hi Marco, > > > > > > Marco Felsch <m.felsch@pengutronix.de> wrote on Mon, 19 Aug 2019 > > > 15:30:42 +0200: > > > > > > > Hi Miquel, > > > > > > > > On 19-08-19 10:17, Miquel Raynal wrote: > > > > > Hi Marco, > > > > > > > > > > Marco Felsch <m.felsch@pengutronix.de> wrote on Wed, 14 Aug 2019 > > > > > 10:22:32 +0200: > > > > > > > > > > > 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%2F&data=02%7C01%7Csshivamurthy%40micron.co > > m%7C420c4296ddc9420ba7da08d7253924ce%7Cf38a5ecd28134862b11bac1d5 > > 63c806f%7C0%7C1%7C637018799689823280&sdata=wZHbyU68pOT%2Bs > > 3lrcuEk2FqG0DDggzLVpKpMDcYink0%3D&reserved=0 \ > > > > > > MT29F1G01AAADDH4-IT:D-Micron-datasheet-11572380.pdf > > > > > > > > > > > > Cc: Peter Pan <peterpandong@micron.com> > > > > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > > > > > --- > > > > > > 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..9d63450afc69 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 * 0x10) + 8; > > > > > > > > > > Any reason to use hex here? ^ > > > > > > > > > > If not I would prefer decimal numbers. > > > > > > > > Since the datasheet describe it in hex to. > > > > > > > > Can you have a look on [1] table 11? May we do something like: > > > > > > > > region->offset = (section * 0x10) + 0x8; > > > > > > > > [1] > > https://nam01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdata > > sheet.octopart.com%2FMT29F1G01AAADDH4-IT%3AD-Micron-datasheet- > > 11572380.pdf&data=02%7C01%7Csshivamurthy%40micron.com%7C420c > > 4296ddc9420ba7da08d7253924ce%7Cf38a5ecd28134862b11bac1d563c806f%7 > > C0%7C1%7C637018799689823280&sdata=XaTab%2BxmXRmz7jxwINT2B > > BqAV0aRlyR1EGDz%2BktS%2BQs%3D&reserved=0 > > > > > > > > > > > > > > Otherwise looks fine. > > > > > > > > Anyway I can change the above code to use only decimal values if you > > > > like it more. > > > > > > I think it is better to reserve hexadecimal values to register > > > operations. Please translate into decimal. > > > > Okay. Just one last question. What is the common way to go to specify > > the free area? By this I mean that the NAND has two areas to store the > > user metadata calling it 'user metadata I' and 'user metadata II'. 'user > > metadata II' isn't ecc protected so I skip them. But the current > > supported chip does not skip the user metadata area which isn't > > protected [1] table 10. > > > > [1] https://www.micron.com/~/media/documents/products/data- > > sheet/nand-flash/70-series/m79a_2gb_3v_nand_spi.pdf > > > > I have written patch to make helpers to be more generic. > They work for Micron's M78A, M79A and M70A series SPI NANDs. Sounds good =) But my question is still open. Regards, Marco > > Regards, > Shiva > > > Regards, > > Marco > > > > > > > > Thanks, > > > Miquèl > > > > > > > > > > -- > > Pengutronix e.K. | | > > Industrial Linux Solutions | > > https://nam01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww > > .pengutronix.de%2F&data=02%7C01%7Csshivamurthy%40micron.com% > > 7C420c4296ddc9420ba7da08d7253924ce%7Cf38a5ecd28134862b11bac1d563c > > 806f%7C0%7C1%7C637018799689823280&sdata=k%2BwLO84bN9Dt02% > > 2FJ%2BLLboEx8t29T8my7oKrchrV6bMw%3D&reserved=0 | > > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | > > > > ______________________________________________________ > > Linux MTD discussion mailing list > > https://nam01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.i > > nfradead.org%2Fmailman%2Flistinfo%2Flinux- > > mtd%2F&data=02%7C01%7Csshivamurthy%40micron.com%7C420c4296 > > ddc9420ba7da08d7253924ce%7Cf38a5ecd28134862b11bac1d563c806f%7C0% > > 7C1%7C637018799689823280&sdata=03Qz9zc098PqOiGOIALy1PkgVNGB > > NYqDPuctarAddGg%3D&reserved=0 >
Hi Shivamurthy, Miquel, On 19-08-20 11:33, Shivamurthy Shastri (sshivamurthy) wrote: > Hi Marco, [ ... ] > > > Okay. Just one last question. What is the common way to go to specify > > > the free area? By this I mean that the NAND has two areas to store the > > > user metadata calling it 'user metadata I' and 'user metadata II'. 'user > > > metadata II' isn't ecc protected so I skip them. But the current > > > supported chip does not skip the user metadata area which isn't > > > protected [1] table 10. > > > > > > [1] https://www.micron.com/~/media/documents/products/data- > > > sheet/nand-flash/70-series/m79a_2gb_3v_nand_spi.pdf @Miquel Do you can me help with that? > > > > I have written patch to make helpers to be more generic. > > They work for Micron's M78A, M79A and M70A series SPI NANDs. > > > > I missed link in last email, here it is. > > http://patchwork.ozlabs.org/patch/1134724/ This patch seem not to address my ooblayout.. So my patch is still needed. Regards, Marco
Hi Marco, Marco Felsch <m.felsch@pengutronix.de> wrote on Wed, 21 Aug 2019 09:19:14 +0200: > Hi Shivamurthy, Miquel, > > On 19-08-20 11:33, Shivamurthy Shastri (sshivamurthy) wrote: > > Hi Marco, > > [ ... ] > > > > > Okay. Just one last question. What is the common way to go to specify > > > > the free area? By this I mean that the NAND has two areas to store the > > > > user metadata calling it 'user metadata I' and 'user metadata II'. 'user > > > > metadata II' isn't ecc protected so I skip them. But the current > > > > supported chip does not skip the user metadata area which isn't > > > > protected [1] table 10. > > > > > > > > [1] https://www.micron.com/~/media/documents/products/data- > > > > sheet/nand-flash/70-series/m79a_2gb_3v_nand_spi.pdf > > @Miquel > Do you can me help with that? The xxx_ooblayout_free/ecc helpers are here for that. Section is the number of distinct chunks you have in the OOB. If you have two chunks but (metadata I and II) but you don't want to expose the unprotected bytes, then just hide metadata II with a if (section) return -ERANGE Does this answer your question? > > > > > > > I have written patch to make helpers to be more generic. > > > They work for Micron's M78A, M79A and M70A series SPI NANDs. > > > > > > > I missed link in last email, here it is. > > > > http://patchwork.ozlabs.org/patch/1134724/ > > This patch seem not to address my ooblayout.. So my patch is still > needed. > > Regards, > Marco > Thanks, Miquèl
diff --git a/drivers/mtd/nand/spi/micron.c b/drivers/mtd/nand/spi/micron.c index 7d7b1f7fcf71..9d63450afc69 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 * 0x10) + 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 * 0x10) + 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), + 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)
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://datasheet.octopart.com/ \ MT29F1G01AAADDH4-IT:D-Micron-datasheet-11572380.pdf Cc: Peter Pan <peterpandong@micron.com> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> --- drivers/mtd/nand/spi/micron.c | 68 +++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+)