Skip to contentSkip to conversation

MXS SPI driver

PATCH Superseded Marek Vasut 8 participants 30 messages Jul 23, 2012 – Aug 3, 2012

Subsystems (5)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

v2 cover MXS SPI driver

This patchset adds SPI master support for Freescale i.MX233/i.MX28. This is rebased on top of next-20120713. Please consider applying, bugs shall now be squashed during the previous two review rounds.

Marek Vasut (10):
mmc: spi: Move SSP register definitions into separate file
mmc: spi: Rename IMX2[38]_MMC to IMX2[38]_SSP
mmc: spi: Add necessary bits into mxs-spi.h
mmc: spi: Pull out parts shared between MMC and SPI
mmc: spi: Pull out the SSP clock configuration function
spi: Add SPI driver for mx233/mx28
mmc: spi: Pull out common DMA parts from MXS MMC
spi: Add DMA support into SPI driver
spi: Add SSP/SPI device tree documentation
ARM: mx28: Add SPI pinmux into imx28.dtsi

 Documentation/devicetree/bindings/spi/mxs-spi.txt |   18 +
 arch/arm/boot/dts/imx28.dtsi                      |   39 ++
 drivers/clk/mxs/Makefile                          |    2 +-
 drivers/clk/mxs/clk-ssp.c                         |   62 ++
 drivers/mmc/host/mxs-mmc.c                        |  325 ++++-------
 drivers/spi/Kconfig                               |    7 +
 drivers/spi/Makefile                              |    1 +
 drivers/spi/spi-mxs.c                             |  626 +++++++++++++++++++++
 include/linux/spi/mxs-spi.h                       |  150 +++++
 9 files changed, 1011 insertions(+), 219 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/spi/mxs-spi.txt
 create mode 100644 drivers/clk/mxs/clk-ssp.c
 create mode 100644 drivers/spi/spi-mxs.c
 create mode 100644 include/linux/spi/mxs-spi.h

V2: Fixes for MX233, to allow compiling as a module (thanks to Attila Kinali).

    Rebase on top of next-20120723
Cc Attila Kinali <[email protected]>
Cc Chris Ball <[email protected]>
CC Dong Aisheng <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc Grant Likely <[email protected]>
Cc Linux ARM kernel <[email protected]>
Cc Mark Brown <[email protected]>
CC Shawn Guo <[email protected]>
Signature · 1 line
1.7.10.4
#1 lore raw

PATCH RESEND v2 1/10 mmc: spi: Move SSP register definitions into separate file

Move the definitions into separate file so separate SPI driver can be implemented. The SSP controller in MXS can act both as a MMC host and as a SPI host.

Based on previous attempt by:
Fabio Estevam <[email protected]>

Cc Attila Kinali <[email protected]>
Cc Chris Ball <[email protected]>
CC Dong Aisheng <[email protected]>
Cc Grant Likely <[email protected]>
Cc Linux ARM kernel <[email protected]>
Cc Mark Brown <[email protected]>
CC Shawn Guo <[email protected]>
---
 drivers/mmc/host/mxs-mmc.c  |   87 ++--------------------------------
 include/linux/spi/mxs-spi.h |  109 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 112 insertions(+), 84 deletions(-)
 create mode 100644 include/linux/spi/mxs-spi.h
drivers/mmc/host/mxs-mmc.c +3 -84
index a51f930..26c95dc 100644
@@ -45,87 +45,10 @@
 #include <linux/pinctrl/consumer.h>
 #include <linux/stmp_device.h>
 #include <linux/mmc/mxs-mmc.h>
+#include <linux/spi/mxs-spi.h>
 
 #define DRIVER_NAME	"mxs-mmc"
 
-/* card detect polling timeout */
-#define MXS_MMC_DETECT_TIMEOUT			(HZ/2)
-
-#define ssp_is_old(host)	((host)->devid == IMX23_MMC)
-
-/* SSP registers */
-#define HW_SSP_CTRL0				0x000
-#define  BM_SSP_CTRL0_RUN			(1 << 29)
-#define  BM_SSP_CTRL0_SDIO_IRQ_CHECK		(1 << 28)
-#define  BM_SSP_CTRL0_IGNORE_CRC		(1 << 26)
-#define  BM_SSP_CTRL0_READ			(1 << 25)
-#define  BM_SSP_CTRL0_DATA_XFER			(1 << 24)
-#define  BP_SSP_CTRL0_BUS_WIDTH			(22)
-#define  BM_SSP_CTRL0_BUS_WIDTH			(0x3 << 22)
-#define  BM_SSP_CTRL0_WAIT_FOR_IRQ		(1 << 21)
-#define  BM_SSP_CTRL0_LONG_RESP			(1 << 19)
-#define  BM_SSP_CTRL0_GET_RESP			(1 << 17)
-#define  BM_SSP_CTRL0_ENABLE			(1 << 16)
-#define  BP_SSP_CTRL0_XFER_COUNT		(0)
-#define  BM_SSP_CTRL0_XFER_COUNT		(0xffff)
-#define HW_SSP_CMD0				0x010
-#define  BM_SSP_CMD0_DBL_DATA_RATE_EN		(1 << 25)
-#define  BM_SSP_CMD0_SLOW_CLKING_EN		(1 << 22)
-#define  BM_SSP_CMD0_CONT_CLKING_EN		(1 << 21)
-#define  BM_SSP_CMD0_APPEND_8CYC		(1 << 20)
-#define  BP_SSP_CMD0_BLOCK_SIZE			(16)
-#define  BM_SSP_CMD0_BLOCK_SIZE			(0xf << 16)
-#define  BP_SSP_CMD0_BLOCK_COUNT		(8)
-#define  BM_SSP_CMD0_BLOCK_COUNT		(0xff << 8)
-#define  BP_SSP_CMD0_CMD			(0)
-#define  BM_SSP_CMD0_CMD			(0xff)
-#define HW_SSP_CMD1				0x020
-#define HW_SSP_XFER_SIZE			0x030
-#define HW_SSP_BLOCK_SIZE			0x040
-#define  BP_SSP_BLOCK_SIZE_BLOCK_COUNT		(4)
-#define  BM_SSP_BLOCK_SIZE_BLOCK_COUNT		(0xffffff << 4)
-#define  BP_SSP_BLOCK_SIZE_BLOCK_SIZE		(0)
-#define  BM_SSP_BLOCK_SIZE_BLOCK_SIZE		(0xf)
-#define HW_SSP_TIMING(h)			(ssp_is_old(h) ? 0x050 : 0x070)
-#define  BP_SSP_TIMING_TIMEOUT			(16)
-#define  BM_SSP_TIMING_TIMEOUT			(0xffff << 16)
-#define  BP_SSP_TIMING_CLOCK_DIVIDE		(8)
-#define  BM_SSP_TIMING_CLOCK_DIVIDE		(0xff << 8)
-#define  BP_SSP_TIMING_CLOCK_RATE		(0)
-#define  BM_SSP_TIMING_CLOCK_RATE		(0xff)
-#define HW_SSP_CTRL1(h)				(ssp_is_old(h) ? 0x060 : 0x080)
-#define  BM_SSP_CTRL1_SDIO_IRQ			(1 << 31)
-#define  BM_SSP_CTRL1_SDIO_IRQ_EN		(1 << 30)
-#define  BM_SSP_CTRL1_RESP_ERR_IRQ		(1 << 29)
-#define  BM_SSP_CTRL1_RESP_ERR_IRQ_EN		(1 << 28)
-#define  BM_SSP_CTRL1_RESP_TIMEOUT_IRQ		(1 << 27)
-#define  BM_SSP_CTRL1_RESP_TIMEOUT_IRQ_EN	(1 << 26)
-#define  BM_SSP_CTRL1_DATA_TIMEOUT_IRQ		(1 << 25)
-#define  BM_SSP_CTRL1_DATA_TIMEOUT_IRQ_EN	(1 << 24)
-#define  BM_SSP_CTRL1_DATA_CRC_IRQ		(1 << 23)
-#define  BM_SSP_CTRL1_DATA_CRC_IRQ_EN		(1 << 22)
-#define  BM_SSP_CTRL1_FIFO_UNDERRUN_IRQ		(1 << 21)
-#define  BM_SSP_CTRL1_FIFO_UNDERRUN_IRQ_EN	(1 << 20)
-#define  BM_SSP_CTRL1_RECV_TIMEOUT_IRQ		(1 << 17)
-#define  BM_SSP_CTRL1_RECV_TIMEOUT_IRQ_EN	(1 << 16)
-#define  BM_SSP_CTRL1_FIFO_OVERRUN_IRQ		(1 << 15)
-#define  BM_SSP_CTRL1_FIFO_OVERRUN_IRQ_EN	(1 << 14)
-#define  BM_SSP_CTRL1_DMA_ENABLE		(1 << 13)
-#define  BM_SSP_CTRL1_POLARITY			(1 << 9)
-#define  BP_SSP_CTRL1_WORD_LENGTH		(4)
-#define  BM_SSP_CTRL1_WORD_LENGTH		(0xf << 4)
-#define  BP_SSP_CTRL1_SSP_MODE			(0)
-#define  BM_SSP_CTRL1_SSP_MODE			(0xf)
-#define HW_SSP_SDRESP0(h)			(ssp_is_old(h) ? 0x080 : 0x0a0)
-#define HW_SSP_SDRESP1(h)			(ssp_is_old(h) ? 0x090 : 0x0b0)
-#define HW_SSP_SDRESP2(h)			(ssp_is_old(h) ? 0x0a0 : 0x0c0)
-#define HW_SSP_SDRESP3(h)			(ssp_is_old(h) ? 0x0b0 : 0x0d0)
-#define HW_SSP_STATUS(h)			(ssp_is_old(h) ? 0x0c0 : 0x100)
-#define  BM_SSP_STATUS_CARD_DETECT		(1 << 28)
-#define  BM_SSP_STATUS_SDIO_IRQ			(1 << 17)
-
-#define BF_SSP(value, field)	(((value) << BP_SSP_##field) & BM_SSP_##field)
-
 #define MXS_MMC_IRQ_BITS	(BM_SSP_CTRL1_SDIO_IRQ		| \
 				 BM_SSP_CTRL1_RESP_ERR_IRQ	| \
 				 BM_SSP_CTRL1_RESP_TIMEOUT_IRQ	| \
@@ -135,12 +58,8 @@
 				 BM_SSP_CTRL1_RECV_TIMEOUT_IRQ  | \
 				 BM_SSP_CTRL1_FIFO_OVERRUN_IRQ)
 
-#define SSP_PIO_NUM	3
-
-enum mxs_mmc_id {
-	IMX23_MMC,
-	IMX28_MMC,
-};
+/* card detect polling timeout */
+#define MXS_MMC_DETECT_TIMEOUT			(HZ/2)
 
 struct mxs_mmc_host {
 	struct mmc_host			*mmc;
include/linux/spi/mxs-spi.h new file +109 -0

109 lines not on this page · raw message

Signature · 1 line
1.7.10.4
#2 lore raw

PATCH RESEND v2 2/10 mmc: spi: Rename IMX2[38]_MMC to IMX2[38]_SSP

In reply to Marek Vasut

Since the SSP controller can act as both SPI and MMC host, renaming the enum to properly reflect the naming seems appropriate.

Based on previous attempt by:
Fabio Estevam <[email protected]>

Cc Attila Kinali <[email protected]>
Cc Chris Ball <[email protected]>
CC Dong Aisheng <[email protected]>
Cc Grant Likely <[email protected]>
Cc Linux ARM kernel <[email protected]>
Cc Mark Brown <[email protected]>
CC Shawn Guo <[email protected]>
---
 drivers/mmc/host/mxs-mmc.c  |   18 +++++++++---------
 include/linux/spi/mxs-spi.h |    8 ++++----
 2 files changed, 13 insertions(+), 13 deletions(-)
drivers/mmc/host/mxs-mmc.c +9 -9
index 26c95dc..e80c2b6 100644
@@ -78,7 +78,7 @@ struct mxs_mmc_host {
 	enum dma_transfer_direction	slave_dirn;
 	u32				ssp_pio_words[SSP_PIO_NUM];
 
-	enum mxs_mmc_id			devid;
+	enum mxs_ssp_id			devid;
 	unsigned char			bus_width;
 	spinlock_t			lock;
 	int				sdio_irq_en;
@@ -601,22 +601,22 @@ static bool mxs_mmc_dma_filter(struct dma_chan *chan, void *param)
 	return true;
 }
 
-static struct platform_device_id mxs_mmc_ids[] = {
+static struct platform_device_id mxs_ssp_ids[] = {
 	{
 		.name = "imx23-mmc",
-		.driver_data = IMX23_MMC,
+		.driver_data = IMX23_SSP,
 	}, {
 		.name = "imx28-mmc",
-		.driver_data = IMX28_MMC,
+		.driver_data = IMX28_SSP,
 	}, {
 		/* sentinel */
 	}
 };
-MODULE_DEVICE_TABLE(platform, mxs_mmc_ids);
+MODULE_DEVICE_TABLE(platform, mxs_ssp_ids);
 
 static const struct of_device_id mxs_mmc_dt_ids[] = {
-	{ .compatible = "fsl,imx23-mmc", .data = (void *) IMX23_MMC, },
-	{ .compatible = "fsl,imx28-mmc", .data = (void *) IMX28_MMC, },
+	{ .compatible = "fsl,imx23-mmc", .data = (void *) IMX23_SSP, },
+	{ .compatible = "fsl,imx28-mmc", .data = (void *) IMX28_SSP, },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, mxs_mmc_dt_ids);
@@ -655,7 +655,7 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 	}
 
 	if (np) {
-		host->devid = (enum mxs_mmc_id) of_id->data;
+		host->devid = (enum mxs_ssp_id) of_id->data;
 		/*
 		 * TODO: This is a temporary solution and should be changed
 		 * to use generic DMA binding later when the helpers get in.
@@ -829,7 +829,7 @@ static const struct dev_pm_ops mxs_mmc_pm_ops = {
 static struct platform_driver mxs_mmc_driver = {
 	.probe		= mxs_mmc_probe,
 	.remove		= mxs_mmc_remove,
-	.id_table	= mxs_mmc_ids,
+	.id_table	= mxs_ssp_ids,
 	.driver		= {
 		.name	= DRIVER_NAME,
 		.owner	= THIS_MODULE,
include/linux/spi/mxs-spi.h +4 -4
index b7ccd57..c08aef5 100644
@@ -24,7 +24,7 @@
 #ifndef __LINUX_SPI_MXS_SPI_H__
 #define __LINUX_SPI_MXS_SPI_H__
 
-#define ssp_is_old(host)	((host)->devid == IMX23_MMC)
+#define ssp_is_old(host)	((host)->devid == IMX23_SSP)
 
 /* SSP registers */
 #define HW_SSP_CTRL0				0x000
@@ -101,9 +101,9 @@
 
 #define SSP_PIO_NUM	3
 
-enum mxs_mmc_id {
-	IMX23_MMC,
-	IMX28_MMC,
+enum mxs_ssp_id {
+	IMX23_SSP,
+	IMX28_SSP,
 };
 
 #endif	/* __LINUX_SPI_MXS_SPI_H__ */
Signature · 1 line
1.7.10.4
#3 lore raw

PATCH v2 3/10 mmc: spi: Add necessary bits into mxs-spi.h

In reply to Marek Vasut

Add missing register bits and registers into mxs-spi.h . These will be used by the SPI driver.

Based on previous attempt by:
Fabio Estevam <[email protected]>

Cc Attila Kinali <[email protected]>
Cc Chris Ball <[email protected]>
CC Dong Aisheng <[email protected]>
Cc Grant Likely <[email protected]>
Cc Linux ARM kernel <[email protected]>
Cc Mark Brown <[email protected]>
CC Shawn Guo <[email protected]>
---
 include/linux/spi/mxs-spi.h |   22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

V2: Location of HW_SSP_DATA register differs on MX233 and MX28,
handle accordingly. (Thanks to Attila Kinali for finding this).

include/linux/spi/mxs-spi.h +22 -0
index c08aef5..7dfa1d7 100644
@@ -30,12 +30,14 @@
 #define HW_SSP_CTRL0				0x000
 #define  BM_SSP_CTRL0_RUN			(1 << 29)
 #define  BM_SSP_CTRL0_SDIO_IRQ_CHECK		(1 << 28)
+#define  BM_SSP_CTRL0_LOCK_CS			(1 << 27)
 #define  BM_SSP_CTRL0_IGNORE_CRC		(1 << 26)
 #define  BM_SSP_CTRL0_READ			(1 << 25)
 #define  BM_SSP_CTRL0_DATA_XFER			(1 << 24)
 #define  BP_SSP_CTRL0_BUS_WIDTH			22
 #define  BM_SSP_CTRL0_BUS_WIDTH			(0x3 << 22)
 #define  BM_SSP_CTRL0_WAIT_FOR_IRQ		(1 << 21)
+#define  BM_SSP_CTRL0_WAIT_FOR_CMD		(1 << 20)
 #define  BM_SSP_CTRL0_LONG_RESP			(1 << 19)
 #define  BM_SSP_CTRL0_GET_RESP			(1 << 17)
 #define  BM_SSP_CTRL0_ENABLE			(1 << 16)
@@ -64,8 +66,12 @@
 #define  BM_SSP_TIMING_TIMEOUT			(0xffff << 16)
 #define  BP_SSP_TIMING_CLOCK_DIVIDE		8
 #define  BM_SSP_TIMING_CLOCK_DIVIDE		(0xff << 8)
+#define  BF_SSP_TIMING_CLOCK_DIVIDE(v)		\
+			(((v) << 8) & BM_SSP_TIMING_CLOCK_DIVIDE)
 #define  BP_SSP_TIMING_CLOCK_RATE		0
 #define  BM_SSP_TIMING_CLOCK_RATE		0xff
+#define BF_SSP_TIMING_CLOCK_RATE(v)		\
+			(((v) << 0) & BM_SSP_TIMING_CLOCK_RATE)
 #define HW_SSP_CTRL1(h)				(ssp_is_old(h) ? 0x060 : 0x080)
 #define  BM_SSP_CTRL1_SDIO_IRQ			(1 << 31)
 #define  BM_SSP_CTRL1_SDIO_IRQ_EN		(1 << 30)
@@ -84,11 +90,26 @@
 #define  BM_SSP_CTRL1_FIFO_OVERRUN_IRQ		(1 << 15)
 #define  BM_SSP_CTRL1_FIFO_OVERRUN_IRQ_EN	(1 << 14)
 #define  BM_SSP_CTRL1_DMA_ENABLE		(1 << 13)
+#define  BM_SSP_CTRL1_PHASE			(1 << 10)
 #define  BM_SSP_CTRL1_POLARITY			(1 << 9)
 #define  BP_SSP_CTRL1_WORD_LENGTH		4
 #define  BM_SSP_CTRL1_WORD_LENGTH		(0xf << 4)
+#define  BF_SSP_CTRL1_WORD_LENGTH(v)		\
+			(((v) << 4) & BM_SSP_CTRL1_WORD_LENGTH)
+#define  BV_SSP_CTRL1_WORD_LENGTH__FOUR_BITS	0x3
+#define  BV_SSP_CTRL1_WORD_LENGTH__EIGHT_BITS	0x7
+#define  BV_SSP_CTRL1_WORD_LENGTH__SIXTEEN_BITS	0xF
 #define  BP_SSP_CTRL1_SSP_MODE			0
 #define  BM_SSP_CTRL1_SSP_MODE			0xf
+#define  BF_SSP_CTRL1_SSP_MODE(v)		\
+			(((v) << 0) & BM_SSP_CTRL1_SSP_MODE)
+#define  BV_SSP_CTRL1_SSP_MODE__SPI		0x0
+#define  BV_SSP_CTRL1_SSP_MODE__SSI		0x1
+#define  BV_SSP_CTRL1_SSP_MODE__SD_MMC		0x3
+#define  BV_SSP_CTRL1_SSP_MODE__MS		0x4
+
+#define HW_SSP_DATA(h)				(ssp_is_old(h) ? 0x070 : 0x090)
+
 #define HW_SSP_SDRESP0(h)			(ssp_is_old(h) ? 0x080 : 0x0a0)
 #define HW_SSP_SDRESP1(h)			(ssp_is_old(h) ? 0x090 : 0x0b0)
 #define HW_SSP_SDRESP2(h)			(ssp_is_old(h) ? 0x0a0 : 0x0c0)
@@ -96,6 +117,7 @@
 #define HW_SSP_STATUS(h)			(ssp_is_old(h) ? 0x0c0 : 0x100)
 #define  BM_SSP_STATUS_CARD_DETECT		(1 << 28)
 #define  BM_SSP_STATUS_SDIO_IRQ			(1 << 17)
+#define  BM_SSP_STATUS_FIFO_EMPTY		(1 << 5)
 
 #define BF_SSP(value, field)	(((value) << BP_SSP_##field) & BM_SSP_##field)
 
Signature · 1 line
1.7.10.4
#4 lore raw

PATCH RESEND v2 4/10 mmc: spi: Pull out parts shared between MMC and SPI

In reply to Marek Vasut

Abstract out the common part of private data shared between MMC and SPI. These shall later allow to use common clock configuration function.

Cc Attila Kinali <[email protected]>
Cc Chris Ball <[email protected]>
CC Dong Aisheng <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc Grant Likely <[email protected]>
Cc Linux ARM kernel <[email protected]>
Cc Mark Brown <[email protected]>
CC Shawn Guo <[email protected]>
---
 drivers/mmc/host/mxs-mmc.c  |  107 ++++++++++++++++++++++++-------------------
 include/linux/spi/mxs-spi.h |    8 ++++
 2 files changed, 67 insertions(+), 48 deletions(-)
drivers/mmc/host/mxs-mmc.c +59 -48
index e80c2b6..7b85e03 100644
@@ -62,23 +62,20 @@
 #define MXS_MMC_DETECT_TIMEOUT			(HZ/2)
 
 struct mxs_mmc_host {
+	struct mxs_ssp			ssp;
+
 	struct mmc_host			*mmc;
 	struct mmc_request		*mrq;
 	struct mmc_command		*cmd;
 	struct mmc_data			*data;
 
-	void __iomem			*base;
 	int				dma_channel;
-	struct clk			*clk;
-	unsigned int			clk_rate;
-
 	struct dma_chan         	*dmach;
 	struct mxs_dma_data		dma_data;
 	unsigned int			dma_dir;
 	enum dma_transfer_direction	slave_dirn;
 	u32				ssp_pio_words[SSP_PIO_NUM];
 
-	enum mxs_ssp_id			devid;
 	unsigned char			bus_width;
 	spinlock_t			lock;
 	int				sdio_irq_en;
@@ -105,16 +102,18 @@ static int mxs_mmc_get_ro(struct mmc_host *mmc)
 static int mxs_mmc_get_cd(struct mmc_host *mmc)
 {
 	struct mxs_mmc_host *host = mmc_priv(mmc);
+	struct mxs_ssp *ssp = &host->ssp;
 
-	return !(readl(host->base + HW_SSP_STATUS(host)) &
+	return !(readl(ssp->base + HW_SSP_STATUS(ssp)) &
 		 BM_SSP_STATUS_CARD_DETECT);
 }
 
 static void mxs_mmc_reset(struct mxs_mmc_host *host)
 {
+	struct mxs_ssp *ssp = &host->ssp;
 	u32 ctrl0, ctrl1;
 
-	stmp_reset_block(host->base);
+	stmp_reset_block(ssp->base);
 
 	ctrl0 = BM_SSP_CTRL0_IGNORE_CRC;
 	ctrl1 = BF_SSP(0x3, CTRL1_SSP_MODE) |
@@ -130,15 +129,15 @@ static void mxs_mmc_reset(struct mxs_mmc_host *host)
 	writel(BF_SSP(0xffff, TIMING_TIMEOUT) |
 	       BF_SSP(2, TIMING_CLOCK_DIVIDE) |
 	       BF_SSP(0, TIMING_CLOCK_RATE),
-	       host->base + HW_SSP_TIMING(host));
+	       ssp->base + HW_SSP_TIMING(ssp));
 
 	if (host->sdio_irq_en) {
 		ctrl0 |= BM_SSP_CTRL0_SDIO_IRQ_CHECK;
 		ctrl1 |= BM_SSP_CTRL1_SDIO_IRQ_EN;
 	}
 
-	writel(ctrl0, host->base + HW_SSP_CTRL0);
-	writel(ctrl1, host->base + HW_SSP_CTRL1(host));
+	writel(ctrl0, ssp->base + HW_SSP_CTRL0);
+	writel(ctrl1, ssp->base + HW_SSP_CTRL1(ssp));
 }
 
 static void mxs_mmc_start_cmd(struct mxs_mmc_host *host,
@@ -149,15 +148,16 @@ static void mxs_mmc_request_done(struct mxs_mmc_host *host)
 	struct mmc_command *cmd = host->cmd;
 	struct mmc_data *data = host->data;
 	struct mmc_request *mrq = host->mrq;
+	struct mxs_ssp *ssp = &host->ssp;
 
 	if (mmc_resp_type(cmd) & MMC_RSP_PRESENT) {
 		if (mmc_resp_type(cmd) & MMC_RSP_136) {
-			cmd->resp[3] = readl(host->base + HW_SSP_SDRESP0(host));
-			cmd->resp[2] = readl(host->base + HW_SSP_SDRESP1(host));
-			cmd->resp[1] = readl(host->base + HW_SSP_SDRESP2(host));
-			cmd->resp[0] = readl(host->base + HW_SSP_SDRESP3(host));
+			cmd->resp[3] = readl(ssp->base + HW_SSP_SDRESP0(ssp));
+			cmd->resp[2] = readl(ssp->base + HW_SSP_SDRESP1(ssp));
+			cmd->resp[1] = readl(ssp->base + HW_SSP_SDRESP2(ssp));
+			cmd->resp[0] = readl(ssp->base + HW_SSP_SDRESP3(ssp));
 		} else {
-			cmd->resp[0] = readl(host->base + HW_SSP_SDRESP0(host));
+			cmd->resp[0] = readl(ssp->base + HW_SSP_SDRESP0(ssp));
 		}
 	}
 
@@ -196,13 +196,14 @@ static irqreturn_t mxs_mmc_irq_handler(int irq, void *dev_id)
 	struct mxs_mmc_host *host = dev_id;
 	struct mmc_command *cmd = host->cmd;
 	struct mmc_data *data = host->data;
+	struct mxs_ssp *ssp = &host->ssp;
 	u32 stat;
 
 	spin_lock(&host->lock);
 
-	stat = readl(host->base + HW_SSP_CTRL1(host));
+	stat = readl(ssp->base + HW_SSP_CTRL1(ssp));
 	writel(stat & MXS_MMC_IRQ_BITS,
-	       host->base + HW_SSP_CTRL1(host) + STMP_OFFSET_REG_CLR);
+	       ssp->base + HW_SSP_CTRL1(ssp) + STMP_OFFSET_REG_CLR);
 
 	if ((stat & BM_SSP_CTRL1_SDIO_IRQ) && (stat & BM_SSP_CTRL1_SDIO_IRQ_EN))
 		mmc_signal_sdio_irq(host->mmc);
@@ -366,6 +367,8 @@ static void mxs_mmc_adtc(struct mxs_mmc_host *host)
 	unsigned int data_size = 0, log2_blksz;
 	unsigned int blocks = data->blocks;
 
+	struct mxs_ssp *ssp = &host->ssp;
+
 	u32 ignore_crc, get_resp, long_resp, read;
 	u32 ctrl0, cmd0, cmd1, val;
 
@@ -408,15 +411,15 @@ static void mxs_mmc_adtc(struct mxs_mmc_host *host)
 		blocks = 1;
 
 	/* xfer count, block size and count need to be set differently */
-	if (ssp_is_old(host)) {
+	if (ssp_is_old(ssp)) {
 		ctrl0 |= BF_SSP(data_size, CTRL0_XFER_COUNT);
 		cmd0 |= BF_SSP(log2_blksz, CMD0_BLOCK_SIZE) |
 			BF_SSP(blocks - 1, CMD0_BLOCK_COUNT);
 	} else {
-		writel(data_size, host->base + HW_SSP_XFER_SIZE);
+		writel(data_size, ssp->base + HW_SSP_XFER_SIZE);
 		writel(BF_SSP(log2_blksz, BLOCK_SIZE_BLOCK_SIZE) |
 		       BF_SSP(blocks - 1, BLOCK_SIZE_BLOCK_COUNT),
-		       host->base + HW_SSP_BLOCK_SIZE);
+		       ssp->base + HW_SSP_BLOCK_SIZE);
 	}
 
 	if ((cmd->opcode == MMC_STOP_TRANSMISSION) ||
@@ -431,11 +434,11 @@ static void mxs_mmc_adtc(struct mxs_mmc_host *host)
 	}
 
 	/* set the timeout count */
-	timeout = mxs_ns_to_ssp_ticks(host->clk_rate, data->timeout_ns);
-	val = readl(host->base + HW_SSP_TIMING(host));
+	timeout = mxs_ns_to_ssp_ticks(ssp->clk_rate, data->timeout_ns);
+	val = readl(ssp->base + HW_SSP_TIMING(ssp));
 	val &= ~(BM_SSP_TIMING_TIMEOUT);
 	val |= BF_SSP(timeout, TIMING_TIMEOUT);
-	writel(val, host->base + HW_SSP_TIMING(host));
+	writel(val, ssp->base + HW_SSP_TIMING(ssp));
 
 	/* pio */
 	host->ssp_pio_words[0] = ctrl0;
@@ -500,11 +503,12 @@ static void mxs_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 static void mxs_mmc_set_clk_rate(struct mxs_mmc_host *host, unsigned int rate)
 {
+	struct mxs_ssp *ssp = &host->ssp;
 	unsigned int ssp_clk, ssp_sck;
 	u32 clock_divide, clock_rate;
 	u32 val;
 
-	ssp_clk = clk_get_rate(host->clk);
+	ssp_clk = clk_get_rate(ssp->clk);
 
 	for (clock_divide = 2; clock_divide <= 254; clock_divide += 2) {
 		clock_rate = DIV_ROUND_UP(ssp_clk, rate * clock_divide);
@@ -521,13 +525,13 @@ static void mxs_mmc_set_clk_rate(struct mxs_mmc_host *host, unsigned int rate)
 
 	ssp_sck = ssp_clk / clock_divide / (1 + clock_rate);
 
-	val = readl(host->base + HW_SSP_TIMING(host));
+	val = readl(ssp->base + HW_SSP_TIMING(ssp));
 	val &= ~(BM_SSP_TIMING_CLOCK_DIVIDE | BM_SSP_TIMING_CLOCK_RATE);
 	val |= BF_SSP(clock_divide, TIMING_CLOCK_DIVIDE);
 	val |= BF_SSP(clock_rate, TIMING_CLOCK_RATE);
-	writel(val, host->base + HW_SSP_TIMING(host));
+	writel(val, ssp->base + HW_SSP_TIMING(ssp));
 
-	host->clk_rate = ssp_sck;
+	ssp->clk_rate = ssp_sck;
 
 	dev_dbg(mmc_dev(host->mmc),
 		"%s: clock_divide %d, clock_rate %d, ssp_clk %d, rate_actual %d, rate_requested %d\n",
@@ -552,6 +556,7 @@ static void mxs_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 static void mxs_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
 {
 	struct mxs_mmc_host *host = mmc_priv(mmc);
+	struct mxs_ssp *ssp = &host->ssp;
 	unsigned long flags;
 
 	spin_lock_irqsave(&host->lock, flags);
@@ -560,19 +565,19 @@ static void mxs_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
 
 	if (enable) {
 		writel(BM_SSP_CTRL0_SDIO_IRQ_CHECK,
-		       host->base + HW_SSP_CTRL0 + STMP_OFFSET_REG_SET);
+		       ssp->base + HW_SSP_CTRL0 + STMP_OFFSET_REG_SET);
 		writel(BM_SSP_CTRL1_SDIO_IRQ_EN,
-		       host->base + HW_SSP_CTRL1(host) + STMP_OFFSET_REG_SET);
+		       ssp->base + HW_SSP_CTRL1(ssp) + STMP_OFFSET_REG_SET);
 
-		if (readl(host->base + HW_SSP_STATUS(host)) &
+		if (readl(ssp->base + HW_SSP_STATUS(ssp)) &
 				BM_SSP_STATUS_SDIO_IRQ)
 			mmc_signal_sdio_irq(host->mmc);
 
 	} else {
 		writel(BM_SSP_CTRL0_SDIO_IRQ_CHECK,
-		       host->base + HW_SSP_CTRL0 + STMP_OFFSET_REG_CLR);
+		       ssp->base + HW_SSP_CTRL0 + STMP_OFFSET_REG_CLR);
 		writel(BM_SSP_CTRL1_SDIO_IRQ_EN,
-		       host->base + HW_SSP_CTRL1(host) + STMP_OFFSET_REG_CLR);
+		       ssp->base + HW_SSP_CTRL1(ssp) + STMP_OFFSET_REG_CLR);
 	}
 
 	spin_unlock_irqrestore(&host->lock, flags);
@@ -635,6 +640,7 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 	dma_cap_mask_t mask;
 	struct regulator *reg_vmmc;
 	enum of_gpio_flags flags;
+	struct mxs_ssp *ssp;
 
 	iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	dmares = platform_get_resource(pdev, IORESOURCE_DMA, 0);
@@ -648,14 +654,16 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	host = mmc_priv(mmc);
-	host->base = devm_request_and_ioremap(&pdev->dev, iores);
-	if (!host->base) {
+	ssp = &host->ssp;
+	ssp->dev = &pdev->dev;
+	ssp->base = devm_request_and_ioremap(&pdev->dev, iores);
+	if (!ssp->base) {
 		ret = -EADDRNOTAVAIL;
 		goto out_mmc_free;
 	}
 
 	if (np) {
-		host->devid = (enum mxs_ssp_id) of_id->data;
+		ssp->devid = (enum mxs_ssp_id) of_id->data;
 		/*
 		 * TODO: This is a temporary solution and should be changed
 		 * to use generic DMA binding later when the helpers get in.
@@ -668,7 +676,7 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 			goto out_mmc_free;
 		}
 	} else {
-		host->devid = pdev->id_entry->driver_data;
+		ssp->devid = pdev->id_entry->driver_data;
 		host->dma_channel = dmares->start;
 	}
 
@@ -691,12 +699,12 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 		goto out_mmc_free;
 	}
 
-	host->clk = clk_get(&pdev->dev, NULL);
-	if (IS_ERR(host->clk)) {
-		ret = PTR_ERR(host->clk);
+	ssp->clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR(ssp->clk)) {
+		ret = PTR_ERR(ssp->clk);
 		goto out_mmc_free;
 	}
-	clk_prepare_enable(host->clk);
+	clk_prepare_enable(ssp->clk);
 
 	mxs_mmc_reset(host);
 
@@ -741,8 +749,8 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 
 	mmc->max_segs = 52;
 	mmc->max_blk_size = 1 << 0xf;
-	mmc->max_blk_count = (ssp_is_old(host)) ? 0xff : 0xffffff;
-	mmc->max_req_size = (ssp_is_old(host)) ? 0xffff : 0xffffffff;
+	mmc->max_blk_count = (ssp_is_old(ssp)) ? 0xff : 0xffffff;
+	mmc->max_req_size = (ssp_is_old(ssp)) ? 0xffff : 0xffffffff;
 	mmc->max_seg_size = dma_get_max_seg_size(host->dmach->device->dev);
 
 	platform_set_drvdata(pdev, mmc);
@@ -766,8 +774,8 @@ out_free_dma:
 	if (host->dmach)
 		dma_release_channel(host->dmach);
 out_clk_put:
-	clk_disable_unprepare(host->clk);
-	clk_put(host->clk);
+	clk_disable_unprepare(ssp->clk);
+	clk_put(ssp->clk);
 out_mmc_free:
 	mmc_free_host(mmc);
 	return ret;
@@ -777,6 +785,7 @@ static int mxs_mmc_remove(struct platform_device *pdev)
 {
 	struct mmc_host *mmc = platform_get_drvdata(pdev);
 	struct mxs_mmc_host *host = mmc_priv(mmc);
+	struct mxs_ssp *ssp = &host->ssp;
 
 	mmc_remove_host(mmc);
 
@@ -785,8 +794,8 @@ static int mxs_mmc_remove(struct platform_device *pdev)
 	if (host->dmach)
 		dma_release_channel(host->dmach);
 
-	clk_disable_unprepare(host->clk);
-	clk_put(host->clk);
+	clk_disable_unprepare(ssp->clk);
+	clk_put(ssp->clk);
 
 	mmc_free_host(mmc);
 
@@ -798,11 +807,12 @@ static int mxs_mmc_suspend(struct device *dev)
 {
 	struct mmc_host *mmc = dev_get_drvdata(dev);
 	struct mxs_mmc_host *host = mmc_priv(mmc);
+	struct mxs_ssp *ssp = &host->ssp;
 	int ret = 0;
 
 	ret = mmc_suspend_host(mmc);
 
-	clk_disable_unprepare(host->clk);
+	clk_disable_unprepare(ssp->clk);
 
 	return ret;
 }
@@ -811,9 +821,10 @@ static int mxs_mmc_resume(struct device *dev)
 {
 	struct mmc_host *mmc = dev_get_drvdata(dev);
 	struct mxs_mmc_host *host = mmc_priv(mmc);
+	struct mxs_ssp *ssp = &host->ssp;
 	int ret = 0;
 
-	clk_prepare_enable(host->clk);
+	clk_prepare_enable(ssp->clk);
 
 	ret = mmc_resume_host(mmc);
 
include/linux/spi/mxs-spi.h +8 -0
index 7dfa1d7..475f69f 100644
@@ -128,4 +128,12 @@ enum mxs_ssp_id {
 	IMX28_SSP,
 };
 
+struct mxs_ssp {
+	struct device			*dev;
+	void __iomem			*base;
+	struct clk			*clk;
+	unsigned int			clk_rate;
+	enum mxs_ssp_id			devid;
+};
+
 #endif	/* __LINUX_SPI_MXS_SPI_H__ */
Signature · 1 line
1.7.10.4
#5 lore raw

PATCH v2 5/10 mmc: spi: Pull out the SSP clock configuration function

In reply to Marek Vasut

Pull out the MMC clock configuration function and make it into SSP clock configuration function, so it can be used by the SPI driver too.

Cc Attila Kinali <[email protected]>
Cc Chris Ball <[email protected]>
CC Dong Aisheng <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc Grant Likely <[email protected]>
Cc Linux ARM kernel <[email protected]>
Cc Mark Brown <[email protected]>
CC Shawn Guo <[email protected]>
---
 drivers/clk/mxs/Makefile    |    2 +-
 drivers/clk/mxs/clk-ssp.c   |   62 +++++++++++++++++++++++++++++++++++++++++++
 drivers/mmc/host/mxs-mmc.c  |   39 +--------------------------
 include/linux/spi/mxs-spi.h |    2 ++
 4 files changed, 66 insertions(+), 39 deletions(-)
 create mode 100644 drivers/clk/mxs/clk-ssp.c

V2: Add EXPORT_SYMBOL_GPL(mxs_ssp_set_clk_rate); to allow compiling as a module.
(Thanks to Attila Kinali).

drivers/clk/mxs/Makefile +1 -1
index 7bedeec..a6a2223 100644
@@ -2,7 +2,7 @@
 # Makefile for mxs specific clk
 #
 
-obj-y += clk.o clk-pll.o clk-ref.o clk-div.o clk-frac.o
+obj-y += clk.o clk-pll.o clk-ref.o clk-div.o clk-frac.o clk-ssp.o
 
 obj-$(CONFIG_SOC_IMX23) += clk-imx23.o
 obj-$(CONFIG_SOC_IMX28) += clk-imx28.o
drivers/clk/mxs/clk-ssp.c new file +62 -0

62 lines not on this page · raw message

drivers/mmc/host/mxs-mmc.c +1 -38
index 7b85e03..0813340 100644
@@ -501,43 +501,6 @@ static void mxs_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	mxs_mmc_start_cmd(host, mrq->cmd);
 }
 
-static void mxs_mmc_set_clk_rate(struct mxs_mmc_host *host, unsigned int rate)
-{
-	struct mxs_ssp *ssp = &host->ssp;
-	unsigned int ssp_clk, ssp_sck;
-	u32 clock_divide, clock_rate;
-	u32 val;
-
-	ssp_clk = clk_get_rate(ssp->clk);
-
-	for (clock_divide = 2; clock_divide <= 254; clock_divide += 2) {
-		clock_rate = DIV_ROUND_UP(ssp_clk, rate * clock_divide);
-		clock_rate = (clock_rate > 0) ? clock_rate - 1 : 0;
-		if (clock_rate <= 255)
-			break;
-	}
-
-	if (clock_divide > 254) {
-		dev_err(mmc_dev(host->mmc),
-			"%s: cannot set clock to %d\n", __func__, rate);
-		return;
-	}
-
-	ssp_sck = ssp_clk / clock_divide / (1 + clock_rate);
-
-	val = readl(ssp->base + HW_SSP_TIMING(ssp));
-	val &= ~(BM_SSP_TIMING_CLOCK_DIVIDE | BM_SSP_TIMING_CLOCK_RATE);
-	val |= BF_SSP(clock_divide, TIMING_CLOCK_DIVIDE);
-	val |= BF_SSP(clock_rate, TIMING_CLOCK_RATE);
-	writel(val, ssp->base + HW_SSP_TIMING(ssp));
-
-	ssp->clk_rate = ssp_sck;
-
-	dev_dbg(mmc_dev(host->mmc),
-		"%s: clock_divide %d, clock_rate %d, ssp_clk %d, rate_actual %d, rate_requested %d\n",
-		__func__, clock_divide, clock_rate, ssp_clk, ssp_sck, rate);
-}
-
 static void mxs_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 {
 	struct mxs_mmc_host *host = mmc_priv(mmc);
@@ -550,7 +513,7 @@ static void mxs_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		host->bus_width = 0;
 
 	if (ios->clock)
-		mxs_mmc_set_clk_rate(host, ios->clock);
+		mxs_ssp_set_clk_rate(&host->ssp, ios->clock);
 }
 
 static void mxs_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
include/linux/spi/mxs-spi.h +2 -0
index 475f69f..d07f8dc 100644
@@ -136,4 +136,6 @@ struct mxs_ssp {
 	enum mxs_ssp_id			devid;
 };
 
+void mxs_ssp_set_clk_rate(struct mxs_ssp *ssp, unsigned int rate);
+
 #endif	/* __LINUX_SPI_MXS_SPI_H__ */
Signature · 1 line
1.7.10.4
#6 lore raw

PATCH v2 6/10 spi: Add SPI driver for mx233/mx28

In reply to Marek Vasut

This is slightly reworked version of the SPI driver. Support for DT has been added and it's been converted to queued API.

Based on previous attempt by:
Fabio Estevam <[email protected]>

Cc Attila Kinali <[email protected]>
Cc Chris Ball <[email protected]>
CC Dong Aisheng <[email protected]>
Cc Grant Likely <[email protected]>
Cc Linux ARM kernel <[email protected]>
Cc Mark Brown <[email protected]>
CC Shawn Guo <[email protected]>
---
 drivers/spi/Kconfig   |    7 +
 drivers/spi/Makefile  |    1 +
 drivers/spi/spi-mxs.c |  428 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 436 insertions(+)
 create mode 100644 drivers/spi/spi-mxs.c

V2: Adjust the HW_SSP_DATA usage as it's now a parametrized macro

drivers/spi/Kconfig +7 -0
index 5f84b55..6992b30 100644
@@ -364,6 +364,13 @@ config SPI_STMP3XXX
 	help
 	  SPI driver for Freescale STMP37xx/378x SoC SSP interface
 
+config SPI_MXS
+	tristate "Freescale MXS SPI controller"
+	depends on ARCH_MXS
+	select STMP_DEVICE
+	help
+	  SPI driver for Freescale MXS devices.
+
 config SPI_TEGRA
 	tristate "Nvidia Tegra SPI controller"
 	depends on ARCH_TEGRA && (TEGRA_SYSTEM_DMA || TEGRA20_APB_DMA)
drivers/spi/Makefile +1 -0
index 3920dcf..3b72d87 100644
@@ -36,6 +36,7 @@ obj-$(CONFIG_SPI_LM70_LLP)		+= spi-lm70llp.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_MXS)			+= spi-mxs.o
 obj-$(CONFIG_SPI_NUC900)		+= spi-nuc900.o
 obj-$(CONFIG_SPI_OC_TINY)		+= spi-oc-tiny.o
 obj-$(CONFIG_SPI_OMAP_UWIRE)		+= spi-omap-uwire.o
drivers/spi/spi-mxs.c new file +428 -0

428 lines not on this page · raw message

Signature · 1 line
1.7.10.4
#7 lore raw

PATCH v2 6/10 spi: Add SPI driver for mx233/mx28

Quoted 3 lines · Marek Vasut

This is slightly reworked version of the SPI driver. Support for DT has been added and it's been converted to queued API.

Looks reasonable overall.

Quoted 8 lines
drivers/spi/spi-mxs.c new file +8 -0
+	bits_per_word = dev->bits_per_word;
+	if (t && t->bits_per_word)
+		bits_per_word = t->bits_per_word;
+
+	if (bits_per_word != 8) {
+		dev_err(&dev->dev, "%s, unsupported bits_per_word=%d\n",
+					__func__, bits_per_word);
+		return -EINVAL;

drivers/spi/spi-mxs.c line 88

Quoted 10 lines
drivers/spi/spi-mxs.c new file +9 -0
+	}
 
+	if (dev->max_speed_hz)
+		hz = dev->max_speed_hz;
+	if (t && t->speed_hz)
+		hz = t->speed_hz;
+	if (hz == 0) {
+		dev_err(&dev->dev, "Cannot continue with zero clock\n");
+		return -EINVAL;
+	}

These two blocks use a different style (the first does the first assign unconditionally, the second uses initialisation with declaration). I prefer the first style but YMMV and it doesn't matter.

For the missing clock rate might it make sense to just use whatever the clock happens to come out as?

drivers/spi/spi-mxs.c line 108

Quoted 4 lines
drivers/spi/spi-mxs.c new file +4 -0
+static void mxs_spi_cleanup(struct spi_device *dev)
+{
+	return;
+}

Empty functions are generally a warning sign... why do we need this one?

drivers/spi/spi-mxs.c line 405

Quoted 4 lines
drivers/spi/spi-mxs.c new file +3 -0
+static int __devexit mxs_spi_remove(struct platform_device *pdev)
+{
 
+	clk_disable_unprepare(ssp->clk);

It'd be nice to only keep the clocks enabled while doing transfers but again totally non-essential.

#8 lore raw

PATCH v2 6/10 spi: Add SPI driver for mx233/mx28

Dear Mark Brown,

Thanks for the review!

Quoted 30 lines · Marek Vasut
Quoted 3 lines

This is slightly reworked version of the SPI driver. Support for DT has been added and it's been converted to queued API.

Looks reasonable overall.

Quoted 8 lines
drivers/spi/spi-mxs.c new file +8 -0
+	bits_per_word = dev->bits_per_word;
+	if (t && t->bits_per_word)
+		bits_per_word = t->bits_per_word;
+
+	if (bits_per_word != 8) {
+		dev_err(&dev->dev, "%s, unsupported bits_per_word=%d\n",
+					__func__, bits_per_word);
+		return -EINVAL;

drivers/spi/spi-mxs.c line 88

Quoted 10 lines
drivers/spi/spi-mxs.c new file +9 -0
+	}
 
+	if (dev->max_speed_hz)
+		hz = dev->max_speed_hz;
+	if (t && t->speed_hz)
+		hz = t->speed_hz;
+	if (hz == 0) {
+		dev_err(&dev->dev, "Cannot continue with zero clock\n");
+		return -EINVAL;
+	}

These two blocks use a different style (the first does the first assign unconditionally, the second uses initialisation with declaration). I prefer the first style but YMMV and it doesn't matter.

Quoted 2 lines

For the missing clock rate might it make sense to just use whatever the clock happens to come out as?

Hm, the max_speed_hz should be the cap for the transfer speed. I will change the function to this:

hz = dev->max_speed_hz;
if (t && t->speed_hz)

        hz = min(hz, t->speed_hz);
Quoted 7 lines

drivers/spi/spi-mxs.c line 108

Quoted 4 lines
drivers/spi/spi-mxs.c new file +4 -0
+static void mxs_spi_cleanup(struct spi_device *dev)
+{
+	return;
+}

Empty functions are generally a warning sign... why do we need this one?

Good catch, thanks!

Quoted 7 lines

drivers/spi/spi-mxs.c line 405

Quoted 4 lines
drivers/spi/spi-mxs.c new file +3 -0
+static int __devexit mxs_spi_remove(struct platform_device *pdev)
+{
 
+	clk_disable_unprepare(ssp->clk);

It'd be nice to only keep the clocks enabled while doing transfers but again totally non-essential.

Hm, this is spread across mxs. Shawn, is there any plan for PM implementation for MXS ?

Best regards,
Marek Vasut

#9 lore raw

PATCH v2 6/10 spi: Add SPI driver for mx233/mx28

Quoted 5 lines · Marek Vasut
Quoted 2 lines

It'd be nice to only keep the clocks enabled while doing transfers but again totally non-essential.

Hm, this is spread across mxs. Shawn, is there any plan for PM implementation for MXS ?

Take a look at s3c64xx - I did something with runtime PM for this, it's something that can reasonably be implemented in drivers and as far as I can tell most framework implementations end up using runtime PM anyway.

#10 lore raw

PATCH v2 6/10 spi: Add SPI driver for mx233/mx28

Dear Mark Brown,

Quoted 10 lines · Marek Vasut
Quoted 5 lines
Quoted 2 lines

It'd be nice to only keep the clocks enabled while doing transfers but again totally non-essential.

Hm, this is spread across mxs. Shawn, is there any plan for PM implementation for MXS ?

Take a look at s3c64xx - I did something with runtime PM for this, it's something that can reasonably be implemented in drivers and as far as I can tell most framework implementations end up using runtime PM anyway.

We don't even support runtime pm on mxs yet IIRC

Best regards,
Marek Vasut

#11 lore raw

PATCH v2 6/10 spi: Add SPI driver for mx233/mx28

In reply to Marek Vasut
Quoted 3 lines · Marek Vasut

Hm, this is spread across mxs. Shawn, is there any plan for PM implementation for MXS ?

Near term, no. Long term, yes.

Signature · 2 lines
Regards,
Shawn
#12 lore raw

PATCH v2 6/10 spi: Add SPI driver for mx233/mx28

In reply to Marek Vasut

Marek,

Le Mon, 23 Jul 2012 22:40:48 +0200,
Marek Vasut <[email protected]> a ?crit :

drivers/spi/spi-mxs.c line 139

Quoted 11 lines
drivers/spi/spi-mxs.c new file +11 -0
+static uint32_t mxs_spi_cs_to_reg(unsigned cs)
+{
+	uint32_t select = 0;
+
+	if (cs & 1)
+		select |= BM_SSP_CTRL0_WAIT_FOR_CMD;
+	if (cs & 2)
+		select |= BM_SSP_CTRL0_WAIT_FOR_IRQ;
+
+	return select;
+}

It sounds really strange to manipulate WAIT_FOR_CMD and WAIT_FOR_IRQ bits to adjust the chip select, and when reading the driver, it seemed suspicious to me. After going through the datasheet, indeed those bits are the appropriate one to select between the SS0, SS1 and SS2 chip selects, but I find the code not really obvious. Would it be possible to make it more obvious either by adding or comment or doing something like:

/* Should be put in some header file */
#define BM_SSP_CTRL0_SPI_CS_BITS (20)

+static void mxs_spi_set_cs(struct mxs_spi *spi, unsigned cs) +{
+ struct mxs_ssp *ssp = &spi->ssp;
+
+ writel(0x3 << BM_SSP_CTRL0_SPI_CS_BITS,
+ ssp->base + HW_SSP_CTRL0 + STMP_OFFSET_REG_CLR);
+ writel(cs,
+ ssp->base + HW_SSP_CTRL0 + STMP_OFFSET_REG_SET);
+}

Regards,

Thomas

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
reviewer
#13 lore raw

PATCH v2 6/10 spi: Add SPI driver for mx233/mx28

Quoted 12 lines · Thomas Petazzoni

It sounds really strange to manipulate WAIT_FOR_CMD and WAIT_FOR_IRQ bits to adjust the chip select, and when reading the driver, it seemed suspicious to me. After going through the datasheet, indeed those bits are the appropriate one to select between the SS0, SS1 and SS2 chip selects, but I find the code not really obvious. Would it be possible to make it more obvious either by adding or comment or doing something like:

/* Should be put in some header file */
#define BM_SSP_CTRL0_SPI_CS_BITS (20)

drivers/spi/spi-mxs.c line 171

Quoted 9 lines
drivers/spi/spi-mxs.c new file +9 -0
+static void mxs_spi_set_cs(struct mxs_spi *spi, unsigned cs)
+{
+       struct mxs_ssp *ssp = &spi->ssp;
+
+       writel(0x3 << BM_SSP_CTRL0_SPI_CS_BITS,
+              ssp->base + HW_SSP_CTRL0 + STMP_OFFSET_REG_CLR);
+       writel(cs,
+              ssp->base + HW_SSP_CTRL0 + STMP_OFFSET_REG_SET);
+}

I agree with Thomas.

In U-boot I did the following in order to be able to select the different chip selects:
http://git.denx.de/?p=u-boot.git;a=commitdiff;h=148ca64f327a89ef77e84756f5d351af33e59b64

Thanks,

Fabio Estevam

#14 lore raw

PATCH v2 6/10 spi: Add SPI driver for mx233/mx28

Dear Fabio Estevam,

Quoted 26 lines

On Fri, Aug 3, 2012 at 10:38 AM, Thomas Petazzoni

Quoted 11 lines · <[email protected]> wrote:

It sounds really strange to manipulate WAIT_FOR_CMD and WAIT_FOR_IRQ bits to adjust the chip select, and when reading the driver, it seemed suspicious to me. After going through the datasheet, indeed those bits are the appropriate one to select between the SS0, SS1 and SS2 chip selects, but I find the code not really obvious. Would it be possible to make it more obvious either by adding or comment or doing something like:

/* Should be put in some header file */
#define BM_SSP_CTRL0_SPI_CS_BITS (20)

drivers/spi/spi-mxs.c line 171

Quoted 9 lines
drivers/spi/spi-mxs.c new file +9 -0
+static void mxs_spi_set_cs(struct mxs_spi *spi, unsigned cs)
+{
+       struct mxs_ssp *ssp = &spi->ssp;
+
+       writel(0x3 << BM_SSP_CTRL0_SPI_CS_BITS,
+              ssp->base + HW_SSP_CTRL0 + STMP_OFFSET_REG_CLR);
+       writel(cs,
+              ssp->base + HW_SSP_CTRL0 + STMP_OFFSET_REG_SET);
+}

I agree with Thomas.

Quoted 4 lines

In U-boot I did the following in order to be able to select the different chip selects:
http://git.denx.de/?p=u-boot.git;a=commitdiff;h=148ca64f327a89ef77e84756f5d
351af33e59b64

Good thing I waited with submission :)

I'll fix it in a bit and resubmit in the evening.

Quoted 3 lines

Thanks,

Fabio Estevam

Best regards,
Marek Vasut

#15 lore raw

PATCH v2 6/10 spi: Add SPI driver for mx233/mx28

In reply to Fabio Estevam

Dear Fabio Estevam,

Quoted 26 lines

On Fri, Aug 3, 2012 at 10:38 AM, Thomas Petazzoni

Quoted 11 lines · <[email protected]> wrote:

It sounds really strange to manipulate WAIT_FOR_CMD and WAIT_FOR_IRQ bits to adjust the chip select, and when reading the driver, it seemed suspicious to me. After going through the datasheet, indeed those bits are the appropriate one to select between the SS0, SS1 and SS2 chip selects, but I find the code not really obvious. Would it be possible to make it more obvious either by adding or comment or doing something like:

/* Should be put in some header file */
#define BM_SSP_CTRL0_SPI_CS_BITS (20)

drivers/spi/spi-mxs.c line 171

Quoted 9 lines
drivers/spi/spi-mxs.c new file +9 -0
+static void mxs_spi_set_cs(struct mxs_spi *spi, unsigned cs)
+{
+       struct mxs_ssp *ssp = &spi->ssp;
+
+       writel(0x3 << BM_SSP_CTRL0_SPI_CS_BITS,
+              ssp->base + HW_SSP_CTRL0 + STMP_OFFSET_REG_CLR);
+       writel(cs,
+              ssp->base + HW_SSP_CTRL0 + STMP_OFFSET_REG_SET);
+}

I agree with Thomas.

Quoted 4 lines

In U-boot I did the following in order to be able to select the different chip selects:
http://git.denx.de/?p=u-boot.git;a=commitdiff;h=148ca64f327a89ef77e84756f5d
351af33e59b64

I'll just add the following comment if it's ok with you:

/*
* i.MX28 Datasheet: 17.10.1: HW_SSP_CTRL0
*
* The bits BM_SSP_CTRL0_WAIT_FOR_CMD and BM_SSP_CTRL0_WAIT_FOR_IRQ
* in HW_SSP_CTRL0 register do have multiple usage, please refer to
* the datasheet for further details. In SPI mode, they are used to
* toggle the chip-select lines (nCS pins).
*/

I hope it'll suffice. Recycling bits in registers is really crazy practice and I'd like to avoid these getting out of scope of this flaw's location.

Quoted 3 lines

Thanks,

Fabio Estevam

Best regards,
Marek Vasut

#16 lore raw

PATCH RESEND v2 7/10 mmc: spi: Pull out common DMA parts from MXS MMC

In reply to Marek Vasut

These parts will be used by the MXS SPI driver too.

Cc Attila Kinali <[email protected]>
Cc Chris Ball <[email protected]>
CC Dong Aisheng <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc Grant Likely <[email protected]>
Cc Linux ARM kernel <[email protected]>
Cc Mark Brown <[email protected]>
CC Shawn Guo <[email protected]>
---
 drivers/mmc/host/mxs-mmc.c  |   88 +++++++++++++++++++++----------------------
 drivers/spi/spi-mxs.c       |    1 -
 include/linux/spi/mxs-spi.h |    9 +++++
 3 files changed, 51 insertions(+), 47 deletions(-)
drivers/mmc/host/mxs-mmc.c +42 -46
index 0813340..4da9966 100644
@@ -41,7 +41,6 @@
 #include <linux/gpio.h>
 #include <linux/regulator/consumer.h>
 #include <linux/module.h>
-#include <linux/fsl/mxs-dma.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/stmp_device.h>
 #include <linux/mmc/mxs-mmc.h>
@@ -69,13 +68,6 @@ struct mxs_mmc_host {
 	struct mmc_command		*cmd;
 	struct mmc_data			*data;
 
-	int				dma_channel;
-	struct dma_chan         	*dmach;
-	struct mxs_dma_data		dma_data;
-	unsigned int			dma_dir;
-	enum dma_transfer_direction	slave_dirn;
-	u32				ssp_pio_words[SSP_PIO_NUM];
-
 	unsigned char			bus_width;
 	spinlock_t			lock;
 	int				sdio_irq_en;
@@ -163,7 +155,7 @@ static void mxs_mmc_request_done(struct mxs_mmc_host *host)
 
 	if (data) {
 		dma_unmap_sg(mmc_dev(host->mmc), data->sg,
-			     data->sg_len, host->dma_dir);
+			     data->sg_len, ssp->dma_dir);
 		/*
 		 * If there was an error on any block, we mark all
 		 * data blocks as being in error.
@@ -232,6 +224,7 @@ static irqreturn_t mxs_mmc_irq_handler(int irq, void *dev_id)
 static struct dma_async_tx_descriptor *mxs_mmc_prep_dma(
 	struct mxs_mmc_host *host, unsigned long flags)
 {
+	struct mxs_ssp *ssp = &host->ssp;
 	struct dma_async_tx_descriptor *desc;
 	struct mmc_data *data = host->data;
 	struct scatterlist * sgl;
@@ -240,24 +233,24 @@ static struct dma_async_tx_descriptor *mxs_mmc_prep_dma(
 	if (data) {
 		/* data */
 		dma_map_sg(mmc_dev(host->mmc), data->sg,
-			   data->sg_len, host->dma_dir);
+			   data->sg_len, ssp->dma_dir);
 		sgl = data->sg;
 		sg_len = data->sg_len;
 	} else {
 		/* pio */
-		sgl = (struct scatterlist *) host->ssp_pio_words;
+		sgl = (struct scatterlist *) ssp->ssp_pio_words;
 		sg_len = SSP_PIO_NUM;
 	}
 
-	desc = dmaengine_prep_slave_sg(host->dmach,
-				sgl, sg_len, host->slave_dirn, flags);
+	desc = dmaengine_prep_slave_sg(ssp->dmach,
+				sgl, sg_len, ssp->slave_dirn, flags);
 	if (desc) {
 		desc->callback = mxs_mmc_dma_irq_callback;
 		desc->callback_param = host;
 	} else {
 		if (data)
 			dma_unmap_sg(mmc_dev(host->mmc), data->sg,
-				     data->sg_len, host->dma_dir);
+				     data->sg_len, ssp->dma_dir);
 	}
 
 	return desc;
@@ -265,6 +258,7 @@ static struct dma_async_tx_descriptor *mxs_mmc_prep_dma(
 
 static void mxs_mmc_bc(struct mxs_mmc_host *host)
 {
+	struct mxs_ssp *ssp = &host->ssp;
 	struct mmc_command *cmd = host->cmd;
 	struct dma_async_tx_descriptor *desc;
 	u32 ctrl0, cmd0, cmd1;
@@ -278,17 +272,17 @@ static void mxs_mmc_bc(struct mxs_mmc_host *host)
 		cmd0 |= BM_SSP_CMD0_CONT_CLKING_EN | BM_SSP_CMD0_SLOW_CLKING_EN;
 	}
 
-	host->ssp_pio_words[0] = ctrl0;
-	host->ssp_pio_words[1] = cmd0;
-	host->ssp_pio_words[2] = cmd1;
-	host->dma_dir = DMA_NONE;
-	host->slave_dirn = DMA_TRANS_NONE;
+	ssp->ssp_pio_words[0] = ctrl0;
+	ssp->ssp_pio_words[1] = cmd0;
+	ssp->ssp_pio_words[2] = cmd1;
+	ssp->dma_dir = DMA_NONE;
+	ssp->slave_dirn = DMA_TRANS_NONE;
 	desc = mxs_mmc_prep_dma(host, DMA_CTRL_ACK);
 	if (!desc)
 		goto out;
 
 	dmaengine_submit(desc);
-	dma_async_issue_pending(host->dmach);
+	dma_async_issue_pending(ssp->dmach);
 	return;
 
 out:
@@ -298,6 +292,7 @@ out:
 
 static void mxs_mmc_ac(struct mxs_mmc_host *host)
 {
+	struct mxs_ssp *ssp = &host->ssp;
 	struct mmc_command *cmd = host->cmd;
 	struct dma_async_tx_descriptor *desc;
 	u32 ignore_crc, get_resp, long_resp;
@@ -319,17 +314,17 @@ static void mxs_mmc_ac(struct mxs_mmc_host *host)
 		cmd0 |= BM_SSP_CMD0_CONT_CLKING_EN | BM_SSP_CMD0_SLOW_CLKING_EN;
 	}
 
-	host->ssp_pio_words[0] = ctrl0;
-	host->ssp_pio_words[1] = cmd0;
-	host->ssp_pio_words[2] = cmd1;
-	host->dma_dir = DMA_NONE;
-	host->slave_dirn = DMA_TRANS_NONE;
+	ssp->ssp_pio_words[0] = ctrl0;
+	ssp->ssp_pio_words[1] = cmd0;
+	ssp->ssp_pio_words[2] = cmd1;
+	ssp->dma_dir = DMA_NONE;
+	ssp->slave_dirn = DMA_TRANS_NONE;
 	desc = mxs_mmc_prep_dma(host, DMA_CTRL_ACK);
 	if (!desc)
 		goto out;
 
 	dmaengine_submit(desc);
-	dma_async_issue_pending(host->dmach);
+	dma_async_issue_pending(ssp->dmach);
 	return;
 
 out:
@@ -441,11 +436,11 @@ static void mxs_mmc_adtc(struct mxs_mmc_host *host)
 	writel(val, ssp->base + HW_SSP_TIMING(ssp));
 
 	/* pio */
-	host->ssp_pio_words[0] = ctrl0;
-	host->ssp_pio_words[1] = cmd0;
-	host->ssp_pio_words[2] = cmd1;
-	host->dma_dir = DMA_NONE;
-	host->slave_dirn = DMA_TRANS_NONE;
+	ssp->ssp_pio_words[0] = ctrl0;
+	ssp->ssp_pio_words[1] = cmd0;
+	ssp->ssp_pio_words[2] = cmd1;
+	ssp->dma_dir = DMA_NONE;
+	ssp->slave_dirn = DMA_TRANS_NONE;
 	desc = mxs_mmc_prep_dma(host, 0);
 	if (!desc)
 		goto out;
@@ -453,14 +448,14 @@ static void mxs_mmc_adtc(struct mxs_mmc_host *host)
 	/* append data sg */
 	WARN_ON(host->data != NULL);
 	host->data = data;
-	host->dma_dir = dma_data_dir;
-	host->slave_dirn = slave_dirn;
+	ssp->dma_dir = dma_data_dir;
+	ssp->slave_dirn = slave_dirn;
 	desc = mxs_mmc_prep_dma(host, DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
 	if (!desc)
 		goto out;
 
 	dmaengine_submit(desc);
-	dma_async_issue_pending(host->dmach);
+	dma_async_issue_pending(ssp->dmach);
 	return;
 out:
 	dev_warn(mmc_dev(host->mmc),
@@ -557,14 +552,15 @@ static const struct mmc_host_ops mxs_mmc_ops = {
 static bool mxs_mmc_dma_filter(struct dma_chan *chan, void *param)
 {
 	struct mxs_mmc_host *host = param;
+	struct mxs_ssp *ssp = &host->ssp;
 
 	if (!mxs_dma_is_apbh(chan))
 		return false;
 
-	if (chan->chan_id != host->dma_channel)
+	if (chan->chan_id != ssp->dma_channel)
 		return false;
 
-	chan->private = &host->dma_data;
+	chan->private = &ssp->dma_data;
 
 	return true;
 }
@@ -632,7 +628,7 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 		 * to use generic DMA binding later when the helpers get in.
 		 */
 		ret = of_property_read_u32(np, "fsl,ssp-dma-channel",
-					   &host->dma_channel);
+					   &ssp->dma_channel);
 		if (ret) {
 			dev_err(mmc_dev(host->mmc),
 				"failed to get dma channel\n");
@@ -640,7 +636,7 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 		}
 	} else {
 		ssp->devid = pdev->id_entry->driver_data;
-		host->dma_channel = dmares->start;
+		ssp->dma_channel = dmares->start;
 	}
 
 	host->mmc = mmc;
@@ -673,9 +669,9 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 
 	dma_cap_zero(mask);
 	dma_cap_set(DMA_SLAVE, mask);
-	host->dma_data.chan_irq = irq_dma;
-	host->dmach = dma_request_channel(mask, mxs_mmc_dma_filter, host);
-	if (!host->dmach) {
+	ssp->dma_data.chan_irq = irq_dma;
+	ssp->dmach = dma_request_channel(mask, mxs_mmc_dma_filter, host);
+	if (!ssp->dmach) {
 		dev_err(mmc_dev(host->mmc),
 			"%s: failed to request dma\n", __func__);
 		goto out_clk_put;
@@ -714,7 +710,7 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 	mmc->max_blk_size = 1 << 0xf;
 	mmc->max_blk_count = (ssp_is_old(ssp)) ? 0xff : 0xffffff;
 	mmc->max_req_size = (ssp_is_old(ssp)) ? 0xffff : 0xffffffff;
-	mmc->max_seg_size = dma_get_max_seg_size(host->dmach->device->dev);
+	mmc->max_seg_size = dma_get_max_seg_size(ssp->dmach->device->dev);
 
 	platform_set_drvdata(pdev, mmc);
 
@@ -734,8 +730,8 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 	return 0;
 
 out_free_dma:
-	if (host->dmach)
-		dma_release_channel(host->dmach);
+	if (ssp->dmach)
+		dma_release_channel(ssp->dmach);
 out_clk_put:
 	clk_disable_unprepare(ssp->clk);
 	clk_put(ssp->clk);
@@ -754,8 +750,8 @@ static int mxs_mmc_remove(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, NULL);
 
-	if (host->dmach)
-		dma_release_channel(host->dmach);
+	if (ssp->dmach)
+		dma_release_channel(ssp->dmach);
 
 	clk_disable_unprepare(ssp->clk);
 	clk_put(ssp->clk);
drivers/spi/spi-mxs.c +0 -1
index dce986c..bd2f2fd 100644
@@ -46,7 +46,6 @@
 #include <linux/gpio.h>
 #include <linux/regulator/consumer.h>
 #include <linux/module.h>
-#include <linux/fsl/mxs-dma.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/stmp_device.h>
 #include <linux/spi/spi.h>
include/linux/spi/mxs-spi.h +9 -0
index d07f8dc..61ae130 100644
@@ -24,6 +24,8 @@
 #ifndef __LINUX_SPI_MXS_SPI_H__
 #define __LINUX_SPI_MXS_SPI_H__
 
+#include <linux/fsl/mxs-dma.h>
+
 #define ssp_is_old(host)	((host)->devid == IMX23_SSP)
 
 /* SSP registers */
@@ -134,6 +136,13 @@ struct mxs_ssp {
 	struct clk			*clk;
 	unsigned int			clk_rate;
 	enum mxs_ssp_id			devid;
+
+	int				dma_channel;
+	struct dma_chan			*dmach;
+	struct mxs_dma_data		dma_data;
+	unsigned int			dma_dir;
+	enum dma_transfer_direction	slave_dirn;
+	u32				ssp_pio_words[SSP_PIO_NUM];
 };
 
 void mxs_ssp_set_clk_rate(struct mxs_ssp *ssp, unsigned int rate);
Signature · 1 line
1.7.10.4
#17 lore raw

PATCH RESEND v2 8/10 spi: Add DMA support into SPI driver

In reply to Marek Vasut
Cc Attila Kinali <[email protected]>
Cc Chris Ball <[email protected]>
CC Dong Aisheng <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc Grant Likely <[email protected]>
Cc Linux ARM kernel <[email protected]>
Cc Mark Brown <[email protected]>
CC Shawn Guo <[email protected]>
---
 drivers/spi/spi-mxs.c |  229 +++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 214 insertions(+), 15 deletions(-)
drivers/spi/spi-mxs.c +214 -15
index bd2f2fd..d6a80a1 100644
@@ -55,8 +55,12 @@
 
 #define SSP_TIMEOUT		1000	/* 1000 ms */
 
+#define SG_NUM			4
+#define SG_MAXLEN		0xff00
+
 struct mxs_spi {
 	struct mxs_ssp		ssp;
+	struct completion	c;
 };
 
 static int mxs_spi_setup_transfer(struct spi_device *dev,
@@ -192,6 +196,115 @@ static int mxs_ssp_wait(struct mxs_spi *spi, int offset, int mask, bool set)
 	return 0;
 }
 
+static void mxs_ssp_dma_irq_callback(void *param)
+{
+	struct mxs_spi *spi = param;
+	complete(&spi->c);
+}
+
+static irqreturn_t mxs_ssp_irq_handler(int irq, void *dev_id)
+{
+	struct mxs_ssp *ssp = dev_id;
+	dev_err(ssp->dev, "%s[%i] CTRL1=%08x STATUS=%08x\n",
+		__func__, __LINE__,
+		readl(ssp->base + HW_SSP_CTRL1(ssp)),
+		readl(ssp->base + HW_SSP_STATUS(ssp)));
+	return IRQ_HANDLED;
+}
+
+static int mxs_spi_txrx_dma(struct mxs_spi *spi, int cs,
+			    unsigned char *buf, int len,
+			    int *first, int *last, int write)
+{
+	struct mxs_ssp *ssp = &spi->ssp;
+	struct dma_async_tx_descriptor *desc;
+	struct scatterlist sg[SG_NUM];
+	int sg_count;
+	uint32_t pio = BM_SSP_CTRL0_DATA_XFER | mxs_spi_cs_to_reg(cs);
+	int ret;
+
+	if (len > SG_NUM * SG_MAXLEN) {
+		dev_err(ssp->dev, "Data chunk too big for DMA\n");
+		return -EINVAL;
+	}
+
+	init_completion(&spi->c);
+
+	if (*first)
+		pio |= BM_SSP_CTRL0_LOCK_CS;
+	if (*last)
+		pio |= BM_SSP_CTRL0_IGNORE_CRC;
+	if (!write)
+		pio |= BM_SSP_CTRL0_READ;
+
+	if (ssp->devid == IMX23_SSP)
+		pio |= len;
+	else
+		writel(len, ssp->base + HW_SSP_XFER_SIZE);
+
+	/* Queue the PIO register write transfer. */
+	desc = dmaengine_prep_slave_sg(ssp->dmach,
+			(struct scatterlist *)&pio,
+			1, DMA_TRANS_NONE, 0);
+	if (!desc) {
+		dev_err(ssp->dev,
+			"Failed to get PIO reg. write descriptor.\n");
+		return -EINVAL;
+	}
+
+	/* Queue the DMA data transfer. */
+	sg_init_table(sg, (len / SG_MAXLEN) + 1);
+	sg_count = 0;
+	while (len) {
+		sg_set_buf(&sg[sg_count++], buf, min(len, SG_MAXLEN));
+		len -= min(len, SG_MAXLEN);
+		buf += min(len, SG_MAXLEN);
+	}
+	dma_map_sg(ssp->dev, sg, sg_count,
+		write ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
+
+	desc = dmaengine_prep_slave_sg(ssp->dmach, sg, sg_count,
+			write ? DMA_MEM_TO_DEV : DMA_DEV_TO_MEM,
+			DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+
+	if (!desc) {
+		dev_err(ssp->dev,
+			"Failed to get DMA data write descriptor.\n");
+		ret = -EINVAL;
+		goto err;
+	}
+
+	/*
+	 * The last descriptor must have this callback,
+	 * to finish the DMA transaction.
+	 */
+	desc->callback = mxs_ssp_dma_irq_callback;
+	desc->callback_param = spi;
+
+	/* Start the transfer. */
+	dmaengine_submit(desc);
+	dma_async_issue_pending(ssp->dmach);
+
+	ret = wait_for_completion_timeout(&spi->c,
+				msecs_to_jiffies(SSP_TIMEOUT));
+
+	if (!ret) {
+		dev_err(ssp->dev, "DMA transfer timeout\n");
+		ret = -ETIMEDOUT;
+		goto err;
+	}
+
+	ret = 0;
+
+err:
+	for (--sg_count; sg_count >= 0; sg_count--) {
+		dma_unmap_sg(ssp->dev, &sg[sg_count], 1,
+			write ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
+	}
+
+	return ret;
+}
+
 static int mxs_spi_txrx_pio(struct mxs_spi *spi, int cs,
 			    unsigned char *buf, int len,
 			    int *first, int *last, int write)
@@ -277,18 +390,48 @@ static int mxs_spi_transfer_one(struct spi_master *master,
 			first = 1;
 		if (&t->transfer_list == m->transfers.prev)
 			last = 1;
-		if (t->rx_buf && t->tx_buf) {
+		if ((t->rx_buf && t->tx_buf) || (t->rx_dma && t->tx_dma)) {
 			dev_err(ssp->dev,
 				"Cannot send and receive simultaneously\n");
 			return -EINVAL;
 		}
 
-		if (t->tx_buf)
-			status = mxs_spi_txrx_pio(spi, cs, (void *)t->tx_buf,
-					     t->len, &first, &last, 1);
-		if (t->rx_buf)
-			status = mxs_spi_txrx_pio(spi, cs, t->rx_buf,
-					     t->len, &first, &last, 0);
+		/*
+		 * Small blocks can be transfered via PIO.
+		 * Measured by empiric means:
+		 *
+		 * dd if=/dev/mtdblock0 of=/dev/null bs=1024k count=1
+		 *
+		 * DMA only: 2.164808 seconds, 473.0KB/s
+		 * Combined: 1.676276 seconds, 610.9KB/s
+		 */
+		if (t->len <= 256) {
+			writel(BM_SSP_CTRL1_DMA_ENABLE,
+				ssp->base + HW_SSP_CTRL1(ssp) +
+				STMP_OFFSET_REG_CLR);
+
+			if (t->tx_buf)
+				status = mxs_spi_txrx_pio(spi, cs,
+						(void *)t->tx_buf,
+						t->len, &first, &last, 1);
+			if (t->rx_buf)
+				status = mxs_spi_txrx_pio(spi, cs,
+						t->rx_buf, t->len,
+						&first, &last, 0);
+		} else {
+			writel(BM_SSP_CTRL1_DMA_ENABLE,
+				ssp->base + HW_SSP_CTRL1(ssp) +
+				STMP_OFFSET_REG_SET);
+
+			if (t->tx_buf)
+				status = mxs_spi_txrx_dma(spi, cs,
+						(void *)t->tx_buf, t->len,
+						&first, &last, 1);
+			if (t->rx_buf)
+				status = mxs_spi_txrx_dma(spi, cs,
+						t->rx_buf, t->len,
+						&first, &last, 0);
+		}
 
 		m->actual_length += t->len;
 		if (status)
@@ -303,6 +446,21 @@ static int mxs_spi_transfer_one(struct spi_master *master,
 	return status;
 }
 
+static bool mxs_ssp_dma_filter(struct dma_chan *chan, void *param)
+{
+	struct mxs_ssp *ssp = param;
+
+	if (!mxs_dma_is_apbh(chan))
+		return false;
+
+	if (chan->chan_id != ssp->dma_channel)
+		return false;
+
+	chan->private = &ssp->dma_data;
+
+	return true;
+}
+
 static const struct of_device_id mxs_spi_dt_ids[] = {
 	{ .compatible = "fsl,imx23-spi", .data = (void *) IMX23_SSP, },
 	{ .compatible = "fsl,imx28-spi", .data = (void *) IMX28_SSP, },
@@ -318,15 +476,18 @@ static int __devinit mxs_spi_probe(struct platform_device *pdev)
 	struct spi_master *master;
 	struct mxs_spi *spi;
 	struct mxs_ssp *ssp;
-	struct resource *iores;
+	struct resource *iores, *dmares;
 	struct pinctrl *pinctrl;
 	struct clk *clk;
 	void __iomem *base;
-	int devid;
-	int ret = 0;
+	int devid, dma_channel;
+	int ret = 0, irq_err, irq_dma;
+	dma_cap_mask_t mask;
 
 	iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!iores)
+	irq_err = platform_get_irq(pdev, 0);
+	irq_dma = platform_get_irq(pdev, 1);
+	if (!iores || irq_err < 0 || irq_dma < 0)
 		return -EINVAL;
 
 	base = devm_request_and_ioremap(&pdev->dev, iores);
@@ -341,10 +502,26 @@ static int __devinit mxs_spi_probe(struct platform_device *pdev)
 	if (IS_ERR(clk))
 		return PTR_ERR(clk);
 
-	if (np)
+	if (np) {
 		devid = (enum mxs_ssp_id) of_id->data;
-	else
+		/*
+		 * TODO: This is a temporary solution and should be changed
+		 * to use generic DMA binding later when the helpers get in.
+		 */
+		ret = of_property_read_u32(np, "fsl,ssp-dma-channel",
+					   &dma_channel);
+		if (ret) {
+			dev_err(&pdev->dev,
+				"Failed to get DMA channel\n");
+			return -EINVAL;
+		}
+	} else {
+		dmares = platform_get_resource(pdev, IORESOURCE_DMA, 0);
+		if (!dmares)
+			return -EINVAL;
 		devid = pdev->id_entry->driver_data;
+		dma_channel = dmares->start;
+	}
 
 	master = spi_alloc_master(&pdev->dev, sizeof(*spi));
 	if (!master)
@@ -364,8 +541,28 @@ static int __devinit mxs_spi_probe(struct platform_device *pdev)
 	ssp->clk = clk;
 	ssp->base = base;
 	ssp->devid = devid;
+	ssp->dma_channel = dma_channel;
+
+	ret = devm_request_irq(&pdev->dev, irq_err, mxs_ssp_irq_handler, 0,
+			       DRIVER_NAME, ssp);
+	if (ret)
+		goto out_master_free;
+
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+	ssp->dma_data.chan_irq = irq_dma;
+	ssp->dmach = dma_request_channel(mask, mxs_ssp_dma_filter, ssp);
+	if (!ssp->dmach) {
+		dev_err(ssp->dev, "Failed to request DMA\n");
+		goto out_master_free;
+	}
 
+	/*
+	 * Crank up the clock to 120MHz, this will be further divided onto a
+	 * proper speed.
+	 */
 	clk_prepare_enable(ssp->clk);
+	clk_set_rate(ssp->clk, 120 * 1000 * 1000);
 	ssp->clk_rate = clk_get_rate(ssp->clk) / 1000;
 
 	stmp_reset_block(ssp->base);
@@ -375,13 +572,15 @@ static int __devinit mxs_spi_probe(struct platform_device *pdev)
 	ret = spi_register_master(master);
 	if (ret) {
 		dev_err(&pdev->dev, "Cannot register SPI master, %d\n", ret);
-		goto out_master_free;
+		goto out_free_dma;
 	}
 
 	return 0;
 
-out_master_free:
+out_free_dma:
+	dma_release_channel(ssp->dmach);
 	clk_disable_unprepare(ssp->clk);
+out_master_free:
 	spi_master_put(master);
 	kfree(master);
 	return ret;
Signature · 1 line
1.7.10.4
#18 lore raw

PATCH RESEND 8/10 spi: Add DMA support into SPI driver

drivers/spi/spi-mxs.c static int mxs_spi_transfer_one(struct spi_master *master, · line 407

Quoted 9 lines · Marek Vasut
drivers/spi/spi-mxs.c +9 -0
+		/*
+		 * Small blocks can be transfered via PIO.
+		 * Measured by empiric means:
+		 *
+		 * dd if=/dev/mtdblock0 of=/dev/null bs=1024k count=1
+		 *
+		 * DMA only: 2.164808 seconds, 473.0KB/s
+		 * Combined: 1.676276 seconds, 610.9KB/s
+		 */

I've seen other devices end up using the FIFO length as the decision point here which makes intuitive sense to me unless the FIFO is extremely deep.

Not reviewed this properly yet but it looks OK from a first read through.

#19 lore raw

PATCH RESEND v2 8/10 spi: Add DMA support into SPI driver

Dear Mark Brown,

Quoted 6 lines · Marek Vasut
Quoted 5 lines
drivers/spi/spi-mxs.c +5 -0
+		/*
+		 * Small blocks can be transfered via PIO.
+		 * Measured by empiric means:
+		 *
+		 * dd if=/dev/mtdblock0 of=/dev/null bs=1024k count=1
Quoted 8 lines

drivers/spi/spi-mxs.c static int mxs_spi_transfer_one(struct spi_master *master, · line 407

Quoted 4 lines
drivers/spi/spi-mxs.c +4 -0
+		 *
+		 * DMA only: 2.164808 seconds, 473.0KB/s
+		 * Combined: 1.676276 seconds, 610.9KB/s
+		 */

I've seen other devices end up using the FIFO length as the decision point here which makes intuitive sense to me unless the FIFO is extremely deep.

Yep, that's all good. This device has not too deep FIFO, but the reason I chose not to go that way is because the DMA allocations have some overhead too and this values seemed to be the best.

Quoted 2 lines

Not reviewed this properly yet but it looks OK from a first read through.

Thanks!

Best regards,
Marek Vasut

#20 lore raw

PATCH RESEND v2 9/10 spi: Add SSP/SPI device tree documentation

In reply to Marek Vasut
Cc Attila Kinali <[email protected]>
Cc Chris Ball <[email protected]>
CC Dong Aisheng <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc Grant Likely <[email protected]>
Cc Linux ARM kernel <[email protected]>
Cc Mark Brown <[email protected]>
CC Shawn Guo <[email protected]>
---
 Documentation/devicetree/bindings/spi/mxs-spi.txt |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/mxs-spi.txt
Documentation/devicetree/bindings/spi/mxs-spi.txt new file +18 -0
new file mode 100644
index 0000000..f1263a9
@@ -0,0 +1,18 @@
+* Freescale MX233/MX28 SSP/SPI
+
+Required properties:
+- compatible: Should be "fsl,<soc>-spi", where soc is "imx23" or "imx28"
+- reg: Offset and length of the register set for the device
+- interrupts: Should contain SSP interrupts (error irq first, dma irq second)
+- fsl,ssp-dma-channel: APBX DMA channel for the SSP
+
+Example:
+
+ssp0: ssp at 80010000 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	compatible = "fsl,imx28-spi";
+	reg = <0x80010000 2000>;
+	interrupts = <96 82>;
+	fsl,ssp-dma-channel = <0>;
+};
Signature · 1 line
1.7.10.4
#21 lore raw

PATCH RESEND 9/10 spi: Add SSP/SPI device tree documentation

Hello.

Quoted 25 lines · Marek Vasut
Cc Attila Kinali <[email protected]>
Cc Chris Ball <[email protected]>
CC Dong Aisheng <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc Grant Likely <[email protected]>
Cc Linux ARM kernel <[email protected]>
Cc Mark Brown <[email protected]>
CC Shawn Guo <[email protected]>
---
 Documentation/devicetree/bindings/spi/mxs-spi.txt |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/mxs-spi.txt
Documentation/devicetree/bindings/spi/mxs-spi.txt new file +5 -0
new file mode 100644
index 0000000..f1263a9
@@ -0,0 +1,18 @@
+* Freescale MX233/MX28 SSP/SPI
+
+Required properties:
+- compatible: Should be "fsl,<soc>-spi", where soc is "imx23" or "imx28"
+- reg: Offset and length of the register set for the device

Documentation/devicetree/bindings/spi/mxs-spi.txt line 15

Quoted 10 lines
Documentation/devicetree/bindings/spi/mxs-spi.txt new file +10 -0
+- interrupts: Should contain SSP interrupts (error irq first, dma irq second)
+- fsl,ssp-dma-channel: APBX DMA channel for the SSP
+
+Example:
+
+ssp0: ssp at 80010000 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	compatible = "fsl,imx28-spi";
+	reg = <0x80010000 2000>;

Not 0x2000?

WBR, Sergei

#22 lore raw

PATCH RESEND v2 9/10 spi: Add SSP/SPI device tree documentation

Dear Sergei Shtylyov,

Quoted 33 lines

Hello.

Quoted 30 lines · Marek Vasut
Cc Attila Kinali <[email protected]>
Cc Chris Ball <[email protected]>
CC Dong Aisheng <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc Grant Likely <[email protected]>
Cc Linux ARM kernel <[email protected]>
Cc Mark Brown <[email protected]>
CC Shawn Guo <[email protected]>

---

 Documentation/devicetree/bindings/spi/mxs-spi.txt |   18
 ++++++++++++++++++ 1 file changed, 18 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/mxs-spi.txt
(quoted diff) +0 -0

b/Documentation/devicetree/bindings/spi/mxs-spi.txt new file mode 100644 index 0000000..f1263a9

Documentation/devicetree/bindings/spi/mxs-spi.txt new file +6 -0
@@ -0,0 +1,18 @@
+* Freescale MX233/MX28 SSP/SPI
+
+Required properties:
+- compatible: Should be "fsl,<soc>-spi", where soc is "imx23" or "imx28"
+- reg: Offset and length of the register set for the device
+- interrupts: Should contain SSP interrupts (error irq first, dma irq

second) +- fsl,ssp-dma-channel: APBX DMA channel for the SSP

Documentation/devicetree/bindings/spi/mxs-spi.txt new file +2 -0
+
+Example:
Quoted 8 lines

Documentation/devicetree/bindings/spi/mxs-spi.txt line 15

Quoted 6 lines
Documentation/devicetree/bindings/spi/mxs-spi.txt new file +6 -0
+
+ssp0: ssp at 80010000 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	compatible = "fsl,imx28-spi";
+	reg = <0x80010000 2000>;
   Not 0x2000?

This is actually not the first place I was curious about in the imx28 DTS ... yes, the size is 0x2000, but "2000" as is is used in multiple places across imx28. Shawn?

Quoted 1 line

WBR, Sergei

Best regards,
Marek Vasut

#23 lore raw

PATCH RESEND 9/10 spi: Add SSP/SPI device tree documentation

Quoted 4 lines · Marek Vasut

This is actually not the first place I was curious about in the imx28 DTS ... yes, the size is 0x2000, but "2000" as is is used in multiple places across imx28. Shawn?

It seems that I overlooked the problem when it got introduced at the first place. I will send a patch to fix it. Thanks for spotting it.

Signature · 2 lines
Regards,
Shawn
#24 lore raw

PATCH RESEND v2 9/10 spi: Add SSP/SPI device tree documentation

Dear Shawn Guo,

Quoted 7 lines · Marek Vasut
Quoted 3 lines

This is actually not the first place I was curious about in the imx28 DTS ... yes, the size is 0x2000, but "2000" as is is used in multiple places across imx28. Shawn?

It seems that I overlooked the problem when it got introduced at the first place. I will send a patch to fix it. Thanks for spotting it.

Ok good.

Mark, do you want me to send a V2 of only this particular patch or whole set (I think that's overkill)?

Best regards,
Marek Vasut

#25 lore raw

PATCH RESEND v2 10/10 ARM: mx28: Add SPI pinmux into imx28.dtsi

In reply to Marek Vasut
Cc Attila Kinali <[email protected]>
Cc Chris Ball <[email protected]>
CC Dong Aisheng <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc Grant Likely <[email protected]>
Cc Linux ARM kernel <[email protected]>
Cc Mark Brown <[email protected]>
CC Shawn Guo <[email protected]>
---
 arch/arm/boot/dts/imx28.dtsi |   39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)
arch/arm/boot/dts/imx28.dtsi +39 -0
index badc76a..4a0f12c 100644
@@ -507,6 +507,45 @@
 					fsl,voltage = <1>;
 					fsl,pull-up = <0>;
 				};
+
+				spi0_pins_a: spi0 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids =
+						<0x2000 0x2010 0x2020 0x2030
+						0x2040 0x2050 0x2060 0x2070
+						0x2080 0x2090 0x20a0>;
+					fsl,drive-strength = <1>;
+					fsl,voltage = <1>;
+					fsl,pull-up = <1>;
+				};
+
+				spi1_pins_a: spi1 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids =
+						<0x20c0 0x20d0 0x20e0 0x20f0>;
+					fsl,drive-strength = <1>;
+					fsl,voltage = <1>;
+					fsl,pull-up = <1>;
+				};
+
+				spi2_pins_a: spi2 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids =
+						<0x2100 0x2110 0x2120
+						0x2130 0x2140 0x2150>;
+					fsl,drive-strength = <1>;
+					fsl,voltage = <1>;
+					fsl,pull-up = <1>;
+				};
+
+				spi3_pins_a: spi3 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids =
+						<0x2180 0x2190 0x21a0 0x21b0>;
+					fsl,drive-strength = <1>;
+					fsl,voltage = <1>;
+					fsl,pull-up = <1>;
+				};
 			};
 
 			digctl at 8001c000 {
Signature · 1 line
1.7.10.4
#26 lore raw

v2 cover MXS SPI driver

In reply to Marek Vasut
Quoted 3 lines · On Mon, 23 Jul 2012 22:40:42 +0200 Marek Vasut <[email protected]> wrote:

This patchset adds SPI master support for Freescale i.MX233/i.MX28. This is rebased on top of next-20120713. Please consider applying, bugs shall now be squashed during the previous two review rounds.

The patchset is good and works as expectd

Attila Kinali

Signature · 4 lines
The trouble with you, Shev, is you don't say anything until you've saved
up a whole truckload of damned heavy brick arguments and then you dump
them all out and never look at the bleeding body mangled beneath the heap
		-- Tirin, The Dispossessed, U. Le Guin
#27 lore raw

PATCH 1/1 SPI: MXS: Allow to pass the SPI master bus number from the device tree

In reply to Marek Vasut

If you don't do it, the bus number will be a bogus value, possibly used for device file names in drivers like spidev.

---
 drivers/spi/spi-mxs.c |    9 +++++++++
 1 file changed, 9 insertions(+)
drivers/spi/spi-mxs.c +9 -0
index d6a80a1..d6a73ef 100644
@@ -527,6 +527,15 @@ static int __devinit mxs_spi_probe(struct platform_device *pdev)
 	if (!master)
 		return -ENOMEM;
 
+	if (pdev->id != -1)
+		master->bus_num = pdev->id;
+	if (np) {
+		unsigned long prop;
+
+		of_property_read_u32(np, "cell-index", &prop);
+		master->bus_num = prop;
+	}
+
 	master->transfer_one_message = mxs_spi_transfer_one;
 	master->setup = mxs_spi_setup;
 	master->cleanup = mxs_spi_cleanup;
Signature · 1 line
1.7.9.5
#29 lore raw

PATCH 1/1 SPI: MXS: Allow to pass the SPI master bus number from the device tree

Le 03/08/2012 16:43, Shawn Guo a ?crit :

Quoted 7 lines · Maxime Ripard
Quoted 3 lines

If you don't do it, the bus number will be a bogus value, possibly used for device file names in drivers like spidev.

Do you see any problem with that? Here is the take from Grant.

http://thread.gmane.org/gmane.linux.kernel.spi.devel/9839/focus=9863

Ok, I didn't see this thread, obviously.

Thanks,
Maxime

Signature · 4 lines
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com