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

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

[Leilk Liu]

This series of patches are based on spi for-next, and provide 6 patches to support MT7986.

v4:
 1. fix Rob comment in v3;
 2. use "mediatek,mt7986-spi-ipm","mediatek,spi-ipm"

v3:
 1. add Rob Acked-by in "dt-bindings: spi: Add compatible for MT7986 with single mode";
 2. add a fix patch "spi: mediatek: support tick_delay without enhance_timing";
 3. fix Angelogioacchino comments;
 4. use mt7986 instead of ipm in dt-binding.

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

Leilk Liu (6):
  spi: mediatek: support tick_delay without enhance_timing
  dt-bindings: spi: Add compatible for MT7986
  spi: mediatek: add ipm design support for MT7986
  spi: mediatek: add spi memory support for ipm design
  dt-bindings: spi: support hclk
  spi: mediatek: support hclk

 .../bindings/spi/mediatek,spi-mt65xx.yaml     |   8 +
 drivers/spi/spi-mt65xx.c                      | 543 +++++++++++++++++-
 2 files changed, 520 insertions(+), 31 deletions(-)

--
2.25.1

[PATCH V4 1/6] spi: mediatek: support tick_delay without enhance_timing

[Leilk Liu]

this patch support tick_delay bit[31:30] without enhance_timing feature.

Fixes: f84d866ab43f("spi: mediatek: add tick_delay support")
Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/spi/spi-mt65xx.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
index bbfeb8046c17..3fd89548ec3c 100644
--- a/drivers/spi/spi-mt65xx.c
+++ b/drivers/spi/spi-mt65xx.c
@@ -43,8 +43,11 @@
 #define SPI_CFG1_PACKET_LOOP_OFFSET       8
 #define SPI_CFG1_PACKET_LENGTH_OFFSET     16
 #define SPI_CFG1_GET_TICK_DLY_OFFSET      29
+#define SPI_CFG1_GET_TICK_DLY_OFFSET_V1   30
 
 #define SPI_CFG1_GET_TICK_DLY_MASK        0xe0000000
+#define SPI_CFG1_GET_TICK_DLY_MASK_V1     0xc0000000
+
 #define SPI_CFG1_CS_IDLE_MASK             0xff
 #define SPI_CFG1_PACKET_LOOP_MASK         0xff00
 #define SPI_CFG1_PACKET_LENGTH_MASK       0x3ff0000
@@ -346,9 +349,15 @@ static int mtk_spi_prepare_message(struct spi_master *master,
 
 	/* 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);
+	if (mdata->dev_comp->enhance_timing) {
+		reg_val &= ~SPI_CFG1_GET_TICK_DLY_MASK;
+		reg_val |= ((chip_config->tick_delay & 0x7)
+			    << SPI_CFG1_GET_TICK_DLY_OFFSET);
+	} else {
+		reg_val &= ~SPI_CFG1_GET_TICK_DLY_MASK_V1;
+		reg_val |= ((chip_config->tick_delay & 0x3)
+			    << SPI_CFG1_GET_TICK_DLY_OFFSET_V1);
+	}
 	writel(reg_val, mdata->base + SPI_CFG1_REG);
 
 	/* set hw cs timing */
-- 
2.25.1

[PATCH V4 2/6] dt-bindings: spi: Add compatible for MT7986

[Leilk Liu]

This patch adds dt-binding documentation for MT7986.

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

diff --git a/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml b/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
index bfa44acb1bdd..7247a177466b 100644
--- a/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
+++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
@@ -30,6 +30,10 @@ properties:
               - mediatek,mt8192-spi
               - mediatek,mt8195-spi
           - const: mediatek,mt6765-spi
+      - items:
+          - enum:
+              - mediatek,mt7986-spi-ipm
+          - const: mediatek,spi-ipm
       - items:
           - enum:
               - mediatek,mt2701-spi
-- 
2.25.1

Re: [PATCH V4 2/6] dt-bindings: spi: Add compatible for MT7986

[AngeloGioacchino Del Regno]

Il 15/03/22 04:24, Leilk Liu ha scritto:
> This patch adds dt-binding documentation for MT7986.
> 
> Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

> ---
>   .../devicetree/bindings/spi/mediatek,spi-mt65xx.yaml          | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml b/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
> index bfa44acb1bdd..7247a177466b 100644
> --- a/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
> +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
> @@ -30,6 +30,10 @@ properties:
>                 - mediatek,mt8192-spi
>                 - mediatek,mt8195-spi
>             - const: mediatek,mt6765-spi
> +      - items:
> +          - enum:
> +              - mediatek,mt7986-spi-ipm
> +          - const: mediatek,spi-ipm
>         - items:
>             - enum:
>                 - mediatek,mt2701-spi

[PATCH V4 3/6] spi: mediatek: add ipm design support for MT7986

[Leilk Liu]

this patch add the support of ipm design.

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

diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
index 3fd89548ec3c..1a0b3208dfca 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
@@ -51,6 +52,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
 
@@ -71,7 +73,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
@@ -81,6 +89,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)
@@ -96,6 +105,9 @@ struct mtk_spi_compatible {
 	bool dma_ext;
 	/* some IC no need unprepare SPI clk */
 	bool no_need_unprepare;
+	/* IPM design adjust and extend register to support more features */
+	bool ipm_design;
+
 };
 
 struct mtk_spi {
@@ -119,6 +131,12 @@ static const struct mtk_spi_compatible mt2712_compat = {
 	.must_tx = true,
 };
 
+static const struct mtk_spi_compatible mtk_ipm_compat = {
+	.enhance_timing = true,
+	.dma_ext = true,
+	.ipm_design = true,
+};
+
 static const struct mtk_spi_compatible mt6765_compat = {
 	.need_pad_sel = true,
 	.must_tx = true,
@@ -160,6 +178,9 @@ static const struct mtk_chip_config mtk_default_chip_info = {
 };
 
 static const struct of_device_id mtk_spi_of_match[] = {
+	{ .compatible = "mediatek,spi-ipm",
+		.data = (void *)&mtk_ipm_compat,
+	},
 	{ .compatible = "mediatek,mt2701-spi",
 		.data = (void *)&mtk_common_compat,
 	},
@@ -278,12 +299,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);
 
@@ -291,6 +311,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
@@ -348,23 +377,39 @@ static int mtk_spi_prepare_message(struct spi_master *master,
 		       mdata->base + SPI_PAD_SEL_REG);
 
 	/* tick delay */
-	reg_val = readl(mdata->base + SPI_CFG1_REG);
 	if (mdata->dev_comp->enhance_timing) {
-		reg_val &= ~SPI_CFG1_GET_TICK_DLY_MASK;
-		reg_val |= ((chip_config->tick_delay & 0x7)
-			    << SPI_CFG1_GET_TICK_DLY_OFFSET);
+		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);
+		}
 	} else {
+		reg_val = readl(mdata->base + SPI_CFG1_REG);
 		reg_val &= ~SPI_CFG1_GET_TICK_DLY_MASK_V1;
 		reg_val |= ((chip_config->tick_delay & 0x3)
 			    << SPI_CFG1_GET_TICK_DLY_OFFSET_V1);
+		writel(reg_val, mdata->base + SPI_CFG1_REG);
 	}
-	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;
@@ -386,13 +431,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;
 
@@ -423,12 +468,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);
 }
@@ -523,7 +580,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) {
@@ -556,7 +613,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)
@@ -591,6 +648,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
@@ -757,6 +827,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

Re: [PATCH V4 3/6] spi: mediatek: add ipm design support for MT7986

[AngeloGioacchino Del Regno]

Il 15/03/22 04:24, Leilk Liu ha scritto:
> this patch add the support of ipm design.
> 
> Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

> ---
>   drivers/spi/spi-mt65xx.c | 102 +++++++++++++++++++++++++++++++++------
>   1 file changed, 87 insertions(+), 15 deletions(-)
> 

Re: [PATCH V4 3/6] spi: mediatek: add ipm design support for MT7986

[AngeloGioacchino Del Regno]

Il 15/03/22 04:24, Leilk Liu ha scritto:
> this patch add the support of ipm design.
> 
> Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>

Sorry for the double email, the first one was sent from the wrong address.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

> ---
>   drivers/spi/spi-mt65xx.c | 102 +++++++++++++++++++++++++++++++++------
>   1 file changed, 87 insertions(+), 15 deletions(-)
> 

[PATCH V4 4/6] spi: mediatek: add spi memory support for ipm design

[Leilk Liu]

this patch add the support of spi-mem for ipm design.

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

diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
index 1a0b3208dfca..8958c3fa4fea 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
@@ -78,8 +79,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
@@ -90,6 +104,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)
@@ -107,7 +123,6 @@ struct mtk_spi_compatible {
 	bool no_need_unprepare;
 	/* IPM design adjust and extend register to support more features */
 	bool ipm_design;
-
 };
 
 struct mtk_spi {
@@ -123,6 +138,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;
@@ -704,6 +724,12 @@ 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;
@@ -787,6 +813,320 @@ 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);
+	/*
+	 * For each byte we wait for 8 cycles of the SPI clock.
+	 * Since speed is defined in Hz and we want milliseconds,
+	 * so it should be 8 * 1000.
+	 */
+	u64 ms = 8000LL;
+
+	if (op->data.dir == SPI_MEM_NO_DATA)
+		ms *= 32; /* prevent we may get 0 for short transfers. */
+	else
+		ms *= op->data.nbytes;
+	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, 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;
+
+	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) {
+		mdata->use_spimem = false;
+		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 void of_mtk_spi_parse_dt(struct spi_master *master, struct device_node *nc)
+{
+	struct mtk_spi *mdata = spi_master_get_devdata(master);
+	u32 value;
+
+	if (!of_property_read_u32(nc, "spi-tx-bus-width", &value)) {
+		switch (value) {
+		case 1:
+			break;
+		case 2:
+			master->mode_bits |= SPI_TX_DUAL;
+			break;
+		case 4:
+			master->mode_bits |= SPI_TX_QUAD;
+			break;
+		default:
+			dev_warn(mdata->dev,
+				 "spi-tx-bus-width %d not supported\n",
+				value);
+			break;
+		}
+	}
+
+	if (!of_property_read_u32(nc, "spi-rx-bus-width", &value)) {
+		switch (value) {
+		case 1:
+			break;
+		case 2:
+			master->mode_bits |= SPI_RX_DUAL;
+			break;
+		case 4:
+			master->mode_bits |= SPI_RX_QUAD;
+			break;
+		case 8:
+			master->mode_bits |= SPI_RX_OCTAL;
+			break;
+		default:
+			dev_warn(mdata->dev,
+				 "spi-rx-bus-width %d not supported\n",
+				value);
+			break;
+		}
+	}
+}
+
 static int mtk_spi_probe(struct platform_device *pdev)
 {
 	struct spi_master *master;
@@ -830,6 +1170,13 @@ static int mtk_spi_probe(struct platform_device *pdev)
 	if (mdata->dev_comp->ipm_design)
 		master->mode_bits |= SPI_LOOP;
 
+	if (mdata->dev_comp->ipm_design) {
+		mdata->dev = &pdev->dev;
+		master->mem_ops = &mtk_spi_mem_ops;
+		of_mtk_spi_parse_dt(master, pdev->dev.of_node);
+		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

Re: [PATCH V4 4/6] spi: mediatek: add spi memory support for ipm design

[AngeloGioacchino Del Regno]

Il 15/03/22 04:24, Leilk Liu ha scritto:
> this patch add the support of spi-mem for ipm design.
> 
> Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
> ---
>   drivers/spi/spi-mt65xx.c | 349 ++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 348 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
> index 1a0b3208dfca..8958c3fa4fea 100644
> --- a/drivers/spi/spi-mt65xx.c
> +++ b/drivers/spi/spi-mt65xx.c

...snip...

> +
> +static void of_mtk_spi_parse_dt(struct spi_master *master, struct device_node *nc)
> +{
> +	struct mtk_spi *mdata = spi_master_get_devdata(master);
> +	u32 value;
> +
> +	if (!of_property_read_u32(nc, "spi-tx-bus-width", &value)) {

Hello Leilk,

thanks for considering my advice about "spi-{tx,rx}-bus-width", but there's
something that you have misunderstood about it.

Simply, you don't need this function at all. Whatever you are doing here is
already being performed in the Linux SPI framework: at the end of the probe
function, this driver is calling the (legacy) devm_spi_register_master(),
which calls devm_spi_register_controller().

In drivers/spi/spi.c, function spi_register_controller(), will in turn call
of_register_spi_devices(ctlr) -> of_register_spi_device(ctlr, nc)... that
will end up finally calling function of_spi_parse_dt(ctlr, spi, nc).

The last mentioned function already contains the logic and setup to check
devicetree properties "spi-tx-bus-width" and "spi-rx-bus-width" (and some
others, as well).

This means that spi-mt65xx.c already probed these even before your IPM
implementation, hence ***function of_mtk_spi_parse_dt() is not needed***.

Simply drop it and don't check for these properties: that's already done.


Regards,
Angelo

> +		switch (value) {
> +		case 1:
> +			break;
> +		case 2:
> +			master->mode_bits |= SPI_TX_DUAL;
> +			break;
> +		case 4:
> +			master->mode_bits |= SPI_TX_QUAD;
> +			break;
> +		default:
> +			dev_warn(mdata->dev,
> +				 "spi-tx-bus-width %d not supported\n",
> +				value);
> +			break;
> +		}
> +	}
> +
> +	if (!of_property_read_u32(nc, "spi-rx-bus-width", &value)) {
> +		switch (value) {
> +		case 1:
> +			break;
> +		case 2:
> +			master->mode_bits |= SPI_RX_DUAL;
> +			break;
> +		case 4:
> +			master->mode_bits |= SPI_RX_QUAD;
> +			break;
> +		case 8:
> +			master->mode_bits |= SPI_RX_OCTAL;
> +			break;
> +		default:
> +			dev_warn(mdata->dev,
> +				 "spi-rx-bus-width %d not supported\n",
> +				value);
> +			break;
> +		}
> +	}
> +}
> +
>   static int mtk_spi_probe(struct platform_device *pdev)
>   {
>   	struct spi_master *master;
> @@ -830,6 +1170,13 @@ static int mtk_spi_probe(struct platform_device *pdev)
>   	if (mdata->dev_comp->ipm_design)
>   		master->mode_bits |= SPI_LOOP;
>   
> +	if (mdata->dev_comp->ipm_design) {
> +		mdata->dev = &pdev->dev;
> +		master->mem_ops = &mtk_spi_mem_ops;
> +		of_mtk_spi_parse_dt(master, pdev->dev.of_node);
> +		init_completion(&mdata->spimem_done);
> +	}
> +
>   	if (mdata->dev_comp->need_pad_sel) {
>   		mdata->pad_num = of_property_count_u32_elems(
>   			pdev->dev.of_node,

Re: [PATCH V4 4/6] spi: mediatek: add spi memory support for ipm design

[Leilk Liu]

On Tue, 2022-03-15 at 10:31 +0100, AngeloGioacchino Del Regno wrote:
> Il 15/03/22 04:24, Leilk Liu ha scritto:
> > this patch add the support of spi-mem for ipm design.
> > 
> > Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
> > ---
> >   drivers/spi/spi-mt65xx.c | 349
> > ++++++++++++++++++++++++++++++++++++++-
> >   1 file changed, 348 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
> > index 1a0b3208dfca..8958c3fa4fea 100644
> > --- a/drivers/spi/spi-mt65xx.c
> > +++ b/drivers/spi/spi-mt65xx.c
> 
> ...snip...
> 
> > +
> > +static void of_mtk_spi_parse_dt(struct spi_master *master, struct
> > device_node *nc)
> > +{
> > +	struct mtk_spi *mdata = spi_master_get_devdata(master);
> > +	u32 value;
> > +
> > +	if (!of_property_read_u32(nc, "spi-tx-bus-width", &value)) {
> 
> Hello Leilk,
> 
> thanks for considering my advice about "spi-{tx,rx}-bus-width", but
> there's
> something that you have misunderstood about it.
> 
> Simply, you don't need this function at all. Whatever you are doing
> here is
> already being performed in the Linux SPI framework: at the end of the
> probe
> function, this driver is calling the (legacy)
> devm_spi_register_master(),
> which calls devm_spi_register_controller().
> 
> In drivers/spi/spi.c, function spi_register_controller(), will in
> turn call
> of_register_spi_devices(ctlr) -> of_register_spi_device(ctlr, nc)...
> that
> will end up finally calling function of_spi_parse_dt(ctlr, spi, nc).
> 
> The last mentioned function already contains the logic and setup to
> check
> devicetree properties "spi-tx-bus-width" and "spi-rx-bus-width" (and
> some
> others, as well).
> 
> This means that spi-mt65xx.c already probed these even before your
> IPM
> implementation, hence ***function of_mtk_spi_parse_dt() is not
> needed***.
> 
> Simply drop it and don't check for these properties: that's already
> done.
> 
> 
> Regards,
> Angelo
> 
Hi Angelo,

Thanks for your advice.

There are two spi controllor on MT7986. One supports single/dual mode,
the other supports quad mode. Both of them can support spi memory
framework(one's tx/rx bus width is 1/2, the other one's tx/rx bus width
is 1/2/4). 

Can I use of_mtk_spi_parse_dt() to parse the information? What's your
suggestion?

Thanks!


> > +		switch (value) {
> > +		case 1:
> > +			break;
> > +		case 2:
> > +			master->mode_bits |= SPI_TX_DUAL;
> > +			break;
> > +		case 4:
> > +			master->mode_bits |= SPI_TX_QUAD;
> > +			break;
> > +		default:
> > +			dev_warn(mdata->dev,
> > +				 "spi-tx-bus-width %d not supported\n",
> > +				value);
> > +			break;
> > +		}
> > +	}
> > +
> > +	if (!of_property_read_u32(nc, "spi-rx-bus-width", &value)) {
> > +		switch (value) {
> > +		case 1:
> > +			break;
> > +		case 2:
> > +			master->mode_bits |= SPI_RX_DUAL;
> > +			break;
> > +		case 4:
> > +			master->mode_bits |= SPI_RX_QUAD;
> > +			break;
> > +		case 8:
> > +			master->mode_bits |= SPI_RX_OCTAL;
> > +			break;
> > +		default:
> > +			dev_warn(mdata->dev,
> > +				 "spi-rx-bus-width %d not supported\n",
> > +				value);
> > +			break;
> > +		}
> > +	}
> > +}
> > +
> >   static int mtk_spi_probe(struct platform_device *pdev)
> >   {
> >   	struct spi_master *master;
> > @@ -830,6 +1170,13 @@ static int mtk_spi_probe(struct
> > platform_device *pdev)
> >   	if (mdata->dev_comp->ipm_design)
> >   		master->mode_bits |= SPI_LOOP;
> >   
> > +	if (mdata->dev_comp->ipm_design) {
> > +		mdata->dev = &pdev->dev;
> > +		master->mem_ops = &mtk_spi_mem_ops;
> > +		of_mtk_spi_parse_dt(master, pdev->dev.of_node);
> > +		init_completion(&mdata->spimem_done);
> > +	}
> > +
> >   	if (mdata->dev_comp->need_pad_sel) {
> >   		mdata->pad_num = of_property_count_u32_elems(
> >   			pdev->dev.of_node,

Re: [PATCH V4 4/6] spi: mediatek: add spi memory support for ipm design

[AngeloGioacchino Del Regno]

Il 17/03/22 10:27, Leilk Liu ha scritto:
> On Tue, 2022-03-15 at 10:31 +0100, AngeloGioacchino Del Regno wrote:
>> Il 15/03/22 04:24, Leilk Liu ha scritto:
>>> this patch add the support of spi-mem for ipm design.
>>>
>>> Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
>>> ---
>>>    drivers/spi/spi-mt65xx.c | 349
>>> ++++++++++++++++++++++++++++++++++++++-
>>>    1 file changed, 348 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
>>> index 1a0b3208dfca..8958c3fa4fea 100644
>>> --- a/drivers/spi/spi-mt65xx.c
>>> +++ b/drivers/spi/spi-mt65xx.c
>>
>> ...snip...
>>
>>> +
>>> +static void of_mtk_spi_parse_dt(struct spi_master *master, struct
>>> device_node *nc)
>>> +{
>>> +	struct mtk_spi *mdata = spi_master_get_devdata(master);
>>> +	u32 value;
>>> +
>>> +	if (!of_property_read_u32(nc, "spi-tx-bus-width", &value)) {
>>
>> Hello Leilk,
>>
>> thanks for considering my advice about "spi-{tx,rx}-bus-width", but
>> there's
>> something that you have misunderstood about it.
>>
>> Simply, you don't need this function at all. Whatever you are doing
>> here is
>> already being performed in the Linux SPI framework: at the end of the
>> probe
>> function, this driver is calling the (legacy)
>> devm_spi_register_master(),
>> which calls devm_spi_register_controller().
>>
>> In drivers/spi/spi.c, function spi_register_controller(), will in
>> turn call
>> of_register_spi_devices(ctlr) -> of_register_spi_device(ctlr, nc)...
>> that
>> will end up finally calling function of_spi_parse_dt(ctlr, spi, nc).
>>
>> The last mentioned function already contains the logic and setup to
>> check
>> devicetree properties "spi-tx-bus-width" and "spi-rx-bus-width" (and
>> some
>> others, as well).
>>
>> This means that spi-mt65xx.c already probed these even before your
>> IPM
>> implementation, hence ***function of_mtk_spi_parse_dt() is not
>> needed***.
>>
>> Simply drop it and don't check for these properties: that's already
>> done.
>>
>>
>> Regards,
>> Angelo
>>
> Hi Angelo,
> 
> Thanks for your advice.
> 
> There are two spi controllor on MT7986. One supports single/dual mode,
> the other supports quad mode. Both of them can support spi memory
> framework(one's tx/rx bus width is 1/2, the other one's tx/rx bus width
> is 1/2/4).
> 
> Can I use of_mtk_spi_parse_dt() to parse the information? What's your
> suggestion?
> 
> Thanks!
> 

As I've already said, this does NOT require any devicetree handling in
spi-mt65xx.c, as setting the right mode_bits is already handled in
drivers/spi/spi.c - please follow the explaination that I have given
before to fully understand the situation.

Regards,
Angelo


> 
>>> +		switch (value) {
>>> +		case 1:
>>> +			break;
>>> +		case 2:
>>> +			master->mode_bits |= SPI_TX_DUAL;
>>> +			break;
>>> +		case 4:
>>> +			master->mode_bits |= SPI_TX_QUAD;
>>> +			break;
>>> +		default:
>>> +			dev_warn(mdata->dev,
>>> +				 "spi-tx-bus-width %d not supported\n",
>>> +				value);
>>> +			break;
>>> +		}
>>> +	}
>>> +
>>> +	if (!of_property_read_u32(nc, "spi-rx-bus-width", &value)) {
>>> +		switch (value) {
>>> +		case 1:

Re: [PATCH V4 4/6] spi: mediatek: add spi memory support for ipm design

[Leilk Liu]

On Thu, 2022-03-17 at 10:33 +0100, AngeloGioacchino Del Regno wrote:
> Il 17/03/22 10:27, Leilk Liu ha scritto:
> > On Tue, 2022-03-15 at 10:31 +0100, AngeloGioacchino Del Regno
> > wrote:
> > > Il 15/03/22 04:24, Leilk Liu ha scritto:
> > > > this patch add the support of spi-mem for ipm design.
> > > > 
> > > > Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
> > > > ---
> > > >    drivers/spi/spi-mt65xx.c | 349
> > > > ++++++++++++++++++++++++++++++++++++++-
> > > >    1 file changed, 348 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-
> > > > mt65xx.c
> > > > index 1a0b3208dfca..8958c3fa4fea 100644
> > > > --- a/drivers/spi/spi-mt65xx.c
> > > > +++ b/drivers/spi/spi-mt65xx.c
> > > 
> > > ...snip...
> > > 
> > > > +
> > > > +static void of_mtk_spi_parse_dt(struct spi_master *master,
> > > > struct
> > > > device_node *nc)
> > > > +{
> > > > +	struct mtk_spi *mdata = spi_master_get_devdata(master);
> > > > +	u32 value;
> > > > +
> > > > +	if (!of_property_read_u32(nc, "spi-tx-bus-width",
> > > > &value)) {
> > > 
> > > Hello Leilk,
> > > 
> > > thanks for considering my advice about "spi-{tx,rx}-bus-width",
> > > but
> > > there's
> > > something that you have misunderstood about it.
> > > 
> > > Simply, you don't need this function at all. Whatever you are
> > > doing
> > > here is
> > > already being performed in the Linux SPI framework: at the end of
> > > the
> > > probe
> > > function, this driver is calling the (legacy)
> > > devm_spi_register_master(),
> > > which calls devm_spi_register_controller().
> > > 
> > > In drivers/spi/spi.c, function spi_register_controller(), will in
> > > turn call
> > > of_register_spi_devices(ctlr) -> of_register_spi_device(ctlr,
> > > nc)...
> > > that
> > > will end up finally calling function of_spi_parse_dt(ctlr, spi,
> > > nc).
> > > 
> > > The last mentioned function already contains the logic and setup
> > > to
> > > check
> > > devicetree properties "spi-tx-bus-width" and "spi-rx-bus-width"
> > > (and
> > > some
> > > others, as well).
> > > 
> > > This means that spi-mt65xx.c already probed these even before
> > > your
> > > IPM
> > > implementation, hence ***function of_mtk_spi_parse_dt() is not
> > > needed***.
> > > 
> > > Simply drop it and don't check for these properties: that's
> > > already
> > > done.
> > > 
> > > 
> > > Regards,
> > > Angelo
> > > 
> > 
> > Hi Angelo,
> > 
> > Thanks for your advice.
> > 
> > There are two spi controllor on MT7986. One supports single/dual
> > mode,
> > the other supports quad mode. Both of them can support spi memory
> > framework(one's tx/rx bus width is 1/2, the other one's tx/rx bus
> > width
> > is 1/2/4).
> > 
> > Can I use of_mtk_spi_parse_dt() to parse the information? What's
> > your
> > suggestion?
> > 
> > Thanks!
> > 
> 
> As I've already said, this does NOT require any devicetree handling
> in
> spi-mt65xx.c, as setting the right mode_bits is already handled in
> drivers/spi/spi.c - please follow the explaination that I have given
> before to fully understand the situation.
> 
> Regards,
> Angelo
> 
OK, I'll fix it, thanks.

> 
> > 
> > > > +		switch (value) {
> > > > +		case 1:
> > > > +			break;
> > > > +		case 2:
> > > > +			master->mode_bits |= SPI_TX_DUAL;
> > > > +			break;
> > > > +		case 4:
> > > > +			master->mode_bits |= SPI_TX_QUAD;
> > > > +			break;
> > > > +		default:
> > > > +			dev_warn(mdata->dev,
> > > > +				 "spi-tx-bus-width %d not
> > > > supported\n",
> > > > +				value);
> > > > +			break;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	if (!of_property_read_u32(nc, "spi-rx-bus-width",
> > > > &value)) {
> > > > +		switch (value) {
> > > > +		case 1:

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

[Leilk Liu]

this patch support hclk for AHB bus.

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

diff --git a/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml b/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
index 7247a177466b..ecb4a5002cc1 100644
--- a/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
+++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mt65xx.yaml
@@ -53,16 +53,20 @@ properties:
     maxItems: 1
 
   clocks:
+    minItems: 3
     items:
       - 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:
+    minItems: 3
     items:
       - const: parent-clk
       - const: sel-clk
       - const: spi-clk
+      - const: hclk
 
   mediatek,pad-select:
     $ref: /schemas/types.yaml#/definitions/uint32-array
-- 
2.25.1

[PATCH V4 6/6] spi: mediatek: support hclk

[Leilk Liu]

this patch adds 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 8958c3fa4fea..d4a602e78aa7 100644
--- a/drivers/spi/spi-mt65xx.c
+++ b/drivers/spi/spi-mt65xx.c
@@ -130,7 +130,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;
@@ -1252,25 +1252,38 @@ static int mtk_spi_probe(struct platform_device *pdev)
 		goto err_put_master;
 	}
 
+	mdata->spi_hclk = devm_clk_get(&pdev->dev, "hclk");
+	if (!IS_ERR(mdata->spi_hclk)) {
+		ret = clk_prepare_enable(mdata->spi_hclk);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "failed to enable 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);
 
@@ -1310,6 +1323,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);
 
@@ -1325,8 +1343,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;
 }
@@ -1342,8 +1363,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;
 }
@@ -1360,11 +1384,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;
 }
@@ -1376,10 +1412,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;
 }
@@ -1390,13 +1431,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 V4 6/6] spi: mediatek: support hclk

[AngeloGioacchino Del Regno]

Il 15/03/22 04:24, Leilk Liu ha scritto:
> this patch adds 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 8958c3fa4fea..d4a602e78aa7 100644
> --- a/drivers/spi/spi-mt65xx.c
> +++ b/drivers/spi/spi-mt65xx.c
> @@ -130,7 +130,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;
> @@ -1252,25 +1252,38 @@ static int mtk_spi_probe(struct platform_device *pdev)
>   		goto err_put_master;
>   	}
>   
> +	mdata->spi_hclk = devm_clk_get(&pdev->dev, "hclk");
> +	if (!IS_ERR(mdata->spi_hclk)) {

What you're doing here can be simplified by using devm_clk_get_optional() instead.
Please use that.

> +		ret = clk_prepare_enable(mdata->spi_hclk);
> +		if (ret < 0) {
> +			dev_err(&pdev->dev, "failed to enable 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);
>   
> @@ -1310,6 +1323,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))

When using devm_clk_get_optional(), you can simply omit this check, since it will
be impossible to reach this point with an error pointer stored in spi_hclk.

> +		clk_disable_unprepare(mdata->spi_hclk);
>   err_put_master:
>   	spi_master_put(master);
>   
> @@ -1325,8 +1343,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;
>   }
> @@ -1342,8 +1363,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;
>   }
> @@ -1360,11 +1384,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)) {

Since you will be using devm_clk_get_optional(), you can also omit this check.

> +			clk_prepare_enable(mdata->spi_hclk);

There's a typo. ret = clk_prepare_enable....

> +			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))

Same here and everywhere else, no error check if you set this as optional clock.

> +			clk_disable_unprepare(mdata->spi_hclk);
> +	}
>   
>   	return ret;
>   }
> @@ -1376,10 +1412,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;
>   }
> @@ -1390,13 +1431,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;
> 


Regards,
Angelo

Re: [PATCH V4 6/6] spi: mediatek: support hclk

[Leilk Liu]

On Tue, 2022-03-15 at 10:37 +0100, AngeloGioacchino Del Regno wrote:
> Il 15/03/22 04:24, Leilk Liu ha scritto:
> > this patch adds 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 8958c3fa4fea..d4a602e78aa7 100644
> > --- a/drivers/spi/spi-mt65xx.c
> > +++ b/drivers/spi/spi-mt65xx.c
> > @@ -130,7 +130,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;
> > @@ -1252,25 +1252,38 @@ static int mtk_spi_probe(struct
> > platform_device *pdev)
> >   		goto err_put_master;
> >   	}
> >   
> > +	mdata->spi_hclk = devm_clk_get(&pdev->dev, "hclk");
> > +	if (!IS_ERR(mdata->spi_hclk)) {
> 
> What you're doing here can be simplified by using
> devm_clk_get_optional() instead.
> Please use that.

Hi AngeloGioacchino, thanks for your suggestion, I'll fix it.

> 
> > +		ret = clk_prepare_enable(mdata->spi_hclk);
> > +		if (ret < 0) {
> > +			dev_err(&pdev->dev, "failed to enable 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);
> >   
> > @@ -1310,6 +1323,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))
> 
> When using devm_clk_get_optional(), you can simply omit this check,
> since it will
> be impossible to reach this point with an error pointer stored in
> spi_hclk.
OK, I'll fix it.

> 
> > +		clk_disable_unprepare(mdata->spi_hclk);
> >   err_put_master:
> >   	spi_master_put(master);
> >   
> > @@ -1325,8 +1343,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;
> >   }
> > @@ -1342,8 +1363,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;
> >   }
> > @@ -1360,11 +1384,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)) {
> 
> Since you will be using devm_clk_get_optional(), you can also omit
> this check.
OK, I'll fix it.

> 
> > +			clk_prepare_enable(mdata->spi_hclk);
> 
> There's a typo. ret = clk_prepare_enable....
OK, I'll fix it.

> 
> > +			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))
> 
> Same here and everywhere else, no error check if you set this as
> optional clock.
OK, I'll fix it.

> 
> > +			clk_disable_unprepare(mdata->spi_hclk);
> > +	}
> >   
> >   	return ret;
> >   }
> > @@ -1376,10 +1412,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;
> >   }
> > @@ -1390,13 +1431,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;
> > 
> 
> 
> Regards,
> Angelo
> 
> 

Re: (subset) [PATCH V4 0/6] spi: mediatek: add single/quad mode support

[Mark Brown]

On Tue, 15 Mar 2022 11:24:05 +0800, Leilk Liu wrote:
> This series of patches are based on spi for-next, and provide 6 patches to support MT7986.
> 
> v4:
>  1. fix Rob comment in v3;
>  2. use "mediatek,mt7986-spi-ipm","mediatek,spi-ipm"
> 
> v3:
>  1. add Rob Acked-by in "dt-bindings: spi: Add compatible for MT7986 with single mode";
>  2. add a fix patch "spi: mediatek: support tick_delay without enhance_timing";
>  3. fix Angelogioacchino comments;
>  4. use mt7986 instead of ipm in dt-binding.
> 
> [...]

Applied to

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

Thanks!

[1/6] spi: mediatek: support tick_delay without enhance_timing
      commit: 03b1be379dcee2e9c866c2a455a1a4a9581b3efd
[2/6] dt-bindings: spi: Add compatible for MT7986
      commit: da40a352db2bae470a3eea038cc53454c24e67f3
[3/6] spi: mediatek: add ipm design support for MT7986
      commit: 7e963fb2a33ce488e65258ab5be38a4855923033

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