From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pratyush Yadav Date: Mon, 5 Apr 2021 13:48:51 +0530 Subject: [PATCH v8 02/28] spi: spi-mem: allow specifying a command's extension In-Reply-To: <6194306f-beef-539a-6c80-bdad52c8c480@gmail.com> References: <20210401193133.18129-1-p.yadav@ti.com> <20210401193133.18129-3-p.yadav@ti.com> <6194306f-beef-539a-6c80-bdad52c8c480@gmail.com> Message-ID: <20210405081849.fypfibathb2n23cc@ti.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > > --- > > 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.