| Message ID | 20200522101301.26909-3-p.yadav@ti.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | mtd: spi-nor: add xSPI Octal DTR support | expand |
On Fri, 22 May 2020 15:42:43 +0530 Pratyush Yadav <p.yadav@ti.com> 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. A slightly different version of patch 5 should go before this patch, otherwise your series is not bisectable. By slightly different, I mean that you should only write one byte, but put this byte in a temporary var. Or maybe you can squash patch 5 in this one and mention why you do so in your commit message. > > Signed-off-by: Pratyush Yadav <p.yadav@ti.com> > --- > drivers/spi/spi-mem.c | 5 ++++- > include/linux/spi/spi-mem.h | 6 +++++- > 2 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c > index 93e255287ab9..29dcd1d62710 100644 > --- a/drivers/spi/spi-mem.c > +++ b/drivers/spi/spi-mem.c > @@ -159,6 +159,9 @@ bool spi_mem_default_supports_op(struct spi_mem *mem, > 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); > @@ -173,7 +176,7 @@ static bool spi_mem_buswidth_is_valid(u8 buswidth) > > static int spi_mem_check_op(const struct spi_mem_op *op) > { > - if (!op->cmd.buswidth) > + if (!op->cmd.buswidth || !op->cmd.nbytes) > return -EINVAL; > > if ((op->addr.nbytes && !op->addr.buswidth) || > diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h > index e3dcb956bf61..159463cc659c 100644 > --- a/include/linux/spi/spi-mem.h > +++ b/include/linux/spi/spi-mem.h > @@ -17,6 +17,7 @@ > { \ > .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 > @@ -94,9 +97,10 @@ enum spi_mem_data_dir { > */ > struct spi_mem_op { > struct { > + u8 nbytes; > u8 buswidth; > u8 dtr : 1; > - u8 opcode; > + u16 opcode; > } cmd; > > struct {
On 22/05/20 05:32PM, Boris Brezillon wrote: > On Fri, 22 May 2020 15:42:43 +0530 > Pratyush Yadav <p.yadav@ti.com> 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. > > A slightly different version of patch 5 should go before this patch, > otherwise your series is not bisectable. By slightly different, I mean > that you should only write one byte, but put this byte in a temporary > var. Or maybe you can squash patch 5 in this one and mention why you do > so in your commit message. How about the patch below before this patch? The supports_op() will reject multi-byte opcodes anyway, so we only care about single-byte opcodes for now. Multi-byte opcodes can be patched and tested later. This avoids squashing changes in this patch and having the changes split over two patches; one before and one after. -- 8< -- diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c index 69491f3a515d..4e4292f0ee1d 100644 --- a/drivers/spi/spi-mxic.c +++ b/drivers/spi/spi-mxic.c @@ -356,6 +356,7 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, int nio = 1, i, ret; u32 ss_ctrl; u8 addr[8]; + u8 opcode = op->cmd.opcode & 0xff; ret = mxic_spi_set_freq(mxic, mem->spi->max_speed_hz); if (ret) @@ -393,7 +394,7 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, writel(readl(mxic->regs + HC_CFG) | HC_CFG_MAN_CS_ASSERT, mxic->regs + HC_CFG); - ret = mxic_spi_data_xfer(mxic, &op->cmd.opcode, NULL, 1); + ret = mxic_spi_data_xfer(mxic, &opcode, NULL, 1); if (ret) goto out;
diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c index 93e255287ab9..29dcd1d62710 100644 --- a/drivers/spi/spi-mem.c +++ b/drivers/spi/spi-mem.c @@ -159,6 +159,9 @@ bool spi_mem_default_supports_op(struct spi_mem *mem, 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); @@ -173,7 +176,7 @@ static bool spi_mem_buswidth_is_valid(u8 buswidth) static int spi_mem_check_op(const struct spi_mem_op *op) { - if (!op->cmd.buswidth) + if (!op->cmd.buswidth || !op->cmd.nbytes) return -EINVAL; if ((op->addr.nbytes && !op->addr.buswidth) || diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h index e3dcb956bf61..159463cc659c 100644 --- a/include/linux/spi/spi-mem.h +++ b/include/linux/spi/spi-mem.h @@ -17,6 +17,7 @@ { \ .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 @@ -94,9 +97,10 @@ enum spi_mem_data_dir { */ struct spi_mem_op { struct { + u8 nbytes; u8 buswidth; u8 dtr : 1; - u8 opcode; + u16 opcode; } cmd; struct {
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. Signed-off-by: Pratyush Yadav <p.yadav@ti.com> --- drivers/spi/spi-mem.c | 5 ++++- include/linux/spi/spi-mem.h | 6 +++++- 2 files changed, 9 insertions(+), 2 deletions(-)