* [PATCH v4 0/2] Add support for micron SPI NAND MT29F2G01AAAED @ 2020-09-13 16:15 Thirumalesha Narasimhappa 2020-09-13 16:15 ` [PATCH v4 1/2] mtd: spinand: micron: Generalize the function and structure names Thirumalesha Narasimhappa 2020-09-13 16:15 ` [PATCH v4 2/2] mtd: spinand: micron: add support for MT29F2G01AAAED Thirumalesha Narasimhappa 0 siblings, 2 replies; 10+ messages in thread From: Thirumalesha Narasimhappa @ 2020-09-13 16:15 UTC (permalink / raw) To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra, Shivamurthy Shastri, Boris Brezillon, Chuanhong Guo, linux-mtd, linux-kernel Cc: Thirumalesha Narasimhappa Adding support for Micron SPI NAND MT29F2G01AAAED device and generalised the structure & function names as per the review comments v4: Split patch into two parts, 1. Generalise the oob structure & function names as show in v3 2. Add support for MT29F2G01AAAED device a. Add oob generic section check in the function micron_ooblayout_free b. Rename mt29f2g01aaaed_* to generic name micron_4_* v3: As per the review comments, 1. Renamed read_cache_variants as quadio_read_cache_variants, write_cache_variants as x4_write_cache_variants/x1_write_cache_variants, update_cache_variants as x4_update_cache_variants/x1_update_cache_variants, read_cache_variants as x4_read_cache_variants 2. Renamed micron_8_ooblayout as micron_grouped_ooblayout & mt29f2g01aaaed_ooblayout as micron_interleaved_ooblayout 3. Generalized page size based oob section check in mt29f2g01aaaed_ooblayout_ecc function and separate case check for two bytes BBM reserved in mt29f2g01aaaed_ooblayout_free function 4. Removed mt29f2g01aaaed_ecc_get_status function & MICRON_STATUS_ECC_1TO4_BITFLIPS v2: Removed SPINAND_SELECT_TARGET as per the comments & fixed typo errors v1: Add support for Micron SPI Nand device MT29F2G01AAAED Thirumalesha Narasimhappa (2): mtd: spinand: micron: Generalize the function and structure names mtd: spinand: micron: add support for MT29F2G01AAAED drivers/mtd/nand/spi/micron.c | 144 ++++++++++++++++++++++++---------- 1 file changed, 104 insertions(+), 40 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 1/2] mtd: spinand: micron: Generalize the function and structure names 2020-09-13 16:15 [PATCH v4 0/2] Add support for micron SPI NAND MT29F2G01AAAED Thirumalesha Narasimhappa @ 2020-09-13 16:15 ` Thirumalesha Narasimhappa 2020-09-15 8:13 ` Miquel Raynal 2020-09-13 16:15 ` [PATCH v4 2/2] mtd: spinand: micron: add support for MT29F2G01AAAED Thirumalesha Narasimhappa 1 sibling, 1 reply; 10+ messages in thread From: Thirumalesha Narasimhappa @ 2020-09-13 16:15 UTC (permalink / raw) To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra, Shivamurthy Shastri, Boris Brezillon, Chuanhong Guo, linux-mtd, linux-kernel Cc: Thirumalesha Narasimhappa Rename the oob structure and read/write/update function names to a generic names Signed-off-by: Thirumalesha Narasimhappa <nthirumalesha7@gmail.com> --- drivers/mtd/nand/spi/micron.c | 80 +++++++++++++++++------------------ 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/drivers/mtd/nand/spi/micron.c b/drivers/mtd/nand/spi/micron.c index 5d370cfcdaaa..d1b1073d1a55 100644 --- a/drivers/mtd/nand/spi/micron.c +++ b/drivers/mtd/nand/spi/micron.c @@ -28,7 +28,7 @@ #define MICRON_SELECT_DIE(x) ((x) << 6) -static SPINAND_OP_VARIANTS(read_cache_variants, +static SPINAND_OP_VARIANTS(quadio_read_cache_variants, SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 2, NULL, 0), SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0), SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 1, NULL, 0), @@ -36,11 +36,11 @@ static SPINAND_OP_VARIANTS(read_cache_variants, 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, +static SPINAND_OP_VARIANTS(x4_write_cache_variants, SPINAND_PROG_LOAD_X4(true, 0, NULL, 0), SPINAND_PROG_LOAD(true, 0, NULL, 0)); -static SPINAND_OP_VARIANTS(update_cache_variants, +static SPINAND_OP_VARIANTS(x4_update_cache_variants, SPINAND_PROG_LOAD_X4(false, 0, NULL, 0), SPINAND_PROG_LOAD(false, 0, NULL, 0)); @@ -69,7 +69,7 @@ static int micron_8_ooblayout_free(struct mtd_info *mtd, int section, return 0; } -static const struct mtd_ooblayout_ops micron_8_ooblayout = { +static const struct mtd_ooblayout_ops micron_grouped_ooblayout = { .ecc = micron_8_ooblayout_ecc, .free = micron_8_ooblayout_free, }; @@ -120,55 +120,55 @@ static const struct spinand_info micron_spinand_table[] = { SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x24), NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 2, 1, 1), NAND_ECCREQ(8, 512), - SPINAND_INFO_OP_VARIANTS(&read_cache_variants, - &write_cache_variants, - &update_cache_variants), + SPINAND_INFO_OP_VARIANTS(&quadio_read_cache_variants, + &x4_write_cache_variants, + &x4_update_cache_variants), 0, - SPINAND_ECCINFO(µn_8_ooblayout, + SPINAND_ECCINFO(µn_grouped_ooblayout, micron_8_ecc_get_status)), /* M79A 2Gb 1.8V */ SPINAND_INFO("MT29F2G01ABBGD", SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x25), NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 2, 1, 1), NAND_ECCREQ(8, 512), - SPINAND_INFO_OP_VARIANTS(&read_cache_variants, - &write_cache_variants, - &update_cache_variants), + SPINAND_INFO_OP_VARIANTS(&quadio_read_cache_variants, + &x4_write_cache_variants, + &x4_update_cache_variants), 0, - SPINAND_ECCINFO(µn_8_ooblayout, + SPINAND_ECCINFO(µn_grouped_ooblayout, micron_8_ecc_get_status)), /* M78A 1Gb 3.3V */ SPINAND_INFO("MT29F1G01ABAFD", SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x14), NAND_MEMORG(1, 2048, 128, 64, 1024, 20, 1, 1, 1), NAND_ECCREQ(8, 512), - SPINAND_INFO_OP_VARIANTS(&read_cache_variants, - &write_cache_variants, - &update_cache_variants), + SPINAND_INFO_OP_VARIANTS(&quadio_read_cache_variants, + &x4_write_cache_variants, + &x4_update_cache_variants), 0, - SPINAND_ECCINFO(µn_8_ooblayout, + SPINAND_ECCINFO(µn_grouped_ooblayout, micron_8_ecc_get_status)), /* M78A 1Gb 1.8V */ SPINAND_INFO("MT29F1G01ABAFD", SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x15), NAND_MEMORG(1, 2048, 128, 64, 1024, 20, 1, 1, 1), NAND_ECCREQ(8, 512), - SPINAND_INFO_OP_VARIANTS(&read_cache_variants, - &write_cache_variants, - &update_cache_variants), + SPINAND_INFO_OP_VARIANTS(&quadio_read_cache_variants, + &x4_write_cache_variants, + &x4_update_cache_variants), 0, - SPINAND_ECCINFO(µn_8_ooblayout, + SPINAND_ECCINFO(µn_grouped_ooblayout, micron_8_ecc_get_status)), /* M79A 4Gb 3.3V */ SPINAND_INFO("MT29F4G01ADAGD", SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x36), NAND_MEMORG(1, 2048, 128, 64, 2048, 80, 2, 1, 2), NAND_ECCREQ(8, 512), - SPINAND_INFO_OP_VARIANTS(&read_cache_variants, - &write_cache_variants, - &update_cache_variants), + SPINAND_INFO_OP_VARIANTS(&quadio_read_cache_variants, + &x4_write_cache_variants, + &x4_update_cache_variants), 0, - SPINAND_ECCINFO(µn_8_ooblayout, + SPINAND_ECCINFO(µn_grouped_ooblayout, micron_8_ecc_get_status), SPINAND_SELECT_TARGET(micron_select_target)), /* M70A 4Gb 3.3V */ @@ -176,33 +176,33 @@ static const struct spinand_info micron_spinand_table[] = { SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x34), NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1), NAND_ECCREQ(8, 512), - SPINAND_INFO_OP_VARIANTS(&read_cache_variants, - &write_cache_variants, - &update_cache_variants), + SPINAND_INFO_OP_VARIANTS(&quadio_read_cache_variants, + &x4_write_cache_variants, + &x4_update_cache_variants), SPINAND_HAS_CR_FEAT_BIT, - SPINAND_ECCINFO(µn_8_ooblayout, + SPINAND_ECCINFO(µn_grouped_ooblayout, micron_8_ecc_get_status)), /* M70A 4Gb 1.8V */ SPINAND_INFO("MT29F4G01ABBFD", SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x35), NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1), NAND_ECCREQ(8, 512), - SPINAND_INFO_OP_VARIANTS(&read_cache_variants, - &write_cache_variants, - &update_cache_variants), + SPINAND_INFO_OP_VARIANTS(&quadio_read_cache_variants, + &x4_write_cache_variants, + &x4_update_cache_variants), SPINAND_HAS_CR_FEAT_BIT, - SPINAND_ECCINFO(µn_8_ooblayout, + SPINAND_ECCINFO(µn_grouped_ooblayout, micron_8_ecc_get_status)), /* M70A 8Gb 3.3V */ SPINAND_INFO("MT29F8G01ADAFD", SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x46), NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 2), NAND_ECCREQ(8, 512), - SPINAND_INFO_OP_VARIANTS(&read_cache_variants, - &write_cache_variants, - &update_cache_variants), + SPINAND_INFO_OP_VARIANTS(&quadio_read_cache_variants, + &x4_write_cache_variants, + &x4_update_cache_variants), SPINAND_HAS_CR_FEAT_BIT, - SPINAND_ECCINFO(µn_8_ooblayout, + SPINAND_ECCINFO(µn_grouped_ooblayout, micron_8_ecc_get_status), SPINAND_SELECT_TARGET(micron_select_target)), /* M70A 8Gb 1.8V */ @@ -210,11 +210,11 @@ static const struct spinand_info micron_spinand_table[] = { SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x47), NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 2), NAND_ECCREQ(8, 512), - SPINAND_INFO_OP_VARIANTS(&read_cache_variants, - &write_cache_variants, - &update_cache_variants), + SPINAND_INFO_OP_VARIANTS(&quadio_read_cache_variants, + &x4_write_cache_variants, + &x4_update_cache_variants), SPINAND_HAS_CR_FEAT_BIT, - SPINAND_ECCINFO(µn_8_ooblayout, + SPINAND_ECCINFO(µn_grouped_ooblayout, micron_8_ecc_get_status), SPINAND_SELECT_TARGET(micron_select_target)), }; -- 2.25.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] mtd: spinand: micron: Generalize the function and structure names 2020-09-13 16:15 ` [PATCH v4 1/2] mtd: spinand: micron: Generalize the function and structure names Thirumalesha Narasimhappa @ 2020-09-15 8:13 ` Miquel Raynal [not found] ` <CALKVOUomKLZ5GEHmXb+VfEq8UiNUpCN-Vqkx3N+yykEnCrHkDA@mail.gmail.com> 0 siblings, 1 reply; 10+ messages in thread From: Miquel Raynal @ 2020-09-15 8:13 UTC (permalink / raw) To: Thirumalesha Narasimhappa Cc: Richard Weinberger, Vignesh Raghavendra, Shivamurthy Shastri, Boris Brezillon, Chuanhong Guo, linux-mtd, linux-kernel Hi Thirumalesha, Thirumalesha Narasimhappa <nthirumalesha7@gmail.com> wrote on Mon, 14 Sep 2020 00:15:32 +0800: > Rename the oob structure and read/write/update function names to > a generic names > > Signed-off-by: Thirumalesha Narasimhappa <nthirumalesha7@gmail.com> > --- > drivers/mtd/nand/spi/micron.c | 80 +++++++++++++++++------------------ > 1 file changed, 40 insertions(+), 40 deletions(-) > > diff --git a/drivers/mtd/nand/spi/micron.c b/drivers/mtd/nand/spi/micron.c > index 5d370cfcdaaa..d1b1073d1a55 100644 > --- a/drivers/mtd/nand/spi/micron.c > +++ b/drivers/mtd/nand/spi/micron.c > @@ -28,7 +28,7 @@ > > #define MICRON_SELECT_DIE(x) ((x) << 6) > > -static SPINAND_OP_VARIANTS(read_cache_variants, > +static SPINAND_OP_VARIANTS(quadio_read_cache_variants, > SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 2, NULL, 0), > SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0), > SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 1, NULL, 0), > @@ -36,11 +36,11 @@ static SPINAND_OP_VARIANTS(read_cache_variants, > 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, > +static SPINAND_OP_VARIANTS(x4_write_cache_variants, Why quadio_read and x4_write? I don't get the logic. > SPINAND_PROG_LOAD_X4(true, 0, NULL, 0), > SPINAND_PROG_LOAD(true, 0, NULL, 0)); > > -static SPINAND_OP_VARIANTS(update_cache_variants, > +static SPINAND_OP_VARIANTS(x4_update_cache_variants, > SPINAND_PROG_LOAD_X4(false, 0, NULL, 0), > SPINAND_PROG_LOAD(false, 0, NULL, 0)); > > @@ -69,7 +69,7 @@ static int micron_8_ooblayout_free(struct mtd_info *mtd, int section, > return 0; > } > > -static const struct mtd_ooblayout_ops micron_8_ooblayout = { > +static const struct mtd_ooblayout_ops micron_grouped_ooblayout = { Is this necessary? What does "grouped" means? Should we rename all functions with _8_ in it to make sense? > .ecc = micron_8_ooblayout_ecc, > .free = micron_8_ooblayout_free, > }; > @@ -120,55 +120,55 @@ static const struct spinand_info micron_spinand_table[] = { > SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x24), > NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 2, 1, 1), > NAND_ECCREQ(8, 512), > - SPINAND_INFO_OP_VARIANTS(&read_cache_variants, > - &write_cache_variants, > - &update_cache_variants), > + SPINAND_INFO_OP_VARIANTS(&quadio_read_cache_variants, > + &x4_write_cache_variants, > + &x4_update_cache_variants), > 0, > - SPINAND_ECCINFO(µn_8_ooblayout, > + SPINAND_ECCINFO(µn_grouped_ooblayout, > micron_8_ecc_get_status)), > /* M79A 2Gb 1.8V */ > SPINAND_INFO("MT29F2G01ABBGD", > SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x25), > NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 2, 1, 1), > NAND_ECCREQ(8, 512), > - SPINAND_INFO_OP_VARIANTS(&read_cache_variants, > - &write_cache_variants, > - &update_cache_variants), > + SPINAND_INFO_OP_VARIANTS(&quadio_read_cache_variants, > + &x4_write_cache_variants, > + &x4_update_cache_variants), > 0, > - SPINAND_ECCINFO(µn_8_ooblayout, > + SPINAND_ECCINFO(µn_grouped_ooblayout, > micron_8_ecc_get_status)), > /* M78A 1Gb 3.3V */ > SPINAND_INFO("MT29F1G01ABAFD", > SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x14), > NAND_MEMORG(1, 2048, 128, 64, 1024, 20, 1, 1, 1), > NAND_ECCREQ(8, 512), > - SPINAND_INFO_OP_VARIANTS(&read_cache_variants, > - &write_cache_variants, > - &update_cache_variants), > + SPINAND_INFO_OP_VARIANTS(&quadio_read_cache_variants, > + &x4_write_cache_variants, > + &x4_update_cache_variants), > 0, > - SPINAND_ECCINFO(µn_8_ooblayout, > + SPINAND_ECCINFO(µn_grouped_ooblayout, > micron_8_ecc_get_status)), > /* M78A 1Gb 1.8V */ > SPINAND_INFO("MT29F1G01ABAFD", > SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x15), > NAND_MEMORG(1, 2048, 128, 64, 1024, 20, 1, 1, 1), > NAND_ECCREQ(8, 512), > - SPINAND_INFO_OP_VARIANTS(&read_cache_variants, > - &write_cache_variants, > - &update_cache_variants), > + SPINAND_INFO_OP_VARIANTS(&quadio_read_cache_variants, > + &x4_write_cache_variants, > + &x4_update_cache_variants), > 0, > - SPINAND_ECCINFO(µn_8_ooblayout, > + SPINAND_ECCINFO(µn_grouped_ooblayout, > micron_8_ecc_get_status)), > /* M79A 4Gb 3.3V */ > SPINAND_INFO("MT29F4G01ADAGD", > SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x36), > NAND_MEMORG(1, 2048, 128, 64, 2048, 80, 2, 1, 2), > NAND_ECCREQ(8, 512), > - SPINAND_INFO_OP_VARIANTS(&read_cache_variants, > - &write_cache_variants, > - &update_cache_variants), > + SPINAND_INFO_OP_VARIANTS(&quadio_read_cache_variants, > + &x4_write_cache_variants, > + &x4_update_cache_variants), > 0, > - SPINAND_ECCINFO(µn_8_ooblayout, > + SPINAND_ECCINFO(µn_grouped_ooblayout, > micron_8_ecc_get_status), > SPINAND_SELECT_TARGET(micron_select_target)), > /* M70A 4Gb 3.3V */ > @@ -176,33 +176,33 @@ static const struct spinand_info micron_spinand_table[] = { > SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x34), > NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1), > NAND_ECCREQ(8, 512), > - SPINAND_INFO_OP_VARIANTS(&read_cache_variants, > - &write_cache_variants, > - &update_cache_variants), > + SPINAND_INFO_OP_VARIANTS(&quadio_read_cache_variants, > + &x4_write_cache_variants, > + &x4_update_cache_variants), > SPINAND_HAS_CR_FEAT_BIT, > - SPINAND_ECCINFO(µn_8_ooblayout, > + SPINAND_ECCINFO(µn_grouped_ooblayout, > micron_8_ecc_get_status)), > /* M70A 4Gb 1.8V */ > SPINAND_INFO("MT29F4G01ABBFD", > SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x35), > NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1), > NAND_ECCREQ(8, 512), > - SPINAND_INFO_OP_VARIANTS(&read_cache_variants, > - &write_cache_variants, > - &update_cache_variants), > + SPINAND_INFO_OP_VARIANTS(&quadio_read_cache_variants, > + &x4_write_cache_variants, > + &x4_update_cache_variants), > SPINAND_HAS_CR_FEAT_BIT, > - SPINAND_ECCINFO(µn_8_ooblayout, > + SPINAND_ECCINFO(µn_grouped_ooblayout, > micron_8_ecc_get_status)), > /* M70A 8Gb 3.3V */ > SPINAND_INFO("MT29F8G01ADAFD", > SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x46), > NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 2), > NAND_ECCREQ(8, 512), > - SPINAND_INFO_OP_VARIANTS(&read_cache_variants, > - &write_cache_variants, > - &update_cache_variants), > + SPINAND_INFO_OP_VARIANTS(&quadio_read_cache_variants, > + &x4_write_cache_variants, > + &x4_update_cache_variants), > SPINAND_HAS_CR_FEAT_BIT, > - SPINAND_ECCINFO(µn_8_ooblayout, > + SPINAND_ECCINFO(µn_grouped_ooblayout, > micron_8_ecc_get_status), > SPINAND_SELECT_TARGET(micron_select_target)), > /* M70A 8Gb 1.8V */ > @@ -210,11 +210,11 @@ static const struct spinand_info micron_spinand_table[] = { > SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x47), > NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 2), > NAND_ECCREQ(8, 512), > - SPINAND_INFO_OP_VARIANTS(&read_cache_variants, > - &write_cache_variants, > - &update_cache_variants), > + SPINAND_INFO_OP_VARIANTS(&quadio_read_cache_variants, > + &x4_write_cache_variants, > + &x4_update_cache_variants), > SPINAND_HAS_CR_FEAT_BIT, > - SPINAND_ECCINFO(µn_8_ooblayout, > + SPINAND_ECCINFO(µn_grouped_ooblayout, > micron_8_ecc_get_status), > SPINAND_SELECT_TARGET(micron_select_target)), > }; Thanks, Miquèl ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <CALKVOUomKLZ5GEHmXb+VfEq8UiNUpCN-Vqkx3N+yykEnCrHkDA@mail.gmail.com>]
* Re: [PATCH v4 1/2] mtd: spinand: micron: Generalize the function and structure names [not found] ` <CALKVOUomKLZ5GEHmXb+VfEq8UiNUpCN-Vqkx3N+yykEnCrHkDA@mail.gmail.com> @ 2020-09-28 14:55 ` Miquel Raynal 2020-09-28 15:45 ` Boris Brezillon 0 siblings, 1 reply; 10+ messages in thread From: Miquel Raynal @ 2020-09-28 14:55 UTC (permalink / raw) To: Thirumalesha N Cc: Richard Weinberger, Vignesh Raghavendra, Shivamurthy Shastri, Boris Brezillon, Chuanhong Guo, linux-mtd, linux-kernel Hi Thirumalesha, Thirumalesha N <nthirumalesha7@gmail.com> wrote on Sun, 20 Sep 2020 22:38:53 +0800: > Hi Miquèl, > > I adopted these changes as per the Boris Brezillon comments > > On Tue, Sep 15, 2020 at 4:13 PM Miquel Raynal <miquel.raynal@bootlin.com> > wrote: > > > Hi Thirumalesha, > > > > Thirumalesha Narasimhappa <nthirumalesha7@gmail.com> wrote on Mon, 14 > > Sep 2020 00:15:32 +0800: > > > > > Rename the oob structure and read/write/update function names to > > > a generic names > > > > > > Signed-off-by: Thirumalesha Narasimhappa <nthirumalesha7@gmail.com> > > > --- > > > drivers/mtd/nand/spi/micron.c | 80 +++++++++++++++++------------------ > > > 1 file changed, 40 insertions(+), 40 deletions(-) > > > > > > diff --git a/drivers/mtd/nand/spi/micron.c > > b/drivers/mtd/nand/spi/micron.c > > > index 5d370cfcdaaa..d1b1073d1a55 100644 > > > --- a/drivers/mtd/nand/spi/micron.c > > > +++ b/drivers/mtd/nand/spi/micron.c > > > @@ -28,7 +28,7 @@ > > > > > > #define MICRON_SELECT_DIE(x) ((x) << 6) > > > > > > -static SPINAND_OP_VARIANTS(read_cache_variants, > > > +static SPINAND_OP_VARIANTS(quadio_read_cache_variants, > > > SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 2, NULL, 0), > > > SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0), > > > SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 1, NULL, 0), > > > @@ -36,11 +36,11 @@ static SPINAND_OP_VARIANTS(read_cache_variants, > > > 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, > > > +static SPINAND_OP_VARIANTS(x4_write_cache_variants, > > > > Why quadio_read and x4_write? I don't get the logic. > > > > IMHO, quadio_read is derived from SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP & > x4_write is derived from SPINAND_PROG_LOAD_X4 > > Boris, Please correct me, if my understanding is wrong I see. Then please split this change, one updating the helpers with an "x4" prefix when relevant and another one for 8 vs. grouped. > > > > > SPINAND_PROG_LOAD_X4(true, 0, NULL, 0), > > > SPINAND_PROG_LOAD(true, 0, NULL, 0)); > > > > > > -static SPINAND_OP_VARIANTS(update_cache_variants, > > > +static SPINAND_OP_VARIANTS(x4_update_cache_variants, > > > SPINAND_PROG_LOAD_X4(false, 0, NULL, 0), > > > SPINAND_PROG_LOAD(false, 0, NULL, 0)); > > > > > > @@ -69,7 +69,7 @@ static int micron_8_ooblayout_free(struct mtd_info > > *mtd, int section, > > > return 0; > > > } > > > > > > -static const struct mtd_ooblayout_ops micron_8_ooblayout = { > > > +static const struct mtd_ooblayout_ops micron_grouped_ooblayout = { > > > > Is this necessary? What does "grouped" means? Should we rename all > > functions with _8_ in it to make sense? > > > > IMHO, grouped means, ecc bytes are at continuous address, where as > interleaved means ecc bytes splitted into multiple addresses I don't like the name. Interleaved means that there are OOB bytes stored in the data section, which is not the case here. The way OOB bytes are organized do not seem relevant to me, I think i prefer the "_4_/_8_" naming,even if it's not very explicit. Also please be consistent, do not mix "_4_" and "interleaved" (see the other patch). Any feedback from Micron to validate these changes will be appreciated. Thanks, Miquèl ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] mtd: spinand: micron: Generalize the function and structure names 2020-09-28 14:55 ` Miquel Raynal @ 2020-09-28 15:45 ` Boris Brezillon 2020-09-28 15:50 ` Miquel Raynal 0 siblings, 1 reply; 10+ messages in thread From: Boris Brezillon @ 2020-09-28 15:45 UTC (permalink / raw) To: Miquel Raynal Cc: Thirumalesha N, Richard Weinberger, Vignesh Raghavendra, Shivamurthy Shastri, Chuanhong Guo, linux-mtd, linux-kernel On Mon, 28 Sep 2020 16:55:28 +0200 Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > IMHO, grouped means, ecc bytes are at continuous address, where as > > interleaved means ecc bytes splitted into multiple addresses > > I don't like the name. Interleaved means that there are OOB bytes > stored in the data section, which is not the case here. Well, I would argue that the term interleaved alone doesn't say anything about the things that are interleaved. But I guess split/grouped would be fine too if you want to avoid re-using interleaved here. > The way OOB > bytes are organized do not seem relevant to me, I think i prefer the > "_4_/_8_" naming,even if it's not very explicit. The ECC strength doesn't say anything about the scheme used for ECC bytes placement, and you might end up with 2 different schemes providing the same strength, or the same scheme used for 2 different strengths. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] mtd: spinand: micron: Generalize the function and structure names 2020-09-28 15:45 ` Boris Brezillon @ 2020-09-28 15:50 ` Miquel Raynal 2020-09-28 16:03 ` Boris Brezillon 0 siblings, 1 reply; 10+ messages in thread From: Miquel Raynal @ 2020-09-28 15:50 UTC (permalink / raw) To: Boris Brezillon Cc: Thirumalesha N, Richard Weinberger, Vignesh Raghavendra, Shivamurthy Shastri, Chuanhong Guo, linux-mtd, linux-kernel Boris Brezillon <boris.brezillon@collabora.com> wrote on Mon, 28 Sep 2020 17:45:05 +0200: > On Mon, 28 Sep 2020 16:55:28 +0200 > Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > IMHO, grouped means, ecc bytes are at continuous address, where as > > > interleaved means ecc bytes splitted into multiple addresses > > > > I don't like the name. Interleaved means that there are OOB bytes > > stored in the data section, which is not the case here. > > Well, I would argue that the term interleaved alone doesn't say > anything about the things that are interleaved. But I guess I should have said that interleaved, in this subsystem, generally refers to in-band vs. out-of-band data. > split/grouped would be fine too if you want to avoid re-using > interleaved here. split/grouped is fine by me. > > > The way OOB > > bytes are organized do not seem relevant to me, I think i prefer the > > "_4_/_8_" naming,even if it's not very explicit. > > The ECC strength doesn't say anything about the scheme used for ECC > bytes placement, and you might end up with 2 different schemes > providing the same strength, or the same scheme used for 2 different > strengths. So perhaps both should be present in the name? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] mtd: spinand: micron: Generalize the function and structure names 2020-09-28 15:50 ` Miquel Raynal @ 2020-09-28 16:03 ` Boris Brezillon 2020-09-28 16:21 ` Miquel Raynal 0 siblings, 1 reply; 10+ messages in thread From: Boris Brezillon @ 2020-09-28 16:03 UTC (permalink / raw) To: Miquel Raynal Cc: Thirumalesha N, Richard Weinberger, Vignesh Raghavendra, Shivamurthy Shastri, Chuanhong Guo, linux-mtd, linux-kernel On Mon, 28 Sep 2020 17:50:05 +0200 Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > The way OOB > > > bytes are organized do not seem relevant to me, I think i prefer the > > > "_4_/_8_" naming,even if it's not very explicit. > > > > The ECC strength doesn't say anything about the scheme used for ECC > > bytes placement, and you might end up with 2 different schemes > > providing the same strength, or the same scheme used for 2 different > > strengths. > > So perhaps both should be present in the name? No, the point was to re-use the same functions for various strengths if they use the same ECC placement scheme. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] mtd: spinand: micron: Generalize the function and structure names 2020-09-28 16:03 ` Boris Brezillon @ 2020-09-28 16:21 ` Miquel Raynal 2020-09-28 16:25 ` Boris Brezillon 0 siblings, 1 reply; 10+ messages in thread From: Miquel Raynal @ 2020-09-28 16:21 UTC (permalink / raw) To: Boris Brezillon Cc: Thirumalesha N, Richard Weinberger, Vignesh Raghavendra, Shivamurthy Shastri, Chuanhong Guo, linux-mtd, linux-kernel Hi Boris, Boris Brezillon <boris.brezillon@collabora.com> wrote on Mon, 28 Sep 2020 18:03:43 +0200: > On Mon, 28 Sep 2020 17:50:05 +0200 > Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > The way OOB > > > > bytes are organized do not seem relevant to me, I think i prefer the > > > > "_4_/_8_" naming,even if it's not very explicit. > > > > > > The ECC strength doesn't say anything about the scheme used for ECC > > > bytes placement, and you might end up with 2 different schemes > > > providing the same strength, or the same scheme used for 2 different > > > strengths. > > > > So perhaps both should be present in the name? > > No, the point was to re-use the same functions for various strengths if > they use the same ECC placement scheme. I get the point, but is the current implementation generic enough? I see hardcoded numbers, I have no idea if these numbers are common to all strength given a specific layout, or if they only match for a given strength? +static int micron_4_ooblayout_ecc(struct mtd_info *mtd, int section, + struct mtd_oob_region *region) +{ + struct spinand_device *spinand = mtd_to_spinand(mtd); + + if (section >= spinand->base.memorg.pagesize / + mtd->ecc_step_size) + return -ERANGE; + + region->offset = (section * 16) + 8; + region->length = 8; + + return 0; +} If possible, I would like to avoid several successive renaming. Thanks, Miquèl ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] mtd: spinand: micron: Generalize the function and structure names 2020-09-28 16:21 ` Miquel Raynal @ 2020-09-28 16:25 ` Boris Brezillon 0 siblings, 0 replies; 10+ messages in thread From: Boris Brezillon @ 2020-09-28 16:25 UTC (permalink / raw) To: Miquel Raynal Cc: Thirumalesha N, Richard Weinberger, Vignesh Raghavendra, Shivamurthy Shastri, Chuanhong Guo, linux-mtd, linux-kernel On Mon, 28 Sep 2020 18:21:59 +0200 Miquel Raynal <miquel.raynal@bootlin.com> wrote: > Hi Boris, > > Boris Brezillon <boris.brezillon@collabora.com> wrote on Mon, 28 Sep > 2020 18:03:43 +0200: > > > On Mon, 28 Sep 2020 17:50:05 +0200 > > Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > > The way OOB > > > > > bytes are organized do not seem relevant to me, I think i prefer the > > > > > "_4_/_8_" naming,even if it's not very explicit. > > > > > > > > The ECC strength doesn't say anything about the scheme used for ECC > > > > bytes placement, and you might end up with 2 different schemes > > > > providing the same strength, or the same scheme used for 2 different > > > > strengths. > > > > > > So perhaps both should be present in the name? > > > > No, the point was to re-use the same functions for various strengths if > > they use the same ECC placement scheme. > > I get the point, but is the current implementation generic enough? I > see hardcoded numbers, I have no idea if these numbers are common to > all strength given a specific layout, or if they only match for a given > strength? > > +static int micron_4_ooblayout_ecc(struct mtd_info *mtd, int section, > + struct mtd_oob_region *region) > +{ > + struct spinand_device *spinand = mtd_to_spinand(mtd); > + > + if (section >= spinand->base.memorg.pagesize / > + mtd->ecc_step_size) > + return -ERANGE; > + > + region->offset = (section * 16) + 8; > + region->length = 8; > + > + return 0; > +} > > If possible, I would like to avoid several successive renaming. Right, I thought those functions were patched to be generic, but that doesn't seem to be the case, so I guess sticking to _<strength>_ makes sense for now. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 2/2] mtd: spinand: micron: add support for MT29F2G01AAAED 2020-09-13 16:15 [PATCH v4 0/2] Add support for micron SPI NAND MT29F2G01AAAED Thirumalesha Narasimhappa 2020-09-13 16:15 ` [PATCH v4 1/2] mtd: spinand: micron: Generalize the function and structure names Thirumalesha Narasimhappa @ 2020-09-13 16:15 ` Thirumalesha Narasimhappa 1 sibling, 0 replies; 10+ messages in thread From: Thirumalesha Narasimhappa @ 2020-09-13 16:15 UTC (permalink / raw) To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra, Shivamurthy Shastri, Boris Brezillon, Chuanhong Guo, linux-mtd, linux-kernel Cc: Thirumalesha Narasimhappa The MT29F2G01AAAED is a single die, 2Gb Micron SPI NAND Flash with 4-bit ECC Signed-off-by: Thirumalesha Narasimhappa <nthirumalesha7@gmail.com> --- drivers/mtd/nand/spi/micron.c | 64 +++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/drivers/mtd/nand/spi/micron.c b/drivers/mtd/nand/spi/micron.c index d1b1073d1a55..dae3f7189880 100644 --- a/drivers/mtd/nand/spi/micron.c +++ b/drivers/mtd/nand/spi/micron.c @@ -44,6 +44,19 @@ static SPINAND_OP_VARIANTS(x4_update_cache_variants, SPINAND_PROG_LOAD_X4(false, 0, NULL, 0), SPINAND_PROG_LOAD(false, 0, NULL, 0)); +/* Micron MT29F2G01AAAED Device */ +static SPINAND_OP_VARIANTS(x4_read_cache_variants, + 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(x1_write_cache_variants, + SPINAND_PROG_LOAD(true, 0, NULL, 0)); + +static SPINAND_OP_VARIANTS(x1_update_cache_variants, + SPINAND_PROG_LOAD(false, 0, NULL, 0)); + static int micron_8_ooblayout_ecc(struct mtd_info *mtd, int section, struct mtd_oob_region *region) { @@ -74,6 +87,47 @@ static const struct mtd_ooblayout_ops micron_grouped_ooblayout = { .free = micron_8_ooblayout_free, }; +static int micron_4_ooblayout_ecc(struct mtd_info *mtd, int section, + struct mtd_oob_region *region) +{ + struct spinand_device *spinand = mtd_to_spinand(mtd); + + if (section >= spinand->base.memorg.pagesize / + mtd->ecc_step_size) + return -ERANGE; + + region->offset = (section * 16) + 8; + region->length = 8; + + return 0; +} + +static int micron_4_ooblayout_free(struct mtd_info *mtd, int section, + struct mtd_oob_region *region) +{ + struct spinand_device *spinand = mtd_to_spinand(mtd); + + if (section >= spinand->base.memorg.pagesize / + mtd->ecc_step_size) + return -ERANGE; + + if (section) { + region->offset = 16 * section; + region->length = 8; + } else { + /* section 0 has two bytes reserved for the BBM */ + region->offset = 2; + region->length = 6; + } + + return 0; +} + +static const struct mtd_ooblayout_ops micron_interleaved_ooblayout = { + .ecc = micron_4_ooblayout_ecc, + .free = micron_4_ooblayout_free, +}; + static int micron_select_target(struct spinand_device *spinand, unsigned int target) { @@ -217,6 +271,16 @@ static const struct spinand_info micron_spinand_table[] = { SPINAND_ECCINFO(µn_grouped_ooblayout, micron_8_ecc_get_status), SPINAND_SELECT_TARGET(micron_select_target)), + /* M69A 2Gb 3.3V */ + SPINAND_INFO("MT29F2G01AAAED", + SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x9F), + NAND_MEMORG(1, 2048, 64, 64, 2048, 80, 2, 1, 1), + NAND_ECCREQ(4, 512), + SPINAND_INFO_OP_VARIANTS(&x4_read_cache_variants, + &x1_write_cache_variants, + &x1_update_cache_variants), + 0, + SPINAND_ECCINFO(µn_interleaved_ooblayout, NULL)), }; static int micron_spinand_init(struct spinand_device *spinand) -- 2.25.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-09-28 16:25 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-13 16:15 [PATCH v4 0/2] Add support for micron SPI NAND MT29F2G01AAAED Thirumalesha Narasimhappa 2020-09-13 16:15 ` [PATCH v4 1/2] mtd: spinand: micron: Generalize the function and structure names Thirumalesha Narasimhappa 2020-09-15 8:13 ` Miquel Raynal [not found] ` <CALKVOUomKLZ5GEHmXb+VfEq8UiNUpCN-Vqkx3N+yykEnCrHkDA@mail.gmail.com> 2020-09-28 14:55 ` Miquel Raynal 2020-09-28 15:45 ` Boris Brezillon 2020-09-28 15:50 ` Miquel Raynal 2020-09-28 16:03 ` Boris Brezillon 2020-09-28 16:21 ` Miquel Raynal 2020-09-28 16:25 ` Boris Brezillon 2020-09-13 16:15 ` [PATCH v4 2/2] mtd: spinand: micron: add support for MT29F2G01AAAED Thirumalesha Narasimhappa
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).