| Message ID | 1612517808-10010-3-git-send-email-zhengxunli@mxic.com.tw (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add octal DTR support for Macronix flash | expand |
Hi Mark, I see that Pratyush patch "spi: spi-mem: add spi_mem_dtr_supports_op()" has been accepted, can you help review this patch and make some suggestions? "zhengxunli" <zhengxunli@mxic.com.tw> wrote on 2021/02/05 下午 05:36:48: > "zhengxunli" <zhengxunli@mxic.com.tw> > 2021/02/05 下午 05:40 > > To > > linux-mtd@lists.infradead.org, linux-spi@vger.kernel.org, > miquel.raynal@bootlin.com, broonie@kernel.org, vigneshr@ti.com, > > cc > > ycllin@mxic.com.tw, juliensu@mxic.com.tw, "zhengxunli" > <zhengxunli@mxic.com.tw> > > Subject > > [PATCH v2 2/2] spi: mxic: patch for octal DTR mode support > > Driver patch for octal DTR mode support. > > Owing to the spi_mem_default_supports_op() is not support dtr > operation. Based on Pratyush patch "spi: spi-mem: add spi_mem_dtr > _supports_op()" add spi_mem_dtr_supports_op() to support dtr and > keep checking the buswidth and command bytes. > > Signed-off-by: zhengxunli <zhengxunli@mxic.com.tw> > --- > drivers/spi/spi-mxic.c | 41 ++++++++++++++++++++++++++++++----------- > 1 file changed, 30 insertions(+), 11 deletions(-) > > diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c > index 96b4182..32e757a 100644 > --- a/drivers/spi/spi-mxic.c > +++ b/drivers/spi/spi-mxic.c > @@ -335,8 +335,10 @@ 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) > + bool all_false; > + > + 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 && > @@ -346,7 +348,13 @@ static bool mxic_spi_mem_supports_op(struct spi_mem *mem, > if (op->addr.nbytes > 7) > return false; > > - return spi_mem_default_supports_op(mem, op); > + all_false = !op->cmd.dtr && !op->addr.dtr && !op->dummy.dtr && > + !op->data.dtr; > + > + if (all_false) > + return spi_mem_default_supports_op(mem, op); > + else > + return spi_mem_dtr_supports_op(mem, op); > } > > static int mxic_spi_mem_exec_op(struct spi_mem *mem, > @@ -355,14 +363,15 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, > struct mxic_spi *mxic = spi_master_get_devdata(mem->spi->master); > int nio = 1, i, ret; > u32 ss_ctrl; > - u8 addr[8]; > - u8 opcode = op->cmd.opcode; > + u8 addr[8], cmd[2]; > > ret = mxic_spi_set_freq(mxic, mem->spi->max_speed_hz); > if (ret) > return ret; > > - if (mem->spi->mode & (SPI_TX_QUAD | SPI_RX_QUAD)) > + if (mem->spi->mode & (SPI_TX_OCTAL | SPI_RX_OCTAL)) > + nio = 8; > + else if (mem->spi->mode & (SPI_TX_QUAD | SPI_RX_QUAD)) > nio = 4; > else if (mem->spi->mode & (SPI_TX_DUAL | SPI_RX_DUAL)) > nio = 2; > @@ -374,19 +383,25 @@ 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; > + if (op->data.dtr) > + ss_ctrl |= OP_DQS_EN; > } > > writel(ss_ctrl, mxic->regs + SS_CTRL(mem->spi->chip_select)); > @@ -394,7 +409,10 @@ 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, &opcode, NULL, 1); > + for (i = 0; i < op->cmd.nbytes; i++) > + cmd[i] = op->cmd.opcode >> (8 * (op->cmd.nbytes - i - 1)); > + > + ret = mxic_spi_data_xfer(mxic, cmd, NULL, op->cmd.nbytes); > if (ret) > goto out; > > @@ -567,7 +585,8 @@ static int mxic_spi_probe(struct platform_device *pdev) > master->bits_per_word_mask = SPI_BPW_MASK(8); > master->mode_bits = SPI_CPOL | SPI_CPHA | > SPI_RX_DUAL | SPI_TX_DUAL | > - SPI_RX_QUAD | SPI_TX_QUAD; > + SPI_RX_QUAD | SPI_TX_QUAD | > + SPI_RX_OCTAL | SPI_TX_OCTAL; > > mxic_spi_hw_init(mxic); > > -- > 1.9.1 > Thanks, Zhengxun CONFIDENTIALITY NOTE: This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation. Macronix International Co., Ltd. ===================================================================== ============================================================================ CONFIDENTIALITY NOTE: This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation. Macronix International Co., Ltd. =====================================================================
On Fri, Feb 19, 2021 at 01:58:40PM +0800, zhengxunli@mxic.com.tw wrote: > Hi Mark, > > I see that Pratyush patch "spi: spi-mem: add spi_mem_dtr_supports_op()" > has been accepted, can you help review this patch and make some > suggestions? Please don't send content free pings and please allow a reasonable time for review. People get busy, go on holiday, attend conferences and so on so unless there is some reason for urgency (like critical bug fixes) please allow at least a couple of weeks for review. If there have been review comments then people may be waiting for those to be addressed. Sending content free pings adds to the mail volume (if they are seen at all) which is often the problem and since they can't be reviewed directly if something has gone wrong you'll have to resend the patches anyway, so sending again is generally a better approach though there are some other maintainers who like them - if in doubt look at how patches for the subsystem are normally handled. Please don't top post, reply in line with needed context. This allows readers to readily follow the flow of conversation and understand what you are talking about and also helps ensure that everything in the discussion is being addressed.
diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c index 96b4182..32e757a 100644 --- a/drivers/spi/spi-mxic.c +++ b/drivers/spi/spi-mxic.c @@ -335,8 +335,10 @@ 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) + bool all_false; + + 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 && @@ -346,7 +348,13 @@ static bool mxic_spi_mem_supports_op(struct spi_mem *mem, if (op->addr.nbytes > 7) return false; - return spi_mem_default_supports_op(mem, op); + all_false = !op->cmd.dtr && !op->addr.dtr && !op->dummy.dtr && + !op->data.dtr; + + if (all_false) + return spi_mem_default_supports_op(mem, op); + else + return spi_mem_dtr_supports_op(mem, op); } static int mxic_spi_mem_exec_op(struct spi_mem *mem, @@ -355,14 +363,15 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, struct mxic_spi *mxic = spi_master_get_devdata(mem->spi->master); int nio = 1, i, ret; u32 ss_ctrl; - u8 addr[8]; - u8 opcode = op->cmd.opcode; + u8 addr[8], cmd[2]; ret = mxic_spi_set_freq(mxic, mem->spi->max_speed_hz); if (ret) return ret; - if (mem->spi->mode & (SPI_TX_QUAD | SPI_RX_QUAD)) + if (mem->spi->mode & (SPI_TX_OCTAL | SPI_RX_OCTAL)) + nio = 8; + else if (mem->spi->mode & (SPI_TX_QUAD | SPI_RX_QUAD)) nio = 4; else if (mem->spi->mode & (SPI_TX_DUAL | SPI_RX_DUAL)) nio = 2; @@ -374,19 +383,25 @@ 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; + if (op->data.dtr) + ss_ctrl |= OP_DQS_EN; } writel(ss_ctrl, mxic->regs + SS_CTRL(mem->spi->chip_select)); @@ -394,7 +409,10 @@ 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, &opcode, NULL, 1); + for (i = 0; i < op->cmd.nbytes; i++) + cmd[i] = op->cmd.opcode >> (8 * (op->cmd.nbytes - i - 1)); + + ret = mxic_spi_data_xfer(mxic, cmd, NULL, op->cmd.nbytes); if (ret) goto out; @@ -567,7 +585,8 @@ static int mxic_spi_probe(struct platform_device *pdev) master->bits_per_word_mask = SPI_BPW_MASK(8); master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_DUAL | SPI_TX_DUAL | - SPI_RX_QUAD | SPI_TX_QUAD; + SPI_RX_QUAD | SPI_TX_QUAD | + SPI_RX_OCTAL | SPI_TX_OCTAL; mxic_spi_hw_init(mxic);
Driver patch for octal DTR mode support. Owing to the spi_mem_default_supports_op() is not support dtr operation. Based on Pratyush patch "spi: spi-mem: add spi_mem_dtr _supports_op()" add spi_mem_dtr_supports_op() to support dtr and keep checking the buswidth and command bytes. Signed-off-by: zhengxunli <zhengxunli@mxic.com.tw> --- drivers/spi/spi-mxic.c | 41 ++++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 11 deletions(-)