[PATCH v1 0/4] mtd: spinand: Add continuous read mode support

[PATCH v1 0/4] mtd: spinand: Add continuous read mode support

[JaimeLiao]

This series add support continuous read mode for SPI NAND.

JaimeLiao (4):
  mtd: spinand: Add support continuous read mode
  mtd: spinand: Add continuous read state
  mtd: spinand: Add support continuous read operation
  mtd: spinand: macronix: Add continuous read support for MX35LF2GE4AD

 drivers/mtd/nand/spi/core.c     | 118 ++++++++++++++++++++++++++++++++
 drivers/mtd/nand/spi/macronix.c |   2 +-
 include/linux/mtd/spinand.h     |   4 ++
 3 files changed, 123 insertions(+), 1 deletion(-)

-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v1 1/4] mtd: spinand: Add support continuous read mode

[JaimeLiao]

The patch supports setting the "CONT" bit of the configuration
register and create spinand_continuous_read_enable/disable functions.

Signed-off-by: JaimeLiao <jaimeliao.tw@gmail.com>
---
 drivers/mtd/nand/spi/core.c | 11 +++++++++++
 include/linux/mtd/spinand.h |  1 +
 2 files changed, 12 insertions(+)

diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 2c8685f1f2fa..dc2e1dc1dbee 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -193,6 +193,17 @@ static int spinand_init_quad_enable(struct spinand_device *spinand)
 			       enable ? CFG_QUAD_ENABLE : 0);
 }
 
+static int spinand_continuous_read_enable(struct spinand_device *spinand)
+{
+	return spinand_upd_cfg(spinand, CFG_CONT_READ_ENABLE,
+			       CFG_CONT_READ_ENABLE);
+}
+
+static int spinand_continuous_read_disable(struct spinand_device *spinand)
+{
+	return spinand_upd_cfg(spinand, CFG_CONT_READ_ENABLE, 0);
+}
+
 static int spinand_ecc_enable(struct spinand_device *spinand,
 			      bool enable)
 {
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index 6988956b8492..37b46c556bd7 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -154,6 +154,7 @@
 #define REG_CFG			0xb0
 #define CFG_OTP_ENABLE		BIT(6)
 #define CFG_ECC_ENABLE		BIT(4)
+#define CFG_CONT_READ_ENABLE	BIT(2)
 #define CFG_QUAD_ENABLE		BIT(0)
 
 /* status register */
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v1 2/4] mtd: spinand: Add continuous read state

[JaimeLiao]

Add continuous read state and initialize state to
default false.

Signed-off-by: JaimeLiao <jaimeliao.tw@gmail.com>
---
 drivers/mtd/nand/spi/core.c | 8 ++++++++
 include/linux/mtd/spinand.h | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index dc2e1dc1dbee..380411feab6c 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -193,6 +193,11 @@ static int spinand_init_quad_enable(struct spinand_device *spinand)
 			       enable ? CFG_QUAD_ENABLE : 0);
 }
 
+static void spinand_init_continuous_read(struct spinand_device *spinand)
+{
+	spinand->use_continuous_read = false;
+}
+
 static int spinand_continuous_read_enable(struct spinand_device *spinand)
 {
 	return spinand_upd_cfg(spinand, CFG_CONT_READ_ENABLE,
@@ -1261,6 +1266,9 @@ static int spinand_init(struct spinand_device *spinand)
 	mtd->ecc_strength = nanddev_get_ecc_conf(nand)->strength;
 	mtd->ecc_step_size = nanddev_get_ecc_conf(nand)->step_size;
 
+	/* Init continuous read */
+	spinand_init_continuous_read(spinand);
+
 	return 0;
 
 err_cleanup_ecc_engine:
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index 37b46c556bd7..d70a6f49cc6f 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -415,6 +415,7 @@ struct spinand_dirmap {
  *		because the spi-mem interface explicitly requests that buffers
  *		passed in spi_mem_op be DMA-able, so we can't based the bufs on
  *		the stack
+ * @use_continuous_read: record the continuous read status
  * @manufacturer: SPI NAND manufacturer information
  * @priv: manufacturer private data
  */
@@ -443,6 +444,7 @@ struct spinand_device {
 	u8 *databuf;
 	u8 *oobbuf;
 	u8 *scratchbuf;
+	bool use_continuous_read;
 	const struct spinand_manufacturer *manufacturer;
 	void *priv;
 };
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v1 3/4] mtd: spinand: Add support continuous read operation

[JaimeLiao]

The continuous read operation includes three phases:
Firstly, starting with the page read command and the 1st
page data will be read into the cache after the read latency tRD.
Secondly, Issuing the Read From Cache commands
(03h/0Bh/3Bh/6Bh/BBh/EBh) to read out the data from cache continuously.
After all the data is read out, the host should pull CS# high
to terminate this continuous read operation and wait tRST for the NAND
device resets read operation.

Continuous reads have a positive impact when reading more than
one page and the column address is don't care in this operation,
a full page data will be read out for each page.

The performance of continuous read mode is as follows. Set the
flash to QUAD mode and run 25MZ direct mapping mode on the SPI
bus and use the MTD test module to show the performance of
continuous reads.

=================================================
mtd_speedtest: MTD device: 0    count: 100
mtd_speedtest: MTD device size 268435456, eraseblock size 131072,
               page size 2048, count of eraseblocks 2048, pages per
               eraseblock 64, OOB size 64
mtd_test: scanning for bad eraseblocks
mtd_test: scanned 100 eraseblocks, 0 are bad
mtd_speedtest: testing eraseblock write speed
mtd_speedtest: eraseblock write speed is 1298 KiB/s
mtd_speedtest: testing eraseblock read speed
mtd_speedtest: eraseblock read speed is 11053 KiB/s
mtd_speedtest: testing page write speed
mtd_speedtest: page write speed is 1291 KiB/s
mtd_speedtest: testing page read speed
mtd_speedtest: page read speed is 3240 KiB/s
mtd_speedtest: testing 2 page write speed
mtd_speedtest: 2 page write speed is 1289 KiB/s
mtd_speedtest: testing 2 page read speed
mtd_speedtest: 2 page read speed is 2909 KiB/s
mtd_speedtest: Testing erase speed
mtd_speedtest: erase speed is 45229 KiB/s
mtd_speedtest: Testing 2x multi-block erase speed
mtd_speedtest: 2x multi-block erase speed is 62135 KiB/s
mtd_speedtest: Testing 4x multi-block erase speed
mtd_speedtest: 4x multi-block erase speed is 60093 KiB/s
mtd_speedtest: Testing 8x multi-block erase speed
mtd_speedtest: 8x multi-block erase speed is 61244 KiB/s
mtd_speedtest: Testing 16x multi-block erase speed
mtd_speedtest: 16x multi-block erase speed is 61538 KiB/s
mtd_speedtest: Testing 32x multi-block erase speed
mtd_speedtest: 32x multi-block erase speed is 61835 KiB/s
mtd_speedtest: Testing 64x multi-block erase speed
mtd_speedtest: 64x multi-block erase speed is 60663 KiB/s
mtd_speedtest: finished
=================================================

Signed-off-by: JaimeLiao <jaimeliao.tw@gmail.com>
---
 drivers/mtd/nand/spi/core.c | 99 +++++++++++++++++++++++++++++++++++++
 include/linux/mtd/spinand.h |  1 +
 2 files changed, 100 insertions(+)

diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 380411feab6c..ef09bedda8d4 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -386,6 +386,10 @@ static int spinand_read_from_cache_op(struct spinand_device *spinand,
 	if (req->datalen) {
 		buf = spinand->databuf;
 		nbytes = nanddev_page_size(nand);
+		if (spinand->use_continuous_read) {
+			buf = req->databuf.in;
+			nbytes = req->datalen;
+		}
 		column = 0;
 	}
 
@@ -412,6 +416,9 @@ static int spinand_read_from_cache_op(struct spinand_device *spinand,
 		buf += ret;
 	}
 
+	if (spinand->use_continuous_read)
+		return 0;
+
 	if (req->datalen)
 		memcpy(req->databuf.in, spinand->databuf + req->dataoffs,
 		       req->datalen);
@@ -640,6 +647,80 @@ static int spinand_write_page(struct spinand_device *spinand,
 	return nand_ecc_finish_io_req(nand, (struct nand_page_io_req *)req);
 }
 
+static int spinand_mtd_continuous_read(struct mtd_info *mtd, loff_t from,
+				       struct mtd_oob_ops *ops,
+				       struct nand_io_iter *iter)
+{
+	struct spinand_device *spinand = mtd_to_spinand(mtd);
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	int ret = 0;
+
+	/*
+	 * Continuous read mode could reduce some operation in On-die ECC free
+	 * flash when read page sequentially.
+	 */
+	iter->req.type = NAND_PAGE_READ;
+	iter->req.mode = MTD_OPS_RAW;
+	iter->req.dataoffs = nanddev_offs_to_pos(nand, from, &iter->req.pos);
+	iter->req.databuf.in = ops->datbuf;
+	iter->req.datalen = ops->len;
+
+	if (from & (nanddev_page_size(nand) - 1)) {
+		pr_debug("%s: unaligned address\n", __func__);
+		return -EINVAL;
+	}
+
+	ret = spinand_continuous_read_enable(spinand);
+	if (ret)
+		return ret;
+
+	spinand->use_continuous_read = true;
+
+	ret = spinand_select_target(spinand, iter->req.pos.target);
+	if (ret)
+		return ret;
+
+	/*
+	 * The continuous read operation including: firstly, starting with the
+	 * page read command and the 1 st page data will be read into the cache
+	 * after the read latency tRD. Secondly, Issuing the Read From Cache
+	 * commands (03h/0Bh/3Bh/6Bh/BBh/EBh) to read out the data from cache
+	 * continuously.
+	 *
+	 * The cache is divided into two halves, while one half of the cache is
+	 * outputting the data, the other half will be loaded for the new data;
+	 * therefore, the host can read out the data continuously from page to
+	 * page. Multiple of Read From Cache commands can be issued in one
+	 * continuous read operation, each Read From Cache command is required
+	 * to read multiple 4-byte data exactly; otherwise, the data output will
+	 * be out of sequence from one Read From Cache command to another Read
+	 * From Cache command.
+	 *
+	 * After all the data is read out, the host should pull CS# high to
+	 * terminate this continuous read operation and wait a 6us of tRST for
+	 * the NAND device resets read operation. The data output for each page
+	 * will always start from byte 0 and a full page data should be read out
+	 * for each page.
+	 */
+	ret = spinand_read_page(spinand, &iter->req);
+	if (ret)
+		return ret;
+
+	ret = spinand_reset_op(spinand);
+	if (ret)
+		return ret;
+
+	ret = spinand_continuous_read_disable(spinand);
+	if (ret)
+		return ret;
+
+	spinand->use_continuous_read = false;
+
+	ops->retlen = iter->req.datalen;
+
+	return ret;
+}
+
 static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
 			    struct mtd_oob_ops *ops)
 {
@@ -656,6 +737,24 @@ static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
 
 	mutex_lock(&spinand->lock);
 
+	/*
+	 * If the device support continuous read mode and read length larger
+	 * than one page size will enter the continuous read mode. This mode
+	 * helps avoid issuing a page read command and read from cache command
+	 * again, and improves read performance for continuous addresses.
+	 */
+	if ((spinand->flags & SPINAND_HAS_CONT_READ_BIT) &&
+	    (ops->len > nanddev_page_size(nand))) {
+		ret = spinand_mtd_continuous_read(mtd, from, ops, &iter);
+
+		mutex_unlock(&spinand->lock);
+
+		if (ecc_failed && !ret)
+			ret = -EBADMSG;
+
+		return ret ? ret : max_bitflips;
+	}
+
 	nanddev_io_for_each_page(nand, NAND_PAGE_READ, from, ops, &iter) {
 		if (disable_ecc)
 			iter.req.mode = MTD_OPS_RAW;
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index d70a6f49cc6f..c296d4d0dda5 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -308,6 +308,7 @@ struct spinand_ecc_info {
 
 #define SPINAND_HAS_QE_BIT		BIT(0)
 #define SPINAND_HAS_CR_FEAT_BIT		BIT(1)
+#define SPINAND_HAS_CONT_READ_BIT	BIT(2)
 
 /**
  * struct spinand_ondie_ecc_conf - private SPI-NAND on-die ECC engine structure
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v1 3/4] mtd: spinand: Add support continuous read operation

[Miquel Raynal]

Hi JaimeLiao,

jaimeliao.tw@gmail.com wrote on Thu, 11 Aug 2022 17:45:35 +0800:

> The continuous read operation includes three phases:
> Firstly, starting with the page read command and the 1st
> page data will be read into the cache after the read latency tRD.
> Secondly, Issuing the Read From Cache commands
> (03h/0Bh/3Bh/6Bh/BBh/EBh) to read out the data from cache continuously.
> After all the data is read out, the host should pull CS# high
> to terminate this continuous read operation and wait tRST for the NAND
> device resets read operation.
> 
> Continuous reads have a positive impact when reading more than
> one page and the column address is don't care in this operation,
> a full page data will be read out for each page.
> 
> The performance of continuous read mode is as follows. Set the
> flash to QUAD mode and run 25MZ direct mapping mode on the SPI

			       MHz

> bus and use the MTD test module to show the performance of
> continuous reads.

Please show before and after changes and just give us the lines
regarding the read speeds rather than all the output of the tool.

> 
> =================================================
> mtd_speedtest: MTD device: 0    count: 100
> mtd_speedtest: MTD device size 268435456, eraseblock size 131072,
>                page size 2048, count of eraseblocks 2048, pages per
>                eraseblock 64, OOB size 64
> mtd_test: scanning for bad eraseblocks
> mtd_test: scanned 100 eraseblocks, 0 are bad
> mtd_speedtest: testing eraseblock write speed
> mtd_speedtest: eraseblock write speed is 1298 KiB/s
> mtd_speedtest: testing eraseblock read speed
> mtd_speedtest: eraseblock read speed is 11053 KiB/s
> mtd_speedtest: testing page write speed
> mtd_speedtest: page write speed is 1291 KiB/s
> mtd_speedtest: testing page read speed
> mtd_speedtest: page read speed is 3240 KiB/s
> mtd_speedtest: testing 2 page write speed
> mtd_speedtest: 2 page write speed is 1289 KiB/s
> mtd_speedtest: testing 2 page read speed
> mtd_speedtest: 2 page read speed is 2909 KiB/s
> mtd_speedtest: Testing erase speed
> mtd_speedtest: erase speed is 45229 KiB/s
> mtd_speedtest: Testing 2x multi-block erase speed
> mtd_speedtest: 2x multi-block erase speed is 62135 KiB/s
> mtd_speedtest: Testing 4x multi-block erase speed
> mtd_speedtest: 4x multi-block erase speed is 60093 KiB/s
> mtd_speedtest: Testing 8x multi-block erase speed
> mtd_speedtest: 8x multi-block erase speed is 61244 KiB/s
> mtd_speedtest: Testing 16x multi-block erase speed
> mtd_speedtest: 16x multi-block erase speed is 61538 KiB/s
> mtd_speedtest: Testing 32x multi-block erase speed
> mtd_speedtest: 32x multi-block erase speed is 61835 KiB/s
> mtd_speedtest: Testing 64x multi-block erase speed
> mtd_speedtest: 64x multi-block erase speed is 60663 KiB/s
> mtd_speedtest: finished
> =================================================
> 
> Signed-off-by: JaimeLiao <jaimeliao.tw@gmail.com>
> ---
>  drivers/mtd/nand/spi/core.c | 99 +++++++++++++++++++++++++++++++++++++
>  include/linux/mtd/spinand.h |  1 +
>  2 files changed, 100 insertions(+)
> 
> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index 380411feab6c..ef09bedda8d4 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -386,6 +386,10 @@ static int spinand_read_from_cache_op(struct spinand_device *spinand,
>  	if (req->datalen) {
>  		buf = spinand->databuf;
>  		nbytes = nanddev_page_size(nand);
> +		if (spinand->use_continuous_read) {
> +			buf = req->databuf.in;
> +			nbytes = req->datalen;
> +		}

Does not look right. A cache read is still one page, and I believe the
caller should have taken care of that?

>  		column = 0;
>  	}
>  
> @@ -412,6 +416,9 @@ static int spinand_read_from_cache_op(struct spinand_device *spinand,
>  		buf += ret;
>  	}
>  
> +	if (spinand->use_continuous_read)
> +		return 0;

For me this is not the right place.

> +
>  	if (req->datalen)
>  		memcpy(req->databuf.in, spinand->databuf + req->dataoffs,
>  		       req->datalen);
> @@ -640,6 +647,80 @@ static int spinand_write_page(struct spinand_device *spinand,
>  	return nand_ecc_finish_io_req(nand, (struct nand_page_io_req *)req);
>  }
>  
> +static int spinand_mtd_continuous_read(struct mtd_info *mtd, loff_t from,
> +				       struct mtd_oob_ops *ops,
> +				       struct nand_io_iter *iter)
> +{
> +	struct spinand_device *spinand = mtd_to_spinand(mtd);
> +	struct nand_device *nand = mtd_to_nanddev(mtd);
> +	int ret = 0;
> +
> +	/*
> +	 * Continuous read mode could reduce some operation in On-die ECC free
> +	 * flash when read page sequentially.
> +	 */
> +	iter->req.type = NAND_PAGE_READ;
> +	iter->req.mode = MTD_OPS_RAW;

I don't get what you do here.

Continuous read shall either work with ECC engines or just not be
implemented at all. You cannot blindly disable ECC support.

> +	iter->req.dataoffs = nanddev_offs_to_pos(nand, from, &iter->req.pos);
> +	iter->req.databuf.in = ops->datbuf;
> +	iter->req.datalen = ops->len;
> +
> +	if (from & (nanddev_page_size(nand) - 1)) {
> +		pr_debug("%s: unaligned address\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	ret = spinand_continuous_read_enable(spinand);
> +	if (ret)
> +		return ret;
> +
> +	spinand->use_continuous_read = true;
> +
> +	ret = spinand_select_target(spinand, iter->req.pos.target);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * The continuous read operation including: firstly, starting with the
> +	 * page read command and the 1 st page data will be read into the cache
> +	 * after the read latency tRD. Secondly, Issuing the Read From Cache
> +	 * commands (03h/0Bh/3Bh/6Bh/BBh/EBh) to read out the data from cache
> +	 * continuously.
> +	 *
> +	 * The cache is divided into two halves, while one half of the cache is
> +	 * outputting the data, the other half will be loaded for the new data;
> +	 * therefore, the host can read out the data continuously from page to
> +	 * page. Multiple of Read From Cache commands can be issued in one
> +	 * continuous read operation, each Read From Cache command is required
> +	 * to read multiple 4-byte data exactly; otherwise, the data output will
> +	 * be out of sequence from one Read From Cache command to another Read
> +	 * From Cache command.
> +	 *
> +	 * After all the data is read out, the host should pull CS# high to
> +	 * terminate this continuous read operation and wait a 6us of tRST for
> +	 * the NAND device resets read operation. The data output for each page
> +	 * will always start from byte 0 and a full page data should be read out
> +	 * for each page.
> +	 */
> +	ret = spinand_read_page(spinand, &iter->req);
> +	if (ret)
> +		return ret;
> +
> +	ret = spinand_reset_op(spinand);
> +	if (ret)
> +		return ret;
> +
> +	ret = spinand_continuous_read_disable(spinand);
> +	if (ret)
> +		return ret;
> +
> +	spinand->use_continuous_read = false;

Any error in this function should lead to resetting use_continuous_read
to false and disabling continuous read on the flash side.

> +
> +	ops->retlen = iter->req.datalen;
> +
> +	return ret;
> +}
> +
>  static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
>  			    struct mtd_oob_ops *ops)
>  {
> @@ -656,6 +737,24 @@ static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
>  
>  	mutex_lock(&spinand->lock);
>  
> +	/*
> +	 * If the device support continuous read mode and read length larger
> +	 * than one page size will enter the continuous read mode. This mode
> +	 * helps avoid issuing a page read command and read from cache command
> +	 * again, and improves read performance for continuous addresses.
> +	 */
> +	if ((spinand->flags & SPINAND_HAS_CONT_READ_BIT) &&
> +	    (ops->len > nanddev_page_size(nand))) {
> +		ret = spinand_mtd_continuous_read(mtd, from, ops, &iter);
> +
> +		mutex_unlock(&spinand->lock);
> +
> +		if (ecc_failed && !ret)
> +			ret = -EBADMSG;
> +
> +		return ret ? ret : max_bitflips;
> +	}
> +
>  	nanddev_io_for_each_page(nand, NAND_PAGE_READ, from, ops, &iter) {
>  		if (disable_ecc)
>  			iter.req.mode = MTD_OPS_RAW;
> diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
> index d70a6f49cc6f..c296d4d0dda5 100644
> --- a/include/linux/mtd/spinand.h
> +++ b/include/linux/mtd/spinand.h
> @@ -308,6 +308,7 @@ struct spinand_ecc_info {
>  
>  #define SPINAND_HAS_QE_BIT		BIT(0)
>  #define SPINAND_HAS_CR_FEAT_BIT		BIT(1)
> +#define SPINAND_HAS_CONT_READ_BIT	BIT(2)

Is this bit standard?

>  
>  /**
>   * struct spinand_ondie_ecc_conf - private SPI-NAND on-die ECC engine structure


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v1 3/4] mtd: spinand: Add support continuous read operation

[liao jaime]

Hi Miquel

>
> Hi JaimeLiao,
>
> jaimeliao.tw@gmail.com wrote on Thu, 11 Aug 2022 17:45:35 +0800:
>
> > The continuous read operation includes three phases:
> > Firstly, starting with the page read command and the 1st
> > page data will be read into the cache after the read latency tRD.
> > Secondly, Issuing the Read From Cache commands
> > (03h/0Bh/3Bh/6Bh/BBh/EBh) to read out the data from cache continuously.
> > After all the data is read out, the host should pull CS# high
> > to terminate this continuous read operation and wait tRST for the NAND
> > device resets read operation.
> >
> > Continuous reads have a positive impact when reading more than
> > one page and the column address is don't care in this operation,
> > a full page data will be read out for each page.
> >
> > The performance of continuous read mode is as follows. Set the
> > flash to QUAD mode and run 25MZ direct mapping mode on the SPI
>
>                                MHz
Thanks, this will be correct next patch.

>
> > bus and use the MTD test module to show the performance of
> > continuous reads.
>
> Please show before and after changes and just give us the lines
> regarding the read speeds rather than all the output of the tool.
Ok, I will attach test report for two cases.

>
> >
> > =================================================
> > mtd_speedtest: MTD device: 0    count: 100
> > mtd_speedtest: MTD device size 268435456, eraseblock size 131072,
> >                page size 2048, count of eraseblocks 2048, pages per
> >                eraseblock 64, OOB size 64
> > mtd_test: scanning for bad eraseblocks
> > mtd_test: scanned 100 eraseblocks, 0 are bad
> > mtd_speedtest: testing eraseblock write speed
> > mtd_speedtest: eraseblock write speed is 1298 KiB/s
> > mtd_speedtest: testing eraseblock read speed
> > mtd_speedtest: eraseblock read speed is 11053 KiB/s
> > mtd_speedtest: testing page write speed
> > mtd_speedtest: page write speed is 1291 KiB/s
> > mtd_speedtest: testing page read speed
> > mtd_speedtest: page read speed is 3240 KiB/s
> > mtd_speedtest: testing 2 page write speed
> > mtd_speedtest: 2 page write speed is 1289 KiB/s
> > mtd_speedtest: testing 2 page read speed
> > mtd_speedtest: 2 page read speed is 2909 KiB/s
> > mtd_speedtest: Testing erase speed
> > mtd_speedtest: erase speed is 45229 KiB/s
> > mtd_speedtest: Testing 2x multi-block erase speed
> > mtd_speedtest: 2x multi-block erase speed is 62135 KiB/s
> > mtd_speedtest: Testing 4x multi-block erase speed
> > mtd_speedtest: 4x multi-block erase speed is 60093 KiB/s
> > mtd_speedtest: Testing 8x multi-block erase speed
> > mtd_speedtest: 8x multi-block erase speed is 61244 KiB/s
> > mtd_speedtest: Testing 16x multi-block erase speed
> > mtd_speedtest: 16x multi-block erase speed is 61538 KiB/s
> > mtd_speedtest: Testing 32x multi-block erase speed
> > mtd_speedtest: 32x multi-block erase speed is 61835 KiB/s
> > mtd_speedtest: Testing 64x multi-block erase speed
> > mtd_speedtest: 64x multi-block erase speed is 60663 KiB/s
> > mtd_speedtest: finished
> > =================================================
> >
> > Signed-off-by: JaimeLiao <jaimeliao.tw@gmail.com>
> > ---
> >  drivers/mtd/nand/spi/core.c | 99 +++++++++++++++++++++++++++++++++++++
> >  include/linux/mtd/spinand.h |  1 +
> >  2 files changed, 100 insertions(+)
> >
> > diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> > index 380411feab6c..ef09bedda8d4 100644
> > --- a/drivers/mtd/nand/spi/core.c
> > +++ b/drivers/mtd/nand/spi/core.c
> > @@ -386,6 +386,10 @@ static int spinand_read_from_cache_op(struct spinand_device *spinand,
> >       if (req->datalen) {
> >               buf = spinand->databuf;
> >               nbytes = nanddev_page_size(nand);
> > +             if (spinand->use_continuous_read) {
> > +                     buf = req->databuf.in;
> > +                     nbytes = req->datalen;
> > +             }
>
> Does not look right. A cache read is still one page, and I believe the
> caller should have taken care of that?
For this part, buf is req->databuf.in.
Multi page could be receive at this operation.

>
> >               column = 0;
> >       }
> >
> > @@ -412,6 +416,9 @@ static int spinand_read_from_cache_op(struct spinand_device *spinand,
> >               buf += ret;
> >       }
> >
> > +     if (spinand->use_continuous_read)
> > +             return 0;
>
> For me this is not the right place.
Would it be better If using "goto"?

>
> > +
> >       if (req->datalen)
> >               memcpy(req->databuf.in, spinand->databuf + req->dataoffs,
> >                      req->datalen);
> > @@ -640,6 +647,80 @@ static int spinand_write_page(struct spinand_device *spinand,
> >       return nand_ecc_finish_io_req(nand, (struct nand_page_io_req *)req);
> >  }
> >
> > +static int spinand_mtd_continuous_read(struct mtd_info *mtd, loff_t from,
> > +                                    struct mtd_oob_ops *ops,
> > +                                    struct nand_io_iter *iter)
> > +{
> > +     struct spinand_device *spinand = mtd_to_spinand(mtd);
> > +     struct nand_device *nand = mtd_to_nanddev(mtd);
> > +     int ret = 0;
> > +
> > +     /*
> > +      * Continuous read mode could reduce some operation in On-die ECC free
> > +      * flash when read page sequentially.
> > +      */
> > +     iter->req.type = NAND_PAGE_READ;
> > +     iter->req.mode = MTD_OPS_RAW;
>
> I don't get what you do here.
>
> Continuous read shall either work with ECC engines or just not be
> implemented at all. You cannot blindly disable ECC support.
For Macronix continuous read feature is for "On-die ECC" flash only.
So that I think ecc have been handled in flash.

>
> > +     iter->req.dataoffs = nanddev_offs_to_pos(nand, from, &iter->req.pos);
> > +     iter->req.databuf.in = ops->datbuf;
> > +     iter->req.datalen = ops->len;
> > +
> > +     if (from & (nanddev_page_size(nand) - 1)) {
> > +             pr_debug("%s: unaligned address\n", __func__);
> > +             return -EINVAL;
> > +     }
> > +
> > +     ret = spinand_continuous_read_enable(spinand);
> > +     if (ret)
> > +             return ret;
> > +
> > +     spinand->use_continuous_read = true;
> > +
> > +     ret = spinand_select_target(spinand, iter->req.pos.target);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /*
> > +      * The continuous read operation including: firstly, starting with the
> > +      * page read command and the 1 st page data will be read into the cache
> > +      * after the read latency tRD. Secondly, Issuing the Read From Cache
> > +      * commands (03h/0Bh/3Bh/6Bh/BBh/EBh) to read out the data from cache
> > +      * continuously.
> > +      *
> > +      * The cache is divided into two halves, while one half of the cache is
> > +      * outputting the data, the other half will be loaded for the new data;
> > +      * therefore, the host can read out the data continuously from page to
> > +      * page. Multiple of Read From Cache commands can be issued in one
> > +      * continuous read operation, each Read From Cache command is required
> > +      * to read multiple 4-byte data exactly; otherwise, the data output will
> > +      * be out of sequence from one Read From Cache command to another Read
> > +      * From Cache command.
> > +      *
> > +      * After all the data is read out, the host should pull CS# high to
> > +      * terminate this continuous read operation and wait a 6us of tRST for
> > +      * the NAND device resets read operation. The data output for each page
> > +      * will always start from byte 0 and a full page data should be read out
> > +      * for each page.
> > +      */
> > +     ret = spinand_read_page(spinand, &iter->req);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = spinand_reset_op(spinand);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = spinand_continuous_read_disable(spinand);
> > +     if (ret)
> > +             return ret;
> > +
> > +     spinand->use_continuous_read = false;
>
> Any error in this function should lead to resetting use_continuous_read
> to false and disabling continuous read on the flash side.
OK, this will be correct next version.

>
> > +
> > +     ops->retlen = iter->req.datalen;
> > +
> > +     return ret;
> > +}
> > +
> >  static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
> >                           struct mtd_oob_ops *ops)
> >  {
> > @@ -656,6 +737,24 @@ static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
> >
> >       mutex_lock(&spinand->lock);
> >
> > +     /*
> > +      * If the device support continuous read mode and read length larger
> > +      * than one page size will enter the continuous read mode. This mode
> > +      * helps avoid issuing a page read command and read from cache command
> > +      * again, and improves read performance for continuous addresses.
> > +      */
> > +     if ((spinand->flags & SPINAND_HAS_CONT_READ_BIT) &&
> > +         (ops->len > nanddev_page_size(nand))) {
> > +             ret = spinand_mtd_continuous_read(mtd, from, ops, &iter);
> > +
> > +             mutex_unlock(&spinand->lock);
> > +
> > +             if (ecc_failed && !ret)
> > +                     ret = -EBADMSG;
> > +
> > +             return ret ? ret : max_bitflips;
> > +     }
> > +
> >       nanddev_io_for_each_page(nand, NAND_PAGE_READ, from, ops, &iter) {
> >               if (disable_ecc)
> >                       iter.req.mode = MTD_OPS_RAW;
> > diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
> > index d70a6f49cc6f..c296d4d0dda5 100644
> > --- a/include/linux/mtd/spinand.h
> > +++ b/include/linux/mtd/spinand.h
> > @@ -308,6 +308,7 @@ struct spinand_ecc_info {
> >
> >  #define SPINAND_HAS_QE_BIT           BIT(0)
> >  #define SPINAND_HAS_CR_FEAT_BIT              BIT(1)
> > +#define SPINAND_HAS_CONT_READ_BIT    BIT(2)
>
> Is this bit standard?
It is using to add into flash id table and judge whether continuous
read feature is support or not.

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

Thanks
Jaime

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v1 3/4] mtd: spinand: Add support continuous read operation

[Miquel Raynal]

Hi Jaime,

jaimeliao.tw@gmail.com wrote on Wed, 8 Feb 2023 19:14:55 +0800:

> Hi Miquel
> 
> >
> > Hi JaimeLiao,
> >
> > jaimeliao.tw@gmail.com wrote on Thu, 11 Aug 2022 17:45:35 +0800:
> >  
> > > The continuous read operation includes three phases:
> > > Firstly, starting with the page read command and the 1st
> > > page data will be read into the cache after the read latency tRD.
> > > Secondly, Issuing the Read From Cache commands
> > > (03h/0Bh/3Bh/6Bh/BBh/EBh) to read out the data from cache continuously.
> > > After all the data is read out, the host should pull CS# high
> > > to terminate this continuous read operation and wait tRST for the NAND
> > > device resets read operation.
> > >
> > > Continuous reads have a positive impact when reading more than
> > > one page and the column address is don't care in this operation,
> > > a full page data will be read out for each page.
> > >
> > > The performance of continuous read mode is as follows. Set the
> > > flash to QUAD mode and run 25MZ direct mapping mode on the SPI  
> >
> >                                MHz  
> Thanks, this will be correct next patch.
> 
> >  
> > > bus and use the MTD test module to show the performance of
> > > continuous reads.  
> >
> > Please show before and after changes and just give us the lines
> > regarding the read speeds rather than all the output of the tool.  
> Ok, I will attach test report for two cases.
> 
> >  
> > >
> > > =================================================
> > > mtd_speedtest: MTD device: 0    count: 100
> > > mtd_speedtest: MTD device size 268435456, eraseblock size 131072,
> > >                page size 2048, count of eraseblocks 2048, pages per
> > >                eraseblock 64, OOB size 64
> > > mtd_test: scanning for bad eraseblocks
> > > mtd_test: scanned 100 eraseblocks, 0 are bad
> > > mtd_speedtest: testing eraseblock write speed
> > > mtd_speedtest: eraseblock write speed is 1298 KiB/s
> > > mtd_speedtest: testing eraseblock read speed
> > > mtd_speedtest: eraseblock read speed is 11053 KiB/s
> > > mtd_speedtest: testing page write speed
> > > mtd_speedtest: page write speed is 1291 KiB/s
> > > mtd_speedtest: testing page read speed
> > > mtd_speedtest: page read speed is 3240 KiB/s
> > > mtd_speedtest: testing 2 page write speed
> > > mtd_speedtest: 2 page write speed is 1289 KiB/s
> > > mtd_speedtest: testing 2 page read speed
> > > mtd_speedtest: 2 page read speed is 2909 KiB/s
> > > mtd_speedtest: Testing erase speed
> > > mtd_speedtest: erase speed is 45229 KiB/s
> > > mtd_speedtest: Testing 2x multi-block erase speed
> > > mtd_speedtest: 2x multi-block erase speed is 62135 KiB/s
> > > mtd_speedtest: Testing 4x multi-block erase speed
> > > mtd_speedtest: 4x multi-block erase speed is 60093 KiB/s
> > > mtd_speedtest: Testing 8x multi-block erase speed
> > > mtd_speedtest: 8x multi-block erase speed is 61244 KiB/s
> > > mtd_speedtest: Testing 16x multi-block erase speed
> > > mtd_speedtest: 16x multi-block erase speed is 61538 KiB/s
> > > mtd_speedtest: Testing 32x multi-block erase speed
> > > mtd_speedtest: 32x multi-block erase speed is 61835 KiB/s
> > > mtd_speedtest: Testing 64x multi-block erase speed
> > > mtd_speedtest: 64x multi-block erase speed is 60663 KiB/s
> > > mtd_speedtest: finished
> > > =================================================
> > >
> > > Signed-off-by: JaimeLiao <jaimeliao.tw@gmail.com>
> > > ---
> > >  drivers/mtd/nand/spi/core.c | 99 +++++++++++++++++++++++++++++++++++++
> > >  include/linux/mtd/spinand.h |  1 +
> > >  2 files changed, 100 insertions(+)
> > >
> > > diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> > > index 380411feab6c..ef09bedda8d4 100644
> > > --- a/drivers/mtd/nand/spi/core.c
> > > +++ b/drivers/mtd/nand/spi/core.c
> > > @@ -386,6 +386,10 @@ static int spinand_read_from_cache_op(struct spinand_device *spinand,
> > >       if (req->datalen) {
> > >               buf = spinand->databuf;
> > >               nbytes = nanddev_page_size(nand);
> > > +             if (spinand->use_continuous_read) {
> > > +                     buf = req->databuf.in;
> > > +                     nbytes = req->datalen;
> > > +             }  
> >
> > Does not look right. A cache read is still one page, and I believe the
> > caller should have taken care of that?  
> For this part, buf is req->databuf.in.
> Multi page could be receive at this operation.

That is not what we expect from spinand_read_from_cache_op(). If you
change the semantics, please make your own function.

> > >               column = 0;
> > >       }
> > >
> > > @@ -412,6 +416,9 @@ static int spinand_read_from_cache_op(struct spinand_device *spinand,
> > >               buf += ret;
> > >       }
> > >
> > > +     if (spinand->use_continuous_read)
> > > +             return 0;  
> >
> > For me this is not the right place.  
> Would it be better If using "goto"?

I think I meant: "this is not the right function" to do that.

> 
> >  
> > > +
> > >       if (req->datalen)
> > >               memcpy(req->databuf.in, spinand->databuf + req->dataoffs,
> > >                      req->datalen);
> > > @@ -640,6 +647,80 @@ static int spinand_write_page(struct spinand_device *spinand,
> > >       return nand_ecc_finish_io_req(nand, (struct nand_page_io_req *)req);
> > >  }
> > >
> > > +static int spinand_mtd_continuous_read(struct mtd_info *mtd, loff_t from,
> > > +                                    struct mtd_oob_ops *ops,
> > > +                                    struct nand_io_iter *iter)
> > > +{
> > > +     struct spinand_device *spinand = mtd_to_spinand(mtd);
> > > +     struct nand_device *nand = mtd_to_nanddev(mtd);
> > > +     int ret = 0;
> > > +
> > > +     /*
> > > +      * Continuous read mode could reduce some operation in On-die ECC free
> > > +      * flash when read page sequentially.
> > > +      */
> > > +     iter->req.type = NAND_PAGE_READ;
> > > +     iter->req.mode = MTD_OPS_RAW;  
> >
> > I don't get what you do here.
> >
> > Continuous read shall either work with ECC engines or just not be
> > implemented at all. You cannot blindly disable ECC support.  
> For Macronix continuous read feature is for "On-die ECC" flash only.
> So that I think ecc have been handled in flash.

Then just prevent continuous read from being enabled on !on-die ECC
flash devices, and please do not perform anything like changing the mode
of operation.

> > > +     iter->req.dataoffs = nanddev_offs_to_pos(nand, from, &iter->req.pos);
> > > +     iter->req.databuf.in = ops->datbuf;
> > > +     iter->req.datalen = ops->len;
> > > +
> > > +     if (from & (nanddev_page_size(nand) - 1)) {
> > > +             pr_debug("%s: unaligned address\n", __func__);
> > > +             return -EINVAL;
> > > +     }
> > > +
> > > +     ret = spinand_continuous_read_enable(spinand);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     spinand->use_continuous_read = true;
> > > +
> > > +     ret = spinand_select_target(spinand, iter->req.pos.target);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     /*
> > > +      * The continuous read operation including: firstly, starting with the
> > > +      * page read command and the 1 st page data will be read into the cache
> > > +      * after the read latency tRD. Secondly, Issuing the Read From Cache
> > > +      * commands (03h/0Bh/3Bh/6Bh/BBh/EBh) to read out the data from cache
> > > +      * continuously.
> > > +      *
> > > +      * The cache is divided into two halves, while one half of the cache is
> > > +      * outputting the data, the other half will be loaded for the new data;
> > > +      * therefore, the host can read out the data continuously from page to
> > > +      * page. Multiple of Read From Cache commands can be issued in one
> > > +      * continuous read operation, each Read From Cache command is required
> > > +      * to read multiple 4-byte data exactly; otherwise, the data output will
> > > +      * be out of sequence from one Read From Cache command to another Read
> > > +      * From Cache command.
> > > +      *
> > > +      * After all the data is read out, the host should pull CS# high to
> > > +      * terminate this continuous read operation and wait a 6us of tRST for
> > > +      * the NAND device resets read operation. The data output for each page
> > > +      * will always start from byte 0 and a full page data should be read out
> > > +      * for each page.
> > > +      */
> > > +     ret = spinand_read_page(spinand, &iter->req);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     ret = spinand_reset_op(spinand);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     ret = spinand_continuous_read_disable(spinand);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     spinand->use_continuous_read = false;  
> >
> > Any error in this function should lead to resetting use_continuous_read
> > to false and disabling continuous read on the flash side.  
> OK, this will be correct next version.
> 
> >  
> > > +
> > > +     ops->retlen = iter->req.datalen;
> > > +
> > > +     return ret;
> > > +}
> > > +
> > >  static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
> > >                           struct mtd_oob_ops *ops)
> > >  {
> > > @@ -656,6 +737,24 @@ static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
> > >
> > >       mutex_lock(&spinand->lock);
> > >
> > > +     /*
> > > +      * If the device support continuous read mode and read length larger
> > > +      * than one page size will enter the continuous read mode. This mode
> > > +      * helps avoid issuing a page read command and read from cache command
> > > +      * again, and improves read performance for continuous addresses.
> > > +      */
> > > +     if ((spinand->flags & SPINAND_HAS_CONT_READ_BIT) &&
> > > +         (ops->len > nanddev_page_size(nand))) {
> > > +             ret = spinand_mtd_continuous_read(mtd, from, ops, &iter);
> > > +
> > > +             mutex_unlock(&spinand->lock);
> > > +
> > > +             if (ecc_failed && !ret)
> > > +                     ret = -EBADMSG;
> > > +
> > > +             return ret ? ret : max_bitflips;
> > > +     }
> > > +
> > >       nanddev_io_for_each_page(nand, NAND_PAGE_READ, from, ops, &iter) {
> > >               if (disable_ecc)
> > >                       iter.req.mode = MTD_OPS_RAW;
> > > diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
> > > index d70a6f49cc6f..c296d4d0dda5 100644
> > > --- a/include/linux/mtd/spinand.h
> > > +++ b/include/linux/mtd/spinand.h
> > > @@ -308,6 +308,7 @@ struct spinand_ecc_info {
> > >
> > >  #define SPINAND_HAS_QE_BIT           BIT(0)
> > >  #define SPINAND_HAS_CR_FEAT_BIT              BIT(1)
> > > +#define SPINAND_HAS_CONT_READ_BIT    BIT(2)  
> >
> > Is this bit standard?  
> It is using to add into flash id table and judge whether continuous
> read feature is support or not.

I understand what it is meant for, but my question is, shall we
consider this bit standard across manufacturers? Or is it specific to
Macronix?

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v1 4/4] mtd: spinand: macronix: Add continuous read support for MX35LF2GE4AD

[JaimeLiao]

Add continuous read flag for MX35LF2GE4AD

Signed-off-by: JaimeLiao <jaimeliao.tw@gmail.com>
---
 drivers/mtd/nand/spi/macronix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/spi/macronix.c b/drivers/mtd/nand/spi/macronix.c
index 3f31f1381a62..02fb8f37947d 100644
--- a/drivers/mtd/nand/spi/macronix.c
+++ b/drivers/mtd/nand/spi/macronix.c
@@ -126,7 +126,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_CONT_READ_BIT,
 		     SPINAND_ECCINFO(&mx35lfxge4ab_ooblayout,
 				     mx35lf1ge4ab_ecc_get_status)),
 	SPINAND_INFO("MX35LF4GE4AD",
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/