[PATCH v8 02/28] spi: spi-mem: allow specifying a command's extension
Pratyush Yadav
p.yadav at ti.com
Mon Apr 5 10:18:51 CEST 2021
On 02/04/21 06:29PM, Sean Anderson wrote:
> 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?
Right. I just put all the 'u8's together. But this order is now in
mainline Linux so I'm not sure if we should change that here. The
spi-mem subsystem in U-Boot generally tries to mirror what Linux does.
>
> --Sean
>
> > } cmd;
> > struct {
> >
>
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
More information about the U-Boot
mailing list