From mboxrd@z Thu Jan  1 00:00:00 1970
From: Boris Brezillon <boris.brezillon@bootlin.com>
Date: Mon, 6 Aug 2018 21:53:18 +0200
Subject: [U-Boot] [PATCH 1/4] spi: spi-mem: Add optional half-duplex SPI
 transfer mode
In-Reply-To: <20180806151253.31205-1-sr@denx.de>
References: <20180806151253.31205-1-sr@denx.de>
Message-ID: <20180806215318.38d854ff@bbrezillon>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Stefan,

On Mon,  6 Aug 2018 17:12:50 +0200
Stefan Roese <sr@denx.de> wrote:

> Some SPI controller might now support full-duplex SPI transfers.
> This option can be enabled to use half-duplex operation mode for
> such SPI controllers.
> 
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Boris Brezillon <boris.brezillon@bootlin.com>
> Cc: Jagan Teki <jagan@openedev.com>
> ---
>  drivers/spi/Kconfig   |  8 ++++++++
>  drivers/spi/spi-mem.c | 31 ++++++++++++++++++++++++++++---
>  2 files changed, 36 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 9fbd26740d..5bd8289284 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -25,6 +25,14 @@ config SPI_MEM
>  	  This extension is meant to simplify interaction with SPI memories
>  	  by providing an high-level interface to send memory-like commands.
>  
> +config SPI_MEM_HALF_DUPLEX
> +	bool "Use half-duplex SPI transfer"
> +	depends on SPI_MEM
> +	help
> +	  Some SPI controller might not support full-duplex SPI transfers.
> +	  This option can be enabled to use half-duplex operation mode for
> +	  such SPI controllers.
> +
>  config ALTERA_SPI
>  	bool "Altera SPI driver"
>  	help
> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> index 07ce799170..617fbbfe09 100644
> --- a/drivers/spi/spi-mem.c
> +++ b/drivers/spi/spi-mem.c
> @@ -202,6 +202,10 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
>  	struct dm_spi_ops *ops = spi_get_ops(bus);
>  	unsigned int xfer_len, pos = 0;
>  	u8 *tx_buf, *rx_buf = NULL;
> +	unsigned int pos2;
> +	int tx_len;
> +	int rx_len;
> +	u32 flag;
>  	int ret;
>  	int i;
>  
> @@ -370,8 +374,29 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
>  	if (tx_data)
>  		memcpy(tx_buf + pos, op->data.buf.out, op->data.nbytes);
>  
> -	ret = spi_xfer(slave, xfer_len * 8, tx_buf, rx_buf,
> -		       SPI_XFER_BEGIN | SPI_XFER_END);
> +	pos2 = pos;
> +	if (CONFIG_IS_ENABLED(SPI_MEM_HALF_DUPLEX)) {
> +		if (rx_data) {
> +			tx_len = (sizeof(op->cmd.opcode) +
> +				  op->addr.nbytes + op->dummy.nbytes);
> +			rx_len = op->data.nbytes;
> +			flag = SPI_XFER_BEGIN;
> +			pos2 = 0;
> +		} else {
> +			tx_len = xfer_len;
> +			rx_len = 0;
> +			flag = SPI_XFER_BEGIN | SPI_XFER_END;
> +		}
> +		ret = spi_xfer(slave, tx_len * 8, tx_buf, NULL, flag);
> +		if (rx_data) {
> +			ret = spi_xfer(slave, rx_len * 8, NULL,
> +				       rx_buf, SPI_XFER_END);
> +		}

Why not doing that all the time and split things in 3 transfers
instead of 2:

1/ the opcode + address + dummy cycles
2/ the tx data
3/ the rx data (optional)

This way you should have to allocate the rx/tx buf and you'd get rid of
the memcpy.

Of course, that only works if all SPI controllers are capable of
dealing with transfers that have only SPI_XFER_BEGIN or SPI_XFER_END
set.

> +	} else {
> +		ret = spi_xfer(slave, xfer_len * 8, tx_buf, rx_buf,
> +			       SPI_XFER_BEGIN | SPI_XFER_END);
> +	}
> +
>  	spi_release_bus(slave);
>  
>  	for (i = 0; i < pos; i++)
> @@ -387,7 +412,7 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
>  		return ret;
>  
>  	if (rx_data)
> -		memcpy(op->data.buf.in, rx_buf + pos, op->data.nbytes);
> +		memcpy(op->data.buf.in, rx_buf + pos2, op->data.nbytes);
>  
>  	kfree(tx_buf);
>  	kfree(rx_buf);

Regards,

Boris