> -----Original Message----- > From: Vignesh Raghavendra <vigne...@ti.com> > Sent: Friday, April 12, 2019 1:01 PM > To: Rajat Srivastava <rajat.srivast...@nxp.com>; u-boot@lists.denx.de; > tr...@konsulko.com; marek.va...@gmail.com; > marek.vasut+rene...@gmail.com; ja...@openedev.com > Cc: Ashish Kumar <ashish.ku...@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.srivast...@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: > > 1. 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: > > 1. write to register (nor->write_reg) > > 2. 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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot