[PATCH v2 0/2] Add fixups for two-plane serial NAND flash

[PATCH v2 0/2] Add fixups for two-plane serial NAND flash

[Cheng Ming Lin]

From: Cheng Ming Lin <chengminglin@mxic.com.tw>

Macronix serial NAND flash with a two-plane structure
requires insertion of Plane Select bit into the column
address during the write_to_cache operation.

Additionally, for MX35{U,F}2G14AC, insertion of Plane
Select bit into the column address is required during
the read_from_cache operation.

These flashes have been validated on Xilinx zynq-picozed
board which included Macronix SPI Host.

Cheng Ming Lin (2):
  mtd: spinand: Add fixups for spinand
  mtd: spinand: macronix: Fixups for Plane Select bit

 drivers/mtd/nand/spi/core.c     |  7 ++++
 drivers/mtd/nand/spi/macronix.c | 66 ++++++++++++++++++++++++++++++---
 include/linux/mtd/spinand.h     | 17 +++++++++
 3 files changed, 84 insertions(+), 6 deletions(-)

-- 
2.25.1

[PATCH v2 1/2] mtd: spinand: Add fixups for spinand

[Cheng Ming Lin]

From: Cheng Ming Lin <chengminglin@mxic.com.tw>

Add struct spi_nand_fixups as SPI NAND fixup hooks.

To determine whether the Plane Select bit should be
inserted into the column address, add the member
struct spinand_info to struct spinand_device to
ascertain whether the device has fixups.

In the function spinand_read_from_cache_op and
spinand_write_to_cache_op, add an if statement
to determine whether the device has fixups and
their corresponding functions. If so, give the
Plane Select bit to the column address.

In the function spinand_match_and_init, add
spinand_info in spinand_device for determing
whether Plane Select bitg should be inserted.

Signed-off-by: Cheng Ming Lin <chengminglin@mxic.com.tw>
---
 drivers/mtd/nand/spi/core.c |  7 +++++++
 include/linux/mtd/spinand.h | 17 +++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index e0b6715e5dfe..d6d6f3832f9d 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->info->fixups && spinand->info->fixups->read_from_cache)
+		column = spinand->info->fixups->read_from_cache(spinand, req, column);
+
 	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->info->fixups && spinand->info->fixups->write_to_cache)
+		column = spinand->info->fixups->write_to_cache(spinand, req, column);
+
 	while (nbytes) {
 		ret = spi_mem_dirmap_write(wdesc, column, nbytes, buf);
 		if (ret < 0)
@@ -1095,6 +1101,7 @@ int spinand_match_and_init(struct spinand_device *spinand,
 		spinand->flags = table[i].flags;
 		spinand->id.len = 1 + table[i].devid.len;
 		spinand->select_target = table[i].select_target;
+		spinand->info = info;
 
 		op = spinand_select_op_variant(spinand,
 					       info->op_variants.read_cache);
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index 5c19ead60499..c079c6ac1541 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -354,6 +354,7 @@ struct spinand_info {
 	} op_variants;
 	int (*select_target)(struct spinand_device *spinand,
 			     unsigned int target);
+	const struct spi_nand_fixups *fixups;
 };
 
 #define SPINAND_ID(__method, ...)					\
@@ -379,6 +380,9 @@ struct spinand_info {
 #define SPINAND_SELECT_TARGET(__func)					\
 	.select_target = __func,
 
+#define SPINAND_PLANE_SELECT_BIT(__func)					\
+	.fixups = __func,
+
 #define SPINAND_INFO(__model, __id, __memorg, __eccreq, __op_variants,	\
 		     __flags, ...)					\
 	{								\
@@ -398,6 +402,18 @@ struct spinand_dirmap {
 	struct spi_mem_dirmap_desc *rdesc_ecc;
 };
 
+/**
+ * struct spi_nand_fixups - SPI NAND fixup hooks
+ * @write_to_cache: program load requires Plane Select bit in CADD.
+ * @read_from_cache: read from cache requires Plane Select bit in CADD.
+ */
+struct spi_nand_fixups {
+	unsigned int (*write_to_cache)(struct spinand_device *spinand,
+				const struct nand_page_io_req *req, unsigned int column);
+	u16 (*read_from_cache)(struct spinand_device *spinand,
+				const struct nand_page_io_req *req, u16 column);
+};
+
 /**
  * struct spinand_device - SPI NAND device instance
  * @base: NAND device instance
@@ -449,6 +465,7 @@ struct spinand_device {
 	u8 *databuf;
 	u8 *oobbuf;
 	u8 *scratchbuf;
+	const struct spinand_info *info;
 	const struct spinand_manufacturer *manufacturer;
 	void *priv;
 };
-- 
2.25.1

[PATCH v2 2/2] mtd: spinand: macronix: Fixups for Plane Select bit

[Cheng Ming Lin]

From: Cheng Ming Lin <chengminglin@mxic.com.tw>

Macronix serial NAND flash with a two-plane structure
requires insertion of Plane Select bit into the column
address during the write_to_cache operation.

Additionally, for MX35{U,F}2G14AC, insertion of Plane
Select bit into the column address is required during
the read_from_cache operation.

Signed-off-by: Cheng Ming Lin <chengminglin@mxic.com.tw>
---
 drivers/mtd/nand/spi/macronix.c | 66 ++++++++++++++++++++++++++++++---
 1 file changed, 60 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/spi/macronix.c b/drivers/mtd/nand/spi/macronix.c
index 3f9e9c572854..eda67091edc0 100644
--- a/drivers/mtd/nand/spi/macronix.c
+++ b/drivers/mtd/nand/spi/macronix.c
@@ -100,6 +100,54 @@ static int mx35lf1ge4ab_ecc_get_status(struct spinand_device *spinand,
 	return -EINVAL;
 }
 
+/**
+ * write_plane_select_bit_in_cadd - Write Plane Select bit to the column address
+ * @spinand: SPI NAND device
+ * @req: NAND I/O request object
+ * @column: the column address
+ *
+ * Macronix serial NAND flash with a two-plane structure
+ * should insert Plane Select bit into the column address
+ * during the write_to_cache operation.
+ *
+ * Return: the column address after insertion of Plane Select bit
+ */
+static unsigned int write_plane_select_bit_in_cadd(struct spinand_device *spinand,
+			const struct nand_page_io_req *req, unsigned int column)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+
+	return column | (req->pos.plane << fls(nanddev_page_size(nand)));
+}
+
+/**
+ * read_plane_select_bit_in_cadd - Write Plane Select bit to the column address
+ * @spinand: SPI NAND device
+ * @req: NAND I/O request object
+ * @column: the column address
+ *
+ * MX35{U,F}2G14AC also need to insert Plane Select bit
+ * into the column address during the read_from_cache operation.
+ *
+ * Return: the column address after insertion of Plane Select bit
+ */
+static u16 read_plane_select_bit_in_cadd(struct spinand_device *spinand,
+			const struct nand_page_io_req *req, u16 column)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+
+	return column | (req->pos.plane << fls(nanddev_page_size(nand)));
+}
+
+static const struct spi_nand_fixups write_fixups = {
+	.write_to_cache = write_plane_select_bit_in_cadd,
+};
+
+static const struct spi_nand_fixups read_and_write_fixups = {
+	.write_to_cache = write_plane_select_bit_in_cadd,
+	.read_from_cache = read_plane_select_bit_in_cadd,
+};
+
 static const struct spinand_info macronix_spinand_table[] = {
 	SPINAND_INFO("MX35LF1GE4AB",
 		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x12),
@@ -157,7 +205,8 @@ static const struct spinand_info macronix_spinand_table[] = {
 					      &write_cache_variants,
 					      &update_cache_variants),
 		     SPINAND_HAS_QE_BIT,
-		     SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, NULL)),
+		     SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, NULL),
+			 SPINAND_PLANE_SELECT_BIT(&write_fixups)),
 	SPINAND_INFO("MX35LF2G24AD-Z4I8",
 		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x64, 0x03),
 		     NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 1, 1, 1),
@@ -175,7 +224,8 @@ static const struct spinand_info macronix_spinand_table[] = {
 					      &write_cache_variants,
 					      &update_cache_variants),
 		     SPINAND_HAS_QE_BIT,
-		     SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, NULL)),
+		     SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout, NULL),
+			 SPINAND_PLANE_SELECT_BIT(&write_fixups)),
 	SPINAND_INFO("MX35LF4G24AD-Z4I8",
 		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x75, 0x03),
 		     NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1),
@@ -215,7 +265,8 @@ static const struct spinand_info macronix_spinand_table[] = {
 					      &update_cache_variants),
 		     SPINAND_HAS_QE_BIT,
 		     SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout,
-				     mx35lf1ge4ab_ecc_get_status)),
+				     mx35lf1ge4ab_ecc_get_status),
+			 SPINAND_PLANE_SELECT_BIT(&read_and_write_fixups)),
 	SPINAND_INFO("MX35UF4G24AD",
 		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xb5, 0x03),
 		     NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 2, 1, 1),
@@ -225,7 +276,8 @@ static const struct spinand_info macronix_spinand_table[] = {
 					      &update_cache_variants),
 		     SPINAND_HAS_QE_BIT,
 		     SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout,
-				     mx35lf1ge4ab_ecc_get_status)),
+				     mx35lf1ge4ab_ecc_get_status),
+			 SPINAND_PLANE_SELECT_BIT(&write_fixups)),
 	SPINAND_INFO("MX35UF4G24AD-Z4I8",
 		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xf5, 0x03),
 		     NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1),
@@ -255,7 +307,8 @@ static const struct spinand_info macronix_spinand_table[] = {
 					      &update_cache_variants),
 		     SPINAND_HAS_QE_BIT,
 		     SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout,
-				     mx35lf1ge4ab_ecc_get_status)),
+				     mx35lf1ge4ab_ecc_get_status),
+			 SPINAND_PLANE_SELECT_BIT(&read_and_write_fixups)),
 	SPINAND_INFO("MX35UF2G24AD",
 		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xa4, 0x03),
 		     NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 2, 1, 1),
@@ -265,7 +318,8 @@ static const struct spinand_info macronix_spinand_table[] = {
 					      &update_cache_variants),
 		     SPINAND_HAS_QE_BIT,
 		     SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout,
-				     mx35lf1ge4ab_ecc_get_status)),
+				     mx35lf1ge4ab_ecc_get_status),
+			 SPINAND_PLANE_SELECT_BIT(&write_fixups)),
 	SPINAND_INFO("MX35UF2G24AD-Z4I8",
 		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xe4, 0x03),
 		     NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 1, 1, 1),
-- 
2.25.1

Re: [PATCH v2 0/2] Add fixups for two-plane serial NAND flash

[Miquel Raynal]

Hi Cheng,

linchengming884@gmail.com wrote on Thu, 18 Jul 2024 15:53:54 +0800:

> From: Cheng Ming Lin <chengminglin@mxic.com.tw>
> 
> Macronix serial NAND flash with a two-plane structure
> requires insertion of Plane Select bit into the column
> address during the write_to_cache operation.
> 
> Additionally, for MX35{U,F}2G14AC, insertion of Plane
> Select bit into the column address is required during
> the read_from_cache operation.

I guess if the plane bit is needed for your chips, it is also needed
for other whips with two planes? Could it be possible that we never had
support for devices with more than one plane and you just fall into a
common issue? Maybe we should always add the plane information when
there is more than one plane to address? Can you check whether this is
specific to Macronix or not?

In this case we wouldn't need a specific fixup.

> 
> These flashes have been validated on Xilinx zynq-picozed
> board which included Macronix SPI Host.
> 
> Cheng Ming Lin (2):
>   mtd: spinand: Add fixups for spinand
>   mtd: spinand: macronix: Fixups for Plane Select bit
> 
>  drivers/mtd/nand/spi/core.c     |  7 ++++
>  drivers/mtd/nand/spi/macronix.c | 66 ++++++++++++++++++++++++++++++---
>  include/linux/mtd/spinand.h     | 17 +++++++++
>  3 files changed, 84 insertions(+), 6 deletions(-)
> 


Thanks,
Miquèl

Re: [PATCH v2 0/2] Add fixups for two-plane serial NAND flash

[Cheng Ming Lin]

Hi Miquel,

Miquel Raynal <miquel.raynal@bootlin.com> 於 2024年8月12日 週一 下午4:56寫道：
>
> Hi Cheng,
>
> linchengming884@gmail.com wrote on Thu, 18 Jul 2024 15:53:54 +0800:
>
> > From: Cheng Ming Lin <chengminglin@mxic.com.tw>
> >
> > Macronix serial NAND flash with a two-plane structure
> > requires insertion of Plane Select bit into the column
> > address during the write_to_cache operation.
> >
> > Additionally, for MX35{U,F}2G14AC, insertion of Plane
> > Select bit into the column address is required during
> > the read_from_cache operation.
>
> I guess if the plane bit is needed for your chips, it is also needed
> for other whips with two planes? Could it be possible that we never had
> support for devices with more than one plane and you just fall into a
> common issue? Maybe we should always add the plane information when
> there is more than one plane to address? Can you check whether this is
> specific to Macronix or not?
>

I have reviewed the chips listed by each vendor.

Micron offers MT29F2G01AB{A,B}GD, MT29F2G01AAAED with two planes;
however, only MT29F2G01AAAED requires the plane select bit when performing
program load or read from cache.

Link: https://semiconductors.es/pdf-down/M/T/2/MT29F2G01AAAED-MicronTechnology.pdf

Winbond provides W25N04KV with two planes, but it does not require the plane
select bit for program load or cache read operations.

Therefore, we should not always include the plane select bit when dealing with
multiple planes.

> In this case we wouldn't need a specific fixup.
>

Based on the above perspective, do we still need to use fixup, or can we use
flags to determine whether the plane select bit is necessary?

> >
> > These flashes have been validated on Xilinx zynq-picozed
> > board which included Macronix SPI Host.
> >
> > Cheng Ming Lin (2):
> >   mtd: spinand: Add fixups for spinand
> >   mtd: spinand: macronix: Fixups for Plane Select bit
> >
> >  drivers/mtd/nand/spi/core.c     |  7 ++++
> >  drivers/mtd/nand/spi/macronix.c | 66 ++++++++++++++++++++++++++++++---
> >  include/linux/mtd/spinand.h     | 17 +++++++++
> >  3 files changed, 84 insertions(+), 6 deletions(-)
> >
>
>
> Thanks,
> Miquèl

Thanks,
ChengMing Lin

Re: [PATCH v2 0/2] Add fixups for two-plane serial NAND flash

[Miquel Raynal]

Hi Cheng,

linchengming884@gmail.com wrote on Tue, 13 Aug 2024 14:02:44 +0800:

> Hi Miquel,
> 
> Miquel Raynal <miquel.raynal@bootlin.com> 於 2024年8月12日 週一 下午4:56寫道：
> >
> > Hi Cheng,
> >
> > linchengming884@gmail.com wrote on Thu, 18 Jul 2024 15:53:54 +0800:
> >  
> > > From: Cheng Ming Lin <chengminglin@mxic.com.tw>
> > >
> > > Macronix serial NAND flash with a two-plane structure
> > > requires insertion of Plane Select bit into the column
> > > address during the write_to_cache operation.
> > >
> > > Additionally, for MX35{U,F}2G14AC, insertion of Plane
> > > Select bit into the column address is required during
> > > the read_from_cache operation.  
> >
> > I guess if the plane bit is needed for your chips, it is also needed
> > for other whips with two planes? Could it be possible that we never had
> > support for devices with more than one plane and you just fall into a
> > common issue? Maybe we should always add the plane information when
> > there is more than one plane to address? Can you check whether this is
> > specific to Macronix or not?
> >  
> 
> I have reviewed the chips listed by each vendor.
> 
> Micron offers MT29F2G01AB{A,B}GD, MT29F2G01AAAED with two planes;
> however, only MT29F2G01AAAED requires the plane select bit when performing
> program load or read from cache.
> 
> Link: https://semiconductors.es/pdf-down/M/T/2/MT29F2G01AAAED-MicronTechnology.pdf
> 
> Winbond provides W25N04KV with two planes, but it does not require the plane
> select bit for program load or cache read operations.
> 
> Therefore, we should not always include the plane select bit when dealing with
> multiple planes.
> 
> > In this case we wouldn't need a specific fixup.
> >  
> 
> Based on the above perspective, do we still need to use fixup, or can we use
> flags to determine whether the plane select bit is necessary?

I still prefer flags rather than invasive fixup hooks.

> 
> > >
> > > These flashes have been validated on Xilinx zynq-picozed
> > > board which included Macronix SPI Host.
> > >
> > > Cheng Ming Lin (2):
> > >   mtd: spinand: Add fixups for spinand
> > >   mtd: spinand: macronix: Fixups for Plane Select bit
> > >
> > >  drivers/mtd/nand/spi/core.c     |  7 ++++
> > >  drivers/mtd/nand/spi/macronix.c | 66 ++++++++++++++++++++++++++++++---
> > >  include/linux/mtd/spinand.h     | 17 +++++++++
> > >  3 files changed, 84 insertions(+), 6 deletions(-)
> > >  
> >
> >
> > Thanks,
> > Miquèl  
> 
> Thanks,
> ChengMing Lin


Thanks,
Miquèl