Skip to contentSkip to conversation

dma: mxs-dma: make driver mach-independent and add dt support

PATCH Superseded Dong Aisheng 4 participants 21 messages Apr 18, 2012 – Apr 23, 2012

Subsystems (7)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover dma: mxs-dma: make driver mach-independent and add dt support

This patch series make the mxs-dma driver mach-independent which can be used by imx6q and other compatible platforms in the future and also add the dt support.

This first patch depends on Wolfram's
[PATCH V2 1/3] lib: add support for stmp-style devices
http://www.spinics.net/lists/arm-kernel/msg166232.html
which is still not in mainline.

Dong Aisheng (5):
dma: mxs-dma: use global stmp_device functionality
dma: mxs-dma: make platform_device_id more generic
dma: mxs-dma: add device tree probe support
ARM: mxs: do not add dma device by default
ARM: mx28evk: add mxs-dma dt support

 .../devicetree/bindings/dma/fsl-mxs-dma.txt        |   19 +++
 arch/arm/boot/dts/imx28.dtsi                       |    4 +-
 arch/arm/mach-mxs/clock-mx23.c                     |    4 +-
 arch/arm/mach-mxs/clock-mx28.c                     |    6 +-
 arch/arm/mach-mxs/devices-mx23.h                   |    2 +
 arch/arm/mach-mxs/devices-mx28.h                   |    2 +
 arch/arm/mach-mxs/devices/platform-dma.c           |   17 +-
 arch/arm/mach-mxs/include/mach/devices-common.h    |    3 +
 arch/arm/mach-mxs/mach-apx4devkit.c                |    1 +
 arch/arm/mach-mxs/mach-m28evk.c                    |    1 +
 arch/arm/mach-mxs/mach-mx23evk.c                   |    1 +
 arch/arm/mach-mxs/mach-stmp378x_devb.c             |    1 +
 arch/arm/mach-mxs/mach-tx28.c                      |    1 +
 drivers/dma/Kconfig                                |    1 +
 drivers/dma/mxs-dma.c                              |  163 +++++++++++++++-----
 include/linux/fsl/mxs-dma.h                        |   12 +--
 16 files changed, 174 insertions(+), 64 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/dma/fsl-mxs-dma.txt
#1 lore raw

PATCH 1/5 dma: mxs-dma: use global stmp_device functionality

From: Dong Aisheng <[email protected]>

This can get rid of the mach-dependency.

Cc Vinod Koul <[email protected]>
Cc Dan Williams <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Huang Shijie <[email protected]>

---
This patch depends on Wolfram's
[PATCH V2 1/3] lib: add support for stmp-style devices
http://www.spinics.net/lists/arm-kernel/msg166232.html
which is still not in mainline.

---
 drivers/dma/Kconfig   |    1 +
 drivers/dma/mxs-dma.c |   27 +++++++++++++--------------
 2 files changed, 14 insertions(+), 14 deletions(-)
drivers/dma/Kconfig +1 -0
index cf9da36..af2a29f 100644
@@ -239,6 +239,7 @@ config IMX_DMA
 config MXS_DMA
 	bool "MXS DMA support"
 	depends on SOC_IMX23 || SOC_IMX28
+	select STMP_DEVICE
 	select DMA_ENGINE
 	help
 	  Support the MXS DMA engine. This engine including APBH-DMA
drivers/dma/mxs-dma.c +13 -14
index c81ef7e..b0051a8 100644
@@ -23,10 +23,9 @@
 #include <linux/dmaengine.h>
 #include <linux/delay.h>
 #include <linux/fsl/mxs-dma.h>
+#include <linux/stmp_device.h>
 
 #include <asm/irq.h>
-#include <mach/mxs.h>
-#include <mach/common.h>
 
 #include "dmaengine.h"
 
@@ -138,10 +137,10 @@ static void mxs_dma_reset_chan(struct mxs_dma_chan *mxs_chan)
 
 	if (dma_is_apbh() && apbh_is_old())
 		writel(1 << (chan_id + BP_APBH_CTRL0_RESET_CHANNEL),
-			mxs_dma->base + HW_APBHX_CTRL0 + MXS_SET_ADDR);
+			mxs_dma->base + HW_APBHX_CTRL0 + STMP_OFFSET_REG_SET);
 	else
 		writel(1 << (chan_id + BP_APBHX_CHANNEL_CTRL_RESET_CHANNEL),
-			mxs_dma->base + HW_APBHX_CHANNEL_CTRL + MXS_SET_ADDR);
+			mxs_dma->base + HW_APBHX_CHANNEL_CTRL + STMP_OFFSET_REG_SET);
 }
 
 static void mxs_dma_enable_chan(struct mxs_dma_chan *mxs_chan)
@@ -170,10 +169,10 @@ static void mxs_dma_pause_chan(struct mxs_dma_chan *mxs_chan)
 	/* freeze the channel */
 	if (dma_is_apbh() && apbh_is_old())
 		writel(1 << chan_id,
-			mxs_dma->base + HW_APBHX_CTRL0 + MXS_SET_ADDR);
+			mxs_dma->base + HW_APBHX_CTRL0 + STMP_OFFSET_REG_SET);
 	else
 		writel(1 << chan_id,
-			mxs_dma->base + HW_APBHX_CHANNEL_CTRL + MXS_SET_ADDR);
+			mxs_dma->base + HW_APBHX_CHANNEL_CTRL + STMP_OFFSET_REG_SET);
 
 	mxs_chan->status = DMA_PAUSED;
 }
@@ -186,10 +185,10 @@ static void mxs_dma_resume_chan(struct mxs_dma_chan *mxs_chan)
 	/* unfreeze the channel */
 	if (dma_is_apbh() && apbh_is_old())
 		writel(1 << chan_id,
-			mxs_dma->base + HW_APBHX_CTRL0 + MXS_CLR_ADDR);
+			mxs_dma->base + HW_APBHX_CTRL0 + STMP_OFFSET_REG_CLR);
 	else
 		writel(1 << chan_id,
-			mxs_dma->base + HW_APBHX_CHANNEL_CTRL + MXS_CLR_ADDR);
+			mxs_dma->base + HW_APBHX_CHANNEL_CTRL + STMP_OFFSET_REG_CLR);
 
 	mxs_chan->status = DMA_IN_PROGRESS;
 }
@@ -224,11 +223,11 @@ static irqreturn_t mxs_dma_int_handler(int irq, void *dev_id)
 	/* completion status */
 	stat1 = readl(mxs_dma->base + HW_APBHX_CTRL1);
 	stat1 &= MXS_DMA_CHANNELS_MASK;
-	writel(stat1, mxs_dma->base + HW_APBHX_CTRL1 + MXS_CLR_ADDR);
+	writel(stat1, mxs_dma->base + HW_APBHX_CTRL1 + STMP_OFFSET_REG_CLR);
 
 	/* error status */
 	stat2 = readl(mxs_dma->base + HW_APBHX_CTRL2);
-	writel(stat2, mxs_dma->base + HW_APBHX_CTRL2 + MXS_CLR_ADDR);
+	writel(stat2, mxs_dma->base + HW_APBHX_CTRL2 + STMP_OFFSET_REG_CLR);
 
 	/*
 	 * When both completion and error of termination bits set at the
@@ -571,7 +570,7 @@ static int __init mxs_dma_init(struct mxs_dma_engine *mxs_dma)
 	if (ret)
 		return ret;
 
-	ret = mxs_reset_block(mxs_dma->base);
+	ret = stmp_reset_block(mxs_dma->base);
 	if (ret)
 		goto err_out;
 
@@ -584,14 +583,14 @@ static int __init mxs_dma_init(struct mxs_dma_engine *mxs_dma)
 	/* enable apbh burst */
 	if (dma_is_apbh()) {
 		writel(BM_APBH_CTRL0_APB_BURST_EN,
-			mxs_dma->base + HW_APBHX_CTRL0 + MXS_SET_ADDR);
+			mxs_dma->base + HW_APBHX_CTRL0 + STMP_OFFSET_REG_SET);
 		writel(BM_APBH_CTRL0_APB_BURST8_EN,
-			mxs_dma->base + HW_APBHX_CTRL0 + MXS_SET_ADDR);
+			mxs_dma->base + HW_APBHX_CTRL0 + STMP_OFFSET_REG_SET);
 	}
 
 	/* enable irq for all the channels */
 	writel(MXS_DMA_CHANNELS_MASK << MXS_DMA_CHANNELS,
-		mxs_dma->base + HW_APBHX_CTRL1 + MXS_SET_ADDR);
+		mxs_dma->base + HW_APBHX_CTRL1 + STMP_OFFSET_REG_SET);
 
 err_out:
 	clk_disable_unprepare(mxs_dma->clk);
Signature · 1 line
1.7.0.4
#2 lore raw

PATCH 1/5 dma: mxs-dma: use global stmp_device functionality

Dear Dong Aisheng,

Quoted 3 lines

From: Dong Aisheng <[email protected]>

This can get rid of the mach-dependency.

Quoted 46 lines
Cc Vinod Koul <[email protected]>
Cc Dan Williams <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Huang Shijie <[email protected]>

---
This patch depends on Wolfram's
[PATCH V2 1/3] lib: add support for stmp-style devices
http://www.spinics.net/lists/arm-kernel/msg166232.html
which is still not in mainline.

---
 drivers/dma/Kconfig   |    1 +
 drivers/dma/mxs-dma.c |   27 +++++++++++++--------------
 2 files changed, 14 insertions(+), 14 deletions(-)
drivers/dma/Kconfig +1 -0
index cf9da36..af2a29f 100644
@@ -239,6 +239,7 @@ config IMX_DMA
 config MXS_DMA
 	bool "MXS DMA support"
 	depends on SOC_IMX23 || SOC_IMX28
+	select STMP_DEVICE
 	select DMA_ENGINE
 	help
 	  Support the MXS DMA engine. This engine including APBH-DMA
drivers/dma/mxs-dma.c +1 -2
index c81ef7e..b0051a8 100644
@@ -23,10 +23,9 @@
 #include <linux/dmaengine.h>
 #include <linux/delay.h>
 #include <linux/fsl/mxs-dma.h>
+#include <linux/stmp_device.h>
 
 #include <asm/irq.h>
-#include <mach/mxs.h>
-#include <mach/common.h>
 
 #include "dmaengine.h"
 

drivers/dma/mxs-dma.c static void mxs_dma_reset_chan(struct mxs_dma_chan · line 144

Quoted 10 lines
drivers/dma/mxs-dma.c +2 -2
@@ -138,10 +137,10 @@ static void mxs_dma_reset_chan(struct mxs_dma_chan
 *mxs_chan)
 
 	if (dma_is_apbh() && apbh_is_old())
 		writel(1 << (chan_id + BP_APBH_CTRL0_RESET_CHANNEL),
-			mxs_dma->base + HW_APBHX_CTRL0 + MXS_SET_ADDR);
+			mxs_dma->base + HW_APBHX_CTRL0 + STMP_OFFSET_REG_SET);
 	else
 		writel(1 << (chan_id + BP_APBHX_CHANNEL_CTRL_RESET_CHANNEL),
-			mxs_dma->base + HW_APBHX_CHANNEL_CTRL + MXS_SET_ADDR);
+			mxs_dma->base + HW_APBHX_CHANNEL_CTRL + 

STMP_OFFSET_REG_SET);

drivers/dma/mxs-dma.c static void mxs_dma_pause_chan(struct mxs_dma_chan · line 175

Quoted 13 lines
drivers/dma/mxs-dma.c +0 -0
 }
 

static void mxs_dma_enable_chan(struct mxs_dma_chan *mxs_chan)

drivers/dma/mxs-dma.c +2 -2
@@ -170,10 +169,10 @@ static void mxs_dma_pause_chan(struct mxs_dma_chan
 *mxs_chan) /* freeze the channel */
 	if (dma_is_apbh() && apbh_is_old())
 		writel(1 << chan_id,
-			mxs_dma->base + HW_APBHX_CTRL0 + MXS_SET_ADDR);
+			mxs_dma->base + HW_APBHX_CTRL0 + STMP_OFFSET_REG_SET);
 	else
 		writel(1 << chan_id,
-			mxs_dma->base + HW_APBHX_CHANNEL_CTRL + MXS_SET_ADDR);
+			mxs_dma->base + HW_APBHX_CHANNEL_CTRL + 

STMP_OFFSET_REG_SET);

drivers/dma/mxs-dma.c static void mxs_dma_resume_chan(struct mxs_dma_chan · line 191

Quoted 13 lines
drivers/dma/mxs-dma.c +2 -2
 	mxs_chan->status = DMA_PAUSED;
 }
@@ -186,10 +185,10 @@ static void mxs_dma_resume_chan(struct mxs_dma_chan
 *mxs_chan) /* unfreeze the channel */
 	if (dma_is_apbh() && apbh_is_old())
 		writel(1 << chan_id,
-			mxs_dma->base + HW_APBHX_CTRL0 + MXS_CLR_ADDR);
+			mxs_dma->base + HW_APBHX_CTRL0 + STMP_OFFSET_REG_CLR);
 	else
 		writel(1 << chan_id,
-			mxs_dma->base + HW_APBHX_CHANNEL_CTRL + MXS_CLR_ADDR);
+			mxs_dma->base + HW_APBHX_CHANNEL_CTRL + 

STMP_OFFSET_REG_CLR);

Quoted 34 lines
drivers/dma/mxs-dma.c +4 -5
 	mxs_chan->status = DMA_IN_PROGRESS;
 }
@@ -224,11 +223,11 @@ static irqreturn_t mxs_dma_int_handler(int irq, void
 *dev_id) /* completion status */
 	stat1 = readl(mxs_dma->base + HW_APBHX_CTRL1);
 	stat1 &= MXS_DMA_CHANNELS_MASK;
-	writel(stat1, mxs_dma->base + HW_APBHX_CTRL1 + MXS_CLR_ADDR);
+	writel(stat1, mxs_dma->base + HW_APBHX_CTRL1 + STMP_OFFSET_REG_CLR);
 
 	/* error status */
 	stat2 = readl(mxs_dma->base + HW_APBHX_CTRL2);
-	writel(stat2, mxs_dma->base + HW_APBHX_CTRL2 + MXS_CLR_ADDR);
+	writel(stat2, mxs_dma->base + HW_APBHX_CTRL2 + STMP_OFFSET_REG_CLR);
 
 	/*
 	 * When both completion and error of termination bits set at the
@@ -571,7 +570,7 @@ static int __init mxs_dma_init(struct mxs_dma_engine
 *mxs_dma) if (ret)
 		return ret;
 
-	ret = mxs_reset_block(mxs_dma->base);
+	ret = stmp_reset_block(mxs_dma->base);
 	if (ret)
 		goto err_out;
 
@@ -584,14 +583,14 @@ static int __init mxs_dma_init(struct mxs_dma_engine
 *mxs_dma) /* enable apbh burst */
 	if (dma_is_apbh()) {
 		writel(BM_APBH_CTRL0_APB_BURST_EN,
-			mxs_dma->base + HW_APBHX_CTRL0 + MXS_SET_ADDR);
+			mxs_dma->base + HW_APBHX_CTRL0 + STMP_OFFSET_REG_SET);
 		writel(BM_APBH_CTRL0_APB_BURST8_EN,
-			mxs_dma->base + HW_APBHX_CTRL0 + MXS_SET_ADDR);

drivers/dma/mxs-dma.c static int __init mxs_dma_init(struct mxs_dma_engine · line 596

Quoted 10 lines
drivers/dma/mxs-dma.c +2 -1
+			mxs_dma->base + HW_APBHX_CTRL0 + STMP_OFFSET_REG_SET);
 	}
 
 	/* enable irq for all the channels */
 	writel(MXS_DMA_CHANNELS_MASK << MXS_DMA_CHANNELS,
-		mxs_dma->base + HW_APBHX_CTRL1 + MXS_SET_ADDR);
+		mxs_dma->base + HW_APBHX_CTRL1 + STMP_OFFSET_REG_SET);
 
 err_out:
 	clk_disable_unprepare(mxs_dma->clk);

Best regards,
Marek Vasut

#3 lore raw

PATCH 2/5 dma: mxs-dma: make platform_device_id more generic

In reply to Dong Aisheng

From: Dong Aisheng <[email protected]>

Rewrite mxs_dma_is_apbh and mxs_dma_is_apbx in order to support other SoCs like imx6q and reform the platform_device_id for the better further dt support.

Cc Vinod Koul <[email protected]>
Cc Dan Williams <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Huang Shijie <[email protected]>
---
 arch/arm/mach-mxs/clock-mx23.c           |    4 +-
 arch/arm/mach-mxs/clock-mx28.c           |    4 +-
 arch/arm/mach-mxs/devices/platform-dma.c |   14 ++--
 drivers/dma/mxs-dma.c                    |  111 +++++++++++++++++++++++-------
 include/linux/fsl/mxs-dma.h              |   12 +---
 5 files changed, 101 insertions(+), 44 deletions(-)
arch/arm/mach-mxs/clock-mx23.c +2 -2
index e3ac52c..1e67b85 100644
@@ -427,8 +427,8 @@ static struct clk_lookup lookups[] = {
 	_REGISTER_CLOCK("duart", NULL, uart_clk)
 	_REGISTER_CLOCK("mxs-auart.0", NULL, uart_clk)
 	_REGISTER_CLOCK("rtc", NULL, rtc_clk)
-	_REGISTER_CLOCK("mxs-dma-apbh", NULL, hbus_clk)
-	_REGISTER_CLOCK("mxs-dma-apbx", NULL, xbus_clk)
+	_REGISTER_CLOCK("imx23-dma-apbh", NULL, hbus_clk)
+	_REGISTER_CLOCK("imx23-dma-apbx", NULL, xbus_clk)
 	_REGISTER_CLOCK("mxs-mmc.0", NULL, ssp_clk)
 	_REGISTER_CLOCK("mxs-mmc.1", NULL, ssp_clk)
 	_REGISTER_CLOCK(NULL, "usb", usb_clk)
arch/arm/mach-mxs/clock-mx28.c +2 -2
index 1867a17..cca2cf7 100644
@@ -625,8 +625,8 @@ static struct clk_lookup lookups[] = {
 	_REGISTER_CLOCK("mxs-auart.4", NULL, uart_clk)
 	_REGISTER_CLOCK("rtc", NULL, rtc_clk)
 	_REGISTER_CLOCK("pll2", NULL, pll2_clk)
-	_REGISTER_CLOCK("mxs-dma-apbh", NULL, hbus_clk)
-	_REGISTER_CLOCK("mxs-dma-apbx", NULL, xbus_clk)
+	_REGISTER_CLOCK("imx28-dma-apbh", NULL, hbus_clk)
+	_REGISTER_CLOCK("imx28-dma-apbx", NULL, xbus_clk)
 	_REGISTER_CLOCK("mxs-mmc.0", NULL, ssp0_clk)
 	_REGISTER_CLOCK("mxs-mmc.1", NULL, ssp1_clk)
 	_REGISTER_CLOCK("mxs-mmc.2", NULL, ssp2_clk)
arch/arm/mach-mxs/devices/platform-dma.c +8 -6
index 6a0202b..aff4813 100644
@@ -32,17 +32,19 @@ static struct platform_device *__init mxs_add_dma(const char *devid,
 
 static int __init mxs_add_mxs_dma(void)
 {
-	char *apbh = "mxs-dma-apbh";
-	char *apbx = "mxs-dma-apbx";
+	char *mx23_apbh = "imx23-dma-apbh";
+	char *mx23_apbx = "imx23-dma-apbx";
+	char *mx28_apbh = "imx28-dma-apbh";
+	char *mx28_apbx = "imx28-dma-apbx";
 
 	if (cpu_is_mx23()) {
-		mxs_add_dma(apbh, MX23_APBH_DMA_BASE_ADDR);
-		mxs_add_dma(apbx, MX23_APBX_DMA_BASE_ADDR);
+		mxs_add_dma(mx23_apbh, MX23_APBH_DMA_BASE_ADDR);
+		mxs_add_dma(mx23_apbx, MX23_APBX_DMA_BASE_ADDR);
 	}
 
 	if (cpu_is_mx28()) {
-		mxs_add_dma(apbh, MX28_APBH_DMA_BASE_ADDR);
-		mxs_add_dma(apbx, MX28_APBX_DMA_BASE_ADDR);
+		mxs_add_dma(mx28_apbh, MX28_APBH_DMA_BASE_ADDR);
+		mxs_add_dma(mx28_apbx, MX28_APBX_DMA_BASE_ADDR);
 	}
 
 	return 0;
drivers/dma/mxs-dma.c +87 -24

165 lines not on this page · raw message

include/linux/fsl/mxs-dma.h +2 -10
index 203d7c4..55d8702 100644
@@ -15,14 +15,6 @@ struct mxs_dma_data {
 	int chan_irq;
 };
 
-static inline int mxs_dma_is_apbh(struct dma_chan *chan)
-{
-	return !strcmp(dev_name(chan->device->dev), "mxs-dma-apbh");
-}
-
-static inline int mxs_dma_is_apbx(struct dma_chan *chan)
-{
-	return !strcmp(dev_name(chan->device->dev), "mxs-dma-apbx");
-}
-
+extern int mxs_dma_is_apbh(struct dma_chan *chan);
+extern int mxs_dma_is_apbx(struct dma_chan *chan);
 #endif /* __MACH_MXS_DMA_H__ */
Signature · 1 line
1.7.0.4
#4 lore raw

PATCH 2/5 dma: mxs-dma: make platform_device_id more generic

Dear Dong Aisheng,

Quoted 6 lines

From: Dong Aisheng <[email protected]>

Rewrite mxs_dma_is_apbh and mxs_dma_is_apbx in order to support other SoCs like imx6q and reform the platform_device_id for the better further dt support.

Quoted 7 lines
Cc Vinod Koul <[email protected]>
Cc Dan Williams <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Huang Shijie <[email protected]>
Quoted 75 lines
---
 arch/arm/mach-mxs/clock-mx23.c           |    4 +-
 arch/arm/mach-mxs/clock-mx28.c           |    4 +-
 arch/arm/mach-mxs/devices/platform-dma.c |   14 ++--
 drivers/dma/mxs-dma.c                    |  111
+++++++++++++++++++++++------- include/linux/fsl/mxs-dma.h              | 
 12 +---
 5 files changed, 101 insertions(+), 44 deletions(-)
(quoted diff) +0 -0

b/arch/arm/mach-mxs/clock-mx23.c index e3ac52c..1e67b85 100644

arch/arm/mach-mxs/clock-mx23.c +2 -2
@@ -427,8 +427,8 @@ static struct clk_lookup lookups[] = {
 	_REGISTER_CLOCK("duart", NULL, uart_clk)
 	_REGISTER_CLOCK("mxs-auart.0", NULL, uart_clk)
 	_REGISTER_CLOCK("rtc", NULL, rtc_clk)
-	_REGISTER_CLOCK("mxs-dma-apbh", NULL, hbus_clk)
-	_REGISTER_CLOCK("mxs-dma-apbx", NULL, xbus_clk)
+	_REGISTER_CLOCK("imx23-dma-apbh", NULL, hbus_clk)
+	_REGISTER_CLOCK("imx23-dma-apbx", NULL, xbus_clk)
 	_REGISTER_CLOCK("mxs-mmc.0", NULL, ssp_clk)
 	_REGISTER_CLOCK("mxs-mmc.1", NULL, ssp_clk)
 	_REGISTER_CLOCK(NULL, "usb", usb_clk)
(quoted diff) +0 -0

b/arch/arm/mach-mxs/clock-mx28.c index 1867a17..cca2cf7 100644

arch/arm/mach-mxs/clock-mx28.c +2 -2
@@ -625,8 +625,8 @@ static struct clk_lookup lookups[] = {
 	_REGISTER_CLOCK("mxs-auart.4", NULL, uart_clk)
 	_REGISTER_CLOCK("rtc", NULL, rtc_clk)
 	_REGISTER_CLOCK("pll2", NULL, pll2_clk)
-	_REGISTER_CLOCK("mxs-dma-apbh", NULL, hbus_clk)
-	_REGISTER_CLOCK("mxs-dma-apbx", NULL, xbus_clk)
+	_REGISTER_CLOCK("imx28-dma-apbh", NULL, hbus_clk)
+	_REGISTER_CLOCK("imx28-dma-apbx", NULL, xbus_clk)
 	_REGISTER_CLOCK("mxs-mmc.0", NULL, ssp0_clk)
 	_REGISTER_CLOCK("mxs-mmc.1", NULL, ssp1_clk)
 	_REGISTER_CLOCK("mxs-mmc.2", NULL, ssp2_clk)
(quoted diff) +0 -0

b/arch/arm/mach-mxs/devices/platform-dma.c index 6a0202b..aff4813 100644

arch/arm/mach-mxs/devices/platform-dma.c +8 -6
@@ -32,17 +32,19 @@ static struct platform_device *__init mxs_add_dma(const
 char *devid,
 
 static int __init mxs_add_mxs_dma(void)
 {
-	char *apbh = "mxs-dma-apbh";
-	char *apbx = "mxs-dma-apbx";
+	char *mx23_apbh = "imx23-dma-apbh";
+	char *mx23_apbx = "imx23-dma-apbx";
+	char *mx28_apbh = "imx28-dma-apbh";
+	char *mx28_apbx = "imx28-dma-apbx";
 
 	if (cpu_is_mx23()) {
-		mxs_add_dma(apbh, MX23_APBH_DMA_BASE_ADDR);
-		mxs_add_dma(apbx, MX23_APBX_DMA_BASE_ADDR);
+		mxs_add_dma(mx23_apbh, MX23_APBH_DMA_BASE_ADDR);
+		mxs_add_dma(mx23_apbx, MX23_APBX_DMA_BASE_ADDR);
 	}
 
 	if (cpu_is_mx28()) {
-		mxs_add_dma(apbh, MX28_APBH_DMA_BASE_ADDR);
-		mxs_add_dma(apbx, MX28_APBX_DMA_BASE_ADDR);
+		mxs_add_dma(mx28_apbh, MX28_APBH_DMA_BASE_ADDR);
+		mxs_add_dma(mx28_apbx, MX28_APBX_DMA_BASE_ADDR);
 	}
 
arch/arm/mach-mxs/devices/platform-dma.c +0 -0
 	return 0;
drivers/dma/mxs-dma.c +0 -0
index b0051a8..51a29f9 100644

drivers/dma/mxs-dma.c line 53

Quoted 4 lines
drivers/dma/mxs-dma.c +0 -1
@@ -50,7 +50,8 @@
 #define HW_APBHX_CTRL2				0x020
 #define HW_APBHX_CHANNEL_CTRL			0x030
 #define BP_APBHX_CHANNEL_CTRL_RESET_CHANNEL	16
-#define HW_APBH_VERSION				(cpu_is_mx23() ? 0x3f0 : 

0x800)

Quoted 189 lines
drivers/dma/mxs-dma.c +87 -23
+#define HW_APBH_VERSION_IMX23			0x3f0
+#define HW_APBH_VERSION_IMX28			0x800
 #define HW_APBX_VERSION				0x800
 #define BP_APBHX_VERSION_MAJOR			24
 #define HW_APBHX_CHn_NXTCMDAR(n) \
@@ -121,7 +122,8 @@ struct mxs_dma_chan {
 #define MXS_DMA_CHANNELS_MASK		0xffff
 
 struct mxs_dma_engine {
-	int				dev_id;
+	unsigned int			type;
+	unsigned int			dev_id;
 	unsigned int			version;
 	void __iomem			*base;
 	struct clk			*clk;
@@ -130,6 +132,74 @@ struct mxs_dma_engine {
 	struct mxs_dma_chan		mxs_chans[MXS_DMA_CHANNELS];
 };
 
+enum mxs_dma_devtype {
+	IMX23_DMA,
+	IMX28_DMA,
+};
+
+struct mxs_dma_type {
+	unsigned int type;
+	unsigned int id;
+};
+
+static struct mxs_dma_type mxs_dma_types[] = {
+	{
+		.type = IMX23_DMA,
+		.id = MXS_DMA_APBH,
+	}, {
+		.type = IMX23_DMA,
+		.id = MXS_DMA_APBX,
+	}, {
+		.type = IMX28_DMA,
+		.id = MXS_DMA_APBH,
+	}, {
+		.type = IMX28_DMA,
+		.id = MXS_DMA_APBX,
+	}, {
+		/* end of list */
+	}
+};
+
+static struct platform_device_id mxs_dma_idt[] = {
+	{
+		.name = "imx23-dma-apbh",
+		.driver_data = (kernel_ulong_t) &mxs_dma_types[0],
+	}, {
+		.name = "imx23-dma-apbx",
+		.driver_data = (kernel_ulong_t) &mxs_dma_types[1],
+	}, {
+		.name = "imx28-dma-apbh",
+		.driver_data = (kernel_ulong_t) &mxs_dma_types[2],
+	}, {
+		.name = "imx28-dma-apbx",
+		.driver_data = (kernel_ulong_t) &mxs_dma_types[3],
+	}, {
+		/* end of list */
+	}
+};
+
+static struct mxs_dma_chan *to_mxs_dma_chan(struct dma_chan *chan)
+{
+	return container_of(chan, struct mxs_dma_chan, chan);
+}
+
+int mxs_dma_is_apbh(struct dma_chan *chan)
+{
+	struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
+	struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
+
+	return dma_is_apbh();
+}
+
+int mxs_dma_is_apbx(struct dma_chan *chan)
+{
+	struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
+	struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
+
+	return !dma_is_apbh();
+}
+
+
 static void mxs_dma_reset_chan(struct mxs_dma_chan *mxs_chan)
 {
 	struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
@@ -193,11 +263,6 @@ static void mxs_dma_resume_chan(struct mxs_dma_chan
 *mxs_chan) mxs_chan->status = DMA_IN_PROGRESS;
 }
 
-static struct mxs_dma_chan *to_mxs_dma_chan(struct dma_chan *chan)
-{
-	return container_of(chan, struct mxs_dma_chan, chan);
-}
-
 static dma_cookie_t mxs_dma_tx_submit(struct dma_async_tx_descriptor *tx)
 {
 	struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(tx->chan);
@@ -574,11 +639,18 @@ static int __init mxs_dma_init(struct mxs_dma_engine
 *mxs_dma) if (ret)
 		goto err_out;
 
-	/* only major version matters */
-	mxs_dma->version = readl(mxs_dma->base +
+	if (mxs_dma->type == IMX23_DMA) {
+		/* only major version matters */
+		mxs_dma->version = readl(mxs_dma->base +
 				((mxs_dma->dev_id == MXS_DMA_APBX) ?
-				HW_APBX_VERSION : HW_APBH_VERSION)) >>
+				HW_APBX_VERSION : HW_APBH_VERSION_IMX23)) >>
 				BP_APBHX_VERSION_MAJOR;
+	} else if (mxs_dma->type == IMX28_DMA) {
+		mxs_dma->version = readl(mxs_dma->base +
+				((mxs_dma->dev_id == MXS_DMA_APBX) ?
+				HW_APBX_VERSION : HW_APBH_VERSION_IMX28)) >>
+				BP_APBHX_VERSION_MAJOR;
+	}
 
 	/* enable apbh burst */
 	if (dma_is_apbh()) {
@@ -601,6 +673,8 @@ static int __init mxs_dma_probe(struct platform_device
 *pdev) {
 	const struct platform_device_id *id_entry =
 				platform_get_device_id(pdev);
+	const struct mxs_dma_type *mtype =
+			(struct mxs_dma_type *)id_entry->driver_data;
 	struct mxs_dma_engine *mxs_dma;
 	struct resource *iores;
 	int ret, i;
@@ -609,7 +683,8 @@ static int __init mxs_dma_probe(struct platform_device
 *pdev) if (!mxs_dma)
 		return -ENOMEM;
 
-	mxs_dma->dev_id = id_entry->driver_data;
+	mxs_dma->type = mtype->type;
+	mxs_dma->dev_id = mtype->id;
 
 	iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 
@@ -692,23 +767,11 @@ err_request_region:
 	return ret;
 }
 
-static struct platform_device_id mxs_dma_type[] = {
-	{
-		.name = "mxs-dma-apbh",
-		.driver_data = MXS_DMA_APBH,
-	}, {
-		.name = "mxs-dma-apbx",
-		.driver_data = MXS_DMA_APBX,
-	}, {
-		/* end of list */
-	}
-};
-
 static struct platform_driver mxs_dma_driver = {
 	.driver		= {
 		.name	= "mxs-dma",
 	},
-	.id_table	= mxs_dma_type,
+	.id_table	= mxs_dma_idt,
 };
 
 static int __init mxs_dma_module_init(void)
include/linux/fsl/mxs-dma.h +2 -10
index 203d7c4..55d8702 100644
@@ -15,14 +15,6 @@ struct mxs_dma_data {
 	int chan_irq;
 };
 
-static inline int mxs_dma_is_apbh(struct dma_chan *chan)
-{
-	return !strcmp(dev_name(chan->device->dev), "mxs-dma-apbh");
-}
-
-static inline int mxs_dma_is_apbx(struct dma_chan *chan)
-{
-	return !strcmp(dev_name(chan->device->dev), "mxs-dma-apbx");
-}
-
+extern int mxs_dma_is_apbh(struct dma_chan *chan);
+extern int mxs_dma_is_apbx(struct dma_chan *chan);
 #endif /* __MACH_MXS_DMA_H__ */
#5 lore raw

PATCH 2/5 dma: mxs-dma: make platform_device_id more generic

In reply to Dong Aisheng
Quoted 6 lines · Dong Aisheng

From: Dong Aisheng <[email protected]>

Rewrite mxs_dma_is_apbh and mxs_dma_is_apbx in order to support other SoCs like imx6q and reform the platform_device_id for the better further dt support.

Nice cleanup. But I would like to ask more.

Quoted 65 lines
Cc Vinod Koul <[email protected]>
Cc Dan Williams <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Huang Shijie <[email protected]>
---
 arch/arm/mach-mxs/clock-mx23.c           |    4 +-
 arch/arm/mach-mxs/clock-mx28.c           |    4 +-
 arch/arm/mach-mxs/devices/platform-dma.c |   14 ++--
 drivers/dma/mxs-dma.c                    |  111 +++++++++++++++++++++++-------
 include/linux/fsl/mxs-dma.h              |   12 +---
 5 files changed, 101 insertions(+), 44 deletions(-)
arch/arm/mach-mxs/clock-mx23.c +2 -2
index e3ac52c..1e67b85 100644
@@ -427,8 +427,8 @@ static struct clk_lookup lookups[] = {
 	_REGISTER_CLOCK("duart", NULL, uart_clk)
 	_REGISTER_CLOCK("mxs-auart.0", NULL, uart_clk)
 	_REGISTER_CLOCK("rtc", NULL, rtc_clk)
-	_REGISTER_CLOCK("mxs-dma-apbh", NULL, hbus_clk)
-	_REGISTER_CLOCK("mxs-dma-apbx", NULL, xbus_clk)
+	_REGISTER_CLOCK("imx23-dma-apbh", NULL, hbus_clk)
+	_REGISTER_CLOCK("imx23-dma-apbx", NULL, xbus_clk)
 	_REGISTER_CLOCK("mxs-mmc.0", NULL, ssp_clk)
 	_REGISTER_CLOCK("mxs-mmc.1", NULL, ssp_clk)
 	_REGISTER_CLOCK(NULL, "usb", usb_clk)
arch/arm/mach-mxs/clock-mx28.c +2 -2
index 1867a17..cca2cf7 100644
@@ -625,8 +625,8 @@ static struct clk_lookup lookups[] = {
 	_REGISTER_CLOCK("mxs-auart.4", NULL, uart_clk)
 	_REGISTER_CLOCK("rtc", NULL, rtc_clk)
 	_REGISTER_CLOCK("pll2", NULL, pll2_clk)
-	_REGISTER_CLOCK("mxs-dma-apbh", NULL, hbus_clk)
-	_REGISTER_CLOCK("mxs-dma-apbx", NULL, xbus_clk)
+	_REGISTER_CLOCK("imx28-dma-apbh", NULL, hbus_clk)
+	_REGISTER_CLOCK("imx28-dma-apbx", NULL, xbus_clk)
 	_REGISTER_CLOCK("mxs-mmc.0", NULL, ssp0_clk)
 	_REGISTER_CLOCK("mxs-mmc.1", NULL, ssp1_clk)
 	_REGISTER_CLOCK("mxs-mmc.2", NULL, ssp2_clk)
arch/arm/mach-mxs/devices/platform-dma.c +6 -4
index 6a0202b..aff4813 100644
@@ -32,17 +32,19 @@ static struct platform_device *__init mxs_add_dma(const char *devid,
 
 static int __init mxs_add_mxs_dma(void)
 {
-	char *apbh = "mxs-dma-apbh";
-	char *apbx = "mxs-dma-apbx";
+	char *mx23_apbh = "imx23-dma-apbh";
+	char *mx23_apbx = "imx23-dma-apbx";
+	char *mx28_apbh = "imx28-dma-apbh";
+	char *mx28_apbx = "imx28-dma-apbx";
 
 	if (cpu_is_mx23()) {
-		mxs_add_dma(apbh, MX23_APBH_DMA_BASE_ADDR);
-		mxs_add_dma(apbx, MX23_APBX_DMA_BASE_ADDR);
+		mxs_add_dma(mx23_apbh, MX23_APBH_DMA_BASE_ADDR);
+		mxs_add_dma(mx23_apbx, MX23_APBX_DMA_BASE_ADDR);

arch/arm/mach-mxs/devices/platform-dma.c static struct platform_device *__init mxs_add_dma(const char *devid, · line 50

Quoted 10 lines
arch/arm/mach-mxs/devices/platform-dma.c +2 -2
 	}
 
 	if (cpu_is_mx28()) {
-		mxs_add_dma(apbh, MX28_APBH_DMA_BASE_ADDR);
-		mxs_add_dma(apbx, MX28_APBX_DMA_BASE_ADDR);
+		mxs_add_dma(mx28_apbh, MX28_APBH_DMA_BASE_ADDR);
+		mxs_add_dma(mx28_apbx, MX28_APBX_DMA_BASE_ADDR);
 	}
 
 	return 0;

With soc specific hook introduced, we can remove this mxs_add_mxs_dma call completely.

drivers/dma/mxs-dma.c line 54

Quoted 11 lines
drivers/dma/mxs-dma.c +2 -1
index b0051a8..51a29f9 100644
@@ -50,7 +50,8 @@
 #define HW_APBHX_CTRL2				0x020
 #define HW_APBHX_CHANNEL_CTRL			0x030
 #define BP_APBHX_CHANNEL_CTRL_RESET_CHANNEL	16
-#define HW_APBH_VERSION				(cpu_is_mx23() ? 0x3f0 : 0x800)
+#define HW_APBH_VERSION_IMX23			0x3f0
+#define HW_APBH_VERSION_IMX28			0x800

Since the VERSION is broken for identifying the dma type and we are introduce dma type field, I would suggest to remove the use of VERSION completely and use mxs_dma_type everywhere.

Quoted 21 lines
drivers/dma/mxs-dma.c +6 -1
 #define HW_APBX_VERSION				0x800
 #define BP_APBHX_VERSION_MAJOR			24
 #define HW_APBHX_CHn_NXTCMDAR(n) \
@@ -121,7 +122,8 @@ struct mxs_dma_chan {
 #define MXS_DMA_CHANNELS_MASK		0xffff
 
 struct mxs_dma_engine {
-	int				dev_id;
+	unsigned int			type;
+	unsigned int			dev_id;
 	unsigned int			version;
 	void __iomem			*base;
 	struct clk			*clk;
@@ -130,6 +132,74 @@ struct mxs_dma_engine {
 	struct mxs_dma_chan		mxs_chans[MXS_DMA_CHANNELS];
 };
 
+enum mxs_dma_devtype {
+	IMX23_DMA,
+	IMX28_DMA,
+};

drivers/dma/mxs-dma.c struct mxs_dma_engine { · line 148

Quoted 10 lines
drivers/dma/mxs-dma.c +10 -0
+
+struct mxs_dma_type {
+	unsigned int type;
+	unsigned int id;
+};
+
+static struct mxs_dma_type mxs_dma_types[] = {
+	{
+		.type = IMX23_DMA,
+		.id = MXS_DMA_APBH,

To me, IMX23_DMA is more like a id, while MXS_DMA_APBH is more like a type.

drivers/dma/mxs-dma.c struct mxs_dma_engine { · line 160

Quoted 12 lines
drivers/dma/mxs-dma.c +12 -0
+	}, {
+		.type = IMX23_DMA,
+		.id = MXS_DMA_APBX,
+	}, {
+		.type = IMX28_DMA,
+		.id = MXS_DMA_APBH,
+	}, {
+		.type = IMX28_DMA,
+		.id = MXS_DMA_APBX,
+	}, {
+		/* end of list */
+	}

This end sign is not needed, I think.

drivers/dma/mxs-dma.c struct mxs_dma_engine { · line 163

Quoted 3 lines
drivers/dma/mxs-dma.c +3 -0
+};
+
+static struct platform_device_id mxs_dma_idt[] = {

s/mxs_dma_idt/mxs_dma_ids?

Quoted 18 lines
drivers/dma/mxs-dma.c +18 -0
+	{
+		.name = "imx23-dma-apbh",
+		.driver_data = (kernel_ulong_t) &mxs_dma_types[0],
+	}, {
+		.name = "imx23-dma-apbx",
+		.driver_data = (kernel_ulong_t) &mxs_dma_types[1],
+	}, {
+		.name = "imx28-dma-apbh",
+		.driver_data = (kernel_ulong_t) &mxs_dma_types[2],
+	}, {
+		.name = "imx28-dma-apbx",
+		.driver_data = (kernel_ulong_t) &mxs_dma_types[3],
+	}, {
+		/* end of list */
+	}
+};
+
+static struct mxs_dma_chan *to_mxs_dma_chan(struct dma_chan *chan)

drivers/dma/mxs-dma.c struct mxs_dma_engine { · line 191

Quoted 10 lines
drivers/dma/mxs-dma.c +10 -0
+{
+	return container_of(chan, struct mxs_dma_chan, chan);
+}
+
+int mxs_dma_is_apbh(struct dma_chan *chan)
+{
+	struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
+	struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
+
+	return dma_is_apbh();

I would expect it simply be

	return (mxs_dma->dev_id == MXS_DMA_APBH);

And dma_is_apbh() can be removed completely now.

drivers/dma/mxs-dma.c struct mxs_dma_engine { · line 199

Quoted 8 lines
drivers/dma/mxs-dma.c +8 -0
+}
+
+int mxs_dma_is_apbx(struct dma_chan *chan)
+{
+	struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
+	struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
+
+	return !dma_is_apbh();
	return (mxs_dma->dev_id == MXS_DMA_APBX);
Quoted 27 lines
drivers/dma/mxs-dma.c +6 -7
+}
+
+
 static void mxs_dma_reset_chan(struct mxs_dma_chan *mxs_chan)
 {
 	struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
@@ -193,11 +263,6 @@ static void mxs_dma_resume_chan(struct mxs_dma_chan *mxs_chan)
 	mxs_chan->status = DMA_IN_PROGRESS;
 }
 
-static struct mxs_dma_chan *to_mxs_dma_chan(struct dma_chan *chan)
-{
-	return container_of(chan, struct mxs_dma_chan, chan);
-}
-
 static dma_cookie_t mxs_dma_tx_submit(struct dma_async_tx_descriptor *tx)
 {
 	struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(tx->chan);
@@ -574,11 +639,18 @@ static int __init mxs_dma_init(struct mxs_dma_engine *mxs_dma)
 	if (ret)
 		goto err_out;
 
-	/* only major version matters */
-	mxs_dma->version = readl(mxs_dma->base +
+	if (mxs_dma->type == IMX23_DMA) {
+		/* only major version matters */
+		mxs_dma->version = readl(mxs_dma->base +

drivers/dma/mxs-dma.c static int __init mxs_dma_init(struct mxs_dma_engine *mxs_dma) · line 653

Quoted 10 lines
drivers/dma/mxs-dma.c +7 -1
 				((mxs_dma->dev_id == MXS_DMA_APBX) ?
-				HW_APBX_VERSION : HW_APBH_VERSION)) >>
+				HW_APBX_VERSION : HW_APBH_VERSION_IMX23)) >>
 				BP_APBHX_VERSION_MAJOR;
+	} else if (mxs_dma->type == IMX28_DMA) {
+		mxs_dma->version = readl(mxs_dma->base +
+				((mxs_dma->dev_id == MXS_DMA_APBX) ?
+				HW_APBX_VERSION : HW_APBH_VERSION_IMX28)) >>
+				BP_APBHX_VERSION_MAJOR;
+	}

Again, let's just get rid of the use of VERSION completely.

Quoted 57 lines
drivers/dma/mxs-dma.c +5 -14
 	/* enable apbh burst */
 	if (dma_is_apbh()) {
@@ -601,6 +673,8 @@ static int __init mxs_dma_probe(struct platform_device *pdev)
 {
 	const struct platform_device_id *id_entry =
 				platform_get_device_id(pdev);
+	const struct mxs_dma_type *mtype =
+			(struct mxs_dma_type *)id_entry->driver_data;
 	struct mxs_dma_engine *mxs_dma;
 	struct resource *iores;
 	int ret, i;
@@ -609,7 +683,8 @@ static int __init mxs_dma_probe(struct platform_device *pdev)
 	if (!mxs_dma)
 		return -ENOMEM;
 
-	mxs_dma->dev_id = id_entry->driver_data;
+	mxs_dma->type = mtype->type;
+	mxs_dma->dev_id = mtype->id;
 
 	iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 
@@ -692,23 +767,11 @@ err_request_region:
 	return ret;
 }
 
-static struct platform_device_id mxs_dma_type[] = {
-	{
-		.name = "mxs-dma-apbh",
-		.driver_data = MXS_DMA_APBH,
-	}, {
-		.name = "mxs-dma-apbx",
-		.driver_data = MXS_DMA_APBX,
-	}, {
-		/* end of list */
-	}
-};
-
 static struct platform_driver mxs_dma_driver = {
 	.driver		= {
 		.name	= "mxs-dma",
 	},
-	.id_table	= mxs_dma_type,
+	.id_table	= mxs_dma_idt,
 };
 
 static int __init mxs_dma_module_init(void)
include/linux/fsl/mxs-dma.h +0 -2
index 203d7c4..55d8702 100644
@@ -15,14 +15,6 @@ struct mxs_dma_data {
 	int chan_irq;
 };
 
-static inline int mxs_dma_is_apbh(struct dma_chan *chan)
-{

include/linux/fsl/mxs-dma.h struct mxs_dma_data { · line 19

Quoted 10 lines
include/linux/fsl/mxs-dma.h +2 -8
-	return !strcmp(dev_name(chan->device->dev), "mxs-dma-apbh");
-}
-
-static inline int mxs_dma_is_apbx(struct dma_chan *chan)
-{
-	return !strcmp(dev_name(chan->device->dev), "mxs-dma-apbx");
-}
-
+extern int mxs_dma_is_apbh(struct dma_chan *chan);
+extern int mxs_dma_is_apbx(struct dma_chan *chan);

I would expect these two calls still be inline here.

Quoted 9 lines
include/linux/fsl/mxs-dma.h +0 -0
 #endif /* __MACH_MXS_DMA_H__ */
Signature · 7 lines
1.7.0.4


_______________________________________________
devicetree-discuss mailing list
devicetree-discuss at lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Signature · 2 lines
Regards,
Shawn
#6 lore raw

PATCH 2/5 dma: mxs-dma: make platform_device_id more generic

Quoted 83 lines · Shawn Guo
Quoted 6 lines · Dong Aisheng

From: Dong Aisheng <[email protected]>

Rewrite mxs_dma_is_apbh and mxs_dma_is_apbx in order to support other SoCs like imx6q and reform the platform_device_id for the better further dt support.

Nice cleanup. But I would like to ask more.

Quoted 74 lines
Cc Vinod Koul <[email protected]>
Cc Dan Williams <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Huang Shijie <[email protected]>
---
 arch/arm/mach-mxs/clock-mx23.c           |    4 +-
 arch/arm/mach-mxs/clock-mx28.c           |    4 +-
 arch/arm/mach-mxs/devices/platform-dma.c |   14 ++--
 drivers/dma/mxs-dma.c                    |  111 +++++++++++++++++++++++-------
 include/linux/fsl/mxs-dma.h              |   12 +---
 5 files changed, 101 insertions(+), 44 deletions(-)
arch/arm/mach-mxs/clock-mx23.c +2 -2
index e3ac52c..1e67b85 100644
@@ -427,8 +427,8 @@ static struct clk_lookup lookups[] = {
 	_REGISTER_CLOCK("duart", NULL, uart_clk)
 	_REGISTER_CLOCK("mxs-auart.0", NULL, uart_clk)
 	_REGISTER_CLOCK("rtc", NULL, rtc_clk)
-	_REGISTER_CLOCK("mxs-dma-apbh", NULL, hbus_clk)
-	_REGISTER_CLOCK("mxs-dma-apbx", NULL, xbus_clk)
+	_REGISTER_CLOCK("imx23-dma-apbh", NULL, hbus_clk)
+	_REGISTER_CLOCK("imx23-dma-apbx", NULL, xbus_clk)
 	_REGISTER_CLOCK("mxs-mmc.0", NULL, ssp_clk)
 	_REGISTER_CLOCK("mxs-mmc.1", NULL, ssp_clk)
 	_REGISTER_CLOCK(NULL, "usb", usb_clk)
arch/arm/mach-mxs/clock-mx28.c +2 -2
index 1867a17..cca2cf7 100644
@@ -625,8 +625,8 @@ static struct clk_lookup lookups[] = {
 	_REGISTER_CLOCK("mxs-auart.4", NULL, uart_clk)
 	_REGISTER_CLOCK("rtc", NULL, rtc_clk)
 	_REGISTER_CLOCK("pll2", NULL, pll2_clk)
-	_REGISTER_CLOCK("mxs-dma-apbh", NULL, hbus_clk)
-	_REGISTER_CLOCK("mxs-dma-apbx", NULL, xbus_clk)
+	_REGISTER_CLOCK("imx28-dma-apbh", NULL, hbus_clk)
+	_REGISTER_CLOCK("imx28-dma-apbx", NULL, xbus_clk)
 	_REGISTER_CLOCK("mxs-mmc.0", NULL, ssp0_clk)
 	_REGISTER_CLOCK("mxs-mmc.1", NULL, ssp1_clk)
 	_REGISTER_CLOCK("mxs-mmc.2", NULL, ssp2_clk)
arch/arm/mach-mxs/devices/platform-dma.c +8 -6
index 6a0202b..aff4813 100644
@@ -32,17 +32,19 @@ static struct platform_device *__init mxs_add_dma(const char *devid,
 
 static int __init mxs_add_mxs_dma(void)
 {
-	char *apbh = "mxs-dma-apbh";
-	char *apbx = "mxs-dma-apbx";
+	char *mx23_apbh = "imx23-dma-apbh";
+	char *mx23_apbx = "imx23-dma-apbx";
+	char *mx28_apbh = "imx28-dma-apbh";
+	char *mx28_apbx = "imx28-dma-apbx";
 
 	if (cpu_is_mx23()) {
-		mxs_add_dma(apbh, MX23_APBH_DMA_BASE_ADDR);
-		mxs_add_dma(apbx, MX23_APBX_DMA_BASE_ADDR);
+		mxs_add_dma(mx23_apbh, MX23_APBH_DMA_BASE_ADDR);
+		mxs_add_dma(mx23_apbx, MX23_APBX_DMA_BASE_ADDR);
 	}
 
 	if (cpu_is_mx28()) {
-		mxs_add_dma(apbh, MX28_APBH_DMA_BASE_ADDR);
-		mxs_add_dma(apbx, MX28_APBX_DMA_BASE_ADDR);
+		mxs_add_dma(mx28_apbh, MX28_APBH_DMA_BASE_ADDR);
+		mxs_add_dma(mx28_apbx, MX28_APBX_DMA_BASE_ADDR);
 	}
 
Quoted 5 lines

arch/arm/mach-mxs/devices/platform-dma.c static struct platform_device *__init mxs_add_dma(const char *devid, · line 50

Quoted 1 line
arch/arm/mach-mxs/devices/platform-dma.c +0 -0
 	return 0;

With soc specific hook introduced, we can remove this mxs_add_mxs_dma call completely.

Yes, it's true.
A mx23/mx28_soc_init can do it well.

Quoted 7 lines
Quoted 7 lines
drivers/dma/mxs-dma.c +0 -0
index b0051a8..51a29f9 100644
@@ -50,7 +50,8 @@
 #define HW_APBHX_CTRL2				0x020
 #define HW_APBHX_CHANNEL_CTRL			0x030
Quoted 9 lines

drivers/dma/mxs-dma.c line 54

Quoted 4 lines
drivers/dma/mxs-dma.c +2 -1
 #define BP_APBHX_CHANNEL_CTRL_RESET_CHANNEL	16
-#define HW_APBH_VERSION				(cpu_is_mx23() ? 0x3f0 : 0x800)
+#define HW_APBH_VERSION_IMX23			0x3f0
+#define HW_APBH_VERSION_IMX28			0x800

Since the VERSION is broken for identifying the dma type and we are introduce dma type field, I would suggest to remove the use of VERSION
completely and use mxs_dma_type pbh_is_old() (mxs_dma->everywhere.

Remove HW_APBH_VERSION_*?
But i see the some other places in original code still need to use it. Like:
#define apbh_is_old() (mxs_dma->version < APBH_VERSION_LATEST)
..

#define HW_APBHX_CHn_NXTCMDAR(n) \
        (((dma_is_apbh() && apbh_is_old()) ? 0x050 : 0x110) + (n) * 0x70)
#define HW_APBHX_CHn_SEMA(n) \
        (((dma_is_apbh() && apbh_is_old()) ? 0x080 : 0x140) + (n) * 0x70)

Do you mean apbh_is_old represents the same as SoC type and can be replaced by mxs_dma_type?

Quoted 26 lines

#define HW_APBX_VERSION 0x800

Quoted 25 lines
drivers/dma/mxs-dma.c +11 -1
 #define BP_APBHX_VERSION_MAJOR			24
 #define HW_APBHX_CHn_NXTCMDAR(n) \
@@ -121,7 +122,8 @@ struct mxs_dma_chan {
 #define MXS_DMA_CHANNELS_MASK		0xffff
 
 struct mxs_dma_engine {
-	int				dev_id;
+	unsigned int			type;
+	unsigned int			dev_id;
 	unsigned int			version;
 	void __iomem			*base;
 	struct clk			*clk;
@@ -130,6 +132,74 @@ struct mxs_dma_engine {
 	struct mxs_dma_chan		mxs_chans[MXS_DMA_CHANNELS];
 };
 
+enum mxs_dma_devtype {
+	IMX23_DMA,
+	IMX28_DMA,
+};
+
+struct mxs_dma_type {
+	unsigned int type;
+	unsigned int id;
+};
Quoted 9 lines

drivers/dma/mxs-dma.c struct mxs_dma_engine { · line 148

Quoted 5 lines
drivers/dma/mxs-dma.c +5 -0
+
+static struct mxs_dma_type mxs_dma_types[] = {
+	{
+		.type = IMX23_DMA,
+		.id = MXS_DMA_APBH,

To me, IMX23_DMA is more like a id, while MXS_DMA_APBH is more like a type.

Well, i'm more like IMX23_DMA since they're different devices. Id to me is more like a same device but different id. And you see i already have enum mxs_dma_devtype.

Quoted 6 lines
Quoted 6 lines
drivers/dma/mxs-dma.c +6 -0
+	}, {
+		.type = IMX23_DMA,
+		.id = MXS_DMA_APBX,
+	}, {
+		.type = IMX28_DMA,
+		.id = MXS_DMA_APBH,
Quoted 9 lines

drivers/dma/mxs-dma.c struct mxs_dma_engine { · line 160

Quoted 6 lines
drivers/dma/mxs-dma.c +6 -0
+	}, {
+		.type = IMX28_DMA,
+		.id = MXS_DMA_APBX,
+	}, {
+		/* end of list */
+	}

This end sign is not needed, I think.

Ok to me.
btw i always had this question before,
Can you tell what's purpose of adding a '/* end of list */'?

Quoted 6 lines

drivers/dma/mxs-dma.c struct mxs_dma_engine { · line 163

Quoted 3 lines
drivers/dma/mxs-dma.c +3 -0
+};
+
+static struct platform_device_id mxs_dma_idt[] = {

s/mxs_dma_idt/mxs_dma_ids?

Ok to me.

Quoted 31 lines
Quoted 18 lines
drivers/dma/mxs-dma.c +18 -0
+	{
+		.name = "imx23-dma-apbh",
+		.driver_data = (kernel_ulong_t) &mxs_dma_types[0],
+	}, {
+		.name = "imx23-dma-apbx",
+		.driver_data = (kernel_ulong_t) &mxs_dma_types[1],
+	}, {
+		.name = "imx28-dma-apbh",
+		.driver_data = (kernel_ulong_t) &mxs_dma_types[2],
+	}, {
+		.name = "imx28-dma-apbx",
+		.driver_data = (kernel_ulong_t) &mxs_dma_types[3],
+	}, {
+		/* end of list */
+	}
+};
+
+static struct mxs_dma_chan *to_mxs_dma_chan(struct dma_chan *chan)

drivers/dma/mxs-dma.c struct mxs_dma_engine { · line 191

Quoted 10 lines
drivers/dma/mxs-dma.c +10 -0
+{
+	return container_of(chan, struct mxs_dma_chan, chan);
+}
+
+int mxs_dma_is_apbh(struct dma_chan *chan)
+{
+	struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
+	struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
+
+	return dma_is_apbh();

I would expect it simply be

Quoted 4 lines
	return (mxs_dma->dev_id == MXS_DMA_APBH);

And dma_is_apbh() can be removed completely now.

Yes, it's ture.
Basically what i did in this patch is only convert the driver to a more generic platform_device_id, seems i need to do more...

Quoted 106 lines

drivers/dma/mxs-dma.c struct mxs_dma_engine { · line 199

Quoted 8 lines
drivers/dma/mxs-dma.c +8 -0
+}
+
+int mxs_dma_is_apbx(struct dma_chan *chan)
+{
+	struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
+	struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
+
+	return !dma_is_apbh();
	return (mxs_dma->dev_id == MXS_DMA_APBX);
Quoted 27 lines
drivers/dma/mxs-dma.c +6 -7
+}
+
+
 static void mxs_dma_reset_chan(struct mxs_dma_chan *mxs_chan)
 {
 	struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
@@ -193,11 +263,6 @@ static void mxs_dma_resume_chan(struct mxs_dma_chan *mxs_chan)
 	mxs_chan->status = DMA_IN_PROGRESS;
 }
 
-static struct mxs_dma_chan *to_mxs_dma_chan(struct dma_chan *chan)
-{
-	return container_of(chan, struct mxs_dma_chan, chan);
-}
-
 static dma_cookie_t mxs_dma_tx_submit(struct dma_async_tx_descriptor *tx)
 {
 	struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(tx->chan);
@@ -574,11 +639,18 @@ static int __init mxs_dma_init(struct mxs_dma_engine *mxs_dma)
 	if (ret)
 		goto err_out;
 
-	/* only major version matters */
-	mxs_dma->version = readl(mxs_dma->base +
+	if (mxs_dma->type == IMX23_DMA) {
+		/* only major version matters */
+		mxs_dma->version = readl(mxs_dma->base +

drivers/dma/mxs-dma.c static int __init mxs_dma_init(struct mxs_dma_engine *mxs_dma) · line 653

Quoted 10 lines
drivers/dma/mxs-dma.c +7 -1
 				((mxs_dma->dev_id == MXS_DMA_APBX) ?
-				HW_APBX_VERSION : HW_APBH_VERSION)) >>
+				HW_APBX_VERSION : HW_APBH_VERSION_IMX23)) >>
 				BP_APBHX_VERSION_MAJOR;
+	} else if (mxs_dma->type == IMX28_DMA) {
+		mxs_dma->version = readl(mxs_dma->base +
+				((mxs_dma->dev_id == MXS_DMA_APBX) ?
+				HW_APBX_VERSION : HW_APBH_VERSION_IMX28)) >>
+				BP_APBHX_VERSION_MAJOR;
+	}

Again, let's just get rid of the use of VERSION completely.

Quoted 55 lines
drivers/dma/mxs-dma.c +5 -14
 	/* enable apbh burst */
 	if (dma_is_apbh()) {
@@ -601,6 +673,8 @@ static int __init mxs_dma_probe(struct platform_device *pdev)
 {
 	const struct platform_device_id *id_entry =
 				platform_get_device_id(pdev);
+	const struct mxs_dma_type *mtype =
+			(struct mxs_dma_type *)id_entry->driver_data;
 	struct mxs_dma_engine *mxs_dma;
 	struct resource *iores;
 	int ret, i;
@@ -609,7 +683,8 @@ static int __init mxs_dma_probe(struct platform_device *pdev)
 	if (!mxs_dma)
 		return -ENOMEM;
 
-	mxs_dma->dev_id = id_entry->driver_data;
+	mxs_dma->type = mtype->type;
+	mxs_dma->dev_id = mtype->id;
 
 	iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 
@@ -692,23 +767,11 @@ err_request_region:
 	return ret;
 }
 
-static struct platform_device_id mxs_dma_type[] = {
-	{
-		.name = "mxs-dma-apbh",
-		.driver_data = MXS_DMA_APBH,
-	}, {
-		.name = "mxs-dma-apbx",
-		.driver_data = MXS_DMA_APBX,
-	}, {
-		/* end of list */
-	}
-};
-
 static struct platform_driver mxs_dma_driver = {
 	.driver		= {
 		.name	= "mxs-dma",
 	},
-	.id_table	= mxs_dma_type,
+	.id_table	= mxs_dma_idt,
 };
 
 static int __init mxs_dma_module_init(void)
include/linux/fsl/mxs-dma.h +0 -0
index 203d7c4..55d8702 100644
@@ -15,14 +15,6 @@ struct mxs_dma_data {
 	int chan_irq;
 };
 
Quoted 15 lines

include/linux/fsl/mxs-dma.h struct mxs_dma_data { · line 19

Quoted 12 lines
include/linux/fsl/mxs-dma.h +2 -10
-static inline int mxs_dma_is_apbh(struct dma_chan *chan)
-{
-	return !strcmp(dev_name(chan->device->dev), "mxs-dma-apbh");
-}
-
-static inline int mxs_dma_is_apbx(struct dma_chan *chan)
-{
-	return !strcmp(dev_name(chan->device->dev), "mxs-dma-apbx");
-}
-
+extern int mxs_dma_is_apbh(struct dma_chan *chan);
+extern int mxs_dma_is_apbx(struct dma_chan *chan);

I would expect these two calls still be inline here.

Ok to me.

Regards
Dong Aisheng

#7 lore raw

PATCH 2/5 dma: mxs-dma: make platform_device_id more generic

On Mon, Apr 23, 2012 at 11:58:48AM +0800, Dong Aisheng wrote:
...

Quoted 5 lines

Remove HW_APBH_VERSION_*?
But i see the some other places in original code still need to use it. Like:
#define apbh_is_old() (mxs_dma->version < APBH_VERSION_LATEST)
..

Quoted 8 lines
#define HW_APBHX_CHn_NXTCMDAR(n) \
        (((dma_is_apbh() && apbh_is_old()) ? 0x050 : 0x110) + (n) * 0x70)
#define HW_APBHX_CHn_SEMA(n) \
        (((dma_is_apbh() && apbh_is_old()) ? 0x080 : 0x140) + (n) * 0x70)

Do you mean apbh_is_old represents the same as SoC type and can be replaced by mxs_dma_type?

Yes.

Quoted 35 lines
Quoted 26 lines

#define HW_APBX_VERSION 0x800

Quoted 25 lines

#define BP_APBHX_VERSION_MAJOR 24
#define HW_APBHX_CHn_NXTCMDAR(n) \

drivers/dma/mxs-dma.c +11 -1
@@ -121,7 +122,8 @@ struct mxs_dma_chan {
 #define MXS_DMA_CHANNELS_MASK		0xffff
 
 struct mxs_dma_engine {
-	int				dev_id;
+	unsigned int			type;
+	unsigned int			dev_id;
 	unsigned int			version;
 	void __iomem			*base;
 	struct clk			*clk;
@@ -130,6 +132,74 @@ struct mxs_dma_engine {
 	struct mxs_dma_chan		mxs_chans[MXS_DMA_CHANNELS];
 };
 
+enum mxs_dma_devtype {
+	IMX23_DMA,
+	IMX28_DMA,
+};
+
+struct mxs_dma_type {
+	unsigned int type;
+	unsigned int id;
+};
Quoted 9 lines

drivers/dma/mxs-dma.c struct mxs_dma_engine { · line 148

Quoted 5 lines
drivers/dma/mxs-dma.c +5 -0
+
+static struct mxs_dma_type mxs_dma_types[] = {
+	{
+		.type = IMX23_DMA,
+		.id = MXS_DMA_APBH,

To me, IMX23_DMA is more like a id, while MXS_DMA_APBH is more like a type.

Quoted 4 lines

Well, i'm more like IMX23_DMA since they're different devices. Id to me is more like a same device but different id. And you see i already have enum mxs_dma_devtype.

Ok, let me put more comment on this. You have mxs_dma_devtype defined, and then you should use it rather than "unsigned int" to define "type" in struct mxs_dma_type. And for better alignment, the MXS_DMA_APBH and MXS_DMA_APBX should be changed to some enum type from the exsiting macro. Then, struct mxs_dma_type should probably look like the following.

struct mxs_dma_type {
	enum mxs_dma_devtype type;
	enum mxs_dma_id id;
};

As you agreed below, we will have mxs_dma_ids for the platform_device_id table, and mxs_dma_id here for another enum.

I think we really need a better semantics for "type" and "id" in this driver.

Quoted 15 lines
Quoted 6 lines
Quoted 6 lines
drivers/dma/mxs-dma.c +6 -0
+	}, {
+		.type = IMX23_DMA,
+		.id = MXS_DMA_APBX,
+	}, {
+		.type = IMX28_DMA,
+		.id = MXS_DMA_APBH,
Quoted 9 lines

drivers/dma/mxs-dma.c struct mxs_dma_engine { · line 160

Quoted 6 lines
drivers/dma/mxs-dma.c +6 -0
+	}, {
+		.type = IMX28_DMA,
+		.id = MXS_DMA_APBX,
+	}, {
+		/* end of list */
+	}

This end sign is not needed, I think.

Quoted 4 lines

Ok to me.
btw i always had this question before,
Can you tell what's purpose of adding a '/* end of list */'?

The end sign I meant is "{ }". It's needed for platform_device_id and of_device_id table by driver core. But I do not see why we need it for mxs_dma_type table here.

Quoted 25 lines
Quoted 6 lines

drivers/dma/mxs-dma.c struct mxs_dma_engine { · line 163

Quoted 3 lines
drivers/dma/mxs-dma.c +3 -0
+};
+
+static struct platform_device_id mxs_dma_idt[] = {

s/mxs_dma_idt/mxs_dma_ids?

Ok to me.

Quoted 17 lines
Quoted 17 lines
drivers/dma/mxs-dma.c +17 -0
+	{
+		.name = "imx23-dma-apbh",
+		.driver_data = (kernel_ulong_t) &mxs_dma_types[0],
+	}, {
+		.name = "imx23-dma-apbx",
+		.driver_data = (kernel_ulong_t) &mxs_dma_types[1],
+	}, {
+		.name = "imx28-dma-apbh",
+		.driver_data = (kernel_ulong_t) &mxs_dma_types[2],
+	}, {
+		.name = "imx28-dma-apbx",
+		.driver_data = (kernel_ulong_t) &mxs_dma_types[3],
+	}, {
+		/* end of list */
+	}
+};
+
Signature · 2 lines
Regards,
Shawn
#8 lore raw

PATCH 3/5 dma: mxs-dma: add device tree probe support

In reply to Dong Aisheng

From: Dong Aisheng <[email protected]>

Cc Grant Likely <[email protected]>
Cc Rob Herring <[email protected]>
Cc Rob Landley <[email protected]>
Cc Vinod Koul <[email protected]>
Cc Dan Williams <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Huang Shijie <[email protected]>
---
 .../devicetree/bindings/dma/fsl-mxs-dma.txt        |   19 +++++++++++++
 drivers/dma/mxs-dma.c                              |   29 +++++++++++++++++---
 2 files changed, 44 insertions(+), 4 deletions(-)
Documentation/devicetree/bindings/dma/fsl-mxs-dma.txt new file +19 -0
new file mode 100644
index 0000000..ded0398
@@ -0,0 +1,19 @@
+* Freescale MXS DMA
+
+Required properties:
+- compatible : Should be "fsl,<chip>-dma-apbh" or "fsl,<chip>-dma-apbx"
+- reg : Should contain registers location and length
+
+Supported chips:
+imx23, imx28.
+
+Examples:
+dma-apbh at 80004000 {
+	compatible = "fsl,imx28-dma-apbh";
+	reg = <0x80004000 2000>;
+};
+
+dma-apbx at 80024000 {
+	compatible = "fsl,imx28-dma-apbx";
+	reg = <0x80024000 2000>;
+};
drivers/dma/mxs-dma.c +25 -4
index 51a29f9..74c3275 100644
@@ -22,8 +22,11 @@
 #include <linux/platform_device.h>
 #include <linux/dmaengine.h>
 #include <linux/delay.h>
+#include <linux/module.h>
 #include <linux/fsl/mxs-dma.h>
 #include <linux/stmp_device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 #include <asm/irq.h>
 
@@ -178,6 +181,17 @@ static struct platform_device_id mxs_dma_idt[] = {
 	}
 };
 
+#ifdef CONFIG_OF
+static const struct of_device_id mxs_dma_dt_ids[] = {
+	{ .compatible = "fsl,imx23-dma-apbh", .data = &mxs_dma_idt[0], },
+	{ .compatible = "fsl,imx23-dma-apbx", .data = &mxs_dma_idt[1], },
+	{ .compatible = "fsl,imx28-dma-apbh", .data = &mxs_dma_idt[2], },
+	{ .compatible = "fsl,imx28-dma-apbx", .data = &mxs_dma_idt[3], },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mxs_dma_dt_ids);
+#endif
+
 static struct mxs_dma_chan *to_mxs_dma_chan(struct dma_chan *chan)
 {
 	return container_of(chan, struct mxs_dma_chan, chan);
@@ -671,10 +685,9 @@ err_out:
 
 static int __init mxs_dma_probe(struct platform_device *pdev)
 {
-	const struct platform_device_id *id_entry =
-				platform_get_device_id(pdev);
-	const struct mxs_dma_type *mtype =
-			(struct mxs_dma_type *)id_entry->driver_data;
+	const struct platform_device_id *id_entry;
+	const struct of_device_id *of_id;
+	const struct mxs_dma_type *mtype;
 	struct mxs_dma_engine *mxs_dma;
 	struct resource *iores;
 	int ret, i;
@@ -683,6 +696,13 @@ static int __init mxs_dma_probe(struct platform_device *pdev)
 	if (!mxs_dma)
 		return -ENOMEM;
 
+	of_id = of_match_device(mxs_dma_dt_ids, &pdev->dev);
+	if (of_id)
+		id_entry = of_id->data;
+	else
+		id_entry = platform_get_device_id(pdev);
+
+	mtype = (struct mxs_dma_type *)id_entry->driver_data;
 	mxs_dma->type = mtype->type;
 	mxs_dma->dev_id = mtype->id;
 
@@ -770,6 +790,7 @@ err_request_region:
 static struct platform_driver mxs_dma_driver = {
 	.driver		= {
 		.name	= "mxs-dma",
+		.of_match_table = of_match_ptr(mxs_dma_dt_ids),
 	},
 	.id_table	= mxs_dma_idt,
 };
Signature · 1 line
1.7.0.4
#9 lore raw

PATCH 3/5 dma: mxs-dma: add device tree probe support

Dear Dong Aisheng,

Quoted 12 lines

From: Dong Aisheng <[email protected]>

Cc Grant Likely <[email protected]>
Cc Rob Herring <[email protected]>
Cc Rob Landley <[email protected]>
Cc Vinod Koul <[email protected]>
Cc Dan Williams <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Huang Shijie <[email protected]>
Quoted 94 lines
---
 .../devicetree/bindings/dma/fsl-mxs-dma.txt        |   19 +++++++++++++
 drivers/dma/mxs-dma.c                              |   29

+++++++++++++++++--- 2 files changed, 44 insertions(+), 4 deletions(-)

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/dma/fsl-mxs-dma.txt new file mode 100644
index 0000000..ded0398

Documentation/devicetree/bindings/dma/fsl-mxs-dma.txt new file +19 -0
@@ -0,0 +1,19 @@
+* Freescale MXS DMA
+
+Required properties:
+- compatible : Should be "fsl,<chip>-dma-apbh" or "fsl,<chip>-dma-apbx"
+- reg : Should contain registers location and length
+
+Supported chips:
+imx23, imx28.
+
+Examples:
+dma-apbh at 80004000 {
+	compatible = "fsl,imx28-dma-apbh";
+	reg = <0x80004000 2000>;
+};
+
+dma-apbx at 80024000 {
+	compatible = "fsl,imx28-dma-apbx";
+	reg = <0x80024000 2000>;
+};
drivers/dma/mxs-dma.c +24 -4
index 51a29f9..74c3275 100644
@@ -22,8 +22,11 @@
 #include <linux/platform_device.h>
 #include <linux/dmaengine.h>
 #include <linux/delay.h>
+#include <linux/module.h>
 #include <linux/fsl/mxs-dma.h>
 #include <linux/stmp_device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 #include <asm/irq.h>
 
@@ -178,6 +181,17 @@ static struct platform_device_id mxs_dma_idt[] = {
 	}
 };
 
+#ifdef CONFIG_OF
+static const struct of_device_id mxs_dma_dt_ids[] = {
+	{ .compatible = "fsl,imx23-dma-apbh", .data = &mxs_dma_idt[0], },
+	{ .compatible = "fsl,imx23-dma-apbx", .data = &mxs_dma_idt[1], },
+	{ .compatible = "fsl,imx28-dma-apbh", .data = &mxs_dma_idt[2], },
+	{ .compatible = "fsl,imx28-dma-apbx", .data = &mxs_dma_idt[3], },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mxs_dma_dt_ids);
+#endif
+
 static struct mxs_dma_chan *to_mxs_dma_chan(struct dma_chan *chan)
 {
 	return container_of(chan, struct mxs_dma_chan, chan);
@@ -671,10 +685,9 @@ err_out:
 
 static int __init mxs_dma_probe(struct platform_device *pdev)
 {
-	const struct platform_device_id *id_entry =
-				platform_get_device_id(pdev);
-	const struct mxs_dma_type *mtype =
-			(struct mxs_dma_type *)id_entry->driver_data;
+	const struct platform_device_id *id_entry;
+	const struct of_device_id *of_id;
+	const struct mxs_dma_type *mtype;
 	struct mxs_dma_engine *mxs_dma;
 	struct resource *iores;
 	int ret, i;
@@ -683,6 +696,13 @@ static int __init mxs_dma_probe(struct platform_device
 *pdev) if (!mxs_dma)
 		return -ENOMEM;
 
+	of_id = of_match_device(mxs_dma_dt_ids, &pdev->dev);
+	if (of_id)
+		id_entry = of_id->data;
+	else
+		id_entry = platform_get_device_id(pdev);
+
+	mtype = (struct mxs_dma_type *)id_entry->driver_data;
 	mxs_dma->type = mtype->type;
 	mxs_dma->dev_id = mtype->id;
 

drivers/dma/mxs-dma.c err_request_region: · line 796

Quoted 7 lines
drivers/dma/mxs-dma.c +1 -0
@@ -770,6 +790,7 @@ err_request_region:
 static struct platform_driver mxs_dma_driver = {
 	.driver		= {
 		.name	= "mxs-dma",
+		.of_match_table = of_match_ptr(mxs_dma_dt_ids),
 	},
 	.id_table	= mxs_dma_idt,
 };

Best regards,
Marek Vasut

#10 lore raw

PATCH 3/5 dma: mxs-dma: add device tree probe support

In reply to Dong Aisheng
Quoted 63 lines · Dong Aisheng

From: Dong Aisheng <[email protected]>

Cc Grant Likely <[email protected]>
Cc Rob Herring <[email protected]>
Cc Rob Landley <[email protected]>
Cc Vinod Koul <[email protected]>
Cc Dan Williams <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Huang Shijie <[email protected]>
---
 .../devicetree/bindings/dma/fsl-mxs-dma.txt        |   19 +++++++++++++
 drivers/dma/mxs-dma.c                              |   29 +++++++++++++++++---
 2 files changed, 44 insertions(+), 4 deletions(-)
Documentation/devicetree/bindings/dma/fsl-mxs-dma.txt new file +19 -0
new file mode 100644
index 0000000..ded0398
@@ -0,0 +1,19 @@
+* Freescale MXS DMA
+
+Required properties:
+- compatible : Should be "fsl,<chip>-dma-apbh" or "fsl,<chip>-dma-apbx"
+- reg : Should contain registers location and length
+
+Supported chips:
+imx23, imx28.
+
+Examples:
+dma-apbh at 80004000 {
+	compatible = "fsl,imx28-dma-apbh";
+	reg = <0x80004000 2000>;
+};
+
+dma-apbx at 80024000 {
+	compatible = "fsl,imx28-dma-apbx";
+	reg = <0x80024000 2000>;
+};
drivers/dma/mxs-dma.c +4 -0
index 51a29f9..74c3275 100644
@@ -22,8 +22,11 @@
 #include <linux/platform_device.h>
 #include <linux/dmaengine.h>
 #include <linux/delay.h>
+#include <linux/module.h>
 #include <linux/fsl/mxs-dma.h>
 #include <linux/stmp_device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 #include <asm/irq.h>
 
@@ -178,6 +181,17 @@ static struct platform_device_id mxs_dma_idt[] = {
 	}
 };
 
+#ifdef CONFIG_OF

drivers/dma/mxs-dma.c static struct platform_device_id mxs_dma_idt[] = { · line 194

Quoted 10 lines
drivers/dma/mxs-dma.c +10 -0
+static const struct of_device_id mxs_dma_dt_ids[] = {
+	{ .compatible = "fsl,imx23-dma-apbh", .data = &mxs_dma_idt[0], },
+	{ .compatible = "fsl,imx23-dma-apbx", .data = &mxs_dma_idt[1], },
+	{ .compatible = "fsl,imx28-dma-apbh", .data = &mxs_dma_idt[2], },
+	{ .compatible = "fsl,imx28-dma-apbx", .data = &mxs_dma_idt[3], },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mxs_dma_dt_ids);
+#endif
+

As out goal is to convert mxs over to DT, you can remove the #ifdef here ...

Quoted 32 lines
drivers/dma/mxs-dma.c +10 -4
 static struct mxs_dma_chan *to_mxs_dma_chan(struct dma_chan *chan)
 {
 	return container_of(chan, struct mxs_dma_chan, chan);
@@ -671,10 +685,9 @@ err_out:
 
 static int __init mxs_dma_probe(struct platform_device *pdev)
 {
-	const struct platform_device_id *id_entry =
-				platform_get_device_id(pdev);
-	const struct mxs_dma_type *mtype =
-			(struct mxs_dma_type *)id_entry->driver_data;
+	const struct platform_device_id *id_entry;
+	const struct of_device_id *of_id;
+	const struct mxs_dma_type *mtype;
 	struct mxs_dma_engine *mxs_dma;
 	struct resource *iores;
 	int ret, i;
@@ -683,6 +696,13 @@ static int __init mxs_dma_probe(struct platform_device *pdev)
 	if (!mxs_dma)
 		return -ENOMEM;
 
+	of_id = of_match_device(mxs_dma_dt_ids, &pdev->dev);
+	if (of_id)
+		id_entry = of_id->data;
+	else
+		id_entry = platform_get_device_id(pdev);
+
+	mtype = (struct mxs_dma_type *)id_entry->driver_data;
 	mxs_dma->type = mtype->type;
 	mxs_dma->dev_id = mtype->id;
 

drivers/dma/mxs-dma.c err_request_region: · line 793

Quoted 4 lines
drivers/dma/mxs-dma.c +1 -0
@@ -770,6 +790,7 @@ err_request_region:
 static struct platform_driver mxs_dma_driver = {
 	.driver		= {
 		.name	= "mxs-dma",
+		.of_match_table = of_match_ptr(mxs_dma_dt_ids),

... then of_match_ptr is not needed here.

Quoted 11 lines
drivers/dma/mxs-dma.c +0 -0
 	},
 	.id_table	= mxs_dma_idt,
 };
Signature · 7 lines
1.7.0.4


_______________________________________________
devicetree-discuss mailing list
devicetree-discuss at lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Signature · 2 lines
Regards,
Shawn
#11 lore raw

PATCH 3/5 dma: mxs-dma: add device tree probe support

Quoted 63 lines · Shawn Guo
Quoted 62 lines · Dong Aisheng

From: Dong Aisheng <[email protected]>

Cc Grant Likely <[email protected]>
Cc Rob Herring <[email protected]>
Cc Rob Landley <[email protected]>
Cc Vinod Koul <[email protected]>
Cc Dan Williams <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Huang Shijie <[email protected]>
---
 .../devicetree/bindings/dma/fsl-mxs-dma.txt        |   19 +++++++++++++
 drivers/dma/mxs-dma.c                              |   29 +++++++++++++++++---
 2 files changed, 44 insertions(+), 4 deletions(-)
Documentation/devicetree/bindings/dma/fsl-mxs-dma.txt new file +19 -0
new file mode 100644
index 0000000..ded0398
@@ -0,0 +1,19 @@
+* Freescale MXS DMA
+
+Required properties:
+- compatible : Should be "fsl,<chip>-dma-apbh" or "fsl,<chip>-dma-apbx"
+- reg : Should contain registers location and length
+
+Supported chips:
+imx23, imx28.
+
+Examples:
+dma-apbh at 80004000 {
+	compatible = "fsl,imx28-dma-apbh";
+	reg = <0x80004000 2000>;
+};
+
+dma-apbx at 80024000 {
+	compatible = "fsl,imx28-dma-apbx";
+	reg = <0x80024000 2000>;
+};
drivers/dma/mxs-dma.c +3 -0
index 51a29f9..74c3275 100644
@@ -22,8 +22,11 @@
 #include <linux/platform_device.h>
 #include <linux/dmaengine.h>
 #include <linux/delay.h>
+#include <linux/module.h>
 #include <linux/fsl/mxs-dma.h>
 #include <linux/stmp_device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 #include <asm/irq.h>
 
@@ -178,6 +181,17 @@ static struct platform_device_id mxs_dma_idt[] = {
 	}
 };
 
Quoted 14 lines

drivers/dma/mxs-dma.c static struct platform_device_id mxs_dma_idt[] = { · line 194

Quoted 11 lines
drivers/dma/mxs-dma.c +11 -0
+#ifdef CONFIG_OF
+static const struct of_device_id mxs_dma_dt_ids[] = {
+	{ .compatible = "fsl,imx23-dma-apbh", .data = &mxs_dma_idt[0], },
+	{ .compatible = "fsl,imx23-dma-apbx", .data = &mxs_dma_idt[1], },
+	{ .compatible = "fsl,imx28-dma-apbh", .data = &mxs_dma_idt[2], },
+	{ .compatible = "fsl,imx28-dma-apbx", .data = &mxs_dma_idt[3], },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mxs_dma_dt_ids);
+#endif
+

As out goal is to convert mxs over to DT, you can remove the #ifdef here ...

But the non-dt is still exist, right?
So i think we can keep it now unless we totally removed the non-dt code.

Regards
Dong Aisheng

#12 lore raw

PATCH 3/5 dma: mxs-dma: add device tree probe support

Quoted 14 lines · Dong Aisheng
Quoted 5 lines
Quoted 5 lines
drivers/dma/mxs-dma.c +5 -0
+#ifdef CONFIG_OF
+static const struct of_device_id mxs_dma_dt_ids[] = {
+	{ .compatible = "fsl,imx23-dma-apbh", .data = &mxs_dma_idt[0], },
+	{ .compatible = "fsl,imx23-dma-apbx", .data = &mxs_dma_idt[1], },
+	{ .compatible = "fsl,imx28-dma-apbh", .data = &mxs_dma_idt[2], },
Quoted 9 lines

drivers/dma/mxs-dma.c static struct platform_device_id mxs_dma_idt[] = { · line 194

Quoted 6 lines
drivers/dma/mxs-dma.c +6 -0
+	{ .compatible = "fsl,imx28-dma-apbx", .data = &mxs_dma_idt[3], },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mxs_dma_dt_ids);
+#endif
+

As out goal is to convert mxs over to DT, you can remove the #ifdef here ...

Quoted 3 lines

But the non-dt is still exist, right?
So i think we can keep it now unless we totally removed the non-dt code.

The non-dt still exist during the transition. But even without the #ifdef, the driver works for non-dt. It just saves a few bytes for non-dt only build. We will soon enable DT build for mxs permanently. What's the point to have it at all?

Signature · 2 lines
Regards,
Shawn
#13 lore raw

PATCH 3/5 dma: mxs-dma: add device tree probe support

Quoted 15 lines · Shawn Guo
Quoted 14 lines · Dong Aisheng
Quoted 14 lines

drivers/dma/mxs-dma.c static struct platform_device_id mxs_dma_idt[] = { · line 194

Quoted 11 lines
drivers/dma/mxs-dma.c +11 -0
+#ifdef CONFIG_OF
+static const struct of_device_id mxs_dma_dt_ids[] = {
+	{ .compatible = "fsl,imx23-dma-apbh", .data = &mxs_dma_idt[0], },
+	{ .compatible = "fsl,imx23-dma-apbx", .data = &mxs_dma_idt[1], },
+	{ .compatible = "fsl,imx28-dma-apbh", .data = &mxs_dma_idt[2], },
+	{ .compatible = "fsl,imx28-dma-apbx", .data = &mxs_dma_idt[3], },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mxs_dma_dt_ids);
+#endif
+

As out goal is to convert mxs over to DT, you can remove the #ifdef here ...

Quoted 8 lines
Quoted 3 lines

But the non-dt is still exist, right?
So i think we can keep it now unless we totally removed the non-dt code.

The non-dt still exist during the transition. But even without the #ifdef, the driver works for non-dt. It just saves a few bytes for non-dt only build. We will soon enable DT build for mxs permanently. What's the point to have it at all?

Hmm, i think the point is non-dt does not need it, not relate to whether user decide to enable dt or not.
Since you insist that, i can remove it first in my v2 patch and see people's comment.

Regards
Dong Aisheng

#14 lore raw

PATCH 4/5 ARM: mxs: do not add dma device by default

In reply to Dong Aisheng

From: Dong Aisheng <[email protected]>

This will cause conflict when dt is enabled. So let each platform add dma devices respectively.

Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Huang Shijie <[email protected]>
---
 arch/arm/mach-mxs/devices-mx23.h                |    2 ++
 arch/arm/mach-mxs/devices-mx28.h                |    2 ++
 arch/arm/mach-mxs/devices/platform-dma.c        |    3 +--
 arch/arm/mach-mxs/include/mach/devices-common.h |    3 +++
 arch/arm/mach-mxs/mach-apx4devkit.c             |    1 +
 arch/arm/mach-mxs/mach-m28evk.c                 |    1 +
 arch/arm/mach-mxs/mach-mx23evk.c                |    1 +
 arch/arm/mach-mxs/mach-stmp378x_devb.c          |    1 +
 arch/arm/mach-mxs/mach-tx28.c                   |    1 +
 9 files changed, 13 insertions(+), 2 deletions(-)
arch/arm/mach-mxs/devices-mx23.h +2 -0
index 4d1329d..5fc6b50 100644
@@ -35,3 +35,5 @@ struct platform_device *__init mx23_add_mxsfb(
 		const struct mxsfb_platform_data *pdata);
 
 struct platform_device *__init mx23_add_rtc_stmp3xxx(void);
+
+#define mx23_add_dma()	mxs_add_mxs_dma()
arch/arm/mach-mxs/devices-mx28.h +2 -0
index 9dbeae1..0b3a62d 100644
@@ -55,3 +55,5 @@ extern const struct mxs_saif_data mx28_saif_data[] __initconst;
 	mxs_add_saif(&mx28_saif_data[id], pdata)
 
 struct platform_device *__init mx28_add_rtc_stmp3xxx(void);
+
+#define mx28_add_dma()	mxs_add_mxs_dma()
arch/arm/mach-mxs/devices/platform-dma.c +1 -2
index aff4813..d0126ea 100644
@@ -30,7 +30,7 @@ static struct platform_device *__init mxs_add_dma(const char *devid,
 				DMA_BIT_MASK(32));
 }
 
-static int __init mxs_add_mxs_dma(void)
+int __init mxs_add_mxs_dma(void)
 {
 	char *mx23_apbh = "imx23-dma-apbh";
 	char *mx23_apbx = "imx23-dma-apbx";
@@ -49,4 +49,3 @@ static int __init mxs_add_mxs_dma(void)
 
 	return 0;
 }
-arch_initcall(mxs_add_mxs_dma);
arch/arm/mach-mxs/include/mach/devices-common.h +3 -0
index f2e3839..544b38d 100644
@@ -116,3 +116,6 @@ struct mxs_saif_data {
 struct platform_device *__init mxs_add_saif(
 		const struct mxs_saif_data *data,
 		const struct mxs_saif_platform_data *pdata);
+
+/* dma */
+int __init mxs_add_mxs_dma(void);
arch/arm/mach-mxs/mach-apx4devkit.c +1 -0
index 48a7fab..92f47e9 100644
@@ -210,6 +210,7 @@ static void __init apx4devkit_init(void)
 	mxs_iomux_setup_multiple_pads(apx4devkit_pads,
 			ARRAY_SIZE(apx4devkit_pads));
 
+	mx28_add_dma();
 	mx28_add_duart();
 	mx28_add_auart0();
 	mx28_add_auart1();
arch/arm/mach-mxs/mach-m28evk.c +1 -0
index 06d7996..9a74ce6 100644
@@ -321,6 +321,7 @@ static void __init m28evk_init(void)
 {
 	mxs_iomux_setup_multiple_pads(m28evk_pads, ARRAY_SIZE(m28evk_pads));
 
+	mx28_add_dma();
 	mx28_add_duart();
 	mx28_add_auart0();
 	mx28_add_auart3();
arch/arm/mach-mxs/mach-mx23evk.c +1 -0
index 5ea1c57..cabc884 100644
@@ -143,6 +143,7 @@ static void __init mx23evk_init(void)
 
 	mxs_iomux_setup_multiple_pads(mx23evk_pads, ARRAY_SIZE(mx23evk_pads));
 
+	mx23_add_dma();
 	mx23_add_duart();
 	mx23_add_auart0();
 
arch/arm/mach-mxs/mach-stmp378x_devb.c +1 -0
index a626c07..a391b9e 100644
@@ -88,6 +88,7 @@ static void __init stmp378x_dvb_init(void)
 	mxs_iomux_setup_multiple_pads(stmp378x_dvb_pads,
 			ARRAY_SIZE(stmp378x_dvb_pads));
 
+	mx23_add_dma();
 	mx23_add_duart();
 	mx23_add_auart0();
 	mx23_add_rtc_stmp3xxx();
arch/arm/mach-mxs/mach-tx28.c +1 -0
index 2c0862e..e17e262 100644
@@ -149,6 +149,7 @@ static void __init tx28_stk5v3_init(void)
 	mxs_iomux_setup_multiple_pads(tx28_stk5v3_pads,
 			ARRAY_SIZE(tx28_stk5v3_pads));
 
+	mx28_add_dma();
 	mx28_add_duart(); /* UART1 */
 	mx28_add_auart(1); /* UART2 */
 
Signature · 1 line
1.7.0.4
#15 lore raw

PATCH 4/5 ARM: mxs: do not add dma device by default

Dear Dong Aisheng,

Quoted 10 lines

From: Dong Aisheng <[email protected]>

This will cause conflict when dt is enabled. So let each platform add dma devices respectively.

Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Huang Shijie <[email protected]>
Quoted 115 lines
---
 arch/arm/mach-mxs/devices-mx23.h                |    2 ++
 arch/arm/mach-mxs/devices-mx28.h                |    2 ++
 arch/arm/mach-mxs/devices/platform-dma.c        |    3 +--
 arch/arm/mach-mxs/include/mach/devices-common.h |    3 +++
 arch/arm/mach-mxs/mach-apx4devkit.c             |    1 +
 arch/arm/mach-mxs/mach-m28evk.c                 |    1 +
 arch/arm/mach-mxs/mach-mx23evk.c                |    1 +
 arch/arm/mach-mxs/mach-stmp378x_devb.c          |    1 +
 arch/arm/mach-mxs/mach-tx28.c                   |    1 +
 9 files changed, 13 insertions(+), 2 deletions(-)
(quoted diff) +0 -0

b/arch/arm/mach-mxs/devices-mx23.h index 4d1329d..5fc6b50 100644

arch/arm/mach-mxs/devices-mx23.h +2 -0
@@ -35,3 +35,5 @@ struct platform_device *__init mx23_add_mxsfb(
 		const struct mxsfb_platform_data *pdata);
 
 struct platform_device *__init mx23_add_rtc_stmp3xxx(void);
+
+#define mx23_add_dma()	mxs_add_mxs_dma()
(quoted diff) +0 -0

b/arch/arm/mach-mxs/devices-mx28.h index 9dbeae1..0b3a62d 100644

arch/arm/mach-mxs/devices-mx28.h +2 -0
@@ -55,3 +55,5 @@ extern const struct mxs_saif_data mx28_saif_data[]
 __initconst; mxs_add_saif(&mx28_saif_data[id], pdata)
 
 struct platform_device *__init mx28_add_rtc_stmp3xxx(void);
+
+#define mx28_add_dma()	mxs_add_mxs_dma()
(quoted diff) +0 -0

b/arch/arm/mach-mxs/devices/platform-dma.c index aff4813..d0126ea 100644

arch/arm/mach-mxs/devices/platform-dma.c +1 -2
@@ -30,7 +30,7 @@ static struct platform_device *__init mxs_add_dma(const
 char *devid, DMA_BIT_MASK(32));
 }
 
-static int __init mxs_add_mxs_dma(void)
+int __init mxs_add_mxs_dma(void)
 {
 	char *mx23_apbh = "imx23-dma-apbh";
 	char *mx23_apbx = "imx23-dma-apbx";
@@ -49,4 +49,3 @@ static int __init mxs_add_mxs_dma(void)
 
 	return 0;
 }
-arch_initcall(mxs_add_mxs_dma);
(quoted diff) +0 -0

b/arch/arm/mach-mxs/include/mach/devices-common.h index f2e3839..544b38d 100644

arch/arm/mach-mxs/include/mach/devices-common.h +3 -0
@@ -116,3 +116,6 @@ struct mxs_saif_data {
 struct platform_device *__init mxs_add_saif(
 		const struct mxs_saif_data *data,
 		const struct mxs_saif_platform_data *pdata);
+
+/* dma */
+int __init mxs_add_mxs_dma(void);
(quoted diff) +0 -0

b/arch/arm/mach-mxs/mach-apx4devkit.c index 48a7fab..92f47e9 100644

arch/arm/mach-mxs/mach-apx4devkit.c +1 -0
@@ -210,6 +210,7 @@ static void __init apx4devkit_init(void)
 	mxs_iomux_setup_multiple_pads(apx4devkit_pads,
 			ARRAY_SIZE(apx4devkit_pads));
 
+	mx28_add_dma();
 	mx28_add_duart();
 	mx28_add_auart0();
 	mx28_add_auart1();
(quoted diff) +0 -0

b/arch/arm/mach-mxs/mach-m28evk.c index 06d7996..9a74ce6 100644

arch/arm/mach-mxs/mach-m28evk.c +1 -0
@@ -321,6 +321,7 @@ static void __init m28evk_init(void)
 {
 	mxs_iomux_setup_multiple_pads(m28evk_pads, ARRAY_SIZE(m28evk_pads));
 
+	mx28_add_dma();
 	mx28_add_duart();
 	mx28_add_auart0();
 	mx28_add_auart3();
(quoted diff) +0 -0

b/arch/arm/mach-mxs/mach-mx23evk.c index 5ea1c57..cabc884 100644

arch/arm/mach-mxs/mach-mx23evk.c +1 -0
@@ -143,6 +143,7 @@ static void __init mx23evk_init(void)
 
 	mxs_iomux_setup_multiple_pads(mx23evk_pads, ARRAY_SIZE(mx23evk_pads));
 
+	mx23_add_dma();
 	mx23_add_duart();
 	mx23_add_auart0();
 
(quoted diff) +0 -0

b/arch/arm/mach-mxs/mach-stmp378x_devb.c index a626c07..a391b9e 100644

arch/arm/mach-mxs/mach-stmp378x_devb.c +1 -0
@@ -88,6 +88,7 @@ static void __init stmp378x_dvb_init(void)
 	mxs_iomux_setup_multiple_pads(stmp378x_dvb_pads,
 			ARRAY_SIZE(stmp378x_dvb_pads));
 
+	mx23_add_dma();
 	mx23_add_duart();
 	mx23_add_auart0();
 	mx23_add_rtc_stmp3xxx();
arch/arm/mach-mxs/mach-tx28.c +0 -0
index 2c0862e..e17e262 100644

arch/arm/mach-mxs/mach-tx28.c static void __init tx28_stk5v3_init(void) · line 154

Quoted 6 lines
arch/arm/mach-mxs/mach-tx28.c +1 -0
@@ -149,6 +149,7 @@ static void __init tx28_stk5v3_init(void)
 	mxs_iomux_setup_multiple_pads(tx28_stk5v3_pads,
 			ARRAY_SIZE(tx28_stk5v3_pads));
 
+	mx28_add_dma();
 	mx28_add_duart(); /* UART1 */
 	mx28_add_auart(1); /* UART2 */

Best regards,
Marek Vasut

#16 lore raw

PATCH v2 4/5 ARM: mxs: do not add dma device by default

From: Dong Aisheng <[email protected]>

This will cause conflict when dt is enabled. So let each platform add dma devices respectively.

---
ChangeLog v1->v2:
* Add the missed mx28_add_dma() for mx28evk.c.
Thanks for Shijie's reminder.

---
 arch/arm/mach-mxs/devices-mx23.h                |    2 ++
 arch/arm/mach-mxs/devices-mx28.h                |    2 ++
 arch/arm/mach-mxs/devices/platform-dma.c        |    3 +--
 arch/arm/mach-mxs/include/mach/devices-common.h |    3 +++
 arch/arm/mach-mxs/mach-apx4devkit.c             |    1 +
 arch/arm/mach-mxs/mach-m28evk.c                 |    1 +
 arch/arm/mach-mxs/mach-mx23evk.c                |    1 +
 arch/arm/mach-mxs/mach-mx28evk.c                |    1 +
 arch/arm/mach-mxs/mach-stmp378x_devb.c          |    1 +
 arch/arm/mach-mxs/mach-tx28.c                   |    1 +
 10 files changed, 14 insertions(+), 2 deletions(-)
arch/arm/mach-mxs/devices-mx23.h +2 -0
index 4d1329d..5fc6b50 100644
@@ -35,3 +35,5 @@ struct platform_device *__init mx23_add_mxsfb(
 		const struct mxsfb_platform_data *pdata);
 
 struct platform_device *__init mx23_add_rtc_stmp3xxx(void);
+
+#define mx23_add_dma()	mxs_add_mxs_dma()
arch/arm/mach-mxs/devices-mx28.h +2 -0
index 9dbeae1..0b3a62d 100644
@@ -55,3 +55,5 @@ extern const struct mxs_saif_data mx28_saif_data[] __initconst;
 	mxs_add_saif(&mx28_saif_data[id], pdata)
 
 struct platform_device *__init mx28_add_rtc_stmp3xxx(void);
+
+#define mx28_add_dma()	mxs_add_mxs_dma()
arch/arm/mach-mxs/devices/platform-dma.c +1 -2
index aff4813..d0126ea 100644
@@ -30,7 +30,7 @@ static struct platform_device *__init mxs_add_dma(const char *devid,
 				DMA_BIT_MASK(32));
 }
 
-static int __init mxs_add_mxs_dma(void)
+int __init mxs_add_mxs_dma(void)
 {
 	char *mx23_apbh = "imx23-dma-apbh";
 	char *mx23_apbx = "imx23-dma-apbx";
@@ -49,4 +49,3 @@ static int __init mxs_add_mxs_dma(void)
 
 	return 0;
 }
-arch_initcall(mxs_add_mxs_dma);
arch/arm/mach-mxs/include/mach/devices-common.h +3 -0
index f2e3839..544b38d 100644
@@ -116,3 +116,6 @@ struct mxs_saif_data {
 struct platform_device *__init mxs_add_saif(
 		const struct mxs_saif_data *data,
 		const struct mxs_saif_platform_data *pdata);
+
+/* dma */
+int __init mxs_add_mxs_dma(void);
arch/arm/mach-mxs/mach-apx4devkit.c +1 -0
index 48a7fab..92f47e9 100644
@@ -210,6 +210,7 @@ static void __init apx4devkit_init(void)
 	mxs_iomux_setup_multiple_pads(apx4devkit_pads,
 			ARRAY_SIZE(apx4devkit_pads));
 
+	mx28_add_dma();
 	mx28_add_duart();
 	mx28_add_auart0();
 	mx28_add_auart1();
arch/arm/mach-mxs/mach-m28evk.c +1 -0
index 06d7996..9a74ce6 100644
@@ -321,6 +321,7 @@ static void __init m28evk_init(void)
 {
 	mxs_iomux_setup_multiple_pads(m28evk_pads, ARRAY_SIZE(m28evk_pads));
 
+	mx28_add_dma();
 	mx28_add_duart();
 	mx28_add_auart0();
 	mx28_add_auart3();
arch/arm/mach-mxs/mach-mx23evk.c +1 -0
index 5ea1c57..cabc884 100644
@@ -143,6 +143,7 @@ static void __init mx23evk_init(void)
 
 	mxs_iomux_setup_multiple_pads(mx23evk_pads, ARRAY_SIZE(mx23evk_pads));
 
+	mx23_add_dma();
 	mx23_add_duart();
 	mx23_add_auart0();
 
arch/arm/mach-mxs/mach-mx28evk.c +1 -0
index e386c14..4b645eb 100644
@@ -415,6 +415,7 @@ static void __init mx28evk_init(void)
 
 	mxs_iomux_setup_multiple_pads(mx28evk_pads, ARRAY_SIZE(mx28evk_pads));
 
+	mx28_add_dma();
 	mx28_add_duart();
 	mx28_add_auart0();
 	mx28_add_auart3();
arch/arm/mach-mxs/mach-stmp378x_devb.c +1 -0
index a626c07..a391b9e 100644
@@ -88,6 +88,7 @@ static void __init stmp378x_dvb_init(void)
 	mxs_iomux_setup_multiple_pads(stmp378x_dvb_pads,
 			ARRAY_SIZE(stmp378x_dvb_pads));
 
+	mx23_add_dma();
 	mx23_add_duart();
 	mx23_add_auart0();
 	mx23_add_rtc_stmp3xxx();
arch/arm/mach-mxs/mach-tx28.c +1 -0
index 2c0862e..e17e262 100644
@@ -149,6 +149,7 @@ static void __init tx28_stk5v3_init(void)
 	mxs_iomux_setup_multiple_pads(tx28_stk5v3_pads,
 			ARRAY_SIZE(tx28_stk5v3_pads));
 
+	mx28_add_dma();
 	mx28_add_duart(); /* UART1 */
 	mx28_add_auart(1); /* UART2 */
 
Signature · 1 line
1.7.0.4
#17 lore raw

PATCH 4/5 ARM: mxs: do not add dma device by default

In reply to Dong Aisheng
Quoted 10 lines · Dong Aisheng

From: Dong Aisheng <[email protected]>

This will cause conflict when dt is enabled. So let each platform add dma devices respectively.

Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Huang Shijie <[email protected]>
Quoted 12 lines
---
 arch/arm/mach-mxs/devices-mx23.h                |    2 ++
 arch/arm/mach-mxs/devices-mx28.h                |    2 ++
 arch/arm/mach-mxs/devices/platform-dma.c        |    3 +--
 arch/arm/mach-mxs/include/mach/devices-common.h |    3 +++
 arch/arm/mach-mxs/mach-apx4devkit.c             |    1 +
 arch/arm/mach-mxs/mach-m28evk.c                 |    1 +
 arch/arm/mach-mxs/mach-mx23evk.c                |    1 +
 arch/arm/mach-mxs/mach-stmp378x_devb.c          |    1 +
 arch/arm/mach-mxs/mach-tx28.c                   |    1 +
 9 files changed, 13 insertions(+), 2 deletions(-)

I think instead of asking every board file to add dma device explicitly, we probably need to do something similar to imx, having functions imx23_soc_init and imx28_soc_init which will only be called for non-dt boot, and adding devices like dma, gpio there.

Regards,
Shawn

#18 lore raw

PATCH 4/5 ARM: mxs: do not add dma device by default

Quoted 23 lines · Shawn Guo
Quoted 10 lines · Dong Aisheng

From: Dong Aisheng <[email protected]>

This will cause conflict when dt is enabled. So let each platform add dma devices respectively.

Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Huang Shijie <[email protected]>
Quoted 12 lines
---
 arch/arm/mach-mxs/devices-mx23.h                |    2 ++
 arch/arm/mach-mxs/devices-mx28.h                |    2 ++
 arch/arm/mach-mxs/devices/platform-dma.c        |    3 +--
 arch/arm/mach-mxs/include/mach/devices-common.h |    3 +++
 arch/arm/mach-mxs/mach-apx4devkit.c             |    1 +
 arch/arm/mach-mxs/mach-m28evk.c                 |    1 +
 arch/arm/mach-mxs/mach-mx23evk.c                |    1 +
 arch/arm/mach-mxs/mach-stmp378x_devb.c          |    1 +
 arch/arm/mach-mxs/mach-tx28.c                   |    1 +
 9 files changed, 13 insertions(+), 2 deletions(-)
Quoted 5 lines

I think instead of asking every board file to add dma device explicitly, we probably need to do something similar to imx, having functions imx23_soc_init and imx28_soc_init which will only be called for non-dt boot, and adding devices like dma, gpio there.

Yes, it's fine to me since gpio has the same issue. Will revise the patch.

Regards
Dong Aisheng

#19 lore raw

PATCH 5/5 ARM: mx28evk: add mxs-dma dt support

In reply to Dong Aisheng

From: Dong Aisheng <[email protected]>

Cc Grant Likely <[email protected]>
Cc Rob Herring <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Huang Shijie <[email protected]>
---
 arch/arm/boot/dts/imx28.dtsi   |    4 ++--
 arch/arm/mach-mxs/clock-mx28.c |    2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)
arch/arm/boot/dts/imx28.dtsi +2 -2
index d1f4f8c..5f61777 100644
@@ -52,8 +52,8 @@
 			};
 
 			dma-apbh at 80004000 {
+				compatible = "fsl,imx28-dma-apbh";
 				reg = <0x80004000 2000>;
-				status = "disabled";
 			};
 
 			perfmon at 80006000 {
@@ -115,8 +115,8 @@
 			};
 
 			dma-apbx at 80024000 {
+				compatible = "fsl,imx28-dma-apbx";
 				reg = <0x80024000 2000>;
-				status = "disabled";
 			};
 
 			dcp at 80028000 {
arch/arm/mach-mxs/clock-mx28.c +2 -0
index cca2cf7..9c926b6 100644
@@ -652,6 +652,8 @@ static struct clk_lookup lookups[] = {
 	_REGISTER_CLOCK("80074000.serial", NULL, uart_clk)
 	_REGISTER_CLOCK("800f0000.ethernet", NULL, fec_clk)
 	_REGISTER_CLOCK("800f4000.ethernet", NULL, fec_clk)
+	_REGISTER_CLOCK("80004000.dma-apbh", NULL, hbus_clk)
+	_REGISTER_CLOCK("80024000.dma-apbx", NULL, xbus_clk)
 };
 
 static int clk_misc_init(void)
Signature · 1 line
1.7.0.4
#20 lore raw

PATCH 5/5 ARM: mx28evk: add mxs-dma dt support

Dear Dong Aisheng,

Quoted 9 lines

From: Dong Aisheng <[email protected]>

Cc Grant Likely <[email protected]>
Cc Rob Herring <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Huang Shijie <[email protected]>
Quoted 34 lines
---
 arch/arm/boot/dts/imx28.dtsi   |    4 ++--
 arch/arm/mach-mxs/clock-mx28.c |    2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)
arch/arm/boot/dts/imx28.dtsi +2 -2
index d1f4f8c..5f61777 100644
@@ -52,8 +52,8 @@
 			};
 
 			dma-apbh at 80004000 {
+				compatible = "fsl,imx28-dma-apbh";
 				reg = <0x80004000 2000>;
-				status = "disabled";
 			};
 
 			perfmon at 80006000 {
@@ -115,8 +115,8 @@
 			};
 
 			dma-apbx at 80024000 {
+				compatible = "fsl,imx28-dma-apbx";
 				reg = <0x80024000 2000>;
-				status = "disabled";
 			};
 
 			dcp at 80028000 {
(quoted diff) +0 -0

b/arch/arm/mach-mxs/clock-mx28.c index cca2cf7..9c926b6 100644

arch/arm/mach-mxs/clock-mx28.c static struct clk_lookup lookups[] = { · line 659

Quoted 8 lines
arch/arm/mach-mxs/clock-mx28.c +2 -0
@@ -652,6 +652,8 @@ static struct clk_lookup lookups[] = {
 	_REGISTER_CLOCK("80074000.serial", NULL, uart_clk)
 	_REGISTER_CLOCK("800f0000.ethernet", NULL, fec_clk)
 	_REGISTER_CLOCK("800f4000.ethernet", NULL, fec_clk)
+	_REGISTER_CLOCK("80004000.dma-apbh", NULL, hbus_clk)
+	_REGISTER_CLOCK("80024000.dma-apbx", NULL, xbus_clk)
 };
 
 static int clk_misc_init(void)

Best regards,
Marek Vasut