| Message ID | 20181012084825.23697-6-boris.brezillon@bootlin.com |
|---|---|
| State | Under Review, archived |
| Delegated to: | Ambarus Tudor |
| Headers | show |
| Series | mtd: spi-nor: Proposal for 8-8-8 mode support | expand |
Hi Boris, Boris Brezillon <boris.brezillon@bootlin.com> wrote on Fri, 12 Oct 2018 10:48:12 +0200: > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > --- > drivers/spi/spi-mxic.c | 27 +++++++++++++++++++++------ > 1 file changed, 21 insertions(+), 6 deletions(-) > > diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c > index ce59ea2ecfe2..70e6bc9a099e 100644 > --- a/drivers/spi/spi-mxic.c > +++ b/drivers/spi/spi-mxic.c > @@ -281,10 +281,11 @@ static int mxic_spi_data_xfer(struct mxic_spi *mxic, const void *txbuf, > static bool mxic_spi_mem_supports_op(struct spi_mem *mem, > const struct spi_mem_op *op) > { > - if (op->data.buswidth > 4 || op->addr.buswidth > 4 || > - op->dummy.buswidth > 4 || op->cmd.buswidth > 4) > + if (op->data.buswidth > 8 || op->addr.buswidth > 8 || > + op->dummy.buswidth > 8 || op->cmd.buswidth > 8) > return false; > > + Extra space here > if (op->data.nbytes && op->dummy.nbytes && > op->data.buswidth != op->dummy.buswidth) > return false; > @@ -302,6 +303,7 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, > int nio = 1, i, ret; > u32 ss_ctrl; > u8 addr[8]; > + u8 cmd[2]; > > ret = mxic_spi_clk_setup(mem->spi); > if (ret) > @@ -311,6 +313,8 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, > if (ret) > return ret; > > + if (mem->spi->mode & (SPI_RX_OCTO | SPI_TX_OCTO)) > + nio = 8; > if (mem->spi->mode & (SPI_TX_QUAD | SPI_RX_QUAD)) > nio = 4; > else if (mem->spi->mode & (SPI_TX_DUAL | SPI_RX_DUAL)) > @@ -323,17 +327,21 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, > mxic->regs + HC_CFG); > writel(HC_EN_BIT, mxic->regs + HC_EN); > > - ss_ctrl = OP_CMD_BYTES(1) | OP_CMD_BUSW(fls(op->cmd.buswidth) - 1); > + ss_ctrl = OP_CMD_BYTES(op->cmd.nbytes) | > + OP_CMD_BUSW(fls(op->cmd.buswidth) - 1) | > + (op->cmd.dtr ? OP_CMD_DDR : 0); > > if (op->addr.nbytes) > ss_ctrl |= OP_ADDR_BYTES(op->addr.nbytes) | > - OP_ADDR_BUSW(fls(op->addr.buswidth) - 1); > + OP_ADDR_BUSW(fls(op->addr.buswidth) - 1) | > + (op->addr.dtr ? OP_ADDR_DDR : 0); > > if (op->dummy.nbytes) > ss_ctrl |= OP_DUMMY_CYC(op->dummy.nbytes); > > if (op->data.nbytes) { > - ss_ctrl |= OP_DATA_BUSW(fls(op->data.buswidth) - 1); > + ss_ctrl |= OP_DATA_BUSW(fls(op->data.buswidth) - 1) | > + (op->data.dtr ? OP_DATA_DDR : 0); > if (op->data.dir == SPI_MEM_DATA_IN) > ss_ctrl |= OP_READ; > } > @@ -343,7 +351,14 @@ 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); > + if (op->cmd.nbytes == 2) { > + cmd[0] = op->cmd.opcode >> 8; > + cmd[1] = op->cmd.opcode; > + } else { > + cmd[0] = op->cmd.opcode; > + } I haven't played with this code yet and maybe I'll regret this but wouldn't be easier for developers to have this in patch 4: struct spi_mem_op { struct { + u8 nbytes; u8 buswidth; bool dtr; - u8 opcode; + u8 opcode[2]; /* <- an array of opcodes instead of an u16? */ } cmd; This way I think we would avoid endianness considerations and reading would be eased. > + > + ret = mxic_spi_data_xfer(mxic, cmd, NULL, op->cmd.nbytes); > if (ret) > goto out; > Thanks, Miquèl
On Sun, 18 Nov 2018 18:21:11 +0100 Miquel Raynal <miquel.raynal@bootlin.com> wrote: > Hi Boris, > > Boris Brezillon <boris.brezillon@bootlin.com> wrote on Fri, 12 Oct 2018 > 10:48:12 +0200: > > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > > --- > > drivers/spi/spi-mxic.c | 27 +++++++++++++++++++++------ > > 1 file changed, 21 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c > > index ce59ea2ecfe2..70e6bc9a099e 100644 > > --- a/drivers/spi/spi-mxic.c > > +++ b/drivers/spi/spi-mxic.c > > @@ -281,10 +281,11 @@ static int mxic_spi_data_xfer(struct mxic_spi *mxic, const void *txbuf, > > static bool mxic_spi_mem_supports_op(struct spi_mem *mem, > > const struct spi_mem_op *op) > > { > > - if (op->data.buswidth > 4 || op->addr.buswidth > 4 || > > - op->dummy.buswidth > 4 || op->cmd.buswidth > 4) > > + if (op->data.buswidth > 8 || op->addr.buswidth > 8 || > > + op->dummy.buswidth > 8 || op->cmd.buswidth > 8) > > return false; > > > > + > > Extra space here > > > if (op->data.nbytes && op->dummy.nbytes && > > op->data.buswidth != op->dummy.buswidth) > > return false; > > @@ -302,6 +303,7 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, > > int nio = 1, i, ret; > > u32 ss_ctrl; > > u8 addr[8]; > > + u8 cmd[2]; > > > > ret = mxic_spi_clk_setup(mem->spi); > > if (ret) > > @@ -311,6 +313,8 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, > > if (ret) > > return ret; > > > > + if (mem->spi->mode & (SPI_RX_OCTO | SPI_TX_OCTO)) > > + nio = 8; > > if (mem->spi->mode & (SPI_TX_QUAD | SPI_RX_QUAD)) > > nio = 4; > > else if (mem->spi->mode & (SPI_TX_DUAL | SPI_RX_DUAL)) > > @@ -323,17 +327,21 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, > > mxic->regs + HC_CFG); > > writel(HC_EN_BIT, mxic->regs + HC_EN); > > > > - ss_ctrl = OP_CMD_BYTES(1) | OP_CMD_BUSW(fls(op->cmd.buswidth) - 1); > > + ss_ctrl = OP_CMD_BYTES(op->cmd.nbytes) | > > + OP_CMD_BUSW(fls(op->cmd.buswidth) - 1) | > > + (op->cmd.dtr ? OP_CMD_DDR : 0); > > > > if (op->addr.nbytes) > > ss_ctrl |= OP_ADDR_BYTES(op->addr.nbytes) | > > - OP_ADDR_BUSW(fls(op->addr.buswidth) - 1); > > + OP_ADDR_BUSW(fls(op->addr.buswidth) - 1) | > > + (op->addr.dtr ? OP_ADDR_DDR : 0); > > > > if (op->dummy.nbytes) > > ss_ctrl |= OP_DUMMY_CYC(op->dummy.nbytes); > > > > if (op->data.nbytes) { > > - ss_ctrl |= OP_DATA_BUSW(fls(op->data.buswidth) - 1); > > + ss_ctrl |= OP_DATA_BUSW(fls(op->data.buswidth) - 1) | > > + (op->data.dtr ? OP_DATA_DDR : 0); > > if (op->data.dir == SPI_MEM_DATA_IN) > > ss_ctrl |= OP_READ; > > } > > @@ -343,7 +351,14 @@ 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); > > + if (op->cmd.nbytes == 2) { > > + cmd[0] = op->cmd.opcode >> 8; > > + cmd[1] = op->cmd.opcode; > > + } else { > > + cmd[0] = op->cmd.opcode; > > + } > > I haven't played with this code yet and maybe I'll regret this but > wouldn't be easier for developers to have this in patch 4: > > struct spi_mem_op { > struct { > + u8 nbytes; > u8 buswidth; > bool dtr; > - u8 opcode; > + u8 opcode[2]; /* <- an array of opcodes instead of an u16? */ Just wanted to stay consistent with what we have for the address cycles, but maybe I should clarify how the opcode should be transmitted on the wire (MSB first). > } cmd; > > This way I think we would avoid endianness considerations and reading > would be eased. > > > + > > + ret = mxic_spi_data_xfer(mxic, cmd, NULL, op->cmd.nbytes); > > if (ret) > > goto out; > > > > > Thanks, > Miquèl
diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c index ce59ea2ecfe2..70e6bc9a099e 100644 --- a/drivers/spi/spi-mxic.c +++ b/drivers/spi/spi-mxic.c @@ -281,10 +281,11 @@ static int mxic_spi_data_xfer(struct mxic_spi *mxic, const void *txbuf, static bool mxic_spi_mem_supports_op(struct spi_mem *mem, const struct spi_mem_op *op) { - if (op->data.buswidth > 4 || op->addr.buswidth > 4 || - op->dummy.buswidth > 4 || op->cmd.buswidth > 4) + if (op->data.buswidth > 8 || op->addr.buswidth > 8 || + op->dummy.buswidth > 8 || op->cmd.buswidth > 8) return false; + if (op->data.nbytes && op->dummy.nbytes && op->data.buswidth != op->dummy.buswidth) return false; @@ -302,6 +303,7 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, int nio = 1, i, ret; u32 ss_ctrl; u8 addr[8]; + u8 cmd[2]; ret = mxic_spi_clk_setup(mem->spi); if (ret) @@ -311,6 +313,8 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, if (ret) return ret; + if (mem->spi->mode & (SPI_RX_OCTO | SPI_TX_OCTO)) + nio = 8; if (mem->spi->mode & (SPI_TX_QUAD | SPI_RX_QUAD)) nio = 4; else if (mem->spi->mode & (SPI_TX_DUAL | SPI_RX_DUAL)) @@ -323,17 +327,21 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, mxic->regs + HC_CFG); writel(HC_EN_BIT, mxic->regs + HC_EN); - ss_ctrl = OP_CMD_BYTES(1) | OP_CMD_BUSW(fls(op->cmd.buswidth) - 1); + ss_ctrl = OP_CMD_BYTES(op->cmd.nbytes) | + OP_CMD_BUSW(fls(op->cmd.buswidth) - 1) | + (op->cmd.dtr ? OP_CMD_DDR : 0); if (op->addr.nbytes) ss_ctrl |= OP_ADDR_BYTES(op->addr.nbytes) | - OP_ADDR_BUSW(fls(op->addr.buswidth) - 1); + OP_ADDR_BUSW(fls(op->addr.buswidth) - 1) | + (op->addr.dtr ? OP_ADDR_DDR : 0); if (op->dummy.nbytes) ss_ctrl |= OP_DUMMY_CYC(op->dummy.nbytes); if (op->data.nbytes) { - ss_ctrl |= OP_DATA_BUSW(fls(op->data.buswidth) - 1); + ss_ctrl |= OP_DATA_BUSW(fls(op->data.buswidth) - 1) | + (op->data.dtr ? OP_DATA_DDR : 0); if (op->data.dir == SPI_MEM_DATA_IN) ss_ctrl |= OP_READ; } @@ -343,7 +351,14 @@ 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); + if (op->cmd.nbytes == 2) { + cmd[0] = op->cmd.opcode >> 8; + cmd[1] = op->cmd.opcode; + } else { + cmd[0] = op->cmd.opcode; + } + + ret = mxic_spi_data_xfer(mxic, cmd, NULL, op->cmd.nbytes); if (ret) goto out;
Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> --- drivers/spi/spi-mxic.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-)