[PATCH v2 0/5] dmaengine: convert dw_dmac/spear13xx to generic binding

[PATCH v2 0/5] dmaengine: convert dw_dmac/spear13xx to generic binding

[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

-- 
1.8.0

Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jeff Garzik <jgarzik@redhat.com>
Cc: Jon Hunter <jon-hunter@ti.com>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Vinod Koul <vinod.koul@linux.intel.com>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: devicetree-discuss at lists.ozlabs.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: spi-devel-general at lists.sourceforge.net

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

[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.

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

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Vinod Koul <vinod.koul@linux.intel.com>
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(-)

diff --git a/Documentation/devicetree/bindings/dma/snps-dma.txt b/Documentation/devicetree/bindings/dma/snps-dma.txt
index 5bb3dfb..212d387 100644
--- a/Documentation/devicetree/bindings/dma/snps-dma.txt
+++ b/Documentation/devicetree/bindings/dma/snps-dma.txt
@@ -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";
 	};
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 8cfaaf8..88504c2 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -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>
@@ -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;
+
+struct dw_dma_filter_args {
+	struct dw_dma *dw;
+	u32 cfg_lo;
+	u32 cfg_hi;
+	unsigned src;
+	unsigned dst;
+};
+
+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
-	 * 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;
+	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
-	 */
-	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);
+	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;
+}
+
+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)
+		return NULL;
+
+	/* FIXME: This binding is rather clumsy. Can't we use the
+	   request line numbers here instead? */
+	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]);
 }
-EXPORT_SYMBOL(dw_dma_generic_filter);
 
 /* --------------------- Cyclic DMA API extensions -------------------- */
 
@@ -1510,9 +1531,8 @@ static void dw_dma_off(struct dw_dma *dw)
 static struct dw_dma_platform_data *
 dw_dma_parse_dt(struct platform_device *pdev)
 {
-	struct device_node *sn, *cn, *np = pdev->dev.of_node;
+	struct device_node *np = pdev->dev.of_node;
 	struct dw_dma_platform_data *pdata;
-	struct dw_dma_slave *sd;
 	u32 tmp, arr[4];
 
 	if (!np) {
@@ -1524,7 +1544,7 @@ dw_dma_parse_dt(struct platform_device *pdev)
 	if (!pdata)
 		return NULL;
 
-	if (of_property_read_u32(np, "nr_channels", &pdata->nr_channels))
+	if (of_property_read_u32(np, "dma-channels", &pdata->nr_channels))
 		return NULL;
 
 	if (of_property_read_bool(np, "is_private"))
@@ -1539,7 +1559,7 @@ dw_dma_parse_dt(struct platform_device *pdev)
 	if (!of_property_read_u32(np, "block_size", &tmp))
 		pdata->block_size = tmp;
 
-	if (!of_property_read_u32(np, "nr_masters", &tmp)) {
+	if (!of_property_read_u32(np, "dma-masters", &tmp)) {
 		if (tmp > 4)
 			return NULL;
 
@@ -1551,36 +1571,6 @@ dw_dma_parse_dt(struct platform_device *pdev)
 		for (tmp = 0; tmp < pdata->nr_masters; tmp++)
 			pdata->data_width[tmp] = arr[tmp];
 
-	/* parse slave data */
-	sn = of_find_node_by_name(np, "slave_info");
-	if (!sn)
-		return pdata;
-
-	/* calculate number of slaves */
-	tmp = of_get_child_count(sn);
-	if (!tmp)
-		return NULL;
-
-	sd = devm_kzalloc(&pdev->dev, sizeof(*sd) * tmp, GFP_KERNEL);
-	if (!sd)
-		return NULL;
-
-	pdata->sd = sd;
-	pdata->sd_count = tmp;
-
-	for_each_child_of_node(sn, cn) {
-		sd->dma_dev = &pdev->dev;
-		of_property_read_string(cn, "bus_id", &sd->bus_id);
-		of_property_read_u32(cn, "cfg_hi", &sd->cfg_hi);
-		of_property_read_u32(cn, "cfg_lo", &sd->cfg_lo);
-		if (!of_property_read_u32(cn, "src_master", &tmp))
-			sd->src_master = tmp;
-
-		if (!of_property_read_u32(cn, "dst_master", &tmp))
-			sd->dst_master = tmp;
-		sd++;
-	}
-
 	return pdata;
 }
 #else
@@ -1644,8 +1634,6 @@ static int dw_probe(struct platform_device *pdev)
 	clk_prepare_enable(dw->clk);
 
 	dw->regs = regs;
-	dw->sd = pdata->sd;
-	dw->sd_count = pdata->sd_count;
 
 	/* get hardware configuration parameters */
 	if (autocfg) {
@@ -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);
+
+	return err;
 }
 
 static int dw_remove(struct platform_device *pdev)
@@ -1773,6 +1765,7 @@ static int dw_remove(struct platform_device *pdev)
 	struct dw_dma		*dw = platform_get_drvdata(pdev);
 	struct dw_dma_chan	*dwc, *_dwc;
 
+	of_dma_controller_free(pdev->dev.of_node);
 	dw_dma_off(dw);
 	dma_async_device_unregister(&dw->dma);
 
diff --git a/drivers/dma/dw_dmac_regs.h b/drivers/dma/dw_dmac_regs.h
index 88a069f..8896559 100644
--- a/drivers/dma/dw_dmac_regs.h
+++ b/drivers/dma/dw_dmac_regs.h
@@ -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 */
diff --git a/include/linux/dw_dmac.h b/include/linux/dw_dmac.h
index 41766de..481ab23 100644
--- a/include/linux/dw_dmac.h
+++ b/include/linux/dw_dmac.h
@@ -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 */
-- 
1.8.0

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

[Viresh Kumar]

On 29 January 2013 03:28, Arnd Bergmann <arnd@arndb.de> wrote:
> 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.

> diff --git a/Documentation/devicetree/bindings/dma/snps-dma.txt b/Documentation/devicetree/bindings/dma/snps-dma.txt
> index 5bb3dfb..212d387 100644
> --- a/Documentation/devicetree/bindings/dma/snps-dma.txt
> +++ b/Documentation/devicetree/bindings/dma/snps-dma.txt
> @@ -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>

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

> +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.

> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> index 8cfaaf8..88504c2 100644
> --- a/drivers/dma/dw_dmac.c
> +++ b/drivers/dma/dw_dmac.c
> @@ -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>
> @@ -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.

> +
> +struct dw_dma_filter_args {
> +       struct dw_dma *dw;
> +       u32 cfg_lo;
> +       u32 cfg_hi;
> +       unsigned src;

Currently named as sms

> +       unsigned dst;

dms

> +};
> +
> +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
> -        * 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?

> +       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
> -        */
> -       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.

> +       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;
> +}
> +
> +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?

> +               return NULL;
> +
> +       /* FIXME: This binding is rather clumsy. Can't we use the
> +          request line numbers here instead? */

yes.

> +       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?

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

[Arnd Bergmann]

On Tuesday 29 January 2013, Viresh Kumar wrote:
> > diff --git a/Documentation/devicetree/bindings/dma/snps-dma.txt b/Documentation/devicetree/bindings/dma/snps-dma.txt
> > index 5bb3dfb..212d387 100644
> > --- a/Documentation/devicetree/bindings/dma/snps-dma.txt
> > +++ b/Documentation/devicetree/bindings/dma/snps-dma.txt
> > @@ -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>
> 
> 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.

> > -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.

> > -       /*
> > -        * dmaengine framework calls this routine for all channels of all dma
> > -        * controller, until true is returned. If 'param' bus_id is not
> > -        * registered with a dma controller (dw), then there is no need of
> > -        * running below function for all channels of dw.
> > -        *
> > -        * This block of code does this by saving the parameters of last
> > -        * failure. If dw and param are same, i.e. trying on same dw with
> > -        * different channel, return false.
> > -        */
> > -       if ((last_dw == dw) && (last_bus_id == param))
> > +       /* 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?

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

> > -       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?

> > +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?

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

> > +       /* 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?

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

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

[Viresh Kumar]

On 29 January 2013 16:05, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 29 January 2013, Viresh Kumar wrote:

>> 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.

>> > +       /* 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 :)

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

>> > +       /* 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.

> 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.

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

[Andy Shevchenko]

On Tue, 2013-01-29 at 16:19 +0530, Viresh Kumar wrote: 
> On 29 January 2013 16:05, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tuesday 29 January 2013, Viresh Kumar wrote:

> >> > +       /* 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;

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(...

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

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

[Viresh Kumar]

On 29 January 2013 16:24, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Tue, 2013-01-29 at 16:19 +0530, Viresh Kumar wrote:
>> 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(...

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

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

[Andy Shevchenko]

On Tue, 2013-01-29 at 16:27 +0530, Viresh Kumar wrote: 
> On 29 January 2013 16:24, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Tue, 2013-01-29 at 16:19 +0530, Viresh Kumar wrote:
> >> 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(...
> 
> 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.


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

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

[Arnd Bergmann]

On Tuesday 29 January 2013, Viresh Kumar wrote:
> 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.

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.

> > 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

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

[Andy Shevchenko]

On Tue, 2013-01-29 at 13:31 +0000, Arnd Bergmann wrote: 
> On Tuesday 29 January 2013, Viresh Kumar wrote:
> > 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.
> 
> 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.

> 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.

>  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.


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

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

[Russell King - ARM Linux]

On Tue, Jan 29, 2013 at 03:45:40PM +0200, Andy Shevchenko wrote:
> On Tue, 2013-01-29 at 13:31 +0000, Arnd Bergmann wrote: 
> > On Tuesday 29 January 2013, Viresh Kumar wrote:
> > > 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.
> > 
> > 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.)

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

[Arnd Bergmann]

On Tuesday 29 January 2013, Andy Shevchenko wrote:
> On Tue, 2013-01-29 at 13:31 +0000, Arnd Bergmann wrote: 
> > 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.

> > 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.

> >  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

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

[Viresh Kumar]

On 29 January 2013 19:01, Arnd Bergmann <arnd@arndb.de> wrote:
> Ah, good. So I guess the "dma-requests" property should actually
> be "16" then.

yes, even i was checking on that separately :)

> 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.

> 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 :)

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

not required.

> 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

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

[Arnd Bergmann]

On Tuesday 29 January 2013, Viresh Kumar wrote:
> On 29 January 2013 19:01, Arnd Bergmann <arnd@arndb.de> wrote:
> > 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?
 
> > 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 :)
> 
> > 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

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

[Viresh Kumar]

On 29 January 2013 21:51, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 29 January 2013, Viresh Kumar wrote:
>> On 29 January 2013 19:01, Arnd Bergmann <arnd@arndb.de> wrote:
>> > 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?

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. 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.

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

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

[Arnd Bergmann]

On Wednesday 30 January 2013, Viresh Kumar wrote:
> On 29 January 2013 21:51, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tuesday 29 January 2013, Viresh Kumar wrote:
> >> On 29 January 2013 19:01, Arnd Bergmann <arnd@arndb.de> wrote:
> >> > 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?
> 
> 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.

> > 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.
> 
> 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

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

[Viresh Kumar]

On Wed, Jan 30, 2013 at 3:11 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 30 January 2013, Viresh Kumar wrote:
>> 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.

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 :)

>> > 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.
>>
>> 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..

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

[Arnd Bergmann]

On Wednesday 30 January 2013, Viresh Kumar wrote:
> On Wed, Jan 30, 2013 at 3:11 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Wednesday 30 January 2013, Viresh Kumar wrote:
> >> 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.
> 
> 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.
 
> 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

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

[Viresh Kumar]

On 30 January 2013 15:38, Arnd Bergmann <arnd@arndb.de> wrote:
> 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 :)

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

[Arnd Bergmann]

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

On Tuesday 29 January 2013, Andy Shevchenko wrote:
> On Mon, 2013-01-28 at 21:58 +0000, Arnd Bergmann wrote: 
> > -	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.

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

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

[Russell King - ARM Linux]

On Tue, Jan 29, 2013 at 10:50:23AM +0000, Arnd Bergmann wrote:
> (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.

> On Tuesday 29 January 2013, Andy Shevchenko wrote:
> > On Mon, 2013-01-28 at 21:58 +0000, Arnd Bergmann wrote: 
> > > -	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?

> 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?

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

[Arnd Bergmann]

On Tuesday 29 January 2013, Russell King - ARM Linux wrote:
> > On Tuesday 29 January 2013, Andy Shevchenko wrote:
> > > On Mon, 2013-01-28 at 21:58 +0000, Arnd Bergmann wrote: 
> > > > - 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.

> > 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

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

[Russell King - ARM Linux]

On Tue, Jan 29, 2013 at 01:44:10PM +0000, Arnd Bergmann wrote:
> 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;
> 		...
> 	};

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.

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

[Arnd Bergmann]

On Tuesday 29 January 2013, Russell King - ARM Linux wrote:
> 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";
		};
	};

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

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

[Russell King - ARM Linux]

On Tue, Jan 29, 2013 at 02:55:49PM +0000, Arnd Bergmann wrote:
> On Tuesday 29 January 2013, Russell King - ARM Linux wrote:
> > 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";
> 		};
> 	};
> 
> 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.

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

[Arnd Bergmann]

On Tuesday 29 January 2013, Russell King - ARM Linux wrote:
> 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...

Ok.

> 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

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

[Russell King - ARM Linux]

On Tue, Jan 29, 2013 at 04:36:38PM +0000, Arnd Bergmann wrote:
> 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.

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

[Arnd Bergmann]

On Tuesday 29 January 2013, Russell King - ARM Linux wrote:
> 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

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

[Linus Walleij]

On Tue, Jan 29, 2013 at 9:40 PM, Arnd Bergmann <arnd@arndb.de> wrote:

> 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.

> 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.

> 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

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

[Andy Shevchenko]

On Mon, 2013-01-28 at 21:58 +0000, Arnd Bergmann wrote: 
> 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.

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 :-) )

> --- a/drivers/dma/dw_dmac.c
> +++ b/drivers/dma/dw_dmac.c

> @@ -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.


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

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

[Arnd Bergmann]

On Friday 15 February 2013, Andy Shevchenko wrote:
> 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.

> > --- a/drivers/dma/dw_dmac.c
> > +++ b/drivers/dma/dw_dmac.c
> 
> > @@ -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.

Ok, makes sense. Thanks!

	Arnd

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

[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.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: spi-devel-general at lists.sourceforge.net
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Vinod Koul <vinod.koul@linux.intel.com>
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(-)

diff --git a/Documentation/devicetree/bindings/spi/spi_pl022.txt b/Documentation/devicetree/bindings/spi/spi_pl022.txt
index f158fd3..22ed679 100644
--- a/Documentation/devicetree/bindings/spi/spi_pl022.txt
+++ b/Documentation/devicetree/bindings/spi/spi_pl022.txt
@@ -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>;
+		};
+	};
+	
diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
index b0fe393..371cc66f 100644
--- a/drivers/spi/spi-pl022.c
+++ b/drivers/spi/spi-pl022.c
@@ -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;
-- 
1.8.0

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

[Mark Brown]

On Mon, Jan 28, 2013 at 09:58:17PM +0000, Arnd Bergmann wrote:
> 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.

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

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>

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

[Andy Shevchenko]

On Mon, 2013-01-28 at 21:58 +0000, Arnd Bergmann wrote: 
> 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.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Cc: spi-devel-general at lists.sourceforge.net
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Vinod Koul <vinod.koul@linux.intel.com>
> 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(-)
> 

> --- a/drivers/spi/spi-pl022.c
> +++ b/drivers/spi/spi-pl022.c
> @@ -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);

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

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

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

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

[Arnd Bergmann]

On Tuesday 29 January 2013, Andy Shevchenko wrote:

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

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

[Linus Walleij]

On Tue, Jan 29, 2013 at 2:13 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 29 January 2013, Andy Shevchenko wrote:

>> > +   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.

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

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

[Arnd Bergmann]

On Thursday 07 February 2013, Linus Walleij wrote:
> On Tue, Jan 29, 2013 at 2:13 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tuesday 29 January 2013, Andy Shevchenko wrote:
> 
> >> > +   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.
> 
> 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

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

[Linus Walleij]

On Thu, Feb 7, 2013 at 8:42 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 07 February 2013, Linus Walleij wrote:

>> 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

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

[Arnd Bergmann]

On Thursday 07 February 2013 21:19:04 Linus Walleij wrote:
> On Thu, Feb 7, 2013 at 8:42 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thursday 07 February 2013, Linus Walleij wrote:
> 
> >> 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.

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

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

[Russell King - ARM Linux]

On Thu, Feb 07, 2013 at 10:15:48PM +0100, Arnd Bergmann wrote:
> On Thursday 07 February 2013 21:19:04 Linus Walleij wrote:
> > On Thu, Feb 7, 2013 at 8:42 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Thursday 07 February 2013, Linus Walleij wrote:
> > 
> > >> 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.
> 
> 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.)

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

[Arnd Bergmann]

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

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

[Linus Walleij]

On Fri, Feb 8, 2013 at 5:28 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> 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

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

[Russell King - ARM Linux]

On Thu, Feb 07, 2013 at 07:29:17PM +0100, Linus Walleij wrote:
> 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.

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

[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.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: spi-devel-general at lists.sourceforge.net
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Vinod Koul <vinod.koul@linux.intel.com>
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

diff --git a/Documentation/devicetree/bindings/arm/primecell.txt b/Documentation/devicetree/bindings/arm/primecell.txt
index 64fc82b..0df6aca 100644
--- a/Documentation/devicetree/bindings/arm/primecell.txt
+++ b/Documentation/devicetree/bindings/arm/primecell.txt
@@ -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>;
 };
 
diff --git a/Documentation/devicetree/bindings/serial/pl011.txt b/Documentation/devicetree/bindings/serial/pl011.txt
new file mode 100644
index 0000000..5d2e840
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/pl011.txt
@@ -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
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 7fca402..f9af04d 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -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
 
@@ -2023,7 +2035,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);
-- 
1.8.0

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

[Greg Kroah-Hartman]

On Mon, Jan 28, 2013 at 09:58:18PM +0000, Arnd Bergmann wrote:
> 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.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Jiri Slaby <jslaby@suse.cz>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: spi-devel-general at lists.sourceforge.net
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Vinod Koul <vinod.koul@linux.intel.com>
> Cc: devicetree-discuss at lists.ozlabs.org
> Cc: linux-arm-kernel at lists.infradead.org

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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

[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.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Viresh Kumar <viresh.linux@gmail.com>
Cc: Vinod Koul <vinod.koul@intel.com>
Cc: Jeff Garzik <jgarzik@redhat.com>
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(-)

diff --git a/Documentation/devicetree/bindings/ata/pata-arasan.txt b/Documentation/devicetree/bindings/ata/pata-arasan.txt
index 95ec7f8..2aff154 100644
--- a/Documentation/devicetree/bindings/ata/pata-arasan.txt
+++ b/Documentation/devicetree/bindings/ata/pata-arasan.txt
@@ -6,6 +6,26 @@ Required properties:
 - interrupt-parent: Should be the phandle for the interrupt controller
   that services interrupts for this device
 - interrupt: Should contain the 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";
 	};
diff --git a/drivers/ata/pata_arasan_cf.c b/drivers/ata/pata_arasan_cf.c
index 405022d..7638121 100644
--- a/drivers/ata/pata_arasan_cf.c
+++ b/drivers/ata/pata_arasan_cf.c
@@ -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;
diff --git a/include/linux/pata_arasan_cf_data.h b/include/linux/pata_arasan_cf_data.h
index a7b4fc3..3cc21c9 100644
--- a/include/linux/pata_arasan_cf_data.h
+++ b/include/linux/pata_arasan_cf_data.h
@@ -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
-- 
1.8.0

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

[Viresh Kumar]

On 29 January 2013 03:28, Arnd Bergmann <arnd@arndb.de> wrote:
> 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.

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

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

[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.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Vinod Koul <vinod.koul@linux.intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
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

diff --git a/arch/arm/boot/dts/spear1340.dtsi b/arch/arm/boot/dts/spear1340.dtsi
index 34da11a..e1786a0 100644
--- a/arch/arm/boot/dts/spear1340.dtsi
+++ b/arch/arm/boot/dts/spear1340.dtsi
@@ -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 {
diff --git a/arch/arm/boot/dts/spear13xx.dtsi b/arch/arm/boot/dts/spear13xx.dtsi
index b4ca60f..45597fd 100644
--- a/arch/arm/boot/dts/spear13xx.dtsi
+++ b/arch/arm/boot/dts/spear13xx.dtsi
@@ -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 {
diff --git a/arch/arm/mach-spear/generic.h b/arch/arm/mach-spear/generic.h
index 32b7d00..01c6ca3 100644
--- a/arch/arm/mach-spear/generic.h
+++ b/arch/arm/mach-spear/generic.h
@@ -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,
diff --git a/arch/arm/mach-spear/include/mach/spear.h b/arch/arm/mach-spear/include/mach/spear.h
index 374ddc3..cf3a536 100644
--- a/arch/arm/mach-spear/include/mach/spear.h
+++ b/arch/arm/mach-spear/include/mach/spear.h
@@ -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 */
diff --git a/arch/arm/mach-spear/spear1310.c b/arch/arm/mach-spear/spear1310.c
index 7c4ed3c..2622b1e 100644
--- a/arch/arm/mach-spear/spear1310.c
+++ b/arch/arm/mach-spear/spear1310.c
@@ -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[] = {
diff --git a/arch/arm/mach-spear/spear1340.c b/arch/arm/mach-spear/spear1340.c
index 98d509c..3fb3fdf 100644
--- a/arch/arm/mach-spear/spear1340.c
+++ b/arch/arm/mach-spear/spear1340.c
@@ -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),
 	{}
 };
 
diff --git a/arch/arm/mach-spear/spear13xx-dma.h b/arch/arm/mach-spear/spear13xx-dma.h
deleted file mode 100644
index d50bdb6..0000000
--- a/arch/arm/mach-spear/spear13xx-dma.h
+++ /dev/null
@@ -1,128 +0,0 @@
-/*
- * arch/arm/mach-spear13xx/include/mach/dma.h
- *
- * DMA information for SPEAr13xx machine family
- *
- * Copyright (C) 2012 ST Microelectronics
- * Viresh Kumar <viresh.linux@gmail.com>
- *
- * This file is licensed under the terms of the GNU General Public
- * License version 2. This program is licensed "as is" without any
- * warranty of any kind, whether express or implied.
- */
-
-#ifndef __MACH_DMA_H
-#define __MACH_DMA_H
-
-/* request id of all the peripherals */
-enum dma_master_info {
-	/* Accessible from only one master */
-	DMA_MASTER_MCIF = 0,
-	DMA_MASTER_FSMC = 1,
-	/* Accessible from both 0 & 1 */
-	DMA_MASTER_MEMORY = 0,
-	DMA_MASTER_ADC = 0,
-	DMA_MASTER_UART0 = 0,
-	DMA_MASTER_SSP0 = 0,
-	DMA_MASTER_I2C0 = 0,
-
-#ifdef CONFIG_MACH_SPEAR1310
-	/* Accessible from only one master */
-	SPEAR1310_DMA_MASTER_JPEG = 1,
-
-	/* Accessible from both 0 & 1 */
-	SPEAR1310_DMA_MASTER_I2S = 0,
-	SPEAR1310_DMA_MASTER_UART1 = 0,
-	SPEAR1310_DMA_MASTER_UART2 = 0,
-	SPEAR1310_DMA_MASTER_UART3 = 0,
-	SPEAR1310_DMA_MASTER_UART4 = 0,
-	SPEAR1310_DMA_MASTER_UART5 = 0,
-	SPEAR1310_DMA_MASTER_I2C1 = 0,
-	SPEAR1310_DMA_MASTER_I2C2 = 0,
-	SPEAR1310_DMA_MASTER_I2C3 = 0,
-	SPEAR1310_DMA_MASTER_I2C4 = 0,
-	SPEAR1310_DMA_MASTER_I2C5 = 0,
-	SPEAR1310_DMA_MASTER_I2C6 = 0,
-	SPEAR1310_DMA_MASTER_I2C7 = 0,
-	SPEAR1310_DMA_MASTER_SSP1 = 0,
-#endif
-
-#ifdef CONFIG_MACH_SPEAR1340
-	/* Accessible from only one master */
-	SPEAR1340_DMA_MASTER_I2S_PLAY = 1,
-	SPEAR1340_DMA_MASTER_I2S_REC = 1,
-	SPEAR1340_DMA_MASTER_I2C1 = 1,
-	SPEAR1340_DMA_MASTER_UART1 = 1,
-
-	/* following are accessible from both master 0 & 1 */
-	SPEAR1340_DMA_MASTER_SPDIF = 0,
-	SPEAR1340_DMA_MASTER_CAM = 1,
-	SPEAR1340_DMA_MASTER_VIDEO_IN = 0,
-	SPEAR1340_DMA_MASTER_MALI = 0,
-#endif
-};
-
-enum request_id {
-	DMA_REQ_ADC = 0,
-	DMA_REQ_SSP0_TX = 4,
-	DMA_REQ_SSP0_RX = 5,
-	DMA_REQ_UART0_TX = 6,
-	DMA_REQ_UART0_RX = 7,
-	DMA_REQ_I2C0_TX = 8,
-	DMA_REQ_I2C0_RX = 9,
-
-#ifdef CONFIG_MACH_SPEAR1310
-	SPEAR1310_DMA_REQ_FROM_JPEG = 2,
-	SPEAR1310_DMA_REQ_TO_JPEG = 3,
-	SPEAR1310_DMA_REQ_I2S_TX = 10,
-	SPEAR1310_DMA_REQ_I2S_RX = 11,
-
-	SPEAR1310_DMA_REQ_I2C1_RX = 0,
-	SPEAR1310_DMA_REQ_I2C1_TX = 1,
-	SPEAR1310_DMA_REQ_I2C2_RX = 2,
-	SPEAR1310_DMA_REQ_I2C2_TX = 3,
-	SPEAR1310_DMA_REQ_I2C3_RX = 4,
-	SPEAR1310_DMA_REQ_I2C3_TX = 5,
-	SPEAR1310_DMA_REQ_I2C4_RX = 6,
-	SPEAR1310_DMA_REQ_I2C4_TX = 7,
-	SPEAR1310_DMA_REQ_I2C5_RX = 8,
-	SPEAR1310_DMA_REQ_I2C5_TX = 9,
-	SPEAR1310_DMA_REQ_I2C6_RX = 10,
-	SPEAR1310_DMA_REQ_I2C6_TX = 11,
-	SPEAR1310_DMA_REQ_UART1_RX = 12,
-	SPEAR1310_DMA_REQ_UART1_TX = 13,
-	SPEAR1310_DMA_REQ_UART2_RX = 14,
-	SPEAR1310_DMA_REQ_UART2_TX = 15,
-	SPEAR1310_DMA_REQ_UART5_RX = 16,
-	SPEAR1310_DMA_REQ_UART5_TX = 17,
-	SPEAR1310_DMA_REQ_SSP1_RX = 18,
-	SPEAR1310_DMA_REQ_SSP1_TX = 19,
-	SPEAR1310_DMA_REQ_I2C7_RX = 20,
-	SPEAR1310_DMA_REQ_I2C7_TX = 21,
-	SPEAR1310_DMA_REQ_UART3_RX = 28,
-	SPEAR1310_DMA_REQ_UART3_TX = 29,
-	SPEAR1310_DMA_REQ_UART4_RX = 30,
-	SPEAR1310_DMA_REQ_UART4_TX = 31,
-#endif
-
-#ifdef CONFIG_MACH_SPEAR1340
-	SPEAR1340_DMA_REQ_SPDIF_TX = 2,
-	SPEAR1340_DMA_REQ_SPDIF_RX = 3,
-	SPEAR1340_DMA_REQ_I2S_TX = 10,
-	SPEAR1340_DMA_REQ_I2S_RX = 11,
-	SPEAR1340_DMA_REQ_UART1_TX = 12,
-	SPEAR1340_DMA_REQ_UART1_RX = 13,
-	SPEAR1340_DMA_REQ_I2C1_TX = 14,
-	SPEAR1340_DMA_REQ_I2C1_RX = 15,
-	SPEAR1340_DMA_REQ_CAM0_EVEN = 0,
-	SPEAR1340_DMA_REQ_CAM0_ODD = 1,
-	SPEAR1340_DMA_REQ_CAM1_EVEN = 2,
-	SPEAR1340_DMA_REQ_CAM1_ODD = 3,
-	SPEAR1340_DMA_REQ_CAM2_EVEN = 4,
-	SPEAR1340_DMA_REQ_CAM2_ODD = 5,
-	SPEAR1340_DMA_REQ_CAM3_EVEN = 6,
-	SPEAR1340_DMA_REQ_CAM3_ODD = 7,
-#endif
-};
-
-#endif /* __MACH_DMA_H */
diff --git a/arch/arm/mach-spear/spear13xx.c b/arch/arm/mach-spear/spear13xx.c
index c1af386..371fc10 100644
--- a/arch/arm/mach-spear/spear13xx.c
+++ b/arch/arm/mach-spear/spear13xx.c
@@ -15,7 +15,6 @@
 
 #include <linux/amba/pl022.h>
 #include <linux/clk.h>
-#include <linux/dw_dmac.h>
 #include <linux/err.h>
 #include <linux/of.h>
 #include <asm/hardware/cache-l2x0.h>
@@ -24,63 +23,6 @@
 #include "generic.h"
 #include <mach/spear.h>
 
-#include "spear13xx-dma.h"
-
-/* common dw_dma filter routine to be used by peripherals */
-bool dw_dma_filter(struct dma_chan *chan, void *slave)
-{
-	struct dw_dma_slave *dws = (struct dw_dma_slave *)slave;
-
-	if (chan->device->dev == dws->dma_dev) {
-		chan->private = slave;
-		return true;
-	} else {
-		return false;
-	}
-}
-
-/* ssp device registration */
-static struct dw_dma_slave ssp_dma_param[] = {
-	{
-		/* Tx */
-		.cfg_hi = DWC_CFGH_DST_PER(DMA_REQ_SSP0_TX),
-		.cfg_lo = 0,
-		.src_master = DMA_MASTER_MEMORY,
-		.dst_master = DMA_MASTER_SSP0,
-	}, {
-		/* Rx */
-		.cfg_hi = DWC_CFGH_SRC_PER(DMA_REQ_SSP0_RX),
-		.cfg_lo = 0,
-		.src_master = DMA_MASTER_SSP0,
-		.dst_master = DMA_MASTER_MEMORY,
-	}
-};
-
-struct pl022_ssp_controller pl022_plat_data = {
-	.enable_dma = 1,
-	.dma_filter = dw_dma_filter,
-	.dma_rx_param = &ssp_dma_param[1],
-	.dma_tx_param = &ssp_dma_param[0],
-};
-
-/* CF device registration */
-struct dw_dma_slave cf_dma_priv = {
-	.cfg_hi = 0,
-	.cfg_lo = 0,
-	.src_master = 0,
-	.dst_master = 0,
-};
-
-/* dmac device registeration */
-struct dw_dma_platform_data dmac_plat_data = {
-	.nr_channels = 8,
-	.chan_allocation_order = CHAN_ALLOCATION_DESCENDING,
-	.chan_priority = CHAN_PRIORITY_DESCENDING,
-	.block_size = 4095U,
-	.nr_masters = 2,
-	.data_width = { 3, 3, 0, 0 },
-};
-
 void __init spear13xx_l2x0_init(void)
 {
 	/*
-- 
1.8.0

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

[Viresh Kumar]

You forgot spear-devel for this series.

On 29 January 2013 03:28, Arnd Bergmann <arnd@arndb.de> wrote:
> 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.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> Cc: Vinod Koul <vinod.koul@linux.intel.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 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
>
> diff --git a/arch/arm/boot/dts/spear1340.dtsi b/arch/arm/boot/dts/spear1340.dtsi
> index 34da11a..e1786a0 100644
> --- a/arch/arm/boot/dts/spear1340.dtsi
> +++ b/arch/arm/boot/dts/spear1340.dtsi
> @@ -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 {
> diff --git a/arch/arm/boot/dts/spear13xx.dtsi b/arch/arm/boot/dts/spear13xx.dtsi
> index b4ca60f..45597fd 100644
> --- a/arch/arm/boot/dts/spear13xx.dtsi
> +++ b/arch/arm/boot/dts/spear13xx.dtsi
> @@ -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>;

4?

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

[Arnd Bergmann]

On Tuesday 29 January 2013, Viresh Kumar wrote:
> You forgot spear-devel for this series.

Ok, thanks for adding it.

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