Re: [PATCH 2/2] mtd: m25p80: consider max_transfer_size when reading

[Brian Norris <computersforpeace@gmail.com>]

+ others

Hi Heiner,

On Sat, Feb 27, 2016 at 11:50:11PM +0100, Heiner Kallweit wrote:
> Some controllers have transfer size limits. To allow to deal with this
> max_transfer_size was introduced in the SPI core recently.
> Use this new feature to read in chunks if needed.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Michal has been working on a similar series, with some differences (I'll
comment below). I think his latest work is here:

http://lists.infradead.org/pipermail/linux-mtd/2015-December/063865.html

Is there a particular reason you've continued on your series instead of
reviewing/fixing his?

> ---
>  drivers/mtd/devices/m25p80.c | 30 +++++++++++++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index c2d1f65..69f3acf 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -119,7 +119,7 @@ static inline unsigned int m25p80_rx_nbits(struct spi_nor *nor)
>   * Read an address range from the nor chip.  The address range
>   * may be any size provided it is within the physical boundaries.
>   */
> -static int m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
> +static int _m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
>  			size_t *retlen, u_char *buf)
>  {
>  	struct m25p *flash = nor->priv;
> @@ -153,6 +153,34 @@ static int m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
>  	return ret;
>  }
>  
> +/* Read in max_read_len chunks if len > max_read_len */
> +static int m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
> +		       size_t *retlen, u_char *buf)
> +{
> +	struct m25p *flash = nor->priv;
> +	/*
> +	 * The controller transfer size limit refers to the overall transfer
> +	 * including read command + actually read data. Therefore subtract
> +	 * the command size when calculating the max read length.
> +	 */
> +	size_t max_read_len = spi_max_transfer_size(flash->spi) -
> +			      (m25p_cmdsz(nor) + nor->read_dummy / 8);
> +	size_t read_len;
> +	int ret;
> +
> +	while (len > 0) {

^^ I really don't want this loop to be in m25p80.c. This should be in
the core spi-nor.c, and then the driver (like m25p80.c, but also
possibly other non-SPI drivers that also use spi-nor.c) can simply
pass its limitation up the spi-nor.c. (See Michal's patch 10.)

So I plan to take some form of Michal's and not yours. This will require
a bit of rebasing.

Brian

> +		read_len = min(len, max_read_len);
> +		ret = _m25p80_read(nor, from, read_len, retlen, buf);
> +		if (ret)
> +			return ret;
> +		from += read_len;
> +		buf += read_len;
> +		len -= read_len;
> +	}
> +
> +	return 0;
> +}
> +
>  /*
>   * board specific setup should have ensured the SPI clock used here
>   * matches what the READ command supports, at least until this driver
> -- 
> 2.7.1
>