[PATCH v8 02/28] spi: spi-mem: allow specifying a command's extension

[Sean Anderson]

On 4/1/21 3:31 PM, Pratyush Yadav wrote:
> In xSPI mode, flashes expect 2-byte opcodes. The second byte is called
> the "command extension". There can be 3 types of extensions in xSPI:
> repeat, invert, and hex. When the extension type is "repeat", the same
> opcode is sent twice. When it is "invert", the second byte is the
> inverse of the opcode. When it is "hex" an additional opcode byte based
> is sent with the command whose value can be anything.
> 
> So, make opcode a 16-bit value and add a 'nbytes', similar to how
> multiple address widths are handled.
> 
> All usages of sizeof(op->cmd.opcode) also need to be changed to be
> op->cmd.nbytes because that is the actual indicator of opcode size.
> 
> Signed-off-by: Pratyush Yadav <p.yadav at ti.com>
> ---
>   drivers/spi/mtk_snfi_spi.c |  3 +--
>   drivers/spi/spi-mem-nodm.c |  4 ++--
>   drivers/spi/spi-mem.c      | 13 +++++++------
>   include/spi-mem.h          |  6 +++++-
>   4 files changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/spi/mtk_snfi_spi.c b/drivers/spi/mtk_snfi_spi.c
> index b6ab5fa3ad..65d0ce0981 100644
> --- a/drivers/spi/mtk_snfi_spi.c
> +++ b/drivers/spi/mtk_snfi_spi.c
> @@ -64,8 +64,7 @@ static int mtk_snfi_adjust_op_size(struct spi_slave *slave,
>   	 * or the output+input data must not exceed the GPRAM size.
>   	 */
>   
> -	nbytes = sizeof(op->cmd.opcode) + op->addr.nbytes +
> -		op->dummy.nbytes;
> +	nbytes = op->cmd.nbytes + op->addr.nbytes + op->dummy.nbytes;
>   
>   	if (nbytes + op->data.nbytes <= SNFI_GPRAM_SIZE)
>   		return 0;
> diff --git a/drivers/spi/spi-mem-nodm.c b/drivers/spi/spi-mem-nodm.c
> index 765f05fe54..db54101383 100644
> --- a/drivers/spi/spi-mem-nodm.c
> +++ b/drivers/spi/spi-mem-nodm.c
> @@ -27,7 +27,7 @@ int spi_mem_exec_op(struct spi_slave *slave,
>   			tx_buf = op->data.buf.out;
>   	}
>   
> -	op_len = sizeof(op->cmd.opcode) + op->addr.nbytes + op->dummy.nbytes;
> +	op_len = op->cmd.nbytes + op->addr.nbytes + op->dummy.nbytes;
>   	op_buf = calloc(1, op_len);
>   
>   	ret = spi_claim_bus(slave);
> @@ -89,7 +89,7 @@ int spi_mem_adjust_op_size(struct spi_slave *slave,
>   {
>   	unsigned int len;
>   
> -	len = sizeof(op->cmd.opcode) + op->addr.nbytes + op->dummy.nbytes;
> +	len = op->cmd.nbytes + op->addr.nbytes + op->dummy.nbytes;
>   	if (slave->max_write_size && len > slave->max_write_size)
>   		return -EINVAL;
>   
> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> index 427f7c13c5..541cd0e5a7 100644
> --- a/drivers/spi/spi-mem.c
> +++ b/drivers/spi/spi-mem.c
> @@ -167,6 +167,9 @@ bool spi_mem_default_supports_op(struct spi_slave *slave,
>   	if (op->cmd.dtr || op->addr.dtr || op->dummy.dtr || op->data.dtr)
>   		return false;
>   
> +	if (op->cmd.nbytes != 1)
> +		return false;
> +
>   	return true;
>   }
>   EXPORT_SYMBOL_GPL(spi_mem_default_supports_op);
> @@ -273,8 +276,7 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
>   	}
>   
>   #ifndef __UBOOT__
> -	tmpbufsize = sizeof(op->cmd.opcode) + op->addr.nbytes +
> -		     op->dummy.nbytes;
> +	tmpbufsize = op->cmd.nbytes + op->addr.nbytes + op->dummy.nbytes;
>   
>   	/*
>   	 * Allocate a buffer to transmit the CMD, ADDR cycles with kmalloc() so
> @@ -289,7 +291,7 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
>   
>   	tmpbuf[0] = op->cmd.opcode;
>   	xfers[xferpos].tx_buf = tmpbuf;
> -	xfers[xferpos].len = sizeof(op->cmd.opcode);
> +	xfers[xferpos].len = op->cmd.nbytes;
>   	xfers[xferpos].tx_nbits = op->cmd.buswidth;
>   	spi_message_add_tail(&xfers[xferpos], &msg);
>   	xferpos++;
> @@ -353,7 +355,7 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
>   			tx_buf = op->data.buf.out;
>   	}
>   
> -	op_len = sizeof(op->cmd.opcode) + op->addr.nbytes + op->dummy.nbytes;
> +	op_len = op->cmd.nbytes + op->addr.nbytes + op->dummy.nbytes;
>   
>   	/*
>   	 * Avoid using malloc() here so that we can use this code in SPL where
> @@ -442,8 +444,7 @@ int spi_mem_adjust_op_size(struct spi_slave *slave, struct spi_mem_op *op)
>   	if (!ops->mem_ops || !ops->mem_ops->exec_op) {
>   		unsigned int len;
>   
> -		len = sizeof(op->cmd.opcode) + op->addr.nbytes +
> -			op->dummy.nbytes;
> +		len = op->cmd.nbytes + op->addr.nbytes + op->dummy.nbytes;
>   		if (slave->max_write_size && len > slave->max_write_size)
>   			return -EINVAL;
>   
> diff --git a/include/spi-mem.h b/include/spi-mem.h
> index 9e6b044548..3e5b771045 100644
> --- a/include/spi-mem.h
> +++ b/include/spi-mem.h
> @@ -17,6 +17,7 @@ struct udevice;
>   	{							\
>   		.buswidth = __buswidth,				\
>   		.opcode = __opcode,				\
> +		.nbytes = 1,					\
>   	}
>   
>   #define SPI_MEM_OP_ADDR(__nbytes, __val, __buswidth)		\
> @@ -69,6 +70,8 @@ enum spi_mem_data_dir {
>   
>   /**
>    * struct spi_mem_op - describes a SPI memory operation
> + * @cmd.nbytes: number of opcode bytes (only 1 or 2 are valid). The opcode is
> + *		sent MSB-first.
>    * @cmd.buswidth: number of IO lines used to transmit the command
>    * @cmd.opcode: operation opcode
>    * @cmd.dtr: whether the command opcode should be sent in DTR mode or not
> @@ -92,9 +95,10 @@ enum spi_mem_data_dir {
>    */
>   struct spi_mem_op {
>   	struct {
> +		u8 nbytes;
>   		u8 buswidth;
> -		u8 opcode;
>   		u8 dtr : 1;
> +		u16 opcode;

Shouldn't the opcode go first for alignment?

--Sean

>   	} cmd;
>   
>   	struct {
>