Re: [PATCH v2] mtd: spi-nor: use spi-mem dirmap API

From: Boris Brezillon <boris.brezillon@collabora.com>
To: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Cc: Vignesh Raghavendra <vigneshr@ti.com>,
	Boris Brezillon <bbrezillon@kernel.org>,
	Richard Weinberger <richard@nod.at>,
	Marek Vasut <marek.vasut@gmail.com>,
	linux-mtd@lists.infradead.org,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Brian Norris <computersforpeace@gmail.com>,
	David Woodhouse <dwmw2@infradead.org>
Subject: Re: [PATCH v2] mtd: spi-nor: use spi-mem dirmap API
Date: Sat, 9 Nov 2019 20:35:58 +0100	[thread overview]
Message-ID: <20191109203558.4a03c398@collabora.com> (raw)
In-Reply-To: <ef173d36-c918-4636-ace6-f6ba82dac672@cogentembedded.com>

On Thu, 7 Nov 2019 23:49:12 +0300
Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote:

> Make use of the spi-mem direct mapping API to let advanced controllers
> optimize read/write operations when they support direct mapping.
> 
> Based on the original patch by Boris Brezillon <boris.brezillon@bootlin.com>.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
> Changes in version 2:
> - moved the spi_mem_dirmap_{read|write}() calls from spi_nor_{read|write}() to
>   spi_nor_spimem_{read|write}_data().
> 
>  drivers/mtd/spi-nor/spi-nor.c |  125 +++++++++++++++++++++++++++++++++---------
>  include/linux/mtd/spi-nor.h   |    5 +
>  2 files changed, 104 insertions(+), 26 deletions(-)
> 
> Index: linux/drivers/mtd/spi-nor/spi-nor.c
> ===================================================================
> --- linux.orig/drivers/mtd/spi-nor/spi-nor.c
> +++ linux/drivers/mtd/spi-nor/spi-nor.c
> @@ -305,22 +305,28 @@ static ssize_t spi_nor_spimem_xfer_data(
>  static ssize_t spi_nor_spimem_read_data(struct spi_nor *nor, loff_t from,
>  					size_t len, u8 *buf)
>  {
> -	struct spi_mem_op op =
> -		SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1),
> -			   SPI_MEM_OP_ADDR(nor->addr_width, from, 1),
> -			   SPI_MEM_OP_DUMMY(nor->read_dummy, 1),
> -			   SPI_MEM_OP_DATA_IN(len, buf, 1));
> -
> -	/* get transfer protocols. */
> -	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->read_proto);
> -	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->read_proto);
> -	op.dummy.buswidth = op.addr.buswidth;
> -	op.data.buswidth = spi_nor_get_protocol_data_nbits(nor->read_proto);
> +	if (!nor->dirmap.rdesc) {
> +		struct spi_mem_op op =
> +			SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1),
> +				   SPI_MEM_OP_ADDR(nor->addr_width, from, 1),
> +				   SPI_MEM_OP_DUMMY(nor->read_dummy, 1),
> +				   SPI_MEM_OP_DATA_IN(len, buf, 1));
> +
> +		/* get transfer protocols. */
> +		op.cmd.buswidth =
> +			spi_nor_get_protocol_inst_nbits(nor->read_proto);
> +		op.addr.buswidth =
> +			spi_nor_get_protocol_addr_nbits(nor->read_proto);
> +		op.dummy.buswidth = op.addr.buswidth;
> +		op.data.buswidth =
> +			spi_nor_get_protocol_data_nbits(nor->read_proto);
>  
> -	/* convert the dummy cycles to the number of bytes */
> -	op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
> +		/* convert the dummy cycles to the number of bytes */
> +		op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
>  
> -	return spi_nor_spimem_xfer_data(nor, &op);
> +		return spi_nor_spimem_xfer_data(nor, &op);
> +	}
> +	return spi_mem_dirmap_read(nor->dirmap.rdesc, from, len, buf);

Can we put the spi_mem_dirmap_read() in the if() branch instead of
having an extra level of indentation for the most complex block.

	if (nor->dirmap.rdesc)
		return spi_mem_dirmap_read(nor->dirmap.rdesc, from,
					   len, buf);

	...

(same comment applies to the write path BTW).

With this addressed, you can add

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

Thanks,

Boris

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/