[PATCH v2] serial: pl011: Add Device Tree support to request DMA channels

[PATCH v2] serial: pl011: Add Device Tree support to request DMA channels

[Lee Jones]

The new DMA API is operational. It is now possible to request DMA
channels from information passed via Device Tree.

Cc: Russell King <linux@arm.linux.org.uk>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.cz>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/tty/serial/amba-pl011.c |   19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 3ea5408..1b39f58 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -249,6 +249,7 @@ static void pl011_dma_probe_initcall(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;
+	struct device_node *np = uap->port.dev->of_node;
 	struct dma_slave_config tx_conf = {
 		.dst_addr = uap->port.mapbase + UART01x_DR,
 		.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE,
@@ -259,9 +260,9 @@ 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");
+	/* We need platform data or DT */
+	if (!(plat && plat->dma_filter) && !np) {
+		dev_info(uap->port.dev, "no DMA platform data or DT\n");
 		return;
 	}
 
@@ -269,7 +270,10 @@ static void pl011_dma_probe_initcall(struct uart_amba_port *uap)
 	dma_cap_zero(mask);
 	dma_cap_set(DMA_SLAVE, mask);
 
-	chan = dma_request_channel(mask, plat->dma_filter, plat->dma_tx_param);
+	chan = dma_request_slave_channel_compat(mask,
+				(plat) ? plat->dma_filter : NULL,
+				(plat) ? plat->dma_tx_param : NULL,
+				uap->port.dev, "tx");
 	if (!chan) {
 		dev_err(uap->port.dev, "no TX DMA channel!\n");
 		return;
@@ -282,7 +286,7 @@ 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) {
+	if ((plat && plat->dma_rx_param) || np) {
 		struct dma_slave_config rx_conf = {
 			.src_addr = uap->port.mapbase + UART01x_DR,
 			.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE,
@@ -291,7 +295,10 @@ 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);
+		chan = dma_request_slave_channel_compat(mask,
+					(plat) ? plat->dma_filter : NULL,
+					(plat) ? plat->dma_rx_param : NULL,
+					uap->port.dev, "rx");
 		if (!chan) {
 			dev_err(uap->port.dev, "no RX DMA channel!\n");
 			return;
-- 
1.7.10.4

Re: [PATCH v2] serial: pl011: Add Device Tree support to request DMA channels

[Russell King - ARM Linux]

On Thu, Apr 18, 2013 at 09:14:16AM +0100, Lee Jones wrote:
> @@ -269,7 +270,10 @@ static void pl011_dma_probe_initcall(struct uart_amba_port *uap)
>  	dma_cap_zero(mask);
>  	dma_cap_set(DMA_SLAVE, mask);
>  
> -	chan = dma_request_channel(mask, plat->dma_filter, plat->dma_tx_param);
> +	chan = dma_request_slave_channel_compat(mask,
> +				(plat) ? plat->dma_filter : NULL,
> +				(plat) ? plat->dma_tx_param : NULL,
> +				uap->port.dev, "tx");
>  	if (!chan) {
>  		dev_err(uap->port.dev, "no TX DMA channel!\n");
>  		return;

This suffers the same problem with your MMCI patch.  If you're using DT and
don't provide the DMA information, you get errors printed.  That's not on
for an optional driver feature, especially when that feature causes
functional difficulties on various platforms and so is _purposely_ omitted.

Re: [PATCH v2] serial: pl011: Add Device Tree support to request DMA channels

[Lee Jones]

On Thu, 18 Apr 2013, Russell King - ARM Linux wrote:

> On Thu, Apr 18, 2013 at 09:14:16AM +0100, Lee Jones wrote:
> > @@ -269,7 +270,10 @@ static void pl011_dma_probe_initcall(struct uart_amba_port *uap)
> >  	dma_cap_zero(mask);
> >  	dma_cap_set(DMA_SLAVE, mask);
> >  
> > -	chan = dma_request_channel(mask, plat->dma_filter, plat->dma_tx_param);
> > +	chan = dma_request_slave_channel_compat(mask,
> > +				(plat) ? plat->dma_filter : NULL,
> > +				(plat) ? plat->dma_tx_param : NULL,
> > +				uap->port.dev, "tx");
> >  	if (!chan) {
> >  		dev_err(uap->port.dev, "no TX DMA channel!\n");
> >  		return;
> 
> This suffers the same problem with your MMCI patch.  If you're using DT and
> don't provide the DMA information, you get errors printed.  That's not on
> for an optional driver feature, especially when that feature causes
> functional difficulties on various platforms and so is _purposely_ omitted.

How does that differ from using pdata and not passing DMA information?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2] serial: pl011: Add Device Tree support to request DMA channels

[Russell King - ARM Linux]

On Thu, Apr 18, 2013 at 09:39:22AM +0100, Lee Jones wrote:
> On Thu, 18 Apr 2013, Russell King - ARM Linux wrote:
> > This suffers the same problem with your MMCI patch.  If you're using DT and
> > don't provide the DMA information, you get errors printed.  That's not on
> > for an optional driver feature, especially when that feature causes
> > functional difficulties on various platforms and so is _purposely_ omitted.
> 
> How does that differ from using pdata and not passing DMA information?

If you have pdata but no DMA information then, this gets triggered:

        if (!plat || !plat->dma_filter) {
                dev_info(uap->port.dev, "no DMA platform data\n");
                return;
        }

However, with your change, this becomes:

	if (!(plat && plat->dma_filter) && !np) {
		dev_info(uap->port.dev, "no DMA platform data or DT\n");
		return;
	}

wihch means it is _unconditionally_ bypassed if there is a DT node.  So,
if the MMC device has been created from DT, we will drop through to the
following code irrespective of whether there is any DMA data passed.
Hence, we get to here unconditionally in the DT case:

	chan = dma_request_slave_channel_compat(mask,
				(plat) ? plat->dma_filter : NULL,
				(plat) ? plat->dma_tx_param : NULL,
				uap->port.dev, "tx");
	if (!chan) {
		dev_err(uap->port.dev, "no TX DMA channel!\n");
		return;

and if the DT transmit DMA information is not provided, chan will be NULL,
which means we produce an error level "no TX DMA channel!" message.  That
message is supposed to indicate only when it has not been possible to
request the channel, not when there has been no DMA information provided.

This is where AMBA drivers differ from the majority of other drivers -
DMA is *strictly* optional for them, and that must remain the case as long
as we have platforms where AMBA devices are not hooked up to DMA engines
or where the DMA engines they are hooked up to are buggy, or the entire
DMA design (PL08x connected to PL18x) is buggy.

Re: [PATCH v2] serial: pl011: Add Device Tree support to request DMA channels

[Arnd Bergmann]

Hi Lee and Russell,

While going through the arm-soc tree yesterday, I noticed two things:

* I had already done a patch for this in January which I meant to queue up
  for 3.10, as it is needed to make spear13xx use the dma-engine binding
* I actually forgot to merge that branch into for-next :(

I've put it into a "late" branch now. Could you have a look please?
If there are any problems with this, I'll probably just drop the branch
and queue it for 3.11 instead with the required fixes.

	Arnd

commit 787b0c1f8e1975157fe73104e67cac18f955281b
Author: Arnd Bergmann <arnd@arndb.de>
Date:   Mon Jan 28 16:24:37 2013 +0000

    serial: pl011: use generic DMA slave configuration if possible
    
    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>
    Acked-by: Grant Likely <grant.likely@secretlab.ca>
    Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    Cc: Russell King <linux@arm.linux.org.uk>
    Cc: Jiri Slaby <jslaby@suse.cz>
    Cc: Viresh Kumar <viresh.kumar@linaro.org>
    Cc: devicetree-discuss@lists.ozlabs.org
    Cc: linux-arm-kernel@lists.infradead.org

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@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 3ea5408..c25b00e 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
 
@@ -991,7 +991,7 @@ static inline bool pl011_dma_rx_running(struct uart_amba_port *uap)
 
 #else
 /* Blank functions if the DMA engine is not available */
-static inline void pl011_dma_probe(struct uart_amba_port *uap)
+static inline void pl011_dma_probe(struct device *dev, struct uart_amba_port *uap)
 {
 }
 @@ -2020,7 +2032,7 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
 	uap->port.ops = &amba_pl011_pops;
 	uap->port.flags = UPF_BOOT_AUTOCONF;
 	uap->port.line = i;
-	pl011_dma_probe(uap);
+	pl011_dma_probe(&dev->dev, uap);
 
 	/* Ensure interrupts from this UART are masked and cleared */
 	writew(0, uap->port.membase + UART011_IMSC);

Re: [PATCH v2] serial: pl011: Add Device Tree support to request DMA channels

[Russell King - ARM Linux]

On Sat, Apr 20, 2013 at 12:33:45PM +0200, Arnd Bergmann wrote:
> @@ -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;

There is really no need for this.  This device is available via uap->port.dev.