[PATCH V3 1/3] dmaengine: dw_dmac: Update documentation style comments for dw_dma_platform_data

[PATCH V3 1/3] dmaengine: dw_dmac: Update documentation style comments for dw_dma_platform_data

[Viresh Kumar]

Documentation style comments were missing for few fields in struct
dw_dma_platform_data. Add these.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/dw_dmac.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/dw_dmac.h b/include/linux/dw_dmac.h
index e1c8c9e..62a6190 100644
--- a/include/linux/dw_dmac.h
+++ b/include/linux/dw_dmac.h
@@ -19,6 +19,8 @@
  * @nr_channels: Number of channels supported by hardware (max 8)
  * @is_private: The device channels should be marked as private and not for
  *	by the general purpose DMA channel allocator.
+ * @chan_allocation_order: Allocate channels starting from 0 or 7
+ * @chan_priority: Set channel priority increasing from 0 to 7 or 7 to 0.
  * @block_size: Maximum block size supported by the controller
  * @nr_masters: Number of AHB masters supported by the controller
  * @data_width: Maximum data width supported by hardware per AHB master
-- 
1.7.12.rc2.18.g61b472e

[PATCH V3 2/3] dmaengine: dw_dmac: Enhance device tree support

[Viresh Kumar]

dw_dmac driver already supports device tree but it used to have its platform
data passed the non-DT way.

This patch does following changes:
- pass platform data via DT, non-DT way still takes precedence if both are used.
- create generic filter routine
- Earlier slave information was made available by slave specific filter routines
  in chan->private field. Now, this information would be passed from within dmac
  DT node. Slave drivers would now be required to pass bus_id (a string) as
  parameter to this generic filter(), which would be compared against the slave
  data passed from DT, by the generic filter routine.
- Update binding document

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
V2->V3:
------
- Simplified an equation in filter routine
- renamed variable 'val' as 'tmp' in DT parsing routine

V1->V2:
------
- Optimized filter & DT parsing routine
- Removed unnecessary casts from changes
- renamed filter function
- Fixed function prototype and return value of DT parsing routine for !CONFIG_OF
  case
- use of_get_child_count()

 Documentation/devicetree/bindings/dma/snps-dma.txt |  44 +++++++
 drivers/dma/dw_dmac.c                              | 134 +++++++++++++++++++++
 drivers/dma/dw_dmac_regs.h                         |   4 +
 include/linux/dw_dmac.h                            |  43 ++++---
 4 files changed, 208 insertions(+), 17 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/snps-dma.txt b/Documentation/devicetree/bindings/dma/snps-dma.txt
index c0d85db..5bb3dfb 100644
--- a/Documentation/devicetree/bindings/dma/snps-dma.txt
+++ b/Documentation/devicetree/bindings/dma/snps-dma.txt
@@ -6,6 +6,26 @@ Required properties:
 - interrupt-parent: Should be the phandle for the interrupt controller
   that services interrupts for this device
 - interrupt: Should contain the DMAC interrupt number
+- nr_channels: Number of channels supported by hardware
+- is_private: The device channels should be marked as private and not for by the
+  general purpose DMA channel allocator. False if not passed.
+- chan_allocation_order: order of allocation of channel, 0 (default): ascending,
+  1: descending
+- chan_priority: priority of channels. 0 (default): increase from chan 0->n, 1:
+  increase from chan n->0
+- block_size: Maximum block size supported by the controller
+- nr_masters: Number of AHB masters supported by the controller
+- data_width: Maximum data width supported by hardware per AHB master
+  (0 - 8bits, 1 - 16bits, ..., 5 - 256bits)
+- slave_info:
+	- bus_id: name of this device channel, not just a device name since
+	  devices may have more than one channel e.g. "foo_tx". For using the
+	  dw_generic_filter(), slave drivers must pass exactly this string as
+	  param to filter function.
+	- cfg_hi: Platform-specific initializer for the CFG_HI register
+	- cfg_lo: Platform-specific initializer for the CFG_LO register
+	- src_master: src master for transfers on allocated channel.
+	- dst_master: dest master for transfers on allocated channel.
 
 Example:
 
@@ -14,4 +34,28 @@ Example:
 		reg = <0xfc000000 0x1000>;
 		interrupt-parent = <&vic1>;
 		interrupts = <12>;
+
+		nr_channels = <8>;
+		chan_allocation_order = <1>;
+		chan_priority = <1>;
+		block_size = <0xfff>;
+		nr_masters = <2>;
+		data_width = <3 3 0 0>;
+
+		slave_info {
+			uart0-tx {
+				bus_id = "uart0-tx";
+				cfg_hi = <0x4000>;	/* 0x8 << 11 */
+				cfg_lo = <0>;
+				src_master = <0>;
+				dst_master = <1>;
+			};
+			spi0-tx {
+				bus_id = "spi0-tx";
+				cfg_hi = <0x2000>;	/* 0x4 << 11 */
+				cfg_lo = <0>;
+				src_master = <0>;
+				dst_master = <0>;
+			};
+		};
 	};
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index c4b0eb3..98f33a7 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1179,6 +1179,50 @@ static void dwc_free_chan_resources(struct dma_chan *chan)
 	dev_vdbg(chan2dev(chan), "%s: done\n", __func__);
 }
 
+bool dw_dma_generic_filter(struct dma_chan *chan, void *param)
+{
+	struct dw_dma *dw = to_dw_dma(chan->device);
+	static struct dw_dma *last_dw;
+	static char *last_bus_id;
+	int i = -1;
+
+	/*
+	 * dmaengine framework calls this routine for all channels of all dma
+	 * controller, until true is returned. If 'param' bus_id is not
+	 * registered with a dma controller (dw), then there is no need of
+	 * running below function for all channels of dw.
+	 *
+	 * This block of code does this by saving the parameters of last
+	 * failure. If dw and param are same, i.e. trying on same dw with
+	 * different channel, return false.
+	 */
+	if ((last_dw == dw) && (last_bus_id == param))
+		return false;
+	/*
+	 * Return true:
+	 * - If dw_dma's platform data is not filled with slave info, then all
+	 *   dma controllers are fine for transfer.
+	 * - Or if param is NULL
+	 */
+	if (!dw->sd || !param)
+		return true;
+
+	while (++i < dw->sd_count) {
+		if (!strcmp(dw->sd[i].bus_id, param)) {
+			chan->private = &dw->sd[i];
+			last_dw = NULL;
+			last_bus_id = NULL;
+
+			return true;
+		}
+	}
+
+	last_dw = dw;
+	last_bus_id = param;
+	return false;
+}
+EXPORT_SYMBOL(dw_dma_generic_filter);
+
 /* --------------------- Cyclic DMA API extensions -------------------- */
 
 /**
@@ -1462,6 +1506,91 @@ static void dw_dma_off(struct dw_dma *dw)
 		dw->chan[i].initialized = false;
 }
 
+#ifdef CONFIG_OF
+static struct dw_dma_platform_data *
+__devinit dw_dma_parse_dt(struct platform_device *pdev)
+{
+	struct device_node *sn, *cn, *np = pdev->dev.of_node;
+	struct dw_dma_platform_data *pdata;
+	struct dw_dma_slave *sd;
+	u32 tmp, arr[4];
+
+	if (!np) {
+		dev_err(&pdev->dev, "Missing DT data\n");
+		return NULL;
+	}
+
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return NULL;
+
+	if (of_property_read_u32(np, "nr_channels", &pdata->nr_channels))
+		return NULL;
+
+	if (of_property_read_bool(np, "is_private"))
+		pdata->is_private = true;
+
+	if (!of_property_read_u32(np, "chan_allocation_order", &tmp))
+		pdata->chan_allocation_order = (unsigned char)tmp;
+
+	if (!of_property_read_u32(np, "chan_priority", &tmp))
+		pdata->chan_priority = tmp;
+
+	if (!of_property_read_u32(np, "block_size", &tmp))
+		pdata->block_size = tmp;
+
+	if (!of_property_read_u32(np, "nr_masters", &tmp)) {
+		if (tmp > 4)
+			return NULL;
+
+		pdata->nr_masters = tmp;
+	}
+
+	if (!of_property_read_u32_array(np, "data_width", arr,
+				pdata->nr_masters))
+		for (tmp = 0; tmp < pdata->nr_masters; tmp++)
+			pdata->data_width[tmp] = arr[tmp];
+
+	/* parse slave data */
+	sn = of_find_node_by_name(np, "slave_info");
+	if (!sn)
+		return pdata;
+
+	/* calculate number of slaves */
+	tmp = of_get_child_count(sn);
+	if (!tmp)
+		return NULL;
+
+	sd = devm_kzalloc(&pdev->dev, sizeof(*sd) * tmp, GFP_KERNEL);
+	if (!sd)
+		return NULL;
+
+	pdata->sd = sd;
+	pdata->sd_count = tmp;
+
+	for_each_child_of_node(sn, cn) {
+		sd->dma_dev = &pdev->dev;
+		of_property_read_string(cn, "bus_id", &sd->bus_id);
+		of_property_read_u32(cn, "cfg_hi", &sd->cfg_hi);
+		of_property_read_u32(cn, "cfg_lo", &sd->cfg_lo);
+		if (!of_property_read_u32(cn, "src_master", &tmp))
+			sd->src_master = tmp;
+
+		if (!of_property_read_u32(cn, "dst_master", &tmp))
+			sd->dst_master = tmp;
+		sd++;
+	}
+
+	return pdata;
+}
+#else
+static inline struct dw_dma_platform_data *
+dw_dma_parse_dt(struct platform_device *pdev)
+{
+	return NULL;
+}
+#endif
+
 static int __devinit dw_probe(struct platform_device *pdev)
 {
 	struct dw_dma_platform_data *pdata;
@@ -1478,6 +1607,9 @@ static int __devinit dw_probe(struct platform_device *pdev)
 	int			i;
 
 	pdata = dev_get_platdata(&pdev->dev);
+	if (!pdata)
+		pdata = dw_dma_parse_dt(pdev);
+
 	if (!pdata || pdata->nr_channels > DW_DMA_MAX_NR_CHANNELS)
 		return -EINVAL;
 
@@ -1512,6 +1644,8 @@ static int __devinit dw_probe(struct platform_device *pdev)
 	clk_prepare_enable(dw->clk);
 
 	dw->regs = regs;
+	dw->sd = pdata->sd;
+	dw->sd_count = pdata->sd_count;
 
 	/* get hardware configuration parameters */
 	if (autocfg) {
diff --git a/drivers/dma/dw_dmac_regs.h b/drivers/dma/dw_dmac_regs.h
index ff39fa6..5cc61ba 100644
--- a/drivers/dma/dw_dmac_regs.h
+++ b/drivers/dma/dw_dmac_regs.h
@@ -231,6 +231,10 @@ struct dw_dma {
 	struct tasklet_struct	tasklet;
 	struct clk		*clk;
 
+	/* slave information */
+	struct dw_dma_slave	*sd;
+	unsigned int		sd_count;
+
 	u8			all_chan_mask;
 
 	/* hardware configuration */
diff --git a/include/linux/dw_dmac.h b/include/linux/dw_dmac.h
index 62a6190..41766de 100644
--- a/include/linux/dw_dmac.h
+++ b/include/linux/dw_dmac.h
@@ -15,6 +15,26 @@
 #include <linux/dmaengine.h>
 
 /**
+ * struct dw_dma_slave - Controller-specific information about a slave
+ *
+ * @dma_dev: required DMA master device. Depricated.
+ * @bus_id: name of this device channel, not just a device name since
+ *          devices may have more than one channel e.g. "foo_tx"
+ * @cfg_hi: Platform-specific initializer for the CFG_HI register
+ * @cfg_lo: Platform-specific initializer for the CFG_LO register
+ * @src_master: src master for transfers on allocated channel.
+ * @dst_master: dest master for transfers on allocated channel.
+ */
+struct dw_dma_slave {
+	struct device		*dma_dev;
+	const char		*bus_id;
+	u32			cfg_hi;
+	u32			cfg_lo;
+	u8			src_master;
+	u8			dst_master;
+};
+
+/**
  * struct dw_dma_platform_data - Controller configuration parameters
  * @nr_channels: Number of channels supported by hardware (max 8)
  * @is_private: The device channels should be marked as private and not for
@@ -25,6 +45,8 @@
  * @nr_masters: Number of AHB masters supported by the controller
  * @data_width: Maximum data width supported by hardware per AHB master
  *		(0 - 8bits, 1 - 16bits, ..., 5 - 256bits)
+ * @sd: slave specific data. Used for configuring channels
+ * @sd_count: count of slave data structures passed.
  */
 struct dw_dma_platform_data {
 	unsigned int	nr_channels;
@@ -38,6 +60,9 @@ struct dw_dma_platform_data {
 	unsigned short	block_size;
 	unsigned char	nr_masters;
 	unsigned char	data_width[4];
+
+	struct dw_dma_slave *sd;
+	unsigned int sd_count;
 };
 
 /* bursts size */
@@ -52,23 +77,6 @@ enum dw_dma_msize {
 	DW_DMA_MSIZE_256,
 };
 
-/**
- * struct dw_dma_slave - Controller-specific information about a slave
- *
- * @dma_dev: required DMA master device
- * @cfg_hi: Platform-specific initializer for the CFG_HI register
- * @cfg_lo: Platform-specific initializer for the CFG_LO register
- * @src_master: src master for transfers on allocated channel.
- * @dst_master: dest master for transfers on allocated channel.
- */
-struct dw_dma_slave {
-	struct device		*dma_dev;
-	u32			cfg_hi;
-	u32			cfg_lo;
-	u8			src_master;
-	u8			dst_master;
-};
-
 /* Platform-configurable bits in CFG_HI */
 #define DWC_CFGH_FCMODE		(1 << 0)
 #define DWC_CFGH_FIFO_MODE	(1 << 1)
@@ -106,5 +114,6 @@ void dw_dma_cyclic_stop(struct dma_chan *chan);
 dma_addr_t dw_dma_get_src_addr(struct dma_chan *chan);
 
 dma_addr_t dw_dma_get_dst_addr(struct dma_chan *chan);
+bool dw_dma_generic_filter(struct dma_chan *chan, void *param);
 
 #endif /* DW_DMAC_H */
-- 
1.7.12.rc2.18.g61b472e

[PATCH V3 3/3] ARM: SPEAr13xx: Pass DW DMAC platform data from DT

[Viresh Kumar]

This patch adds dw_dmac's platform data to DT node. It also creates slave info
node for SPEAr13xx, for the devices which were using dw_dmac.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
V1->V3:
------
- renamed filter function

 arch/arm/boot/dts/spear1340.dtsi             | 19 ++++++++++
 arch/arm/boot/dts/spear13xx.dtsi             | 38 ++++++++++++++++++++
 arch/arm/mach-spear13xx/include/mach/spear.h |  2 --
 arch/arm/mach-spear13xx/spear1310.c          |  4 +--
 arch/arm/mach-spear13xx/spear1340.c          | 27 +++-----------
 arch/arm/mach-spear13xx/spear13xx.c          | 54 ++--------------------------
 6 files changed, 65 insertions(+), 79 deletions(-)

diff --git a/arch/arm/boot/dts/spear1340.dtsi b/arch/arm/boot/dts/spear1340.dtsi
index d71fe2a..8ea3f66 100644
--- a/arch/arm/boot/dts/spear1340.dtsi
+++ b/arch/arm/boot/dts/spear1340.dtsi
@@ -24,6 +24,25 @@
 			status = "disabled";
 		};
 
+		dma@ea800000 {
+			slave_info {
+				uart1_tx {
+					bus_id = "uart1_tx";
+					cfg_hi = <0x6000>;	/* 0xC << 11 */
+					cfg_lo = <0>;
+					src_master = <0>;
+					dst_master = <1>;
+				};
+				uart1_tx {
+					bus_id = "uart1_tx";
+					cfg_hi = <0x680>;	/* 0xD << 7 */
+					cfg_lo = <0>;
+					src_master = <1>;
+					dst_master = <0>;
+				};
+			};
+		};
+
 		spi1: spi@5d400000 {
 			compatible = "arm,pl022", "arm,primecell";
 			reg = <0x5d400000 0x1000>;
diff --git a/arch/arm/boot/dts/spear13xx.dtsi b/arch/arm/boot/dts/spear13xx.dtsi
index f7b84ac..f06bb50 100644
--- a/arch/arm/boot/dts/spear13xx.dtsi
+++ b/arch/arm/boot/dts/spear13xx.dtsi
@@ -91,6 +91,37 @@
 			reg = <0xea800000 0x1000>;
 			interrupts = <0 19 0x4>;
 			status = "disabled";
+
+			nr_channels = <8>;
+			chan_allocation_order = <1>;
+			chan_priority = <1>;
+			block_size = <0xfff>;
+			nr_masters = <2>;
+			data_width = <3 3 0 0>;
+
+			slave_info {
+				ssp0_tx {
+					bus_id = "ssp0_tx";
+					cfg_hi = <0x2000>;	/* 0x4 << 11 */
+					cfg_lo = <0>;
+					src_master = <0>;
+					dst_master = <0>;
+				};
+				ssp0_rx {
+					bus_id = "ssp0_rx";
+					cfg_hi = <0x280>;	/* 0x5 << 7 */
+					cfg_lo = <0>;
+					src_master = <0>;
+					dst_master = <0>;
+				};
+				cf {
+					bus_id = "cf";
+					cfg_hi = <0>;
+					cfg_lo = <0>;
+					src_master = <0>;
+					dst_master = <0>;
+				};
+			};
 		};
 
 		dma@eb000000 {
@@ -98,6 +129,13 @@
 			reg = <0xeb000000 0x1000>;
 			interrupts = <0 59 0x4>;
 			status = "disabled";
+
+			nr_channels = <8>;
+			chan_allocation_order = <1>;
+			chan_priority = <1>;
+			block_size = <0xfff>;
+			nr_masters = <2>;
+			data_width = <3 3 0 0>;
 		};
 
 		fsmc: flash@b0000000 {
diff --git a/arch/arm/mach-spear13xx/include/mach/spear.h b/arch/arm/mach-spear13xx/include/mach/spear.h
index 07d90ac..71bf5b6 100644
--- a/arch/arm/mach-spear13xx/include/mach/spear.h
+++ b/arch/arm/mach-spear13xx/include/mach/spear.h
@@ -43,8 +43,6 @@
 #define VA_L2CC_BASE				IOMEM(UL(0xFB000000))
 
 /* others */
-#define DMAC0_BASE				UL(0xEA800000)
-#define DMAC1_BASE				UL(0xEB000000)
 #define MCIF_CF_BASE				UL(0xB2800000)
 
 /* Devices present in SPEAr1310 */
diff --git a/arch/arm/mach-spear13xx/spear1310.c b/arch/arm/mach-spear13xx/spear1310.c
index 9fbbfc5..0e60195 100644
--- a/arch/arm/mach-spear13xx/spear1310.c
+++ b/arch/arm/mach-spear13xx/spear1310.c
@@ -36,9 +36,7 @@ static struct pl022_ssp_controller ssp1_plat_data = {
 
 /* Add SPEAr1310 auxdata to pass platform data */
 static struct of_dev_auxdata spear1310_auxdata_lookup[] __initdata = {
-	OF_DEV_AUXDATA("arasan,cf-spear1340", MCIF_CF_BASE, NULL, &cf_dma_priv),
-	OF_DEV_AUXDATA("snps,dma-spear1340", DMAC0_BASE, NULL, &dmac_plat_data),
-	OF_DEV_AUXDATA("snps,dma-spear1340", DMAC1_BASE, NULL, &dmac_plat_data),
+	OF_DEV_AUXDATA("arasan,cf-spear1340", MCIF_CF_BASE, NULL, "cf"),
 	OF_DEV_AUXDATA("arm,pl022", SSP_BASE, NULL, &pl022_plat_data),
 
 	OF_DEV_AUXDATA("arm,pl022", SPEAR1310_SSP1_BASE, NULL, &ssp1_plat_data),
diff --git a/arch/arm/mach-spear13xx/spear1340.c b/arch/arm/mach-spear13xx/spear1340.c
index 081014f..7301f14 100644
--- a/arch/arm/mach-spear13xx/spear1340.c
+++ b/arch/arm/mach-spear13xx/spear1340.c
@@ -20,7 +20,6 @@
 #include <linux/of_platform.h>
 #include <asm/hardware/gic.h>
 #include <asm/mach/arch.h>
-#include <mach/dma.h>
 #include <mach/generic.h>
 #include <mach/spear.h>
 
@@ -78,26 +77,10 @@
 			(SPEAR1340_MIPHY_OSC_BYPASS_EXT | \
 			SPEAR1340_MIPHY_PLL_RATIO_TOP(25))
 
-static struct dw_dma_slave uart1_dma_param[] = {
-	{
-		/* Tx */
-		.cfg_hi = DWC_CFGH_DST_PER(SPEAR1340_DMA_REQ_UART1_TX),
-		.cfg_lo = 0,
-		.src_master = DMA_MASTER_MEMORY,
-		.dst_master = SPEAR1340_DMA_MASTER_UART1,
-	}, {
-		/* Rx */
-		.cfg_hi = DWC_CFGH_SRC_PER(SPEAR1340_DMA_REQ_UART1_RX),
-		.cfg_lo = 0,
-		.src_master = SPEAR1340_DMA_MASTER_UART1,
-		.dst_master = DMA_MASTER_MEMORY,
-	}
-};
-
 static struct amba_pl011_data uart1_data = {
-	.dma_filter = dw_dma_filter,
-	.dma_tx_param = &uart1_dma_param[0],
-	.dma_rx_param = &uart1_dma_param[1],
+	.dma_filter = dw_dma_generic_filter,
+	.dma_tx_param = "uart1_tx",
+	.dma_rx_param = "uart1_rx",
 };
 
 /* SATA device registration */
@@ -158,9 +141,7 @@ static struct ahci_platform_data sata_pdata = {
 
 /* Add SPEAr1340 auxdata to pass platform data */
 static struct of_dev_auxdata spear1340_auxdata_lookup[] __initdata = {
-	OF_DEV_AUXDATA("arasan,cf-spear1340", MCIF_CF_BASE, NULL, &cf_dma_priv),
-	OF_DEV_AUXDATA("snps,dma-spear1340", DMAC0_BASE, NULL, &dmac_plat_data),
-	OF_DEV_AUXDATA("snps,dma-spear1340", DMAC1_BASE, NULL, &dmac_plat_data),
+	OF_DEV_AUXDATA("arasan,cf-spear1340", MCIF_CF_BASE, NULL, "cf"),
 	OF_DEV_AUXDATA("arm,pl022", SSP_BASE, NULL, &pl022_plat_data),
 
 	OF_DEV_AUXDATA("snps,spear-ahci", SPEAR1340_SATA_BASE, NULL,
diff --git a/arch/arm/mach-spear13xx/spear13xx.c b/arch/arm/mach-spear13xx/spear13xx.c
index 5633d69..0e166fa 100644
--- a/arch/arm/mach-spear13xx/spear13xx.c
+++ b/arch/arm/mach-spear13xx/spear13xx.c
@@ -22,67 +22,19 @@
 #include <asm/hardware/gic.h>
 #include <asm/mach/map.h>
 #include <asm/smp_twd.h>
-#include <mach/dma.h>
 #include <mach/generic.h>
 #include <mach/spear.h>
 
-/* common dw_dma filter routine to be used by peripherals */
-bool dw_dma_filter(struct dma_chan *chan, void *slave)
-{
-	struct dw_dma_slave *dws = (struct dw_dma_slave *)slave;
-
-	if (chan->device->dev == dws->dma_dev) {
-		chan->private = slave;
-		return true;
-	} else {
-		return false;
-	}
-}
-
 /* ssp device registration */
-static struct dw_dma_slave ssp_dma_param[] = {
-	{
-		/* Tx */
-		.cfg_hi = DWC_CFGH_DST_PER(DMA_REQ_SSP0_TX),
-		.cfg_lo = 0,
-		.src_master = DMA_MASTER_MEMORY,
-		.dst_master = DMA_MASTER_SSP0,
-	}, {
-		/* Rx */
-		.cfg_hi = DWC_CFGH_SRC_PER(DMA_REQ_SSP0_RX),
-		.cfg_lo = 0,
-		.src_master = DMA_MASTER_SSP0,
-		.dst_master = DMA_MASTER_MEMORY,
-	}
-};
-
 struct pl022_ssp_controller pl022_plat_data = {
 	.bus_id = 0,
 	.enable_dma = 1,
-	.dma_filter = dw_dma_filter,
-	.dma_rx_param = &ssp_dma_param[1],
-	.dma_tx_param = &ssp_dma_param[0],
+	.dma_filter = dw_dma_generic_filter,
+	.dma_rx_param = "ssp0_rx",
+	.dma_tx_param = "ssp0_tx",
 	.num_chipselect = 3,
 };
 
-/* CF device registration */
-struct dw_dma_slave cf_dma_priv = {
-	.cfg_hi = 0,
-	.cfg_lo = 0,
-	.src_master = 0,
-	.dst_master = 0,
-};
-
-/* dmac device registeration */
-struct dw_dma_platform_data dmac_plat_data = {
-	.nr_channels = 8,
-	.chan_allocation_order = CHAN_ALLOCATION_DESCENDING,
-	.chan_priority = CHAN_PRIORITY_DESCENDING,
-	.block_size = 4095U,
-	.nr_masters = 2,
-	.data_width = { 3, 3, 0, 0 },
-};
-
 void __init spear13xx_l2x0_init(void)
 {
 	/*
-- 
1.7.12.rc2.18.g61b472e

Re: [PATCH V3 2/3] dmaengine: dw_dmac: Enhance device tree support

[Vinod Koul]

On Fri, 2012-10-26 at 14:25 +0530, Viresh Kumar wrote:
> Hi Vinod,
> 
> On 16 October 2012 09:49, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > dw_dmac driver already supports device tree but it used to have its platform
> > data passed the non-DT way.
> >
> > This patch does following changes:
> > - pass platform data via DT, non-DT way still takes precedence if both are used.
> > - create generic filter routine
> > - Earlier slave information was made available by slave specific filter routines
> >   in chan->private field. Now, this information would be passed from within dmac
> >   DT node. Slave drivers would now be required to pass bus_id (a string) as
> >   parameter to this generic filter(), which would be compared against the slave
> >   data passed from DT, by the generic filter routine.
> > - Update binding document
> 
> I can see that you applied these patches and they are present in
> linux-next. But i feel
> the order of patches is bad.
Yes looks like I forgot to sort the mbox series :(

since all patches were applied nicely, and they seem fairly independent
of each other it should cause issue. Let me know if you wnat me to redo
my -next.

-- 
Vinod Koul
Intel Corp.

Re: [PATCH V3 2/3] dmaengine: dw_dmac: Enhance device tree support

[Viresh Kumar]

Hi Vinod,

On 16 October 2012 09:49, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> dw_dmac driver already supports device tree but it used to have its platform
> data passed the non-DT way.
>
> This patch does following changes:
> - pass platform data via DT, non-DT way still takes precedence if both are used.
> - create generic filter routine
> - Earlier slave information was made available by slave specific filter routines
>   in chan->private field. Now, this information would be passed from within dmac
>   DT node. Slave drivers would now be required to pass bus_id (a string) as
>   parameter to this generic filter(), which would be compared against the slave
>   data passed from DT, by the generic filter routine.
> - Update binding document

I can see that you applied these patches and they are present in
linux-next. But i feel
the order of patches is bad.

git log --oneline gives following:

879a0ec dmaengine: dw_dmac: Update documentation style comments for
dw_dma_platform_data
d0e35f3 dmaengine: dw_dmac: Enhance device tree support
4c4c30c ARM: SPEAr13xx: Pass DW DMAC platform data from DT

The last patch "ARM: SPEAr13xx: Pass DW DMAC platform data from DT" has
dependency on second patch.

The correct order would be:
4c4c30c ARM: SPEAr13xx: Pass DW DMAC platform data from DT
d0e35f3 dmaengine: dw_dmac: Enhance device tree support
879a0ec dmaengine: dw_dmac: Update documentation style comments for
dw_dma_platform_data


PS: We are looking at output of git log. Last patch (or first from
bottom) was applied first.

--
viresh

Re: [PATCH V3 2/3] dmaengine: dw_dmac: Enhance device tree support

[Viresh Kumar]

On 26 October 2012 14:18, Vinod Koul <vkoul@infradead.org> wrote:
> On Fri, 2012-10-26 at 14:25 +0530, Viresh Kumar wrote:
>> I can see that you applied these patches and they are present in
>> linux-next. But i feel
>> the order of patches is bad.
> Yes looks like I forgot to sort the mbox series :(
>
> since all patches were applied nicely, and they seem fairly independent
> of each other it should cause issue. Let me know if you wnat me to redo
> my -next.

yes. The ARCH specific patch 3/3 uses a routine created in 2/3. So they have
to be in right order for git bisect to work.

--
viresh

Re: [PATCH V3 2/3] dmaengine: dw_dmac: Enhance device tree support

[Vinod Koul]

On Fri, 2012-10-26 at 14:35 +0530, Viresh Kumar wrote:
> >> I can see that you applied these patches and they are present in
> >> linux-next. But i feel
> >> the order of patches is bad.
> > Yes looks like I forgot to sort the mbox series :(
> >
> > since all patches were applied nicely, and they seem fairly independent
> > of each other it should cause issue. Let me know if you wnat me to redo
> > my -next.
> 
> yes. The ARCH specific patch 3/3 uses a routine created in 2/3. So they have
> to be in right order for git bisect to work. 

Done, it should reflect now.
Sorry for the mess.

-- 
Vinod Koul
Intel Corp.

Re: [PATCH V3 2/3] dmaengine: dw_dmac: Enhance device tree support

[Arnd Bergmann]

On Tuesday 16 October 2012, Viresh Kumar wrote:
> dw_dmac driver already supports device tree but it used to have its platform
> data passed the non-DT way.
> 
> This patch does following changes:
> - pass platform data via DT, non-DT way still takes precedence if both are used.
> - create generic filter routine
> - Earlier slave information was made available by slave specific filter routines
>   in chan->private field. Now, this information would be passed from within dmac
>   DT node. Slave drivers would now be required to pass bus_id (a string) as
>   parameter to this generic filter(), which would be compared against the slave
>   data passed from DT, by the generic filter routine.
> - Update binding document
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

I'm deeply sorry for the very late complaint on this, but I only now
noticed this patch as I was seeing build breakage in linux-next
because of it.

The build bug is not the problem however, but the abuse of the
API is. Andy, are you sure you understood what this does when
you gave you Reviewed-by comment?

Viresh, there are multiple problems with your approach unfortunately:

* It does not follow the binding from
  Documentation/devicetree/bindings/dma/dma.txt

* It requires slave drivers to know that they are using the dw_dmac
  driver and pass a pointer to dw_generic_filter, which is not
  generic at all

* It requires the dmac node to have information about all slaves

There are also some minor issues, such as the naming of DT
properties, but the above need to be resolved first.

	Arnd

Re: [PATCH V3 2/3] dmaengine: dw_dmac: Enhance device tree support

[Arnd Bergmann]

On Monday 10 December 2012, Arnd Bergmann wrote:
> The build bug is not the problem however, but the abuse of the
> API is. Andy, are you sure you understood what this does when
> you gave you Reviewed-by comment?
> 

Ah, I just saw that Andy had the right intuition when commenting on
it at first, but Viresh managed to convince him otherwise, and appearently
neither had followed the discussion about the dmaengine bindings...

	Arnd

Re: [PATCH V3 2/3] dmaengine: dw_dmac: Enhance device tree support

[Viresh Kumar]

Sorry for replying late, was too busy with other work yesterday.

On 11 December 2012 03:38, Arnd Bergmann <arnd@arndb.de> wrote:
> I'm deeply sorry for the very late complaint on this, but I only now

Better late than never :)

> noticed this patch as I was seeing build breakage in linux-next
> because of it.

Oops!! Here is a fix for that

Author: Viresh Kumar <viresh.kumar@linaro.org>
Date:   Wed Dec 12 08:28:07 2012 +0530

    ARM: SPEAr1310: Fix CF DMA data

    We need to pass string with device-channel name to dma controller
instead of dma
    controller specific dma struct. Fix CF dma data.

    Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 arch/arm/mach-spear13xx/spear1310.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-spear13xx/spear1310.c
b/arch/arm/mach-spear13xx/spear1310.c
index f65ad5b..54f0e2e 100644
--- a/arch/arm/mach-spear13xx/spear1310.c
+++ b/arch/arm/mach-spear13xx/spear1310.c
@@ -36,7 +36,7 @@
 static struct arasan_cf_pdata cf_pdata = {
        .cf_if_clk = CF_IF_CLK_166M,
        .quirk = CF_BROKEN_UDMA,
-       .dma_priv = &cf_dma_priv,
+       .dma_priv = "cf",
 };

> Viresh, there are multiple problems with your approach unfortunately:
>
> * It does not follow the binding from
>   Documentation/devicetree/bindings/dma/dma.txt

When i patched it, this patch wasn't there in linux-next/master.
Probably was getting reviewed somewhere :)

> * It requires slave drivers to know that they are using the dw_dmac
>   driver and pass a pointer to dw_generic_filter, which is not
>   generic at all
>
> * It requires the dmac node to have information about all slaves
>
> There are also some minor issues, such as the naming of DT
> properties, but the above need to be resolved first.

I saw the binding document and it looks it can be applied to dw_dmac
too, as there is nothing special for it.

The question is how? We are already late for merge window and this
one is queued. Supplying a new patch, getting it reviewed/tested and
being pulled by Linus is not so easy :)

Two ways:
- Keep it as is, and i will fix it separately and quickly
- Drop it :(

--
viresh

Re: [PATCH V3 2/3] dmaengine: dw_dmac: Enhance device tree support

[Andy Shevchenko]

On Wed, 2012-12-12 at 08:30 +0530, Viresh Kumar wrote: 
> > * It requires slave drivers to know that they are using the dw_dmac
> >   driver and pass a pointer to dw_generic_filter, which is not
> >   generic at all
> >
> > * It requires the dmac node to have information about all slaves
> >
> > There are also some minor issues, such as the naming of DT
> > properties, but the above need to be resolved first.
> 
> I saw the binding document and it looks it can be applied to dw_dmac
> too, as there is nothing special for it.
> 
> The question is how? We are already late for merge window and this
> one is queued. Supplying a new patch, getting it reviewed/tested and
> being pulled by Linus is not so easy :)
> 
> Two ways:
> - Keep it as is, and i will fix it separately and quickly
> - Drop it :(
Will we survive if the patch is in mainline? I mean how big the impact
of it is?


-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH V3 2/3] dmaengine: dw_dmac: Enhance device tree support

[Andy Shevchenko]

On Mon, 2012-12-10 at 22:08 +0000, Arnd Bergmann wrote: 
> The build bug is not the problem however, but the abuse of the
> API is. Andy, are you sure you understood what this does when
> you gave you Reviewed-by comment?

Thank you for pointing this out to refresh my memories and go through
documentation again.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH V3 2/3] dmaengine: dw_dmac: Enhance device tree support

[Viresh Kumar]

On 12 December 2012 14:10, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> Will we survive if the patch is in mainline? I mean how big the impact
> of it is?

It doesn't fail to do fulfill its purpose and even ALL DT stuff would work well.
Its just the matter of using the right API's, design and bindings :)

Re: [PATCH V3 2/3] dmaengine: dw_dmac: Enhance device tree support

[Arnd Bergmann]

On Wednesday 12 December 2012, Viresh Kumar wrote:
> I saw the binding document and it looks it can be applied to dw_dmac
> too, as there is nothing special for it.
> 
> The question is how? We are already late for merge window and this
> one is queued. Supplying a new patch, getting it reviewed/tested and
> being pulled by Linus is not so easy :)
> 
> Two ways:
> - Keep it as is, and i will fix it separately and quickly
> - Drop it :(

It seems we got both lucky and unlucky here, as all the dmaengine patches,
including this set and the generic DT bindings missed out in the end.
So there is time to fix it all for 3.9.

	Arnd