[U-Boot] [PATCH 1/5] mtd: spi-nor: fix page program issue when using spi-mem driver

[U-Boot] [PATCH 1/5] mtd: spi-nor: fix page program issue when using spi-mem driver

[Weijie Gao]

Some SPI controllers can't write nor->page_size bytes in a single step
because their TX FIFO is too small, but when that happens we should
make sure a WRITE_EN command before each write access and READ_SR command
after each write access is issued.

We should allow nor->write() to return a size that is smaller than the
requested write size to gracefully handle this case.

Also, the spi_nor_write_data() should return the actual number of bytes
that were written during the spi_mem_exec_op() operation.

This patch is a combination of two commits backported from kernel:

  commit 630d6bd8a3b4 ("mtd: spi-nor: Support controllers with limit ...")
  commit 3baa8ec88c2f ("mtd: devices: m25p80: Make sure WRITE_EN is ...")

Cc: Vignesh R <vigneshr@ti.com>
Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
---
 drivers/mtd/spi/spi-nor-core.c | 27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index c4e2f6a..1acff74 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -116,7 +116,6 @@ static ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t len,
 				   SPI_MEM_OP_ADDR(nor->addr_width, to, 1),
 				   SPI_MEM_OP_NO_DUMMY,
 				   SPI_MEM_OP_DATA_OUT(len, buf, 1));
-	size_t remaining = len;
 	int ret;
 
 	/* get transfer protocols. */
@@ -127,22 +126,16 @@ static ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t len,
 	if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
 		op.addr.nbytes = 0;
 
-	while (remaining) {
-		op.data.nbytes = remaining < UINT_MAX ? remaining : UINT_MAX;
-		ret = spi_mem_adjust_op_size(nor->spi, &op);
-		if (ret)
-			return ret;
-
-		ret = spi_mem_exec_op(nor->spi, &op);
-		if (ret)
-			return ret;
+	ret = spi_mem_adjust_op_size(nor->spi, &op);
+	if (ret)
+		return ret;
+	op.data.nbytes = len < op.data.nbytes ? len : op.data.nbytes;
 
-		op.addr.val += op.data.nbytes;
-		remaining -= op.data.nbytes;
-		op.data.buf.out += op.data.nbytes;
-	}
+	ret = spi_mem_exec_op(nor->spi, &op);
+	if (ret)
+		return ret;
 
-	return len;
+	return op.data.nbytes;
 }
 
 /*
@@ -1101,10 +1094,6 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
 			goto write_err;
 		*retlen += written;
 		i += written;
-		if (written != page_remain) {
-			ret = -EIO;
-			goto write_err;
-		}
 	}
 
 write_err:
-- 
1.9.1

[U-Boot] [PATCH 1/5] mtd: spi-nor: fix page program issue when using spi-mem driver

[Vignesh Raghavendra]

Hi,

+Jagan

On 26/04/19 2:52 PM, Weijie Gao wrote:
> Some SPI controllers can't write nor->page_size bytes in a single step
> because their TX FIFO is too small, but when that happens we should
> make sure a WRITE_EN command before each write access and READ_SR command
> after each write access is issued.
> 
> We should allow nor->write() to return a size that is smaller than the
> requested write size to gracefully handle this case.
> 
> Also, the spi_nor_write_data() should return the actual number of bytes
> that were written during the spi_mem_exec_op() operation.
> 
> This patch is a combination of two commits backported from kernel:
> 
>   commit 630d6bd8a3b4 ("mtd: spi-nor: Support controllers with limit ...")
>   commit 3baa8ec88c2f ("mtd: devices: m25p80: Make sure WRITE_EN is ...")
> 
> Cc: Vignesh R <vigneshr@ti.com>
> Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>

Thanks for the patch.

Acked-by: Vignesh R <vigneshr@ti.com>

Jagan

This patch fixes a regression wrt writing to flash on two platforms.
Could you consider queuing this patch for next -rc?

Regards
Vignesh

> ---
>  drivers/mtd/spi/spi-nor-core.c | 27 ++++++++-------------------
>  1 file changed, 8 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index c4e2f6a..1acff74 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -116,7 +116,6 @@ static ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t len,
>  				   SPI_MEM_OP_ADDR(nor->addr_width, to, 1),
>  				   SPI_MEM_OP_NO_DUMMY,
>  				   SPI_MEM_OP_DATA_OUT(len, buf, 1));
> -	size_t remaining = len;
>  	int ret;
>  
>  	/* get transfer protocols. */
> @@ -127,22 +126,16 @@ static ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t len,
>  	if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
>  		op.addr.nbytes = 0;
>  
> -	while (remaining) {
> -		op.data.nbytes = remaining < UINT_MAX ? remaining : UINT_MAX;
> -		ret = spi_mem_adjust_op_size(nor->spi, &op);
> -		if (ret)
> -			return ret;
> -
> -		ret = spi_mem_exec_op(nor->spi, &op);
> -		if (ret)
> -			return ret;
> +	ret = spi_mem_adjust_op_size(nor->spi, &op);
> +	if (ret)
> +		return ret;
> +	op.data.nbytes = len < op.data.nbytes ? len : op.data.nbytes;
>  
> -		op.addr.val += op.data.nbytes;
> -		remaining -= op.data.nbytes;
> -		op.data.buf.out += op.data.nbytes;
> -	}
> +	ret = spi_mem_exec_op(nor->spi, &op);
> +	if (ret)
> +		return ret;
>  
> -	return len;
> +	return op.data.nbytes;
>  }
>  
>  /*
> @@ -1101,10 +1094,6 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
>  			goto write_err;
>  		*retlen += written;
>  		i += written;
> -		if (written != page_remain) {
> -			ret = -EIO;
> -			goto write_err;
> -		}
>  	}
>  
>  write_err:
> 

-- 
Regards
Vignesh

[U-Boot] [PATCH 1/5] mtd: spi-nor: fix page program issue when using spi-mem driver

[Shyam Saini]

Hi Weijie,

> Some SPI controllers can't write nor->page_size bytes in a single step
> because their TX FIFO is too small, but when that happens we should
> make sure a WRITE_EN command before each write access and READ_SR command
> after each write access is issued.
>
> We should allow nor->write() to return a size that is smaller than the
> requested write size to gracefully handle this case.
>
> Also, the spi_nor_write_data() should return the actual number of bytes
> that were written during the spi_mem_exec_op() operation.
>
> This patch is a combination of two commits backported from kernel:
>
>   commit 630d6bd8a3b4 ("mtd: spi-nor: Support controllers with limit ...")
>   commit 3baa8ec88c2f ("mtd: devices: m25p80: Make sure WRITE_EN is ...")
>
> Cc: Vignesh R <vigneshr@ti.com>
> Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
> ---
>  drivers/mtd/spi/spi-nor-core.c | 27 ++++++++-------------------
>  1 file changed, 8 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index c4e2f6a..1acff74 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -116,7 +116,6 @@ static ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t len,
>                                    SPI_MEM_OP_ADDR(nor->addr_width, to, 1),
>                                    SPI_MEM_OP_NO_DUMMY,
>                                    SPI_MEM_OP_DATA_OUT(len, buf, 1));
> -       size_t remaining = len;
>         int ret;
>
>         /* get transfer protocols. */
> @@ -127,22 +126,16 @@ static ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t len,
>         if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
>                 op.addr.nbytes = 0;
>
> -       while (remaining) {
> -               op.data.nbytes = remaining < UINT_MAX ? remaining : UINT_MAX;
> -               ret = spi_mem_adjust_op_size(nor->spi, &op);
> -               if (ret)
> -                       return ret;
> -
> -               ret = spi_mem_exec_op(nor->spi, &op);
> -               if (ret)
> -                       return ret;
> +       ret = spi_mem_adjust_op_size(nor->spi, &op);
> +       if (ret)
> +               return ret;
> +       op.data.nbytes = len < op.data.nbytes ? len : op.data.nbytes;
>
> -               op.addr.val += op.data.nbytes;
> -               remaining -= op.data.nbytes;
> -               op.data.buf.out += op.data.nbytes;
> -       }
> +       ret = spi_mem_exec_op(nor->spi, &op);
> +       if (ret)
> +               return ret;
>
> -       return len;
> +       return op.data.nbytes;
>  }
>
>  /*
> @@ -1101,10 +1094,6 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
>                         goto write_err;
>                 *retlen += written;
>                 i += written;
> -               if (written != page_remain) {
> -                       ret = -EIO;
> -                       goto write_err;
> -               }
>         }
>

I've tested it on Zynq MicroZed board and it is working, so

Tested-by: Shyam Saini <shyam.saini@amarulasolutions.com>

[U-Boot] [PATCH 1/5] mtd: spi-nor: fix page program issue when using spi-mem driver

[Jagan Teki]

On Fri, Apr 26, 2019 at 2:53 PM Weijie Gao <weijie.gao@mediatek.com> wrote:
>
> Some SPI controllers can't write nor->page_size bytes in a single step
> because their TX FIFO is too small, but when that happens we should
> make sure a WRITE_EN command before each write access and READ_SR command
> after each write access is issued.
>
> We should allow nor->write() to return a size that is smaller than the
> requested write size to gracefully handle this case.
>
> Also, the spi_nor_write_data() should return the actual number of bytes
> that were written during the spi_mem_exec_op() operation.
>
> This patch is a combination of two commits backported from kernel:
>
>   commit 630d6bd8a3b4 ("mtd: spi-nor: Support controllers with limit ...")
>   commit 3baa8ec88c2f ("mtd: devices: m25p80: Make sure WRITE_EN is ...")
>
> Cc: Vignesh R <vigneshr@ti.com>
> Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
> ---

Applied to u-boot-spi/master