2015-08-07 07:20:29

by Leilk Liu

[permalink] [raw]
Subject: [PATCH v5 0/3] Add Mediatek SPI bus driver

This series are based on 4.2-rc1 and provide three patches to add mediatek spi driver.

Change in v5:
1. add changelogs in the individual patches.
2. modify clk relevant implement.
3. describe dt-binding document in more detail.

Change in v4:
1. fix Mark Brown review comment.

Change in v3:
1. support fifo mode function.
2. support any non-prime length transfer in dma mode.
3. use interrupt to handle scatterlist.
4. fix review comment.

Change in v2:
1. Remove spi bitbang framwork, use can_dma() and transfer_one().
2. Add PM functions.
3. Fix Mark Brown review comment.

Leilk Liu (3):
spi: Mediatek: Document devicetree bindings for spi bus
spi: mediatek: Add spi bus for Mediatek MT8173
arm64: dts: Add spi bus dts

.../devicetree/bindings/spi/spi-mt65xx.txt | 51 ++
arch/arm64/boot/dts/mediatek/mt8173.dtsi | 23 +
drivers/spi/Kconfig | 9 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-mt65xx.c | 749 +++++++++++++++++++++
include/linux/platform_data/spi-mt65xx.h | 22 +
6 files changed, 855 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/spi-mt65xx.txt
create mode 100644 drivers/spi/spi-mt65xx.c
create mode 100644 include/linux/platform_data/spi-mt65xx.h

--
1.8.1.1.dirty


2015-08-07 07:20:34

by Leilk Liu

[permalink] [raw]
Subject: [PATCH v5 1/3] spi: Mediatek: Document devicetree bindings for spi bus

Signed-off-by: Leilk Liu <[email protected]>
---
Change in this patch:
1. change this patch title.
2. change "MTK SPI device" to "MTK SPI controller".
3. "pad-select" is a vendor property, so change it to "mediatek,pad-select".
4. modify the property of clock and clock name.
5. explain what the pad-select values 0-3 mean.
---
.../devicetree/bindings/spi/spi-mt65xx.txt | 51 ++++++++++++++++++++++
1 file changed, 51 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/spi-mt65xx.txt

diff --git a/Documentation/devicetree/bindings/spi/spi-mt65xx.txt b/Documentation/devicetree/bindings/spi/spi-mt65xx.txt
new file mode 100644
index 0000000..dcefc43
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-mt65xx.txt
@@ -0,0 +1,51 @@
+Binding for MTK SPI controller
+
+Required properties:
+- compatible: should be one of the following.
+ - mediatek,mt8173-spi: for mt8173 platforms
+ - mediatek,mt8135-spi: for mt8135 platforms
+ - mediatek,mt6589-spi: for mt6589 platforms
+
+- #address-cells: should be 1.
+
+- #size-cells: should be 0.
+
+- reg: Address and length of the register set for the device
+
+- interrupts: Should contain spi interrupt
+
+- clocks: phandles to input clocks.
+ The first should be <&topckgen CLK_TOP_SPI_SEL>.
+ The second should be one of the following.
+ - <&clk26m>: specify parent clock 26MHZ.
+ - <&topckgen CLK_TOP_SYSPLL3_D2>: specify parent clock 109MHZ.
+ It's the default one.
+ - <&topckgen CLK_TOP_SYSPLL4_D2>: specify parent clock 78MHZ.
+ - <&topckgen CLK_TOP_UNIVPLL2_D4>: specify parent clock 104MHZ.
+ - <&topckgen CLK_TOP_UNIVPLL1_D8>: specify parent clock 78MHZ.
+
+- clock-names: shall be "spi-clk" for the controller clock, and
+ "parent-clk" for the parent clock.
+
+Optional properties:
+- mediatek,pad-select: specify which pins group(ck/mi/mo/cs) spi
+ controller used, this value should be 0~3, only required for MT8173.
+ 0: specify GPIO69,70,71,72 for spi pins.
+ 1: specify GPIO102,103,104,105 for spi pins.
+ 2: specify GPIO128,129,130,131 for spi pins.
+ 3: specify GPIO5,6,7,8 for spi pins.
+
+Example:
+
+- SoC Specific Portion:
+spi: spi@1100a000 {
+ compatible = "mediatek,mt8173-spi";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0 0x1100a000 0 0x1000>;
+ interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&topckgen CLK_TOP_SPI_SEL>, <&topckgen CLK_TOP_SYSPLL3_D2>;
+ clock-names = "spi-clk", "parent-clk";
+ mediatek,pad-select = <0>;
+ status = "disabled";
+};
--
1.8.1.1.dirty

2015-08-07 07:21:59

by Leilk Liu

[permalink] [raw]
Subject: [PATCH v5 2/3] spi: mediatek: Add spi bus for Mediatek MT8173

This patch adds basic spi bus for MT8173.

Signed-off-by: Leilk Liu <[email protected]>
---
Change in this patch:
1. change "pad-select" to "mediatek,pad-select".
2. modify clk relevant implement.
---
drivers/spi/Kconfig | 9 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-mt65xx.c | 749 +++++++++++++++++++++++++++++++
include/linux/platform_data/spi-mt65xx.h | 22 +
4 files changed, 781 insertions(+)
create mode 100644 drivers/spi/spi-mt65xx.c
create mode 100644 include/linux/platform_data/spi-mt65xx.h

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 0cae169..38ddfba 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -326,6 +326,15 @@ config SPI_MESON_SPIFC
This enables master mode support for the SPIFC (SPI flash
controller) available in Amlogic Meson SoCs.

+config SPI_MT65XX
+ tristate "MediaTek SPI controller"
+ depends on ARCH_MEDIATEK || COMPILE_TEST
+ help
+ This selects the MediaTek(R) SPI bus driver.
+ If you want to use MediaTek(R) SPI interface,
+ say Y or M here.If you are not sure, say N.
+ SPI drivers for Mediatek MT65XX and MT81XX series ARM SoCs.
+
config SPI_OC_TINY
tristate "OpenCores tiny SPI"
depends on GPIOLIB || COMPILE_TEST
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 1154dba..9746beb2 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_SPI_MESON_SPIFC) += spi-meson-spifc.o
obj-$(CONFIG_SPI_MPC512x_PSC) += spi-mpc512x-psc.o
obj-$(CONFIG_SPI_MPC52xx_PSC) += spi-mpc52xx-psc.o
obj-$(CONFIG_SPI_MPC52xx) += spi-mpc52xx.o
+obj-$(CONFIG_SPI_MT65XX) += spi-mt65xx.o
obj-$(CONFIG_SPI_MXS) += spi-mxs.o
obj-$(CONFIG_SPI_NUC900) += spi-nuc900.o
obj-$(CONFIG_SPI_OC_TINY) += spi-oc-tiny.o
diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
new file mode 100644
index 0000000..4676b01
--- /dev/null
+++ b/drivers/spi/spi-mt65xx.c
@@ -0,0 +1,749 @@
+/*
+ * Copyright (c) 2015 MediaTek Inc.
+ * Author: Leilk Liu <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/ioport.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/platform_data/spi-mt65xx.h>
+#include <linux/pm_runtime.h>
+#include <linux/spi/spi.h>
+
+#define SPI_CFG0_REG 0x0000
+#define SPI_CFG1_REG 0x0004
+#define SPI_TX_SRC_REG 0x0008
+#define SPI_RX_DST_REG 0x000c
+#define SPI_TX_DATA_REG 0x0010
+#define SPI_RX_DATA_REG 0x0014
+#define SPI_CMD_REG 0x0018
+#define SPI_STATUS0_REG 0x001c
+#define SPI_PAD_SEL_REG 0x0024
+
+#define SPI_CFG0_SCK_HIGH_OFFSET 0
+#define SPI_CFG0_SCK_LOW_OFFSET 8
+#define SPI_CFG0_CS_HOLD_OFFSET 16
+#define SPI_CFG0_CS_SETUP_OFFSET 24
+
+#define SPI_CFG1_CS_IDLE_OFFSET 0
+#define SPI_CFG1_PACKET_LOOP_OFFSET 8
+#define SPI_CFG1_PACKET_LENGTH_OFFSET 16
+#define SPI_CFG1_GET_TICK_DLY_OFFSET 30
+
+#define SPI_CFG1_CS_IDLE_MASK 0xff
+#define SPI_CFG1_PACKET_LOOP_MASK 0xff00
+#define SPI_CFG1_PACKET_LENGTH_MASK 0x3ff0000
+
+#define SPI_CMD_ACT_OFFSET 0
+#define SPI_CMD_RESUME_OFFSET 1
+#define SPI_CMD_CPHA_OFFSET 8
+#define SPI_CMD_CPOL_OFFSET 9
+#define SPI_CMD_TXMSBF_OFFSET 12
+#define SPI_CMD_RXMSBF_OFFSET 13
+#define SPI_CMD_RX_ENDIAN_OFFSET 14
+#define SPI_CMD_TX_ENDIAN_OFFSET 15
+
+#define SPI_CMD_RST BIT(2)
+#define SPI_CMD_PAUSE_EN BIT(4)
+#define SPI_CMD_DEASSERT BIT(5)
+#define SPI_CMD_CPHA BIT(8)
+#define SPI_CMD_CPOL BIT(9)
+#define SPI_CMD_RX_DMA BIT(10)
+#define SPI_CMD_TX_DMA BIT(11)
+#define SPI_CMD_TXMSBF BIT(12)
+#define SPI_CMD_RXMSBF BIT(13)
+#define SPI_CMD_RX_ENDIAN BIT(14)
+#define SPI_CMD_TX_ENDIAN BIT(15)
+#define SPI_CMD_FINISH_IE BIT(16)
+#define SPI_CMD_PAUSE_IE BIT(17)
+
+#define MTK_SPI_QUIRK_PAD_SELECT 1
+/* Must explicitly send dummy Tx bytes to do Rx only transfer */
+#define MTK_SPI_QUIRK_MUST_TX 1
+
+#define MT8173_SPI_MAX_PAD_SEL 3
+
+#define MTK_SPI_IDLE 0
+#define MTK_SPI_PAUSED 1
+
+#define MTK_SPI_MAX_FIFO_SIZE 32
+#define MTK_SPI_PACKET_SIZE 1024
+
+struct mtk_spi_compatible {
+ u32 need_pad_sel;
+ u32 must_tx;
+};
+
+struct mtk_spi {
+ void __iomem *base;
+ u32 state;
+ u32 pad_sel;
+ struct clk *spi_clk, *parent_clk;
+ struct spi_transfer *cur_transfer;
+ u32 xfer_len;
+ struct scatterlist *tx_sgl, *rx_sgl;
+ u32 tx_sgl_len, rx_sgl_len;
+ const struct mtk_spi_compatible *dev_comp;
+};
+
+static const struct mtk_spi_compatible mt6589_compat = {
+ .need_pad_sel = 0,
+ .must_tx = 0,
+};
+
+static const struct mtk_spi_compatible mt8135_compat = {
+ .need_pad_sel = 0,
+ .must_tx = 0,
+};
+
+static const struct mtk_spi_compatible mt8173_compat = {
+ .need_pad_sel = MTK_SPI_QUIRK_PAD_SELECT,
+ .must_tx = MTK_SPI_QUIRK_MUST_TX,
+};
+
+/*
+ * A piece of default chip info unless the platform
+ * supplies it.
+ */
+static const struct mtk_chip_config mtk_default_chip_info = {
+ .rx_mlsb = 1,
+ .tx_mlsb = 1,
+ .tx_endian = 0,
+ .rx_endian = 0,
+};
+
+static const struct of_device_id mtk_spi_of_match[] = {
+ { .compatible = "mediatek,mt6589-spi", .data = (void *)&mt6589_compat },
+ { .compatible = "mediatek,mt8135-spi", .data = (void *)&mt8135_compat },
+ { .compatible = "mediatek,mt8173-spi", .data = (void *)&mt8173_compat },
+ {}
+};
+MODULE_DEVICE_TABLE(of, mtk_spi_of_match);
+
+static void mtk_spi_reset(struct mtk_spi *mdata)
+{
+ u32 reg_val;
+
+ /* set the software reset bit in SPI_CMD_REG. */
+ reg_val = readl(mdata->base + SPI_CMD_REG);
+ reg_val |= SPI_CMD_RST;
+ writel(reg_val, mdata->base + SPI_CMD_REG);
+
+ reg_val = readl(mdata->base + SPI_CMD_REG);
+ reg_val &= ~SPI_CMD_RST;
+ writel(reg_val, mdata->base + SPI_CMD_REG);
+}
+
+static void mtk_spi_config(struct mtk_spi *mdata,
+ struct mtk_chip_config *chip_config)
+{
+ u32 reg_val;
+
+ reg_val = readl(mdata->base + SPI_CMD_REG);
+
+ /* set the mlsbx and mlsbtx */
+ reg_val &= ~(SPI_CMD_TXMSBF | SPI_CMD_RXMSBF);
+ reg_val |= (chip_config->tx_mlsb << SPI_CMD_TXMSBF_OFFSET);
+ reg_val |= (chip_config->rx_mlsb << SPI_CMD_RXMSBF_OFFSET);
+
+ /* set the tx/rx endian */
+ reg_val &= ~(SPI_CMD_TX_ENDIAN | SPI_CMD_RX_ENDIAN);
+ reg_val |= (chip_config->tx_endian << SPI_CMD_TX_ENDIAN_OFFSET);
+ reg_val |= (chip_config->rx_endian << SPI_CMD_RX_ENDIAN_OFFSET);
+
+ /* set finish and pause interrupt always enable */
+ reg_val |= SPI_CMD_FINISH_IE | SPI_CMD_PAUSE_EN;
+
+ /* disable dma mode */
+ reg_val &= ~(SPI_CMD_TX_DMA | SPI_CMD_RX_DMA);
+
+ /* disable deassert mode */
+ reg_val &= ~SPI_CMD_DEASSERT;
+
+ writel(reg_val, mdata->base + SPI_CMD_REG);
+
+ /* pad select */
+ if (mdata->dev_comp->need_pad_sel)
+ writel(mdata->pad_sel, mdata->base + SPI_PAD_SEL_REG);
+}
+
+static int mtk_spi_prepare_hardware(struct spi_master *master)
+{
+ struct spi_transfer *trans;
+ struct mtk_spi *mdata = spi_master_get_devdata(master);
+ struct spi_message *msg = master->cur_msg;
+ int ret;
+
+ ret = clk_prepare_enable(mdata->spi_clk);
+ if (ret < 0) {
+ dev_err(&master->dev, "failed to enable clock (%d)\n", ret);
+ return ret;
+ }
+
+ trans = list_first_entry(&msg->transfers, struct spi_transfer,
+ transfer_list);
+ if (trans->cs_change == 0) {
+ mdata->state = MTK_SPI_IDLE;
+ mtk_spi_reset(mdata);
+ }
+
+ return ret;
+}
+
+static int mtk_spi_unprepare_hardware(struct spi_master *master)
+{
+ struct mtk_spi *mdata = spi_master_get_devdata(master);
+
+ clk_disable_unprepare(mdata->spi_clk);
+
+ return 0;
+}
+
+static int mtk_spi_prepare_message(struct spi_master *master,
+ struct spi_message *msg)
+{
+ u32 reg_val;
+ u8 cpha, cpol;
+ struct mtk_chip_config *chip_config;
+ struct spi_device *spi = msg->spi;
+ struct mtk_spi *mdata = spi_master_get_devdata(master);
+
+ cpha = spi->mode & SPI_CPHA ? 1 : 0;
+ cpol = spi->mode & SPI_CPOL ? 1 : 0;
+
+ reg_val = readl(mdata->base + SPI_CMD_REG);
+ reg_val &= ~(SPI_CMD_CPHA | SPI_CMD_CPOL);
+ reg_val |= (cpha << SPI_CMD_CPHA_OFFSET);
+ reg_val |= (cpol << SPI_CMD_CPOL_OFFSET);
+ writel(reg_val, mdata->base + SPI_CMD_REG);
+
+ chip_config = spi->controller_data;
+ if (!chip_config) {
+ chip_config = (void *)&mtk_default_chip_info;
+ spi->controller_data = chip_config;
+ }
+ mtk_spi_config(mdata, chip_config);
+
+ return 0;
+}
+
+static void mtk_spi_set_cs(struct spi_device *spi, bool enable)
+{
+ u32 reg_val;
+ struct mtk_spi *mdata = spi_master_get_devdata(spi->master);
+
+ reg_val = readl(mdata->base + SPI_CMD_REG);
+ if (!enable)
+ reg_val |= SPI_CMD_PAUSE_EN;
+ else
+ reg_val &= ~SPI_CMD_PAUSE_EN;
+ writel(reg_val, mdata->base + SPI_CMD_REG);
+}
+
+static void mtk_spi_prepare_transfer(struct spi_master *master,
+ struct spi_transfer *xfer)
+{
+ u32 spi_clk_hz, div, high_time, low_time, holdtime,
+ setuptime, cs_idletime, reg_val = 0;
+ struct mtk_spi *mdata = spi_master_get_devdata(master);
+
+ spi_clk_hz = clk_get_rate(mdata->spi_clk);
+ if (xfer->speed_hz < spi_clk_hz / 2)
+ div = DIV_ROUND_UP(spi_clk_hz, xfer->speed_hz);
+ else
+ div = 1;
+
+ high_time = (div + 1) / 2;
+ low_time = (div + 1) / 2;
+ holdtime = (div + 1) / 2 * 2;
+ setuptime = (div + 1) / 2 * 2;
+ cs_idletime = (div + 1) / 2 * 2;
+
+ reg_val |= (((high_time - 1) & 0xff) << SPI_CFG0_SCK_HIGH_OFFSET);
+ reg_val |= (((low_time - 1) & 0xff) << SPI_CFG0_SCK_LOW_OFFSET);
+ reg_val |= (((holdtime - 1) & 0xff) << SPI_CFG0_CS_HOLD_OFFSET);
+ reg_val |= (((setuptime - 1) & 0xff) << SPI_CFG0_CS_SETUP_OFFSET);
+ writel(reg_val, mdata->base + SPI_CFG0_REG);
+
+ reg_val = readl(mdata->base + SPI_CFG1_REG);
+ reg_val &= ~SPI_CFG1_CS_IDLE_MASK;
+ reg_val |= (((cs_idletime - 1) & 0xff) << SPI_CFG1_CS_IDLE_OFFSET);
+ writel(reg_val, mdata->base + SPI_CFG1_REG);
+}
+
+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(unsigned, 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);
+ reg_val |= (packet_size - 1) << SPI_CFG1_PACKET_LENGTH_OFFSET;
+ reg_val |= (packet_loop - 1) << SPI_CFG1_PACKET_LOOP_OFFSET;
+ writel(reg_val, mdata->base + SPI_CFG1_REG);
+}
+
+static void mtk_spi_enable_transfer(struct spi_master *master)
+{
+ int cmd;
+ struct mtk_spi *mdata = spi_master_get_devdata(master);
+
+ cmd = readl(mdata->base + SPI_CMD_REG);
+ if (mdata->state == MTK_SPI_IDLE)
+ cmd |= 1 << SPI_CMD_ACT_OFFSET;
+ else
+ cmd |= 1 << SPI_CMD_RESUME_OFFSET;
+ writel(cmd, mdata->base + SPI_CMD_REG);
+}
+
+static int mtk_spi_get_mult_delta(int xfer_len)
+{
+ int mult_delta;
+
+ if (xfer_len > MTK_SPI_PACKET_SIZE)
+ mult_delta = xfer_len % MTK_SPI_PACKET_SIZE;
+ else
+ mult_delta = 0;
+
+ return mult_delta;
+}
+
+static void mtk_spi_update_mdata_len(struct spi_master *master)
+{
+ int mult_delta;
+ struct mtk_spi *mdata = spi_master_get_devdata(master);
+
+ if (mdata->tx_sgl_len && mdata->rx_sgl_len) {
+ if (mdata->tx_sgl_len > mdata->rx_sgl_len) {
+ mult_delta = mtk_spi_get_mult_delta(mdata->rx_sgl_len);
+ mdata->xfer_len = mdata->rx_sgl_len - mult_delta;
+ mdata->rx_sgl_len = mult_delta;
+ mdata->tx_sgl_len -= mdata->xfer_len;
+ } else {
+ mult_delta = mtk_spi_get_mult_delta(mdata->tx_sgl_len);
+ mdata->xfer_len = mdata->tx_sgl_len - mult_delta;
+ mdata->tx_sgl_len = mult_delta;
+ mdata->rx_sgl_len -= mdata->xfer_len;
+ }
+ } else if (mdata->tx_sgl_len) {
+ mult_delta = mtk_spi_get_mult_delta(mdata->tx_sgl_len);
+ mdata->xfer_len = mdata->tx_sgl_len - mult_delta;
+ mdata->tx_sgl_len = mult_delta;
+ } else if (mdata->rx_sgl_len) {
+ mult_delta = mtk_spi_get_mult_delta(mdata->rx_sgl_len);
+ mdata->xfer_len = mdata->rx_sgl_len - mult_delta;
+ mdata->rx_sgl_len = mult_delta;
+ }
+}
+
+static void mtk_spi_setup_dma_addr(struct spi_master *master,
+ struct spi_transfer *xfer)
+{
+ struct mtk_spi *mdata = spi_master_get_devdata(master);
+
+ if (mdata->tx_sgl)
+ writel(cpu_to_le32(xfer->tx_dma), mdata->base + SPI_TX_SRC_REG);
+ if (mdata->rx_sgl)
+ writel(cpu_to_le32(xfer->rx_dma), mdata->base + SPI_RX_DST_REG);
+}
+
+static int mtk_spi_fifo_transfer(struct spi_master *master,
+ struct spi_device *spi,
+ struct spi_transfer *xfer)
+{
+ int cnt, i;
+ struct mtk_spi *mdata = spi_master_get_devdata(master);
+
+ mdata->cur_transfer = xfer;
+ mdata->xfer_len = xfer->len;
+ mtk_spi_prepare_transfer(master, xfer);
+ mtk_spi_setup_packet(master);
+
+ if (xfer->len % 4)
+ cnt = xfer->len / 4 + 1;
+ else
+ cnt = xfer->len / 4;
+
+ for (i = 0; i < cnt; i++)
+ writel(*((u32 *)xfer->tx_buf + i),
+ mdata->base + SPI_TX_DATA_REG);
+
+ mtk_spi_enable_transfer(master);
+
+ return 1;
+}
+
+static int mtk_spi_dma_transfer(struct spi_master *master,
+ struct spi_device *spi,
+ struct spi_transfer *xfer)
+{
+ int cmd;
+ struct mtk_spi *mdata = spi_master_get_devdata(master);
+
+ mdata->tx_sgl = NULL;
+ mdata->rx_sgl = NULL;
+ mdata->tx_sgl_len = 0;
+ mdata->rx_sgl_len = 0;
+ mdata->cur_transfer = xfer;
+
+ mtk_spi_prepare_transfer(master, xfer);
+
+ cmd = readl(mdata->base + SPI_CMD_REG);
+ if (xfer->tx_buf)
+ cmd |= SPI_CMD_TX_DMA;
+ if (xfer->rx_buf)
+ cmd |= SPI_CMD_RX_DMA;
+ writel(cmd, mdata->base + SPI_CMD_REG);
+
+ if (xfer->tx_buf)
+ mdata->tx_sgl = xfer->tx_sg.sgl;
+ if (xfer->rx_buf)
+ mdata->rx_sgl = xfer->rx_sg.sgl;
+
+ if (mdata->tx_sgl) {
+ xfer->tx_dma = sg_dma_address(mdata->tx_sgl);
+ mdata->tx_sgl_len = sg_dma_len(mdata->tx_sgl);
+ }
+ if (mdata->rx_sgl) {
+ xfer->rx_dma = sg_dma_address(mdata->rx_sgl);
+ mdata->rx_sgl_len = sg_dma_len(mdata->rx_sgl);
+ }
+
+ mtk_spi_update_mdata_len(master);
+ mtk_spi_setup_packet(master);
+ mtk_spi_setup_dma_addr(master, xfer);
+ mtk_spi_enable_transfer(master);
+
+ return 1;
+}
+
+static int mtk_spi_transfer_one(struct spi_master *master,
+ struct spi_device *spi,
+ struct spi_transfer *xfer)
+{
+ if (master->can_dma(master, spi, xfer))
+ return mtk_spi_dma_transfer(master, spi, xfer);
+ else
+ return mtk_spi_fifo_transfer(master, spi, xfer);
+}
+
+static bool mtk_spi_can_dma(struct spi_master *master,
+ struct spi_device *spi,
+ struct spi_transfer *xfer)
+{
+ return xfer->len > MTK_SPI_MAX_FIFO_SIZE;
+}
+
+static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
+{
+ u32 cmd, reg_val, i;
+ struct spi_master *master = dev_id;
+ struct mtk_spi *mdata = spi_master_get_devdata(master);
+ struct spi_transfer *trans = mdata->cur_transfer;
+
+ reg_val = readl(mdata->base + SPI_STATUS0_REG);
+ if (reg_val & 0x2)
+ mdata->state = MTK_SPI_PAUSED;
+ else
+ mdata->state = MTK_SPI_IDLE;
+
+ if (!master->can_dma(master, master->cur_msg->spi, trans)) {
+ /* xfer len is not N*4 bytes every time in a transfer,
+ * but SPI_RX_DATA_REG must reads 4 bytes once,
+ * so rx buffer byte by byte.
+ */
+ if (trans->rx_buf) {
+ for (i = 0; i < mdata->xfer_len; i++) {
+ if (i % 4 == 0)
+ reg_val =
+ readl(mdata->base + SPI_RX_DATA_REG);
+ *((u8 *)(trans->rx_buf + i)) =
+ (reg_val >> ((i % 4) * 8)) & 0xff;
+ }
+ }
+ spi_finalize_current_transfer(master);
+ return IRQ_HANDLED;
+ }
+
+ if (mdata->tx_sgl)
+ trans->tx_dma += mdata->xfer_len;
+ if (mdata->rx_sgl)
+ trans->rx_dma += mdata->xfer_len;
+
+ if (mdata->tx_sgl && (mdata->tx_sgl_len == 0)) {
+ mdata->tx_sgl = sg_next(mdata->tx_sgl);
+ if (mdata->tx_sgl) {
+ trans->tx_dma = sg_dma_address(mdata->tx_sgl);
+ mdata->tx_sgl_len = sg_dma_len(mdata->tx_sgl);
+ }
+ }
+ if (mdata->rx_sgl && (mdata->rx_sgl_len == 0)) {
+ mdata->rx_sgl = sg_next(mdata->rx_sgl);
+ if (mdata->rx_sgl) {
+ trans->rx_dma = sg_dma_address(mdata->rx_sgl);
+ mdata->rx_sgl_len = sg_dma_len(mdata->rx_sgl);
+ }
+ }
+
+ if (!mdata->tx_sgl && !mdata->rx_sgl) {
+ /* spi disable dma */
+ cmd = readl(mdata->base + SPI_CMD_REG);
+ cmd &= ~SPI_CMD_TX_DMA;
+ cmd &= ~SPI_CMD_RX_DMA;
+ writel(cmd, mdata->base + SPI_CMD_REG);
+
+ spi_finalize_current_transfer(master);
+ return IRQ_HANDLED;
+ }
+
+ mtk_spi_update_mdata_len(master);
+ mtk_spi_setup_packet(master);
+ mtk_spi_setup_dma_addr(master, trans);
+ mtk_spi_enable_transfer(master);
+
+ return IRQ_HANDLED;
+}
+
+static int mtk_spi_probe(struct platform_device *pdev)
+{
+ struct spi_master *master;
+ struct mtk_spi *mdata;
+ const struct of_device_id *of_id;
+ struct resource *res;
+ int irq, ret;
+
+ master = spi_alloc_master(&pdev->dev, sizeof(*mdata));
+ if (!master) {
+ dev_err(&pdev->dev, "failed to alloc spi master\n");
+ return -ENOMEM;
+ }
+
+ master->auto_runtime_pm = true;
+ master->dev.of_node = pdev->dev.of_node;
+ master->mode_bits = SPI_CPOL | SPI_CPHA;
+
+ master->set_cs = mtk_spi_set_cs;
+ master->prepare_transfer_hardware = mtk_spi_prepare_hardware;
+ master->unprepare_transfer_hardware = mtk_spi_unprepare_hardware;
+ master->prepare_message = mtk_spi_prepare_message;
+ master->transfer_one = mtk_spi_transfer_one;
+ master->can_dma = mtk_spi_can_dma;
+
+ of_id = of_match_node(mtk_spi_of_match, pdev->dev.of_node);
+ if (!of_id) {
+ dev_err(&pdev->dev, "failed to probe of_node\n");
+ ret = -EINVAL;
+ goto err_put_master;
+ }
+
+ mdata = spi_master_get_devdata(master);
+ mdata->dev_comp = of_id->data;
+ if (mdata->dev_comp->must_tx)
+ master->flags = SPI_MASTER_MUST_TX;
+
+ if (mdata->dev_comp->need_pad_sel) {
+ ret = of_property_read_u32(pdev->dev.of_node,
+ "mediatek,pad-select",
+ &mdata->pad_sel);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to read pad select: %d\n",
+ ret);
+ goto err_put_master;
+ }
+
+ if (mdata->pad_sel > MT8173_SPI_MAX_PAD_SEL) {
+ dev_err(&pdev->dev, "wrong pad-select: %u\n",
+ mdata->pad_sel);
+ ret = -EINVAL;
+ goto err_put_master;
+ }
+ }
+
+ platform_set_drvdata(pdev, master);
+
+ 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_master;
+ }
+
+ mdata->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(mdata->base)) {
+ ret = PTR_ERR(mdata->base);
+ goto err_put_master;
+ }
+
+ 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_master;
+ }
+
+ if (!pdev->dev.dma_mask)
+ pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
+
+ ret = devm_request_irq(&pdev->dev, irq, mtk_spi_interrupt,
+ IRQF_TRIGGER_NONE, dev_name(&pdev->dev), master);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register irq (%d)\n", ret);
+ goto err_put_master;
+ }
+
+ mdata->spi_clk = devm_clk_get(&pdev->dev, "spi-clk");
+ 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_master;
+ }
+
+ mdata->parent_clk = devm_clk_get(&pdev->dev, "parent-clk");
+ if (IS_ERR(mdata->parent_clk)) {
+ ret = PTR_ERR(mdata->parent_clk);
+ dev_err(&pdev->dev, "failed to get parent-clk: %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;
+ }
+
+ ret = clk_set_parent(mdata->spi_clk, mdata->parent_clk);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "failed to clk_set_parent (%d)\n", ret);
+ goto err_disable_clk;
+ }
+
+ clk_disable_unprepare(mdata->spi_clk);
+
+ pm_runtime_enable(&pdev->dev);
+
+ ret = devm_spi_register_master(&pdev->dev, master);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register master (%d)\n", ret);
+ goto err_put_master;
+ }
+
+ return 0;
+
+err_disable_clk:
+ clk_disable_unprepare(mdata->spi_clk);
+err_put_master:
+ spi_master_put(master);
+
+ return ret;
+}
+
+static int mtk_spi_remove(struct platform_device *pdev)
+{
+ struct spi_master *master = platform_get_drvdata(pdev);
+ struct mtk_spi *mdata = spi_master_get_devdata(master);
+
+ pm_runtime_disable(&pdev->dev);
+
+ mtk_spi_reset(mdata);
+ clk_disable_unprepare(mdata->spi_clk);
+ spi_master_put(master);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int mtk_spi_suspend(struct device *dev)
+{
+ int ret;
+ struct spi_master *master = dev_get_drvdata(dev);
+ struct mtk_spi *mdata = spi_master_get_devdata(master);
+
+ ret = spi_master_suspend(master);
+ if (ret)
+ return ret;
+
+ if (!pm_runtime_suspended(dev))
+ clk_disable_unprepare(mdata->spi_clk);
+
+ return ret;
+}
+
+static int mtk_spi_resume(struct device *dev)
+{
+ int ret;
+ struct spi_master *master = dev_get_drvdata(dev);
+ struct mtk_spi *mdata = spi_master_get_devdata(master);
+
+ if (!pm_runtime_suspended(dev)) {
+ ret = clk_prepare_enable(mdata->spi_clk);
+ if (ret < 0)
+ return ret;
+ }
+
+ ret = spi_master_resume(master);
+ if (ret < 0)
+ clk_disable_unprepare(mdata->spi_clk);
+
+ return ret;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+#ifdef CONFIG_PM
+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);
+
+ clk_disable_unprepare(mdata->spi_clk);
+
+ return 0;
+}
+
+static int mtk_spi_runtime_resume(struct device *dev)
+{
+ struct spi_master *master = dev_get_drvdata(dev);
+ struct mtk_spi *mdata = spi_master_get_devdata(master);
+
+ return clk_prepare_enable(mdata->spi_clk);
+}
+#endif /* CONFIG_PM */
+
+static const struct dev_pm_ops mtk_spi_pm = {
+ SET_SYSTEM_SLEEP_PM_OPS(mtk_spi_suspend, mtk_spi_resume)
+ SET_RUNTIME_PM_OPS(mtk_spi_runtime_suspend,
+ mtk_spi_runtime_resume, NULL)
+};
+
+struct platform_driver mtk_spi_driver = {
+ .driver = {
+ .name = "mtk-spi",
+ .pm = &mtk_spi_pm,
+ .of_match_table = mtk_spi_of_match,
+ },
+ .probe = mtk_spi_probe,
+ .remove = mtk_spi_remove,
+};
+
+module_platform_driver(mtk_spi_driver);
+
+MODULE_DESCRIPTION("MTK SPI Controller driver");
+MODULE_AUTHOR("Leilk Liu <[email protected]>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform: mtk_spi");
diff --git a/include/linux/platform_data/spi-mt65xx.h b/include/linux/platform_data/spi-mt65xx.h
new file mode 100644
index 0000000..7512255
--- /dev/null
+++ b/include/linux/platform_data/spi-mt65xx.h
@@ -0,0 +1,22 @@
+/*
+ * MTK SPI bus driver definitions
+ *
+ * Copyright (c) 2015 MediaTek Inc.
+ * Author: Leilk Liu <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef ____LINUX_PLATFORM_DATA_SPI_MTK_H
+#define ____LINUX_PLATFORM_DATA_SPI_MTK_H
+
+/* Board specific platform_data */
+struct mtk_chip_config {
+ u32 tx_mlsb;
+ u32 rx_mlsb;
+ u32 tx_endian;
+ u32 rx_endian;
+};
+#endif
--
1.8.1.1.dirty

2015-08-07 07:20:55

by Leilk Liu

[permalink] [raw]
Subject: [PATCH v5 3/3] arm64: dts: Add spi bus dts

This patch adds MT8173 spi bus controllers into device tree.

Signed-off-by: Leilk Liu <[email protected]>
---
Change in this patch:
1. "pad-select" is a vendor property, so change it to "mediatek,pad-select".
2. modify the property of clocks and clock-names.
---
arch/arm64/boot/dts/mediatek/mt8173.dtsi | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index d18ee42..066bd6a 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -220,6 +220,15 @@
bias-disable;
};
};
+
+ spi_pins_a: spi0 {
+ pins_spi {
+ pinmux = <MT8173_PIN_69_SPI_CK__FUNC_SPI_CK_0_>,
+ <MT8173_PIN_70_SPI_MI__FUNC_SPI_MI_0_>,
+ <MT8173_PIN_71_SPI_MO__FUNC_SPI_MO_0_>,
+ <MT8173_PIN_72_SPI_CS__FUNC_SPI_CS_0_>;
+ };
+ };
};

scpsys: scpsys@10006000 {
@@ -365,6 +374,20 @@
status = "disabled";
};

+ spi: spi@1100a000 {
+ compatible = "mediatek,mt8173-spi";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0 0x1100a000 0 0x1000>;
+ interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&topckgen CLK_TOP_SPI_SEL>, <&topckgen CLK_TOP_SYSPLL3_D2>;
+ clock-names = "spi-clk", "parent-clk";
+ pinctrl-names = "default";
+ pinctrl-0 = <&spi_pins_a>;
+ mediatek,pad-select = <0>;
+ status = "disabled";
+ };
+
i2c3: i2c3@11010000 {
compatible = "mediatek,mt8173-i2c";
reg = <0 0x11010000 0 0x70>,
--
1.8.1.1.dirty

2015-08-11 10:52:26

by Nicolas Boichat

[permalink] [raw]
Subject: Re: [v5,2/3] spi: mediatek: Add spi bus for Mediatek MT8173

Hi Leilk,

On Fri, Aug 7, 2015 at 3:19 PM, <[email protected]> wrote:
> This patch adds basic spi bus for MT8173.
>
> Signed-off-by: Leilk Liu <[email protected]>
>
> ---
> Change in this patch:
> 1. change "pad-select" to "mediatek,pad-select".
> 2. modify clk relevant implement.
> ---
> drivers/spi/Kconfig | 9 +
> drivers/spi/Makefile | 1 +
> drivers/spi/spi-mt65xx.c | 749 +++++++++++++++++++++++++++++++
> include/linux/platform_data/spi-mt65xx.h | 22 +
> 4 files changed, 781 insertions(+)
> create mode 100644 drivers/spi/spi-mt65xx.c
> create mode 100644 include/linux/platform_data/spi-mt65xx.h
>
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 0cae169..38ddfba 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -326,6 +326,15 @@ config SPI_MESON_SPIFC
> This enables master mode support for the SPIFC (SPI flash
> controller) available in Amlogic Meson SoCs.
>
> +config SPI_MT65XX
> + tristate "MediaTek SPI controller"
> + depends on ARCH_MEDIATEK || COMPILE_TEST
> + help
> + This selects the MediaTek(R) SPI bus driver.
> + If you want to use MediaTek(R) SPI interface,
> + say Y or M here.If you are not sure, say N.
> + SPI drivers for Mediatek MT65XX and MT81XX series ARM SoCs.
> +
> config SPI_OC_TINY
> tristate "OpenCores tiny SPI"
> depends on GPIOLIB || COMPILE_TEST
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index 1154dba..9746beb2 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -48,6 +48,7 @@ obj-$(CONFIG_SPI_MESON_SPIFC) += spi-meson-spifc.o
> obj-$(CONFIG_SPI_MPC512x_PSC) += spi-mpc512x-psc.o
> obj-$(CONFIG_SPI_MPC52xx_PSC) += spi-mpc52xx-psc.o
> obj-$(CONFIG_SPI_MPC52xx) += spi-mpc52xx.o
> +obj-$(CONFIG_SPI_MT65XX) += spi-mt65xx.o
> obj-$(CONFIG_SPI_MXS) += spi-mxs.o
> obj-$(CONFIG_SPI_NUC900) += spi-nuc900.o
> obj-$(CONFIG_SPI_OC_TINY) += spi-oc-tiny.o
> diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
> new file mode 100644
> index 0000000..4676b01
> --- /dev/null
> +++ b/drivers/spi/spi-mt65xx.c
> @@ -0,0 +1,749 @@
> +/*
> + * Copyright (c) 2015 MediaTek Inc.
> + * Author: Leilk Liu <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>

Since you are using readl/writel, please import linux/io.h as well (it
is implicitly imported by spi/spi.h, but better be safe...)

> +#include <linux/ioport.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/platform_data/spi-mt65xx.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/spi/spi.h>
> +
> +#define SPI_CFG0_REG 0x0000
> +#define SPI_CFG1_REG 0x0004
> +#define SPI_TX_SRC_REG 0x0008
> +#define SPI_RX_DST_REG 0x000c
> +#define SPI_TX_DATA_REG 0x0010
> +#define SPI_RX_DATA_REG 0x0014
> +#define SPI_CMD_REG 0x0018
> +#define SPI_STATUS0_REG 0x001c
> +#define SPI_PAD_SEL_REG 0x0024
> +
> +#define SPI_CFG0_SCK_HIGH_OFFSET 0
> +#define SPI_CFG0_SCK_LOW_OFFSET 8
> +#define SPI_CFG0_CS_HOLD_OFFSET 16
> +#define SPI_CFG0_CS_SETUP_OFFSET 24
> +
> +#define SPI_CFG1_CS_IDLE_OFFSET 0
> +#define SPI_CFG1_PACKET_LOOP_OFFSET 8
> +#define SPI_CFG1_PACKET_LENGTH_OFFSET 16
> +#define SPI_CFG1_GET_TICK_DLY_OFFSET 30
> +
> +#define SPI_CFG1_CS_IDLE_MASK 0xff
> +#define SPI_CFG1_PACKET_LOOP_MASK 0xff00
> +#define SPI_CFG1_PACKET_LENGTH_MASK 0x3ff0000
> +
> +#define SPI_CMD_ACT_OFFSET 0
> +#define SPI_CMD_RESUME_OFFSET 1
> +#define SPI_CMD_CPHA_OFFSET 8
> +#define SPI_CMD_CPOL_OFFSET 9
> +#define SPI_CMD_TXMSBF_OFFSET 12
> +#define SPI_CMD_RXMSBF_OFFSET 13
> +#define SPI_CMD_RX_ENDIAN_OFFSET 14
> +#define SPI_CMD_TX_ENDIAN_OFFSET 15

It feels error-prone that you are defining these offsets here, then
redefining the bits just below.

Looking at the code, I think it's better if you remove these
SPI_CMD_*_OFFSET defines, and only use the BIT(x) defines below.

> +#define SPI_CMD_RST BIT(2)
> +#define SPI_CMD_PAUSE_EN BIT(4)
> +#define SPI_CMD_DEASSERT BIT(5)
> +#define SPI_CMD_CPHA BIT(8)
> +#define SPI_CMD_CPOL BIT(9)
> +#define SPI_CMD_RX_DMA BIT(10)
> +#define SPI_CMD_TX_DMA BIT(11)
> +#define SPI_CMD_TXMSBF BIT(12)
> +#define SPI_CMD_RXMSBF BIT(13)
> +#define SPI_CMD_RX_ENDIAN BIT(14)
> +#define SPI_CMD_TX_ENDIAN BIT(15)
> +#define SPI_CMD_FINISH_IE BIT(16)
> +#define SPI_CMD_PAUSE_IE BIT(17)
> +
> +#define MTK_SPI_QUIRK_PAD_SELECT 1
> +/* Must explicitly send dummy Tx bytes to do Rx only transfer */
> +#define MTK_SPI_QUIRK_MUST_TX 1
> +
> +#define MT8173_SPI_MAX_PAD_SEL 3
> +
> +#define MTK_SPI_IDLE 0
> +#define MTK_SPI_PAUSED 1
> +
> +#define MTK_SPI_MAX_FIFO_SIZE 32
> +#define MTK_SPI_PACKET_SIZE 1024
> +
> +struct mtk_spi_compatible {
> + u32 need_pad_sel;
> + u32 must_tx;

Since the quirks are true/false, define these as bool, and remove
MTK_SPI_QUIRK_PAD_SELECT/MTK_SPI_QUIRK_MUST_TX. Move the comment here
too.

> +};
> +
> +struct mtk_spi {
> + void __iomem *base;
> + u32 state;
> + u32 pad_sel;
> + struct clk *spi_clk, *parent_clk;
> + struct spi_transfer *cur_transfer;
> + u32 xfer_len;
> + struct scatterlist *tx_sgl, *rx_sgl;
> + u32 tx_sgl_len, rx_sgl_len;
> + const struct mtk_spi_compatible *dev_comp;
> +};
> +
> +static const struct mtk_spi_compatible mt6589_compat = {
> + .need_pad_sel = 0,
> + .must_tx = 0,
> +};
> +
> +static const struct mtk_spi_compatible mt8135_compat = {
> + .need_pad_sel = 0,
> + .must_tx = 0,
> +};

You don't need to set these explicitly to 0/false.

> +
> +static const struct mtk_spi_compatible mt8173_compat = {
> + .need_pad_sel = MTK_SPI_QUIRK_PAD_SELECT,
> + .must_tx = MTK_SPI_QUIRK_MUST_TX,

Then you can set those as "true".

> +};
> +
> +/*
> + * A piece of default chip info unless the platform
> + * supplies it.
> + */
> +static const struct mtk_chip_config mtk_default_chip_info = {
> + .rx_mlsb = 1,
> + .tx_mlsb = 1,
> + .tx_endian = 0,
> + .rx_endian = 0,
> +};
> +
> +static const struct of_device_id mtk_spi_of_match[] = {
> + { .compatible = "mediatek,mt6589-spi", .data = (void *)&mt6589_compat },
> + { .compatible = "mediatek,mt8135-spi", .data = (void *)&mt8135_compat },
> + { .compatible = "mediatek,mt8173-spi", .data = (void *)&mt8173_compat },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, mtk_spi_of_match);
> +
> +static void mtk_spi_reset(struct mtk_spi *mdata)
> +{
> + u32 reg_val;
> +
> + /* set the software reset bit in SPI_CMD_REG. */
> + reg_val = readl(mdata->base + SPI_CMD_REG);
> + reg_val |= SPI_CMD_RST;
> + writel(reg_val, mdata->base + SPI_CMD_REG);
> +
> + reg_val = readl(mdata->base + SPI_CMD_REG);
> + reg_val &= ~SPI_CMD_RST;
> + writel(reg_val, mdata->base + SPI_CMD_REG);
> +}
> +
> +static void mtk_spi_config(struct mtk_spi *mdata,
> + struct mtk_chip_config *chip_config)
> +{
> + u32 reg_val;
> +
> + reg_val = readl(mdata->base + SPI_CMD_REG);
> +
> + /* set the mlsbx and mlsbtx */
> + reg_val &= ~(SPI_CMD_TXMSBF | SPI_CMD_RXMSBF);
> + reg_val |= (chip_config->tx_mlsb << SPI_CMD_TXMSBF_OFFSET);
> + reg_val |= (chip_config->rx_mlsb << SPI_CMD_RXMSBF_OFFSET);
> +
> + /* set the tx/rx endian */
> + reg_val &= ~(SPI_CMD_TX_ENDIAN | SPI_CMD_RX_ENDIAN);
> + reg_val |= (chip_config->tx_endian << SPI_CMD_TX_ENDIAN_OFFSET);
> + reg_val |= (chip_config->rx_endian << SPI_CMD_RX_ENDIAN_OFFSET);
> +
> + /* set finish and pause interrupt always enable */
> + reg_val |= SPI_CMD_FINISH_IE | SPI_CMD_PAUSE_EN;
> +
> + /* disable dma mode */
> + reg_val &= ~(SPI_CMD_TX_DMA | SPI_CMD_RX_DMA);
> +
> + /* disable deassert mode */
> + reg_val &= ~SPI_CMD_DEASSERT;
> +
> + writel(reg_val, mdata->base + SPI_CMD_REG);
> +
> + /* pad select */
> + if (mdata->dev_comp->need_pad_sel)
> + writel(mdata->pad_sel, mdata->base + SPI_PAD_SEL_REG);
> +}
> +
> +static int mtk_spi_prepare_hardware(struct spi_master *master)
> +{
> + struct spi_transfer *trans;
> + struct mtk_spi *mdata = spi_master_get_devdata(master);
> + struct spi_message *msg = master->cur_msg;
> + int ret;
> +
> + ret = clk_prepare_enable(mdata->spi_clk);
> + if (ret < 0) {
> + dev_err(&master->dev, "failed to enable clock (%d)\n", ret);
> + return ret;
> + }
> +
> + trans = list_first_entry(&msg->transfers, struct spi_transfer,
> + transfer_list);
> + if (trans->cs_change == 0) {

!trans->cs_change

> + mdata->state = MTK_SPI_IDLE;
> + mtk_spi_reset(mdata);
> + }
> +
> + return ret;
> +}
> +
> +static int mtk_spi_unprepare_hardware(struct spi_master *master)
> +{
> + struct mtk_spi *mdata = spi_master_get_devdata(master);
> +
> + clk_disable_unprepare(mdata->spi_clk);
> +
> + return 0;
> +}

In this case, prepare_hardware/unprepare_hardware is redundant with
pm_runtime (apart from the cs_change check, possibly).

If PM_RUNTIME is disabled, leave the clock enabled all the time, if
not enable/disable in pm_runtime functions (as you already do)

See https://git.kernel.org/cgit/linux/kernel/git/broonie/spi.git/commit/?id=db91841b58f9ad0ecbb81ed0fa496c3a1b67fd63
"spi/omap100k: Convert to runtime PM" for an example (it's one of the
last driver that used prepare/unprepare calls, and got converted to
pm_runtime)

> +static int mtk_spi_prepare_message(struct spi_master *master,
> + struct spi_message *msg)
> +{
> + u32 reg_val;
> + u8 cpha, cpol;
> + struct mtk_chip_config *chip_config;
> + struct spi_device *spi = msg->spi;
> + struct mtk_spi *mdata = spi_master_get_devdata(master);
> +
> + cpha = spi->mode & SPI_CPHA ? 1 : 0;
> + cpol = spi->mode & SPI_CPOL ? 1 : 0;
> +
> + reg_val = readl(mdata->base + SPI_CMD_REG);
> + reg_val &= ~(SPI_CMD_CPHA | SPI_CMD_CPOL);
> + reg_val |= (cpha << SPI_CMD_CPHA_OFFSET);
> + reg_val |= (cpol << SPI_CMD_CPOL_OFFSET);

This can actually be simplified a bit by using
SPI_CMD_CPHA/SPI_CMD_CPOL instead.

> + writel(reg_val, mdata->base + SPI_CMD_REG);
> +
> + chip_config = spi->controller_data;
> + if (!chip_config) {
> + chip_config = (void *)&mtk_default_chip_info;
> + spi->controller_data = chip_config;
> + }
> + mtk_spi_config(mdata, chip_config);
> +
> + return 0;
> +}
> +
> +static void mtk_spi_set_cs(struct spi_device *spi, bool enable)
> +{
> + u32 reg_val;
> + struct mtk_spi *mdata = spi_master_get_devdata(spi->master);
> +
> + reg_val = readl(mdata->base + SPI_CMD_REG);
> + if (!enable)
> + reg_val |= SPI_CMD_PAUSE_EN;
> + else
> + reg_val &= ~SPI_CMD_PAUSE_EN;
> + writel(reg_val, mdata->base + SPI_CMD_REG);
> +}
> +
> +static void mtk_spi_prepare_transfer(struct spi_master *master,
> + struct spi_transfer *xfer)
> +{
> + u32 spi_clk_hz, div, high_time, low_time, holdtime,
> + setuptime, cs_idletime, reg_val = 0;
> + struct mtk_spi *mdata = spi_master_get_devdata(master);
> +
> + spi_clk_hz = clk_get_rate(mdata->spi_clk);
> + if (xfer->speed_hz < spi_clk_hz / 2)
> + div = DIV_ROUND_UP(spi_clk_hz, xfer->speed_hz);
> + else
> + div = 1;
> +
> + high_time = (div + 1) / 2;
> + low_time = (div + 1) / 2;
> + holdtime = (div + 1) / 2 * 2;
> + setuptime = (div + 1) / 2 * 2;
> + cs_idletime = (div + 1) / 2 * 2;

Why setting variables with the exact same value? Can you do with just 2?

> + reg_val |= (((high_time - 1) & 0xff) << SPI_CFG0_SCK_HIGH_OFFSET);
> + reg_val |= (((low_time - 1) & 0xff) << SPI_CFG0_SCK_LOW_OFFSET);
> + reg_val |= (((holdtime - 1) & 0xff) << SPI_CFG0_CS_HOLD_OFFSET);
> + reg_val |= (((setuptime - 1) & 0xff) << SPI_CFG0_CS_SETUP_OFFSET);

Not sure of the details, but can you guarantee this will never
overflow? I.e. can div be larger than 256?

> + writel(reg_val, mdata->base + SPI_CFG0_REG);
> +
> + reg_val = readl(mdata->base + SPI_CFG1_REG);
> + reg_val &= ~SPI_CFG1_CS_IDLE_MASK;
> + reg_val |= (((cs_idletime - 1) & 0xff) << SPI_CFG1_CS_IDLE_OFFSET);
> + writel(reg_val, mdata->base + SPI_CFG1_REG);
> +}
> +
> +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(unsigned, mdata->xfer_len, MTK_SPI_PACKET_SIZE);

u32 instead of unsigned.

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

Use |, not +.

> + reg_val |= (packet_size - 1) << SPI_CFG1_PACKET_LENGTH_OFFSET;
> + reg_val |= (packet_loop - 1) << SPI_CFG1_PACKET_LOOP_OFFSET;

Can packet_loop be >256?

> + writel(reg_val, mdata->base + SPI_CFG1_REG);
> +}
> +
> +static void mtk_spi_enable_transfer(struct spi_master *master)
> +{
> + int cmd;

u32

> + struct mtk_spi *mdata = spi_master_get_devdata(master);
> +
> + cmd = readl(mdata->base + SPI_CMD_REG);
> + if (mdata->state == MTK_SPI_IDLE)
> + cmd |= 1 << SPI_CMD_ACT_OFFSET;
> + else
> + cmd |= 1 << SPI_CMD_RESUME_OFFSET;
> + writel(cmd, mdata->base + SPI_CMD_REG);
> +}
> +
> +static int mtk_spi_get_mult_delta(int xfer_len)

xfer_len is a u32, so should be mult_delta.

> +{
> + int mult_delta;
> +
> + if (xfer_len > MTK_SPI_PACKET_SIZE)
> + mult_delta = xfer_len % MTK_SPI_PACKET_SIZE;
> + else
> + mult_delta = 0;
> +
> + return mult_delta;
> +}
> +
> +static void mtk_spi_update_mdata_len(struct spi_master *master)
> +{
> + int mult_delta;
> + struct mtk_spi *mdata = spi_master_get_devdata(master);
> +
> + if (mdata->tx_sgl_len && mdata->rx_sgl_len) {
> + if (mdata->tx_sgl_len > mdata->rx_sgl_len) {
> + mult_delta = mtk_spi_get_mult_delta(mdata->rx_sgl_len);
> + mdata->xfer_len = mdata->rx_sgl_len - mult_delta;
> + mdata->rx_sgl_len = mult_delta;
> + mdata->tx_sgl_len -= mdata->xfer_len;
> + } else {
> + mult_delta = mtk_spi_get_mult_delta(mdata->tx_sgl_len);
> + mdata->xfer_len = mdata->tx_sgl_len - mult_delta;
> + mdata->tx_sgl_len = mult_delta;
> + mdata->rx_sgl_len -= mdata->xfer_len;
> + }
> + } else if (mdata->tx_sgl_len) {
> + mult_delta = mtk_spi_get_mult_delta(mdata->tx_sgl_len);
> + mdata->xfer_len = mdata->tx_sgl_len - mult_delta;
> + mdata->tx_sgl_len = mult_delta;
> + } else if (mdata->rx_sgl_len) {
> + mult_delta = mtk_spi_get_mult_delta(mdata->rx_sgl_len);
> + mdata->xfer_len = mdata->rx_sgl_len - mult_delta;
> + mdata->rx_sgl_len = mult_delta;
> + }
> +}
> +
> +static void mtk_spi_setup_dma_addr(struct spi_master *master,
> + struct spi_transfer *xfer)
> +{
> + struct mtk_spi *mdata = spi_master_get_devdata(master);
> +
> + if (mdata->tx_sgl)
> + writel(cpu_to_le32(xfer->tx_dma), mdata->base + SPI_TX_SRC_REG);
> + if (mdata->rx_sgl)
> + writel(cpu_to_le32(xfer->rx_dma), mdata->base + SPI_RX_DST_REG);
> +}
> +
> +static int mtk_spi_fifo_transfer(struct spi_master *master,
> + struct spi_device *spi,
> + struct spi_transfer *xfer)
> +{
> + int cnt, i;
> + struct mtk_spi *mdata = spi_master_get_devdata(master);
> +
> + mdata->cur_transfer = xfer;
> + mdata->xfer_len = xfer->len;
> + mtk_spi_prepare_transfer(master, xfer);
> + mtk_spi_setup_packet(master);
> +
> + if (xfer->len % 4)
> + cnt = xfer->len / 4 + 1;
> + else
> + cnt = xfer->len / 4;
> +
> + for (i = 0; i < cnt; i++)
> + writel(*((u32 *)xfer->tx_buf + i),

This will access past the end of tx_buf if len%4 > 0.

> + mdata->base + SPI_TX_DATA_REG);
> +
> + mtk_spi_enable_transfer(master);
> +
> + return 1;
> +}
> +
> +static int mtk_spi_dma_transfer(struct spi_master *master,
> + struct spi_device *spi,
> + struct spi_transfer *xfer)
> +{
> + int cmd;
> + struct mtk_spi *mdata = spi_master_get_devdata(master);
> +
> + mdata->tx_sgl = NULL;
> + mdata->rx_sgl = NULL;
> + mdata->tx_sgl_len = 0;
> + mdata->rx_sgl_len = 0;
> + mdata->cur_transfer = xfer;
> +
> + mtk_spi_prepare_transfer(master, xfer);
> +
> + cmd = readl(mdata->base + SPI_CMD_REG);
> + if (xfer->tx_buf)
> + cmd |= SPI_CMD_TX_DMA;
> + if (xfer->rx_buf)
> + cmd |= SPI_CMD_RX_DMA;
> + writel(cmd, mdata->base + SPI_CMD_REG);
> +
> + if (xfer->tx_buf)
> + mdata->tx_sgl = xfer->tx_sg.sgl;
> + if (xfer->rx_buf)
> + mdata->rx_sgl = xfer->rx_sg.sgl;
> +
> + if (mdata->tx_sgl) {
> + xfer->tx_dma = sg_dma_address(mdata->tx_sgl);
> + mdata->tx_sgl_len = sg_dma_len(mdata->tx_sgl);
> + }
> + if (mdata->rx_sgl) {
> + xfer->rx_dma = sg_dma_address(mdata->rx_sgl);
> + mdata->rx_sgl_len = sg_dma_len(mdata->rx_sgl);
> + }
> +
> + mtk_spi_update_mdata_len(master);
> + mtk_spi_setup_packet(master);
> + mtk_spi_setup_dma_addr(master, xfer);
> + mtk_spi_enable_transfer(master);
> +
> + return 1;
> +}
> +
> +static int mtk_spi_transfer_one(struct spi_master *master,
> + struct spi_device *spi,
> + struct spi_transfer *xfer)
> +{
> + if (master->can_dma(master, spi, xfer))
> + return mtk_spi_dma_transfer(master, spi, xfer);
> + else
> + return mtk_spi_fifo_transfer(master, spi, xfer);
> +}
> +
> +static bool mtk_spi_can_dma(struct spi_master *master,
> + struct spi_device *spi,
> + struct spi_transfer *xfer)
> +{
> + return xfer->len > MTK_SPI_MAX_FIFO_SIZE;
> +}
> +
> +static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
> +{
> + u32 cmd, reg_val, i;
> + struct spi_master *master = dev_id;
> + struct mtk_spi *mdata = spi_master_get_devdata(master);
> + struct spi_transfer *trans = mdata->cur_transfer;
> +
> + reg_val = readl(mdata->base + SPI_STATUS0_REG);
> + if (reg_val & 0x2)

define 0x2?

> + mdata->state = MTK_SPI_PAUSED;
> + else
> + mdata->state = MTK_SPI_IDLE;
> +
> + if (!master->can_dma(master, master->cur_msg->spi, trans)) {
> + /* xfer len is not N*4 bytes every time in a transfer,
> + * but SPI_RX_DATA_REG must reads 4 bytes once,
> + * so rx buffer byte by byte.
> + */
> + if (trans->rx_buf) {
> + for (i = 0; i < mdata->xfer_len; i++) {
> + if (i % 4 == 0)
> + reg_val =
> + readl(mdata->base + SPI_RX_DATA_REG);

Strange indentation. Might be better to put readl on the same line,
and SPI_RX_DATA_REG on the next one.

> + *((u8 *)(trans->rx_buf + i)) =
> + (reg_val >> ((i % 4) * 8)) & 0xff;

This feels a bit awkward... Also, & 0xff is not needed.

> + }
> + }
> + spi_finalize_current_transfer(master);
> + return IRQ_HANDLED;
> + }
> +
> + if (mdata->tx_sgl)
> + trans->tx_dma += mdata->xfer_len;
> + if (mdata->rx_sgl)
> + trans->rx_dma += mdata->xfer_len;
> +
> + if (mdata->tx_sgl && (mdata->tx_sgl_len == 0)) {
> + mdata->tx_sgl = sg_next(mdata->tx_sgl);
> + if (mdata->tx_sgl) {
> + trans->tx_dma = sg_dma_address(mdata->tx_sgl);
> + mdata->tx_sgl_len = sg_dma_len(mdata->tx_sgl);
> + }
> + }
> + if (mdata->rx_sgl && (mdata->rx_sgl_len == 0)) {
> + mdata->rx_sgl = sg_next(mdata->rx_sgl);
> + if (mdata->rx_sgl) {
> + trans->rx_dma = sg_dma_address(mdata->rx_sgl);
> + mdata->rx_sgl_len = sg_dma_len(mdata->rx_sgl);
> + }
> + }
> +
> + if (!mdata->tx_sgl && !mdata->rx_sgl) {
> + /* spi disable dma */
> + cmd = readl(mdata->base + SPI_CMD_REG);
> + cmd &= ~SPI_CMD_TX_DMA;
> + cmd &= ~SPI_CMD_RX_DMA;
> + writel(cmd, mdata->base + SPI_CMD_REG);
> +
> + spi_finalize_current_transfer(master);
> + return IRQ_HANDLED;
> + }
> +
> + mtk_spi_update_mdata_len(master);
> + mtk_spi_setup_packet(master);
> + mtk_spi_setup_dma_addr(master, trans);
> + mtk_spi_enable_transfer(master);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int mtk_spi_probe(struct platform_device *pdev)
> +{
> + struct spi_master *master;
> + struct mtk_spi *mdata;
> + const struct of_device_id *of_id;
> + struct resource *res;
> + int irq, ret;

Space, not tab, between int and irq.

> +
> + master = spi_alloc_master(&pdev->dev, sizeof(*mdata));
> + if (!master) {
> + dev_err(&pdev->dev, "failed to alloc spi master\n");
> + return -ENOMEM;
> + }
> +
> + master->auto_runtime_pm = true;
> + master->dev.of_node = pdev->dev.of_node;
> + master->mode_bits = SPI_CPOL | SPI_CPHA;
> +
> + master->set_cs = mtk_spi_set_cs;
> + master->prepare_transfer_hardware = mtk_spi_prepare_hardware;
> + master->unprepare_transfer_hardware = mtk_spi_unprepare_hardware;
> + master->prepare_message = mtk_spi_prepare_message;
> + master->transfer_one = mtk_spi_transfer_one;
> + master->can_dma = mtk_spi_can_dma;
> +
> + of_id = of_match_node(mtk_spi_of_match, pdev->dev.of_node);
> + if (!of_id) {
> + dev_err(&pdev->dev, "failed to probe of_node\n");
> + ret = -EINVAL;
> + goto err_put_master;
> + }
> +
> + mdata = spi_master_get_devdata(master);
> + mdata->dev_comp = of_id->data;
> + if (mdata->dev_comp->must_tx)
> + master->flags = SPI_MASTER_MUST_TX;
> +
> + if (mdata->dev_comp->need_pad_sel) {
> + ret = of_property_read_u32(pdev->dev.of_node,
> + "mediatek,pad-select",
> + &mdata->pad_sel);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to read pad select: %d\n",
> + ret);
> + goto err_put_master;
> + }
> +
> + if (mdata->pad_sel > MT8173_SPI_MAX_PAD_SEL) {
> + dev_err(&pdev->dev, "wrong pad-select: %u\n",
> + mdata->pad_sel);
> + ret = -EINVAL;
> + goto err_put_master;
> + }
> + }
> +
> + platform_set_drvdata(pdev, master);
> +
> + 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_master;
> + }
> +
> + mdata->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(mdata->base)) {
> + ret = PTR_ERR(mdata->base);
> + goto err_put_master;
> + }
> +
> + 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_master;
> + }
> +
> + if (!pdev->dev.dma_mask)
> + pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
> +
> + ret = devm_request_irq(&pdev->dev, irq, mtk_spi_interrupt,
> + IRQF_TRIGGER_NONE, dev_name(&pdev->dev), master);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to register irq (%d)\n", ret);
> + goto err_put_master;
> + }
> +
> + mdata->spi_clk = devm_clk_get(&pdev->dev, "spi-clk");
> + 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_master;
> + }
> +
> + mdata->parent_clk = devm_clk_get(&pdev->dev, "parent-clk");
> + if (IS_ERR(mdata->parent_clk)) {
> + ret = PTR_ERR(mdata->parent_clk);
> + dev_err(&pdev->dev, "failed to get parent-clk: %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;
> + }
> +
> + ret = clk_set_parent(mdata->spi_clk, mdata->parent_clk);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "failed to clk_set_parent (%d)\n", ret);
> + goto err_disable_clk;
> + }
> +
> + clk_disable_unprepare(mdata->spi_clk);
> +
> + pm_runtime_enable(&pdev->dev);
> +
> + ret = devm_spi_register_master(&pdev->dev, master);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to register master (%d)\n", ret);
> + goto err_put_master;
> + }
> +
> + return 0;
> +
> +err_disable_clk:
> + clk_disable_unprepare(mdata->spi_clk);
> +err_put_master:
> + spi_master_put(master);
> +
> + return ret;
> +}
> +
> +static int mtk_spi_remove(struct platform_device *pdev)
> +{
> + struct spi_master *master = platform_get_drvdata(pdev);
> + struct mtk_spi *mdata = spi_master_get_devdata(master);
> +
> + pm_runtime_disable(&pdev->dev);
> +
> + mtk_spi_reset(mdata);
> + clk_disable_unprepare(mdata->spi_clk);
> + spi_master_put(master);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int mtk_spi_suspend(struct device *dev)
> +{
> + int ret;
> + struct spi_master *master = dev_get_drvdata(dev);
> + struct mtk_spi *mdata = spi_master_get_devdata(master);
> +
> + ret = spi_master_suspend(master);
> + if (ret)
> + return ret;
> +
> + if (!pm_runtime_suspended(dev))
> + clk_disable_unprepare(mdata->spi_clk);
> +
> + return ret;
> +}
> +
> +static int mtk_spi_resume(struct device *dev)
> +{
> + int ret;
> + struct spi_master *master = dev_get_drvdata(dev);
> + struct mtk_spi *mdata = spi_master_get_devdata(master);
> +
> + if (!pm_runtime_suspended(dev)) {
> + ret = clk_prepare_enable(mdata->spi_clk);
> + if (ret < 0)
> + return ret;
> + }
> +
> + ret = spi_master_resume(master);
> + if (ret < 0)
> + clk_disable_unprepare(mdata->spi_clk);
> +
> + return ret;
> +}
> +#endif /* CONFIG_PM_SLEEP */
> +
> +#ifdef CONFIG_PM
> +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);
> +
> + clk_disable_unprepare(mdata->spi_clk);
> +
> + return 0;
> +}
> +
> +static int mtk_spi_runtime_resume(struct device *dev)
> +{
> + struct spi_master *master = dev_get_drvdata(dev);
> + struct mtk_spi *mdata = spi_master_get_devdata(master);
> +
> + return clk_prepare_enable(mdata->spi_clk);
> +}
> +#endif /* CONFIG_PM */
> +
> +static const struct dev_pm_ops mtk_spi_pm = {
> + SET_SYSTEM_SLEEP_PM_OPS(mtk_spi_suspend, mtk_spi_resume)
> + SET_RUNTIME_PM_OPS(mtk_spi_runtime_suspend,
> + mtk_spi_runtime_resume, NULL)
> +};
> +
> +struct platform_driver mtk_spi_driver = {
> + .driver = {
> + .name = "mtk-spi",
> + .pm = &mtk_spi_pm,
> + .of_match_table = mtk_spi_of_match,
> + },
> + .probe = mtk_spi_probe,
> + .remove = mtk_spi_remove,
> +};
> +
> +module_platform_driver(mtk_spi_driver);
> +
> +MODULE_DESCRIPTION("MTK SPI Controller driver");
> +MODULE_AUTHOR("Leilk Liu <[email protected]>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform: mtk_spi");
> diff --git a/include/linux/platform_data/spi-mt65xx.h b/include/linux/platform_data/spi-mt65xx.h
> new file mode 100644
> index 0000000..7512255
> --- /dev/null
> +++ b/include/linux/platform_data/spi-mt65xx.h
> @@ -0,0 +1,22 @@
> +/*
> + * MTK SPI bus driver definitions
> + *
> + * Copyright (c) 2015 MediaTek Inc.
> + * Author: Leilk Liu <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef ____LINUX_PLATFORM_DATA_SPI_MTK_H
> +#define ____LINUX_PLATFORM_DATA_SPI_MTK_H
> +
> +/* Board specific platform_data */
> +struct mtk_chip_config {
> + u32 tx_mlsb;
> + u32 rx_mlsb;
> + u32 tx_endian;
> + u32 rx_endian;
> +};
> +#endif

2015-08-11 12:37:57

by Daniel Kurtz

[permalink] [raw]
Subject: Re: [PATCH v5 3/3] arm64: dts: Add spi bus dts

On Fri, Aug 7, 2015 at 3:19 PM, Leilk Liu <[email protected]> wrote:
> This patch adds MT8173 spi bus controllers into device tree.
>
> Signed-off-by: Leilk Liu <[email protected]>
> ---
> Change in this patch:
> 1. "pad-select" is a vendor property, so change it to "mediatek,pad-select".
> 2. modify the property of clocks and clock-names.
> ---
> arch/arm64/boot/dts/mediatek/mt8173.dtsi | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> index d18ee42..066bd6a 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> @@ -220,6 +220,15 @@
> bias-disable;
> };
> };
> +
> + spi_pins_a: spi0 {
> + pins_spi {
> + pinmux = <MT8173_PIN_69_SPI_CK__FUNC_SPI_CK_0_>,
> + <MT8173_PIN_70_SPI_MI__FUNC_SPI_MI_0_>,
> + <MT8173_PIN_71_SPI_MO__FUNC_SPI_MO_0_>,
> + <MT8173_PIN_72_SPI_CS__FUNC_SPI_CS_0_>;
> + };
> + };
> };
>
> scpsys: scpsys@10006000 {
> @@ -365,6 +374,20 @@
> status = "disabled";
> };
>
> + spi: spi@1100a000 {
> + compatible = "mediatek,mt8173-spi";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0 0x1100a000 0 0x1000>;
> + interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&topckgen CLK_TOP_SPI_SEL>, <&topckgen CLK_TOP_SYSPLL3_D2>;
> + clock-names = "spi-clk", "parent-clk";
> + pinctrl-names = "default";
> + pinctrl-0 = <&spi_pins_a>;
> + mediatek,pad-select = <0>;

The pinctl and pad-select fields are board specific.
Please move to mt8173-evb.dtsi, along with status = "okay";

> + status = "disabled";
> + };
> +
> i2c3: i2c3@11010000 {
> compatible = "mediatek,mt8173-i2c";
> reg = <0 0x11010000 0 0x70>,
> --
> 1.8.1.1.dirty
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2015-08-13 03:35:24

by Leilk Liu

[permalink] [raw]
Subject: Re: [PATCH v5 3/3] arm64: dts: Add spi bus dts

> > + spi: spi@1100a000 {
> > + compatible = "mediatek,mt8173-spi";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + reg = <0 0x1100a000 0 0x1000>;
> > + interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_LOW>;
> > + clocks = <&topckgen CLK_TOP_SPI_SEL>, <&topckgen CLK_TOP_SYSPLL3_D2>;
> > + clock-names = "spi-clk", "parent-clk";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&spi_pins_a>;
> > + mediatek,pad-select = <0>;
>
> The pinctl and pad-select fields are board specific.
> Please move to mt8173-evb.dtsi, along with status = "okay";
>

OK, I will fix it.

2015-08-13 06:29:41

by Leilk Liu

[permalink] [raw]
Subject: Re: [v5,2/3] spi: mediatek: Add spi bus for Mediatek MT8173

Hi,

> > +#include <linux/clk.h>
> > +#include <linux/device.h>
> > +#include <linux/err.h>
> > +#include <linux/interrupt.h>
>
> Since you are using readl/writel, please import linux/io.h as well (it
> is implicitly imported by spi/spi.h, but better be safe...)
>
OK, I'll fix it.

> > +#include <linux/ioport.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>

> > +
> > +#define SPI_CMD_ACT_OFFSET 0
> > +#define SPI_CMD_RESUME_OFFSET 1
> > +#define SPI_CMD_CPHA_OFFSET 8
> > +#define SPI_CMD_CPOL_OFFSET 9
> > +#define SPI_CMD_TXMSBF_OFFSET 12
> > +#define SPI_CMD_RXMSBF_OFFSET 13
> > +#define SPI_CMD_RX_ENDIAN_OFFSET 14
> > +#define SPI_CMD_TX_ENDIAN_OFFSET 15
>
> It feels error-prone that you are defining these offsets here, then
> redefining the bits just below.
>
> Looking at the code, I think it's better if you remove these
> SPI_CMD_*_OFFSET defines, and only use the BIT(x) defines below.
>
OK, I will remove those defines.

> > +#define SPI_CMD_RST BIT(2)
> > +#define SPI_CMD_PAUSE_EN BIT(4)
> > +#define SPI_CMD_DEASSERT BIT(5)
> > +#define SPI_CMD_CPHA BIT(8)
> > +#define SPI_CMD_CPOL BIT(9)
> > +#define SPI_CMD_RX_DMA BIT(10)
> > +#define SPI_CMD_TX_DMA BIT(11)
> > +#define SPI_CMD_TXMSBF BIT(12)
> > +#define SPI_CMD_RXMSBF BIT(13)
> > +#define SPI_CMD_RX_ENDIAN BIT(14)
> > +#define SPI_CMD_TX_ENDIAN BIT(15)
> > +#define SPI_CMD_FINISH_IE BIT(16)
> > +#define SPI_CMD_PAUSE_IE BIT(17)
> > +

> > +
> > +struct mtk_spi_compatible {
> > + u32 need_pad_sel;
> > + u32 must_tx;
>
> Since the quirks are true/false, define these as bool, and remove
> MTK_SPI_QUIRK_PAD_SELECT/MTK_SPI_QUIRK_MUST_TX. Move the comment here
> too.
>
OK. I will fix it.

> > +};
> > +

> > +static const struct mtk_spi_compatible mt6589_compat = {
> > + .need_pad_sel = 0,
> > + .must_tx = 0,
> > +};
> > +
> > +static const struct mtk_spi_compatible mt8135_compat = {
> > + .need_pad_sel = 0,
> > + .must_tx = 0,
> > +};
>
> You don't need to set these explicitly to 0/false.
>
OK, I will fix it.

> > +
> > +static const struct mtk_spi_compatible mt8173_compat = {
> > + .need_pad_sel = MTK_SPI_QUIRK_PAD_SELECT,
> > + .must_tx = MTK_SPI_QUIRK_MUST_TX,
>
> Then you can set those as "true".
>
OK, I will fix it.

> > +};
> > +
> > +/*
> > + * A piece of default chip info unless the platform
> > + * supplies it.
> > + */
> > +static const struct mtk_chip_config mtk_default_chip_info = {
> > + .rx_mlsb = 1,
> > + .tx_mlsb = 1,
> > + .tx_endian = 0,
> > + .rx_endian = 0,
> > +};
> > +

> > +
> > + trans = list_first_entry(&msg->transfers, struct spi_transfer,
> > + transfer_list);
> > + if (trans->cs_change == 0) {
>
> !trans->cs_change
>
OK, I will fix it.

> > + mdata->state = MTK_SPI_IDLE;
> > + mtk_spi_reset(mdata);
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int mtk_spi_unprepare_hardware(struct spi_master *master)
> > +{
> > + struct mtk_spi *mdata = spi_master_get_devdata(master);
> > +
> > + clk_disable_unprepare(mdata->spi_clk);
> > +
> > + return 0;
> > +}
>
> In this case, prepare_hardware/unprepare_hardware is redundant with
> pm_runtime (apart from the cs_change check, possibly).
>
> If PM_RUNTIME is disabled, leave the clock enabled all the time, if
> not enable/disable in pm_runtime functions (as you already do)
>
> See https://git.kernel.org/cgit/linux/kernel/git/broonie/spi.git/commit/?id=db91841b58f9ad0ecbb81ed0fa496c3a1b67fd63
> "spi/omap100k: Convert to runtime PM" for an example (it's one of the
> last driver that used prepare/unprepare calls, and got converted to
> pm_runtime)
>
OK, I'll fix it.

> > +static int mtk_spi_prepare_message(struct spi_master *master,
> > + struct spi_message *msg)
> > +{
> > + u32 reg_val;
> > + u8 cpha, cpol;
> > + struct mtk_chip_config *chip_config;
> > + struct spi_device *spi = msg->spi;
> > + struct mtk_spi *mdata = spi_master_get_devdata(master);
> > +
> > + cpha = spi->mode & SPI_CPHA ? 1 : 0;
> > + cpol = spi->mode & SPI_CPOL ? 1 : 0;
> > +
> > + reg_val = readl(mdata->base + SPI_CMD_REG);
> > + reg_val &= ~(SPI_CMD_CPHA | SPI_CMD_CPOL);
> > + reg_val |= (cpha << SPI_CMD_CPHA_OFFSET);
> > + reg_val |= (cpol << SPI_CMD_CPOL_OFFSET);
>
> This can actually be simplified a bit by using
> SPI_CMD_CPHA/SPI_CMD_CPOL instead.
>
OK, I will fix it.

> > + writel(reg_val, mdata->base + SPI_CMD_REG);
> > +

> > +
> > +static void mtk_spi_prepare_transfer(struct spi_master *master,
> > + struct spi_transfer *xfer)
> > +{
> > + u32 spi_clk_hz, div, high_time, low_time, holdtime,
> > + setuptime, cs_idletime, reg_val = 0;
> > + struct mtk_spi *mdata = spi_master_get_devdata(master);
> > +
> > + spi_clk_hz = clk_get_rate(mdata->spi_clk);
> > + if (xfer->speed_hz < spi_clk_hz / 2)
> > + div = DIV_ROUND_UP(spi_clk_hz, xfer->speed_hz);
> > + else
> > + div = 1;
> > +
> > + high_time = (div + 1) / 2;
> > + low_time = (div + 1) / 2;
> > + holdtime = (div + 1) / 2 * 2;
> > + setuptime = (div + 1) / 2 * 2;
> > + cs_idletime = (div + 1) / 2 * 2;
>
> Why setting variables with the exact same value? Can you do with just 2?
>
OK, I'll fix it.

> > + reg_val |= (((high_time - 1) & 0xff) << SPI_CFG0_SCK_HIGH_OFFSET);
> > + reg_val |= (((low_time - 1) & 0xff) << SPI_CFG0_SCK_LOW_OFFSET);
> > + reg_val |= (((holdtime - 1) & 0xff) << SPI_CFG0_CS_HOLD_OFFSET);
> > + reg_val |= (((setuptime - 1) & 0xff) << SPI_CFG0_CS_SETUP_OFFSET);
>
> Not sure of the details, but can you guarantee this will never
> overflow? I.e. can div be larger than 256?
>
If xfer->speed_hz is too small, maybe div may be larger than 256, but
0xff will mask div here, so I think it doesn't matter.


> > + writel(reg_val, mdata->base + SPI_CFG0_REG);
> > +
> > + reg_val = readl(mdata->base + SPI_CFG1_REG);
> > + reg_val &= ~SPI_CFG1_CS_IDLE_MASK;
> > + reg_val |= (((cs_idletime - 1) & 0xff) << SPI_CFG1_CS_IDLE_OFFSET);
> > + writel(reg_val, mdata->base + SPI_CFG1_REG);
> > +}
> > +
> > +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(unsigned, mdata->xfer_len, MTK_SPI_PACKET_SIZE);
>
> u32 instead of unsigned.
OK, I will fix it.

>
> > + 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);
>
> Use |, not +.
OK, I will fix it.

>
> > + reg_val |= (packet_size - 1) << SPI_CFG1_PACKET_LENGTH_OFFSET;
> > + reg_val |= (packet_loop - 1) << SPI_CFG1_PACKET_LOOP_OFFSET;
>
> Can packet_loop be >256?
>
packet_loop can never be >256. If packet_loop=256, the xfer_len will be
256*1024 bytes. It's not possible because packet_loop is from
mdata->xfer_len / packet_size, mdata->xfer_len is from sg_dma_len().

> > + writel(reg_val, mdata->base + SPI_CFG1_REG);
> > +}
> > +
> > +static void mtk_spi_enable_transfer(struct spi_master *master)
> > +{
> > + int cmd;
>
> u32
>
OK. I will fix it.

> > + struct mtk_spi *mdata = spi_master_get_devdata(master);
> > +
> > + cmd = readl(mdata->base + SPI_CMD_REG);
> > + if (mdata->state == MTK_SPI_IDLE)
> > + cmd |= 1 << SPI_CMD_ACT_OFFSET;
> > + else
> > + cmd |= 1 << SPI_CMD_RESUME_OFFSET;
> > + writel(cmd, mdata->base + SPI_CMD_REG);
> > +}
> > +
> > +static int mtk_spi_get_mult_delta(int xfer_len)
>
> xfer_len is a u32, so should be mult_delta.
>
OK, I'll fix it.

> > +{
> > + int mult_delta;
> > +
> > + if (xfer_len > MTK_SPI_PACKET_SIZE)
> > + mult_delta = xfer_len % MTK_SPI_PACKET_SIZE;
> > + else
> > + mult_delta = 0;
> > +
> > + return mult_delta;
> > +}
> > +
> > +static void mtk_spi_update_mdata_len(struct spi_master *master)
> > +{
> > + int mult_delta;
> > + struct mtk_spi *mdata = spi_master_get_devdata(master);
> > +
> > + if (mdata->tx_sgl_len && mdata->rx_sgl_len) {
> > + if (mdata->tx_sgl_len > mdata->rx_sgl_len) {
> > + mult_delta = mtk_spi_get_mult_delta(mdata->rx_sgl_len);
> > + mdata->xfer_len = mdata->rx_sgl_len - mult_delta;
> > + mdata->rx_sgl_len = mult_delta;
> > + mdata->tx_sgl_len -= mdata->xfer_len;
> > + } else {
> > + mult_delta = mtk_spi_get_mult_delta(mdata->tx_sgl_len);
> > + mdata->xfer_len = mdata->tx_sgl_len - mult_delta;
> > + mdata->tx_sgl_len = mult_delta;
> > + mdata->rx_sgl_len -= mdata->xfer_len;
> > + }
> > + } else if (mdata->tx_sgl_len) {
> > + mult_delta = mtk_spi_get_mult_delta(mdata->tx_sgl_len);
> > + mdata->xfer_len = mdata->tx_sgl_len - mult_delta;
> > + mdata->tx_sgl_len = mult_delta;
> > + } else if (mdata->rx_sgl_len) {
> > + mult_delta = mtk_spi_get_mult_delta(mdata->rx_sgl_len);
> > + mdata->xfer_len = mdata->rx_sgl_len - mult_delta;
> > + mdata->rx_sgl_len = mult_delta;
> > + }
> > +}
> > +
> > +static void mtk_spi_setup_dma_addr(struct spi_master *master,
> > + struct spi_transfer *xfer)
> > +{
> > + struct mtk_spi *mdata = spi_master_get_devdata(master);
> > +
> > + if (mdata->tx_sgl)
> > + writel(cpu_to_le32(xfer->tx_dma), mdata->base + SPI_TX_SRC_REG);
> > + if (mdata->rx_sgl)
> > + writel(cpu_to_le32(xfer->rx_dma), mdata->base + SPI_RX_DST_REG);
> > +}
> > +
> > +static int mtk_spi_fifo_transfer(struct spi_master *master,
> > + struct spi_device *spi,
> > + struct spi_transfer *xfer)
> > +{
> > + int cnt, i;
> > + struct mtk_spi *mdata = spi_master_get_devdata(master);
> > +
> > + mdata->cur_transfer = xfer;
> > + mdata->xfer_len = xfer->len;
> > + mtk_spi_prepare_transfer(master, xfer);
> > + mtk_spi_setup_packet(master);
> > +
> > + if (xfer->len % 4)
> > + cnt = xfer->len / 4 + 1;
> > + else
> > + cnt = xfer->len / 4;
> > +
> > + for (i = 0; i < cnt; i++)
> > + writel(*((u32 *)xfer->tx_buf + i),
>
> This will access past the end of tx_buf if len%4 > 0.

SPI_TX_DATA_REG requires write 4 bytes once. If len%4 > 0, this just
reads more data from dram(xfer->tx_buf) to register, and that spi hw
only tranfer length according to xfer->len, so I think it doesn't
matter.

> > + mdata->base + SPI_TX_DATA_REG);
> > +
> > + mtk_spi_enable_transfer(master);
> > +
> > + return 1;
> > +}
> > +
> > +static int mtk_spi_dma_transfer(struct spi_master *master,
> > + struct spi_device *spi,
> > + struct spi_transfer *xfer)
> > +{
> > + int cmd;
> > + struct mtk_spi *mdata = spi_master_get_devdata(master);
> > +
> > + mdata->tx_sgl = NULL;
> > + mdata->rx_sgl = NULL;
> > + mdata->tx_sgl_len = 0;
> > + mdata->rx_sgl_len = 0;
> > + mdata->cur_transfer = xfer;
> > +
> > + mtk_spi_prepare_transfer(master, xfer);
> > +
> > + cmd = readl(mdata->base + SPI_CMD_REG);
> > + if (xfer->tx_buf)
> > + cmd |= SPI_CMD_TX_DMA;
> > + if (xfer->rx_buf)
> > + cmd |= SPI_CMD_RX_DMA;
> > + writel(cmd, mdata->base + SPI_CMD_REG);
> > +
> > + if (xfer->tx_buf)
> > + mdata->tx_sgl = xfer->tx_sg.sgl;
> > + if (xfer->rx_buf)
> > + mdata->rx_sgl = xfer->rx_sg.sgl;
> > +
> > + if (mdata->tx_sgl) {
> > + xfer->tx_dma = sg_dma_address(mdata->tx_sgl);
> > + mdata->tx_sgl_len = sg_dma_len(mdata->tx_sgl);
> > + }
> > + if (mdata->rx_sgl) {
> > + xfer->rx_dma = sg_dma_address(mdata->rx_sgl);
> > + mdata->rx_sgl_len = sg_dma_len(mdata->rx_sgl);
> > + }
> > +
> > + mtk_spi_update_mdata_len(master);
> > + mtk_spi_setup_packet(master);
> > + mtk_spi_setup_dma_addr(master, xfer);
> > + mtk_spi_enable_transfer(master);
> > +
> > + return 1;
> > +}
> > +
> > +static int mtk_spi_transfer_one(struct spi_master *master,
> > + struct spi_device *spi,
> > + struct spi_transfer *xfer)
> > +{
> > + if (master->can_dma(master, spi, xfer))
> > + return mtk_spi_dma_transfer(master, spi, xfer);
> > + else
> > + return mtk_spi_fifo_transfer(master, spi, xfer);
> > +}
> > +
> > +static bool mtk_spi_can_dma(struct spi_master *master,
> > + struct spi_device *spi,
> > + struct spi_transfer *xfer)
> > +{
> > + return xfer->len > MTK_SPI_MAX_FIFO_SIZE;
> > +}
> > +
> > +static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
> > +{
> > + u32 cmd, reg_val, i;
> > + struct spi_master *master = dev_id;
> > + struct mtk_spi *mdata = spi_master_get_devdata(master);
> > + struct spi_transfer *trans = mdata->cur_transfer;
> > +
> > + reg_val = readl(mdata->base + SPI_STATUS0_REG);
> > + if (reg_val & 0x2)
>
> define 0x2?

OK. I will fix it.

> > + mdata->state = MTK_SPI_PAUSED;
> > + else
> > + mdata->state = MTK_SPI_IDLE;
> > +
> > + if (!master->can_dma(master, master->cur_msg->spi, trans)) {
> > + /* xfer len is not N*4 bytes every time in a transfer,
> > + * but SPI_RX_DATA_REG must reads 4 bytes once,
> > + * so rx buffer byte by byte.
> > + */
> > + if (trans->rx_buf) {
> > + for (i = 0; i < mdata->xfer_len; i++) {
> > + if (i % 4 == 0)
> > + reg_val =
> > + readl(mdata->base + SPI_RX_DATA_REG);
>
> Strange indentation. Might be better to put readl on the same line,
> and SPI_RX_DATA_REG on the next one.

OK. I will fix it.

> > + *((u8 *)(trans->rx_buf + i)) =
> > + (reg_val >> ((i % 4) * 8)) & 0xff;
>
> This feels a bit awkward... Also, & 0xff is not needed.
>
OK, I will fix it.

> > +
> > +static int mtk_spi_probe(struct platform_device *pdev)
> > +{
> > + struct spi_master *master;
> > + struct mtk_spi *mdata;
> > + const struct of_device_id *of_id;
> > + struct resource *res;
> > + int irq, ret;
>
> Space, not tab, between int and irq.
>
OK. I will fix it.

> > +
> > + master = spi_alloc_master(&pdev->dev, sizeof(*mdata));
> > + if (!master) {
> > + dev_err(&pdev->dev, "failed to alloc spi master\n");