diff mbox series

[RFC,05/18] spi: spi-mem: mxic: Add support for DTR and Octo mode

Message ID 20181012084825.23697-6-boris.brezillon@bootlin.com (mailing list archive)
State New, archived
Headers show
Series mtd: spi-nor: Proposal for 8-8-8 mode support | expand

Commit Message

Boris Brezillon Oct. 12, 2018, 8:48 a.m. UTC 
Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
 drivers/spi/spi-mxic.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

Comments

Miquel Raynal Nov. 18, 2018, 5:21 p.m. UTC | #1 
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
Boris Brezillon Nov. 18, 2018, 5:32 p.m. UTC | #2 
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 mbox series

Patch

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;