From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.bootlin.com ([62.4.15.54]) by merlin.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1gOQlD-0001BB-34 for linux-mtd@lists.infradead.org; Sun, 18 Nov 2018 17:21:37 +0000 Date: Sun, 18 Nov 2018 18:21:11 +0100 From: Miquel Raynal To: Boris Brezillon Cc: David Woodhouse , Brian Norris , Marek Vasut , Richard Weinberger , linux-mtd@lists.infradead.org, Yogesh Gaur , Vignesh R , Cyrille Pitchen , Julien Su , Mark Brown , Mason Yang , linux-spi@vger.kernel.org, zhengxunli@mxic.com.tw Subject: Re: [PATCH RFC 05/18] spi: spi-mem: mxic: Add support for DTR and Octo mode Message-ID: <20181118182111.3c199434@xps13> In-Reply-To: <20181012084825.23697-6-boris.brezillon@bootlin.com> References: <20181012084825.23697-1-boris.brezillon@bootlin.com> <20181012084825.23697-6-boris.brezillon@bootlin.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Hi Boris, Boris Brezillon wrote on Fri, 12 Oct 2018 10:48:12 +0200: > Signed-off-by: Boris Brezillon > --- > drivers/spi/spi-mxic.c | 27 +++++++++++++++++++++------ > 1 file changed, 21 insertions(+), 6 deletions(-) >=20 > 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; > =20 > + Extra space here > if (op->data.nbytes && op->dummy.nbytes && > op->data.buswidth !=3D op->dummy.buswidth) > return false; > @@ -302,6 +303,7 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem, > int nio =3D 1, i, ret; > u32 ss_ctrl; > u8 addr[8]; > + u8 cmd[2]; > =20 > ret =3D 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; > =20 > + if (mem->spi->mode & (SPI_RX_OCTO | SPI_TX_OCTO)) > + nio =3D 8; > if (mem->spi->mode & (SPI_TX_QUAD | SPI_RX_QUAD)) > nio =3D 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); > =20 > - ss_ctrl =3D OP_CMD_BYTES(1) | OP_CMD_BUSW(fls(op->cmd.buswidth) - 1); > + ss_ctrl =3D OP_CMD_BYTES(op->cmd.nbytes) | > + OP_CMD_BUSW(fls(op->cmd.buswidth) - 1) | > + (op->cmd.dtr ? OP_CMD_DDR : 0); > =20 > if (op->addr.nbytes) > ss_ctrl |=3D 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); > =20 > if (op->dummy.nbytes) > ss_ctrl |=3D OP_DUMMY_CYC(op->dummy.nbytes); > =20 > if (op->data.nbytes) { > - ss_ctrl |=3D OP_DATA_BUSW(fls(op->data.buswidth) - 1); > + ss_ctrl |=3D OP_DATA_BUSW(fls(op->data.buswidth) - 1) | > + (op->data.dtr ? OP_DATA_DDR : 0); > if (op->data.dir =3D=3D SPI_MEM_DATA_IN) > ss_ctrl |=3D 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); > =20 > - ret =3D mxic_spi_data_xfer(mxic, &op->cmd.opcode, NULL, 1); > + if (op->cmd.nbytes =3D=3D 2) { > + cmd[0] =3D op->cmd.opcode >> 8; > + cmd[1] =3D op->cmd.opcode; > + } else { > + cmd[0] =3D 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 =3D mxic_spi_data_xfer(mxic, cmd, NULL, op->cmd.nbytes); > if (ret) > goto out; > =20 Thanks, Miqu=C3=A8l