[PATCH V2 0/6] spi: mediatek: add single/quad mode support

[PATCH V2 0/6] spi: mediatek: add single/quad mode support

[Leilk Liu]

v2:
 1. rebase this series on spi for-next.
 2. fix Rob and Krzysztof comments in v1. 

Leilk Liu (6):
  dt-bindings: spi: Add compatible for Mediatek IPM IP with single mode
  spi: mediatek: add IPM single mode design support
  dt-bindings: spi: Add compatible for Mediatek IPM IP with quad mode
  spi: mediatek: add spi memory support
  dt-bindings: spi: support spi-hclk
  spi: mediatek: support spi-hclk

 .../bindings/spi/mediatek,spi-mt65xx.yaml     |   4 +
 drivers/spi/spi-mt65xx.c                      | 496 ++++++++++++++++--
 2 files changed, 468 insertions(+), 32 deletions(-)

--
2.25.1

[PATCH V2 1/6] dt-bindings: spi: Add compatible for Mediatek IPM IP with single mode

[Leilk Liu]

This patch adds dt-binding documentation for Mediatek SPI IPM IP with
single mode.

Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
---
 Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml b/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
index bfa44acb1bdd..0a2fc0404cb3 100644
--- a/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
+++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
@@ -41,6 +41,7 @@ properties:
               - mediatek,mt8135-spi
               - mediatek,mt8173-spi
               - mediatek,mt8183-spi
+              - mediatek,ipm-spi-single
 
   reg:
     maxItems: 1
-- 
2.25.1

[PATCH V2 2/6] spi: mediatek: add IPM single mode design support

[Leilk Liu]

this patch add the support of IPM single mode design.

Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
---
 drivers/spi/spi-mt65xx.c | 103 +++++++++++++++++++++++++++++++++------
 1 file changed, 87 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
index bbfeb8046c17..5fa677a589a4 100644
--- a/drivers/spi/spi-mt65xx.c
+++ b/drivers/spi/spi-mt65xx.c
@@ -31,6 +31,7 @@
 #define SPI_CFG2_REG                      0x0028
 #define SPI_TX_SRC_REG_64                 0x002c
 #define SPI_RX_DST_REG_64                 0x0030
+#define SPI_CFG3_IPM_REG                  0x0040
 
 #define SPI_CFG0_SCK_HIGH_OFFSET          0
 #define SPI_CFG0_SCK_LOW_OFFSET           8
@@ -48,6 +49,7 @@
 #define SPI_CFG1_CS_IDLE_MASK             0xff
 #define SPI_CFG1_PACKET_LOOP_MASK         0xff00
 #define SPI_CFG1_PACKET_LENGTH_MASK       0x3ff0000
+#define SPI_CFG1_IPM_PACKET_LENGTH_MASK   GENMASK(31, 16)
 #define SPI_CFG2_SCK_HIGH_OFFSET          0
 #define SPI_CFG2_SCK_LOW_OFFSET           16
 
@@ -68,7 +70,13 @@
 #define SPI_CMD_TX_ENDIAN            BIT(15)
 #define SPI_CMD_FINISH_IE            BIT(16)
 #define SPI_CMD_PAUSE_IE             BIT(17)
+#define SPI_CMD_IPM_NONIDLE_MODE     BIT(19)
+#define SPI_CMD_IPM_SPIM_LOOP        BIT(21)
+#define SPI_CMD_IPM_GET_TICKDLY_OFFSET    22
 
+#define SPI_CMD_IPM_GET_TICKDLY_MASK	GENMASK(24, 22)
+#define SPI_CFG3_IPM_HALF_DUPLEX_DIR		BIT(2)
+#define SPI_CFG3_IPM_HALF_DUPLEX_EN		BIT(3)
 #define MT8173_SPI_MAX_PAD_SEL 3
 
 #define MTK_SPI_PAUSE_INT_STATUS 0x2
@@ -78,6 +86,7 @@
 
 #define MTK_SPI_MAX_FIFO_SIZE 32U
 #define MTK_SPI_PACKET_SIZE 1024
+#define MTK_SPI_IPM_PACKET_SIZE SZ_64K
 #define MTK_SPI_32BITS_MASK  (0xffffffff)
 
 #define DMA_ADDR_EXT_BITS (36)
@@ -93,6 +102,9 @@ struct mtk_spi_compatible {
 	bool dma_ext;
 	/* some IC no need unprepare SPI clk */
 	bool no_need_unprepare;
+	/* IPM design improve some single mode features */
+	bool ipm_design;
+
 };
 
 struct mtk_spi {
@@ -116,6 +128,12 @@ static const struct mtk_spi_compatible mt2712_compat = {
 	.must_tx = true,
 };
 
+static const struct mtk_spi_compatible ipm_compat_single = {
+	.enhance_timing = true,
+	.dma_ext = true,
+	.ipm_design = true,
+};
+
 static const struct mtk_spi_compatible mt6765_compat = {
 	.need_pad_sel = true,
 	.must_tx = true,
@@ -157,6 +175,9 @@ static const struct mtk_chip_config mtk_default_chip_info = {
 };
 
 static const struct of_device_id mtk_spi_of_match[] = {
+	{ .compatible = "mediatek,ipm-spi-single",
+		.data = (void *)&ipm_compat_single,
+	},
 	{ .compatible = "mediatek,mt2701-spi",
 		.data = (void *)&mtk_common_compat,
 	},
@@ -275,12 +296,11 @@ static int mtk_spi_set_hw_cs_timing(struct spi_device *spi)
 	return 0;
 }
 
-static int mtk_spi_prepare_message(struct spi_master *master,
-				   struct spi_message *msg)
+static int mtk_spi_hw_init(struct spi_master *master,
+			   struct spi_device *spi)
 {
 	u16 cpha, cpol;
 	u32 reg_val;
-	struct spi_device *spi = msg->spi;
 	struct mtk_chip_config *chip_config = spi->controller_data;
 	struct mtk_spi *mdata = spi_master_get_devdata(master);
 
@@ -288,6 +308,15 @@ static int mtk_spi_prepare_message(struct spi_master *master,
 	cpol = spi->mode & SPI_CPOL ? 1 : 0;
 
 	reg_val = readl(mdata->base + SPI_CMD_REG);
+	if (mdata->dev_comp->ipm_design) {
+		/* SPI transfer without idle time until packet length done */
+		reg_val |= SPI_CMD_IPM_NONIDLE_MODE;
+		if (spi->mode & SPI_LOOP)
+			reg_val |= SPI_CMD_IPM_SPIM_LOOP;
+		else
+			reg_val &= ~SPI_CMD_IPM_SPIM_LOOP;
+	}
+
 	if (cpha)
 		reg_val |= SPI_CMD_CPHA;
 	else
@@ -344,18 +373,33 @@ static int mtk_spi_prepare_message(struct spi_master *master,
 		writel(mdata->pad_sel[spi->chip_select],
 		       mdata->base + SPI_PAD_SEL_REG);
 
-	/* tick delay */
-	reg_val = readl(mdata->base + SPI_CFG1_REG);
-	reg_val &= ~SPI_CFG1_GET_TICK_DLY_MASK;
-	reg_val |= ((chip_config->tick_delay & 0x7)
-		<< SPI_CFG1_GET_TICK_DLY_OFFSET);
-	writel(reg_val, mdata->base + SPI_CFG1_REG);
+	if (mdata->dev_comp->enhance_timing) {
+		if (mdata->dev_comp->ipm_design) {
+			reg_val = readl(mdata->base + SPI_CMD_REG);
+			reg_val &= ~SPI_CMD_IPM_GET_TICKDLY_MASK;
+			reg_val |= ((chip_config->tick_delay & 0x7)
+				   << SPI_CMD_IPM_GET_TICKDLY_OFFSET);
+			writel(reg_val, mdata->base + SPI_CMD_REG);
+		} else {
+			reg_val = readl(mdata->base + SPI_CFG1_REG);
+			reg_val &= ~SPI_CFG1_GET_TICK_DLY_MASK;
+			reg_val |= ((chip_config->tick_delay & 0x7)
+				<< SPI_CFG1_GET_TICK_DLY_OFFSET);
+			writel(reg_val, mdata->base + SPI_CFG1_REG);
+		}
+	}
 
 	/* set hw cs timing */
 	mtk_spi_set_hw_cs_timing(spi);
 	return 0;
 }
 
+static int mtk_spi_prepare_message(struct spi_master *master,
+				   struct spi_message *msg)
+{
+	return mtk_spi_hw_init(master, msg->spi);
+}
+
 static void mtk_spi_set_cs(struct spi_device *spi, bool enable)
 {
 	u32 reg_val;
@@ -377,13 +421,13 @@ static void mtk_spi_set_cs(struct spi_device *spi, bool enable)
 }
 
 static void mtk_spi_prepare_transfer(struct spi_master *master,
-				     struct spi_transfer *xfer)
+				     u32 speed_hz)
 {
 	u32 div, sck_time, reg_val;
 	struct mtk_spi *mdata = spi_master_get_devdata(master);
 
-	if (xfer->speed_hz < mdata->spi_clk_hz / 2)
-		div = DIV_ROUND_UP(mdata->spi_clk_hz, xfer->speed_hz);
+	if (speed_hz < mdata->spi_clk_hz / 2)
+		div = DIV_ROUND_UP(mdata->spi_clk_hz, speed_hz);
 	else
 		div = 1;
 
@@ -414,12 +458,24 @@ static void mtk_spi_setup_packet(struct spi_master *master)
 	u32 packet_size, packet_loop, reg_val;
 	struct mtk_spi *mdata = spi_master_get_devdata(master);
 
-	packet_size = min_t(u32, mdata->xfer_len, MTK_SPI_PACKET_SIZE);
+	if (mdata->dev_comp->ipm_design)
+		packet_size = min_t(u32,
+				    mdata->xfer_len,
+				    MTK_SPI_IPM_PACKET_SIZE);
+	else
+		packet_size = min_t(u32,
+				    mdata->xfer_len,
+				    MTK_SPI_PACKET_SIZE);
+
 	packet_loop = mdata->xfer_len / packet_size;
 
 	reg_val = readl(mdata->base + SPI_CFG1_REG);
-	reg_val &= ~(SPI_CFG1_PACKET_LENGTH_MASK | SPI_CFG1_PACKET_LOOP_MASK);
+	if (mdata->dev_comp->ipm_design)
+		reg_val &= ~SPI_CFG1_IPM_PACKET_LENGTH_MASK;
+	else
+		reg_val &= ~SPI_CFG1_PACKET_LENGTH_MASK;
 	reg_val |= (packet_size - 1) << SPI_CFG1_PACKET_LENGTH_OFFSET;
+	reg_val &= ~SPI_CFG1_PACKET_LOOP_MASK;
 	reg_val |= (packet_loop - 1) << SPI_CFG1_PACKET_LOOP_OFFSET;
 	writel(reg_val, mdata->base + SPI_CFG1_REG);
 }
@@ -514,7 +570,7 @@ static int mtk_spi_fifo_transfer(struct spi_master *master,
 	mdata->cur_transfer = xfer;
 	mdata->xfer_len = min(MTK_SPI_MAX_FIFO_SIZE, xfer->len);
 	mdata->num_xfered = 0;
-	mtk_spi_prepare_transfer(master, xfer);
+	mtk_spi_prepare_transfer(master, xfer->speed_hz);
 	mtk_spi_setup_packet(master);
 
 	if (xfer->tx_buf) {
@@ -547,7 +603,7 @@ static int mtk_spi_dma_transfer(struct spi_master *master,
 	mdata->cur_transfer = xfer;
 	mdata->num_xfered = 0;
 
-	mtk_spi_prepare_transfer(master, xfer);
+	mtk_spi_prepare_transfer(master, xfer->speed_hz);
 
 	cmd = readl(mdata->base + SPI_CMD_REG);
 	if (xfer->tx_buf)
@@ -582,6 +638,19 @@ static int mtk_spi_transfer_one(struct spi_master *master,
 				struct spi_device *spi,
 				struct spi_transfer *xfer)
 {
+	struct mtk_spi *mdata = spi_master_get_devdata(spi->master);
+	u32 reg_val = 0;
+
+	/* prepare xfer direction and duplex mode */
+	if (mdata->dev_comp->ipm_design) {
+		if (!xfer->tx_buf || !xfer->rx_buf) {
+			reg_val |= SPI_CFG3_IPM_HALF_DUPLEX_EN;
+			if (xfer->rx_buf)
+				reg_val |= SPI_CFG3_IPM_HALF_DUPLEX_DIR;
+		}
+		writel(reg_val, mdata->base + SPI_CFG3_IPM_REG);
+	}
+
 	if (master->can_dma(master, spi, xfer))
 		return mtk_spi_dma_transfer(master, spi, xfer);
 	else
@@ -748,6 +817,8 @@ static int mtk_spi_probe(struct platform_device *pdev)
 
 	if (mdata->dev_comp->must_tx)
 		master->flags = SPI_MASTER_MUST_TX;
+	if (mdata->dev_comp->ipm_design)
+		master->mode_bits |= SPI_LOOP;
 
 	if (mdata->dev_comp->need_pad_sel) {
 		mdata->pad_num = of_property_count_u32_elems(
-- 
2.25.1

[PATCH V2 3/6] dt-bindings: spi: Add compatible for Mediatek IPM IP with quad mode

[Leilk Liu]

This patch adds dt-binding documentation for Mediatek SPI IPM IP with
quad mode.

Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
---
 Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml b/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
index 0a2fc0404cb3..241c0f5880d3 100644
--- a/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
+++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
@@ -42,6 +42,7 @@ properties:
               - mediatek,mt8173-spi
               - mediatek,mt8183-spi
               - mediatek,ipm-spi-single
+              - mediatek,ipm-spi-quad
 
   reg:
     maxItems: 1
-- 
2.25.1

[PATCH V2 4/6] spi: mediatek: add spi memory support

[Leilk Liu]

this patch add the support of spi-mem.

Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
---
 drivers/spi/spi-mt65xx.c | 310 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 309 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
index 5fa677a589a4..852fc008329a 100644
--- a/drivers/spi/spi-mt65xx.c
+++ b/drivers/spi/spi-mt65xx.c
@@ -17,6 +17,7 @@
 #include <linux/platform_data/spi-mt65xx.h>
 #include <linux/pm_runtime.h>
 #include <linux/spi/spi.h>
+#include <linux/spi/spi-mem.h>
 #include <linux/dma-mapping.h>
 
 #define SPI_CFG0_REG                      0x0000
@@ -75,8 +76,21 @@
 #define SPI_CMD_IPM_GET_TICKDLY_OFFSET    22
 
 #define SPI_CMD_IPM_GET_TICKDLY_MASK	GENMASK(24, 22)
+
+#define PIN_MODE_CFG(x)	((x) / 2)
+
+#define SPI_CFG3_IPM_PIN_MODE_OFFSET		0
 #define SPI_CFG3_IPM_HALF_DUPLEX_DIR		BIT(2)
 #define SPI_CFG3_IPM_HALF_DUPLEX_EN		BIT(3)
+#define SPI_CFG3_IPM_XMODE_EN			BIT(4)
+#define SPI_CFG3_IPM_NODATA_FLAG		BIT(5)
+#define SPI_CFG3_IPM_CMD_BYTELEN_OFFSET		8
+#define SPI_CFG3_IPM_ADDR_BYTELEN_OFFSET	12
+
+#define SPI_CFG3_IPM_CMD_PIN_MODE_MASK		GENMASK(1, 0)
+#define SPI_CFG3_IPM_CMD_BYTELEN_MASK		GENMASK(11, 8)
+#define SPI_CFG3_IPM_ADDR_BYTELEN_MASK		GENMASK(15, 12)
+
 #define MT8173_SPI_MAX_PAD_SEL 3
 
 #define MTK_SPI_PAUSE_INT_STATUS 0x2
@@ -87,6 +101,8 @@
 #define MTK_SPI_MAX_FIFO_SIZE 32U
 #define MTK_SPI_PACKET_SIZE 1024
 #define MTK_SPI_IPM_PACKET_SIZE SZ_64K
+#define MTK_SPI_IPM_PACKET_LOOP SZ_256
+
 #define MTK_SPI_32BITS_MASK  (0xffffffff)
 
 #define DMA_ADDR_EXT_BITS (36)
@@ -104,7 +120,8 @@ struct mtk_spi_compatible {
 	bool no_need_unprepare;
 	/* IPM design improve some single mode features */
 	bool ipm_design;
-
+	/* IPM design that support quad mode */
+	bool support_quad;
 };
 
 struct mtk_spi {
@@ -120,6 +137,11 @@ struct mtk_spi {
 	u32 tx_sgl_len, rx_sgl_len;
 	const struct mtk_spi_compatible *dev_comp;
 	u32 spi_clk_hz;
+	struct completion spimem_done;
+	bool use_spimem;
+	struct device *dev;
+	dma_addr_t tx_dma;
+	dma_addr_t rx_dma;
 };
 
 static const struct mtk_spi_compatible mtk_common_compat;
@@ -134,6 +156,13 @@ static const struct mtk_spi_compatible ipm_compat_single = {
 	.ipm_design = true,
 };
 
+static const struct mtk_spi_compatible ipm_compat_quad = {
+	.enhance_timing = true,
+	.dma_ext = true,
+	.ipm_design = true,
+	.support_quad = true,
+};
+
 static const struct mtk_spi_compatible mt6765_compat = {
 	.need_pad_sel = true,
 	.must_tx = true,
@@ -178,6 +207,9 @@ static const struct of_device_id mtk_spi_of_match[] = {
 	{ .compatible = "mediatek,ipm-spi-single",
 		.data = (void *)&ipm_compat_single,
 	},
+	{ .compatible = "mediatek,ipm-spi-quad",
+		.data = (void *)&ipm_compat_quad,
+	},
 	{ .compatible = "mediatek,mt2701-spi",
 		.data = (void *)&mtk_common_compat,
 	},
@@ -694,6 +726,13 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
 	else
 		mdata->state = MTK_SPI_IDLE;
 
+	/* SPI-MEM ops */
+	if (mdata->use_spimem) {
+		complete(&mdata->spimem_done);
+
+		return IRQ_HANDLED;
+	}
+
 	if (!master->can_dma(master, NULL, trans)) {
 		if (trans->rx_buf) {
 			cnt = mdata->xfer_len / 4;
@@ -777,6 +816,266 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static int mtk_spi_mem_adjust_op_size(struct spi_mem *mem,
+				      struct spi_mem_op *op)
+{
+	int opcode_len;
+
+	if (op->data.dir != SPI_MEM_NO_DATA) {
+		opcode_len = 1 + op->addr.nbytes + op->dummy.nbytes;
+		if (opcode_len + op->data.nbytes > MTK_SPI_IPM_PACKET_SIZE) {
+			op->data.nbytes = MTK_SPI_IPM_PACKET_SIZE - opcode_len;
+			/* force data buffer dma-aligned. */
+			op->data.nbytes -= op->data.nbytes % 4;
+		}
+	}
+
+	return 0;
+}
+
+static bool mtk_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)
+		return false;
+
+	if (op->addr.nbytes && op->dummy.nbytes &&
+	    op->addr.buswidth != op->dummy.buswidth)
+		return false;
+
+	if (op->addr.nbytes + op->dummy.nbytes > 16)
+		return false;
+
+	if (op->data.nbytes > MTK_SPI_IPM_PACKET_SIZE) {
+		if (op->data.nbytes / MTK_SPI_IPM_PACKET_SIZE >
+		    MTK_SPI_IPM_PACKET_LOOP ||
+		    op->data.nbytes % MTK_SPI_IPM_PACKET_SIZE != 0)
+			return false;
+	}
+
+	return true;
+}
+
+static void mtk_spi_mem_setup_dma_xfer(struct spi_master *master,
+				       const struct spi_mem_op *op)
+{
+	struct mtk_spi *mdata = spi_master_get_devdata(master);
+
+	writel((u32)(mdata->tx_dma & MTK_SPI_32BITS_MASK),
+	       mdata->base + SPI_TX_SRC_REG);
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+	if (mdata->dev_comp->dma_ext)
+		writel((u32)(mdata->tx_dma >> 32),
+		       mdata->base + SPI_TX_SRC_REG_64);
+#endif
+
+	if (op->data.dir == SPI_MEM_DATA_IN) {
+		writel((u32)(mdata->rx_dma & MTK_SPI_32BITS_MASK),
+		       mdata->base + SPI_RX_DST_REG);
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+		if (mdata->dev_comp->dma_ext)
+			writel((u32)(mdata->rx_dma >> 32),
+			       mdata->base + SPI_RX_DST_REG_64);
+#endif
+	}
+}
+
+static int mtk_spi_transfer_wait(struct spi_mem *mem,
+				 const struct spi_mem_op *op)
+{
+	struct mtk_spi *mdata = spi_master_get_devdata(mem->spi->master);
+	unsigned long long ms = 1;
+
+	if (op->data.dir == SPI_MEM_NO_DATA)
+		ms = 8LL * 1000LL * 32;
+	else
+		ms = 8LL * 1000LL * op->data.nbytes;
+	do_div(ms, mem->spi->max_speed_hz);
+	ms += ms + 1000; /* 1s tolerance */
+
+	if (ms > UINT_MAX)
+		ms = UINT_MAX;
+
+	if (!wait_for_completion_timeout(&mdata->spimem_done,
+					 msecs_to_jiffies(ms))) {
+		dev_err(mdata->dev, "spi-mem transfer timeout\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static int mtk_spi_mem_exec_op(struct spi_mem *mem,
+			       const struct spi_mem_op *op)
+{
+	struct mtk_spi *mdata = spi_master_get_devdata(mem->spi->master);
+	u32 reg_val, nio = 1, tx_size;
+	char *tx_tmp_buf, *rx_tmp_buf;
+	int ret = 0;
+
+	mdata->use_spimem = true;
+	reinit_completion(&mdata->spimem_done);
+
+	mtk_spi_reset(mdata);
+	mtk_spi_hw_init(mem->spi->master, mem->spi);
+	mtk_spi_prepare_transfer(mem->spi->master, mem->spi->max_speed_hz);
+
+	reg_val = readl(mdata->base + SPI_CFG3_IPM_REG);
+	/* opcode byte len */
+	reg_val &= ~SPI_CFG3_IPM_CMD_BYTELEN_MASK;
+	reg_val |= 1 << SPI_CFG3_IPM_CMD_BYTELEN_OFFSET;
+
+	/* addr & dummy byte len */
+	reg_val &= ~SPI_CFG3_IPM_ADDR_BYTELEN_MASK;
+	if (op->addr.nbytes || op->dummy.nbytes)
+		reg_val |= (op->addr.nbytes + op->dummy.nbytes) <<
+			    SPI_CFG3_IPM_ADDR_BYTELEN_OFFSET;
+
+	/* data byte len */
+	if (op->data.dir == SPI_MEM_NO_DATA) {
+		reg_val |= SPI_CFG3_IPM_NODATA_FLAG;
+		writel(0, mdata->base + SPI_CFG1_REG);
+	} else {
+		reg_val &= ~SPI_CFG3_IPM_NODATA_FLAG;
+		mdata->xfer_len = op->data.nbytes;
+		mtk_spi_setup_packet(mem->spi->master);
+	}
+
+	if (op->addr.nbytes || op->dummy.nbytes) {
+		if (op->addr.buswidth == 1 || op->dummy.buswidth == 1)
+			reg_val |= SPI_CFG3_IPM_XMODE_EN;
+		else
+			reg_val &= ~SPI_CFG3_IPM_XMODE_EN;
+	}
+
+	if (op->addr.buswidth == 2 ||
+	    op->dummy.buswidth == 2 ||
+	    op->data.buswidth == 2)
+		nio = 2;
+	else if (op->addr.buswidth == 4 ||
+		 op->dummy.buswidth == 4 ||
+		 op->data.buswidth == 4)
+		nio = 4;
+
+	reg_val &= ~SPI_CFG3_IPM_CMD_PIN_MODE_MASK;
+	reg_val |= PIN_MODE_CFG(nio) << SPI_CFG3_IPM_PIN_MODE_OFFSET;
+
+	reg_val |= SPI_CFG3_IPM_HALF_DUPLEX_EN;
+	if (op->data.dir == SPI_MEM_DATA_IN)
+		reg_val |= SPI_CFG3_IPM_HALF_DUPLEX_DIR;
+	else
+		reg_val &= ~SPI_CFG3_IPM_HALF_DUPLEX_DIR;
+	writel(reg_val, mdata->base + SPI_CFG3_IPM_REG);
+
+	tx_size = 1 + op->addr.nbytes + op->dummy.nbytes;
+	if (op->data.dir == SPI_MEM_DATA_OUT)
+		tx_size += op->data.nbytes;
+
+	tx_size = max_t(u32, tx_size, 32);
+
+	tx_tmp_buf = kzalloc(tx_size, GFP_KERNEL | GFP_DMA);
+	if (!tx_tmp_buf)
+		return -ENOMEM;
+
+	tx_tmp_buf[0] = op->cmd.opcode;
+
+	if (op->addr.nbytes) {
+		int i;
+
+		for (i = 0; i < op->addr.nbytes; i++)
+			tx_tmp_buf[i + 1] = op->addr.val >>
+					(8 * (op->addr.nbytes - i - 1));
+	}
+
+	if (op->dummy.nbytes)
+		memset(tx_tmp_buf + op->addr.nbytes + 1,
+		       0xff,
+		       op->dummy.nbytes);
+
+	if (op->data.nbytes && op->data.dir == SPI_MEM_DATA_OUT)
+		memcpy(tx_tmp_buf + op->dummy.nbytes + op->addr.nbytes + 1,
+		       op->data.buf.out,
+		       op->data.nbytes);
+
+	mdata->tx_dma = dma_map_single(mdata->dev, tx_tmp_buf,
+				       tx_size, DMA_TO_DEVICE);
+	if (dma_mapping_error(mdata->dev, mdata->tx_dma)) {
+		ret = -ENOMEM;
+		goto err_exit;
+	}
+
+	if (op->data.dir == SPI_MEM_DATA_IN) {
+		if (!IS_ALIGNED((size_t)op->data.buf.in, 4)) {
+			rx_tmp_buf = kzalloc(op->data.nbytes,
+					     GFP_KERNEL | GFP_DMA);
+			if (!rx_tmp_buf) {
+				ret = -ENOMEM;
+				goto unmap_tx_dma;
+			}
+		} else {
+			rx_tmp_buf = op->data.buf.in;
+		}
+
+		mdata->rx_dma = dma_map_single(mdata->dev,
+					       rx_tmp_buf,
+					       op->data.nbytes,
+					       DMA_FROM_DEVICE);
+		if (dma_mapping_error(mdata->dev, mdata->rx_dma)) {
+			ret = -ENOMEM;
+			goto kfree_rx_tmp_buf;
+		}
+	}
+
+	reg_val = readl(mdata->base + SPI_CMD_REG);
+	reg_val |= SPI_CMD_TX_DMA;
+	if (op->data.dir == SPI_MEM_DATA_IN)
+		reg_val |= SPI_CMD_RX_DMA;
+	writel(reg_val, mdata->base + SPI_CMD_REG);
+
+	mtk_spi_mem_setup_dma_xfer(mem->spi->master, op);
+
+	mtk_spi_enable_transfer(mem->spi->master);
+
+	/* Wait for the interrupt. */
+	ret = mtk_spi_transfer_wait(mem, op);
+	if (ret)
+		goto unmap_rx_dma;
+
+	/* spi disable dma */
+	reg_val = readl(mdata->base + SPI_CMD_REG);
+	reg_val &= ~SPI_CMD_TX_DMA;
+	if (op->data.dir == SPI_MEM_DATA_IN)
+		reg_val &= ~SPI_CMD_RX_DMA;
+	writel(reg_val, mdata->base + SPI_CMD_REG);
+
+unmap_rx_dma:
+	if (op->data.dir == SPI_MEM_DATA_IN) {
+		dma_unmap_single(mdata->dev, mdata->rx_dma,
+				 op->data.nbytes, DMA_FROM_DEVICE);
+		if (!IS_ALIGNED((size_t)op->data.buf.in, 4))
+			memcpy(op->data.buf.in, rx_tmp_buf, op->data.nbytes);
+	}
+kfree_rx_tmp_buf:
+	if (op->data.dir == SPI_MEM_DATA_IN &&
+	    !IS_ALIGNED((size_t)op->data.buf.in, 4))
+		kfree(rx_tmp_buf);
+unmap_tx_dma:
+	dma_unmap_single(mdata->dev, mdata->tx_dma,
+			 tx_size, DMA_TO_DEVICE);
+err_exit:
+	kfree(tx_tmp_buf);
+	mdata->use_spimem = false;
+
+	return ret;
+}
+
+static const struct spi_controller_mem_ops mtk_spi_mem_ops = {
+	.adjust_op_size = mtk_spi_mem_adjust_op_size,
+	.supports_op = mtk_spi_mem_supports_op,
+	.exec_op = mtk_spi_mem_exec_op,
+};
+
 static int mtk_spi_probe(struct platform_device *pdev)
 {
 	struct spi_master *master;
@@ -820,6 +1119,15 @@ static int mtk_spi_probe(struct platform_device *pdev)
 	if (mdata->dev_comp->ipm_design)
 		master->mode_bits |= SPI_LOOP;
 
+	if (mdata->dev_comp->support_quad) {
+		master->mem_ops = &mtk_spi_mem_ops;
+		master->mode_bits |= SPI_RX_DUAL | SPI_TX_DUAL |
+				     SPI_RX_QUAD | SPI_TX_QUAD;
+
+		mdata->dev = &pdev->dev;
+		init_completion(&mdata->spimem_done);
+	}
+
 	if (mdata->dev_comp->need_pad_sel) {
 		mdata->pad_num = of_property_count_u32_elems(
 			pdev->dev.of_node,
-- 
2.25.1

[PATCH V2 5/6] dt-bindings: spi: support spi-hclk

[Leilk Liu]

this patch support spi-hclk.

Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
---
 Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml b/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
index 241c0f5880d3..6920ced5451e 100644
--- a/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
+++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
@@ -55,12 +55,14 @@ properties:
       - description: clock used for the parent clock
       - description: clock used for the muxes clock
       - description: clock used for the clock gate
+      - description: clock used for the AHB bus, this clock is optional
 
   clock-names:
     items:
       - const: parent-clk
       - const: sel-clk
       - const: spi-clk
+      - const: spi-hclk
 
   mediatek,pad-select:
     $ref: /schemas/types.yaml#/definitions/uint32-array
-- 
2.25.1

[PATCH V2 6/6] spi: mediatek: support spi-hclk

[Leilk Liu]

this patch adds spi-hclk support.

Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
---
 drivers/spi/spi-mt65xx.c | 85 ++++++++++++++++++++++++++++++++--------
 1 file changed, 69 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
index 852fc008329a..7d6d8d7dea47 100644
--- a/drivers/spi/spi-mt65xx.c
+++ b/drivers/spi/spi-mt65xx.c
@@ -129,7 +129,7 @@ struct mtk_spi {
 	u32 state;
 	int pad_num;
 	u32 *pad_sel;
-	struct clk *parent_clk, *sel_clk, *spi_clk;
+	struct clk *parent_clk, *sel_clk, *spi_clk, *spi_hclk;
 	struct spi_transfer *cur_transfer;
 	u32 xfer_len;
 	u32 num_xfered;
@@ -1203,25 +1203,38 @@ static int mtk_spi_probe(struct platform_device *pdev)
 		goto err_put_master;
 	}
 
+	mdata->spi_hclk = devm_clk_get(&pdev->dev, "spi-hclk");
+	if (!IS_ERR(mdata->spi_hclk)) {
+		ret = clk_prepare_enable(mdata->spi_hclk);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "failed to enable spi_hclk (%d)\n", ret);
+			goto err_put_master;
+		}
+	}
+
 	ret = clk_prepare_enable(mdata->spi_clk);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to enable spi_clk (%d)\n", ret);
-		goto err_put_master;
+		goto err_disable_spi_hclk;
 	}
 
 	ret = clk_set_parent(mdata->sel_clk, mdata->parent_clk);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to clk_set_parent (%d)\n", ret);
-		clk_disable_unprepare(mdata->spi_clk);
-		goto err_put_master;
+		goto err_disable_spi_clk;
 	}
 
 	mdata->spi_clk_hz = clk_get_rate(mdata->spi_clk);
 
-	if (mdata->dev_comp->no_need_unprepare)
+	if (mdata->dev_comp->no_need_unprepare) {
 		clk_disable(mdata->spi_clk);
-	else
+		if (!IS_ERR(mdata->spi_hclk))
+			clk_disable(mdata->spi_hclk);
+	} else {
 		clk_disable_unprepare(mdata->spi_clk);
+		if (!IS_ERR(mdata->spi_hclk))
+			clk_disable_unprepare(mdata->spi_hclk);
+	}
 
 	pm_runtime_enable(&pdev->dev);
 
@@ -1261,6 +1274,11 @@ static int mtk_spi_probe(struct platform_device *pdev)
 
 err_disable_runtime_pm:
 	pm_runtime_disable(&pdev->dev);
+err_disable_spi_clk:
+	clk_disable_unprepare(mdata->spi_clk);
+err_disable_spi_hclk:
+	if (!IS_ERR(mdata->spi_hclk))
+		clk_disable_unprepare(mdata->spi_hclk);
 err_put_master:
 	spi_master_put(master);
 
@@ -1276,8 +1294,11 @@ static int mtk_spi_remove(struct platform_device *pdev)
 
 	mtk_spi_reset(mdata);
 
-	if (mdata->dev_comp->no_need_unprepare)
+	if (mdata->dev_comp->no_need_unprepare) {
 		clk_unprepare(mdata->spi_clk);
+		if (!IS_ERR(mdata->spi_hclk))
+			clk_unprepare(mdata->spi_hclk);
+	}
 
 	return 0;
 }
@@ -1293,8 +1314,11 @@ static int mtk_spi_suspend(struct device *dev)
 	if (ret)
 		return ret;
 
-	if (!pm_runtime_suspended(dev))
+	if (!pm_runtime_suspended(dev)) {
 		clk_disable_unprepare(mdata->spi_clk);
+		if (!IS_ERR(mdata->spi_hclk))
+			clk_disable_unprepare(mdata->spi_hclk);
+	}
 
 	return ret;
 }
@@ -1311,11 +1335,23 @@ static int mtk_spi_resume(struct device *dev)
 			dev_err(dev, "failed to enable spi_clk (%d)\n", ret);
 			return ret;
 		}
+
+		if (!IS_ERR(mdata->spi_hclk)) {
+			clk_prepare_enable(mdata->spi_hclk);
+			if (ret < 0) {
+				dev_err(dev, "failed to enable spi_hclk (%d)\n", ret);
+				clk_disable_unprepare(mdata->spi_clk);
+				return ret;
+			}
+		}
 	}
 
 	ret = spi_master_resume(master);
-	if (ret < 0)
+	if (ret < 0) {
 		clk_disable_unprepare(mdata->spi_clk);
+		if (!IS_ERR(mdata->spi_hclk))
+			clk_disable_unprepare(mdata->spi_hclk);
+	}
 
 	return ret;
 }
@@ -1327,10 +1363,15 @@ static int mtk_spi_runtime_suspend(struct device *dev)
 	struct spi_master *master = dev_get_drvdata(dev);
 	struct mtk_spi *mdata = spi_master_get_devdata(master);
 
-	if (mdata->dev_comp->no_need_unprepare)
+	if (mdata->dev_comp->no_need_unprepare) {
 		clk_disable(mdata->spi_clk);
-	else
+		if (!IS_ERR(mdata->spi_hclk))
+			clk_disable(mdata->spi_hclk);
+	} else {
 		clk_disable_unprepare(mdata->spi_clk);
+		if (!IS_ERR(mdata->spi_hclk))
+			clk_disable_unprepare(mdata->spi_hclk);
+	}
 
 	return 0;
 }
@@ -1341,13 +1382,25 @@ static int mtk_spi_runtime_resume(struct device *dev)
 	struct mtk_spi *mdata = spi_master_get_devdata(master);
 	int ret;
 
-	if (mdata->dev_comp->no_need_unprepare)
+	if (mdata->dev_comp->no_need_unprepare) {
 		ret = clk_enable(mdata->spi_clk);
-	else
+		if (!IS_ERR(mdata->spi_hclk))
+			clk_enable(mdata->spi_hclk);
+	} else {
 		ret = clk_prepare_enable(mdata->spi_clk);
-	if (ret < 0) {
-		dev_err(dev, "failed to enable spi_clk (%d)\n", ret);
-		return ret;
+		if (ret < 0) {
+			dev_err(dev, "failed to enable spi_clk (%d)\n", ret);
+			return ret;
+		}
+
+		if (!IS_ERR(mdata->spi_hclk)) {
+			ret = clk_prepare_enable(mdata->spi_hclk);
+			if (ret < 0) {
+				dev_err(dev, "failed to enable spi_hclk (%d)\n", ret);
+				clk_disable_unprepare(mdata->spi_clk);
+				return ret;
+			}
+		}
 	}
 
 	return 0;
-- 
2.25.1

Re: [PATCH V2 2/6] spi: mediatek: add IPM single mode design support

[AngeloGioacchino Del Regno]

Il 21/02/22 05:07, Leilk Liu ha scritto:
> this patch add the support of IPM single mode design.
> 
> Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
> ---
>   drivers/spi/spi-mt65xx.c | 103 +++++++++++++++++++++++++++++++++------
>   1 file changed, 87 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
> index bbfeb8046c17..5fa677a589a4 100644
> --- a/drivers/spi/spi-mt65xx.c
> +++ b/drivers/spi/spi-mt65xx.c
> @@ -31,6 +31,7 @@
>   #define SPI_CFG2_REG                      0x0028
>   #define SPI_TX_SRC_REG_64                 0x002c
>   #define SPI_RX_DST_REG_64                 0x0030
> +#define SPI_CFG3_IPM_REG                  0x0040
>   
>   #define SPI_CFG0_SCK_HIGH_OFFSET          0
>   #define SPI_CFG0_SCK_LOW_OFFSET           8
> @@ -48,6 +49,7 @@
>   #define SPI_CFG1_CS_IDLE_MASK             0xff
>   #define SPI_CFG1_PACKET_LOOP_MASK         0xff00
>   #define SPI_CFG1_PACKET_LENGTH_MASK       0x3ff0000
> +#define SPI_CFG1_IPM_PACKET_LENGTH_MASK   GENMASK(31, 16)
>   #define SPI_CFG2_SCK_HIGH_OFFSET          0
>   #define SPI_CFG2_SCK_LOW_OFFSET           16
>   
> @@ -68,7 +70,13 @@
>   #define SPI_CMD_TX_ENDIAN            BIT(15)
>   #define SPI_CMD_FINISH_IE            BIT(16)
>   #define SPI_CMD_PAUSE_IE             BIT(17)
> +#define SPI_CMD_IPM_NONIDLE_MODE     BIT(19)
> +#define SPI_CMD_IPM_SPIM_LOOP        BIT(21)
> +#define SPI_CMD_IPM_GET_TICKDLY_OFFSET    22
>   
> +#define SPI_CMD_IPM_GET_TICKDLY_MASK	GENMASK(24, 22)
> +#define SPI_CFG3_IPM_HALF_DUPLEX_DIR		BIT(2)
> +#define SPI_CFG3_IPM_HALF_DUPLEX_EN		BIT(3)
>   #define MT8173_SPI_MAX_PAD_SEL 3
>   
>   #define MTK_SPI_PAUSE_INT_STATUS 0x2
> @@ -78,6 +86,7 @@
>   
>   #define MTK_SPI_MAX_FIFO_SIZE 32U
>   #define MTK_SPI_PACKET_SIZE 1024
> +#define MTK_SPI_IPM_PACKET_SIZE SZ_64K
>   #define MTK_SPI_32BITS_MASK  (0xffffffff)
>   
>   #define DMA_ADDR_EXT_BITS (36)
> @@ -93,6 +102,9 @@ struct mtk_spi_compatible {
>   	bool dma_ext;
>   	/* some IC no need unprepare SPI clk */
>   	bool no_need_unprepare;
> +	/* IPM design improve some single mode features */
> +	bool ipm_design;
> +
>   };
>   
>   struct mtk_spi {
> @@ -116,6 +128,12 @@ static const struct mtk_spi_compatible mt2712_compat = {
>   	.must_tx = true,
>   };
>   
> +static const struct mtk_spi_compatible ipm_compat_single = {
> +	.enhance_timing = true,
> +	.dma_ext = true,
> +	.ipm_design = true,
> +};
> +
>   static const struct mtk_spi_compatible mt6765_compat = {
>   	.need_pad_sel = true,
>   	.must_tx = true,
> @@ -157,6 +175,9 @@ static const struct mtk_chip_config mtk_default_chip_info = {
>   };
>   
>   static const struct of_device_id mtk_spi_of_match[] = {
> +	{ .compatible = "mediatek,ipm-spi-single",
> +		.data = (void *)&ipm_compat_single,
> +	},
>   	{ .compatible = "mediatek,mt2701-spi",
>   		.data = (void *)&mtk_common_compat,
>   	},
> @@ -275,12 +296,11 @@ static int mtk_spi_set_hw_cs_timing(struct spi_device *spi)
>   	return 0;
>   }
>   
> -static int mtk_spi_prepare_message(struct spi_master *master,
> -				   struct spi_message *msg)
> +static int mtk_spi_hw_init(struct spi_master *master,
> +			   struct spi_device *spi)
>   {
>   	u16 cpha, cpol;
>   	u32 reg_val;
> -	struct spi_device *spi = msg->spi;
>   	struct mtk_chip_config *chip_config = spi->controller_data;
>   	struct mtk_spi *mdata = spi_master_get_devdata(master);
>   
> @@ -288,6 +308,15 @@ static int mtk_spi_prepare_message(struct spi_master *master,
>   	cpol = spi->mode & SPI_CPOL ? 1 : 0;
>   
>   	reg_val = readl(mdata->base + SPI_CMD_REG);
> +	if (mdata->dev_comp->ipm_design) {
> +		/* SPI transfer without idle time until packet length done */
> +		reg_val |= SPI_CMD_IPM_NONIDLE_MODE;
> +		if (spi->mode & SPI_LOOP)
> +			reg_val |= SPI_CMD_IPM_SPIM_LOOP;
> +		else
> +			reg_val &= ~SPI_CMD_IPM_SPIM_LOOP;
> +	}
> +
>   	if (cpha)
>   		reg_val |= SPI_CMD_CPHA;
>   	else
> @@ -344,18 +373,33 @@ static int mtk_spi_prepare_message(struct spi_master *master,
>   		writel(mdata->pad_sel[spi->chip_select],
>   		       mdata->base + SPI_PAD_SEL_REG);
>   
> -	/* tick delay */
> -	reg_val = readl(mdata->base + SPI_CFG1_REG);
> -	reg_val &= ~SPI_CFG1_GET_TICK_DLY_MASK;
> -	reg_val |= ((chip_config->tick_delay & 0x7)
> -		<< SPI_CFG1_GET_TICK_DLY_OFFSET);
> -	writel(reg_val, mdata->base + SPI_CFG1_REG);

Hello Leilk,

with this change, you are excluding this code from MT2712: is that
intentional?
If it is, then this should reside in a different commit with a Fixes
tag, also explaining the reason for not setting the tick delay on
that SoC.

Also, please don't remove the /* tick delay */ comment.

Regards,
Angelo

> +	if (mdata->dev_comp->enhance_timing) {
> +		if (mdata->dev_comp->ipm_design) {
> +			reg_val = readl(mdata->base + SPI_CMD_REG);
> +			reg_val &= ~SPI_CMD_IPM_GET_TICKDLY_MASK;
> +			reg_val |= ((chip_config->tick_delay & 0x7)
> +				   << SPI_CMD_IPM_GET_TICKDLY_OFFSET);
> +			writel(reg_val, mdata->base + SPI_CMD_REG);
> +		} else {
> +			reg_val = readl(mdata->base + SPI_CFG1_REG);
> +			reg_val &= ~SPI_CFG1_GET_TICK_DLY_MASK;
> +			reg_val |= ((chip_config->tick_delay & 0x7)
> +				<< SPI_CFG1_GET_TICK_DLY_OFFSET);
> +			writel(reg_val, mdata->base + SPI_CFG1_REG);
> +		}
> +	}
>   
>   	/* set hw cs timing */
>   	mtk_spi_set_hw_cs_timing(spi);
>   	return 0;
>   }

Re: [PATCH V2 4/6] spi: mediatek: add spi memory support

[AngeloGioacchino Del Regno]

Il 21/02/22 05:07, Leilk Liu ha scritto:
> this patch add the support of spi-mem.
> 
> Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
> ---
>   drivers/spi/spi-mt65xx.c | 310 ++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 309 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
> index 5fa677a589a4..852fc008329a 100644
> --- a/drivers/spi/spi-mt65xx.c
> +++ b/drivers/spi/spi-mt65xx.c
> @@ -17,6 +17,7 @@
>   #include <linux/platform_data/spi-mt65xx.h>
>   #include <linux/pm_runtime.h>
>   #include <linux/spi/spi.h>
> +#include <linux/spi/spi-mem.h>
>   #include <linux/dma-mapping.h>
>   
>   #define SPI_CFG0_REG                      0x0000
> @@ -75,8 +76,21 @@
>   #define SPI_CMD_IPM_GET_TICKDLY_OFFSET    22
>   
>   #define SPI_CMD_IPM_GET_TICKDLY_MASK	GENMASK(24, 22)
> +
> +#define PIN_MODE_CFG(x)	((x) / 2)
> +
> +#define SPI_CFG3_IPM_PIN_MODE_OFFSET		0
>   #define SPI_CFG3_IPM_HALF_DUPLEX_DIR		BIT(2)
>   #define SPI_CFG3_IPM_HALF_DUPLEX_EN		BIT(3)
> +#define SPI_CFG3_IPM_XMODE_EN			BIT(4)
> +#define SPI_CFG3_IPM_NODATA_FLAG		BIT(5)
> +#define SPI_CFG3_IPM_CMD_BYTELEN_OFFSET		8
> +#define SPI_CFG3_IPM_ADDR_BYTELEN_OFFSET	12
> +
> +#define SPI_CFG3_IPM_CMD_PIN_MODE_MASK		GENMASK(1, 0)
> +#define SPI_CFG3_IPM_CMD_BYTELEN_MASK		GENMASK(11, 8)
> +#define SPI_CFG3_IPM_ADDR_BYTELEN_MASK		GENMASK(15, 12)
> +
>   #define MT8173_SPI_MAX_PAD_SEL 3
>   
>   #define MTK_SPI_PAUSE_INT_STATUS 0x2
> @@ -87,6 +101,8 @@
>   #define MTK_SPI_MAX_FIFO_SIZE 32U
>   #define MTK_SPI_PACKET_SIZE 1024
>   #define MTK_SPI_IPM_PACKET_SIZE SZ_64K
> +#define MTK_SPI_IPM_PACKET_LOOP SZ_256
> +
>   #define MTK_SPI_32BITS_MASK  (0xffffffff)
>   
>   #define DMA_ADDR_EXT_BITS (36)
> @@ -104,7 +120,8 @@ struct mtk_spi_compatible {
>   	bool no_need_unprepare;
>   	/* IPM design improve some single mode features */
>   	bool ipm_design;
> -
> +	/* IPM design that support quad mode */
> +	bool support_quad;
>   };
>   
>   struct mtk_spi {
> @@ -120,6 +137,11 @@ struct mtk_spi {
>   	u32 tx_sgl_len, rx_sgl_len;
>   	const struct mtk_spi_compatible *dev_comp;
>   	u32 spi_clk_hz;
> +	struct completion spimem_done;
> +	bool use_spimem;
> +	struct device *dev;
> +	dma_addr_t tx_dma;
> +	dma_addr_t rx_dma;
>   };
>   
>   static const struct mtk_spi_compatible mtk_common_compat;
> @@ -134,6 +156,13 @@ static const struct mtk_spi_compatible ipm_compat_single = {
>   	.ipm_design = true,
>   };
>   
> +static const struct mtk_spi_compatible ipm_compat_quad = {
> +	.enhance_timing = true,
> +	.dma_ext = true,
> +	.ipm_design = true,
> +	.support_quad = true,
> +};
> +
>   static const struct mtk_spi_compatible mt6765_compat = {
>   	.need_pad_sel = true,
>   	.must_tx = true,
> @@ -178,6 +207,9 @@ static const struct of_device_id mtk_spi_of_match[] = {
>   	{ .compatible = "mediatek,ipm-spi-single",
>   		.data = (void *)&ipm_compat_single,
>   	},
> +	{ .compatible = "mediatek,ipm-spi-quad",
> +		.data = (void *)&ipm_compat_quad,
> +	},
>   	{ .compatible = "mediatek,mt2701-spi",
>   		.data = (void *)&mtk_common_compat,
>   	},
> @@ -694,6 +726,13 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
>   	else
>   		mdata->state = MTK_SPI_IDLE;
>   
> +	/* SPI-MEM ops */
> +	if (mdata->use_spimem) {
> +		complete(&mdata->spimem_done);
> +
> +		return IRQ_HANDLED;
> +	}
> +

I would instead make a new ISR for the SPI-MEM case... as you're bypassing the
entire mtk_spi_interrupt() function like that.

Example:

static void mtk_spi_check_and_set_state(struct mtk_spi *mdata)
{
	u32 reg_val;

	reg_val = readl(mdata->base + SPI_STATUS0_REG);

	if (reg_val & MTK_SPI_PAUSE_INT_STATUS)

		mdata->state = MTK_SPI_PAUSED;

	else

		mdata->state = MTK_SPI_IDLE;
}

static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)

{
	u32 cmd, reg_val, cnt, remainder, len;

	struct spi_master *master = dev_id;

	struct mtk_spi *mdata = spi_master_get_devdata(master);

	struct spi_transfer *trans = mdata->cur_transfer;

	mtk_spi_check_and_set_state(mdata);

	if (!master->can_dma(........
	.... blurb...
}

static irqreturn_t mtk_spimem_interrupt(int irq, void *dev_id)
{
	struct spi_master *master = dev_id;

	struct mtk_spi *mdata = spi_master_get_devdata(master);

	mtk_spi_check_and_set_state(mdata);
	complete(&mdata->spimem_done);

	return IRQ_HANDLED;
}

... of course, in mtk_spi_probe(), you would do something like

if (mdata->use_spimem)
	ret = devm_request_irq(&pdev->dev, irq, mtk_spimem_interrupt,
		...... blah ......
else
	ret = devm_request_irq(&pdev->dev, irq, mtk_spi_interrupt,
		..... blah ......

This way, you're separating the two - increasing human readability and showing
the simplifications (in that regard) in the IPM IP's SPI-MEM handling.

>   	if (!master->can_dma(master, NULL, trans)) {
>   		if (trans->rx_buf) {
>   			cnt = mdata->xfer_len / 4;
> @@ -777,6 +816,266 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
>   	return IRQ_HANDLED;
>   }
>   
> +static int mtk_spi_mem_adjust_op_size(struct spi_mem *mem,
> +				      struct spi_mem_op *op)
> +{
> +	int opcode_len;
> +
> +	if (op->data.dir != SPI_MEM_NO_DATA) {
> +		opcode_len = 1 + op->addr.nbytes + op->dummy.nbytes;
> +		if (opcode_len + op->data.nbytes > MTK_SPI_IPM_PACKET_SIZE) {
> +			op->data.nbytes = MTK_SPI_IPM_PACKET_SIZE - opcode_len;
> +			/* force data buffer dma-aligned. */
> +			op->data.nbytes -= op->data.nbytes % 4;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static bool mtk_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)
> +		return false;
> +
> +	if (op->addr.nbytes && op->dummy.nbytes &&
> +	    op->addr.buswidth != op->dummy.buswidth)
> +		return false;
> +
> +	if (op->addr.nbytes + op->dummy.nbytes > 16)
> +		return false;
> +
> +	if (op->data.nbytes > MTK_SPI_IPM_PACKET_SIZE) {
> +		if (op->data.nbytes / MTK_SPI_IPM_PACKET_SIZE >
> +		    MTK_SPI_IPM_PACKET_LOOP ||
> +		    op->data.nbytes % MTK_SPI_IPM_PACKET_SIZE != 0)
> +			return false;
> +	}
> +
> +	return true;
> +}
> +
> +static void mtk_spi_mem_setup_dma_xfer(struct spi_master *master,
> +				       const struct spi_mem_op *op)
> +{
> +	struct mtk_spi *mdata = spi_master_get_devdata(master);
> +
> +	writel((u32)(mdata->tx_dma & MTK_SPI_32BITS_MASK),
> +	       mdata->base + SPI_TX_SRC_REG);
> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> +	if (mdata->dev_comp->dma_ext)
> +		writel((u32)(mdata->tx_dma >> 32),
> +		       mdata->base + SPI_TX_SRC_REG_64);
> +#endif
> +
> +	if (op->data.dir == SPI_MEM_DATA_IN) {
> +		writel((u32)(mdata->rx_dma & MTK_SPI_32BITS_MASK),
> +		       mdata->base + SPI_RX_DST_REG);
> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> +		if (mdata->dev_comp->dma_ext)
> +			writel((u32)(mdata->rx_dma >> 32),
> +			       mdata->base + SPI_RX_DST_REG_64);
> +#endif
> +	}
> +}
> +
> +static int mtk_spi_transfer_wait(struct spi_mem *mem,
> +				 const struct spi_mem_op *op)
> +{
> +	struct mtk_spi *mdata = spi_master_get_devdata(mem->spi->master);
> +	unsigned long long ms = 1;

Initializing ms to 1 here is useless, as you're anyway reinitializing it
right after. I would do it as following:

u64 ms = 8000LL;

if (op->data.dir == SPI_MEM_NO_DATA)
	ms *= 32;
else
	ms *= op->data.nbytes;

Besides, can you please add a comment to the code explaining why the
reason for the waits (why 8, why 1000, why 32)?

> +
> +	if (op->data.dir == SPI_MEM_NO_DATA)
> +		ms = 8LL * 1000LL * 32;
> +	else
> +		ms = 8LL * 1000LL * op->data.nbytes;
> +	do_div(ms, mem->spi->max_speed_hz);

I appreciate the usage of safe division helpers, but this is the wrong one:
you have a unsigned long long (64-bits) dividend and a u32 divisor, so the
right function to use here is div_u64().

	ms = div_u64(ms, mem->spi->max_speed_hz);

> +	ms += ms + 1000; /* 1s tolerance */
> +
> +	if (ms > UINT_MAX)
> +		ms = UINT_MAX;
> +
> +	if (!wait_for_completion_timeout(&mdata->spimem_done,
> +					 msecs_to_jiffies(ms))) {
> +		dev_err(mdata->dev, "spi-mem transfer timeout\n");
> +		return -ETIMEDOUT;
> +	}
> +
> +	return 0;
> +}
> +
> +static int mtk_spi_mem_exec_op(struct spi_mem *mem,
> +			       const struct spi_mem_op *op)
> +{
> +	struct mtk_spi *mdata = spi_master_get_devdata(mem->spi->master);
> +	u32 reg_val, nio = 1, tx_size;
> +	char *tx_tmp_buf, *rx_tmp_buf;
> +	int ret = 0;
> +
> +	mdata->use_spimem = true;
> +	reinit_completion(&mdata->spimem_done);
> +
> +	mtk_spi_reset(mdata);
> +	mtk_spi_hw_init(mem->spi->master, mem->spi);
> +	mtk_spi_prepare_transfer(mem->spi->master, mem->spi->max_speed_hz);
> +
> +	reg_val = readl(mdata->base + SPI_CFG3_IPM_REG);
> +	/* opcode byte len */
> +	reg_val &= ~SPI_CFG3_IPM_CMD_BYTELEN_MASK;
> +	reg_val |= 1 << SPI_CFG3_IPM_CMD_BYTELEN_OFFSET;
> +
> +	/* addr & dummy byte len */
> +	reg_val &= ~SPI_CFG3_IPM_ADDR_BYTELEN_MASK;
> +	if (op->addr.nbytes || op->dummy.nbytes)
> +		reg_val |= (op->addr.nbytes + op->dummy.nbytes) <<
> +			    SPI_CFG3_IPM_ADDR_BYTELEN_OFFSET;
> +
> +	/* data byte len */
> +	if (op->data.dir == SPI_MEM_NO_DATA) {
> +		reg_val |= SPI_CFG3_IPM_NODATA_FLAG;
> +		writel(0, mdata->base + SPI_CFG1_REG);
> +	} else {
> +		reg_val &= ~SPI_CFG3_IPM_NODATA_FLAG;
> +		mdata->xfer_len = op->data.nbytes;
> +		mtk_spi_setup_packet(mem->spi->master);
> +	}
> +
> +	if (op->addr.nbytes || op->dummy.nbytes) {
> +		if (op->addr.buswidth == 1 || op->dummy.buswidth == 1)
> +			reg_val |= SPI_CFG3_IPM_XMODE_EN;
> +		else
> +			reg_val &= ~SPI_CFG3_IPM_XMODE_EN;
> +	}
> +
> +	if (op->addr.buswidth == 2 ||
> +	    op->dummy.buswidth == 2 ||
> +	    op->data.buswidth == 2)
> +		nio = 2;
> +	else if (op->addr.buswidth == 4 ||
> +		 op->dummy.buswidth == 4 ||
> +		 op->data.buswidth == 4)
> +		nio = 4;

	else
		nio = 1;

...so that you can avoid double initialization of the `nio` variable.

> +
> +	reg_val &= ~SPI_CFG3_IPM_CMD_PIN_MODE_MASK;
> +	reg_val |= PIN_MODE_CFG(nio) << SPI_CFG3_IPM_PIN_MODE_OFFSET;
> +

Regards,
Angelo

Re: [PATCH V2 2/6] spi: mediatek: add IPM single mode design support

[Leilk Liu]

On Tue, 2022-02-22 at 11:09 +0100, AngeloGioacchino Del Regno wrote:
> Il 21/02/22 05:07, Leilk Liu ha scritto:
> > this patch add the support of IPM single mode design.
> > 
> > Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
> > ---
> >   drivers/spi/spi-mt65xx.c | 103 +++++++++++++++++++++++++++++++++-
> > -----
> >   1 file changed, 87 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
> > index bbfeb8046c17..5fa677a589a4 100644
> > --- a/drivers/spi/spi-mt65xx.c
> > +++ b/drivers/spi/spi-mt65xx.c
> > @@ -31,6 +31,7 @@
> >   #define SPI_CFG2_REG                      0x0028
> >   #define SPI_TX_SRC_REG_64                 0x002c
> >   #define SPI_RX_DST_REG_64                 0x0030
> > +#define SPI_CFG3_IPM_REG                  0x0040
> >   
> >   #define SPI_CFG0_SCK_HIGH_OFFSET          0
> >   #define SPI_CFG0_SCK_LOW_OFFSET           8
> > @@ -48,6 +49,7 @@
> >   #define SPI_CFG1_CS_IDLE_MASK             0xff
> >   #define SPI_CFG1_PACKET_LOOP_MASK         0xff00
> >   #define SPI_CFG1_PACKET_LENGTH_MASK       0x3ff0000
> > +#define SPI_CFG1_IPM_PACKET_LENGTH_MASK   GENMASK(31, 16)
> >   #define SPI_CFG2_SCK_HIGH_OFFSET          0
> >   #define SPI_CFG2_SCK_LOW_OFFSET           16
> >   
> > @@ -68,7 +70,13 @@
> >   #define SPI_CMD_TX_ENDIAN            BIT(15)
> >   #define SPI_CMD_FINISH_IE            BIT(16)
> >   #define SPI_CMD_PAUSE_IE             BIT(17)
> > +#define SPI_CMD_IPM_NONIDLE_MODE     BIT(19)
> > +#define SPI_CMD_IPM_SPIM_LOOP        BIT(21)
> > +#define SPI_CMD_IPM_GET_TICKDLY_OFFSET    22
> >   
> > +#define SPI_CMD_IPM_GET_TICKDLY_MASK	GENMASK(24, 22)
> > +#define SPI_CFG3_IPM_HALF_DUPLEX_DIR		BIT(2)
> > +#define SPI_CFG3_IPM_HALF_DUPLEX_EN		BIT(3)
> >   #define MT8173_SPI_MAX_PAD_SEL 3
> >   
> >   #define MTK_SPI_PAUSE_INT_STATUS 0x2
> > @@ -78,6 +86,7 @@
> >   
> >   #define MTK_SPI_MAX_FIFO_SIZE 32U
> >   #define MTK_SPI_PACKET_SIZE 1024
> > +#define MTK_SPI_IPM_PACKET_SIZE SZ_64K
> >   #define MTK_SPI_32BITS_MASK  (0xffffffff)
> >   
> >   #define DMA_ADDR_EXT_BITS (36)
> > @@ -93,6 +102,9 @@ struct mtk_spi_compatible {
> >   	bool dma_ext;
> >   	/* some IC no need unprepare SPI clk */
> >   	bool no_need_unprepare;
> > +	/* IPM design improve some single mode features */
> > +	bool ipm_design;
> > +
> >   };
> >   
> >   struct mtk_spi {
> > @@ -116,6 +128,12 @@ static const struct mtk_spi_compatible
> > mt2712_compat = {
> >   	.must_tx = true,
> >   };
> >   
> > +static const struct mtk_spi_compatible ipm_compat_single = {
> > +	.enhance_timing = true,
> > +	.dma_ext = true,
> > +	.ipm_design = true,
> > +};
> > +
> >   static const struct mtk_spi_compatible mt6765_compat = {
> >   	.need_pad_sel = true,
> >   	.must_tx = true,
> > @@ -157,6 +175,9 @@ static const struct mtk_chip_config
> > mtk_default_chip_info = {
> >   };
> >   
> >   static const struct of_device_id mtk_spi_of_match[] = {
> > +	{ .compatible = "mediatek,ipm-spi-single",
> > +		.data = (void *)&ipm_compat_single,
> > +	},
> >   	{ .compatible = "mediatek,mt2701-spi",
> >   		.data = (void *)&mtk_common_compat,
> >   	},
> > @@ -275,12 +296,11 @@ static int mtk_spi_set_hw_cs_timing(struct
> > spi_device *spi)
> >   	return 0;
> >   }
> >   
> > -static int mtk_spi_prepare_message(struct spi_master *master,
> > -				   struct spi_message *msg)
> > +static int mtk_spi_hw_init(struct spi_master *master,
> > +			   struct spi_device *spi)
> >   {
> >   	u16 cpha, cpol;
> >   	u32 reg_val;
> > -	struct spi_device *spi = msg->spi;
> >   	struct mtk_chip_config *chip_config = spi->controller_data;
> >   	struct mtk_spi *mdata = spi_master_get_devdata(master);
> >   
> > @@ -288,6 +308,15 @@ static int mtk_spi_prepare_message(struct
> > spi_master *master,
> >   	cpol = spi->mode & SPI_CPOL ? 1 : 0;
> >   
> >   	reg_val = readl(mdata->base + SPI_CMD_REG);
> > +	if (mdata->dev_comp->ipm_design) {
> > +		/* SPI transfer without idle time until packet length
> > done */
> > +		reg_val |= SPI_CMD_IPM_NONIDLE_MODE;
> > +		if (spi->mode & SPI_LOOP)
> > +			reg_val |= SPI_CMD_IPM_SPIM_LOOP;
> > +		else
> > +			reg_val &= ~SPI_CMD_IPM_SPIM_LOOP;
> > +	}
> > +
> >   	if (cpha)
> >   		reg_val |= SPI_CMD_CPHA;
> >   	else
> > @@ -344,18 +373,33 @@ static int mtk_spi_prepare_message(struct
> > spi_master *master,
> >   		writel(mdata->pad_sel[spi->chip_select],
> >   		       mdata->base + SPI_PAD_SEL_REG);
> >   
> > -	/* tick delay */
> > -	reg_val = readl(mdata->base + SPI_CFG1_REG);
> > -	reg_val &= ~SPI_CFG1_GET_TICK_DLY_MASK;
> > -	reg_val |= ((chip_config->tick_delay & 0x7)
> > -		<< SPI_CFG1_GET_TICK_DLY_OFFSET);
> > -	writel(reg_val, mdata->base + SPI_CFG1_REG);
> 
> Hello Leilk,
> 
> with this change, you are excluding this code from MT2712: is that
> intentional?
> If it is, then this should reside in a different commit with a Fixes
> tag, also explaining the reason for not setting the tick delay on
> that SoC.
It's different for MT2712 and the other ICs for tick delay. I'll send a
patch with fixes tag, thanks

> 
> Also, please don't remove the /* tick delay */ comment.
> 
OK, I'll fix it,thanks

> Regards,
> Angelo
> 
> > +	if (mdata->dev_comp->enhance_timing) {
> > +		if (mdata->dev_comp->ipm_design) {
> > +			reg_val = readl(mdata->base + SPI_CMD_REG);
> > +			reg_val &= ~SPI_CMD_IPM_GET_TICKDLY_MASK;
> > +			reg_val |= ((chip_config->tick_delay & 0x7)
> > +				   << SPI_CMD_IPM_GET_TICKDLY_OFFSET);
> > +			writel(reg_val, mdata->base + SPI_CMD_REG);
> > +		} else {
> > +			reg_val = readl(mdata->base + SPI_CFG1_REG);
> > +			reg_val &= ~SPI_CFG1_GET_TICK_DLY_MASK;
> > +			reg_val |= ((chip_config->tick_delay & 0x7)
> > +				<< SPI_CFG1_GET_TICK_DLY_OFFSET);
> > +			writel(reg_val, mdata->base + SPI_CFG1_REG);
> > +		}
> > +	}
> >   
> >   	/* set hw cs timing */
> >   	mtk_spi_set_hw_cs_timing(spi);
> >   	return 0;
> >   }
> 
> 

Re: [PATCH V2 4/6] spi: mediatek: add spi memory support

[Leilk Liu]

On Tue, 2022-02-22 at 11:49 +0100, AngeloGioacchino Del Regno wrote:
> Il 21/02/22 05:07, Leilk Liu ha scritto:
> > this patch add the support of spi-mem.
> > 
> > Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
> > ---
> >   drivers/spi/spi-mt65xx.c | 310
> > ++++++++++++++++++++++++++++++++++++++-
> >   1 file changed, 309 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
> > index 5fa677a589a4..852fc008329a 100644
> > --- a/drivers/spi/spi-mt65xx.c
> > +++ b/drivers/spi/spi-mt65xx.c
> > @@ -17,6 +17,7 @@
> >   #include <linux/platform_data/spi-mt65xx.h>
> >   #include <linux/pm_runtime.h>
> >   #include <linux/spi/spi.h>
> > +#include <linux/spi/spi-mem.h>
> >   #include <linux/dma-mapping.h>
> >   
> >   #define SPI_CFG0_REG                      0x0000
> > @@ -75,8 +76,21 @@
> >   #define SPI_CMD_IPM_GET_TICKDLY_OFFSET    22
> >   
> >   #define SPI_CMD_IPM_GET_TICKDLY_MASK	GENMASK(24, 22)
> > +
> > +#define PIN_MODE_CFG(x)	((x) / 2)
> > +
> > +#define SPI_CFG3_IPM_PIN_MODE_OFFSET		0
> >   #define SPI_CFG3_IPM_HALF_DUPLEX_DIR		BIT(2)
> >   #define SPI_CFG3_IPM_HALF_DUPLEX_EN		BIT(3)
> > +#define SPI_CFG3_IPM_XMODE_EN			BIT(4)
> > +#define SPI_CFG3_IPM_NODATA_FLAG		BIT(5)
> > +#define SPI_CFG3_IPM_CMD_BYTELEN_OFFSET		8
> > +#define SPI_CFG3_IPM_ADDR_BYTELEN_OFFSET	12
> > +
> > +#define SPI_CFG3_IPM_CMD_PIN_MODE_MASK		GENMASK(1, 0)
> > +#define SPI_CFG3_IPM_CMD_BYTELEN_MASK		GENMASK(11, 8)
> > +#define SPI_CFG3_IPM_ADDR_BYTELEN_MASK		GENMASK(15, 12)
> > +
> >   #define MT8173_SPI_MAX_PAD_SEL 3
> >   
> >   #define MTK_SPI_PAUSE_INT_STATUS 0x2
> > @@ -87,6 +101,8 @@
> >   #define MTK_SPI_MAX_FIFO_SIZE 32U
> >   #define MTK_SPI_PACKET_SIZE 1024
> >   #define MTK_SPI_IPM_PACKET_SIZE SZ_64K
> > +#define MTK_SPI_IPM_PACKET_LOOP SZ_256
> > +
> >   #define MTK_SPI_32BITS_MASK  (0xffffffff)
> >   
> >   #define DMA_ADDR_EXT_BITS (36)
> > @@ -104,7 +120,8 @@ struct mtk_spi_compatible {
> >   	bool no_need_unprepare;
> >   	/* IPM design improve some single mode features */
> >   	bool ipm_design;
> > -
> > +	/* IPM design that support quad mode */
> > +	bool support_quad;
> >   };
> >   
> >   struct mtk_spi {
> > @@ -120,6 +137,11 @@ struct mtk_spi {
> >   	u32 tx_sgl_len, rx_sgl_len;
> >   	const struct mtk_spi_compatible *dev_comp;
> >   	u32 spi_clk_hz;
> > +	struct completion spimem_done;
> > +	bool use_spimem;
> > +	struct device *dev;
> > +	dma_addr_t tx_dma;
> > +	dma_addr_t rx_dma;
> >   };
> >   
> >   static const struct mtk_spi_compatible mtk_common_compat;
> > @@ -134,6 +156,13 @@ static const struct mtk_spi_compatible
> > ipm_compat_single = {
> >   	.ipm_design = true,
> >   };
> >   
> > +static const struct mtk_spi_compatible ipm_compat_quad = {
> > +	.enhance_timing = true,
> > +	.dma_ext = true,
> > +	.ipm_design = true,
> > +	.support_quad = true,
> > +};
> > +
> >   static const struct mtk_spi_compatible mt6765_compat = {
> >   	.need_pad_sel = true,
> >   	.must_tx = true,
> > @@ -178,6 +207,9 @@ static const struct of_device_id
> > mtk_spi_of_match[] = {
> >   	{ .compatible = "mediatek,ipm-spi-single",
> >   		.data = (void *)&ipm_compat_single,
> >   	},
> > +	{ .compatible = "mediatek,ipm-spi-quad",
> > +		.data = (void *)&ipm_compat_quad,
> > +	},
> >   	{ .compatible = "mediatek,mt2701-spi",
> >   		.data = (void *)&mtk_common_compat,
> >   	},
> > @@ -694,6 +726,13 @@ static irqreturn_t mtk_spi_interrupt(int irq,
> > void *dev_id)
> >   	else
> >   		mdata->state = MTK_SPI_IDLE;
> >   
> > +	/* SPI-MEM ops */
> > +	if (mdata->use_spimem) {
> > +		complete(&mdata->spimem_done);
> > +
> > +		return IRQ_HANDLED;
> > +	}
> > +
> 
> I would instead make a new ISR for the SPI-MEM case... as you're
> bypassing the
> entire mtk_spi_interrupt() function like that.
> 
> Example:
> 
> static void mtk_spi_check_and_set_state(struct mtk_spi *mdata)
> {
> 	u32 reg_val;
> 
> 	reg_val = readl(mdata->base + SPI_STATUS0_REG);
> 
> 	if (reg_val & MTK_SPI_PAUSE_INT_STATUS)
> 
> 		mdata->state = MTK_SPI_PAUSED;
> 
> 	else
> 
> 		mdata->state = MTK_SPI_IDLE;
> }
> 
> static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
> 
> {
> 	u32 cmd, reg_val, cnt, remainder, len;
> 
> 	struct spi_master *master = dev_id;
> 
> 	struct mtk_spi *mdata = spi_master_get_devdata(master);
> 
> 	struct spi_transfer *trans = mdata->cur_transfer;
> 
> 	mtk_spi_check_and_set_state(mdata);
> 
> 	if (!master->can_dma(........
> 	.... blurb...
> }
> 
> static irqreturn_t mtk_spimem_interrupt(int irq, void *dev_id)
> {
> 	struct spi_master *master = dev_id;
> 
> 	struct mtk_spi *mdata = spi_master_get_devdata(master);
> 
> 	mtk_spi_check_and_set_state(mdata);
> 	complete(&mdata->spimem_done);
> 
> 	return IRQ_HANDLED;
> }
> 
> ... of course, in mtk_spi_probe(), you would do something like
> 
> if (mdata->use_spimem)
> 	ret = devm_request_irq(&pdev->dev, irq, mtk_spimem_interrupt,
> 		...... blah ......
> else
> 	ret = devm_request_irq(&pdev->dev, irq, mtk_spi_interrupt,
> 		..... blah ......
> 
> This way, you're separating the two - increasing human readability
> and showing
> the simplifications (in that regard) in the IPM IP's SPI-MEM
> handling.
> 
thanks for your advise, I'll do it in the next version.

> >   	if (!master->can_dma(master, NULL, trans)) {
> >   		if (trans->rx_buf) {
> >   			cnt = mdata->xfer_len / 4;
> > @@ -777,6 +816,266 @@ static irqreturn_t mtk_spi_interrupt(int irq,
> > void *dev_id)
> >   	return IRQ_HANDLED;
> >   }
> >   
> > +static int mtk_spi_mem_adjust_op_size(struct spi_mem *mem,
> > +				      struct spi_mem_op *op)
> > +{
> > +	int opcode_len;
> > +
> > +	if (op->data.dir != SPI_MEM_NO_DATA) {
> > +		opcode_len = 1 + op->addr.nbytes + op->dummy.nbytes;
> > +		if (opcode_len + op->data.nbytes >
> > MTK_SPI_IPM_PACKET_SIZE) {
> > +			op->data.nbytes = MTK_SPI_IPM_PACKET_SIZE -
> > opcode_len;
> > +			/* force data buffer dma-aligned. */
> > +			op->data.nbytes -= op->data.nbytes % 4;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static bool mtk_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)
> > +		return false;
> > +
> > +	if (op->addr.nbytes && op->dummy.nbytes &&
> > +	    op->addr.buswidth != op->dummy.buswidth)
> > +		return false;
> > +
> > +	if (op->addr.nbytes + op->dummy.nbytes > 16)
> > +		return false;
> > +
> > +	if (op->data.nbytes > MTK_SPI_IPM_PACKET_SIZE) {
> > +		if (op->data.nbytes / MTK_SPI_IPM_PACKET_SIZE >
> > +		    MTK_SPI_IPM_PACKET_LOOP ||
> > +		    op->data.nbytes % MTK_SPI_IPM_PACKET_SIZE != 0)
> > +			return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > +static void mtk_spi_mem_setup_dma_xfer(struct spi_master *master,
> > +				       const struct spi_mem_op *op)
> > +{
> > +	struct mtk_spi *mdata = spi_master_get_devdata(master);
> > +
> > +	writel((u32)(mdata->tx_dma & MTK_SPI_32BITS_MASK),
> > +	       mdata->base + SPI_TX_SRC_REG);
> > +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> > +	if (mdata->dev_comp->dma_ext)
> > +		writel((u32)(mdata->tx_dma >> 32),
> > +		       mdata->base + SPI_TX_SRC_REG_64);
> > +#endif
> > +
> > +	if (op->data.dir == SPI_MEM_DATA_IN) {
> > +		writel((u32)(mdata->rx_dma & MTK_SPI_32BITS_MASK),
> > +		       mdata->base + SPI_RX_DST_REG);
> > +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> > +		if (mdata->dev_comp->dma_ext)
> > +			writel((u32)(mdata->rx_dma >> 32),
> > +			       mdata->base + SPI_RX_DST_REG_64);
> > +#endif
> > +	}
> > +}
> > +
> > +static int mtk_spi_transfer_wait(struct spi_mem *mem,
> > +				 const struct spi_mem_op *op)
> > +{
> > +	struct mtk_spi *mdata = spi_master_get_devdata(mem->spi-
> > >master);
> > +	unsigned long long ms = 1;
> 
> Initializing ms to 1 here is useless, as you're anyway reinitializing
> it
> right after. I would do it as following:
> 
> u64 ms = 8000LL;
> 
> if (op->data.dir == SPI_MEM_NO_DATA)
> 	ms *= 32;
> else
> 	ms *= op->data.nbytes;
> 
OK,I'll fix it.

> Besides, can you please add a comment to the code explaining why the
> reason for the waits (why 8, why 1000, why 32)?
> 
OK,THANKS

> > +
> > +	if (op->data.dir == SPI_MEM_NO_DATA)
> > +		ms = 8LL * 1000LL * 32;
> > +	else
> > +		ms = 8LL * 1000LL * op->data.nbytes;
> > +	do_div(ms, mem->spi->max_speed_hz);
> 
> I appreciate the usage of safe division helpers, but this is the
> wrong one:
> you have a unsigned long long (64-bits) dividend and a u32 divisor,
> so the
> right function to use here is div_u64().
> 
OK, I'll fix it. thanks!

> 	ms = div_u64(ms, mem->spi->max_speed_hz);
> 
> > +	ms += ms + 1000; /* 1s tolerance */
> > +
> > +	if (ms > UINT_MAX)
> > +		ms = UINT_MAX;
> > +
> > +	if (!wait_for_completion_timeout(&mdata->spimem_done,
> > +					 msecs_to_jiffies(ms))) {
> > +		dev_err(mdata->dev, "spi-mem transfer timeout\n");
> > +		return -ETIMEDOUT;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int mtk_spi_mem_exec_op(struct spi_mem *mem,
> > +			       const struct spi_mem_op *op)
> > +{
> > +	struct mtk_spi *mdata = spi_master_get_devdata(mem->spi-
> > >master);
> > +	u32 reg_val, nio = 1, tx_size;
> > +	char *tx_tmp_buf, *rx_tmp_buf;
> > +	int ret = 0;
> > +
> > +	mdata->use_spimem = true;
> > +	reinit_completion(&mdata->spimem_done);
> > +
> > +	mtk_spi_reset(mdata);
> > +	mtk_spi_hw_init(mem->spi->master, mem->spi);
> > +	mtk_spi_prepare_transfer(mem->spi->master, mem->spi-
> > >max_speed_hz);
> > +
> > +	reg_val = readl(mdata->base + SPI_CFG3_IPM_REG);
> > +	/* opcode byte len */
> > +	reg_val &= ~SPI_CFG3_IPM_CMD_BYTELEN_MASK;
> > +	reg_val |= 1 << SPI_CFG3_IPM_CMD_BYTELEN_OFFSET;
> > +
> > +	/* addr & dummy byte len */
> > +	reg_val &= ~SPI_CFG3_IPM_ADDR_BYTELEN_MASK;
> > +	if (op->addr.nbytes || op->dummy.nbytes)
> > +		reg_val |= (op->addr.nbytes + op->dummy.nbytes) <<
> > +			    SPI_CFG3_IPM_ADDR_BYTELEN_OFFSET;
> > +
> > +	/* data byte len */
> > +	if (op->data.dir == SPI_MEM_NO_DATA) {
> > +		reg_val |= SPI_CFG3_IPM_NODATA_FLAG;
> > +		writel(0, mdata->base + SPI_CFG1_REG);
> > +	} else {
> > +		reg_val &= ~SPI_CFG3_IPM_NODATA_FLAG;
> > +		mdata->xfer_len = op->data.nbytes;
> > +		mtk_spi_setup_packet(mem->spi->master);
> > +	}
> > +
> > +	if (op->addr.nbytes || op->dummy.nbytes) {
> > +		if (op->addr.buswidth == 1 || op->dummy.buswidth == 1)
> > +			reg_val |= SPI_CFG3_IPM_XMODE_EN;
> > +		else
> > +			reg_val &= ~SPI_CFG3_IPM_XMODE_EN;
> > +	}
> > +
> > +	if (op->addr.buswidth == 2 ||
> > +	    op->dummy.buswidth == 2 ||
> > +	    op->data.buswidth == 2)
> > +		nio = 2;
> > +	else if (op->addr.buswidth == 4 ||
> > +		 op->dummy.buswidth == 4 ||
> > +		 op->data.buswidth == 4)
> > +		nio = 4;
> 
> 	else
> 		nio = 1;
> 
> ...so that you can avoid double initialization of the `nio` variable.
> 
OK, I'll fix it,thanks

> > +
> > +	reg_val &= ~SPI_CFG3_IPM_CMD_PIN_MODE_MASK;
> > +	reg_val |= PIN_MODE_CFG(nio) << SPI_CFG3_IPM_PIN_MODE_OFFSET;
> > +
> 
> Regards,
> Angelo

Re: [PATCH V2 1/6] dt-bindings: spi: Add compatible for Mediatek IPM IP with single mode

[Rob Herring]

On Mon, 21 Feb 2022 12:07:12 +0800, Leilk Liu wrote:
> This patch adds dt-binding documentation for Mediatek SPI IPM IP with
> single mode.
> 
> Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
> ---
>  Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH V2 3/6] dt-bindings: spi: Add compatible for Mediatek IPM IP with quad mode

[Rob Herring]

On Mon, Feb 21, 2022 at 12:07:14PM +0800, Leilk Liu wrote:
> This patch adds dt-binding documentation for Mediatek SPI IPM IP with
> quad mode.
> 
> Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
> ---
>  Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml b/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
> index 0a2fc0404cb3..241c0f5880d3 100644
> --- a/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
> +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
> @@ -42,6 +42,7 @@ properties:
>                - mediatek,mt8173-spi
>                - mediatek,mt8183-spi
>                - mediatek,ipm-spi-single
> +              - mediatek,ipm-spi-quad

Can't you use the existing width properties to distinguish? Or is the 
programming not a superset?

Is 'ipm' an SoC? If not, these need to be SoC specific.

Rob

Re: [PATCH V2 5/6] dt-bindings: spi: support spi-hclk

[Rob Herring]

On Mon, Feb 21, 2022 at 12:07:16PM +0800, Leilk Liu wrote:
> this patch support spi-hclk.
> 
> Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
> ---
>  Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml b/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
> index 241c0f5880d3..6920ced5451e 100644
> --- a/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
> +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
> @@ -55,12 +55,14 @@ properties:
>        - description: clock used for the parent clock
>        - description: clock used for the muxes clock
>        - description: clock used for the clock gate
> +      - description: clock used for the AHB bus, this clock is optional

Not optional unless you have minItems.

>  
>    clock-names:

       minItems: 3


>      items:
>        - const: parent-clk
>        - const: sel-clk
>        - const: spi-clk
> +      - const: spi-hclk
>  
>    mediatek,pad-select:
>      $ref: /schemas/types.yaml#/definitions/uint32-array
> -- 
> 2.25.1
> 
> 

Re: [PATCH V2 5/6] dt-bindings: spi: support spi-hclk

[Leilk Liu]

On Fri, 2022-02-25 at 13:03 -0600, Rob Herring wrote:
> On Mon, Feb 21, 2022 at 12:07:16PM +0800, Leilk Liu wrote:
> > this patch support spi-hclk.
> > 
> > Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
> > ---
> >  Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml | 2
> > ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/spi/mediatek,spi-
> > mt65xx.yaml b/Documentation/devicetree/bindings/spi/mediatek,spi-
> > mt65xx.yaml
> > index 241c0f5880d3..6920ced5451e 100644
> > --- a/Documentation/devicetree/bindings/spi/mediatek,spi-
> > mt65xx.yaml
> > +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-
> > mt65xx.yaml
> > @@ -55,12 +55,14 @@ properties:
> >        - description: clock used for the parent clock
> >        - description: clock used for the muxes clock
> >        - description: clock used for the clock gate
> > +      - description: clock used for the AHB bus, this clock is
> > optional
> 
> Not optional unless you have minItems.
> 
OK, I'll add minItems,thanks for your comment.

> >  
> >    clock-names:
> 
>        minItems: 3
> 
Got it, thanks

> 
> >      items:
> >        - const: parent-clk
> >        - const: sel-clk
> >        - const: spi-clk
> > +      - const: spi-hclk
> >  
> >    mediatek,pad-select:
> >      $ref: /schemas/types.yaml#/definitions/uint32-array
> > -- 
> > 2.25.1
> > 
> > 

Re: [PATCH V2 3/6] dt-bindings: spi: Add compatible for Mediatek IPM IP with quad mode

[Leilk Liu]

On Fri, 2022-02-25 at 13:01 -0600, Rob Herring wrote:
> On Mon, Feb 21, 2022 at 12:07:14PM +0800, Leilk Liu wrote:
> > This patch adds dt-binding documentation for Mediatek SPI IPM IP
> > with
> > quad mode.
> > 
> > Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
> > ---
> >  Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml | 1
> > +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/spi/mediatek,spi-
> > mt65xx.yaml b/Documentation/devicetree/bindings/spi/mediatek,spi-
> > mt65xx.yaml
> > index 0a2fc0404cb3..241c0f5880d3 100644
> > --- a/Documentation/devicetree/bindings/spi/mediatek,spi-
> > mt65xx.yaml
> > +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-
> > mt65xx.yaml
> > @@ -42,6 +42,7 @@ properties:
> >                - mediatek,mt8173-spi
> >                - mediatek,mt8183-spi
> >                - mediatek,ipm-spi-single
> > +              - mediatek,ipm-spi-quad
> 
> Can't you use the existing width properties to distinguish? Or is
> the 
> programming not a superset?
> 
"mediatek,ipm-spi-quad" include the feature of "mediatek,ipm-spi-
single".

> Is 'ipm' an SoC? If not, these need to be SoC specific.
> 
IPM is an version of SPI ip, we use this IP with ICs now.
So I define a compatible strings "mediatek,ipm-spi-xxx", then I can
reuse this strings with many ICs.
Currently, we plan to upstream MT7986 IC, this IC has two spi
controller ports, one support single mode only, the other support
single & quad mode both.
for example:
spi0 {
  ...
  compatible = "mediatek,ipm-spi-single";
  ...
}

spi1 {
  ...
  compatible = "mediatek,ipm-spi-quad";
  ...
}


> Rob

Re: [PATCH V2 4/6] spi: mediatek: add spi memory support

[Leilk Liu]

On Wed, 2022-02-23 at 09:59 +0800, Leilk Liu wrote:
> On Tue, 2022-02-22 at 11:49 +0100, AngeloGioacchino Del Regno wrote:
> > Il 21/02/22 05:07, Leilk Liu ha scritto:
> > > this patch add the support of spi-mem.
> > > 
> > > Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
> > > ---
> > >   drivers/spi/spi-mt65xx.c | 310
> > > ++++++++++++++++++++++++++++++++++++++-
> > >   1 file changed, 309 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
> > > index 5fa677a589a4..852fc008329a 100644
> > > --- a/drivers/spi/spi-mt65xx.c
> > > +++ b/drivers/spi/spi-mt65xx.c
> > > @@ -17,6 +17,7 @@
> > >   #include <linux/platform_data/spi-mt65xx.h>
> > >   #include <linux/pm_runtime.h>
> > >   #include <linux/spi/spi.h>
> > > +#include <linux/spi/spi-mem.h>
> > >   #include <linux/dma-mapping.h>
> > >   
> > >   #define SPI_CFG0_REG                      0x0000
> > > @@ -75,8 +76,21 @@
> > >   #define SPI_CMD_IPM_GET_TICKDLY_OFFSET    22
> > >   
> > >   #define SPI_CMD_IPM_GET_TICKDLY_MASK	GENMASK(24, 22)
> > > +
> > > +#define PIN_MODE_CFG(x)	((x) / 2)
> > > +
> > > +#define SPI_CFG3_IPM_PIN_MODE_OFFSET		0
> > >   #define SPI_CFG3_IPM_HALF_DUPLEX_DIR		BIT(2)
> > >   #define SPI_CFG3_IPM_HALF_DUPLEX_EN		BIT(3)
> > > +#define SPI_CFG3_IPM_XMODE_EN			BIT(4)
> > > +#define SPI_CFG3_IPM_NODATA_FLAG		BIT(5)
> > > +#define SPI_CFG3_IPM_CMD_BYTELEN_OFFSET		8
> > > +#define SPI_CFG3_IPM_ADDR_BYTELEN_OFFSET	12
> > > +
> > > +#define SPI_CFG3_IPM_CMD_PIN_MODE_MASK		GENMASK(1, 0)
> > > +#define SPI_CFG3_IPM_CMD_BYTELEN_MASK		GENMASK(11, 8)
> > > +#define SPI_CFG3_IPM_ADDR_BYTELEN_MASK		GENMASK(15, 12)
> > > +
> > >   #define MT8173_SPI_MAX_PAD_SEL 3
> > >   
> > >   #define MTK_SPI_PAUSE_INT_STATUS 0x2
> > > @@ -87,6 +101,8 @@
> > >   #define MTK_SPI_MAX_FIFO_SIZE 32U
> > >   #define MTK_SPI_PACKET_SIZE 1024
> > >   #define MTK_SPI_IPM_PACKET_SIZE SZ_64K
> > > +#define MTK_SPI_IPM_PACKET_LOOP SZ_256
> > > +
> > >   #define MTK_SPI_32BITS_MASK  (0xffffffff)
> > >   
> > >   #define DMA_ADDR_EXT_BITS (36)
> > > @@ -104,7 +120,8 @@ struct mtk_spi_compatible {
> > >   	bool no_need_unprepare;
> > >   	/* IPM design improve some single mode features */
> > >   	bool ipm_design;
> > > -
> > > +	/* IPM design that support quad mode */
> > > +	bool support_quad;
> > >   };
> > >   
> > >   struct mtk_spi {
> > > @@ -120,6 +137,11 @@ struct mtk_spi {
> > >   	u32 tx_sgl_len, rx_sgl_len;
> > >   	const struct mtk_spi_compatible *dev_comp;
> > >   	u32 spi_clk_hz;
> > > +	struct completion spimem_done;
> > > +	bool use_spimem;
> > > +	struct device *dev;
> > > +	dma_addr_t tx_dma;
> > > +	dma_addr_t rx_dma;
> > >   };
> > >   
> > >   static const struct mtk_spi_compatible mtk_common_compat;
> > > @@ -134,6 +156,13 @@ static const struct mtk_spi_compatible
> > > ipm_compat_single = {
> > >   	.ipm_design = true,
> > >   };
> > >   
> > > +static const struct mtk_spi_compatible ipm_compat_quad = {
> > > +	.enhance_timing = true,
> > > +	.dma_ext = true,
> > > +	.ipm_design = true,
> > > +	.support_quad = true,
> > > +};
> > > +
> > >   static const struct mtk_spi_compatible mt6765_compat = {
> > >   	.need_pad_sel = true,
> > >   	.must_tx = true,
> > > @@ -178,6 +207,9 @@ static const struct of_device_id
> > > mtk_spi_of_match[] = {
> > >   	{ .compatible = "mediatek,ipm-spi-single",
> > >   		.data = (void *)&ipm_compat_single,
> > >   	},
> > > +	{ .compatible = "mediatek,ipm-spi-quad",
> > > +		.data = (void *)&ipm_compat_quad,
> > > +	},
> > >   	{ .compatible = "mediatek,mt2701-spi",
> > >   		.data = (void *)&mtk_common_compat,
> > >   	},
> > > @@ -694,6 +726,13 @@ static irqreturn_t mtk_spi_interrupt(int
> > > irq,
> > > void *dev_id)
> > >   	else
> > >   		mdata->state = MTK_SPI_IDLE;
> > >   
> > > +	/* SPI-MEM ops */
> > > +	if (mdata->use_spimem) {
> > > +		complete(&mdata->spimem_done);
> > > +
> > > +		return IRQ_HANDLED;
> > > +	}
> > > +
> > 
> > I would instead make a new ISR for the SPI-MEM case... as you're
> > bypassing the
> > entire mtk_spi_interrupt() function like that.
> > 
> > Example:
> > 
> > static void mtk_spi_check_and_set_state(struct mtk_spi *mdata)
> > {
> > 	u32 reg_val;
> > 
> > 	reg_val = readl(mdata->base + SPI_STATUS0_REG);
> > 
> > 	if (reg_val & MTK_SPI_PAUSE_INT_STATUS)
> > 
> > 		mdata->state = MTK_SPI_PAUSED;
> > 
> > 	else
> > 
> > 		mdata->state = MTK_SPI_IDLE;
> > }
> > 
> > static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
> > 
> > {
> > 	u32 cmd, reg_val, cnt, remainder, len;
> > 
> > 	struct spi_master *master = dev_id;
> > 
> > 	struct mtk_spi *mdata = spi_master_get_devdata(master);
> > 
> > 	struct spi_transfer *trans = mdata->cur_transfer;
> > 
> > 	mtk_spi_check_and_set_state(mdata);
> > 
> > 	if (!master->can_dma(........
> > 	.... blurb...
> > }
> > 
> > static irqreturn_t mtk_spimem_interrupt(int irq, void *dev_id)
> > {
> > 	struct spi_master *master = dev_id;
> > 
> > 	struct mtk_spi *mdata = spi_master_get_devdata(master);
> > 
> > 	mtk_spi_check_and_set_state(mdata);
> > 	complete(&mdata->spimem_done);
> > 
> > 	return IRQ_HANDLED;
> > }
> > 
> > ... of course, in mtk_spi_probe(), you would do something like
> > 
> > if (mdata->use_spimem)
> > 	ret = devm_request_irq(&pdev->dev, irq, mtk_spimem_interrupt,
> > 		...... blah ......
> > else
> > 	ret = devm_request_irq(&pdev->dev, irq, mtk_spi_interrupt,
> > 		..... blah ......
> > 
> > This way, you're separating the two - increasing human readability
> > and showing
> > the simplifications (in that regard) in the IPM IP's SPI-MEM
> > handling.
> > 
> 
> thanks for your advise, I'll do it in the next version.
Hi AngeloGioacchino,
 It can't request mtk_spimem_interrupt() only while support spimem
core, since spi driver should support spi_sync and spi_mem_exec_op
both.

> 
> > >   	if (!master->can_dma(master, NULL, trans)) {
> > >   		if (trans->rx_buf) {
> > >   			cnt = mdata->xfer_len / 4;
> > > @@ -777,6 +816,266 @@ static irqreturn_t mtk_spi_interrupt(int
> > > irq,
> > > void *dev_id)
> > >   	return IRQ_HANDLED;
> > >   }
> > >   
> > > +static int mtk_spi_mem_adjust_op_size(struct spi_mem *mem,
> > > +				      struct spi_mem_op *op)
> > > +{
> > > +	int opcode_len;
> > > +
> > > +	if (op->data.dir != SPI_MEM_NO_DATA) {
> > > +		opcode_len = 1 + op->addr.nbytes + op->dummy.nbytes;
> > > +		if (opcode_len + op->data.nbytes >
> > > MTK_SPI_IPM_PACKET_SIZE) {
> > > +			op->data.nbytes = MTK_SPI_IPM_PACKET_SIZE -
> > > opcode_len;
> > > +			/* force data buffer dma-aligned. */
> > > +			op->data.nbytes -= op->data.nbytes % 4;
> > > +		}
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static bool mtk_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)
> > > +		return false;
> > > +
> > > +	if (op->addr.nbytes && op->dummy.nbytes &&
> > > +	    op->addr.buswidth != op->dummy.buswidth)
> > > +		return false;
> > > +
> > > +	if (op->addr.nbytes + op->dummy.nbytes > 16)
> > > +		return false;
> > > +
> > > +	if (op->data.nbytes > MTK_SPI_IPM_PACKET_SIZE) {
> > > +		if (op->data.nbytes / MTK_SPI_IPM_PACKET_SIZE >
> > > +		    MTK_SPI_IPM_PACKET_LOOP ||
> > > +		    op->data.nbytes % MTK_SPI_IPM_PACKET_SIZE != 0)
> > > +			return false;
> > > +	}
> > > +
> > > +	return true;
> > > +}
> > > +
> > > +static void mtk_spi_mem_setup_dma_xfer(struct spi_master
> > > *master,
> > > +				       const struct spi_mem_op *op)
> > > +{
> > > +	struct mtk_spi *mdata = spi_master_get_devdata(master);
> > > +
> > > +	writel((u32)(mdata->tx_dma & MTK_SPI_32BITS_MASK),
> > > +	       mdata->base + SPI_TX_SRC_REG);
> > > +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> > > +	if (mdata->dev_comp->dma_ext)
> > > +		writel((u32)(mdata->tx_dma >> 32),
> > > +		       mdata->base + SPI_TX_SRC_REG_64);
> > > +#endif
> > > +
> > > +	if (op->data.dir == SPI_MEM_DATA_IN) {
> > > +		writel((u32)(mdata->rx_dma & MTK_SPI_32BITS_MASK),
> > > +		       mdata->base + SPI_RX_DST_REG);
> > > +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> > > +		if (mdata->dev_comp->dma_ext)
> > > +			writel((u32)(mdata->rx_dma >> 32),
> > > +			       mdata->base + SPI_RX_DST_REG_64);
> > > +#endif
> > > +	}
> > > +}
> > > +
> > > +static int mtk_spi_transfer_wait(struct spi_mem *mem,
> > > +				 const struct spi_mem_op *op)
> > > +{
> > > +	struct mtk_spi *mdata = spi_master_get_devdata(mem->spi-
> > > > master);
> > > 
> > > +	unsigned long long ms = 1;
> > 
> > Initializing ms to 1 here is useless, as you're anyway
> > reinitializing
> > it
> > right after. I would do it as following:
> > 
> > u64 ms = 8000LL;
> > 
> > if (op->data.dir == SPI_MEM_NO_DATA)
> > 	ms *= 32;
> > else
> > 	ms *= op->data.nbytes;
> > 
> 
> OK,I'll fix it.
> 
> > Besides, can you please add a comment to the code explaining why
> > the
> > reason for the waits (why 8, why 1000, why 32)?
> > 
> 
> OK,THANKS
> 
> > > +
> > > +	if (op->data.dir == SPI_MEM_NO_DATA)
> > > +		ms = 8LL * 1000LL * 32;
> > > +	else
> > > +		ms = 8LL * 1000LL * op->data.nbytes;
> > > +	do_div(ms, mem->spi->max_speed_hz);
> > 
> > I appreciate the usage of safe division helpers, but this is the
> > wrong one:
> > you have a unsigned long long (64-bits) dividend and a u32 divisor,
> > so the
> > right function to use here is div_u64().
> > 
> 
> OK, I'll fix it. thanks!
> 
> > 	ms = div_u64(ms, mem->spi->max_speed_hz);
> > 
> > > +	ms += ms + 1000; /* 1s tolerance */
> > > +
> > > +	if (ms > UINT_MAX)
> > > +		ms = UINT_MAX;
> > > +
> > > +	if (!wait_for_completion_timeout(&mdata->spimem_done,
> > > +					 msecs_to_jiffies(ms))) {
> > > +		dev_err(mdata->dev, "spi-mem transfer timeout\n");
> > > +		return -ETIMEDOUT;
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int mtk_spi_mem_exec_op(struct spi_mem *mem,
> > > +			       const struct spi_mem_op *op)
> > > +{
> > > +	struct mtk_spi *mdata = spi_master_get_devdata(mem->spi-
> > > > master);
> > > 
> > > +	u32 reg_val, nio = 1, tx_size;
> > > +	char *tx_tmp_buf, *rx_tmp_buf;
> > > +	int ret = 0;
> > > +
> > > +	mdata->use_spimem = true;
> > > +	reinit_completion(&mdata->spimem_done);
> > > +
> > > +	mtk_spi_reset(mdata);
> > > +	mtk_spi_hw_init(mem->spi->master, mem->spi);
> > > +	mtk_spi_prepare_transfer(mem->spi->master, mem->spi-
> > > > max_speed_hz);
> > > 
> > > +
> > > +	reg_val = readl(mdata->base + SPI_CFG3_IPM_REG);
> > > +	/* opcode byte len */
> > > +	reg_val &= ~SPI_CFG3_IPM_CMD_BYTELEN_MASK;
> > > +	reg_val |= 1 << SPI_CFG3_IPM_CMD_BYTELEN_OFFSET;
> > > +
> > > +	/* addr & dummy byte len */
> > > +	reg_val &= ~SPI_CFG3_IPM_ADDR_BYTELEN_MASK;
> > > +	if (op->addr.nbytes || op->dummy.nbytes)
> > > +		reg_val |= (op->addr.nbytes + op->dummy.nbytes) <<
> > > +			    SPI_CFG3_IPM_ADDR_BYTELEN_OFFSET;
> > > +
> > > +	/* data byte len */
> > > +	if (op->data.dir == SPI_MEM_NO_DATA) {
> > > +		reg_val |= SPI_CFG3_IPM_NODATA_FLAG;
> > > +		writel(0, mdata->base + SPI_CFG1_REG);
> > > +	} else {
> > > +		reg_val &= ~SPI_CFG3_IPM_NODATA_FLAG;
> > > +		mdata->xfer_len = op->data.nbytes;
> > > +		mtk_spi_setup_packet(mem->spi->master);
> > > +	}
> > > +
> > > +	if (op->addr.nbytes || op->dummy.nbytes) {
> > > +		if (op->addr.buswidth == 1 || op->dummy.buswidth == 1)
> > > +			reg_val |= SPI_CFG3_IPM_XMODE_EN;
> > > +		else
> > > +			reg_val &= ~SPI_CFG3_IPM_XMODE_EN;
> > > +	}
> > > +
> > > +	if (op->addr.buswidth == 2 ||
> > > +	    op->dummy.buswidth == 2 ||
> > > +	    op->data.buswidth == 2)
> > > +		nio = 2;
> > > +	else if (op->addr.buswidth == 4 ||
> > > +		 op->dummy.buswidth == 4 ||
> > > +		 op->data.buswidth == 4)
> > > +		nio = 4;
> > 
> > 	else
> > 		nio = 1;
> > 
> > ...so that you can avoid double initialization of the `nio`
> > variable.
> > 
> 
> OK, I'll fix it,thanks
> 
> > > +
> > > +	reg_val &= ~SPI_CFG3_IPM_CMD_PIN_MODE_MASK;
> > > +	reg_val |= PIN_MODE_CFG(nio) << SPI_CFG3_IPM_PIN_MODE_OFFSET;
> > > +
> > 
> > Regards,
> > Angelo
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek