Skip to contentSkip to conversation

dmaengine: convert dw_dmac/spear13xx to generic binding

PATCH In review Arnd Bergmann 10 participants 60 messages Jan 28, 2013 – Apr 19, 2013 4 messages not in the archive

Subsystems (11)
lore
Share & tools
lore
#0 lore raw

cover dmaengine: convert dw_dmac/spear13xx to generic binding

Hi everyone,

This is my attempt to convert the spear platform and the dw_dmac to the generic device tree binding for DMA, so that we don't get a release with the broken version. I'm pretty sure that this has bugs, but it's as good as I could do without access to hardware or specs.

Please review and comment,

Arnd

Arnd Bergmann (5):
dmaengine: dw_dmac: move to generic DMA binding
spi: pl022: use generic DMA slave configuration if possible
serial: pl011: use generic DMA slave configuration if possible
ata: arasan: remove the need for platform_data
ARM: SPEAr13xx: Pass generic DW DMAC platform data from DT

 .../devicetree/bindings/ata/pata-arasan.txt        |  22 ++++
 Documentation/devicetree/bindings/dma/snps-dma.txt |  70 +++++------
 arch/arm/boot/dts/spear1340.dtsi                   |   2 +
 arch/arm/boot/dts/spear13xx.dtsi                   |  25 +++-
 arch/arm/mach-spear/generic.h                      |   6 -
 arch/arm/mach-spear/include/mach/spear.h           |   2 -
 arch/arm/mach-spear/spear1310.c                    |  30 +----
 arch/arm/mach-spear/spear1340.c                    |  32 +----
 arch/arm/mach-spear/spear13xx-dma.h                | 128 --------------------
 arch/arm/mach-spear/spear13xx.c                    |  58 ---------
 drivers/ata/pata_arasan_cf.c                       |  31 +++--
 drivers/dma/dw_dmac.c                              | 130 ++++++++++-----------
 drivers/dma/dw_dmac_regs.h                         |   4 -
 drivers/spi/spi-pl022.c                            |  43 ++++++-
 drivers/tty/serial/amba-pl011.c                    |  62 ++++++----
 include/linux/dw_dmac.h                            |   5 -
 include/linux/pata_arasan_cf_data.h                |   2 -
 17 files changed, 243 insertions(+), 409 deletions(-)
 delete mode 100644 arch/arm/mach-spear/spear13xx-dma.h
Signature · 14 lines
1.8.0

Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/[email protected]>
Grant Likely <grant.likely-s3s/[email protected]>
Greg Kroah-Hartman <[email protected]>
Jeff Garzik <[email protected]>
Jiri Slaby <[email protected]>
Linus Walleij <[email protected]>
Mark Brown <broonie-yzvPICuk2AATkU/[email protected]>
Vinod Koul <vinod.koul-VuQAYsv1563Yd54FQh9/[email protected]>
Viresh Kumar <[email protected]>
devicetree-discuss-uLR06cmDAlY/[email protected]
[email protected]
spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org

A message that is not in the archive (sent off-list or not yet received): 1359395857-1235-1-git-send-email-arnd-r2nGTMty4D4@public.gmane.org

#2 lore raw

PATCH 1/5 dmaengine: dw_dmac: move to generic DMA binding

The original device tree binding for this driver, from Viresh Kumar unfortunately conflicted with the generic DMA binding, and did not allow to completely seperate slave device configuration from the controller.

This is an attempt to replace it with an implementation of the generic binding, but it is currently completely untested, because I do not have any hardware with this particular controller.

The patch applies on top of linux-next, which contains both the base support for the generic DMA binding, as well as the earlier attempt from Viresh. Both of these are currently not merged upstream however.

There are a couple of TODO items that are left remaining and are open for ideas from other people.

Cc Viresh Kumar <[email protected]>
Cc Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/[email protected]>
Cc Vinod Koul <vinod.koul-VuQAYsv1563Yd54FQh9/[email protected]>
Cc devicetree-discuss-uLR06cmDAlY/[email protected]
---
 Documentation/devicetree/bindings/dma/snps-dma.txt |  70 +++++------
 drivers/dma/dw_dmac.c                              | 130 ++++++++++-----------
 drivers/dma/dw_dmac_regs.h                         |   4 -
 include/linux/dw_dmac.h                            |   5 -
 4 files changed, 95 insertions(+), 114 deletions(-)
Documentation/devicetree/bindings/dma/snps-dma.txt +36 -34
index 5bb3dfb..8539adc 100644
@@ -3,59 +3,61 @@
 Required properties:
 - compatible: "snps,dma-spear1340"
 - reg: Address range of the DMAC registers
-- 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.
+- dma-channels: Number of channels supported by hardware
+- dma-requests: Number of DMA request lines supported
+- dma-masters: Number of AHB masters supported by the controller
+- #dma-cells: must be <3>
 - 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.
+
+
+Optional properties:
+- interrupt-parent: Should be the phandle for the interrupt controller
+  that services interrupts for this device
+- is_private: The device channels should be marked as private and not for by the
+  general purpose DMA channel allocator. False if not passed.
 
 Example:
 
-	dma@fc000000 {
+	dmahost: dma@fc000000 {
 		compatible = "snps,dma-spear1340";
 		reg = <0xfc000000 0x1000>;
 		interrupt-parent = <&vic1>;
 		interrupts = <12>;
 
-		nr_channels = <8>;
+		dma-channels = <8>;
+		dma-requests = <32>;
+		dma-masters = <2>;
+		#dma-cells = <3>;
 		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>;
-			};
-		};
+DMA clients connected to the Designware DMA controller must use the format
+described in the dma.txt file, using a five-cell specifier for each channel.
+The five cells in order are:
+
+1. A phandle pointing to the DMA controller
+2. The number of the request line.
+3. Source master for transfers on allocated channel.
+4. Destination master for transfers on allocated channel.
+
+Example:
+	
+	serial@e0000000 {
+		compatible = "arm,pl011", "arm,primecell";
+		reg = <0xe0000000 0x1000>;
+		interrupts = <0 35 0x4>;
+		status = "disabled";
+		dmas = <&dmahost 12 0 1>,
+			<&dmahost 13 1 0>;
+		dma-names = "rx", "rx";
 	};
drivers/dma/dw_dmac.c +59 -71

194 lines not on this page · raw message

drivers/dma/dw_dmac_regs.h +0 -4
index fef296d..0c5244d 100644
@@ -239,10 +239,6 @@ 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 */
include/linux/dw_dmac.h +0 -5
index 41766de..481ab23 100644
@@ -27,7 +27,6 @@
  */
 struct dw_dma_slave {
 	struct device		*dma_dev;
-	const char		*bus_id;
 	u32			cfg_hi;
 	u32			cfg_lo;
 	u8			src_master;
@@ -60,9 +59,6 @@ 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 */
@@ -114,6 +110,5 @@ 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 */
Signature · 1 line
1.8.0
#3 lore raw

PATCH 2/5 spi: pl022: use generic DMA slave configuration if possible

In reply to a message not in the archive

With the new OF DMA binding, it is possible to completely avoid the need for platform_data for configuring a DMA channel. In cases where the platform has already been converted, calling dma_request_slave_channel should get all the necessary information from the device tree.

Like the patch that converts the dw_dma controller, this is completely untested and is looking for someone to try it out.

Cc Grant Likely <grant.likely-s3s/[email protected]>
Cc Mark Brown <broonie-yzvPICuk2AATkU/[email protected]>
Cc spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
Cc Viresh Kumar <[email protected]>
Cc Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/[email protected]>
Cc Vinod Koul <vinod.koul-VuQAYsv1563Yd54FQh9/[email protected]>
Cc devicetree-discuss-uLR06cmDAlY/[email protected]
---
 drivers/spi/spi-pl022.c | 43 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 2 deletions(-)
drivers/spi/spi-pl022.c +41 -2
index b0fe393..371cc66f 100644
@@ -1139,6 +1139,35 @@ err_no_rxchan:
 	return -ENODEV;
 }
 
+static int pl022_dma_autoprobe(struct pl022 *pl022)
+{
+	struct device *dev = &pl022->adev->dev;
+
+	/* automatically configure DMA channels from platform, normally using DT */
+	pl022->dma_rx_channel = dma_request_slave_channel(dev, "rx");
+	if (!pl022->dma_rx_channel)
+		goto err_no_rxchan;
+
+	pl022->dma_tx_channel = dma_request_slave_channel(dev, "tx");
+	if (!pl022->dma_tx_channel)
+		goto err_no_txchan;
+
+	pl022->dummypage = kmalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!pl022->dummypage)
+		goto err_no_dummypage;
+
+	return 0;
+
+err_no_dummypage:
+	dma_release_channel(pl022->dma_tx_channel);
+	pl022->dma_tx_channel = NULL;
+err_no_txchan:
+	dma_release_channel(pl022->dma_rx_channel);
+	pl022->dma_rx_channel = NULL;
+err_no_rxchan:
+	return -ENODEV;
+}
+		
 static void terminate_dma(struct pl022 *pl022)
 {
 	struct dma_chan *rxchan = pl022->dma_rx_channel;
@@ -1167,6 +1196,11 @@ static inline int configure_dma(struct pl022 *pl022)
 	return -ENODEV;
 }
 
+static inline int pl022_dma_autoprobe(struct pl022 *pl022)
+{
+	return 0;
+}
+
 static inline int pl022_dma_probe(struct pl022 *pl022)
 {
 	return 0;
@@ -2226,8 +2260,13 @@ static int pl022_probe(struct amba_device *adev, const struct amba_id *id)
 		goto err_no_irq;
 	}
 
-	/* Get DMA channels */
-	if (platform_info->enable_dma) {
+	/* Get DMA channels, try autoconfiguration first */
+	status = pl022_dma_autoprobe(pl022);
+
+	/* If that failed, use channels from platform_info */
+	if (status == 0)
+		platform_info->enable_dma = 1;
+	else if (platform_info->enable_dma) {
 		status = pl022_dma_probe(pl022);
 		if (status != 0)
 			platform_info->enable_dma = 0;
Signature · 9 lines
1.8.0


------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnnow-d2d

A message that is not in the archive (sent off-list or not yet received): 1359395857-1235-3-git-send-email-arnd-r2nGTMty4D4@public.gmane.org

Acked-by
#5 lore raw

Re: [PATCH 2/5] spi: pl022: use generic DMA slave configuration if possible

Quoted 17 lines · Arnd Bergmann

With the new OF DMA binding, it is possible to completely avoid the need for platform_data for configuring a DMA channel. In cases where the platform has already been converted, calling dma_request_slave_channel should get all the necessary information from the device tree.

Like the patch that converts the dw_dma controller, this is completely untested and is looking for someone to try it out.

Cc Grant Likely <grant.likely-s3s/[email protected]>
Cc Mark Brown <broonie-yzvPICuk2AATkU/[email protected]>
Cc spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
Cc Viresh Kumar <[email protected]>
Cc Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/[email protected]>
Cc Vinod Koul <vinod.koul-VuQAYsv1563Yd54FQh9/[email protected]>
Cc devicetree-discuss-uLR06cmDAlY/[email protected]
Quoted 3 lines
---
 drivers/spi/spi-pl022.c | 43 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 2 deletions(-)
Quoted 72 lines
drivers/spi/spi-pl022.c +41 -2
index b0fe393..371cc66f 100644
@@ -1139,6 +1139,35 @@ err_no_rxchan:
 	return -ENODEV;
 }
 
+static int pl022_dma_autoprobe(struct pl022 *pl022)
+{
+	struct device *dev = &pl022->adev->dev;
+
+	/* automatically configure DMA channels from platform, normally using DT */
+	pl022->dma_rx_channel = dma_request_slave_channel(dev, "rx");
+	if (!pl022->dma_rx_channel)
+		goto err_no_rxchan;
+
+	pl022->dma_tx_channel = dma_request_slave_channel(dev, "tx");
+	if (!pl022->dma_tx_channel)
+		goto err_no_txchan;
+
+	pl022->dummypage = kmalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!pl022->dummypage)
+		goto err_no_dummypage;
+
+	return 0;
+
+err_no_dummypage:
+	dma_release_channel(pl022->dma_tx_channel);
+	pl022->dma_tx_channel = NULL;
+err_no_txchan:
+	dma_release_channel(pl022->dma_rx_channel);
+	pl022->dma_rx_channel = NULL;
+err_no_rxchan:
+	return -ENODEV;
+}
+		
 static void terminate_dma(struct pl022 *pl022)
 {
 	struct dma_chan *rxchan = pl022->dma_rx_channel;
@@ -1167,6 +1196,11 @@ static inline int configure_dma(struct pl022 *pl022)
 	return -ENODEV;
 }
 
+static inline int pl022_dma_autoprobe(struct pl022 *pl022)
+{
+	return 0;
+}
+
 static inline int pl022_dma_probe(struct pl022 *pl022)
 {
 	return 0;
@@ -2226,8 +2260,13 @@ static int pl022_probe(struct amba_device *adev, const struct amba_id *id)
 		goto err_no_irq;
 	}
 
-	/* Get DMA channels */
-	if (platform_info->enable_dma) {
+	/* Get DMA channels, try autoconfiguration first */
+	status = pl022_dma_autoprobe(pl022);
+
+	/* If that failed, use channels from platform_info */
+	if (status == 0)
+		platform_info->enable_dma = 1;
+	else if (platform_info->enable_dma) {
 		status = pl022_dma_probe(pl022);
 		if (status != 0)
 			platform_info->enable_dma = 0;
Signature · 1 line
1.8.0
Signature · 8 lines
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
Acked-by
#6 lore raw

Re: [PATCH 2/5] spi: pl022: use generic DMA slave configuration if possible

In reply to a message not in the archive
Quoted 9 lines · Arnd Bergmann

With the new OF DMA binding, it is possible to completely avoid the need for platform_data for configuring a DMA channel. In cases where the platform has already been converted, calling dma_request_slave_channel should get all the necessary information from the device tree.

Like the patch that converts the dw_dma controller, this is completely untested and is looking for someone to try it out.

This looks correct to me atleast:

Yours,
Linus Walleij

------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb

#7 lore raw

PATCH 3/5 serial: pl011: use generic DMA slave configuration if possible

In reply to a message not in the archive

With the new OF DMA binding, it is possible to completely avoid the need for platform_data for configuring a DMA channel. In cases where the platform has already been converted, calling dma_request_slave_channel should get all the necessary information from the device tree.

Like the patch that converts the dw_dma controller, this is completely untested and is looking for someone to try it out.

Cc Grant Likely <grant.likely-s3s/[email protected]>
Cc Jiri Slaby <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc Linus Walleij <[email protected]>
Cc spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
Cc Viresh Kumar <[email protected]>
Cc Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/[email protected]>
Cc Vinod Koul <vinod.koul-VuQAYsv1563Yd54FQh9/[email protected]>
Cc devicetree-discuss-uLR06cmDAlY/[email protected]
---
 drivers/tty/serial/amba-pl011.c | 62 ++++++++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 25 deletions(-)
drivers/tty/serial/amba-pl011.c +37 -25
index 3ea5408..c25b00e 100644
@@ -245,7 +245,7 @@ static void pl011_sgbuf_free(struct dma_chan *chan, struct pl011_sgbuf *sg,
 	}
 }
 
-static void pl011_dma_probe_initcall(struct uart_amba_port *uap)
+static void pl011_dma_probe_initcall(struct device *dev, struct uart_amba_port *uap)
 {
 	/* DMA is the sole user of the platform data right now */
 	struct amba_pl011_data *plat = uap->port.dev->platform_data;
@@ -259,20 +259,25 @@ static void pl011_dma_probe_initcall(struct uart_amba_port *uap)
 	struct dma_chan *chan;
 	dma_cap_mask_t mask;
 
-	/* We need platform data */
-	if (!plat || !plat->dma_filter) {
-		dev_info(uap->port.dev, "no DMA platform data\n");
-		return;
-	}
+	chan = dma_request_slave_channel(dev, "tx");
 
-	/* Try to acquire a generic DMA engine slave TX channel */
-	dma_cap_zero(mask);
-	dma_cap_set(DMA_SLAVE, mask);
-
-	chan = dma_request_channel(mask, plat->dma_filter, plat->dma_tx_param);
 	if (!chan) {
-		dev_err(uap->port.dev, "no TX DMA channel!\n");
-		return;
+		/* We need platform data */
+		if (!plat || !plat->dma_filter) {
+			dev_info(uap->port.dev, "no DMA platform data\n");
+			return;
+		}
+
+		/* Try to acquire a generic DMA engine slave TX channel */
+		dma_cap_zero(mask);
+		dma_cap_set(DMA_SLAVE, mask);
+
+		chan = dma_request_channel(mask, plat->dma_filter,
+						plat->dma_tx_param);
+		if (!chan) {
+			dev_err(uap->port.dev, "no TX DMA channel!\n");
+			return;
+		}
 	}
 
 	dmaengine_slave_config(chan, &tx_conf);
@@ -282,7 +287,18 @@ static void pl011_dma_probe_initcall(struct uart_amba_port *uap)
 		 dma_chan_name(uap->dmatx.chan));
 
 	/* Optionally make use of an RX channel as well */
-	if (plat->dma_rx_param) {
+	chan = dma_request_slave_channel(dev, "rx");
+	
+	if (!chan && plat->dma_rx_param) {
+		chan = dma_request_channel(mask, plat->dma_filter, plat->dma_rx_param);
+
+		if (!chan) {
+			dev_err(uap->port.dev, "no RX DMA channel!\n");
+			return;
+		}
+	}
+
+	if (chan) {
 		struct dma_slave_config rx_conf = {
 			.src_addr = uap->port.mapbase + UART01x_DR,
 			.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE,
@@ -291,12 +307,6 @@ static void pl011_dma_probe_initcall(struct uart_amba_port *uap)
 			.device_fc = false,
 		};
 
-		chan = dma_request_channel(mask, plat->dma_filter, plat->dma_rx_param);
-		if (!chan) {
-			dev_err(uap->port.dev, "no RX DMA channel!\n");
-			return;
-		}
-
 		dmaengine_slave_config(chan, &rx_conf);
 		uap->dmarx.chan = chan;
 
@@ -315,6 +325,7 @@ static void pl011_dma_probe_initcall(struct uart_amba_port *uap)
 struct dma_uap {
 	struct list_head node;
 	struct uart_amba_port *uap;
+	struct device *dev;
 };
 
 static LIST_HEAD(pl011_dma_uarts);
@@ -325,7 +336,7 @@ static int __init pl011_dma_initcall(void)
 
 	list_for_each_safe(node, tmp, &pl011_dma_uarts) {
 		struct dma_uap *dmau = list_entry(node, struct dma_uap, node);
-		pl011_dma_probe_initcall(dmau->uap);
+		pl011_dma_probe_initcall(dmau->dev, dmau->uap);
 		list_del(node);
 		kfree(dmau);
 	}
@@ -334,18 +345,19 @@ static int __init pl011_dma_initcall(void)
 
 device_initcall(pl011_dma_initcall);
 
-static void pl011_dma_probe(struct uart_amba_port *uap)
+static void pl011_dma_probe(struct device *dev, struct uart_amba_port *uap)
 {
 	struct dma_uap *dmau = kzalloc(sizeof(struct dma_uap), GFP_KERNEL);
 	if (dmau) {
 		dmau->uap = uap;
+		dmau->dev = dev;
 		list_add_tail(&dmau->node, &pl011_dma_uarts);
 	}
 }
 #else
-static void pl011_dma_probe(struct uart_amba_port *uap)
+static void pl011_dma_probe(struct device *dev, struct uart_amba_port *uap)
 {
-	pl011_dma_probe_initcall(uap);
+	pl011_dma_probe_initcall(dev, uap);
 }
 #endif
 
@@ -2020,7 +2032,7 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
 	uap->port.ops = &amba_pl011_pops;
 	uap->port.flags = UPF_BOOT_AUTOCONF;
 	uap->port.line = i;
-	pl011_dma_probe(uap);
+	pl011_dma_probe(&dev->dev, uap);
 
 	/* Ensure interrupts from this UART are masked and cleared */
 	writew(0, uap->port.membase + UART011_IMSC);
Signature · 9 lines
1.8.0


------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnnow-d2d

A message that is not in the archive (sent off-list or not yet received): 1359395857-1235-4-git-send-email-arnd-r2nGTMty4D4@public.gmane.org

Acked-by
#9 lore raw

Re: [PATCH 3/5] serial: pl011: use generic DMA slave configuration if possible

Quoted 8 lines · Arnd Bergmann

With the new OF DMA binding, it is possible to completely avoid the need for platform_data for configuring a DMA channel. In cases where the platform has already been converted, calling dma_request_slave_channel should get all the necessary information from the device tree.

Like the patch that converts the dw_dma controller, this is completely untested and is looking for someone to try it out.

Quoted 11 lines
Cc Grant Likely <grant.likely-s3s/[email protected]>
Cc Jiri Slaby <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc Linus Walleij <[email protected]>
Cc spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
Cc Viresh Kumar <[email protected]>
Cc Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/[email protected]>
Cc Vinod Koul <vinod.koul-VuQAYsv1563Yd54FQh9/[email protected]>
Cc devicetree-discuss-uLR06cmDAlY/[email protected]
Quoted 140 lines
---
 drivers/tty/serial/amba-pl011.c | 62 ++++++++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 25 deletions(-)
drivers/tty/serial/amba-pl011.c +37 -25
index 3ea5408..c25b00e 100644
@@ -245,7 +245,7 @@ static void pl011_sgbuf_free(struct dma_chan *chan, struct pl011_sgbuf *sg,
 	}
 }
 
-static void pl011_dma_probe_initcall(struct uart_amba_port *uap)
+static void pl011_dma_probe_initcall(struct device *dev, struct uart_amba_port *uap)
 {
 	/* DMA is the sole user of the platform data right now */
 	struct amba_pl011_data *plat = uap->port.dev->platform_data;
@@ -259,20 +259,25 @@ static void pl011_dma_probe_initcall(struct uart_amba_port *uap)
 	struct dma_chan *chan;
 	dma_cap_mask_t mask;
 
-	/* We need platform data */
-	if (!plat || !plat->dma_filter) {
-		dev_info(uap->port.dev, "no DMA platform data\n");
-		return;
-	}
+	chan = dma_request_slave_channel(dev, "tx");
 
-	/* Try to acquire a generic DMA engine slave TX channel */
-	dma_cap_zero(mask);
-	dma_cap_set(DMA_SLAVE, mask);
-
-	chan = dma_request_channel(mask, plat->dma_filter, plat->dma_tx_param);
 	if (!chan) {
-		dev_err(uap->port.dev, "no TX DMA channel!\n");
-		return;
+		/* We need platform data */
+		if (!plat || !plat->dma_filter) {
+			dev_info(uap->port.dev, "no DMA platform data\n");
+			return;
+		}
+
+		/* Try to acquire a generic DMA engine slave TX channel */
+		dma_cap_zero(mask);
+		dma_cap_set(DMA_SLAVE, mask);
+
+		chan = dma_request_channel(mask, plat->dma_filter,
+						plat->dma_tx_param);
+		if (!chan) {
+			dev_err(uap->port.dev, "no TX DMA channel!\n");
+			return;
+		}
 	}
 
 	dmaengine_slave_config(chan, &tx_conf);
@@ -282,7 +287,18 @@ static void pl011_dma_probe_initcall(struct uart_amba_port *uap)
 		 dma_chan_name(uap->dmatx.chan));
 
 	/* Optionally make use of an RX channel as well */
-	if (plat->dma_rx_param) {
+	chan = dma_request_slave_channel(dev, "rx");
+	
+	if (!chan && plat->dma_rx_param) {
+		chan = dma_request_channel(mask, plat->dma_filter, plat->dma_rx_param);
+
+		if (!chan) {
+			dev_err(uap->port.dev, "no RX DMA channel!\n");
+			return;
+		}
+	}
+
+	if (chan) {
 		struct dma_slave_config rx_conf = {
 			.src_addr = uap->port.mapbase + UART01x_DR,
 			.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE,
@@ -291,12 +307,6 @@ static void pl011_dma_probe_initcall(struct uart_amba_port *uap)
 			.device_fc = false,
 		};
 
-		chan = dma_request_channel(mask, plat->dma_filter, plat->dma_rx_param);
-		if (!chan) {
-			dev_err(uap->port.dev, "no RX DMA channel!\n");
-			return;
-		}
-
 		dmaengine_slave_config(chan, &rx_conf);
 		uap->dmarx.chan = chan;
 
@@ -315,6 +325,7 @@ static void pl011_dma_probe_initcall(struct uart_amba_port *uap)
 struct dma_uap {
 	struct list_head node;
 	struct uart_amba_port *uap;
+	struct device *dev;
 };
 
 static LIST_HEAD(pl011_dma_uarts);
@@ -325,7 +336,7 @@ static int __init pl011_dma_initcall(void)
 
 	list_for_each_safe(node, tmp, &pl011_dma_uarts) {
 		struct dma_uap *dmau = list_entry(node, struct dma_uap, node);
-		pl011_dma_probe_initcall(dmau->uap);
+		pl011_dma_probe_initcall(dmau->dev, dmau->uap);
 		list_del(node);
 		kfree(dmau);
 	}
@@ -334,18 +345,19 @@ static int __init pl011_dma_initcall(void)
 
 device_initcall(pl011_dma_initcall);
 
-static void pl011_dma_probe(struct uart_amba_port *uap)
+static void pl011_dma_probe(struct device *dev, struct uart_amba_port *uap)
 {
 	struct dma_uap *dmau = kzalloc(sizeof(struct dma_uap), GFP_KERNEL);
 	if (dmau) {
 		dmau->uap = uap;
+		dmau->dev = dev;
 		list_add_tail(&dmau->node, &pl011_dma_uarts);
 	}
 }
 #else
-static void pl011_dma_probe(struct uart_amba_port *uap)
+static void pl011_dma_probe(struct device *dev, struct uart_amba_port *uap)
 {
-	pl011_dma_probe_initcall(uap);
+	pl011_dma_probe_initcall(dev, uap);
 }
 #endif
 
@@ -2020,7 +2032,7 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
 	uap->port.ops = &amba_pl011_pops;
 	uap->port.flags = UPF_BOOT_AUTOCONF;
 	uap->port.line = i;
-	pl011_dma_probe(uap);
+	pl011_dma_probe(&dev->dev, uap);
 
 	/* Ensure interrupts from this UART are masked and cleared */
 	writew(0, uap->port.membase + UART011_IMSC);
Signature · 1 line
1.8.0
Signature · 2 lines
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.
#10 lore raw

PATCH 4/5 ata: arasan: remove the need for platform_data

In reply to a message not in the archive

This adds a complete DT binding for the arasan device driver. There is currently only one user, which is the spear13xx platform, so we don't actually have to parse all the properties until another user comes in, but this does use the generic DMA binding to find the DMA channel.

The patch is untested so far and is part of a series to convert the spear platform over to use the generic DMA binding, so it should stay with the rest of the series.

Cc Viresh Kumar <[email protected]>
Cc Vinod Koul <[email protected]>
Cc Jeff Garzik <[email protected]>
Cc devicetree-discuss-uLR06cmDAlY/[email protected]
---
 .../devicetree/bindings/ata/pata-arasan.txt        | 22 +++++++++++++++
 drivers/ata/pata_arasan_cf.c                       | 31 +++++++++++++---------
 include/linux/pata_arasan_cf_data.h                |  2 --
 3 files changed, 41 insertions(+), 14 deletions(-)
Documentation/devicetree/bindings/ata/pata-arasan.txt +22 -0
index 95ec7f8..2aff154 100644
@@ -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 CF interrupt number
+- clock-frequency: Interface clock rate, in Hz, one of
+       25000000
+       33000000
+       40000000
+       50000000
+       66000000
+       75000000
+      100000000
+      125000000
+      150000000
+      166000000
+      200000000
+
+Optional properties:
+- arasan,broken-udma: if present, UDMA mode is unusable
+- arasan,broken-mwdma: if present, MWDMA mode is unusable
+- arasan,broken-pio: if present, PIO mode is unusable
+- dmas: one DMA channel, as described in bindings/dma/dma.txt
+  required unless both UDMA and MWDMA mode are broken
+- dma-names: the corresponding channel name, must be "data"
 
 Example:
 
@@ -14,4 +34,6 @@ Example:
 		reg = <0xfc000000 0x1000>;
 		interrupt-parent = <&vic1>;
 		interrupts = <12>;
+		dmas = <&dma-controller 23>;
+		dma-names = "data";
 	};
drivers/ata/pata_arasan_cf.c +19 -12
index 405022d..42d3195 100644
@@ -209,8 +209,6 @@ struct arasan_cf_dev {
 	struct dma_chan *dma_chan;
 	/* Mask for DMA transfers */
 	dma_cap_mask_t mask;
-	/* dma channel private data */
-	void *dma_priv;
 	/* DMA transfer work */
 	struct work_struct work;
 	/* DMA delayed finish work */
@@ -308,6 +306,7 @@ static void cf_card_detect(struct arasan_cf_dev *acdev, bool hotplugged)
 static int cf_init(struct arasan_cf_dev *acdev)
 {
 	struct arasan_cf_pdata *pdata = dev_get_platdata(acdev->host->dev);
+	unsigned int if_clk;
 	unsigned long flags;
 	int ret = 0;
 
@@ -325,8 +324,12 @@ static int cf_init(struct arasan_cf_dev *acdev)
 
 	spin_lock_irqsave(&acdev->host->lock, flags);
 	/* configure CF interface clock */
-	writel((pdata->cf_if_clk <= CF_IF_CLK_200M) ? pdata->cf_if_clk :
-			CF_IF_CLK_166M, acdev->vbase + CLK_CFG);
+	/* TODO: read from device tree */
+	if_clk = CF_IF_CLK_166M;
+	if (pdata && pdata->cf_if_clk <= CF_IF_CLK_200M)
+		if_clk = pdata->cf_if_clk;
+
+	writel(if_clk, acdev->vbase + CLK_CFG);
 
 	writel(TRUE_IDE_MODE | CFHOST_ENB, acdev->vbase + OP_MODE);
 	cf_interrupt_enable(acdev, CARD_DETECT_IRQ, 1);
@@ -530,8 +533,7 @@ static void data_xfer(struct work_struct *work)
 
 	/* request dma channels */
 	/* dma_request_channel may sleep, so calling from process context */
-	acdev->dma_chan = dma_request_channel(acdev->mask, filter,
-			acdev->dma_priv);
+	acdev->dma_chan = dma_request_slave_channel(acdev->dev, "data");
 	if (!acdev->dma_chan) {
 		dev_err(acdev->host->dev, "Unable to get dma_chan\n");
 		goto chan_request_fail;
@@ -798,6 +800,7 @@ static int arasan_cf_probe(struct platform_device *pdev)
 	struct ata_host *host;
 	struct ata_port *ap;
 	struct resource *res;
+	u32 quirk;
 	irq_handler_t irq_handler = NULL;
 	int ret = 0;
 
@@ -817,12 +820,17 @@ static int arasan_cf_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	if (pdata)
+		quirk = pdata->quirk;
+	else
+		quirk = CF_BROKEN_UDMA; /* as it is on spear1340 */
+
 	/* if irq is 0, support only PIO */
 	acdev->irq = platform_get_irq(pdev, 0);
 	if (acdev->irq)
 		irq_handler = arasan_cf_interrupt;
 	else
-		pdata->quirk |= CF_BROKEN_MWDMA | CF_BROKEN_UDMA;
+		quirk |= CF_BROKEN_MWDMA | CF_BROKEN_UDMA;
 
 	acdev->pbase = res->start;
 	acdev->vbase = devm_ioremap_nocache(&pdev->dev, res->start,
@@ -859,17 +867,16 @@ static int arasan_cf_probe(struct platform_device *pdev)
 	INIT_WORK(&acdev->work, data_xfer);
 	INIT_DELAYED_WORK(&acdev->dwork, delayed_finish);
 	dma_cap_set(DMA_MEMCPY, acdev->mask);
-	acdev->dma_priv = pdata->dma_priv;
 
 	/* Handle platform specific quirks */
-	if (pdata->quirk) {
-		if (pdata->quirk & CF_BROKEN_PIO) {
+	if (quirk) {
+		if (quirk & CF_BROKEN_PIO) {
 			ap->ops->set_piomode = NULL;
 			ap->pio_mask = 0;
 		}
-		if (pdata->quirk & CF_BROKEN_MWDMA)
+		if (quirk & CF_BROKEN_MWDMA)
 			ap->mwdma_mask = 0;
-		if (pdata->quirk & CF_BROKEN_UDMA)
+		if (quirk & CF_BROKEN_UDMA)
 			ap->udma_mask = 0;
 	}
 	ap->flags |= ATA_FLAG_PIO_POLLING | ATA_FLAG_NO_ATAPI;
include/linux/pata_arasan_cf_data.h +0 -2
index a7b4fc3..3cc21c9 100644
@@ -37,8 +37,6 @@ struct arasan_cf_pdata {
 	#define CF_BROKEN_PIO			(1)
 	#define CF_BROKEN_MWDMA			(1 << 1)
 	#define CF_BROKEN_UDMA			(1 << 2)
-	/* This is platform specific data for the DMA controller */
-	void *dma_priv;
 };
 
 static inline void
Signature · 1 line
1.8.0
#11 lore raw

PATCH 5/5 ARM: SPEAr13xx: Pass generic DW DMAC platform data from DT

In reply to a message not in the archive

This replaces an earlier patch from Viresh Kumar to move the spear platform over to the generic DMA binding. This version is now based on the merged multiplatform capable spear platform, rather than the separate spear13xx/3xx/6xx directories.

Cc Viresh Kumar <[email protected]>
Cc Vinod Koul <vinod.koul-VuQAYsv1563Yd54FQh9/[email protected]>
Cc Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/[email protected]>
Cc devicetree-discuss-uLR06cmDAlY/[email protected]
---
 arch/arm/boot/dts/spear1340.dtsi         |   2 +
 arch/arm/boot/dts/spear13xx.dtsi         |  25 +++++-
 arch/arm/mach-spear/generic.h            |   6 --
 arch/arm/mach-spear/include/mach/spear.h |   2 -
 arch/arm/mach-spear/spear1310.c          |  30 +-------
 arch/arm/mach-spear/spear1340.c          |  32 +-------
 arch/arm/mach-spear/spear13xx-dma.h      | 128 -------------------------------
 arch/arm/mach-spear/spear13xx.c          |  58 --------------
 8 files changed, 29 insertions(+), 254 deletions(-)
 delete mode 100644 arch/arm/mach-spear/spear13xx-dma.h
arch/arm/boot/dts/spear1340.dtsi +2 -0
index 34da11a..4d63289 100644
@@ -113,6 +113,8 @@
 				reg = <0xb4100000 0x1000>;
 				interrupts = <0 105 0x4>;
 				status = "disabled";
+				dmas = <&dwdma 0xc 0 1>,
+					<&dwdma 0xd 1 0>;
 			};
 
 			thermal@e07008c4 {
arch/arm/boot/dts/spear13xx.dtsi +24 -1
index b4ca60f..f8f22ce 100644
@@ -98,6 +98,8 @@
 			reg = <0xb2800000 0x1000>;
 			interrupts = <0 29 0x4>;
 			status = "disabled";
+			dmas = <&dwdma 0 0 0>;
+			dma-names = "data";
 		};
 
 		dma@ea800000 {
@@ -105,13 +107,31 @@
 			reg = <0xea800000 0x1000>;
 			interrupts = <0 19 0x4>;
 			status = "disabled";
+
+			dma-channels = <8>;
+			#dma-cells = <3>;
+			dma-requests = <32>;
+			chan_allocation_order = <1>;
+			chan_priority = <1>;
+			block_size = <0xfff>;
+			dma-masters = <2>;
+			data_width = <3 3 0 0>;
 		};
 
-		dma@eb000000 {
+		dwdma: dma@eb000000 {
 			compatible = "snps,dma-spear1340";
 			reg = <0xeb000000 0x1000>;
 			interrupts = <0 59 0x4>;
 			status = "disabled";
+
+			dma-requests = <32>;
+			dma-channels = <8>;
+			dma-masters = <2>;
+			#dma-cells = <3>;
+			chan_allocation_order = <1>;
+			chan_priority = <1>;
+			block_size = <0xfff>;
+			data_width = <3 3 0 0>;
 		};
 
 		fsmc: flash@b0000000 {
@@ -261,6 +281,9 @@
 				#size-cells = <0>;
 				interrupts = <0 31 0x4>;
 				status = "disabled";
+				dmas = <&dwdma 11 0 0>,
+					<&dwdma 7 0 0>;
+				dma-names = "tx", "rx";
 			};
 
 			rtc@e0580000 {
arch/arm/mach-spear/generic.h +0 -6
index 8ba7e75..69c38d2 100644
@@ -20,13 +20,7 @@
 
 extern void spear13xx_timer_init(void);
 extern void spear3xx_timer_init(void);
-extern struct pl022_ssp_controller pl022_plat_data;
 extern struct pl08x_platform_data pl080_plat_data;
-extern struct dw_dma_platform_data dmac_plat_data;
-extern struct dw_dma_slave cf_dma_priv;
-extern struct dw_dma_slave nand_read_dma_priv;
-extern struct dw_dma_slave nand_write_dma_priv;
-bool dw_dma_filter(struct dma_chan *chan, void *slave);
 
 void __init spear_setup_of_timer(void);
 void __init spear3xx_clk_init(void __iomem *misc_base,
arch/arm/mach-spear/include/mach/spear.h +0 -2
index 374ddc3..cf3a536 100644
@@ -82,8 +82,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)
 
 /* Debug uart for linux, will be used for debug and uncompress messages */
arch/arm/mach-spear/spear1310.c +1 -29
index ed3b5c2..9eaac2c 100644
@@ -23,40 +23,12 @@
 #include <mach/spear.h>
 
 /* Base addresses */
-#define SPEAR1310_SSP1_BASE			UL(0x5D400000)
-#define SPEAR1310_SATA0_BASE			UL(0xB1000000)
-#define SPEAR1310_SATA1_BASE			UL(0xB1800000)
-#define SPEAR1310_SATA2_BASE			UL(0xB4000000)
-
 #define SPEAR1310_RAS_GRP1_BASE			UL(0xD8000000)
 #define VA_SPEAR1310_RAS_GRP1_BASE		UL(0xFA000000)
 
-static struct arasan_cf_pdata cf_pdata = {
-	.cf_if_clk = CF_IF_CLK_166M,
-	.quirk = CF_BROKEN_UDMA,
-	.dma_priv = &cf_dma_priv,
-};
-
-/* ssp device registration */
-static struct pl022_ssp_controller ssp1_plat_data = {
-	.enable_dma = 0,
-};
-
-/* 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_pdata),
-	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("arm,pl022", SSP_BASE, NULL, &pl022_plat_data),
-
-	OF_DEV_AUXDATA("arm,pl022", SPEAR1310_SSP1_BASE, NULL, &ssp1_plat_data),
-	{}
-};
-
 static void __init spear1310_dt_init(void)
 {
-	of_platform_populate(NULL, of_default_bus_match_table,
-			spear1310_auxdata_lookup, NULL);
+	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 }
 
 static const char * const spear1310_dt_board_compat[] = {
arch/arm/mach-spear/spear1340.c +2 -30
index df17863..0f361ee 100644
@@ -16,7 +16,6 @@
 #include <linux/ahci_platform.h>
 #include <linux/amba/serial.h>
 #include <linux/delay.h>
-#include <linux/dw_dmac.h>
 #include <linux/of_platform.h>
 #include <linux/pata_arasan_cf_data.h>
 #include <linux/irqchip.h>
@@ -26,9 +25,10 @@
 
 #include "spear13xx-dma.h"
 
+/* FIXME: Move SATA PHY code into a standalone driver */
+
 /* Base addresses */
 #define SPEAR1340_SATA_BASE			UL(0xB1000000)
-#define SPEAR1340_UART1_BASE			UL(0xB4100000)
 
 /* Power Management Registers */
 #define SPEAR1340_PCM_CFG			(VA_MISC_BASE + 0x100)
@@ -80,28 +80,6 @@
 			(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],
-};
-
 /* SATA device registration */
 static int sata_miphy_init(struct device *dev, void __iomem *addr)
 {
@@ -160,14 +138,8 @@ 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("arm,pl022", SSP_BASE, NULL, &pl022_plat_data),
-
 	OF_DEV_AUXDATA("snps,spear-ahci", SPEAR1340_SATA_BASE, NULL,
 			&sata_pdata),
-	OF_DEV_AUXDATA("arm,pl011", SPEAR1340_UART1_BASE, NULL, &uart1_data),
 	{}
 };
 
arch/arm/mach-spear/spear13xx-dma.h deleted +0 -128

128 lines not on this page · raw message

arch/arm/mach-spear/spear13xx.c +0 -58

70 lines not on this page · raw message

Signature · 1 line
1.8.0
#12 lore raw
In reply to a message not in the archive
Quoted 7 lines · Arnd Bergmann

Hi everyone,

This is my attempt to convert the spear platform and the dw_dmac to the generic device tree binding for DMA, so that we don't get a release with the broken version. I'm pretty sure that this has bugs, but it's as good as I could do without access to hardware or specs.

I noticed now that I forgot to put the arch/arm changes into the for-next branch for 3.9. I'm putting it into late/cleanup now, as this is a cleanup removing unnecessary definitions from spear13xx.

All the patches for the drivers/dma were already merged by Vinod in 3.9, so this has no impact on his subsystem.

Arnd

#13 lore raw

v2 cover dmaengine: convert dw_dmac/spear13xx to generic binding

In reply to Arnd Bergmann

Hi everyone,

I'm rather embarrassed to have sent yet another patch series to the wrong mailing list address, this now goes to the correct linux-arm-kernel list, so please comment here, not on the first version. I have also made some smaller changes and updated the DT bindings where I extended the drivers. I also uploaded the git branch to the spear/dma branch of
git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git

This is my attempt to convert the spear platform and the dw_dmac to the generic device tree binding for DMA, so that we don't get a release with the broken version. I'm pretty sure that this has bugs, but it's as good as I could do without access to hardware or specs.

Please review and comment,

Arnd

Arnd Bergmann (5):
dmaengine: dw_dmac: move to generic DMA binding
spi: pl022: use generic DMA slave configuration if possible
serial: pl011: use generic DMA slave configuration if possible
ata: arasan: remove the need for platform_data
ARM: SPEAr13xx: Pass generic DW DMAC platform data from DT

 .../devicetree/bindings/ata/pata-arasan.txt        |  22 ++++
 Documentation/devicetree/bindings/dma/snps-dma.txt |  70 +++++------
 arch/arm/boot/dts/spear1340.dtsi                   |   2 +
 arch/arm/boot/dts/spear13xx.dtsi                   |  25 +++-
 arch/arm/mach-spear/generic.h                      |   6 -
 arch/arm/mach-spear/include/mach/spear.h           |   2 -
 arch/arm/mach-spear/spear1310.c                    |  30 +----
 arch/arm/mach-spear/spear1340.c                    |  32 +----
 arch/arm/mach-spear/spear13xx-dma.h                | 128 --------------------
 arch/arm/mach-spear/spear13xx.c                    |  58 ---------
 drivers/ata/pata_arasan_cf.c                       |  31 +++--
 drivers/dma/dw_dmac.c                              | 130 ++++++++++-----------
 drivers/dma/dw_dmac_regs.h                         |   4 -
 drivers/spi/spi-pl022.c                            |  43 ++++++-
 drivers/tty/serial/amba-pl011.c                    |  62 ++++++----
 include/linux/dw_dmac.h                            |   5 -
 include/linux/pata_arasan_cf_data.h                |   2 -
 17 files changed, 243 insertions(+), 409 deletions(-)
 delete mode 100644 arch/arm/mach-spear/spear13xx-dma.h
Signature · 15 lines
1.8.0

Cc: Andy Shevchenko <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jeff Garzik <[email protected]>
Cc: Jon Hunter <[email protected]>
Cc: Jiri Slaby <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: Mark Brown <[email protected]>
Cc: Vinod Koul <[email protected]>
Cc: Viresh Kumar <[email protected]>
Cc: devicetree-discuss at lists.ozlabs.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: spi-devel-general at lists.sourceforge.net
#14 lore raw

PATCH v2 1/5 dmaengine: dw_dmac: move to generic DMA binding

The original device tree binding for this driver, from Viresh Kumar unfortunately conflicted with the generic DMA binding, and did not allow to completely seperate slave device configuration from the controller.

This is an attempt to replace it with an implementation of the generic binding, but it is currently completely untested, because I do not have any hardware with this particular controller.

The patch applies on top of linux-next, which contains both the base support for the generic DMA binding, as well as the earlier attempt from Viresh. Both of these are currently not merged upstream however.

There are a couple of TODO items that are left remaining and are open for ideas from other people.

Cc Viresh Kumar <[email protected]>
Cc Andy Shevchenko <[email protected]>
Cc Vinod Koul <[email protected]>
Cc devicetree-discuss at lists.ozlabs.org
Cc linux-arm-kernel at lists.infradead.org
---
 Documentation/devicetree/bindings/dma/snps-dma.txt |  71 ++++++-----
 drivers/dma/dw_dmac.c                              | 137 ++++++++++-----------
 drivers/dma/dw_dmac_regs.h                         |   4 -
 include/linux/dw_dmac.h                            |   5 -
 4 files changed, 102 insertions(+), 115 deletions(-)
Documentation/devicetree/bindings/dma/snps-dma.txt +37 -34
index 5bb3dfb..212d387 100644
@@ -3,59 +3,62 @@
 Required properties:
 - compatible: "snps,dma-spear1340"
 - reg: Address range of the DMAC registers
-- 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.
+- dma-channels: Number of channels supported by hardware
+- dma-requests: Number of DMA request lines supported
+- dma-masters: Number of AHB masters supported by the controller
+- #dma-cells: must be <3>
 - 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.
+
+
+Optional properties:
+- interrupt-parent: Should be the phandle for the interrupt controller
+  that services interrupts for this device
+- is_private: The device channels should be marked as private and not for by the
+  general purpose DMA channel allocator. False if not passed.
 
 Example:
 
-	dma at fc000000 {
+	dmahost: dma at fc000000 {
 		compatible = "snps,dma-spear1340";
 		reg = <0xfc000000 0x1000>;
 		interrupt-parent = <&vic1>;
 		interrupts = <12>;
 
-		nr_channels = <8>;
+		dma-channels = <8>;
+		dma-requests = <32>;
+		dma-masters = <2>;
+		#dma-cells = <3>;
 		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>;
-			};
-		};
+DMA clients connected to the Designware DMA controller must use the format
+described in the dma.txt file, using a five-cell specifier for each channel.
+The five cells in order are:
+
+1. A phandle pointing to the DMA controller
+2. The value for the cfg_hi register.
+3. The value for the cfg_lo register.
+4. Source master for transfers on allocated channel.
+5. Destination master for transfers on allocated channel.
+
+Example:
+	
+	serial at e0000000 {
+		compatible = "arm,pl011", "arm,primecell";
+		reg = <0xe0000000 0x1000>;
+		interrupts = <0 35 0x4>;
+		status = "disabled";
+		dmas = <&dmahost 0x6000 0 0 1>,
+			<&dmahost 0x680 0 1 0>;
+		dma-names = "rx", "rx";
 	};
drivers/dma/dw_dmac.c +65 -72

200 lines not on this page · raw message

drivers/dma/dw_dmac_regs.h +0 -4
index 88a069f..8896559 100644
@@ -239,10 +239,6 @@ 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 */
include/linux/dw_dmac.h +0 -5
index 41766de..481ab23 100644
@@ -27,7 +27,6 @@
  */
 struct dw_dma_slave {
 	struct device		*dma_dev;
-	const char		*bus_id;
 	u32			cfg_hi;
 	u32			cfg_lo;
 	u8			src_master;
@@ -60,9 +59,6 @@ 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 */
@@ -114,6 +110,5 @@ 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 */
Signature · 1 line
1.8.0
#15 lore raw

PATCH 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 11 lines · Arnd Bergmann

The original device tree binding for this driver, from Viresh Kumar unfortunately conflicted with the generic DMA binding, and did not allow to completely seperate slave device configuration from the controller.

This is an attempt to replace it with an implementation of the generic binding, but it is currently completely untested, because I do not have any hardware with this particular controller.

The patch applies on top of linux-next, which contains both the base support for the generic DMA binding, as well as the earlier attempt from Viresh. Both of these are currently not merged upstream however.

This was really my work and i am feeling bad that i couldn't allocate any time for it. Thanks for starting it.

Quoted 8 lines
Documentation/devicetree/bindings/dma/snps-dma.txt +0 -0
index 5bb3dfb..212d387 100644
@@ -3,59 +3,62 @@
 Required properties:
 - compatible: "snps,dma-spear1340"
 - reg: Address range of the DMAC registers

Documentation/devicetree/bindings/dma/snps-dma.txt line 10

Quoted 10 lines
Documentation/devicetree/bindings/dma/snps-dma.txt +4 -5
-- 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.
+- dma-channels: Number of channels supported by hardware
+- dma-requests: Number of DMA request lines supported
+- dma-masters: Number of AHB masters supported by the controller
+- #dma-cells: must be <3>

Shouldn't this be 4? Would be better to mention what fields are these, right here.
I have seen them below though.

Quoted 23 lines
Documentation/devicetree/bindings/dma/snps-dma.txt +9 -0
+DMA clients connected to the Designware DMA controller must use the format
+described in the dma.txt file, using a five-cell specifier for each channel.
+The five cells in order are:
+
+1. A phandle pointing to the DMA controller
+2. The value for the cfg_hi register.
+3. The value for the cfg_lo register.
+4. Source master for transfers on allocated channel.
+5. Destination master for transfers on allocated channel.
 
drivers/dma/dw_dmac.c +1 -0
index 8cfaaf8..88504c2 100644
@@ -18,6 +18,7 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/of.h>
+#include <linux/of_dma.h>
 #include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>

drivers/dma/dw_dmac.c static void dwc_free_chan_resources(struct dma_chan *chan) · line 1184

Quoted 6 lines
drivers/dma/dw_dmac.c +2 -1
@@ -1179,49 +1180,69 @@ 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)
+/* forward declaration used in filter */
+static struct platform_driver dw_driver;

extern? This is not a declaration but definition.

drivers/dma/dw_dmac.c static void dwc_free_chan_resources(struct dma_chan *chan) · line 1190

Quoted 6 lines
drivers/dma/dw_dmac.c +6 -0
+
+struct dw_dma_filter_args {
+       struct dw_dma *dw;
+       u32 cfg_lo;
+       u32 cfg_hi;
+       unsigned src;

Currently named as sms

drivers/dma/dw_dmac.c static void dwc_free_chan_resources(struct dma_chan *chan) · line 1191

Quoted 1 line
drivers/dma/dw_dmac.c +1 -0
+       unsigned dst;

dms

Quoted 15 lines
drivers/dma/dw_dmac.c +5 -7
+};
+
+static 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;
+       struct dw_dma_filter_args *fargs = param;
+       struct dw_dma_slave *sd;
 
-       /*
-        * 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

drivers/dma/dw_dmac.c static void dwc_free_chan_resources(struct dma_chan *chan) · line 1202

Quoted 10 lines
drivers/dma/dw_dmac.c +3 -7
-        * 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))
+       /* both the driver and the device must match */
+        if (chan->device->dev->driver != &dw_driver.driver)
+                return false;

Can this ever happen? Isn't it the case that this routine would be called only for dw_dmac?

Quoted 8 lines
drivers/dma/dw_dmac.c +1 -6
+       if (dw != fargs->dw)
                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
-        */

drivers/dma/dw_dmac.c static void dwc_free_chan_resources(struct dma_chan *chan) · line 1207

Quoted 10 lines
drivers/dma/dw_dmac.c +2 -7
-       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;
+       /* FIXME: memory leak! could we put this into dw_dma_chan? */
+       sd = devm_kzalloc(dw->dma.dev, sizeof (*sd), GFP_KERNEL);

Yes.

Quoted 20 lines
drivers/dma/dw_dmac.c +12 -6
+       if (!sd)
+               return false;
 
-                       return true;
-               }
-       }
+       sd->dma_dev     = dw->dma.dev;
+       sd->cfg_hi      = fargs->cfg_hi;
+       sd->cfg_lo      = fargs->cfg_lo;
+       sd->src_master  = fargs->src;
+       sd->dst_master  = fargs->dst;
+
+       chan->private = sd;
 
-       last_dw = dw;
-       last_bus_id = param;
-       return false;
+       return true;
+}
+

drivers/dma/dw_dmac.c static void dwc_free_chan_resources(struct dma_chan *chan) · line 1231

Quoted 10 lines
drivers/dma/dw_dmac.c +10 -0
+static struct dma_chan *dw_dma_xlate(struct of_phandle_args *dma_spec,
+                                        struct of_dma *ofdma)
+{
+       struct dw_dma *dw = ofdma->of_dma_data;
+       struct dw_dma_filter_args fargs = {
+               .dw = dw,
+       };
+       dma_cap_mask_t cap;
+
+       if (dma_spec->args_count != 4)

args_count contains count of all params leaving the phandle?

drivers/dma/dw_dmac.c static void dwc_free_chan_resources(struct dma_chan *chan) · line 1235

Quoted 4 lines
drivers/dma/dw_dmac.c +4 -0
+               return NULL;
+
+       /* FIXME: This binding is rather clumsy. Can't we use the
+          request line numbers here instead? */

yes.

drivers/dma/dw_dmac.c static void dwc_free_chan_resources(struct dma_chan *chan) · line 1244

Quoted 9 lines
drivers/dma/dw_dmac.c +9 -0
+       fargs.cfg_lo = be32_to_cpup(dma_spec->args+0);
+       fargs.cfg_hi = be32_to_cpup(dma_spec->args+1);
+       fargs.src = be32_to_cpup(dma_spec->args+2);
+       fargs.dst = be32_to_cpup(dma_spec->args+3);
+
+       dma_cap_zero(cap);
+       dma_cap_set(DMA_SLAVE, cap);
+       /* FIXME: there should be a simpler way to do this */
+       return dma_request_channel(cap, dw_dma_generic_filter, &dma_spec->args[0]);

don't you need to send &fargs as the last argument?

#16 lore raw

PATCH v2 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 13 lines · Viresh Kumar
Quoted 13 lines
Documentation/devicetree/bindings/dma/snps-dma.txt +0 -4
index 5bb3dfb..212d387 100644
@@ -3,59 +3,62 @@
 Required properties:
 - compatible: "snps,dma-spear1340"
 - reg: Address range of the DMAC registers
-- 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
Quoted 8 lines

Documentation/devicetree/bindings/dma/snps-dma.txt line 10

Quoted 5 lines
Documentation/devicetree/bindings/dma/snps-dma.txt +4 -1
-  general purpose DMA channel allocator. False if not passed.
+- dma-channels: Number of channels supported by hardware
+- dma-requests: Number of DMA request lines supported
+- dma-masters: Number of AHB masters supported by the controller
+- #dma-cells: must be <3>

Shouldn't this be 4? Would be better to mention what fields are these, right here. I have seen them below though.

Correct. I changed these a couple of times while trying to understand what the fields are, and I missed this instance. I'm still not sure whether we actually need all four fields, or what the simplest format for them would be. This just mirrors what you had in your binding.

Quoted 5 lines

Documentation/devicetree/bindings/dma/snps-dma.txt line 12

Quoted 3 lines
Documentation/devicetree/bindings/dma/snps-dma.txt +2 -1
-bool dw_dma_generic_filter(struct dma_chan *chan, void *param)
+/* forward declaration used in filter */
+static struct platform_driver dw_driver;

extern? This is not a declaration but definition.

No. You can have multiple declarations for a static symbol like this, but only one of them with an initilizer. I usually recommend against doing this myself, because it's confusing and somewhat bad style, but it is correct C.

Quoted 15 lines
Quoted 4 lines
Documentation/devicetree/bindings/dma/snps-dma.txt +0 -4
-       /*
-        * 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

Documentation/devicetree/bindings/dma/snps-dma.txt line 15

Quoted 10 lines
Documentation/devicetree/bindings/dma/snps-dma.txt +3 -7
-        * 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))
+       /* both the driver and the device must match */
+        if (chan->device->dev->driver != &dw_driver.driver)
+                return false;
Quoted 2 lines

Can this ever happen? Isn't it the case that this routine would be called only for dw_dmac?

I think not. AFAIK the filter function will be called for each channel on each DMA engine until one of them matches.

Quoted 9 lines

Documentation/devicetree/bindings/dma/snps-dma.txt line 17

Quoted 7 lines
Documentation/devicetree/bindings/dma/snps-dma.txt +2 -5
-       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;
+       /* FIXME: memory leak! could we put this into dw_dma_chan? */
+       sd = devm_kzalloc(dw->dma.dev, sizeof (*sd), GFP_KERNEL);

Yes.

Yes it can be in dw_dma_chan or yes it is a memory leak?

Quoted 4 lines
Quoted 4 lines
Documentation/devicetree/bindings/dma/snps-dma.txt +4 -0
+static struct dma_chan *dw_dma_xlate(struct of_phandle_args *dma_spec,
+                                        struct of_dma *ofdma)
+{
+       struct dw_dma *dw = ofdma->of_dma_data;
Quoted 8 lines

Documentation/devicetree/bindings/dma/snps-dma.txt line 27

Quoted 6 lines
Documentation/devicetree/bindings/dma/snps-dma.txt +6 -0
+       struct dw_dma_filter_args fargs = {
+               .dw = dw,
+       };
+       dma_cap_mask_t cap;
+
+       if (dma_spec->args_count != 4)

args_count contains count of all params leaving the phandle?

That was my interpretation from reading the code, but I have not tried it.

Quoted 4 lines

Documentation/devicetree/bindings/dma/snps-dma.txt line 29

Quoted 2 lines
Documentation/devicetree/bindings/dma/snps-dma.txt +2 -0
+       /* FIXME: This binding is rather clumsy. Can't we use the
+          request line numbers here instead? */

yes.

Ok, Very good. What is the encoding of the registers then?

Quoted 11 lines

Documentation/devicetree/bindings/dma/snps-dma.txt line 38

Quoted 9 lines
Documentation/devicetree/bindings/dma/snps-dma.txt +9 -0
+       fargs.cfg_lo = be32_to_cpup(dma_spec->args+0);
+       fargs.cfg_hi = be32_to_cpup(dma_spec->args+1);
+       fargs.src = be32_to_cpup(dma_spec->args+2);
+       fargs.dst = be32_to_cpup(dma_spec->args+3);
+
+       dma_cap_zero(cap);
+       dma_cap_set(DMA_SLAVE, cap);
+       /* FIXME: there should be a simpler way to do this */
+       return dma_request_channel(cap, dw_dma_generic_filter, &dma_spec->args[0]);

don't you need to send &fargs as the last argument?

Right, my mistake.

Thanks a lot for the input. When I fix the above, are actually able to test the changes, or have you lost access to the hardware when leaving ST?

Arnd

#17 lore raw

PATCH 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 9 lines · Arnd Bergmann
Quoted 2 lines · Viresh Kumar

Shouldn't this be 4? Would be better to mention what fields are these, right here. I have seen them below though.

Correct. I changed these a couple of times while trying to understand what the fields are, and I missed this instance. I'm still not sure whether we actually need all four fields, or what the simplest format for them would be. This just mirrors what you had in your binding.

You can add request_line number and leave first two fields, cfghi and lo.

Quoted 6 lines
Quoted 4 lines

drivers/dma/dw_dmac.c static void dwc_free_chan_resources(struct dma_chan *chan) · line 1207

Quoted 2 lines
drivers/dma/dw_dmac.c +2 -0
+       /* FIXME: memory leak! could we put this into dw_dma_chan? */
+       sd = devm_kzalloc(dw->dma.dev, sizeof (*sd), GFP_KERNEL);

Yes.

Yes it can be in dw_dma_chan or yes it is a memory leak?

Yes it can be in dw_dma_chan :)

Quoted 5 lines
Quoted 3 lines
Quoted 1 line

+ if (dma_spec->args_count != 4)

args_count contains count of all params leaving the phandle?

That was my interpretation from reading the code, but I have not tried it.

Okay, it was just a question from my side :)

Quoted 6 lines
Quoted 4 lines

drivers/dma/dw_dmac.c static void dwc_free_chan_resources(struct dma_chan *chan) · line 1235

Quoted 2 lines
drivers/dma/dw_dmac.c +2 -0
+       /* FIXME: This binding is rather clumsy. Can't we use the
+          request line numbers here instead? */

yes.

Ok, Very good. What is the encoding of the registers then?

You can still keep fargs as is and just fill them as:

       fargs.cfg_lo = 0;

if (DMA_TO_DEV)

       // dest is periph
       fargs.cfg_hi = be32_to_cpup(dma_spec->args+0) << 11;
else if (DEV_TO_DMA)
       // src is periph
       fargs.cfg_hi = be32_to_cpup(dma_spec->args+0) << 7;

The field size is 4 bits.

Quoted 3 lines

Thanks a lot for the input. When I fix the above, are actually able to test the changes, or have you lost access to the hardware when leaving ST?

I don't have any sort of access for testing these :( But, Vipul might try these at his end.

#18 lore raw

PATCH 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 12 lines · Viresh Kumar
Quoted 8 lines · Arnd Bergmann
Quoted 4 lines · Viresh Kumar

drivers/dma/dw_dmac.c static void dwc_free_chan_resources(struct dma_chan *chan) · line 1235

Quoted 2 lines
drivers/dma/dw_dmac.c +2 -0
+       /* FIXME: This binding is rather clumsy. Can't we use the
+          request line numbers here instead? */

yes.

Ok, Very good. What is the encoding of the registers then?

You can still keep fargs as is and just fill them as:

Quoted 8 lines
       fargs.cfg_lo = 0;

if (DMA_TO_DEV)

       // dest is periph
       fargs.cfg_hi = be32_to_cpup(dma_spec->args+0) << 11;
else if (DEV_TO_DMA)
       // src is periph
       fargs.cfg_hi = be32_to_cpup(dma_spec->args+0) << 7;

We have macros for such shifts.

drivers/dma/dw_dmac.c:187: cfghi = DWC_CFGH_DST_PER(...
drivers/dma/dw_dmac.c:189: cfghi = DWC_CFGH_SRC_PER(...

Signature · 2 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy
#19 lore raw

PATCH 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 8 lines · Andy Shevchenko
Quoted 6 lines · Viresh Kumar

if (DMA_TO_DEV)

       // dest is periph
       fargs.cfg_hi = be32_to_cpup(dma_spec->args+0) << 11;
else if (DEV_TO_DMA)
       // src is periph
       fargs.cfg_hi = be32_to_cpup(dma_spec->args+0) << 7;
Quoted 4 lines

We have macros for such shifts.

drivers/dma/dw_dmac.c:187: cfghi = DWC_CFGH_DST_PER(...
drivers/dma/dw_dmac.c:189: cfghi = DWC_CFGH_SRC_PER(...

I am getting older now, bad memory :)
I grepped into drivers/dma/dw_dmac_regs.h and left include/linux/dw_dmac.h :(

#20 lore raw

PATCH 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 10 lines · Viresh Kumar
Quoted 8 lines · Andy Shevchenko
Quoted 6 lines · Viresh Kumar

if (DMA_TO_DEV)

       // dest is periph
       fargs.cfg_hi = be32_to_cpup(dma_spec->args+0) << 11;
else if (DEV_TO_DMA)
       // src is periph
       fargs.cfg_hi = be32_to_cpup(dma_spec->args+0) << 7;
Quoted 7 lines
Quoted 4 lines

We have macros for such shifts.

drivers/dma/dw_dmac.c:187: cfghi = DWC_CFGH_DST_PER(...
drivers/dma/dw_dmac.c:189: cfghi = DWC_CFGH_SRC_PER(...

I am getting older now, bad memory :)
I grepped into drivers/dma/dw_dmac_regs.h and left include/linux/dw_dmac.h :(

Moreover the excerpt I showed from dw_dmac.c is the same piece of code you wrote above as a sample.

Signature · 2 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy
#21 lore raw

PATCH v2 1/5 dmaengine: dw_dmac: move to generic DMA binding

In reply to Viresh Kumar
Quoted 4 lines · Viresh Kumar

You can still keep fargs as is and just fill them as:

       fargs.cfg_lo = 0;
Quoted 8 lines

if (DMA_TO_DEV)

       // dest is periph
       fargs.cfg_hi = be32_to_cpup(dma_spec->args+0) << 11;
else if (DEV_TO_DMA)
       // src is periph
       fargs.cfg_hi = be32_to_cpup(dma_spec->args+0) << 7;

The field size is 4 bits.

Ah, good. So I guess the "dma-requests" property should actually be "16" then.

Does this mean that an implicit zero request line means memory?

Could we have device-to-device DMAs with this controller, and if we can, should we have both 'src' and 'dst' fields? Are the two number ranges sharing the same address space, i.e. is request '7' as the destination guaranteed to be the same device as request '7' in the source?

If we need two lines, we could interleave them with the bus master numbers:

dmas = <&dwdma0 // controller

	7   // source request 7
	1   // source master 1
	0   // dest   request 0
	0>, // dest   master 1
       <&dwdma0
	0   // source request 0
	0   // source master 0
	8   // dest   request 8
	1>; // dest   master 1

In theory, we could use bit-stuffing to put them all into a single 32 bit word I guess, but generally people don't seem to like that for new bindings.

Quoted 6 lines
Quoted 3 lines

Thanks a lot for the input. When I fix the above, are actually able to test the changes, or have you lost access to the hardware when leaving ST?

I don't have any sort of access for testing these :( But, Vipul might try these at his end.

Ok, I see.

Arnd

#22 lore raw

PATCH 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 12 lines · Arnd Bergmann
Quoted 11 lines · Viresh Kumar

You can still keep fargs as is and just fill them as:

       fargs.cfg_lo = 0;

if (DMA_TO_DEV)

       // dest is periph
       fargs.cfg_hi = be32_to_cpup(dma_spec->args+0) << 11;
else if (DEV_TO_DMA)
       // src is periph
       fargs.cfg_hi = be32_to_cpup(dma_spec->args+0) << 7;
Quoted 6 lines
Quoted 1 line

The field size is 4 bits.

Ah, good. So I guess the "dma-requests" property should actually be "16" then.

Does this mean that an implicit zero request line means memory?

No, it doesn't.
When dma is doing mem2mem transfers the request line field is ignored by the hw.

Quoted 2 lines

Could we have device-to-device DMAs with this controller, and if we can, should we have both 'src' and 'dst' fields?

As far as I know there is no driver under dmaengine that supports per2per transfers.

Quoted 4 lines

Are the
two number ranges sharing the same address space, i.e. is request '7' as the destination guaranteed to be the same device as request '7' in the source?

Request line should be unique. It means a real wire from slave hw to the dmac.

Signature · 2 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy
#23 lore raw

PATCH 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 15 lines · Andy Shevchenko
Quoted 14 lines · Arnd Bergmann
Quoted 12 lines · Viresh Kumar

You can still keep fargs as is and just fill them as:

       fargs.cfg_lo = 0;

if (DMA_TO_DEV)

       // dest is periph
       fargs.cfg_hi = be32_to_cpup(dma_spec->args+0) << 11;
else if (DEV_TO_DMA)
       // src is periph
       fargs.cfg_hi = be32_to_cpup(dma_spec->args+0) << 7;

The field size is 4 bits.

Quoted 8 lines
Quoted 4 lines

Ah, good. So I guess the "dma-requests" property should actually be "16" then.

Does this mean that an implicit zero request line means memory?

No, it doesn't.
When dma is doing mem2mem transfers the request line field is ignored by the hw.

Memory to memory transfers are dealt with using a totally different API to the slave API. Look at the rest of the DMA engine API to see how it's used - any channel is selected with a DMA_MEMCPY capability.

(IMHO, the MEM2MEM transfer type against the slave API should never have been permitted.)

#24 lore raw

PATCH v2 1/5 dmaengine: dw_dmac: move to generic DMA binding

In reply to Andy Shevchenko
Quoted 10 lines · Andy Shevchenko
Quoted 5 lines · Arnd Bergmann

On Tuesday 29 January 2013, Viresh Kumar wrote:
Ah, good. So I guess the "dma-requests" property should actually be "16" then.

Does this mean that an implicit zero request line means memory?

No, it doesn't.
When dma is doing mem2mem transfers the request line field is ignored by the hw.

Ok.

Quoted 5 lines
Quoted 2 lines

Could we have device-to-device DMAs with this controller, and if we can, should we have both 'src' and 'dst' fields?

As far as I know there is no driver under dmaengine that supports per2per transfers.

Well, I'm asking because we need to describe the hardware properly, even if drivers today don't do this does not mean that it is not possible. We can always change the Linux implementation, but we cannot as easily change an established DT binding, just like we don't change user space interfaces.

Of course, if that is a thing we don't expect anyone to do, we don't have to describe it, or we could make a compatible extension to the binding later.

Quoted 7 lines
Quoted 4 lines

Are the
two number ranges sharing the same address space, i.e. is request '7' as the destination guaranteed to be the same device as request '7' in the source?

Request line should be unique. It means a real wire from slave hw to the dmac.

Ok, good.

Arnd

#25 lore raw

PATCH 1/5 dmaengine: dw_dmac: move to generic DMA binding

In reply to Arnd Bergmann
Quoted 2 lines · Arnd Bergmann

Ah, good. So I guess the "dma-requests" property should actually be "16" then.

yes, even i was checking on that separately :)

Quoted 1 line

Does this mean that an implicit zero request line means memory?

No. 0 is also request line for a peripheral and numbers are from 0 to 15. memcpy are identified by setting type of transfer as memcpy.

Quoted 5 lines

Could we have device-to-device DMAs with this controller, and if we can, should we have both 'src' and 'dst' fields? Are the two number ranges sharing the same address space, i.e. is request '7' as the destination guaranteed to be the same device as request '7' in the source?

Request lines are per master... So, for a master single request line is independent of direction. Many DMA controllers have capability of doing dev-to-dev transfers but DMAENGINE doesn't have any support for it, even we don't have a usecase too :)

Quoted 2 lines

If we need two lines, we could interleave them with the bus master numbers:

not required.

Quoted 3 lines

In theory, we could use bit-stuffing to put them all into a single 32 bit word I guess, but generally people don't seem to like that for new bindings.

:)

--
viresh

#26 lore raw

PATCH v2 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 5 lines · Viresh Kumar
Quoted 2 lines · Arnd Bergmann

Ah, good. So I guess the "dma-requests" property should actually be "16" then.

yes, even i was checking on that separately :)

Actually, I just discovered something odd in the arch/arm/mach-spear/spear13xx-dma.h file that gets removed in the last patch: there, we define request numbers up to 32, e.g.

-       SPEAR1310_DMA_REQ_UART2_RX = 14,
-       SPEAR1310_DMA_REQ_UART2_TX = 15,
-       SPEAR1310_DMA_REQ_UART5_RX = 16,
-       SPEAR1310_DMA_REQ_UART5_TX = 17,

What is the meaning of this, if the maximum request number is 15?

Quoted 11 lines
Quoted 5 lines

Could we have device-to-device DMAs with this controller, and if we can, should we have both 'src' and 'dst' fields? Are the two number ranges sharing the same address space, i.e. is request '7' as the destination guaranteed to be the same device as request '7' in the source?

Request lines are per master... So, for a master single request line is independent of direction. Many DMA controllers have capability of doing dev-to-dev transfers but DMAENGINE doesn't have any support for it, even we don't have a usecase too :)

Quoted 4 lines
Quoted 2 lines

If we need two lines, we could interleave them with the bus master numbers:

not required.

Ok. Would it be enough to have only one master and one request field in the DT dma descriptor then, and have the code figure whether to use it as source or destination, based on the configuration? Which one should come first? Since you have multiple masters per controller, and multiple requests per master, it sounds like the cleanest descriptor form would be

	<controller master request>;

Or possibly

	<controller master request direction>;

if the direction needs to be known at the time the channel is requested.

Arnd

#27 lore raw

PATCH 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 17 lines · Arnd Bergmann
Quoted 5 lines · Viresh Kumar
Quoted 2 lines · Arnd Bergmann

Ah, good. So I guess the "dma-requests" property should actually be "16" then.

yes, even i was checking on that separately :)

Actually, I just discovered something odd in the arch/arm/mach-spear/spear13xx-dma.h file that gets removed in the last patch: there, we define request numbers up to 32, e.g.

(quoted diff) +0 -4
-       SPEAR1310_DMA_REQ_UART2_RX = 14,
-       SPEAR1310_DMA_REQ_UART2_TX = 15,
-       SPEAR1310_DMA_REQ_UART5_RX = 16,
-       SPEAR1310_DMA_REQ_UART5_TX = 17,
Quoted 1 line

What is the meaning of this, if the maximum request number is 15?

I knew you will come to this :)
So, the hardware is like: there are 16 request line slots per master, a platform can choose to connect same or separate devices to these.

So, these are really 16 per master.

Quoted 8 lines

Ok. Would it be enough to have only one master and one request field in the DT dma descriptor then, and have the code figure whether to use it as source or destination, based on the configuration? Which one should come first? Since you have multiple masters per controller, and multiple requests per master, it sounds like the cleanest descriptor form would be

Quoted 8 lines
        <controller master request>;

Or possibly

        <controller master request direction>;

if the direction needs to be known at the time the channel is requested.

Its better to keep masters as is. So, that we can use appropriate masters for peripheral and memory to make the transfer fast.

#28 lore raw

PATCH v2 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 18 lines · Viresh Kumar
Quoted 17 lines · Arnd Bergmann
Quoted 5 lines · Viresh Kumar
Quoted 2 lines · Arnd Bergmann

Ah, good. So I guess the "dma-requests" property should actually be "16" then.

yes, even i was checking on that separately :)

Actually, I just discovered something odd in the arch/arm/mach-spear/spear13xx-dma.h file that gets removed in the last patch: there, we define request numbers up to 32, e.g.

(quoted diff) +0 -4
-       SPEAR1310_DMA_REQ_UART2_RX = 14,
-       SPEAR1310_DMA_REQ_UART2_TX = 15,
-       SPEAR1310_DMA_REQ_UART5_RX = 16,
-       SPEAR1310_DMA_REQ_UART5_TX = 17,
Quoted 7 lines
Quoted 1 line

What is the meaning of this, if the maximum request number is 15?

I knew you will come to this :)
So, the hardware is like: there are 16 request line slots per master, a platform can choose to connect same or separate devices to these.

So, these are really 16 per master.

Ok, I see. Do you know how these are numbered in the data sheet?

If the convention is to have subsequent numbers for these in the hardware description, we should probably just have that single request number in the binding, too, and calculate the master number from that. If it lists pairs of request/master number, we should use pairs in the binding as well, in the same order.

Quoted 12 lines
Quoted 12 lines

Ok. Would it be enough to have only one master and one request field in the DT dma descriptor then, and have the code figure whether to use it as source or destination, based on the configuration? Which one should come first? Since you have multiple masters per controller, and multiple requests per master, it sounds like the cleanest descriptor form would be

        <controller master request>;

Or possibly

Quoted 7 lines
Quoted 4 lines
        <controller master request direction>;

if the direction needs to be known at the time the channel is requested.

Its better to keep masters as is. So, that we can use appropriate masters for peripheral and memory to make the transfer fast.

So you mean keep the format as

	<controller request src-master dst-master>;

?

Arnd

#29 lore raw

PATCH 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 7 lines · Arnd Bergmann
Quoted 5 lines · Viresh Kumar

I knew you will come to this :)
So, the hardware is like: there are 16 request line slots per master, a platform can choose to connect same or separate devices to these.

So, these are really 16 per master.

Quoted 7 lines

Ok, I see. Do you know how these are numbered in the data sheet?

If the convention is to have subsequent numbers for these in the hardware description, we should probably just have that single request number in the binding, too, and calculate the master number from that. If it lists pairs of request/master number, we should use pairs in the binding as well, in the same order.

Actually what would be better to have is:
- have this range from 0-15 only
- together with the master we want to use for peripheral

this should be enough.

Datasheet of dw_dmac doesn't tell much about it.. just four bits for programming it and so values are from 0-15 :)

Quoted 17 lines
Quoted 17 lines
Quoted 16 lines

Ok. Would it be enough to have only one master and one request field in the DT dma descriptor then, and have the code figure whether to use it as source or destination, based on the configuration? Which one should come first? Since you have multiple masters per controller, and multiple requests per master, it sounds like the cleanest descriptor form would be

        <controller master request>;

Or possibly

        <controller master request direction>;

if the direction needs to be known at the time the channel is requested.

Quoted 8 lines
Quoted 2 lines

Its better to keep masters as is. So, that we can use appropriate masters for peripheral and memory to make the transfer fast.

So you mean keep the format as

        <controller request src-master dst-master>;

?

Yes..

#30 lore raw

PATCH v2 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 16 lines · Viresh Kumar
Quoted 7 lines · Arnd Bergmann
Quoted 5 lines · Viresh Kumar

I knew you will come to this :)
So, the hardware is like: there are 16 request line slots per master, a platform can choose to connect same or separate devices to these.

So, these are really 16 per master.

Quoted 7 lines

Ok, I see. Do you know how these are numbered in the data sheet?

If the convention is to have subsequent numbers for these in the hardware description, we should probably just have that single request number in the binding, too, and calculate the master number from that. If it lists pairs of request/master number, we should use pairs in the binding as well, in the same order.

Quoted 5 lines

Actually what would be better to have is:
- have this range from 0-15 only
- together with the master we want to use for peripheral

this should be enough.

Ok.

Quoted 2 lines

Datasheet of dw_dmac doesn't tell much about it.. just four bits for programming it and so values are from 0-15 :)

I meant the spear13xx data sheet, which has to list the request lines for its integrated components. There may be other SoCs using the same dw_dmac, but this is the main one that is upstream now, and it's probably as good as any other one. I just wouldn't want to establish a binding that doesn't match any of the known implementations in the way it expresses request lines.

Arnd

#31 lore raw

PATCH 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 6 lines · Arnd Bergmann

I meant the spear13xx data sheet, which has to list the request lines for its integrated components. There may be other SoCs using the same dw_dmac, but this is the main one that is upstream now, and it's probably as good as any other one. I just wouldn't want to establish a binding that doesn't match any of the known implementations in the way it expresses request lines.

Your binding (<controller request-line src-master dst-master) is good enough for SPEAr :)

SPEAr13xx is a bit complex. Request lines are distributed among masters.
+ there is multiplexing among them.. over that there are two controllers..

But yes, your binding is good enough :)

A message that is not in the archive (sent off-list or not yet received): 1359445171.31148.30.camel@smile

#33 lore raw

PATCH v2 1/5 dmaengine: dw_dmac: move to generic DMA binding

(putting back the Cc list, I assumed you dropped them accidentally)

Quoted 7 lines · Andy Shevchenko

drivers/dma/dw_dmac.c static void dwc_free_chan_resources(struct dma_chan *chan) · line 1201

Quoted 3 lines · Arnd Bergmann
drivers/dma/dw_dmac.c +2 -1
-	if ((last_dw == dw) && (last_bus_id == param))
+	/* both the driver and the device must match */
+        if (chan->device->dev->driver != &dw_driver.driver)

Could we somehow pass the &.driver to the generic filter function (via *_dma_controller_register() ? ) and do this to each DMA driver?

My hope is still that we can avoid using filter functions entirely when we use xlate() logic, and instead just go through the channels of the dma engine we know we are looking at.

I would also assume that the argument passed to *_dma_controller_register normally holds a pointer to the dma device. Now that I think about it, the check 'if (dw != fargs->dw)' already implies that we are looking at the correct driver, but it feels dirty to cast a random dma_device pointer to a driver specific one before we know which driver it is for.

However, since we have a valid pointer to a dw_dma object, we can extract the driver from there:

	struct dw_dma_filter_args *fargs = param;
	struct dma_device *ddev = &fargs->dw->dma;
	if (dma != chan->device)
		return -EINVAL;

which is simpler and cleaner that what I had.

Quoted 7 lines

drivers/dma/dw_dmac.c static void dwc_free_chan_resources(struct dma_chan *chan) · line 1215

Quoted 5 lines
drivers/dma/dw_dmac.c +5 -0
+	sd->dma_dev	= dw->dma.dev;
+	sd->cfg_hi	= fargs->cfg_hi;
+	sd->cfg_lo	= fargs->cfg_lo;
+	sd->src_master	= fargs->src;
+	sd->dst_master	= fargs->dst;

Could we use fargs structure directly?

We could probably have no fargs but use the dw_dma_slave structure directly to pass the data, if cannot figure out a way to avoid the need for a filter function. I thought about doing that, but intermediate versions of my patch had a different layout here.

Arnd

#34 lore raw

PATCH 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 1 line · Arnd Bergmann

(putting back the Cc list, I assumed you dropped them accidentally)

That'll be why I don't have a copy of Andy's email to reply to.

Quoted 8 lines · Andy Shevchenko
Quoted 7 lines · Arnd Bergmann

drivers/dma/dw_dmac.c static void dwc_free_chan_resources(struct dma_chan *chan) · line 1201

Quoted 3 lines
drivers/dma/dw_dmac.c +2 -1
-	if ((last_dw == dw) && (last_bus_id == param))
+	/* both the driver and the device must match */
+        if (chan->device->dev->driver != &dw_driver.driver)

Could we somehow pass the &.driver to the generic filter function (via *_dma_controller_register() ? ) and do this to each DMA driver?

How, and what driver gets passed?

Quoted 3 lines

My hope is still that we can avoid using filter functions entirely when we use xlate() logic, and instead just go through the channels of the dma engine we know we are looking at.

Has anyone yet determined whether any of these new DMA engine slave APIs are usable for implementations which have a separate MUX between the DMA engine and the DMA peripheral?

#35 lore raw

PATCH v2 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 10 lines · Russell King - ARM Linux
Quoted 8 lines · Andy Shevchenko
Quoted 7 lines · Arnd Bergmann
Quoted 3 lines

- if ((last_dw == dw) && (last_bus_id == param))
+ /* both the driver and the device must match */
+ if (chan->device->dev->driver != &dw_driver.driver)

Could we somehow pass the &.driver to the generic filter function (via *_dma_controller_register() ? ) and do this to each DMA driver?

How, and what driver gets passed?

of_dma_controller_register (see linux-next) already gets a device_node of a device that is owned by the driver calling this function. This driver could in theory pass its 'struct device_driver' as another argument, although I would expect it to pass its own device specific object (e.g. struct dw_dma or struct dma_pl330_dmac) as the 'data' pointer, and from there it can easily check if the device matches.

Quoted 7 lines
Quoted 3 lines

My hope is still that we can avoid using filter functions entirely when we use xlate() logic, and instead just go through the channels of the dma engine we know we are looking at.

Has anyone yet determined whether any of these new DMA engine slave APIs are usable for implementations which have a separate MUX between the DMA engine and the DMA peripheral?

Can you give an example for this? We were careful to make sure it works with platforms that connect a slave to multiple dma engines, out of which any could be used for a given transfer. In the device tree binding, you specify all possible controllers and give the alternatives the same name, for example:

	serial at 10000000 {
		compatible = "arm,pl011", "arm,primecell";
		dmas = <dwdma0 7 0>, <dwdma0 8 1>, <&pl330 17>, <&pl330 15>;
		dma-names = "rx", "tx", "rx", "tx;
		...
	};

Here, you hve a UART that can use DMA for both receive and transmit, and can use either the dw_dma or the pl330 controller for each channel. The common dmaengine code will try to pick the first available channel on either one. We can probably try to be smarter about making the decision which one to use.

Is this what you are referring to?

Arnd

#36 lore raw

PATCH 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 6 lines · Arnd Bergmann

Can you give an example for this? We were careful to make sure it works with platforms that connect a slave to multiple dma engines, out of which any could be used for a given transfer. In the device tree binding, you specify all possible controllers and give the alternatives the same name, for example:

Quoted 6 lines
	serial at 10000000 {
		compatible = "arm,pl011", "arm,primecell";
		dmas = <dwdma0 7 0>, <dwdma0 8 1>, <&pl330 17>, <&pl330 15>;
		dma-names = "rx", "tx", "rx", "tx;
		...
	};

No, that's not what I mean. I mean the situation we find on Versatile platforms:

8 3 >3
PL080 DMA --/--+--/------> FPGA Mux --/--> {bunch of off-CPU peripherals}

               |  5
               `--/------> {On-CPU peripherals}

This is something that I've been raising _every time_ I've been involved with DMA engine discussions when it comes to the matching stuff, so this is nothing new, and it's not unknown about.

What is different this time around is that I've been purposely omitted from the discussions (like what seems to be happening more and more - I notice that I'm no longer copied on PL011 patches despite being the driver author, or GIC patches, or VIC patches) so this stuff doesn't get properly considered.

But it doesn't matter anymore; I'm soo out of the loop on stuff like DT and the like that my input would be more of a hinderence now.

#37 lore raw

PATCH v2 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 11 lines · Russell King - ARM Linux

No, that's not what I mean. I mean the situation we find on Versatile platforms:

8 3 >3
PL080 DMA --/--+--/------> FPGA Mux --/--> {bunch of off-CPU peripherals}

               |  5
               `--/------> {On-CPU peripherals}

This is something that I've been raising _every time_ I've been involved with DMA engine discussions when it comes to the matching stuff, so this is nothing new, and it's not unknown about.

Ok, I see. I have not actually been involved with the DMA engine API much, so for me it's the first time /I/ see this being explained.

Quoted 1 line

From the DT binding perspective, doing this should be no problem. I guess

you would model the MUX as a trivial dma engine device that forwards to another one, just like we do for nested interrupt controllers:

	pl080: dma-engine at 10000000 {
		compatible ="arm,pl080", "arm,primecell";
		reg = <0x10000000 0x1000>;
		dma-requests = <8>;
		dma-channels = <4>;
		#dma-cells = <2>;
	};

	fpga {
		mux: dma-mux at f0008000 {
			reg = <0xf0008000 100>;
			compatible = "arm,versatile-fpga-dmamux";
			dma-requests = <7>;
			dma-channels = <3>;
			#dma-cells = <1>;
			dmas = <&pl080 5 0>, <&pl080 6 0> <&pl080 7 0>;
			dma-names = "mux5", "mux6", "mux7";
		};
		...

		some-device at f000a000 {
			compatible = "foo,using-muxed-dma";
			dmas = <&mux 3>, <&mux 4>;
			dma-names = "rx", "tx";
		};
	};

The driver for foo,using-muxed-dma just requests a slave channel for its lines, which ends up calling the xlate function of the driver for the mux. That driver still needs to get written, of course, but it should be able to recursively call dma_request_slave_channel on the pl080 device. The arm,versatile-fpga-dmamux driver would not be registered to the dmaengine layer, but it would register as an of_dma_controller.

Arnd

#38 lore raw

PATCH 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 47 lines · Arnd Bergmann
Quoted 11 lines · Russell King - ARM Linux

No, that's not what I mean. I mean the situation we find on Versatile platforms:

8 3 >3
PL080 DMA --/--+--/------> FPGA Mux --/--> {bunch of off-CPU peripherals}

               |  5
               `--/------> {On-CPU peripherals}

This is something that I've been raising _every time_ I've been involved with DMA engine discussions when it comes to the matching stuff, so this is nothing new, and it's not unknown about.

Ok, I see. I have not actually been involved with the DMA engine API much, so for me it's the first time /I/ see this being explained.

From the DT binding perspective, doing this should be no problem. I guess you would model the MUX as a trivial dma engine device that forwards to another one, just like we do for nested interrupt controllers:

	pl080: dma-engine at 10000000 {
		compatible ="arm,pl080", "arm,primecell";
		reg = <0x10000000 0x1000>;
		dma-requests = <8>;
		dma-channels = <4>;
		#dma-cells = <2>;
	};

	fpga {
		mux: dma-mux at f0008000 {
			reg = <0xf0008000 100>;
			compatible = "arm,versatile-fpga-dmamux";
			dma-requests = <7>;
			dma-channels = <3>;
			#dma-cells = <1>;
			dmas = <&pl080 5 0>, <&pl080 6 0> <&pl080 7 0>;
			dma-names = "mux5", "mux6", "mux7";
		};
		...

		some-device at f000a000 {
			compatible = "foo,using-muxed-dma";
			dmas = <&mux 3>, <&mux 4>;
			dma-names = "rx", "tx";
		};
	};
Quoted 6 lines

The driver for foo,using-muxed-dma just requests a slave channel for its lines, which ends up calling the xlate function of the driver for the mux. That driver still needs to get written, of course, but it should be able to recursively call dma_request_slave_channel on the pl080 device. The arm,versatile-fpga-dmamux driver would not be registered to the dmaengine layer, but it would register as an of_dma_controller.

That's a good way to represent it but it fails in a very big way:
You're stuffing N peripherals down to 3 request lines to the DMA engine, and you may want more than 3 of those peripherals to be making use of the DMA engine at any one time.

Before anyone says "you shouldn't be doing this" consider this:
your typical DMA slave engine already has this structure:

N DMA channels <---> M DMA requests <---> M peripherals

where N < M. In other words, there is _already_ a MUX between the peripherals and the DMA engine channels themselves (what do you think the "request index" which you have to program into DMA channel control registers is doing...

We support this external mux today in the PL080 driver - and we do that by having the PL080 driver do the scheduling of virtual DMA channels on the actual hardware itself. The PL080 driver has knowledge that there may be some sort of additional muxing layer between it and the peripheral.

As the APIs stand today, you just can't do this without having the DMA engine driver itself intimately involved because a layer above doesn't really have much clue as to what's going on, and the DMA engine stuff itself doesn't layer particularly well.

#39 lore raw

PATCH v2 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 8 lines · Russell King - ARM Linux

That's a good way to represent it but it fails in a very big way:
You're stuffing N peripherals down to 3 request lines to the DMA engine, and you may want more than 3 of those peripherals to be making use of the DMA engine at any one time.

Before anyone says "you shouldn't be doing this" consider this:
your typical DMA slave engine already has this structure:

Quoted 6 lines

N DMA channels <---> M DMA requests <---> M peripherals

where N < M. In other words, there is already a MUX between the peripherals and the DMA engine channels themselves (what do you think the "request index" which you have to program into DMA channel control registers is doing...

Ok.

Quoted 10 lines

We support this external mux today in the PL080 driver - and we do that by having the PL080 driver do the scheduling of virtual DMA channels on the actual hardware itself. The PL080 driver has knowledge that there may be some sort of additional muxing layer between it and the peripheral.

As the APIs stand today, you just can't do this without having the DMA engine driver itself intimately involved because a layer above doesn't really have much clue as to what's going on, and the DMA engine stuff itself doesn't layer particularly well.

If the pl080 driver already has code for the mux in it, then it should handle both of_dma_controller instances in my example. It would not change anything regarding the binding, which just describes the way that the hardware is connected. I have not looked at the implementation of the pl080 driver, but I'd assume we could get away with just having two separate xlate() functions. It's slightly ugly to have one driver take responsibility for two device_node:s, but it's not unheard of. In the probe function for the pl080 node, the driver can walk the entire device tree to find any mux devices connected to it and register an of_dma_controller() with its xlate function for those.

Unless you see another issue with this, I'd assume it's all covered by the new interface, but it also doesn't get better than what we have today.

Arnd

#40 lore raw

PATCH 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 6 lines · Arnd Bergmann

If the pl080 driver already has code for the mux in it, then it should handle both of_dma_controller instances in my example. It would not change anything regarding the binding, which just describes the way that the hardware is connected. I have not looked at the implementation of the pl080 driver, but I'd assume we could get away with just having two separate xlate() functions.

Well, how it all works in the PL08x driver at present is:

- the platform code passes into the PL08x driver a description of the virtual channels, eg:

        [1] = {
                /* Muxed on channel 0-3 */
                .bus_id = "aacirx",
                .min_signal = 0,
                .max_signal = 2,
                .muxval = 0x01,
                .periph_buses = PL08X_AHB1 | PL08X_AHB2,
        },

- the virtual channels include:
- the minimum and maximum index of the DMA request signals into the PL08x that can be used with this peripheral.
- the the register value for the external mux, if any.
- other PL08x specific data to do with this DMA peripheral

- when a virtual channel is requested by a DMA client, it claims just the virtual channel. No actual hardware resources are assigned, and no mappings are setup.

- when a transfer is prepared on a virtual channel, part of the preparation is to locate the request signal to be used - and platform code is requested to supply that from the description associated with the channel (such as the above fragment.)

- the versatile PL08x code looks at min_signal/max_signal, and walks this space looking for an unassigned request signal. If there is an external mux associated with this request signal, the mux is appropriately programmed. If a request signal is currently assigned the next request signal will be tried until there are no further possibilities, when failure occurs. In that case, the prepare function also fails.

- the PL08x driver then knows which request signal is associated with the peripheral, and sets up the descriptors to be dependent on this request signal. This mapping must not change until the transfer has completed.

- when the descriptor is completed - and freed, the muxing is released and the DMA request signal becomes available for other users to make use of.

Practically, what this means is that:

(a) we've ensured that all drivers using PL08x do _not_ expect that descriptors can always be prepared; they must continue to work correctly when the prepare function returns NULL.

(b) several devices end up sharing the first three request signals and they're used on an opportunistic basis. Theoretically, if you have one of these boards where AACI DMA works, you can be using one of these DMA requests for audio playback, another for record, and the remaining one can be used on an opportunistic basis between the second MMCI interface (should that also work - which I've proven is an impossiblity!) and the 3rd UART... or the USB if there was a driver for that device.

#41 lore raw

PATCH v2 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 1 line · Russell King - ARM Linux

Well, how it all works in the PL08x driver at present is:

<snip>

Thanks for the explanations. If I end up implementing the DT support for pl08x, this will be very helpful. I looked at the git history for mach-versatile and could not find any of it there, although the patches were certainly on the mailing list
http://lists.infradead.org/pipermail/linux-arm-kernel/2010-June/017818.html

Do you (or Linus) know what happened to the patch series?

Based on your description, it sounds all doable, but the split between platform specific code and device driver code would be
different: While the muxing that you describe all takes place in the get_signal/put_signal functions in platform code, doing a proper DT binding for the mux would imply moving that into the pl080 driver itself, at least as a compile-time option for the driver. Do you think that would be acceptable?

While I guess we could still keep the get_signal/put_signal callbacks in mach-versatile but drop them for platforms without a mux, I'm not sure if that would make the overall driver better or worse.

Arnd

#42 lore raw

PATCH 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 4 lines · Arnd Bergmann

the patches were certainly on the mailing list
http://lists.infradead.org/pipermail/linux-arm-kernel/2010-June/017818.html

Do you (or Linus) know what happened to the patch series?

Just stalled. My fault probably, and IIRC Russell sent a modified version of this patch at one time.

Quoted 7 lines

Based on your description, it sounds all doable, but the split between platform specific code and device driver code would be
different: While the muxing that you describe all takes place in the get_signal/put_signal functions in platform code, doing a proper DT binding for the mux would imply moving that into the pl080 driver itself, at least as a compile-time option for the driver. Do you think that would be acceptable?

This is the pushing-to-driver paradigm, and for the legacy support it will be troublesome as it needs to supply the base address of the mux etc through the platform data, and then with an ampersand reference to the syscon in the dma node.

Quoted 4 lines

While I guess we could still keep the get_signal/put_signal callbacks in mach-versatile but drop them for platforms without a mux, I'm not sure if that would make the overall driver better or worse.

I think that is not the big issue though.

I think the most important thing is to come up with a OS-neutral description of channels vs muxes and signals.

Basically all DMA hardware has a mux, but in many cases it is fused with the DMA controller itself, and sometimes even hidden from software, i.e. it seems there is one channel per peripheral, but if you try to use them all at the same time something like round-robin bursts can be expected (in best case).

For each channel, you need to be able to specify a number of applicable request lines, with an optional mux component inbetween. And I think the bindings should be generic, because the problem is generic.

I tried to express this in DT syntax but failed, I just don't know how to do that, DT seems to be very much not about muxes and things that take optional paths, more about describing how static electronics are set up.

The basic assumption of a 1:1 mapping between a peripheral and a channel is bogus - this is true on some hardware (like the COH901318), but certainly not all, the Versatile and RealView being the most problematic examples to date with the separate mux and all.

Maybe stupid analogy:
http://en.wikipedia.org/wiki/File:JT_Switchboard_770x540.jpg
The actual channels are the cords.
The the sockets are the devices.
The telephonist is the mux.

So looking up a DMA channel for RX for the PL011 can be like trying to find a blue cord to put in the PL011 socket when the red light goes on for RX DMA. If no blue cord is available the telephonist can plug in the handset and say "sorry, failed to find a free line".

Conversely to TX DMA you may need a red
cord. Having a blue cord at hand will not help you.

Coding it down in the device tree like in the example is like plugging the cords in to some holes at boot time and let the telephonist go home.

Yours,
Linus Walleij

#43 lore raw

PATCH 1/5 dmaengine: dw_dmac: move to generic DMA binding

In reply to Arnd Bergmann
Quoted 8 lines · Arnd Bergmann

The original device tree binding for this driver, from Viresh Kumar unfortunately conflicted with the generic DMA binding, and did not allow to completely seperate slave device configuration from the controller.

This is an attempt to replace it with an implementation of the generic binding, but it is currently completely untested, because I do not have any hardware with this particular controller.

Quoted 6 lines

The patch applies on top of linux-next, which contains both the base support for the generic DMA binding, as well as the earlier attempt from Viresh. Both of these are currently not merged upstream however.

There are a couple of TODO items that are left remaining and are open for ideas from other people.

Have one question and one comment.

So, what is the status of this work? Do you manage to provide something for v3.9? (Oh, two questions :-) )

drivers/dma/dw_dmac.c static int dw_probe(struct platform_device *pdev) · line 1758

Quoted 11 lines
drivers/dma/dw_dmac.c +3 -1
@@ -1765,7 +1753,11 @@ static int dw_probe(struct platform_device *pdev)
 
 	dma_async_device_register(&dw->dma);
 
-	return 0;
+	err = of_dma_controller_register(pdev->dev.of_node, dw_dma_xlate, dw);
+	if (err)
+		dma_async_device_unregister(&dw->dma);

I don't think this is a good idea. The impossibility to register in the of-dma helper is not critical. Just printing debug message is enough.

Signature · 2 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy
#44 lore raw

PATCH v2 1/5 dmaengine: dw_dmac: move to generic DMA binding

Quoted 4 lines · Andy Shevchenko

Have one question and one comment.

So, what is the status of this work? Do you manage to provide something for v3.9? (Oh, two questions :-) )

I was going to bring it up today myself. Unfortunately the patches have never been tested on real hardware and I have not posted a version that includes the feedback I got, so I don't think it's a good idea to use this in v3.9. However, not doing it causes problems since Vinod's dma-slave tree still contains Viresh's earlier patches, causing a few problems:

* With these patches, the spear3xx platform currently does not build. (this one is easy to fix though)
* There is a conflict between these patches and my spear multiplatform series, which I have not yet queued up for 3.9 because of this, since that would have meant that Stephen Rothwell would have to discard either the arm-soc tree or the dma-slave tree from linux-next.
* I really don't want the broken binding to appear in 3.9.

I believe the best way out at this point is that I do an updated version of my patch, and Vinod first reverts the patch f9965aa20 "ARM: SPEAr13xx: Pass DW DMAC platform data from DT" in his tree and then applies my update. This will give us the right DT binding for dw-dmac but no in-tree users, which means that nothing breaks if I get it wrong.

I can then decide with Olof whether or not to take the spear multiplatform changes that no longer conflict with the dma slave tree as a "late" branch into 3.9 or wait until 3.10, but that is something you don't need to worry about then. Also the conversion of spear to use the new binding (patch 5 of this series) can go through the arm-soc tree for 3.10 after the ST folks have tested that it works.

Quoted 7 lines
Quoted 7 lines
drivers/dma/dw_dmac.c +0 -0
@@ -1765,7 +1753,11 @@ static int dw_probe(struct platform_device *pdev)
 
      dma_async_device_register(&dw->dma);
 
Quoted 7 lines

drivers/dma/dw_dmac.c static int dw_probe(struct platform_device *pdev) · line 1758

Quoted 4 lines
drivers/dma/dw_dmac.c +3 -1
-     return 0;
+     err = of_dma_controller_register(pdev->dev.of_node, dw_dma_xlate, dw);
+     if (err)
+             dma_async_device_unregister(&dw->dma);

I don't think this is a good idea. The impossibility to register in the of-dma helper is not critical. Just printing debug message is enough.

Ok, makes sense. Thanks!

Arnd

#45 lore raw

PATCH v2 2/5 spi: pl022: use generic DMA slave configuration if possible

In reply to Arnd Bergmann

With the new OF DMA binding, it is possible to completely avoid the need for platform_data for configuring a DMA channel. In cases where the platform has already been converted, calling dma_request_slave_channel should get all the necessary information from the device tree.

Like the patch that converts the dw_dma controller, this is completely untested and is looking for someone to try it out.

Cc Grant Likely <[email protected]>
Cc Mark Brown <[email protected]>
Cc spi-devel-general at lists.sourceforge.net
Cc Viresh Kumar <[email protected]>
Cc Andy Shevchenko <[email protected]>
Cc Vinod Koul <[email protected]>
Cc devicetree-discuss at lists.ozlabs.org
Cc linux-arm-kernel at lists.infradead.org
---
 .../devicetree/bindings/spi/spi_pl022.txt          | 36 ++++++++++++++++++
 drivers/spi/spi-pl022.c                            | 43 +++++++++++++++++++++-
 2 files changed, 77 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/spi/spi_pl022.txt +36 -0
index f158fd3..22ed679 100644
@@ -16,6 +16,11 @@ Optional properties:
                             device will be suspended immediately
 - pl022,rt : indicates the controller should run the message pump with realtime
              priority to minimise the transfer latency on the bus (boolean)
+- dmas : Two or more DMA channel specifiers following the convention outlined
+         in bindings/dma/dma.txt
+- dma-names: Names for the dma channels, if present. There must be at
+	     least one channel named "tx" for transmit and named "rx" for
+             receive.
 
 
 SPI slave nodes must be children of the SPI master node and can
@@ -32,3 +37,34 @@ contain the following properties.
 - pl022,wait-state : Microwire interface: Wait state
 - pl022,duplex : Microwire interface: Full/Half duplex
 
+
+Example:
+
+	spi at e0100000 {
+		compatible = "arm,pl022", "arm,primecell";
+		reg = <0xe0100000 0x1000>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		interrupts = <0 31 0x4>;
+		dmas = <&dma-controller 23 1>,
+			<&dma-controller 24 0>;
+		dma-names = "rx", "tx";
+
+		m25p80 at 1 {
+			compatible = "st,m25p80";
+			reg = <1>;
+			spi-max-frequency = <12000000>;
+			spi-cpol;
+			spi-cpha;
+			pl022,hierarchy = <0>;
+			pl022,interface = <0>;
+			pl022,slave-tx-disable;
+			pl022,com-mode = <0x2>;
+			pl022,rx-level-trig = <0>;
+			pl022,tx-level-trig = <0>;
+			pl022,ctrl-len = <0x11>;
+			pl022,wait-state = <0>;
+			pl022,duplex = <0>;
+		};
+	};
+	
drivers/spi/spi-pl022.c +41 -2

61 lines not on this page · raw message

Signature · 1 line
1.8.0
#46 lore raw

PATCH 2/5 spi: pl022: use generic DMA slave configuration if possible

Quoted 7 lines · Arnd Bergmann

With the new OF DMA binding, it is possible to completely avoid the need for platform_data for configuring a DMA channel. In cases where the platform has already been converted, calling dma_request_slave_channel should get all the necessary information from the device tree.

Like the patch that converts the dw_dma controller, this is completely untested and is looking for someone to try it out.

I've no ability to test this but it looks good from a code point of view.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130129/bc0b0d67/attachment.sig>

#47 lore raw

PATCH 2/5 spi: pl022: use generic DMA slave configuration if possible

In reply to Arnd Bergmann
Quoted 33 lines · Arnd Bergmann

With the new OF DMA binding, it is possible to completely avoid the need for platform_data for configuring a DMA channel. In cases where the platform has already been converted, calling dma_request_slave_channel should get all the necessary information from the device tree.

Like the patch that converts the dw_dma controller, this is completely untested and is looking for someone to try it out.

Cc Grant Likely <[email protected]>
Cc Mark Brown <[email protected]>
Cc spi-devel-general at lists.sourceforge.net
Cc Viresh Kumar <[email protected]>
Cc Andy Shevchenko <[email protected]>
Cc Vinod Koul <[email protected]>
Cc devicetree-discuss at lists.ozlabs.org
Cc linux-arm-kernel at lists.infradead.org
---
 .../devicetree/bindings/spi/spi_pl022.txt          | 36 ++++++++++++++++++
 drivers/spi/spi-pl022.c                            | 43 +++++++++++++++++++++-
 2 files changed, 77 insertions(+), 2 deletions(-)
drivers/spi/spi-pl022.c +4 -0
@@ -1139,6 +1139,35 @@ err_no_rxchan:
 	return -ENODEV;
 }
 
+static int pl022_dma_autoprobe(struct pl022 *pl022)
+{
+	struct device *dev = &pl022->adev->dev;
+

drivers/spi/spi-pl022.c err_no_rxchan: · line 1155

Quoted 10 lines
drivers/spi/spi-pl022.c +10 -0
+	/* automatically configure DMA channels from platform, normally using DT */
+	pl022->dma_rx_channel = dma_request_slave_channel(dev, "rx");
+	if (!pl022->dma_rx_channel)
+		goto err_no_rxchan;
+
+	pl022->dma_tx_channel = dma_request_slave_channel(dev, "tx");
+	if (!pl022->dma_tx_channel)
+		goto err_no_txchan;
+
+	pl022->dummypage = kmalloc(PAGE_SIZE, GFP_KERNEL);

Where this memory will be freed?
In dependence of the answer could you consider to use devm_kmalloc or __get_free_page?

Quoted 46 lines
drivers/spi/spi-pl022.c +27 -2
+	if (!pl022->dummypage)
+		goto err_no_dummypage;
+
+	return 0;
+
+err_no_dummypage:
+	dma_release_channel(pl022->dma_tx_channel);
+	pl022->dma_tx_channel = NULL;
+err_no_txchan:
+	dma_release_channel(pl022->dma_rx_channel);
+	pl022->dma_rx_channel = NULL;
+err_no_rxchan:
+	return -ENODEV;
+}
+		
 static void terminate_dma(struct pl022 *pl022)
 {
 	struct dma_chan *rxchan = pl022->dma_rx_channel;
@@ -1167,6 +1196,11 @@ static inline int configure_dma(struct pl022 *pl022)
 	return -ENODEV;
 }
 
+static inline int pl022_dma_autoprobe(struct pl022 *pl022)
+{
+	return 0;
+}
+
 static inline int pl022_dma_probe(struct pl022 *pl022)
 {
 	return 0;
@@ -2226,8 +2260,13 @@ static int pl022_probe(struct amba_device *adev, const struct amba_id *id)
 		goto err_no_irq;
 	}
 
-	/* Get DMA channels */
-	if (platform_info->enable_dma) {
+	/* Get DMA channels, try autoconfiguration first */
+	status = pl022_dma_autoprobe(pl022);
+
+	/* If that failed, use channels from platform_info */
+	if (status == 0)
+		platform_info->enable_dma = 1;
+	else if (platform_info->enable_dma) {
 		status = pl022_dma_probe(pl022);
 		if (status != 0)
 			platform_info->enable_dma = 0;
Signature · 2 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy
#48 lore raw

PATCH v2 2/5 spi: pl022: use generic DMA slave configuration if possible

Quoted 10 lines · Andy Shevchenko
Quoted 10 lines
drivers/spi/spi-pl022.c +10 -0
+static int pl022_dma_autoprobe(struct pl022 *pl022)
+{
+	struct device *dev = &pl022->adev->dev;
+
+	/* automatically configure DMA channels from platform, normally using DT */
+	pl022->dma_rx_channel = dma_request_slave_channel(dev, "rx");
+	if (!pl022->dma_rx_channel)
+		goto err_no_rxchan;
+
+	pl022->dma_tx_channel = dma_request_slave_channel(dev, "tx");
Quoted 8 lines

drivers/spi/spi-pl022.c err_no_rxchan: · line 1155

Quoted 4 lines
drivers/spi/spi-pl022.c +4 -0
+	if (!pl022->dma_tx_channel)
+		goto err_no_txchan;
+
+	pl022->dummypage = kmalloc(PAGE_SIZE, GFP_KERNEL);

Where this memory will be freed?
In dependence of the answer could you consider to use devm_kmalloc or __get_free_page?

There is another function like this called pl022_dma_probe() that has the same allocation, and it gets freed in the same place.

It's probably worth changing this into something different, but I felt that it didn't belong into this patch. I was also not sure if the best option would be dmam_alloc_coherent, dev_kzalloc, or __get_free_page.

Arnd

#49 lore raw

PATCH 2/5 spi: pl022: use generic DMA slave configuration if possible

Quoted 8 lines · Arnd Bergmann
Quoted 5 lines · Andy Shevchenko
Quoted 1 line

+ pl022->dummypage = kmalloc(PAGE_SIZE, GFP_KERNEL);

Where this memory will be freed?
In dependence of the answer could you consider to use devm_kmalloc or __get_free_page?

Quoted 7 lines

There is another function like this called pl022_dma_probe() that has the same allocation, and it gets freed in the same place.

It's probably worth changing this into something different, but I felt that it didn't belong into this patch. I was also not sure if the best option would be dmam_alloc_coherent, dev_kzalloc, or __get_free_page.

Actually I once read about a feature where the kernel provides a static page full of zeroes or something like this, that would be ideal to use in cases like this, then all of this dummy page allocation and freeing can be deleted.

Yours,
Linus Walleij

#50 lore raw

PATCH v2 2/5 spi: pl022: use generic DMA slave configuration if possible

Quoted 17 lines · Linus Walleij
Quoted 8 lines · Arnd Bergmann
Quoted 5 lines · Andy Shevchenko
Quoted 1 line

+ pl022->dummypage = kmalloc(PAGE_SIZE, GFP_KERNEL);

Where this memory will be freed?
In dependence of the answer could you consider to use devm_kmalloc or __get_free_page?

Quoted 7 lines

There is another function like this called pl022_dma_probe() that has the same allocation, and it gets freed in the same place.

It's probably worth changing this into something different, but I felt that it didn't belong into this patch. I was also not sure if the best option would be dmam_alloc_coherent, dev_kzalloc, or __get_free_page.

Quoted 4 lines

Actually I once read about a feature where the kernel provides a static page full of zeroes or something like this, that would be ideal to use in cases like this, then all of this dummy page allocation and freeing can be deleted.

You mean empty_zero_page? That only works if this page is read-only from the perspective of the DMA controller, but then it would be a good fit, yes.

Arnd

#51 lore raw

PATCH 2/5 spi: pl022: use generic DMA slave configuration if possible

Quoted 10 lines · Arnd Bergmann
Quoted 4 lines · Linus Walleij

Actually I once read about a feature where the kernel provides a static page full of zeroes or something like this, that would be ideal to use in cases like this, then all of this dummy page allocation and freeing can be deleted.

You mean empty_zero_page? That only works if this page is read-only from the perspective of the DMA controller, but then it would be a good fit, yes.

That's actually how it's used.

SPI is symmetric, and in the DMA case we're not poking data into the buffers from the CPU so the controller need something - anything - to stream to the block.

If we can use that page we'll even save a few remaps.

Yours,
Linus Walleij

#52 lore raw

PATCH v2 2/5 spi: pl022: use generic DMA slave configuration if possible

Quoted 12 lines · On Thursday 07 February 2013 21:19:04 Linus Walleij wrote:
Quoted 10 lines · Arnd Bergmann
Quoted 4 lines · Linus Walleij

Actually I once read about a feature where the kernel provides a static page full of zeroes or something like this, that would be ideal to use in cases like this, then all of this dummy page allocation and freeing can be deleted.

You mean empty_zero_page? That only works if this page is read-only from the perspective of the DMA controller, but then it would be a good fit, yes.

Quoted 7 lines

That's actually how it's used.

SPI is symmetric, and in the DMA case we're not poking data into the buffers from the CPU so the controller need something - anything - to stream to the block.

If we can use that page we'll even save a few remaps.

I'm slightly worried about the caching effects though. The idea of the empty-zero page is that all user processes get it when they read a page before they write to it, so the data in it can essentially always be cache-hot.

If we do DMA from that page to a device what would be the overhead of flushing the (clean) cache lines?

Arnd

#53 lore raw

PATCH 2/5 spi: pl022: use generic DMA slave configuration if possible

Quoted 21 lines · Arnd Bergmann
Quoted 12 lines · On Thursday 07 February 2013 21:19:04 Linus Walleij wrote:
Quoted 10 lines · Arnd Bergmann
Quoted 4 lines · Linus Walleij

Actually I once read about a feature where the kernel provides a static page full of zeroes or something like this, that would be ideal to use in cases like this, then all of this dummy page allocation and freeing can be deleted.

You mean empty_zero_page? That only works if this page is read-only from the perspective of the DMA controller, but then it would be a good fit, yes.

Quoted 7 lines

That's actually how it's used.

SPI is symmetric, and in the DMA case we're not poking data into the buffers from the CPU so the controller need something - anything - to stream to the block.

If we can use that page we'll even save a few remaps.

Quoted 7 lines

I'm slightly worried about the caching effects though. The idea of the empty-zero page is that all user processes get it when they read a page before they write to it, so the data in it can essentially always be cache-hot.

If we do DMA from that page to a device what would be the overhead of flushing the (clean) cache lines?

If it's DMA _to_ a device, then we will only ever clean the lines prior to a transfer, never invalidate them. So that's not really a concern. (There better not be any dirty cache lines associated with the empty zero page either.)

#54 lore raw

PATCH v2 2/5 spi: pl022: use generic DMA slave configuration if possible

Quoted 4 lines · On Friday 08 February 2013 16:22:48 Russell King - ARM Linux wrote:

If it's DMA _to_ a device, then we will only ever clean the lines prior to a transfer, never invalidate them. So that's not really a concern. (There better not be any dirty cache lines associated with the empty zero page either.)

Right, makes sense. I thought I had read about a CPU that could not flush a cache line without also invalidating it, but that must have been something other than ARM, or maybe I'm misremembering it.

Arnd

#55 lore raw

PATCH 2/5 spi: pl022: use generic DMA slave configuration if possible

Quoted 10 lines · Arnd Bergmann
Quoted 4 lines · On Friday 08 February 2013 16:22:48 Russell King - ARM Linux wrote:

If it's DMA _to_ a device, then we will only ever clean the lines prior to a transfer, never invalidate them. So that's not really a concern. (There better not be any dirty cache lines associated with the empty zero page either.)

Right, makes sense. I thought I had read about a CPU that could not flush a cache line without also invalidating it, but that must have been something other than ARM, or maybe I'm misremembering it.

I don't think it matters one bit. The page can contain a bitmap of Donald Duck or zero FWIW. It's just that the DMA controller just neeeds to read *something* that does not cause a bus stall.

It's due to the syncronous nature of the SPI protocol, to get something out you need to put something in. So when reading, this is a way to feed in some junk.

So this goes on my TODO...

Yours,
Linus Walleij

#56 lore raw

PATCH 2/5 spi: pl022: use generic DMA slave configuration if possible

In reply to Linus Walleij
Quoted 4 lines · Linus Walleij

Actually I once read about a feature where the kernel provides a static page full of zeroes or something like this, that would be ideal to use in cases like this, then all of this dummy page allocation and freeing can be deleted.

I think you're thinking of empty_zero_page which is used to provide the initial BSS pages for user apps.

#57 lore raw

PATCH v2 3/5 serial: pl011: use generic DMA slave configuration if possible

In reply to Arnd Bergmann

With the new OF DMA binding, it is possible to completely avoid the need for platform_data for configuring a DMA channel. In cases where the platform has already been converted, calling dma_request_slave_channel should get all the necessary information from the device tree.

This also adds a binding document specific to the pl011 controller, and extends the generic primecell binding to mention "dmas" and other common properties.

Like the patch that converts the dw_dma controller, this is completely untested and is looking for someone to try it out.

Cc Grant Likely <[email protected]>
Cc Jiri Slaby <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc Linus Walleij <[email protected]>
Cc spi-devel-general at lists.sourceforge.net
Cc Viresh Kumar <[email protected]>
Cc Andy Shevchenko <[email protected]>
Cc Vinod Koul <[email protected]>
Cc devicetree-discuss at lists.ozlabs.org
Cc linux-arm-kernel at lists.infradead.org
---
 .../devicetree/bindings/arm/primecell.txt          | 19 ++++++-
 Documentation/devicetree/bindings/serial/pl011.txt | 17 ++++++
 drivers/tty/serial/amba-pl011.c                    | 62 +++++++++++++---------
 3 files changed, 72 insertions(+), 26 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/serial/pl011.txt
Documentation/devicetree/bindings/arm/primecell.txt +18 -1
index 64fc82b..0df6aca 100644
@@ -16,14 +16,31 @@ Optional properties:
 - clocks : From common clock binding. First clock is phandle to clock for apb
 	pclk. Additional clocks are optional and specific to those peripherals.
 - clock-names : From common clock binding. Shall be "apb_pclk" for first clock.
+- dmas : From common DMA binding. If present, refers to one or more dma channels.
+- dma-names : From common DMA binding, needs to match the 'dmas' property.
+              Devices with exactly one receive and transmit channel shall name
+              these "rx" and "tx", respectively.
+- pinctrl-<n> : Pinctrl states as described in bindings/pinctrl/pinctrl-bindings.txt
+- pinctrl-names : Names corresponding to the numbered pinctrl states
+- interrupts : one or more interrupt specifiers
+- interrupt-names : names corresponding to the interrupts properties
 
 Example:
 
 serial at fff36000 {
 	compatible = "arm,pl011", "arm,primecell";
 	arm,primecell-periphid = <0x00341011>;
+
 	clocks = <&pclk>;
 	clock-names = "apb_pclk";
-	
+
+	dmas = <&dma-controller 4>, <&dma-controller 5>;
+	dma-names = "rx", "tx";	
+
+	pinctrl-0 = <&uart0_default_mux>, <&uart0_default_mode>;
+	pinctrl-1 = <&uart0_sleep_mode>;
+	pinctrl-names = "default","sleep";
+
+	interrupts = <0 11 0x4>;
 };
 
Documentation/devicetree/bindings/serial/pl011.txt new file +17 -0
new file mode 100644
index 0000000..5d2e840
@@ -0,0 +1,17 @@
+* ARM AMBA Primecell PL011 serial UART
+
+Required properties:
+- compatible: must be "arm,primecell", "arm,pl011"
+- reg: exactly one register range with length 0x1000
+- interrupts: exactly one interrupt specifier
+
+Optional properties:
+- pinctrl: When present, must have one state named "sleep"
+	   and one state named "default"
+- clocks:  When present, must refer to exactly one clock named
+	   "apb_pclk"
+- dmas:	   When present, may have one or two dma channels.
+	   The first one must be named "rx", the second one
+	   must be named "tx".
+
+See also bindings/arm/primecell.txt
drivers/tty/serial/amba-pl011.c +37 -25

121 lines not on this page · raw message

Signature · 1 line
1.8.0
#58 lore raw

PATCH 3/5 serial: pl011: use generic DMA slave configuration if possible

Quoted 12 lines · Arnd Bergmann

With the new OF DMA binding, it is possible to completely avoid the need for platform_data for configuring a DMA channel. In cases where the platform has already been converted, calling dma_request_slave_channel should get all the necessary information from the device tree.

This also adds a binding document specific to the pl011 controller, and extends the generic primecell binding to mention "dmas" and other common properties.

Like the patch that converts the dw_dma controller, this is completely untested and is looking for someone to try it out.

Quoted 11 lines
Cc Grant Likely <[email protected]>
Cc Jiri Slaby <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc Linus Walleij <[email protected]>
Cc spi-devel-general at lists.sourceforge.net
Cc Viresh Kumar <[email protected]>
Cc Andy Shevchenko <[email protected]>
Cc Vinod Koul <[email protected]>
Cc devicetree-discuss at lists.ozlabs.org
Cc linux-arm-kernel at lists.infradead.org
#59 lore raw

PATCH v2 4/5 ata: arasan: remove the need for platform_data

In reply to Arnd Bergmann

This adds a complete DT binding for the arasan device driver. There is currently only one user, which is the spear13xx platform, so we don't actually have to parse all the properties until another user comes in, but this does use the generic DMA binding to find the DMA channel.

The patch is untested so far and is part of a series to convert the spear platform over to use the generic DMA binding, so it should stay with the rest of the series.

Cc Viresh Kumar <[email protected]>
Cc Vinod Koul <[email protected]>
Cc Jeff Garzik <[email protected]>
Cc devicetree-discuss at lists.ozlabs.org
---
 .../devicetree/bindings/ata/pata-arasan.txt        | 22 +++++++++++++
 drivers/ata/pata_arasan_cf.c                       | 37 +++++++++++-----------
 include/linux/pata_arasan_cf_data.h                |  2 --
 3 files changed, 41 insertions(+), 20 deletions(-)
Documentation/devicetree/bindings/ata/pata-arasan.txt +22 -0
index 95ec7f8..2aff154 100644
@@ -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 CF interrupt number
+- clock-frequency: Interface clock rate, in Hz, one of
+       25000000
+       33000000
+       40000000
+       50000000
+       66000000
+       75000000
+      100000000
+      125000000
+      150000000
+      166000000
+      200000000
+
+Optional properties:
+- arasan,broken-udma: if present, UDMA mode is unusable
+- arasan,broken-mwdma: if present, MWDMA mode is unusable
+- arasan,broken-pio: if present, PIO mode is unusable
+- dmas: one DMA channel, as described in bindings/dma/dma.txt
+  required unless both UDMA and MWDMA mode are broken
+- dma-names: the corresponding channel name, must be "data"
 
 Example:
 
@@ -14,4 +34,6 @@ Example:
 		reg = <0xfc000000 0x1000>;
 		interrupt-parent = <&vic1>;
 		interrupts = <12>;
+		dmas = <&dma-controller 23>;
+		dma-names = "data";
 	};
drivers/ata/pata_arasan_cf.c +19 -18
index 405022d..7638121 100644
@@ -209,8 +209,6 @@ struct arasan_cf_dev {
 	struct dma_chan *dma_chan;
 	/* Mask for DMA transfers */
 	dma_cap_mask_t mask;
-	/* dma channel private data */
-	void *dma_priv;
 	/* DMA transfer work */
 	struct work_struct work;
 	/* DMA delayed finish work */
@@ -308,6 +306,7 @@ static void cf_card_detect(struct arasan_cf_dev *acdev, bool hotplugged)
 static int cf_init(struct arasan_cf_dev *acdev)
 {
 	struct arasan_cf_pdata *pdata = dev_get_platdata(acdev->host->dev);
+	unsigned int if_clk;
 	unsigned long flags;
 	int ret = 0;
 
@@ -325,8 +324,12 @@ static int cf_init(struct arasan_cf_dev *acdev)
 
 	spin_lock_irqsave(&acdev->host->lock, flags);
 	/* configure CF interface clock */
-	writel((pdata->cf_if_clk <= CF_IF_CLK_200M) ? pdata->cf_if_clk :
-			CF_IF_CLK_166M, acdev->vbase + CLK_CFG);
+	/* TODO: read from device tree */
+	if_clk = CF_IF_CLK_166M;
+	if (pdata && pdata->cf_if_clk <= CF_IF_CLK_200M)
+		if_clk = pdata->cf_if_clk;
+
+	writel(if_clk, acdev->vbase + CLK_CFG);
 
 	writel(TRUE_IDE_MODE | CFHOST_ENB, acdev->vbase + OP_MODE);
 	cf_interrupt_enable(acdev, CARD_DETECT_IRQ, 1);
@@ -357,12 +360,6 @@ static void dma_callback(void *dev)
 	complete(&acdev->dma_completion);
 }
 
-static bool filter(struct dma_chan *chan, void *slave)
-{
-	chan->private = slave;
-	return true;
-}
-
 static inline void dma_complete(struct arasan_cf_dev *acdev)
 {
 	struct ata_queued_cmd *qc = acdev->qc;
@@ -530,8 +527,7 @@ static void data_xfer(struct work_struct *work)
 
 	/* request dma channels */
 	/* dma_request_channel may sleep, so calling from process context */
-	acdev->dma_chan = dma_request_channel(acdev->mask, filter,
-			acdev->dma_priv);
+	acdev->dma_chan = dma_request_slave_channel(acdev->host->dev, "data");
 	if (!acdev->dma_chan) {
 		dev_err(acdev->host->dev, "Unable to get dma_chan\n");
 		goto chan_request_fail;
@@ -798,6 +794,7 @@ static int arasan_cf_probe(struct platform_device *pdev)
 	struct ata_host *host;
 	struct ata_port *ap;
 	struct resource *res;
+	u32 quirk;
 	irq_handler_t irq_handler = NULL;
 	int ret = 0;
 
@@ -817,12 +814,17 @@ static int arasan_cf_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	if (pdata)
+		quirk = pdata->quirk;
+	else
+		quirk = CF_BROKEN_UDMA; /* as it is on spear1340 */
+
 	/* if irq is 0, support only PIO */
 	acdev->irq = platform_get_irq(pdev, 0);
 	if (acdev->irq)
 		irq_handler = arasan_cf_interrupt;
 	else
-		pdata->quirk |= CF_BROKEN_MWDMA | CF_BROKEN_UDMA;
+		quirk |= CF_BROKEN_MWDMA | CF_BROKEN_UDMA;
 
 	acdev->pbase = res->start;
 	acdev->vbase = devm_ioremap_nocache(&pdev->dev, res->start,
@@ -859,17 +861,16 @@ static int arasan_cf_probe(struct platform_device *pdev)
 	INIT_WORK(&acdev->work, data_xfer);
 	INIT_DELAYED_WORK(&acdev->dwork, delayed_finish);
 	dma_cap_set(DMA_MEMCPY, acdev->mask);
-	acdev->dma_priv = pdata->dma_priv;
 
 	/* Handle platform specific quirks */
-	if (pdata->quirk) {
-		if (pdata->quirk & CF_BROKEN_PIO) {
+	if (quirk) {
+		if (quirk & CF_BROKEN_PIO) {
 			ap->ops->set_piomode = NULL;
 			ap->pio_mask = 0;
 		}
-		if (pdata->quirk & CF_BROKEN_MWDMA)
+		if (quirk & CF_BROKEN_MWDMA)
 			ap->mwdma_mask = 0;
-		if (pdata->quirk & CF_BROKEN_UDMA)
+		if (quirk & CF_BROKEN_UDMA)
 			ap->udma_mask = 0;
 	}
 	ap->flags |= ATA_FLAG_PIO_POLLING | ATA_FLAG_NO_ATAPI;
include/linux/pata_arasan_cf_data.h +0 -2
index a7b4fc3..3cc21c9 100644
@@ -37,8 +37,6 @@ struct arasan_cf_pdata {
 	#define CF_BROKEN_PIO			(1)
 	#define CF_BROKEN_MWDMA			(1 << 1)
 	#define CF_BROKEN_UDMA			(1 << 2)
-	/* This is platform specific data for the DMA controller */
-	void *dma_priv;
 };
 
 static inline void
Signature · 1 line
1.8.0
#60 lore raw

PATCH 4/5 ata: arasan: remove the need for platform_data

Quoted 8 lines · Arnd Bergmann

This adds a complete DT binding for the arasan device driver. There is currently only one user, which is the spear13xx platform, so we don't actually have to parse all the properties until another user comes in, but this does use the generic DMA binding to find the DMA channel.

The patch is untested so far and is part of a series to convert the spear platform over to use the generic DMA binding, so it should stay with the rest of the series.

#61 lore raw

PATCH v2 5/5 ARM: SPEAr13xx: Pass generic DW DMAC platform data from DT

In reply to Arnd Bergmann

This replaces an earlier patch from Viresh Kumar to move the spear platform over to the generic DMA binding. This version is now based on the merged multiplatform capable spear platform, rather than the separate spear13xx/3xx/6xx directories.

Cc Viresh Kumar <[email protected]>
Cc Vinod Koul <[email protected]>
Cc Andy Shevchenko <[email protected]>
Cc devicetree-discuss at lists.ozlabs.org
---
 arch/arm/boot/dts/spear1340.dtsi         |   3 +
 arch/arm/boot/dts/spear13xx.dtsi         |  25 +++++-
 arch/arm/mach-spear/generic.h            |   6 --
 arch/arm/mach-spear/include/mach/spear.h |   2 -
 arch/arm/mach-spear/spear1310.c          |  30 +-------
 arch/arm/mach-spear/spear1340.c          |  32 +-------
 arch/arm/mach-spear/spear13xx-dma.h      | 128 -------------------------------
 arch/arm/mach-spear/spear13xx.c          |  58 --------------
 8 files changed, 29 insertions(+), 255 deletions(-)
 delete mode 100644 arch/arm/mach-spear/spear13xx-dma.h
arch/arm/boot/dts/spear1340.dtsi +3 -0
index 34da11a..e1786a0 100644
@@ -113,6 +113,9 @@
 				reg = <0xb4100000 0x1000>;
 				interrupts = <0 105 0x4>;
 				status = "disabled";
+				dmas = <&dwdma0 0x600 0 0 1>, /* 0xC << 11 */
+					<&dwdma0 0x680 0 1 0>; /* 0xD << 7 */
+				dma-names = "tx", "rx";
 			};
 
 			thermal@e07008c4 {
arch/arm/boot/dts/spear13xx.dtsi +24 -1
index b4ca60f..45597fd 100644
@@ -98,13 +98,24 @@
 			reg = <0xb2800000 0x1000>;
 			interrupts = <0 29 0x4>;
 			status = "disabled";
+			dmas = <&dwdma0 0 0 0 0>;
+			dma-names = "data";
 		};
 
-		dma at ea800000 {
+		dwdma0: dma at ea800000 {
 			compatible = "snps,dma-spear1340";
 			reg = <0xea800000 0x1000>;
 			interrupts = <0 19 0x4>;
 			status = "disabled";
+
+			dma-channels = <8>;
+			#dma-cells = <3>;
+			dma-requests = <32>;
+			chan_allocation_order = <1>;
+			chan_priority = <1>;
+			block_size = <0xfff>;
+			dma-masters = <2>;
+			data_width = <3 3 0 0>;
 		};
 
 		dma at eb000000 {
@@ -112,6 +123,15 @@
 			reg = <0xeb000000 0x1000>;
 			interrupts = <0 59 0x4>;
 			status = "disabled";
+
+			dma-requests = <32>;
+			dma-channels = <8>;
+			dma-masters = <2>;
+			#dma-cells = <3>;
+			chan_allocation_order = <1>;
+			chan_priority = <1>;
+			block_size = <0xfff>;
+			data_width = <3 3 0 0>;
 		};
 
 		fsmc: flash at b0000000 {
@@ -261,6 +281,9 @@
 				#size-cells = <0>;
 				interrupts = <0 31 0x4>;
 				status = "disabled";
+				dmas = <&dwdma0 0x2000 0 0 0>, /* 0x4 << 11 */
+					<&dwdma0 0x0280 0 0 0>;  /* 0x5 << 7 */
+				dma-names = "tx", "rx";
 			};
 
 			rtc@e0580000 {
arch/arm/mach-spear/generic.h +0 -6
index 32b7d00..01c6ca3 100644
@@ -20,13 +20,7 @@
 
 extern struct sys_timer spear13xx_timer;
 extern struct sys_timer spear3xx_timer;
-extern struct pl022_ssp_controller pl022_plat_data;
 extern struct pl08x_platform_data pl080_plat_data;
-extern struct dw_dma_platform_data dmac_plat_data;
-extern struct dw_dma_slave cf_dma_priv;
-extern struct dw_dma_slave nand_read_dma_priv;
-extern struct dw_dma_slave nand_write_dma_priv;
-bool dw_dma_filter(struct dma_chan *chan, void *slave);
 
 void __init spear_setup_of_timer(void);
 void __init spear3xx_clk_init(void __iomem *misc_base,
arch/arm/mach-spear/include/mach/spear.h +0 -2
index 374ddc3..cf3a536 100644
@@ -82,8 +82,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)
 
 /* Debug uart for linux, will be used for debug and uncompress messages */
arch/arm/mach-spear/spear1310.c +1 -29
index 7c4ed3c..2622b1e 100644
@@ -23,40 +23,12 @@
 #include <mach/spear.h>
 
 /* Base addresses */
-#define SPEAR1310_SSP1_BASE			UL(0x5D400000)
-#define SPEAR1310_SATA0_BASE			UL(0xB1000000)
-#define SPEAR1310_SATA1_BASE			UL(0xB1800000)
-#define SPEAR1310_SATA2_BASE			UL(0xB4000000)
-
 #define SPEAR1310_RAS_GRP1_BASE			UL(0xD8000000)
 #define VA_SPEAR1310_RAS_GRP1_BASE		UL(0xFA000000)
 
-static struct arasan_cf_pdata cf_pdata = {
-	.cf_if_clk = CF_IF_CLK_166M,
-	.quirk = CF_BROKEN_UDMA,
-	.dma_priv = &cf_dma_priv,
-};
-
-/* ssp device registration */
-static struct pl022_ssp_controller ssp1_plat_data = {
-	.enable_dma = 0,
-};
-
-/* 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_pdata),
-	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("arm,pl022", SSP_BASE, NULL, &pl022_plat_data),
-
-	OF_DEV_AUXDATA("arm,pl022", SPEAR1310_SSP1_BASE, NULL, &ssp1_plat_data),
-	{}
-};
-
 static void __init spear1310_dt_init(void)
 {
-	of_platform_populate(NULL, of_default_bus_match_table,
-			spear1310_auxdata_lookup, NULL);
+	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 }
 
 static const char * const spear1310_dt_board_compat[] = {
arch/arm/mach-spear/spear1340.c +1 -31
index 98d509c..3fb3fdf 100644
@@ -16,7 +16,6 @@
 #include <linux/ahci_platform.h>
 #include <linux/amba/serial.h>
 #include <linux/delay.h>
-#include <linux/dw_dmac.h>
 #include <linux/of_platform.h>
 #include <linux/pata_arasan_cf_data.h>
 #include <linux/irqchip.h>
@@ -24,11 +23,10 @@
 #include "generic.h"
 #include <mach/spear.h>
 
-#include "spear13xx-dma.h"
+/* FIXME: Move SATA PHY code into a standalone driver */
 
 /* Base addresses */
 #define SPEAR1340_SATA_BASE			UL(0xB1000000)
-#define SPEAR1340_UART1_BASE			UL(0xB4100000)
 
 /* Power Management Registers */
 #define SPEAR1340_PCM_CFG			(VA_MISC_BASE + 0x100)
@@ -80,28 +78,6 @@
 			(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],
-};
-
 /* SATA device registration */
 static int sata_miphy_init(struct device *dev, void __iomem *addr)
 {
@@ -160,14 +136,8 @@ 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("arm,pl022", SSP_BASE, NULL, &pl022_plat_data),
-
 	OF_DEV_AUXDATA("snps,spear-ahci", SPEAR1340_SATA_BASE, NULL,
 			&sata_pdata),
-	OF_DEV_AUXDATA("arm,pl011", SPEAR1340_UART1_BASE, NULL, &uart1_data),
 	{}
 };
 
arch/arm/mach-spear/spear13xx-dma.h deleted +0 -128

128 lines not on this page · raw message

arch/arm/mach-spear/spear13xx.c +0 -58

70 lines not on this page · raw message

Signature · 1 line
1.8.0
#62 lore raw

PATCH 5/5 ARM: SPEAr13xx: Pass generic DW DMAC platform data from DT

You forgot spear-devel for this series.

Quoted 48 lines · Arnd Bergmann

This replaces an earlier patch from Viresh Kumar to move the spear platform over to the generic DMA binding. This version is now based on the merged multiplatform capable spear platform, rather than the separate spear13xx/3xx/6xx directories.

Cc Viresh Kumar <[email protected]>
Cc Vinod Koul <[email protected]>
Cc Andy Shevchenko <[email protected]>
Cc devicetree-discuss at lists.ozlabs.org
---
 arch/arm/boot/dts/spear1340.dtsi         |   3 +
 arch/arm/boot/dts/spear13xx.dtsi         |  25 +++++-
 arch/arm/mach-spear/generic.h            |   6 --
 arch/arm/mach-spear/include/mach/spear.h |   2 -
 arch/arm/mach-spear/spear1310.c          |  30 +-------
 arch/arm/mach-spear/spear1340.c          |  32 +-------
 arch/arm/mach-spear/spear13xx-dma.h      | 128 -------------------------------
 arch/arm/mach-spear/spear13xx.c          |  58 --------------
 8 files changed, 29 insertions(+), 255 deletions(-)
 delete mode 100644 arch/arm/mach-spear/spear13xx-dma.h
arch/arm/boot/dts/spear1340.dtsi +3 -0
index 34da11a..e1786a0 100644
@@ -113,6 +113,9 @@
                                reg = <0xb4100000 0x1000>;
                                interrupts = <0 105 0x4>;
                                status = "disabled";
+                               dmas = <&dwdma0 0x600 0 0 1>, /* 0xC << 11 */
+                                       <&dwdma0 0x680 0 1 0>; /* 0xD << 7 */
+                               dma-names = "tx", "rx";
                        };
 
                        thermal at e07008c4 {
arch/arm/boot/dts/spear13xx.dtsi +2 -0
index b4ca60f..45597fd 100644
@@ -98,13 +98,24 @@
                        reg = <0xb2800000 0x1000>;
                        interrupts = <0 29 0x4>;
                        status = "disabled";
+                       dmas = <&dwdma0 0 0 0 0>;
+                       dma-names = "data";
                };

arch/arm/boot/dts/spear13xx.dtsi line 112

Quoted 10 lines
arch/arm/boot/dts/spear13xx.dtsi +4 -1
 
-               dma at ea800000 {
+               dwdma0: dma at ea800000 {
                        compatible = "snps,dma-spear1340";
                        reg = <0xea800000 0x1000>;
                        interrupts = <0 19 0x4>;
                        status = "disabled";
+
+                       dma-channels = <8>;
+                       #dma-cells = <3>;

4?

#63 lore raw

PATCH v2 5/5 ARM: SPEAr13xx: Pass generic DW DMAC platform data from DT

Quoted 1 line · Viresh Kumar

You forgot spear-devel for this series.

Ok, thanks for adding it.

Quoted 2 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+                       dma-channels = <8>;
+                       #dma-cells = <3>;

Yep, my mistake again. It was correct in v1 of the patch and then I changed it so it had to be 4.

Arnd