diff mbox series

[v3,2/3] spis: mediatek: add spi slave for Mediatek MT2712

Message ID 1537150762-7072-3-git-send-email-leilk.liu@mediatek.com (mailing list archive)
State Superseded
Headers show
Series Add Mediatek SPI slave driver | expand

Commit Message

Leilk Liu Sept. 17, 2018, 2:19 a.m. UTC
This patchs add basic spi slave for MT2712.

Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
---
 drivers/spi/Kconfig            |    8 +
 drivers/spi/Makefile           |    1 +
 drivers/spi/spi-slave-mt27xx.c |  553 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 562 insertions(+)
 create mode 100644 drivers/spi/spi-slave-mt27xx.c

Comments

Mark Brown Sept. 18, 2018, 4:30 p.m. UTC | #1
On Mon, Sep 17, 2018 at 10:19:21AM +0800, Leilk Liu wrote:

This looks overall pretty good, a few smallish comments below:

Please use subject lines matching the style for the subsystem.  This
makes it easier for people to identify relevant patches.

>  if SPI_SLAVE
> +config SPI_SLAVE_MT27XX
> +	tristate "MediaTek SPI slave device"
> +	depends on ARCH_MEDIATEK || COMPILE_TEST
> +	help
> +	  This selects the MediaTek(R) SPI slave device driver.
> +	  If you want to use MediaTek(R) SPI slave interface,
> +	  say Y or M here.If you are not sure, say N.
> +	  SPI slave drivers for Mediatek MT27XX series ARM SoCs.
>  
>  config SPI_SLAVE_TIME

This is a driver not a slave implementation, it should be with the other
drivers in the Kconfig.  The slave implementations are for the
functionality that uses the drivers, not the drivers themselves.

> +	/* set the tx/rx endian */
> +#ifdef __LITTLE_ENDIAN
> +	reg_val &= ~SPIS_TX_ENDIAN;
> +	reg_val &= ~SPIS_RX_ENDIAN;
> +#else
> +	reg_val |= SPIS_TX_ENDIAN;
> +	reg_val |= SPIS_RX_ENDIAN;
> +#endif
> +	writel(reg_val, mdata->base + SPIS_CFG_REG);

This seems weird - it looks like it's changing the endianess of the
protocol based on the endianness the processor is running in.  What's
going on here?  I'd expect the driver to be just treating everything as
a byte stream and letting the protocol driver handle the endianness
issues, otherwise we might end up with double converions.

> +		xfer->tx_dma = dma_map_single(dev, (void *)xfer->tx_buf,
> +					      xfer->len, DMA_TO_DEVICE);

Why is there a cast to void here?  That's usually a sign that there's a
type safety issue, the whole point with void is that it's compatible
with any other pointer.

> +static irqreturn_t mtk_spi_slave_interrupt(int irq, void *dev_id)
> +{
> +	struct spi_controller *ctlr = dev_id;
> +	struct mtk_spi_slave *mdata = spi_controller_get_devdata(ctlr);
> +	struct spi_transfer *trans = mdata->cur_transfer;
> +	u32 int_status, reg_val, cnt, remainder;
> +
> +	int_status = readl(mdata->base + SPIS_IRQ_ST_REG);
> +	writel(int_status, mdata->base + SPIS_IRQ_CLR_REG);
> +
> +	if (!trans)
> +		return IRQ_HANDLED;

Are you sure that this is the right thing to do if we get a spurious
interrupt - the normal thing would be to return IRQ_NONE, possibly
logging a warning as well?

> +	if (int_status & CMD_INVALID_ST)
> +		dev_err(&ctlr->dev, "cmd invalid\n");

If there's an interrupt that doesn't have any of the interrupt status
flags set I'd expect to see a warning and probably IRQ_NONE being
returned.  That way if the interrupt line is shared we work correctly
and if something goes wrong and the interrupt gets stuck on then the
core interrupt code's error handling can kick in.
Leilk Liu Sept. 19, 2018, 5:50 a.m. UTC | #2
Hi Mark,
  Thanks for your comments.

On Tue, 2018-09-18 at 09:30 -0700, Mark Brown wrote:
> On Mon, Sep 17, 2018 at 10:19:21AM +0800, Leilk Liu wrote:
> 
> This looks overall pretty good, a few smallish comments below:
> 
> Please use subject lines matching the style for the subsystem.  This
> makes it easier for people to identify relevant patches.
> 
OK, I'll fix it, thanks.

> >  if SPI_SLAVE
> > +config SPI_SLAVE_MT27XX
> > +	tristate "MediaTek SPI slave device"
> > +	depends on ARCH_MEDIATEK || COMPILE_TEST
> > +	help
> > +	  This selects the MediaTek(R) SPI slave device driver.
> > +	  If you want to use MediaTek(R) SPI slave interface,
> > +	  say Y or M here.If you are not sure, say N.
> > +	  SPI slave drivers for Mediatek MT27XX series ARM SoCs.
> >  
> >  config SPI_SLAVE_TIME
> 
> This is a driver not a slave implementation, it should be with the other
> drivers in the Kconfig.  The slave implementations are for the
> functionality that uses the drivers, not the drivers themselves.
> 
OK, I'll fix it, thanks.

> > +	/* set the tx/rx endian */
> > +#ifdef __LITTLE_ENDIAN
> > +	reg_val &= ~SPIS_TX_ENDIAN;
> > +	reg_val &= ~SPIS_RX_ENDIAN;
> > +#else
> > +	reg_val |= SPIS_TX_ENDIAN;
> > +	reg_val |= SPIS_RX_ENDIAN;
> > +#endif
> > +	writel(reg_val, mdata->base + SPIS_CFG_REG);
> 
> This seems weird - it looks like it's changing the endianess of the
> protocol based on the endianness the processor is running in.  What's
> going on here?  I'd expect the driver to be just treating everything as
> a byte stream and letting the protocol driver handle the endianness
> issues, otherwise we might end up with double converions.
> 
OK, I'll set the endian of SPI the same with the processor. Thanks.

> > +		xfer->tx_dma = dma_map_single(dev, (void *)xfer->tx_buf,
> > +					      xfer->len, DMA_TO_DEVICE);
> 
> Why is there a cast to void here?  That's usually a sign that there's a
> type safety issue, the whole point with void is that it's compatible
> with any other pointer.
> 
tx_buf is a const void*, here it need a void * for the dma mapping.
And I'll remove void * from dma_map_single((void *)rx_buf).
Thanks.

> > +static irqreturn_t mtk_spi_slave_interrupt(int irq, void *dev_id)
> > +{
> > +	struct spi_controller *ctlr = dev_id;
> > +	struct mtk_spi_slave *mdata = spi_controller_get_devdata(ctlr);
> > +	struct spi_transfer *trans = mdata->cur_transfer;
> > +	u32 int_status, reg_val, cnt, remainder;
> > +
> > +	int_status = readl(mdata->base + SPIS_IRQ_ST_REG);
> > +	writel(int_status, mdata->base + SPIS_IRQ_CLR_REG);
> > +
> > +	if (!trans)
> > +		return IRQ_HANDLED;
> 
> Are you sure that this is the right thing to do if we get a spurious
> interrupt - the normal thing would be to return IRQ_NONE, possibly
> logging a warning as well?
> 
OK, it should reture IRQ_NONE here, thanks.

> > +	if (int_status & CMD_INVALID_ST)
> > +		dev_err(&ctlr->dev, "cmd invalid\n");
> 
> If there's an interrupt that doesn't have any of the interrupt status
> flags set I'd expect to see a warning and probably IRQ_NONE being
> returned.  That way if the interrupt line is shared we work correctly
> and if something goes wrong and the interrupt gets stuck on then the
> core interrupt code's error handling can kick in.
OK, I'll fix it, thanks.
diff mbox series

Patch

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 671d078..86ef6a5 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -802,6 +802,14 @@  config SPI_SLAVE
 	  slave protocol handlers.
 
 if SPI_SLAVE
+config SPI_SLAVE_MT27XX
+	tristate "MediaTek SPI slave device"
+	depends on ARCH_MEDIATEK || COMPILE_TEST
+	help
+	  This selects the MediaTek(R) SPI slave device driver.
+	  If you want to use MediaTek(R) SPI slave interface,
+	  say Y or M here.If you are not sure, say N.
+	  SPI slave drivers for Mediatek MT27XX series ARM SoCs.
 
 config SPI_SLAVE_TIME
 	tristate "SPI slave handler reporting boot up time"
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index a90d559..a5e7b3c 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -109,5 +109,6 @@  obj-$(CONFIG_SPI_XTENSA_XTFPGA)		+= spi-xtensa-xtfpga.o
 obj-$(CONFIG_SPI_ZYNQMP_GQSPI)		+= spi-zynqmp-gqspi.o
 
 # SPI slave protocol handlers
+obj-$(CONFIG_SPI_SLAVE_MT27XX)		+= spi-slave-mt27xx.o
 obj-$(CONFIG_SPI_SLAVE_TIME)		+= spi-slave-time.o
 obj-$(CONFIG_SPI_SLAVE_SYSTEM_CONTROL)	+= spi-slave-system-control.o
diff --git a/drivers/spi/spi-slave-mt27xx.c b/drivers/spi/spi-slave-mt27xx.c
new file mode 100644
index 0000000..2e59339
--- /dev/null
+++ b/drivers/spi/spi-slave-mt27xx.c
@@ -0,0 +1,553 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (c) 2018 MediaTek Inc.
+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/spi/spi.h>
+
+#define SPIS_IRQ_EN_REG		0x0
+#define SPIS_IRQ_CLR_REG	0x4
+#define SPIS_IRQ_ST_REG		0x8
+#define SPIS_IRQ_MASK_REG	0xc
+#define SPIS_CFG_REG		0x10
+#define SPIS_RX_DATA_REG	0x14
+#define SPIS_TX_DATA_REG	0x18
+#define SPIS_RX_DST_REG		0x1c
+#define SPIS_TX_SRC_REG		0x20
+#define SPIS_DMA_CFG_REG	0x30
+#define SPIS_SOFT_RST_REG	0x40
+
+/* SPIS_IRQ_EN_REG */
+#define DMA_DONE_EN		BIT(7)
+#define DATA_DONE_EN		BIT(2)
+#define RSTA_DONE_EN		BIT(1)
+#define CMD_INVALID_EN		BIT(0)
+
+/* SPIS_IRQ_ST_REG */
+#define DMA_DONE_ST		BIT(7)
+#define DATA_DONE_ST		BIT(2)
+#define RSTA_DONE_ST		BIT(1)
+#define CMD_INVALID_ST		BIT(0)
+
+/* SPIS_IRQ_MASK_REG */
+#define DMA_DONE_MASK		BIT(7)
+#define DATA_DONE_MASK		BIT(2)
+#define RSTA_DONE_MASK		BIT(1)
+#define CMD_INVALID_MASK	BIT(0)
+
+/* SPIS_CFG_REG */
+#define SPIS_TX_ENDIAN		BIT(7)
+#define SPIS_RX_ENDIAN		BIT(6)
+#define SPIS_TXMSBF		BIT(5)
+#define SPIS_RXMSBF		BIT(4)
+#define SPIS_CPHA		BIT(3)
+#define SPIS_CPOL		BIT(2)
+#define SPIS_TX_EN		BIT(1)
+#define SPIS_RX_EN		BIT(0)
+
+/* SPIS_DMA_CFG_REG */
+#define TX_DMA_TRIG_EN		BIT(31)
+#define TX_DMA_EN		BIT(30)
+#define RX_DMA_EN		BIT(29)
+#define TX_DMA_LEN		0xfffff
+
+/* SPIS_SOFT_RST_REG */
+#define SPIS_DMA_ADDR_EN	BIT(1)
+#define SPIS_SOFT_RST		BIT(0)
+
+#define MTK_SPI_SLAVE_MAX_FIFO_SIZE 512U
+
+struct mtk_spi_slave {
+	struct device *dev;
+	void __iomem *base;
+	struct clk *spi_clk;
+	struct completion xfer_done;
+	struct spi_transfer *cur_transfer;
+	bool slave_aborted;
+};
+
+static const struct of_device_id mtk_spi_slave_of_match[] = {
+	{ .compatible = "mediatek,mt2712-spi-slave", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, mtk_spi_slave_of_match);
+
+static void mtk_spi_slave_disable_dma(struct mtk_spi_slave *mdata)
+{
+	u32 reg_val;
+
+	reg_val = readl(mdata->base + SPIS_DMA_CFG_REG);
+	reg_val &= ~RX_DMA_EN;
+	reg_val &= ~TX_DMA_EN;
+	writel(reg_val, mdata->base + SPIS_DMA_CFG_REG);
+}
+
+static void mtk_spi_slave_disable_xfer(struct mtk_spi_slave *mdata)
+{
+	u32 reg_val;
+
+	reg_val = readl(mdata->base + SPIS_CFG_REG);
+	reg_val &= ~SPIS_TX_EN;
+	reg_val &= ~SPIS_RX_EN;
+	writel(reg_val, mdata->base + SPIS_CFG_REG);
+}
+
+static int mtk_spi_slave_wait_for_completion(struct mtk_spi_slave *mdata)
+{
+	if (wait_for_completion_interruptible(&mdata->xfer_done) ||
+	    mdata->slave_aborted) {
+		dev_err(mdata->dev, "interrupted\n");
+		return -EINTR;
+	}
+
+	return 0;
+}
+
+static int mtk_spi_slave_prepare_message(struct spi_controller *ctlr,
+					 struct spi_message *msg)
+{
+	struct mtk_spi_slave *mdata = spi_controller_get_devdata(ctlr);
+	struct spi_device *spi = msg->spi;
+	bool cpha, cpol;
+	u32 reg_val;
+
+	cpha = spi->mode & SPI_CPHA ? 1 : 0;
+	cpol = spi->mode & SPI_CPOL ? 1 : 0;
+
+	reg_val = readl(mdata->base + SPIS_CFG_REG);
+	if (cpha)
+		reg_val |= SPIS_CPHA;
+	else
+		reg_val &= ~SPIS_CPHA;
+	if (cpol)
+		reg_val |= SPIS_CPOL;
+	else
+		reg_val &= ~SPIS_CPOL;
+
+	if (spi->mode & SPI_LSB_FIRST)
+		reg_val &= ~(SPIS_TXMSBF | SPIS_RXMSBF);
+	else
+		reg_val |= SPIS_TXMSBF | SPIS_RXMSBF;
+
+	/* set the tx/rx endian */
+#ifdef __LITTLE_ENDIAN
+	reg_val &= ~SPIS_TX_ENDIAN;
+	reg_val &= ~SPIS_RX_ENDIAN;
+#else
+	reg_val |= SPIS_TX_ENDIAN;
+	reg_val |= SPIS_RX_ENDIAN;
+#endif
+	writel(reg_val, mdata->base + SPIS_CFG_REG);
+
+	return 0;
+}
+
+static int mtk_spi_slave_fifo_transfer(struct spi_controller *ctlr,
+				       struct spi_device *spi,
+				       struct spi_transfer *xfer)
+{
+	struct mtk_spi_slave *mdata = spi_controller_get_devdata(ctlr);
+	int reg_val, cnt, remainder, ret;
+
+	writel(SPIS_SOFT_RST, mdata->base + SPIS_SOFT_RST_REG);
+
+	reg_val = readl(mdata->base + SPIS_CFG_REG);
+	if (xfer->rx_buf)
+		reg_val |= SPIS_RX_EN;
+	if (xfer->tx_buf)
+		reg_val |= SPIS_TX_EN;
+	writel(reg_val, mdata->base + SPIS_CFG_REG);
+
+	cnt = xfer->len / 4;
+	if (xfer->tx_buf)
+		iowrite32_rep(mdata->base + SPIS_TX_DATA_REG,
+			      xfer->tx_buf, cnt);
+
+	remainder = xfer->len % 4;
+	if (xfer->tx_buf && remainder > 0) {
+		reg_val = 0;
+		memcpy(&reg_val, xfer->tx_buf + cnt * 4, remainder);
+		writel(reg_val, mdata->base + SPIS_TX_DATA_REG);
+	}
+
+	ret = mtk_spi_slave_wait_for_completion(mdata);
+	if (ret) {
+		mtk_spi_slave_disable_xfer(mdata);
+		writel(SPIS_SOFT_RST, mdata->base + SPIS_SOFT_RST_REG);
+	}
+
+	return ret;
+}
+
+static int mtk_spi_slave_dma_transfer(struct spi_controller *ctlr,
+				      struct spi_device *spi,
+				      struct spi_transfer *xfer)
+{
+	struct mtk_spi_slave *mdata = spi_controller_get_devdata(ctlr);
+	struct device *dev = mdata->dev;
+	int reg_val, ret;
+
+	writel(SPIS_SOFT_RST, mdata->base + SPIS_SOFT_RST_REG);
+
+	if (xfer->tx_buf) {
+		xfer->tx_dma = dma_map_single(dev, (void *)xfer->tx_buf,
+					      xfer->len, DMA_TO_DEVICE);
+		if (dma_mapping_error(dev, xfer->tx_dma)) {
+			ret = -ENOMEM;
+			goto disable_transfer;
+		}
+	}
+
+	if (xfer->rx_buf) {
+		xfer->rx_dma = dma_map_single(dev, (void *)xfer->rx_buf,
+					      xfer->len, DMA_FROM_DEVICE);
+		if (dma_mapping_error(dev, xfer->rx_dma)) {
+			ret = -ENOMEM;
+			goto unmap_txdma;
+		}
+	}
+
+	writel(xfer->tx_dma, mdata->base + SPIS_TX_SRC_REG);
+	writel(xfer->rx_dma, mdata->base + SPIS_RX_DST_REG);
+
+	writel(SPIS_DMA_ADDR_EN, mdata->base + SPIS_SOFT_RST_REG);
+
+	/* enable config reg tx rx_enable */
+	reg_val = readl(mdata->base + SPIS_CFG_REG);
+	if (xfer->tx_buf)
+		reg_val |= SPIS_TX_EN;
+	if (xfer->rx_buf)
+		reg_val |= SPIS_RX_EN;
+	writel(reg_val, mdata->base + SPIS_CFG_REG);
+
+	/* config dma */
+	reg_val = 0;
+	reg_val |= (xfer->len - 1) & TX_DMA_LEN;
+	writel(reg_val, mdata->base + SPIS_DMA_CFG_REG);
+
+	reg_val = readl(mdata->base + SPIS_DMA_CFG_REG);
+	if (xfer->tx_buf)
+		reg_val |= TX_DMA_EN;
+	if (xfer->rx_buf)
+		reg_val |= RX_DMA_EN;
+	reg_val |= TX_DMA_TRIG_EN;
+	writel(reg_val, mdata->base + SPIS_DMA_CFG_REG);
+
+	ret = mtk_spi_slave_wait_for_completion(mdata);
+	if (ret)
+		goto unmap_rxdma;
+
+	return 0;
+
+unmap_rxdma:
+	if (xfer->rx_buf)
+		dma_unmap_single(dev, xfer->rx_dma,
+				 xfer->len, DMA_FROM_DEVICE);
+
+unmap_txdma:
+	if (xfer->tx_buf)
+		dma_unmap_single(dev, xfer->tx_dma,
+				 xfer->len, DMA_TO_DEVICE);
+
+disable_transfer:
+	mtk_spi_slave_disable_dma(mdata);
+	mtk_spi_slave_disable_xfer(mdata);
+	writel(SPIS_SOFT_RST, mdata->base + SPIS_SOFT_RST_REG);
+
+	return ret;
+}
+
+static int mtk_spi_slave_transfer_one(struct spi_controller *ctlr,
+				      struct spi_device *spi,
+				      struct spi_transfer *xfer)
+{
+	struct mtk_spi_slave *mdata = spi_controller_get_devdata(ctlr);
+
+	reinit_completion(&mdata->xfer_done);
+	mdata->slave_aborted = false;
+	mdata->cur_transfer = xfer;
+
+	if (xfer->len > MTK_SPI_SLAVE_MAX_FIFO_SIZE)
+		return mtk_spi_slave_dma_transfer(ctlr, spi, xfer);
+	else
+		return mtk_spi_slave_fifo_transfer(ctlr, spi, xfer);
+}
+
+static int mtk_spi_slave_setup(struct spi_device *spi)
+{
+	struct mtk_spi_slave *mdata = spi_controller_get_devdata(spi->master);
+	u32 reg_val;
+
+	reg_val = DMA_DONE_EN | DATA_DONE_EN |
+		  RSTA_DONE_EN | CMD_INVALID_EN;
+	writel(reg_val, mdata->base + SPIS_IRQ_EN_REG);
+
+	reg_val = DMA_DONE_MASK | DATA_DONE_MASK |
+		  RSTA_DONE_MASK | CMD_INVALID_MASK;
+	writel(reg_val, mdata->base + SPIS_IRQ_MASK_REG);
+
+	mtk_spi_slave_disable_dma(mdata);
+	mtk_spi_slave_disable_xfer(mdata);
+
+	return 0;
+}
+
+static int mtk_slave_abort(struct spi_controller *ctlr)
+{
+	struct mtk_spi_slave *mdata = spi_controller_get_devdata(ctlr);
+
+	mdata->slave_aborted = true;
+	complete(&mdata->xfer_done);
+
+	return 0;
+}
+
+static irqreturn_t mtk_spi_slave_interrupt(int irq, void *dev_id)
+{
+	struct spi_controller *ctlr = dev_id;
+	struct mtk_spi_slave *mdata = spi_controller_get_devdata(ctlr);
+	struct spi_transfer *trans = mdata->cur_transfer;
+	u32 int_status, reg_val, cnt, remainder;
+
+	int_status = readl(mdata->base + SPIS_IRQ_ST_REG);
+	writel(int_status, mdata->base + SPIS_IRQ_CLR_REG);
+
+	if (!trans)
+		return IRQ_HANDLED;
+
+	if ((int_status & DMA_DONE_ST) &&
+	    ((int_status & DATA_DONE_ST) ||
+	    (int_status & RSTA_DONE_ST))) {
+		writel(SPIS_SOFT_RST, mdata->base + SPIS_SOFT_RST_REG);
+
+		if (trans->tx_buf)
+			dma_unmap_single(mdata->dev, trans->tx_dma,
+					 trans->len, DMA_TO_DEVICE);
+		if (trans->rx_buf)
+			dma_unmap_single(mdata->dev, trans->rx_dma,
+					 trans->len, DMA_FROM_DEVICE);
+
+		mtk_spi_slave_disable_dma(mdata);
+		mtk_spi_slave_disable_xfer(mdata);
+	}
+
+	if ((!(int_status & DMA_DONE_ST)) &&
+	    ((int_status & DATA_DONE_ST) ||
+	    (int_status & RSTA_DONE_ST))) {
+		cnt = trans->len / 4;
+		if (trans->rx_buf)
+			ioread32_rep(mdata->base + SPIS_RX_DATA_REG,
+				     trans->rx_buf, cnt);
+		remainder = trans->len % 4;
+		if (trans->rx_buf && remainder > 0) {
+			reg_val = readl(mdata->base + SPIS_RX_DATA_REG);
+			memcpy(trans->rx_buf + (cnt * 4),
+			       &reg_val, remainder);
+		}
+
+		mtk_spi_slave_disable_xfer(mdata);
+	}
+
+	if (int_status & CMD_INVALID_ST)
+		dev_err(&ctlr->dev, "cmd invalid\n");
+
+	mdata->cur_transfer = NULL;
+	complete(&mdata->xfer_done);
+
+	return IRQ_HANDLED;
+}
+
+static int mtk_spi_slave_probe(struct platform_device *pdev)
+{
+	struct spi_controller *ctlr;
+	struct mtk_spi_slave *mdata;
+	struct resource *res;
+	int irq, ret;
+
+	ctlr = spi_alloc_slave(&pdev->dev, sizeof(*mdata));
+	if (!ctlr) {
+		dev_err(&pdev->dev, "failed to alloc spi slave\n");
+		return -ENOMEM;
+	}
+
+	ctlr->auto_runtime_pm = true;
+	ctlr->dev.of_node = pdev->dev.of_node;
+	ctlr->mode_bits = SPI_CPOL | SPI_CPHA;
+	ctlr->mode_bits |= SPI_LSB_FIRST;
+
+	ctlr->prepare_message = mtk_spi_slave_prepare_message;
+	ctlr->transfer_one = mtk_spi_slave_transfer_one;
+	ctlr->setup = mtk_spi_slave_setup;
+	ctlr->slave_abort = mtk_slave_abort;
+
+	mdata = spi_controller_get_devdata(ctlr);
+
+	platform_set_drvdata(pdev, ctlr);
+
+	init_completion(&mdata->xfer_done);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		ret = -ENODEV;
+		dev_err(&pdev->dev, "failed to determine base address\n");
+		goto err_put_ctlr;
+	}
+
+	mdata->dev = &pdev->dev;
+
+	mdata->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(mdata->base)) {
+		ret = PTR_ERR(mdata->base);
+		goto err_put_ctlr;
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "failed to get irq (%d)\n", irq);
+		ret = irq;
+		goto err_put_ctlr;
+	}
+
+	ret = devm_request_irq(&pdev->dev, irq, mtk_spi_slave_interrupt,
+			       IRQF_TRIGGER_NONE, dev_name(&pdev->dev), ctlr);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to register irq (%d)\n", ret);
+		goto err_put_ctlr;
+	}
+
+	mdata->spi_clk = devm_clk_get(&pdev->dev, "spi");
+	if (IS_ERR(mdata->spi_clk)) {
+		ret = PTR_ERR(mdata->spi_clk);
+		dev_err(&pdev->dev, "failed to get spi-clk: %d\n", ret);
+		goto err_put_ctlr;
+	}
+
+	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_ctlr;
+	}
+
+	pm_runtime_enable(&pdev->dev);
+
+	ret = devm_spi_register_controller(&pdev->dev, ctlr);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"failed to register slave controller(%d)\n", ret);
+		clk_disable_unprepare(mdata->spi_clk);
+		goto err_disable_runtime_pm;
+	}
+
+	clk_disable_unprepare(mdata->spi_clk);
+
+	return 0;
+
+err_disable_runtime_pm:
+	pm_runtime_disable(&pdev->dev);
+err_put_ctlr:
+	spi_controller_put(ctlr);
+
+	return ret;
+}
+
+static int mtk_spi_slave_remove(struct platform_device *pdev)
+{
+	pm_runtime_disable(&pdev->dev);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int mtk_spi_slave_suspend(struct device *dev)
+{
+	struct spi_controller *ctlr = dev_get_drvdata(dev);
+	struct mtk_spi_slave *mdata = spi_controller_get_devdata(ctlr);
+	int ret;
+
+	ret = spi_controller_suspend(ctlr);
+	if (ret)
+		return ret;
+
+	if (!pm_runtime_suspended(dev))
+		clk_disable_unprepare(mdata->spi_clk);
+
+	return ret;
+}
+
+static int mtk_spi_slave_resume(struct device *dev)
+{
+	struct spi_controller *ctlr = dev_get_drvdata(dev);
+	struct mtk_spi_slave *mdata = spi_controller_get_devdata(ctlr);
+	int ret;
+
+	if (!pm_runtime_suspended(dev)) {
+		ret = clk_prepare_enable(mdata->spi_clk);
+		if (ret < 0) {
+			dev_err(dev, "failed to enable spi_clk (%d)\n", ret);
+			return ret;
+		}
+	}
+
+	ret = spi_controller_resume(ctlr);
+	if (ret < 0)
+		clk_disable_unprepare(mdata->spi_clk);
+
+	return ret;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+#ifdef CONFIG_PM
+static int mtk_spi_slave_runtime_suspend(struct device *dev)
+{
+	struct spi_controller *ctlr = dev_get_drvdata(dev);
+	struct mtk_spi_slave *mdata = spi_controller_get_devdata(ctlr);
+
+	clk_disable_unprepare(mdata->spi_clk);
+
+	return 0;
+}
+
+static int mtk_spi_slave_runtime_resume(struct device *dev)
+{
+	struct spi_controller *ctlr = dev_get_drvdata(dev);
+	struct mtk_spi_slave *mdata = spi_controller_get_devdata(ctlr);
+	int ret;
+
+	ret = clk_prepare_enable(mdata->spi_clk);
+	if (ret < 0) {
+		dev_err(dev, "failed to enable spi_clk (%d)\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+#endif /* CONFIG_PM */
+
+static const struct dev_pm_ops mtk_spi_slave_pm = {
+	SET_SYSTEM_SLEEP_PM_OPS(mtk_spi_slave_suspend, mtk_spi_slave_resume)
+	SET_RUNTIME_PM_OPS(mtk_spi_slave_runtime_suspend,
+			   mtk_spi_slave_runtime_resume, NULL)
+};
+
+static struct platform_driver mtk_spi_slave_driver = {
+	.driver = {
+		.name = "mtk-spi-slave",
+		.pm	= &mtk_spi_slave_pm,
+		.of_match_table = mtk_spi_slave_of_match,
+	},
+	.probe = mtk_spi_slave_probe,
+	.remove = mtk_spi_slave_remove,
+};
+
+module_platform_driver(mtk_spi_slave_driver);
+
+MODULE_DESCRIPTION("MTK SPI Slave Controller driver");
+MODULE_AUTHOR("Leilk Liu <leilk.liu@mediatek.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:mtk-spi-slave");