[PATCH 0/3] spi: spi-mtk-nor: make use of full capability of program mode

[PATCH 0/3] spi: spi-mtk-nor: make use of full capability of program mode

[Chuanhong Guo]

"program" mode on this controller can trigger up to 56 bits of data
shifting. During the operation, data in PRGDATA[0-5] will be
shifted out from MOSI, and data from MISO will be continuously filling
SHREG[0-9].
Currently this mode is used to implement transfer_one_message for 6-byte
full-duplex transfer, but it can execute a transfer for up-to 7 bytes
as long as the last byte is read only.
transfer_one_message is expected to perform full-duplex transfer,
instead of transfer with specific format. mtk_nor_spi_mem_prg is
added here to use this extra byte.

Newer version of this controller can trigger longer data shifting with
shift bytes more than PRGDATA_MAX + SHREG_MAX. This patch is implemented
with that in mind and it checks against both SHREG_MAX and PRG_CNT_MAX
for future support of new controllers.

Patch 3/3 is a fix for:
commit a59b2c7c56bf7 ("spi: spi-mtk-nor: support standard spi properties")
which breaks supports_op logic. But it can't be separated as it depends
on patch 2/3. Fortuantely the broken commit isn't in stable yet.

Chuanhong Guo (3):
  spi: spi-mtk-nor: make use of full capability of prg mode
  spi: spi-mtk-nor: add helper for checking prg mode ops
  spi: spi-mtk-nor: fix op checks in supports_op

 drivers/spi/spi-mtk-nor.c | 179 +++++++++++++++++++++++++++++++++-----
 1 file changed, 158 insertions(+), 21 deletions(-)

-- 
2.26.2

[PATCH 1/3] spi: spi-mtk-nor: make use of full capability of prg mode

[Chuanhong Guo]

"program" mode on this controller can trigger up to 56 bits of data
shifting. During the operation, data in PRGDATA[0-5] will be
shifted out from MOSI, and data from MISO will be continuously filling
SHREG[0-9].
Currently this mode is used to implement transfer_one_message for 6-byte
full-duplex transfer, but it can execute a transfer for up-to 7 bytes
as long as the last byte is read only.
transfer_one_message is expected to perform full-duplex transfer,
instead of transfer with specific format. mtk_nor_spi_mem_prg is
added here to use this extra byte.

Newer version of this controller can trigger longer data shifting with
shift bytes more than PRGDATA_MAX + SHREG_MAX. This patch is implemented
with that in mind and it checks against both SHREG_MAX and PRG_CNT_MAX
for future support of new controllers.

Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
---
There are two calls of mtk_nor_spi_mem_prg in mtk_nor_exec_op,
which doesn't look great. But I'd prefer keeping it this way
to minimize change of mtk_nor_exec_op.
I'll follow up with a cleanup patchset after this one gets
merged.

 drivers/spi/spi-mtk-nor.c | 82 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 80 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-mtk-nor.c b/drivers/spi/spi-mtk-nor.c
index 62f5ff2779884..a0087a5e869b9 100644
--- a/drivers/spi/spi-mtk-nor.c
+++ b/drivers/spi/spi-mtk-nor.c
@@ -27,6 +27,7 @@
 #define MTK_NOR_CMD_MASK		GENMASK(5, 0)
 
 #define MTK_NOR_REG_PRG_CNT		0x04
+#define MTK_NOR_PRG_CNT_MAX		56
 #define MTK_NOR_REG_RDATA		0x0c
 
 #define MTK_NOR_REG_RADR0		0x10
@@ -404,6 +405,83 @@ static int mtk_nor_pp_unbuffered(struct mtk_nor *sp,
 	return mtk_nor_cmd_exec(sp, MTK_NOR_CMD_WRITE, 6 * BITS_PER_BYTE);
 }
 
+static int mtk_nor_spi_mem_prg(struct mtk_nor *sp, const struct spi_mem_op *op)
+{
+	int rx_len = 0;
+	int reg_offset = MTK_NOR_REG_PRGDATA_MAX;
+	int tx_len, prg_len;
+	int i, ret;
+	void __iomem *reg;
+	u8 bufbyte;
+
+	tx_len = op->cmd.nbytes + op->addr.nbytes;
+
+	// count dummy bytes only if we need to write data after it
+	if (op->data.dir == SPI_MEM_DATA_OUT)
+		tx_len += op->dummy.nbytes + op->data.nbytes;
+	else if (op->data.dir == SPI_MEM_DATA_IN)
+		rx_len = op->data.nbytes;
+
+	prg_len = op->cmd.nbytes + op->addr.nbytes + op->dummy.nbytes +
+		  op->data.nbytes;
+
+	// an invalid op may reach here if the caller calls exec_op without
+	// adjust_op_size. return -EINVAL instead of -ENOTSUPP so that
+	// spi-mem won't try this op again with generic spi transfers.
+	if ((tx_len > MTK_NOR_REG_PRGDATA_MAX + 1) ||
+	    (rx_len > MTK_NOR_REG_SHIFT_MAX + 1) ||
+	    (prg_len > MTK_NOR_PRG_CNT_MAX / 8))
+		return -EINVAL;
+
+	// fill tx data
+	for (i = op->cmd.nbytes; i > 0; i--, reg_offset--) {
+		reg = sp->base + MTK_NOR_REG_PRGDATA(reg_offset);
+		bufbyte = (op->cmd.opcode >> ((i - 1) * BITS_PER_BYTE)) & 0xff;
+		writeb(bufbyte, reg);
+	}
+
+	for (i = op->addr.nbytes; i > 0; i--, reg_offset--) {
+		reg = sp->base + MTK_NOR_REG_PRGDATA(reg_offset);
+		bufbyte = (op->addr.val >> ((i - 1) * BITS_PER_BYTE)) & 0xff;
+		writeb(bufbyte, reg);
+	}
+
+	if (op->data.dir == SPI_MEM_DATA_OUT) {
+		for (i = 0; i < op->dummy.nbytes; i++, reg_offset--) {
+			reg = sp->base + MTK_NOR_REG_PRGDATA(reg_offset);
+			writeb(0, reg);
+		}
+
+		for (i = 0; i < op->data.nbytes; i++, reg_offset--) {
+			reg = sp->base + MTK_NOR_REG_PRGDATA(reg_offset);
+			writeb(((const u8 *)(op->data.buf.out))[i], reg);
+		}
+	}
+
+	for (; reg_offset >= 0; reg_offset--) {
+		reg = sp->base + MTK_NOR_REG_PRGDATA(reg_offset);
+		writeb(0, reg);
+	}
+
+	// trigger op
+	writel(prg_len * BITS_PER_BYTE, sp->base + MTK_NOR_REG_PRG_CNT);
+	ret = mtk_nor_cmd_exec(sp, MTK_NOR_CMD_PROGRAM,
+			       prg_len * BITS_PER_BYTE);
+	if (ret)
+		return ret;
+
+	// fetch read data
+	reg_offset = 0;
+	if (op->data.dir == SPI_MEM_DATA_IN) {
+		for (i = op->data.nbytes - 1; i >= 0; i--, reg_offset++) {
+			reg = sp->base + MTK_NOR_REG_SHIFT(reg_offset);
+			((u8 *)(op->data.buf.in))[i] = readb(reg);
+		}
+	}
+
+	return 0;
+}
+
 static int mtk_nor_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
 {
 	struct mtk_nor *sp = spi_controller_get_devdata(mem->spi->master);
@@ -411,7 +489,7 @@ static int mtk_nor_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
 
 	if ((op->data.nbytes == 0) ||
 	    ((op->addr.nbytes != 3) && (op->addr.nbytes != 4)))
-		return -ENOTSUPP;
+		return mtk_nor_spi_mem_prg(sp, op);
 
 	if (op->data.dir == SPI_MEM_DATA_OUT) {
 		mtk_nor_set_addr(sp, op);
@@ -441,7 +519,7 @@ static int mtk_nor_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
 		}
 	}
 
-	return -ENOTSUPP;
+	return mtk_nor_spi_mem_prg(sp, op);
 }
 
 static int mtk_nor_setup(struct spi_device *spi)
-- 
2.26.2

[PATCH 2/3] spi: spi-mtk-nor: add helper for checking prg mode ops

[Chuanhong Guo]

op checking/resizing logic for the newly added mtk_nor_spi_mem_prg is
more complicated. Add two helper functions for them:
mtk_nor_match_prg: check whether an op is supported by prg mode.
mtk_nor_adj_prg_size: adjust data size for mtk_nor_spi_mem_prg.

mtk_nor_match_prg isn't called yet because supports_op is currently
broken. It'll be used in the next fix commit.

Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
---
 drivers/spi/spi-mtk-nor.c | 76 +++++++++++++++++++++++++++++++++++----
 1 file changed, 69 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-mtk-nor.c b/drivers/spi/spi-mtk-nor.c
index a0087a5e869b9..4bbf38ef5b4b1 100644
--- a/drivers/spi/spi-mtk-nor.c
+++ b/drivers/spi/spi-mtk-nor.c
@@ -168,10 +168,76 @@ static bool mtk_nor_match_read(const struct spi_mem_op *op)
 	return false;
 }
 
-static int mtk_nor_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
+static bool mtk_nor_match_prg(const struct spi_mem_op *op)
 {
-	size_t len;
+	int tx_len, rx_len, prg_len, prg_left;
+
+	// prg mode is spi-only.
+	if ((op->cmd.buswidth > 1) || (op->addr.buswidth > 1) ||
+	    (op->dummy.buswidth > 1) || (op->data.buswidth > 1))
+		return false;
+
+	tx_len = op->cmd.nbytes + op->addr.nbytes;
+
+	if (op->data.dir == SPI_MEM_DATA_OUT) {
+		// count dummy bytes only if we need to write data after it
+		tx_len += op->dummy.nbytes;
+
+		// leave at least one byte for data
+		if (tx_len > MTK_NOR_REG_PRGDATA_MAX)
+			return false;
+
+		// if there's no addr, meaning adjust_op_size is impossible,
+		// check data length as well.
+		if ((!op->addr.nbytes) &&
+		    (tx_len + op->data.nbytes > MTK_NOR_REG_PRGDATA_MAX + 1))
+			return false;
+	} else if (op->data.dir == SPI_MEM_DATA_IN) {
+		if (tx_len > MTK_NOR_REG_PRGDATA_MAX + 1)
+			return false;
+
+		rx_len = op->data.nbytes;
+		prg_left = MTK_NOR_PRG_CNT_MAX / 8 - tx_len - op->dummy.nbytes;
+		if (prg_left > MTK_NOR_REG_SHIFT_MAX + 1)
+			prg_left = MTK_NOR_REG_SHIFT_MAX + 1;
+		if (rx_len > prg_left) {
+			if (!op->addr.nbytes)
+				return false;
+			rx_len = prg_left;
+		}
+
+		prg_len = tx_len + op->dummy.nbytes + rx_len;
+		if (prg_len > MTK_NOR_PRG_CNT_MAX / 8)
+			return false;
+	} else {
+		prg_len = tx_len + op->dummy.nbytes;
+		if (prg_len > MTK_NOR_PRG_CNT_MAX / 8)
+			return false;
+	}
+	return true;
+}
 
+static void mtk_nor_adj_prg_size(struct spi_mem_op *op)
+{
+	int tx_len, tx_left, prg_left;
+
+	tx_len = op->cmd.nbytes + op->addr.nbytes;
+	if (op->data.dir == SPI_MEM_DATA_OUT) {
+		tx_len += op->dummy.nbytes;
+		tx_left = MTK_NOR_REG_PRGDATA_MAX + 1 - tx_len;
+		if (op->data.nbytes > tx_left)
+			op->data.nbytes = tx_left;
+	} else if (op->data.dir == SPI_MEM_DATA_IN) {
+		prg_left = MTK_NOR_PRG_CNT_MAX / 8 - tx_len - op->dummy.nbytes;
+		if (prg_left > MTK_NOR_REG_SHIFT_MAX + 1)
+			prg_left = MTK_NOR_REG_SHIFT_MAX + 1;
+		if (op->data.nbytes > prg_left)
+			op->data.nbytes = prg_left;
+	}
+}
+
+static int mtk_nor_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
+{
 	if (!op->data.nbytes)
 		return 0;
 
@@ -200,11 +266,7 @@ static int mtk_nor_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
 		}
 	}
 
-	len = MTK_NOR_PRG_MAX_SIZE - op->cmd.nbytes - op->addr.nbytes -
-	      op->dummy.nbytes;
-	if (op->data.nbytes > len)
-		op->data.nbytes = len;
-
+	mtk_nor_adj_prg_size(op);
 	return 0;
 }
 
-- 
2.26.2

[PATCH 3/3] spi: spi-mtk-nor: fix op checks in supports_op

[Chuanhong Guo]

commit a59b2c7c56bf7 ("spi: spi-mtk-nor: support standard spi properties")
tries to inverse the logic of supports_op when adding
spi_mem_default_supports_op check, but it didn't get it done properly.
There are two regressions introduced by this commit:
1. reading ops supported by program mode is rejected.
2. all ops with special controller routines are incorrectly further
   checked against program mode.

This commits inverses the logic back:
1. check spi_mem_default_supports_op and reject unsupported ops first.
2. return true for ops with special controller routines.
3. check the left ops against controller program mode.

Fixes: a59b2c7c56bf7 ("spi: spi-mtk-nor: support standard spi properties")
Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
---
 drivers/spi/spi-mtk-nor.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/spi-mtk-nor.c b/drivers/spi/spi-mtk-nor.c
index 4bbf38ef5b4b1..ea39736de2918 100644
--- a/drivers/spi/spi-mtk-nor.c
+++ b/drivers/spi/spi-mtk-nor.c
@@ -273,7 +273,8 @@ static int mtk_nor_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
 static bool mtk_nor_supports_op(struct spi_mem *mem,
 				const struct spi_mem_op *op)
 {
-	size_t len;
+	if (!spi_mem_default_supports_op(mem, op))
+		return false;
 
 	if (op->cmd.buswidth != 1)
 		return false;
@@ -281,25 +282,21 @@ static bool mtk_nor_supports_op(struct spi_mem *mem,
 	if ((op->addr.nbytes == 3) || (op->addr.nbytes == 4)) {
 		switch(op->data.dir) {
 		case SPI_MEM_DATA_IN:
-			if (!mtk_nor_match_read(op))
-				return false;
+			if (mtk_nor_match_read(op))
+				return true;
 			break;
 		case SPI_MEM_DATA_OUT:
-			if ((op->addr.buswidth != 1) ||
-			    (op->dummy.nbytes != 0) ||
-			    (op->data.buswidth != 1))
-				return false;
+			if ((op->addr.buswidth == 1) &&
+			    (op->dummy.nbytes == 0) &&
+			    (op->data.buswidth == 1))
+				return true;
 			break;
 		default:
 			break;
 		}
 	}
-	len = op->cmd.nbytes + op->addr.nbytes + op->dummy.nbytes;
-	if ((len > MTK_NOR_PRG_MAX_SIZE) ||
-	    ((op->data.nbytes) && (len == MTK_NOR_PRG_MAX_SIZE)))
-		return false;
 
-	return spi_mem_default_supports_op(mem, op);
+	return mtk_nor_match_prg(op);
 }
 
 static void mtk_nor_setup_bus(struct mtk_nor *sp, const struct spi_mem_op *op)
-- 
2.26.2

Re: [PATCH 0/3] spi: spi-mtk-nor: make use of full capability of program mode

[Mark Brown]

On Thu, 24 Sep 2020 23:27:27 +0800, Chuanhong Guo wrote:
> "program" mode on this controller can trigger up to 56 bits of data
> shifting. During the operation, data in PRGDATA[0-5] will be
> shifted out from MOSI, and data from MISO will be continuously filling
> SHREG[0-9].
> Currently this mode is used to implement transfer_one_message for 6-byte
> full-duplex transfer, but it can execute a transfer for up-to 7 bytes
> as long as the last byte is read only.
> transfer_one_message is expected to perform full-duplex transfer,
> instead of transfer with specific format. mtk_nor_spi_mem_prg is
> added here to use this extra byte.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/3] spi: spi-mtk-nor: make use of full capability of prg mode
      commit: e7edd2cf4c7d06c6ef3e2030f66eeefa5150f0ff
[2/3] spi: spi-mtk-nor: add helper for checking prg mode ops
      commit: fd806575921ab78c8f0ee7f4dd3d4bb7c16206c8
[3/3] spi: spi-mtk-nor: fix op checks in supports_op
      commit: 81f13f2116cd93910d958c58052ef7dc22f1e577

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark