Re: [PATCH v8 01/28] spi: spi-mem: allow specifying whether an op is DTR or not

On Mon, Apr 05, 2021 at 01:55:06PM +0530, Pratyush Yadav wrote:

On 02/04/21 06:21PM, Sean Anderson wrote:

...

On 4/1/21 3:31 PM, Pratyush Yadav wrote:

...

Each phase is given a separate 'dtr' field so mixed protocols like 4S-4D-4D can be supported.

Signed-off-by: Pratyush Yadav p.yadav@ti.com

drivers/spi/spi-mem.c | 3 +++ include/spi-mem.h | 8 ++++++++ 2 files changed, 11 insertions(+)

diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c index c095ae9505..427f7c13c5 100644 --- a/drivers/spi/spi-mem.c +++ b/drivers/spi/spi-mem.c @@ -164,6 +164,9 @@ bool spi_mem_default_supports_op(struct spi_slave *slave, op->data.dir == SPI_MEM_DATA_OUT)) return false;

• if (op->cmd.dtr || op->addr.dtr || op->dummy.dtr || op->data.dtr)

• return false;
  

• return true; } EXPORT_SYMBOL_GPL(spi_mem_default_supports_op);

diff --git a/include/spi-mem.h b/include/spi-mem.h index 8be3e2bf6b..9e6b044548 100644 --- a/include/spi-mem.h +++ b/include/spi-mem.h @@ -71,6 +71,7 @@ enum spi_mem_data_dir {

• struct spi_mem_op - describes a SPI memory operation
• @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
  • @addr.nbytes: number of address bytes to send. Can be zero if the operation

  •  does not need to send an address
    

  • @addr.buswidth: number of IO lines used to transmit the address cycles

@@ -78,10 +79,13 @@ enum spi_mem_data_dir {

•      Note that only @addr.nbytes are taken into account in this
  

•      address value, so users should make sure the value fits in the
  

•      assigned number of bytes.
  

• • @addr.dtr: whether the address should be sent in DTR mode or not
  • @dummy.nbytes: number of dummy bytes to send after an opcode or address. Can

  •   be zero if the operation does not require dummy bytes
    

  • @dummy.buswidth: number of IO lanes used to transmit the dummy bytes
• • @dummy.dtr: whether the dummy bytes should be sent in DTR mode or not
  • @data.buswidth: number of IO lanes used to send/receive the data
• • @data.dtr: whether the data should be sent in DTR mode or not
  • @data.dir: direction of the transfer
  • @data.buf.in: input buffer
  • @data.buf.out: output buffer

@@ -90,21 +94,25 @@ struct spi_mem_op { struct { u8 buswidth; u8 opcode;

• u8 dtr : 1;
  

  } cmd; struct { u8 nbytes; u8 buswidth;

• u8 dtr : 1;
  

  u64 val; } addr; struct { u8 nbytes; u8 buswidth;

• u8 dtr : 1;
  

  } dummy; struct { u8 buswidth;

• u8 dtr : 1;
  

  enum spi_mem_data_dir dir; unsigned int nbytes; /* buf.{in,out} must be DMA-able. */

I know this is following the Linux code, but are bitfields kosher for U-Boot? This is more of a general question than a specific critique of this code.

I'm not sure. I did a quick grep and I do see some usages of bitfields but I'm not sure if these just slipped in with ports from Linux or they are "officially" allowed.

We try to be a friendly environment to people used to working on the Linux kernel, so is there a reason we wouldn't allow bitfields?