> -----Original Message-----
> From: Rajat Srivastava
> Sent: Friday, April 5, 2019 2:24 PM
> To: u-boot@lists.denx.de; vigne...@ti.com; tr...@konsulko.com;
> marek.va...@gmail.com; marek.vasut+rene...@gmail.com;
> ja...@openedev.com
> Cc: Ashish Kumar <ashish.ku...@nxp.com>; Rajat Srivastava
> <rajat.srivast...@nxp.com>
> Subject: [PATCH] mtd: spi: Improve data write functionality
> 
> 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>
Hi Jagan, Vignesh, Marek,

Did you get some time to look into this Patch, the current data write is also 
broken ?

Regards
Ashish 

> ---
> 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.
> 
>  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) {
> --
> 2.17.1

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to