linux-kernel - RE: [PATCH] mtd: spi-nor: honour max_data

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Wed, 13 Jun 2018 06:14:54 +0000
From:   Yogesh Narayan Gaur <yogeshnarayan.gaur@....com>
To:     Boris Brezillon <boris.brezillon@...tlin.com>
CC:     "linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
        "frieder.schrempf@...eet.de" <frieder.schrempf@...eet.de>,
        "computersforpeace@...il.com" <computersforpeace@...il.com>,
        David Wolfe <david.wolfe@....com>, Han Xu <han.xu@....com>,
        "festevam@...il.com" <festevam@...il.com>,
        "marek.vasut@...il.com" <marek.vasut@...il.com>,
        Prabhakar Kushwaha <prabhakar.kushwaha@....com>,
        "linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        NeilBrown <neil@...wn.name>
Subject: RE: [PATCH] mtd: spi-nor: honour max_data_size for spi-nor writes

Hi Boris,


-----Original Message-----
From: Boris Brezillon [mailto:boris.brezillon@...tlin.com] 
Sent: Monday, June 11, 2018 3:19 PM
To: Yogesh Narayan Gaur <yogeshnarayan.gaur@....com>
Cc: linux-mtd@...ts.infradead.org; boris.brezillon@...e-electrons.com; frieder.schrempf@...eet.de; computersforpeace@...il.com; David Wolfe <david.wolfe@....com>; Han Xu <han.xu@....com>; festevam@...il.com; marek.vasut@...il.com; Prabhakar Kushwaha <prabhakar.kushwaha@....com>; linux-spi@...r.kernel.org; linux-kernel@...r.kernel.org; NeilBrown <neil@...wn.name>
Subject: Re: [PATCH] mtd: spi-nor: honour max_data_size for spi-nor writes

Hi Yogesh,

Unrelated note: no need to send your patches to both boris.brezillo@...e-electrons.com and boris.brezillo@...tlin.com, just use the latter.

Ok, Sure.

On Mon, 11 Jun 2018 14:48:14 +0530
Yogesh Gaur <yogeshnarayan.gaur@....com> wrote:

> Honour max_data_size for spi-nor writes

^ This is no longer relevant.

> In new spi-mem framework, data size to be written is being calculated 
> using spi_mem_adjust_op_size().
> This can return value less than nor->page_size.
> 
> Add check value of data size return from API spi_mem_adjust_op_size() 
> with the actual requested data size and write, max, only supported 
> data size.

This part is not clear.

Also, I'd prefer to have this patch split in 2:
1/ one patch removing the check in spi_nor_write() 2/ and the second patch removing the while() loop in m25p80_write()

How about the following commit messages for those 2 patches:

1:
"
mtd: spi-nor: Support controllers with limited TX FIFO size

Some SPI controllers can't write nor->page_size bytes in a single step because their TX FIFO is too small.

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

2:
"
mtd: devices: m25p80: Make sure WRITE_EN is issued before each write

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 is issued before each write access.

The core is already taking care of that, so all we have to do here is return the actual number of bytes that were written during the
spi_mem_exec_op() operation.
"

Both patches send [1][2]

--
Regards
Yogesh Gaur.

> 
> Signed-off-by: NeilBrown <neil@...wn.name>
> Signed-off-by: Yogesh Gaur <yogeshnarayan.gaur@....com>
> ---
>  drivers/mtd/devices/m25p80.c  | 23 ++++++++---------------  
> drivers/mtd/spi-nor/spi-nor.c |  7 -------
>  2 files changed, 8 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/mtd/devices/m25p80.c 
> b/drivers/mtd/devices/m25p80.c index e84563d..60224fe 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -72,7 +72,6 @@ static ssize_t m25p80_write(struct spi_nor *nor, loff_t to, size_t len,
>  				   SPI_MEM_OP_ADDR(nor->addr_width, to, 1),
>  				   SPI_MEM_OP_DUMMY(0, 1),
>  				   SPI_MEM_OP_DATA_OUT(len, buf, 1));
> -	size_t remaining = len;
>  	int ret;
>  
>  	/* get transfer protocols. */
> @@ -84,22 +83,16 @@ static ssize_t m25p80_write(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(flash->spimem, &op);
> -		if (ret)
> -			return ret;
> -
> -		ret = spi_mem_exec_op(flash->spimem, &op);
> -		if (ret)
> -			return ret;
> +	ret = spi_mem_adjust_op_size(flash->spimem, &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(flash->spimem, &op);
> +	if (ret)
> +		return ret;
>  
> -	return len;
> +	return op.data.nbytes;
>  }
>  
>  /*
> diff --git a/drivers/mtd/spi-nor/spi-nor.c 
> b/drivers/mtd/spi-nor/spi-nor.c index 5bfa36e..3e63543 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1431,13 +1431,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) {
> -			dev_err(nor->dev,
> -				"While writing %zu bytes written %zd bytes\n",
> -				page_remain, written);
> -			ret = -EIO;
> -			goto write_err;
> -		}
>  	}
>  
>  write_err:

[1] https://patchwork.ozlabs.org/patch/928677/
[2] https://patchwork.ozlabs.org/patch/928678/