[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(-)

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.