[PATCH v3 1/1] mtd: spinand: Add flags for the Plane Select bit

[Cheng Ming Lin]

Hi Miquel,

Miquel Raynal <miquel.raynal at bootlin.com> 於 2024年8月28日 週三 下午3:47寫道：
>
> Hi Cheng,
>
> linchengming884 at gmail.com wrote on Wed, 28 Aug 2024 14:21:31 +0800:
>
> > From: Cheng Ming Lin <chengminglin at mxic.com.tw>
> >
> > Add support for Macronix serial NAND flash with a two-plane structure.
> >
> > Add the SPINAND_HAS_PP_PLANE_SELECT_BIT flag for serial NAND flash
> > that require inserting the Plane Select bit into the column address
> > during the write_to_cache operation.
> >
> > Add the SPINAND_HAS_READ_PLANE_SELECT_BIT flag for serial NAND flash
> > that require inserting the Plane Select bit into the column address
> > during the read_from_cache operation.
> >
> > Macronix serial NAND flash with a two-plane structure requires insertion
> > of the Plane Select bit into the column address during the write_to_cache
> > operation.
> >
> > Additionally, for MX35{U,F}2G14AC and MX35LF2GE4AB, insertion of the
> > Plane Select bit into the column address is required during the
> > read_from_cache operation.
> >
> > Signed-off-by: Cheng Ming Lin <chengminglin at mxic.com.tw>
> > ---
> >  drivers/mtd/nand/spi/core.c     |  6 ++++++
> >  drivers/mtd/nand/spi/macronix.c | 17 ++++++++++-------
> >  include/linux/mtd/spinand.h     |  2 ++
> >  3 files changed, 18 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> > index e0b6715e5dfe..49f2d66c3a9c 100644
> > --- a/drivers/mtd/nand/spi/core.c
> > +++ b/drivers/mtd/nand/spi/core.c
> > @@ -386,6 +386,9 @@ static int spinand_read_from_cache_op(struct spinand_device *spinand,
> >       else
> >               rdesc = spinand->dirmaps[req->pos.plane].rdesc_ecc;
> >
> > +     if (spinand->flags & SPINAND_HAS_READ_PLANE_SELECT_BIT)
> > +             column |= req->pos.plane << fls(nanddev_page_size(nand));
> > +
> >       while (nbytes) {
> >               ret = spi_mem_dirmap_read(rdesc, column, nbytes, buf);
> >               if (ret < 0)
> > @@ -460,6 +463,9 @@ static int spinand_write_to_cache_op(struct spinand_device *spinand,
> >       else
> >               wdesc = spinand->dirmaps[req->pos.plane].wdesc_ecc;
> >
> > +     if (spinand->flags & SPINAND_HAS_PP_PLANE_SELECT_BIT)
> > +             column |= req->pos.plane << fls(nanddev_page_size(nand));
>
> Please separate the core changes and the macronix changes.
> 1. Add support for the flag in the core (and the include)
> 2. Use the flag in Macronix driver.
>

Sure, I will separate them in the next version.

> > +
> >       while (nbytes) {
> >               ret = spi_mem_dirmap_write(wdesc, column, nbytes, buf);
> >               if (ret < 0)
> > diff --git a/drivers/mtd/nand/spi/macronix.c b/drivers/mtd/nand/spi/macronix.c
> > index 3f9e9c572854..a531cc8121ff 100644
> > --- a/drivers/mtd/nand/spi/macronix.c
> > +++ b/drivers/mtd/nand/spi/macronix.c
> > @@ -118,7 +118,8 @@ static const struct spinand_info macronix_spinand_table[] = {
> >                    SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> >                                             &write_cache_variants,
> >                                             &update_cache_variants),
> > -                  SPINAND_HAS_QE_BIT,
> > +                  SPINAND_HAS_QE_BIT | SPINAND_HAS_PP_PLANE_SELECT_BIT |
> > +                      SPINAND_HAS_READ_PLANE_SELECT_BIT,
>
> Alignment:           ^
>

Thanks for pointing that out; I'll take care of it.

>
>
> ...
>
> > @@ -263,7 +266,7 @@ static const struct spinand_info macronix_spinand_table[] = {
> >                    SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> >                                             &write_cache_variants,
> >                                             &update_cache_variants),
> > -                  SPINAND_HAS_QE_BIT,
> > +                  SPINAND_HAS_QE_BIT | SPINAND_HAS_PP_PLANE_SELECT_BIT,
> >                    SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout,
> >                                    mx35lf1ge4ab_ecc_get_status)),
> >       SPINAND_INFO("MX35UF2G24AD-Z4I8",
> > diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
> > index 5c19ead60499..cec451e7c71c 100644
> > --- a/include/linux/mtd/spinand.h
> > +++ b/include/linux/mtd/spinand.h
> > @@ -312,6 +312,8 @@ struct spinand_ecc_info {
> >
> >  #define SPINAND_HAS_QE_BIT           BIT(0)
> >  #define SPINAND_HAS_CR_FEAT_BIT              BIT(1)
> > +#define SPINAND_HAS_PP_PLANE_SELECT_BIT              BIT(2)
> > +#define SPINAND_HAS_READ_PLANE_SELECT_BIT            BIT(3)
>
> Do you think we can have the PP plane select bit without the read plane
> select bit? I'd use a single flag for now.

Macronix serial NAND flash with a two-plane structure always requires
the insertion of the Plane Select bit in the write_to_cache function. However,
only the MX35{U,F}2G14AC and MX35LF2GE4AB require the insertion of the
Plane Select bit in the read_from_cache function.

However, I have observed that for flash requiring the insertion of the
Plane Select
bit during the read_from_cache operation, the ECC strength is 4.

Can we use the ECC strength in conjunction with the
SPINAND_HAS_PP_PLANE_SELECT_BIT flag to determine
whether the Plane Select bit needs to be inserted during the
read_from_cache operation?

This method cannot guarantee that a new flash witha two-plane
structure requiring the insertion of the Plane Select bit will have
an ECC strength of 4.

Based on the above points, I think we need to use two separate flags
to handle these requirements effectively.

>
> >
> >  /**
> >   * struct spinand_ondie_ecc_conf - private SPI-NAND on-die ECC engine structure
>
>
> Thanks,
> Miquèl

Thanks,
Cheng Ming Lin