[PATCH 0/3] at_hdmac: dt dma bindings update

[PATCH 0/3] at_hdmac: dt dma bindings update

[ludovic.desroches]

From: Ludovic Desroches <ludovic.desroches@atmel.com>

Hi,

This set of patches update the dt dma binding for at_hdmac since we need one
more parameter. In order to keep backward compatibility, an existing cell will
be used to add this parameter. Since the content of that cell will become a
magic value due to the concatenation of the two parameters, it is time to
use macros.

Regards

Ludovic Desroches (3):
  ARM: at91: dt: add header to define at_hdmac configuration
  ARM: at91: dt: switch DMA DT bindings to pre-processor
  at_hdmac: add FIFO configuration parameter to DMA DT binding

 .../devicetree/bindings/dma/atmel-dma.txt          |  7 ++++--
 arch/arm/boot/dts/at91sam9g45.dtsi                 |  5 ++--
 arch/arm/boot/dts/at91sam9n12.dtsi                 | 11 +++++----
 arch/arm/boot/dts/at91sam9x5.dtsi                  | 17 +++++++-------
 arch/arm/boot/dts/sama5d3.dtsi                     | 19 +++++++--------
 drivers/dma/at_hdmac.c                             | 25 ++++++++++++++++----
 include/dt-bindings/dma/at91.h                     | 27 ++++++++++++++++++++++
 7 files changed, 81 insertions(+), 30 deletions(-)
 create mode 100644 include/dt-bindings/dma/at91.h

-- 
1.7.11.3

[PATCH 1/3] ARM: at91: dt: add header to define at_hdmac configuration

[ludovic.desroches]

From: Ludovic Desroches <ludovic.desroches@atmel.com>

DMA-cell content is a concatenation of several values. In order to keep this
stuff human readable, macros are introduced.

The values for the FIFO configuration are not the same as the ones used in the
configuration register in order to keep backward compatibility. Most devices
use the half FIFO configuration but USART ones have to use the ASAP
configuration. This parameter was not initially planed to be into the at91 dma
dt binding. The third cell will be used to store this parameter, it will
become a concatenation of the FIFO configuration and of the peripheral ID. In
order to keep backward compatibility i.e. FIFO configuration is equal to 0, we
have to perform a translation since the value to put in the register to set
half FIFO is 1.

Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---
 include/dt-bindings/dma/at91.h | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 include/dt-bindings/dma/at91.h

diff --git a/include/dt-bindings/dma/at91.h b/include/dt-bindings/dma/at91.h
new file mode 100644
index 0000000..e835037
--- /dev/null
+++ b/include/dt-bindings/dma/at91.h
@@ -0,0 +1,27 @@
+/*
+ * This header provides macros for at91 dma bindings.
+ *
+ * Copyright (C) 2013 Ludovic Desroches <ludovic.desroches@atmel.com>
+ *
+ * GPLv2 only
+ */
+
+#ifndef __DT_BINDINGS_AT91_DMA_H__
+#define __DT_BINDINGS_AT91_DMA_H__
+
+/*
+ * Source and/or destination peripheral ID
+ */
+#define AT91_DMA_CFG_PER_ID_MASK	(0xff)
+#define AT91_DMA_CFG_PER_ID(id)		(id & AT91_DMA_CFG_PER_ID_MASK)
+
+/*
+ * FIFO configuration: it defines when a request is serviced.
+ */
+#define AT91_DMA_CFG_FIFOCFG_OFFSET	(8)
+#define AT91_DMA_CFG_FIFOCFG_MASK	(0xf << AT91_DMA_CFG_FIFOCFG_OFFSET)
+#define AT91_DMA_CFG_FIFOCFG_HALF	(0x0 << AT91_DMA_CFG_FIFOCFG_OFFSET)	/* half FIFO (default behavior) */
+#define AT91_DMA_CFG_FIFOCFG_ALAP	(0x1 << AT91_DMA_CFG_FIFOCFG_OFFSET)	/* largest defined AHB burst */
+#define AT91_DMA_CFG_FIFOCFG_ASAP	(0x2 << AT91_DMA_CFG_FIFOCFG_OFFSET)	/* single AHB access */
+
+#endif /* __DT_BINDINGS_AT91_DMA_H__ */
-- 
1.7.11.3

[PATCH 2/3] ARM: at91: dt: switch DMA DT bindings to pre-processor

[ludovic.desroches]

From: Ludovic Desroches <ludovic.desroches@atmel.com>

Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---
 arch/arm/boot/dts/at91sam9g45.dtsi |  5 +++--
 arch/arm/boot/dts/at91sam9n12.dtsi | 11 ++++++-----
 arch/arm/boot/dts/at91sam9x5.dtsi  | 17 +++++++++--------
 arch/arm/boot/dts/sama5d3.dtsi     | 19 ++++++++++---------
 4 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi
index f0091af..fe84404 100644
--- a/arch/arm/boot/dts/at91sam9g45.dtsi
+++ b/arch/arm/boot/dts/at91sam9g45.dtsi
@@ -10,6 +10,7 @@
  */
 
 #include "skeleton.dtsi"
+#include <dt-bindings/dma/at91.h>
 #include <dt-bindings/pinctrl/at91.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/gpio/gpio.h>
@@ -537,7 +538,7 @@
 				compatible = "atmel,hsmci";
 				reg = <0xfff80000 0x600>;
 				interrupts = <11 IRQ_TYPE_LEVEL_HIGH 0>;
-				dmas = <&dma 1 0>;
+				dmas = <&dma 1 AT91_DMA_CFG_PER_ID(0)>;
 				dma-names = "rxtx";
 				#address-cells = <1>;
 				#size-cells = <0>;
@@ -548,7 +549,7 @@
 				compatible = "atmel,hsmci";
 				reg = <0xfffd0000 0x600>;
 				interrupts = <29 IRQ_TYPE_LEVEL_HIGH 0>;
-				dmas = <&dma 1 13>;
+				dmas = <&dma 1 AT91_DMA_CFG_PER_ID(13)>;
 				dma-names = "rxtx";
 				#address-cells = <1>;
 				#size-cells = <0>;
diff --git a/arch/arm/boot/dts/at91sam9n12.dtsi b/arch/arm/boot/dts/at91sam9n12.dtsi
index 566af32..3076693 100644
--- a/arch/arm/boot/dts/at91sam9n12.dtsi
+++ b/arch/arm/boot/dts/at91sam9n12.dtsi
@@ -8,6 +8,7 @@
  */
 
 #include "skeleton.dtsi"
+#include <dt-bindings/dma/at91.h>
 #include <dt-bindings/pinctrl/at91.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/gpio/gpio.h>
@@ -92,7 +93,7 @@
 				compatible = "atmel,hsmci";
 				reg = <0xf0008000 0x600>;
 				interrupts = <12 IRQ_TYPE_LEVEL_HIGH 0>;
-				dmas = <&dma 1 0>;
+				dmas = <&dma 1 AT91_DMA_CFG_PER_ID(0)>;
 				dma-names = "rxtx";
 				#address-cells = <1>;
 				#size-cells = <0>;
@@ -384,8 +385,8 @@
 				compatible = "atmel,at91sam9x5-i2c";
 				reg = <0xf8010000 0x100>;
 				interrupts = <9 IRQ_TYPE_LEVEL_HIGH 6>;
-				dmas = <&dma 1 13>,
-				       <&dma 1 14>;
+				dmas = <&dma 1 AT91_DMA_CFG_PER_ID(13)>,
+				       <&dma 1 AT91_DMA_CFG_PER_ID(14)>;
 				dma-names = "tx", "rx";
 				#address-cells = <1>;
 				#size-cells = <0>;
@@ -396,8 +397,8 @@
 				compatible = "atmel,at91sam9x5-i2c";
 				reg = <0xf8014000 0x100>;
 				interrupts = <10 IRQ_TYPE_LEVEL_HIGH 6>;
-				dmas = <&dma 1 15>,
-				       <&dma 1 16>;
+				dmas = <&dma 1 AT91_DMA_CFG_PER_ID(15)>,
+				       <&dma 1 AT91_DMA_CFG_PER_ID(16)>;
 				dma-names = "tx", "rx";
 				#address-cells = <1>;
 				#size-cells = <0>;
diff --git a/arch/arm/boot/dts/at91sam9x5.dtsi b/arch/arm/boot/dts/at91sam9x5.dtsi
index af91599..a08bf77 100644
--- a/arch/arm/boot/dts/at91sam9x5.dtsi
+++ b/arch/arm/boot/dts/at91sam9x5.dtsi
@@ -10,6 +10,7 @@
  */
 
 #include "skeleton.dtsi"
+#include <dt-bindings/dma/at91.h>
 #include <dt-bindings/pinctrl/at91.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/gpio/gpio.h>
@@ -470,7 +471,7 @@
 				compatible = "atmel,hsmci";
 				reg = <0xf0008000 0x600>;
 				interrupts = <12 IRQ_TYPE_LEVEL_HIGH 0>;
-				dmas = <&dma0 1 0>;
+				dmas = <&dma0 1 AT91_DMA_CFG_PER_ID(0)>;
 				dma-names = "rxtx";
 				#address-cells = <1>;
 				#size-cells = <0>;
@@ -481,7 +482,7 @@
 				compatible = "atmel,hsmci";
 				reg = <0xf000c000 0x600>;
 				interrupts = <26 IRQ_TYPE_LEVEL_HIGH 0>;
-				dmas = <&dma1 1 0>;
+				dmas = <&dma1 1 AT91_DMA_CFG_PER_ID(0)>;
 				dma-names = "rxtx";
 				#address-cells = <1>;
 				#size-cells = <0>;
@@ -544,8 +545,8 @@
 				compatible = "atmel,at91sam9x5-i2c";
 				reg = <0xf8010000 0x100>;
 				interrupts = <9 IRQ_TYPE_LEVEL_HIGH 6>;
-				dmas = <&dma0 1 7>,
-				       <&dma0 1 8>;
+				dmas = <&dma0 1 AT91_DMA_CFG_PER_ID(7)>,
+				       <&dma0 1 AT91_DMA_CFG_PER_ID(8)>;
 				dma-names = "tx", "rx";
 				#address-cells = <1>;
 				#size-cells = <0>;
@@ -558,8 +559,8 @@
 				compatible = "atmel,at91sam9x5-i2c";
 				reg = <0xf8014000 0x100>;
 				interrupts = <10 IRQ_TYPE_LEVEL_HIGH 6>;
-				dmas = <&dma1 1 5>,
-				       <&dma1 1 6>;
+				dmas = <&dma1 1 AT91_DMA_CFG_PER_ID(5)>,
+				       <&dma1 1 AT91_DMA_CFG_PER_ID(6)>;
 				dma-names = "tx", "rx";
 				#address-cells = <1>;
 				#size-cells = <0>;
@@ -572,8 +573,8 @@
 				compatible = "atmel,at91sam9x5-i2c";
 				reg = <0xf8018000 0x100>;
 				interrupts = <11 IRQ_TYPE_LEVEL_HIGH 6>;
-				dmas = <&dma0 1 9>,
-				       <&dma0 1 10>;
+				dmas = <&dma0 1 AT91_DMA_CFG_PER_ID(9)>,
+				       <&dma0 1 AT91_DMA_CFG_PER_ID(10)>;
 				dma-names = "tx", "rx";
 				#address-cells = <1>;
 				#size-cells = <0>;
diff --git a/arch/arm/boot/dts/sama5d3.dtsi b/arch/arm/boot/dts/sama5d3.dtsi
index c00e158..03b8c02 100644
--- a/arch/arm/boot/dts/sama5d3.dtsi
+++ b/arch/arm/boot/dts/sama5d3.dtsi
@@ -9,6 +9,7 @@
  */
 
 #include "skeleton.dtsi"
+#include <dt-bindings/dma/at91.h>
 #include <dt-bindings/pinctrl/at91.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/gpio/gpio.h>
@@ -63,7 +64,7 @@
 				compatible = "atmel,hsmci";
 				reg = <0xf0000000 0x600>;
 				interrupts = <21 IRQ_TYPE_LEVEL_HIGH 0>;
-				dmas = <&dma0 2 0>;
+				dmas = <&dma0 2 AT91_DMA_CFG_PER_ID(0)>;
 				dma-names = "rxtx";
 				pinctrl-names = "default";
 				pinctrl-0 = <&pinctrl_mmc0_clk_cmd_dat0 &pinctrl_mmc0_dat1_3 &pinctrl_mmc0_dat4_7>;
@@ -116,8 +117,8 @@
 				compatible = "atmel,at91sam9x5-i2c";
 				reg = <0xf0014000 0x4000>;
 				interrupts = <18 IRQ_TYPE_LEVEL_HIGH 6>;
-				dmas = <&dma0 2 7>,
-				       <&dma0 2 8>;
+				dmas = <&dma0 2 AT91_DMA_CFG_PER_ID(7)>,
+				       <&dma0 2 AT91_DMA_CFG_PER_ID(8)>;
 				dma-names = "tx", "rx";
 				pinctrl-names = "default";
 				pinctrl-0 = <&pinctrl_i2c0>;
@@ -130,8 +131,8 @@
 				compatible = "atmel,at91sam9x5-i2c";
 				reg = <0xf0018000 0x4000>;
 				interrupts = <19 IRQ_TYPE_LEVEL_HIGH 6>;
-				dmas = <&dma0 2 9>,
-				       <&dma0 2 10>;
+				dmas = <&dma0 2 AT91_DMA_CFG_PER_ID(9)>,
+				       <&dma0 2 AT91_DMA_CFG_PER_ID(10)>;
 				dma-names = "tx", "rx";
 				pinctrl-names = "default";
 				pinctrl-0 = <&pinctrl_i2c1>;
@@ -178,7 +179,7 @@
 				compatible = "atmel,hsmci";
 				reg = <0xf8000000 0x600>;
 				interrupts = <22 IRQ_TYPE_LEVEL_HIGH 0>;
-				dmas = <&dma1 2 0>;
+				dmas = <&dma1 2 AT91_DMA_CFG_PER_ID(0)>;
 				dma-names = "rxtx";
 				pinctrl-names = "default";
 				pinctrl-0 = <&pinctrl_mmc1_clk_cmd_dat0 &pinctrl_mmc1_dat1_3>;
@@ -191,7 +192,7 @@
 				compatible = "atmel,hsmci";
 				reg = <0xf8004000 0x600>;
 				interrupts = <23 IRQ_TYPE_LEVEL_HIGH 0>;
-				dmas = <&dma1 2 1>;
+				dmas = <&dma1 2 AT91_DMA_CFG_PER_ID(1)>;
 				dma-names = "rxtx";
 				pinctrl-names = "default";
 				pinctrl-0 = <&pinctrl_mmc2_clk_cmd_dat0 &pinctrl_mmc2_dat1_3>;
@@ -309,8 +310,8 @@
 				compatible = "atmel,at91sam9x5-i2c";
 				reg = <0xf801c000 0x4000>;
 				interrupts = <20 IRQ_TYPE_LEVEL_HIGH 6>;
-				dmas = <&dma1 2 11>,
-				       <&dma1 2 12>;
+				dmas = <&dma1 2 AT91_DMA_CFG_PER_ID(11)>,
+				       <&dma1 2 AT91_DMA_CFG_PER_ID(12)>;
 				dma-names = "tx", "rx";
 				#address-cells = <1>;
 				#size-cells = <0>;
-- 
1.7.11.3

[PATCH 3/3] at_hdmac: add FIFO configuration parameter to DMA DT binding

[ludovic.desroches]

From: Ludovic Desroches <ludovic.desroches@atmel.com>

For most devices the FIFO configuration is the same i.e. when half FIFO size is
available/filled, a source/destination request is serviced. But USART devices
have to do it when there is enough space/data available to perform a single
AHB access so the ASAP configuration.

Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---
 .../devicetree/bindings/dma/atmel-dma.txt          |  7 ++++--
 drivers/dma/at_hdmac.c                             | 25 ++++++++++++++++++----
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/atmel-dma.txt b/Documentation/devicetree/bindings/dma/atmel-dma.txt
index c80e8a3..c280a0e 100644
--- a/Documentation/devicetree/bindings/dma/atmel-dma.txt
+++ b/Documentation/devicetree/bindings/dma/atmel-dma.txt
@@ -24,8 +24,11 @@ The three cells in order are:
 1. A phandle pointing to the DMA controller.
 2. The memory interface (16 most significant bits), the peripheral interface
 (16 less significant bits).
-3. The peripheral identifier for the hardware handshaking interface. The
-identifier can be different for tx and rx.
+3. Parameters for the at91 DMA configuration register which are device
+dependant:
+  - bit 7-0: peripheral identifier for the hardware handshaking interface. The
+  identifier can be different for tx and rx.
+  - bit 11-8: FIFO configuration. 0 for half FIFO, 1 for ALAP, 1 for ASAP.
 
 Example:
 
diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index e923cda..bfd73e1 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -14,6 +14,7 @@
  * found on AT91SAM9263.
  */
 
+#include <dt-bindings/dma/at91.h>
 #include <linux/clk.h>
 #include <linux/dmaengine.h>
 #include <linux/dma-mapping.h>
@@ -1223,14 +1224,30 @@ static struct dma_chan *at_dma_xlate(struct of_phandle_args *dma_spec,
 	atslave = devm_kzalloc(&dmac_pdev->dev, sizeof(*atslave), GFP_KERNEL);
 	if (!atslave)
 		return NULL;
+
+	atslave->cfg = ATC_DST_H2SEL_HW | ATC_SRC_H2SEL_HW;
 	/*
 	 * We can fill both SRC_PER and DST_PER, one of these fields will be
 	 * ignored depending on DMA transfer direction.
 	 */
-	per_id = dma_spec->args[1];
-	atslave->cfg = ATC_FIFOCFG_HALFFIFO | ATC_DST_H2SEL_HW
-		      | ATC_SRC_H2SEL_HW | ATC_DST_PER(per_id)
-		      | ATC_SRC_PER(per_id);
+	per_id = dma_spec->args[1] & AT91_DMA_CFG_PER_ID_MASK;
+	atslave->cfg |= ATC_DST_PER(per_id) | ATC_SRC_PER(per_id);
+	/*
+	 * We have to translate the value we get from the device tree since
+	 * the half FIFO configuration value had to be 0 to keep backward
+	 * compatibility.
+	 */
+	switch(dma_spec->args[1] & AT91_DMA_CFG_FIFOCFG_MASK) {
+		case AT91_DMA_CFG_FIFOCFG_ALAP:
+			atslave->cfg |= ATC_FIFOCFG_LARGESTBURST;
+			break;
+		case AT91_DMA_CFG_FIFOCFG_ASAP:
+			atslave->cfg |= ATC_FIFOCFG_ENOUGHSPACE;
+			break;
+		case AT91_DMA_CFG_FIFOCFG_HALF:
+		default:
+			atslave->cfg |= ATC_FIFOCFG_HALFFIFO;
+	}
 	atslave->dma_dev = &dmac_pdev->dev;
 
 	chan = dma_request_channel(mask, at_dma_filter, atslave);
-- 
1.7.11.3

Re: [PATCH 3/3] at_hdmac: add FIFO configuration parameter to DMA DT binding

[Nicolas Ferre]

On 30/05/2013 18:08, ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org :
> From: Ludovic Desroches <ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
>
> For most devices the FIFO configuration is the same i.e. when half FIFO size is
> available/filled, a source/destination request is serviced. But USART devices
> have to do it when there is enough space/data available to perform a single
> AHB access so the ASAP configuration.
>
> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Ludovic Desroches <ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>

Clear and neat: thanks Ludo.

Acked-by: Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>

> ---
>   .../devicetree/bindings/dma/atmel-dma.txt          |  7 ++++--
>   drivers/dma/at_hdmac.c                             | 25 ++++++++++++++++++----
>   2 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/dma/atmel-dma.txt b/Documentation/devicetree/bindings/dma/atmel-dma.txt
> index c80e8a3..c280a0e 100644
> --- a/Documentation/devicetree/bindings/dma/atmel-dma.txt
> +++ b/Documentation/devicetree/bindings/dma/atmel-dma.txt
> @@ -24,8 +24,11 @@ The three cells in order are:
>   1. A phandle pointing to the DMA controller.
>   2. The memory interface (16 most significant bits), the peripheral interface
>   (16 less significant bits).
> -3. The peripheral identifier for the hardware handshaking interface. The
> -identifier can be different for tx and rx.
> +3. Parameters for the at91 DMA configuration register which are device
> +dependant:
> +  - bit 7-0: peripheral identifier for the hardware handshaking interface. The
> +  identifier can be different for tx and rx.
> +  - bit 11-8: FIFO configuration. 0 for half FIFO, 1 for ALAP, 1 for ASAP.
>
>   Example:
>
> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
> index e923cda..bfd73e1 100644
> --- a/drivers/dma/at_hdmac.c
> +++ b/drivers/dma/at_hdmac.c
> @@ -14,6 +14,7 @@
>    * found on AT91SAM9263.
>    */
>
> +#include <dt-bindings/dma/at91.h>
>   #include <linux/clk.h>
>   #include <linux/dmaengine.h>
>   #include <linux/dma-mapping.h>
> @@ -1223,14 +1224,30 @@ static struct dma_chan *at_dma_xlate(struct of_phandle_args *dma_spec,
>   	atslave = devm_kzalloc(&dmac_pdev->dev, sizeof(*atslave), GFP_KERNEL);
>   	if (!atslave)
>   		return NULL;
> +
> +	atslave->cfg = ATC_DST_H2SEL_HW | ATC_SRC_H2SEL_HW;
>   	/*
>   	 * We can fill both SRC_PER and DST_PER, one of these fields will be
>   	 * ignored depending on DMA transfer direction.
>   	 */
> -	per_id = dma_spec->args[1];
> -	atslave->cfg = ATC_FIFOCFG_HALFFIFO | ATC_DST_H2SEL_HW
> -		      | ATC_SRC_H2SEL_HW | ATC_DST_PER(per_id)
> -		      | ATC_SRC_PER(per_id);
> +	per_id = dma_spec->args[1] & AT91_DMA_CFG_PER_ID_MASK;
> +	atslave->cfg |= ATC_DST_PER(per_id) | ATC_SRC_PER(per_id);
> +	/*
> +	 * We have to translate the value we get from the device tree since
> +	 * the half FIFO configuration value had to be 0 to keep backward
> +	 * compatibility.
> +	 */
> +	switch(dma_spec->args[1] & AT91_DMA_CFG_FIFOCFG_MASK) {
> +		case AT91_DMA_CFG_FIFOCFG_ALAP:
> +			atslave->cfg |= ATC_FIFOCFG_LARGESTBURST;
> +			break;
> +		case AT91_DMA_CFG_FIFOCFG_ASAP:
> +			atslave->cfg |= ATC_FIFOCFG_ENOUGHSPACE;
> +			break;
> +		case AT91_DMA_CFG_FIFOCFG_HALF:
> +		default:
> +			atslave->cfg |= ATC_FIFOCFG_HALFFIFO;
> +	}
>   	atslave->dma_dev = &dmac_pdev->dev;
>
>   	chan = dma_request_channel(mask, at_dma_filter, atslave);
>


-- 
Nicolas Ferre

Re: [PATCH 0/3] at_hdmac: dt dma bindings update

[Jean-Christophe PLAGNIOL-VILLARD]

On 18:08 Thu 30 May     , ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org wrote:
> From: Ludovic Desroches <ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> 
> Hi,
> 
> This set of patches update the dt dma binding for at_hdmac since we need one
> more parameter. In order to keep backward compatibility, an existing cell will
> be used to add this parameter. Since the content of that cell will become a
> magic value due to the concatenation of the two parameters, it is time to
> use macros.

For the record this is based on the cleanup branch for 3.11

Best Regards,
J.
> 
> Regards
> 
> Ludovic Desroches (3):
>   ARM: at91: dt: add header to define at_hdmac configuration
>   ARM: at91: dt: switch DMA DT bindings to pre-processor
>   at_hdmac: add FIFO configuration parameter to DMA DT binding
> 
>  .../devicetree/bindings/dma/atmel-dma.txt          |  7 ++++--
>  arch/arm/boot/dts/at91sam9g45.dtsi                 |  5 ++--
>  arch/arm/boot/dts/at91sam9n12.dtsi                 | 11 +++++----
>  arch/arm/boot/dts/at91sam9x5.dtsi                  | 17 +++++++-------
>  arch/arm/boot/dts/sama5d3.dtsi                     | 19 +++++++--------
>  drivers/dma/at_hdmac.c                             | 25 ++++++++++++++++----
>  include/dt-bindings/dma/at91.h                     | 27 ++++++++++++++++++++++
>  7 files changed, 81 insertions(+), 30 deletions(-)
>  create mode 100644 include/dt-bindings/dma/at91.h
> 
> -- 
> 1.7.11.3
> 

Re: [PATCH 3/3] at_hdmac: add FIFO configuration parameter to DMA DT binding

[Jean-Christophe PLAGNIOL-VILLARD]

On 18:32 Thu 30 May     , Nicolas Ferre wrote:
> On 30/05/2013 18:08, ludovic.desroches@atmel.com :
> >From: Ludovic Desroches <ludovic.desroches@atmel.com>
> >
> >For most devices the FIFO configuration is the same i.e. when half FIFO size is
> >available/filled, a source/destination request is serviced. But USART devices
> >have to do it when there is enough space/data available to perform a single
> >AHB access so the ASAP configuration.
> >
> >Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> >Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> 
> Clear and neat: thanks Ludo.

agreed

can we apply this via AT91 as this depends on some cleanup I did on DT and
could result on some nigthmware conflict

Best Regards,
J.

Re: [PATCH 0/3] at_hdmac: dt dma bindings update

[Vinod Koul]

On Thu, May 30, 2013 at 06:08:20PM +0200, ludovic.desroches@atmel.com wrote:
> From: Ludovic Desroches <ludovic.desroches@atmel.com>
> 
> Hi,
> 
> This set of patches update the dt dma binding for at_hdmac since we need one
> more parameter. In order to keep backward compatibility, an existing cell will
> be used to add this parameter. Since the content of that cell will become a
> magic value due to the concatenation of the two parameters, it is time to
> use macros.
which tree is this targetted to be applied? 

Arnd, can you review this series and ack if this needs to be applied thru
dmaengine?

--
~Vinod
> 
> Regards
> 
> Ludovic Desroches (3):
>   ARM: at91: dt: add header to define at_hdmac configuration
>   ARM: at91: dt: switch DMA DT bindings to pre-processor
>   at_hdmac: add FIFO configuration parameter to DMA DT binding
> 
>  .../devicetree/bindings/dma/atmel-dma.txt          |  7 ++++--
>  arch/arm/boot/dts/at91sam9g45.dtsi                 |  5 ++--
>  arch/arm/boot/dts/at91sam9n12.dtsi                 | 11 +++++----
>  arch/arm/boot/dts/at91sam9x5.dtsi                  | 17 +++++++-------
>  arch/arm/boot/dts/sama5d3.dtsi                     | 19 +++++++--------
>  drivers/dma/at_hdmac.c                             | 25 ++++++++++++++++----
>  include/dt-bindings/dma/at91.h                     | 27 ++++++++++++++++++++++
>  7 files changed, 81 insertions(+), 30 deletions(-)
>  create mode 100644 include/dt-bindings/dma/at91.h
> 
> -- 
> 1.7.11.3
> 

-- 

Re: [PATCH 0/3] at_hdmac: dt dma bindings update

[Jean-Christophe PLAGNIOL-VILLARD]

On 22:56 Thu 30 May     , Vinod Koul wrote:
> On Thu, May 30, 2013 at 06:08:20PM +0200, ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org wrote:
> > From: Ludovic Desroches <ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> > 
> > Hi,
> > 
> > This set of patches update the dt dma binding for at_hdmac since we need one
> > more parameter. In order to keep backward compatibility, an existing cell will
> > be used to add this parameter. Since the content of that cell will become a
> > magic value due to the concatenation of the two parameters, it is time to
> > use macros.
> which tree is this targetted to be applied? 

arm-soc as this depends on alreadied applied cleanup I've 
> 
> Arnd, can you review this series and ack if this needs to be applied thru
> dmaengine?

I prefer the invert

Best Regards,
J.
> 
> --
> ~Vinod
> > 
> > Regards
> > 
> > Ludovic Desroches (3):
> >   ARM: at91: dt: add header to define at_hdmac configuration
> >   ARM: at91: dt: switch DMA DT bindings to pre-processor
> >   at_hdmac: add FIFO configuration parameter to DMA DT binding
> > 
> >  .../devicetree/bindings/dma/atmel-dma.txt          |  7 ++++--
> >  arch/arm/boot/dts/at91sam9g45.dtsi                 |  5 ++--
> >  arch/arm/boot/dts/at91sam9n12.dtsi                 | 11 +++++----
> >  arch/arm/boot/dts/at91sam9x5.dtsi                  | 17 +++++++-------
> >  arch/arm/boot/dts/sama5d3.dtsi                     | 19 +++++++--------
> >  drivers/dma/at_hdmac.c                             | 25 ++++++++++++++++----
> >  include/dt-bindings/dma/at91.h                     | 27 ++++++++++++++++++++++
> >  7 files changed, 81 insertions(+), 30 deletions(-)
> >  create mode 100644 include/dt-bindings/dma/at91.h
> > 
> > -- 
> > 1.7.11.3
> > 
> 
> -- 

Re: [PATCH 0/3] at_hdmac: dt dma bindings update

[Nicolas Ferre]

On 30/05/2013 22:13, Jean-Christophe PLAGNIOL-VILLARD :
> On 22:56 Thu 30 May     , Vinod Koul wrote:
>> On Thu, May 30, 2013 at 06:08:20PM +0200, ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org wrote:
>>> From: Ludovic Desroches <ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
>>>
>>> Hi,
>>>
>>> This set of patches update the dt dma binding for at_hdmac since we need one
>>> more parameter. In order to keep backward compatibility, an existing cell will
>>> be used to add this parameter. Since the content of that cell will become a
>>> magic value due to the concatenation of the two parameters, it is time to
>>> use macros.
>> which tree is this targetted to be applied?
>
> arm-soc as this depends on alreadied applied cleanup I've

... already pushed (it must be the end of your sentence, Jean-Christophe...)

Vinod, Arnd,

- patch 1 shall be applied before any of the two others of the series
- patch 2 is pure DT modification, but relies on Jean-Christophe's
   already pushed cleanup branch in arm-soc
- patch 3 is the pure dmaengine one, but relies on the 1st patch

So, maybe the preferred way to include this material upstream is to go 
through arm-soc as suggested by Jean-Christophe.

Bye,

>> Arnd, can you review this series and ack if this needs to be applied thru
>> dmaengine?
>
> I prefer the invert
>
> Best Regards,
> J.
>>
>> --
>> ~Vinod
>>>
>>> Regards
>>>
>>> Ludovic Desroches (3):
>>>    ARM: at91: dt: add header to define at_hdmac configuration
>>>    ARM: at91: dt: switch DMA DT bindings to pre-processor
>>>    at_hdmac: add FIFO configuration parameter to DMA DT binding
>>>
>>>   .../devicetree/bindings/dma/atmel-dma.txt          |  7 ++++--
>>>   arch/arm/boot/dts/at91sam9g45.dtsi                 |  5 ++--
>>>   arch/arm/boot/dts/at91sam9n12.dtsi                 | 11 +++++----
>>>   arch/arm/boot/dts/at91sam9x5.dtsi                  | 17 +++++++-------
>>>   arch/arm/boot/dts/sama5d3.dtsi                     | 19 +++++++--------
>>>   drivers/dma/at_hdmac.c                             | 25 ++++++++++++++++----
>>>   include/dt-bindings/dma/at91.h                     | 27 ++++++++++++++++++++++
>>>   7 files changed, 81 insertions(+), 30 deletions(-)
>>>   create mode 100644 include/dt-bindings/dma/at91.h
>>>
>>> --
>>> 1.7.11.3
>>>
>>
>> --
>


-- 
Nicolas Ferre

Re: [PATCH 3/3] at_hdmac: add FIFO configuration parameter to DMA DT binding

[Ludovic Desroches]

On Thu, May 30, 2013 at 06:39:36PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 18:32 Thu 30 May     , Nicolas Ferre wrote:
> > On 30/05/2013 18:08, ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org :
> > >From: Ludovic Desroches <ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> > >
> > >For most devices the FIFO configuration is the same i.e. when half FIFO size is
> > >available/filled, a source/destination request is serviced. But USART devices
> > >have to do it when there is enough space/data available to perform a single
> > >AHB access so the ASAP configuration.
> > >
> > >Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
> > >Signed-off-by: Ludovic Desroches <ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> > 
> > Clear and neat: thanks Ludo.
> 
> agreed
> 
> can we apply this via AT91 as this depends on some cleanup I did on DT and
> could result on some nigthmware conflict
> 

In fact, I am not sure it's the best solution. I have noticed that there is a
conflict with a patch sent by Nicolas (dmaengine: at_hdmac: extend hardware
handshaking interface identification) which is already in Vinod's tree but
which is not present on Jean-Christophe's cleanup tree on which I based these
patches.

Moreover this patch doesn't use macros introduced by Nicolas' patch. I may
update it and it should go through Vinod's tree with a dependence on patch 1/3.

What's your point of view?

Re: [PATCH 3/3] at_hdmac: add FIFO configuration parameter to DMA DT binding

[Ludovic Desroches]

On Thu, May 30, 2013 at 06:39:36PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 18:32 Thu 30 May     , Nicolas Ferre wrote:
> > On 30/05/2013 18:08, ludovic.desroches@atmel.com :
> > >From: Ludovic Desroches <ludovic.desroches@atmel.com>
> > >
> > >For most devices the FIFO configuration is the same i.e. when half FIFO size is
> > >available/filled, a source/destination request is serviced. But USART devices
> > >have to do it when there is enough space/data available to perform a single
> > >AHB access so the ASAP configuration.
> > >
> > >Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > >Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> > 
> > Clear and neat: thanks Ludo.
> 
> agreed
> 
> can we apply this via AT91 as this depends on some cleanup I did on DT and
> could result on some nigthmware conflict
> 

In fact, I am not sure it's the best solution. I have noticed that there is a
conflict with a patch sent by Nicolas (dmaengine: at_hdmac: extend hardware
handshaking interface identification) which is already in Vinod's tree but
which is not present on Jean-Christophe's cleanup tree on which I based these
patches.

Moreover this patch doesn't use macros introduced by Nicolas' patch. I may
update it and it should go through Vinod's tree with a dependence on patch 1/3.

What's your point of view?

Re: [PATCH 3/3] at_hdmac: add FIFO configuration parameter to DMA DT binding

[Nicolas Ferre]

On 31/05/2013 11:16, Ludovic Desroches :
> On Thu, May 30, 2013 at 06:39:36PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
>> On 18:32 Thu 30 May     , Nicolas Ferre wrote:
>>> On 30/05/2013 18:08, ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org :
>>>> From: Ludovic Desroches <ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
>>>>
>>>> For most devices the FIFO configuration is the same i.e. when half FIFO size is
>>>> available/filled, a source/destination request is serviced. But USART devices
>>>> have to do it when there is enough space/data available to perform a single
>>>> AHB access so the ASAP configuration.
>>>>
>>>> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
>>>> Signed-off-by: Ludovic Desroches <ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
>>>
>>> Clear and neat: thanks Ludo.
>>
>> agreed
>>
>> can we apply this via AT91 as this depends on some cleanup I did on DT and
>> could result on some nigthmware conflict
>>
>
> In fact, I am not sure it's the best solution. I have noticed that there is a
> conflict with a patch sent by Nicolas (dmaengine: at_hdmac: extend hardware
> handshaking interface identification) which is already in Vinod's tree but
> which is not present on Jean-Christophe's cleanup tree on which I based these
> patches.
>
> Moreover this patch doesn't use macros introduced by Nicolas' patch. I may
> update it and it should go through Vinod's tree with a dependence on patch 1/3.
>
> What's your point of view?

Indeed. Another option could be to push patches 1 and 3 in dmaengine's 
tree and make the 2nd patch go through arm-soc with a dependency on 
slave-dma GIT tree (it seems it is an usual patern).

Arnd, Jean-Christophe, what do you think?

Bye,
-- 
Nicolas Ferre

Re: [PATCH 3/3] at_hdmac: add FIFO configuration parameter to DMA DT binding

[Jean-Christophe PLAGNIOL-VILLARD]

On 11:31 Fri 31 May     , Nicolas Ferre wrote:
> On 31/05/2013 11:16, Ludovic Desroches :
> >On Thu, May 30, 2013 at 06:39:36PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >>On 18:32 Thu 30 May     , Nicolas Ferre wrote:
> >>>On 30/05/2013 18:08, ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org :
> >>>>From: Ludovic Desroches <ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> >>>>
> >>>>For most devices the FIFO configuration is the same i.e. when half FIFO size is
> >>>>available/filled, a source/destination request is serviced. But USART devices
> >>>>have to do it when there is enough space/data available to perform a single
> >>>>AHB access so the ASAP configuration.
> >>>>
> >>>>Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
> >>>>Signed-off-by: Ludovic Desroches <ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> >>>
> >>>Clear and neat: thanks Ludo.
> >>
> >>agreed
> >>
> >>can we apply this via AT91 as this depends on some cleanup I did on DT and
> >>could result on some nigthmware conflict
> >>
> >
> >In fact, I am not sure it's the best solution. I have noticed that there is a
> >conflict with a patch sent by Nicolas (dmaengine: at_hdmac: extend hardware
> >handshaking interface identification) which is already in Vinod's tree but
> >which is not present on Jean-Christophe's cleanup tree on which I based these
> >patches.
> >
> >Moreover this patch doesn't use macros introduced by Nicolas' patch. I may
> >update it and it should go through Vinod's tree with a dependence on patch 1/3.
> >
> >What's your point of view?
> 
> Indeed. Another option could be to push patches 1 and 3 in
> dmaengine's tree and make the 2nd patch go through arm-soc with a
> dependency on slave-dma GIT tree (it seems it is an usual patern).
> 
> Arnd, Jean-Christophe, what do you think?

Nico I let you handle this one my only concern is that the cleanup work can
cause nightware conflict

Best Regards,
J.
> 
> Bye,
> -- 
> Nicolas Ferre

Re: [PATCH 1/3] ARM: at91: dt: add header to define at_hdmac configuration

[Arnd Bergmann]

On Thursday 30 May 2013, ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org wrote:
> +/*
> + * Source and/or destination peripheral ID
> + */
> +#define AT91_DMA_CFG_PER_ID_MASK       (0xff)
> +#define AT91_DMA_CFG_PER_ID(id)                (id & AT91_DMA_CFG_PER_ID_MASK)
> +

I'm a little worried about macros like this one spreading in the name
of readability, and I don't see this as actual value-add here:

The output of the macro is always the same as the input, and passing
an invalid number still results in an invalid output and no error message,
it actually prevents us from warning about the mistake at run-time.

The other macros are fine.

	Arnd

Re: [PATCH 3/3] at_hdmac: add FIFO configuration parameter to DMA DT binding

[Olof Johansson]

On Fri, May 31, 2013 at 2:31 AM, Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org> wrote:
> On 31/05/2013 11:16, Ludovic Desroches :
>>
>> On Thu, May 30, 2013 at 06:39:36PM +0200, Jean-Christophe PLAGNIOL-VILLARD
>> wrote:
>>>
>>> On 18:32 Thu 30 May     , Nicolas Ferre wrote:
>>>>
>>>> On 30/05/2013 18:08, ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org :
>>>>>
>>>>> From: Ludovic Desroches <ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
>>>>>
>>>>> For most devices the FIFO configuration is the same i.e. when half FIFO
>>>>> size is
>>>>> available/filled, a source/destination request is serviced. But USART
>>>>> devices
>>>>> have to do it when there is enough space/data available to perform a
>>>>> single
>>>>> AHB access so the ASAP configuration.
>>>>>
>>>>> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
>>>>> Signed-off-by: Ludovic Desroches <ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
>>>>
>>>>
>>>> Clear and neat: thanks Ludo.
>>>
>>>
>>> agreed
>>>
>>> can we apply this via AT91 as this depends on some cleanup I did on DT
>>> and
>>> could result on some nigthmware conflict
>>>
>>
>> In fact, I am not sure it's the best solution. I have noticed that there
>> is a
>> conflict with a patch sent by Nicolas (dmaengine: at_hdmac: extend
>> hardware
>> handshaking interface identification) which is already in Vinod's tree but
>> which is not present on Jean-Christophe's cleanup tree on which I based
>> these
>> patches.
>>
>> Moreover this patch doesn't use macros introduced by Nicolas' patch. I may
>> update it and it should go through Vinod's tree with a dependence on patch
>> 1/3.
>>
>> What's your point of view?
>
>
> Indeed. Another option could be to push patches 1 and 3 in dmaengine's tree
> and make the 2nd patch go through arm-soc with a dependency on slave-dma GIT
> tree (it seems it is an usual patern).
>
> Arnd, Jean-Christophe, what do you think?

I'm not sure patch 1 needs to go through the dma-engine tree? Sure,
it's a dma-related header file but it's only used to craft the device
tree in patch 2, it doesn't affect the driver.

(Arnd has comments on the patch that should be resolved though).


-Olof

Re: [PATCH 1/3] ARM: at91: dt: add header to define at_hdmac configuration

[Ludovic Desroches]

On Fri, May 31, 2013 at 11:30:29PM +0200, Arnd Bergmann wrote:
> On Thursday 30 May 2013, ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org wrote:
> > +/*
> > + * Source and/or destination peripheral ID
> > + */
> > +#define AT91_DMA_CFG_PER_ID_MASK       (0xff)
> > +#define AT91_DMA_CFG_PER_ID(id)                (id & AT91_DMA_CFG_PER_ID_MASK)
> > +
> 
> I'm a little worried about macros like this one spreading in the name
> of readability, and I don't see this as actual value-add here:
> 
> The output of the macro is always the same as the input, and passing
> an invalid number still results in an invalid output and no error message,
> it actually prevents us from warning about the mistake at run-time.
> 

You are right, this macro is here only for readability in order to not have
something like this:

dmas = <&dma0 AT91_DMA_CFG_FIFOCFG_ASAP | 13>

In this case, I prefer having a macro instead of using a numeric value which
can stand for other things than the peripheral id and which could conflict
with other parameters such as the FIFO configuration.

If adding this macro is really an issue, it could be removed of course but
in this case I am in favour of readability.

Ludovic

> The other macros are fine.
> 
> 	Arnd

Re: [PATCH 1/3] ARM: at91: dt: add header to define at_hdmac configuration

[Ludovic Desroches]

On Fri, May 31, 2013 at 11:30:29PM +0200, Arnd Bergmann wrote:
> On Thursday 30 May 2013, ludovic.desroches@atmel.com wrote:
> > +/*
> > + * Source and/or destination peripheral ID
> > + */
> > +#define AT91_DMA_CFG_PER_ID_MASK       (0xff)
> > +#define AT91_DMA_CFG_PER_ID(id)                (id & AT91_DMA_CFG_PER_ID_MASK)
> > +
> 
> I'm a little worried about macros like this one spreading in the name
> of readability, and I don't see this as actual value-add here:
> 
> The output of the macro is always the same as the input, and passing
> an invalid number still results in an invalid output and no error message,
> it actually prevents us from warning about the mistake at run-time.
> 

You are right, this macro is here only for readability in order to not have
something like this:

dmas = <&dma0 AT91_DMA_CFG_FIFOCFG_ASAP | 13>

In this case, I prefer having a macro instead of using a numeric value which
can stand for other things than the peripheral id and which could conflict
with other parameters such as the FIFO configuration.

If adding this macro is really an issue, it could be removed of course but
in this case I am in favour of readability.

Ludovic

> The other macros are fine.
> 
> 	Arnd

Re: [PATCH 3/3] at_hdmac: add FIFO configuration parameter to DMA DT binding

[Ludovic Desroches]

On Fri, May 31, 2013 at 04:11:40PM -0700, Olof Johansson wrote:
> On Fri, May 31, 2013 at 2:31 AM, Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org> wrote:
> > On 31/05/2013 11:16, Ludovic Desroches :
> >>
> >> On Thu, May 30, 2013 at 06:39:36PM +0200, Jean-Christophe PLAGNIOL-VILLARD
> >> wrote:
> >>>
> >>> On 18:32 Thu 30 May     , Nicolas Ferre wrote:
> >>>>
> >>>> On 30/05/2013 18:08, ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org :
> >>>>>
> >>>>> From: Ludovic Desroches <ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> >>>>>
> >>>>> For most devices the FIFO configuration is the same i.e. when half FIFO
> >>>>> size is
> >>>>> available/filled, a source/destination request is serviced. But USART
> >>>>> devices
> >>>>> have to do it when there is enough space/data available to perform a
> >>>>> single
> >>>>> AHB access so the ASAP configuration.
> >>>>>
> >>>>> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
> >>>>> Signed-off-by: Ludovic Desroches <ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> >>>>
> >>>>
> >>>> Clear and neat: thanks Ludo.
> >>>
> >>>
> >>> agreed
> >>>
> >>> can we apply this via AT91 as this depends on some cleanup I did on DT
> >>> and
> >>> could result on some nigthmware conflict
> >>>
> >>
> >> In fact, I am not sure it's the best solution. I have noticed that there
> >> is a
> >> conflict with a patch sent by Nicolas (dmaengine: at_hdmac: extend
> >> hardware
> >> handshaking interface identification) which is already in Vinod's tree but
> >> which is not present on Jean-Christophe's cleanup tree on which I based
> >> these
> >> patches.
> >>
> >> Moreover this patch doesn't use macros introduced by Nicolas' patch. I may
> >> update it and it should go through Vinod's tree with a dependence on patch
> >> 1/3.
> >>
> >> What's your point of view?
> >
> >
> > Indeed. Another option could be to push patches 1 and 3 in dmaengine's tree
> > and make the 2nd patch go through arm-soc with a dependency on slave-dma GIT
> > tree (it seems it is an usual patern).
> >
> > Arnd, Jean-Christophe, what do you think?
> 
> I'm not sure patch 1 needs to go through the dma-engine tree? Sure,
> it's a dma-related header file but it's only used to craft the device
> tree in patch 2, it doesn't affect the driver.
> 

Some macros are used into the at_hdmac driver. 

Ludovic

> (Arnd has comments on the patch that should be resolved though).
> 
> 
> -Olof

Re: [PATCH 3/3] at_hdmac: add FIFO configuration parameter to DMA DT binding

[Ludovic Desroches]

On Fri, May 31, 2013 at 04:11:40PM -0700, Olof Johansson wrote:
> On Fri, May 31, 2013 at 2:31 AM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
> > On 31/05/2013 11:16, Ludovic Desroches :
> >>
> >> On Thu, May 30, 2013 at 06:39:36PM +0200, Jean-Christophe PLAGNIOL-VILLARD
> >> wrote:
> >>>
> >>> On 18:32 Thu 30 May     , Nicolas Ferre wrote:
> >>>>
> >>>> On 30/05/2013 18:08, ludovic.desroches@atmel.com :
> >>>>>
> >>>>> From: Ludovic Desroches <ludovic.desroches@atmel.com>
> >>>>>
> >>>>> For most devices the FIFO configuration is the same i.e. when half FIFO
> >>>>> size is
> >>>>> available/filled, a source/destination request is serviced. But USART
> >>>>> devices
> >>>>> have to do it when there is enough space/data available to perform a
> >>>>> single
> >>>>> AHB access so the ASAP configuration.
> >>>>>
> >>>>> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> >>>>> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> >>>>
> >>>>
> >>>> Clear and neat: thanks Ludo.
> >>>
> >>>
> >>> agreed
> >>>
> >>> can we apply this via AT91 as this depends on some cleanup I did on DT
> >>> and
> >>> could result on some nigthmware conflict
> >>>
> >>
> >> In fact, I am not sure it's the best solution. I have noticed that there
> >> is a
> >> conflict with a patch sent by Nicolas (dmaengine: at_hdmac: extend
> >> hardware
> >> handshaking interface identification) which is already in Vinod's tree but
> >> which is not present on Jean-Christophe's cleanup tree on which I based
> >> these
> >> patches.
> >>
> >> Moreover this patch doesn't use macros introduced by Nicolas' patch. I may
> >> update it and it should go through Vinod's tree with a dependence on patch
> >> 1/3.
> >>
> >> What's your point of view?
> >
> >
> > Indeed. Another option could be to push patches 1 and 3 in dmaengine's tree
> > and make the 2nd patch go through arm-soc with a dependency on slave-dma GIT
> > tree (it seems it is an usual patern).
> >
> > Arnd, Jean-Christophe, what do you think?
> 
> I'm not sure patch 1 needs to go through the dma-engine tree? Sure,
> it's a dma-related header file but it's only used to craft the device
> tree in patch 2, it doesn't affect the driver.
> 

Some macros are used into the at_hdmac driver. 

Ludovic

> (Arnd has comments on the patch that should be resolved though).
> 
> 
> -Olof

Re: [PATCH 1/3] ARM: at91: dt: add header to define at_hdmac configuration

[Nicolas Ferre]

On 03/06/2013 09:32, Ludovic Desroches :
> On Fri, May 31, 2013 at 11:30:29PM +0200, Arnd Bergmann wrote:
>> On Thursday 30 May 2013, ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org wrote:
>>> +/*
>>> + * Source and/or destination peripheral ID
>>> + */
>>> +#define AT91_DMA_CFG_PER_ID_MASK       (0xff)
>>> +#define AT91_DMA_CFG_PER_ID(id)                (id & AT91_DMA_CFG_PER_ID_MASK)
>>> +
>>
>> I'm a little worried about macros like this one spreading in the name
>> of readability, and I don't see this as actual value-add here:
>>
>> The output of the macro is always the same as the input, and passing
>> an invalid number still results in an invalid output and no error message,
>> it actually prevents us from warning about the mistake at run-time.
>>
>
> You are right, this macro is here only for readability in order to not have
> something like this:
>
> dmas = <&dma0 AT91_DMA_CFG_FIFOCFG_ASAP | 13>
>
> In this case, I prefer having a macro instead of using a numeric value which
> can stand for other things than the peripheral id and which could conflict
> with other parameters such as the FIFO configuration.
>
> If adding this macro is really an issue, it could be removed of course but
> in this case I am in favor of readability.

Arnd,

I think that the case highlighted by Ludovic is indeed a case where 
someone can be confused by the presentation of data in the cell... even 
if I like the use of macro as implemented by Ludovic, I do not have a 
strong opinion about this.

Best regards,
-- 
Nicolas Ferre