-----Original Message----- From: Vignesh Raghavendra vigneshr@ti.com Sent: Friday, April 12, 2019 1:01 PM To: Rajat Srivastava rajat.srivastava@nxp.com; u-boot@lists.denx.de; trini@konsulko.com; marek.vasut@gmail.com; marek.vasut+renesas@gmail.com; jagan@openedev.com Cc: Ashish Kumar ashish.kumar@nxp.com Subject: [EXT] Re: [PATCH] mtd: spi: Improve data write functionality
WARNING: This email was created outside of NXP. DO NOT CLICK links or attachments unless you recognize the sender and know the content is safe.
On 05/04/19 2:24 PM, Rajat Srivastava wrote:
Incorporate write enable and status check in the write data function itself.
Formerly, Write data function used to break the data to be written into smaller chunks and used to send the smaller chunks without write enable or status check for every iteration.
Signed-off-by: Rajat Srivastava rajat.srivastava@nxp.com
While writing any data to a SPI flash every write transaction shall be preceded by a WRITE_ENABLE command and shall be followed by a READ_STATUS process (to check if the flash is not busy). This sequence can be roughly represented as:
- write_enable //issue write enable command 2.
execute_write_operation //write data to flash or register 3. spi_nor_wait_till_ready //read status of flash
The current framework has two types of write operation:
- write to register (nor->write_reg)
- write data to flash memory (nor->write)
There seems to be an issue in writing data to flash memory for which the framework uses spi_nor_write_data() function. Before every call to spi_nor_write_data() function the framework sends a WRITE_ENABLE command and later checks if the flash is busy. However, the spi_nor_write_data() function which executes the data write to flash, breaks the data into smaller chunks. For all of these small transactions there is only a single WRITE_ENABLE command issued and a single check made for status, which breaks the write operation. Only the first chunk of the whole data is successfully written on to the flash.
This patch fixes the bug making the spi_nor_write_data() function issue WRITE_ENABLE command and status checks with every write transactions.
Without this patch write in fsl_qspi.c driver is broken.
I see this is mainly because fsl-qspi uses slave->max_write_size to restrict max write size which leads to fragmentation of traffic as part of spi_mem_adjust_op_size(). So, could you please remove while() loop in spi_nor_write_data(), return actual number of data bytes that is written and make spi_nor_write() loop around until all the data has been written?
Please find the v2 of this patch at https://patchwork.ozlabs.org/patch/1086262/
Regards Rajat
Regards Vignesh
drivers/mtd/spi/spi-nor-core.c | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index c4e2f6a08f..757163369b 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -23,6 +23,9 @@
#include "sf_internal.h"
+static int spi_nor_wait_till_ready(struct spi_nor *nor); static int +write_enable(struct spi_nor *nor);
/* Define max times to check status register before we give up. */
/* @@ -128,6 +131,8 @@ static ssize_t spi_nor_write_data(struct spi_nor
*nor, loff_t to, size_t len,
op.addr.nbytes = 0; while (remaining) {
write_enable(nor);op.data.nbytes = remaining < UINT_MAX ? remaining : UINT_MAX; ret = spi_mem_adjust_op_size(nor->spi, &op); if (ret)@@ -137,6 +142,10 @@ static ssize_t spi_nor_write_data(struct spi_nor
*nor, loff_t to, size_t len,
if (ret) return ret;
ret = spi_nor_wait_till_ready(nor);if (ret)return ret;op.addr.val += op.data.nbytes; remaining -= op.data.nbytes; op.data.buf.out += op.data.nbytes; @@ -961,14 +970,10 @@static int sst_write_byteprogram(struct spi_nor *nor, loff_t to, size_t len, for (actual = 0; actual < len; actual++) { nor->program_opcode = SPINOR_OP_BP;
write_enable(nor); /* write one byte. */ ret = nor->write(nor, to, 1, buf + actual); if (ret < 0) goto sst_write_err;ret = spi_nor_wait_till_ready(nor);if (ret)goto sst_write_err; to++; }@@ -989,8 +994,6 @@ static int sst_write(struct mtd_info *mtd, loff_t to,
size_t len,
if (spi->mode & SPI_TX_BYTE) return sst_write_byteprogram(nor, to, len, retlen, buf);
write_enable(nor);nor->sst_write_second = false; actual = to % 2;@@ -1002,9 +1005,6 @@ static int sst_write(struct mtd_info *mtd, loff_t
to, size_t len,
ret = nor->write(nor, to, 1, buf); if (ret < 0) goto sst_write_err;
ret = spi_nor_wait_till_ready(nor);if (ret)goto sst_write_err; } to += actual;@@ -1016,9 +1016,6 @@ static int sst_write(struct mtd_info *mtd, loff_t
to, size_t len,
ret = nor->write(nor, to, 2, buf + actual); if (ret < 0) goto sst_write_err;
ret = spi_nor_wait_till_ready(nor);if (ret)goto sst_write_err; to += 2; nor->sst_write_second = true; }@@ -1031,15 +1028,10 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
/* Write out trailing byte if it exists. */ if (actual != len) {
write_enable(nor);nor->program_opcode = SPINOR_OP_BP; ret = nor->write(nor, to, 1, buf + actual); if (ret < 0) goto sst_write_err;ret = spi_nor_wait_till_ready(nor);if (ret)goto sst_write_err; write_disable(nor); actual += 1; }@@ -1090,15 +1082,11 @@ static int spi_nor_write(struct mtd_info *mtd,
loff_t to, size_t len,
if (ret < 0) return ret;#endif
write_enable(nor); ret = nor->write(nor, addr, page_remain, buf + i); if (ret < 0) goto write_err; written = ret;ret = spi_nor_wait_till_ready(nor);if (ret)goto write_err; *retlen += written; i += written; if (written != page_remain) {-- Regards Vignesh