| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAAyq3SbLATwXbDN6icx44cm8-PX+DrUvq2xiYor7bLn4wmfVrw@mail.gmail.com>
Date: Wed, 28 Aug 2024 17:01:41 +0800
From: Cheng Ming Lin <linchengming884@...il.com>
To: Miquel Raynal <miquel.raynal@...tlin.com>
Cc: vigneshr@...com, linux-mtd@...ts.infradead.org,
linux-kernel@...r.kernel.org, richard@....at, alvinzhou@...c.com.tw,
leoyu@...c.com.tw, Cheng Ming Lin <chengminglin@...c.com.tw>
Subject: Re: [PATCH v3 1/1] mtd: spinand: Add flags for the Plane Select bit
Hi Miquel,
Miquel Raynal <miquel.raynal@...tlin.com> 於 2024年8月28日 週三 下午3:47寫道：
>
> Hi Cheng,
>
> linchengming884@...il.com wrote on Wed, 28 Aug 2024 14:21:31 +0800:
>
> > From: Cheng Ming Lin <chengminglin@...c.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@...c.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
Powered by blists - more mailing lists