[PATCH v2 0/7] ARM: dma: Support dma-ranges and dma-coherent

[PATCH v2 0/7] ARM: dma: Support dma-ranges and dma-coherent

[Santosh Shilimkar]

Following the RFC [1], [2] comments, suggestions and some IRC discussion
with Arnd Bergmann, here is the refreshed series. Attempt is to use dma-ranges
and dma-coherent properties to setup the dma for ARM platforms.

Platforms needing more funky stuff than the generic ones can use per
platform device callbacks. Differnt buses like PCIE dma parsing still needs
to be addressed though.

Cc: Russell King <linux@arm.linux.org.uk>
Cc: Greg Ungerer <gerg@uclinux.org>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Olof Johansson <olof@lixom.net>
CC: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: Magnus Damm <magnus.damm@gmail.com>
Cc: Linus Walleij <linus.walleij@linaro.org>

Grygorii Strashko (5):
  ARM: mm: Introduce archdata.dma_pfn_offset
  ARM: mm: Remove unsed dma_to_virt()
  dma: of: introduce of_dma_get_range() helper
  ARM: of: introduce common routine for DMA configuration
  ARM: dts: keystone: Use dma-ranges property

Santosh Shilimkar (2):
  dma: of: introduce of_dma_is_coherent() helper
  ARM: dts: keystone: Udate USB node for dma properties

 arch/arm/boot/dts/keystone.dtsi             |    3 +
 arch/arm/include/asm/device.h               |    1 +
 arch/arm/include/asm/dma-mapping.h          |   25 +++----
 arch/arm/include/asm/prom.h                 |    3 +
 arch/arm/kernel/devtree.c                   |   61 +++++++++++++++
 arch/arm/mach-iop13xx/include/mach/memory.h |   11 ---
 arch/arm/mach-ks8695/include/mach/memory.h  |    2 -
 arch/arm/mach-omap1/include/mach/memory.h   |    4 -
 drivers/dma/of-dma.c                        |  108 +++++++++++++++++++++++++++
 drivers/of/platform.c                       |    5 +-
 include/linux/of.h                          |    2 +-
 include/linux/of_dma.h                      |   13 ++++
 12 files changed, 205 insertions(+), 33 deletions(-)

Regards,
Santosh
[1] http://www.spinics.net/lists/arm-kernel/msg304555.html
[2] http://www.spinics.net/lists/arm-kernel/msg310562.html 
-- 
1.7.9.5

[PATCH v2 1/7] ARM: mm: Introduce archdata.dma_pfn_offset

[Santosh Shilimkar]

From: Grygorii Strashko <grygorii.strashko@ti.com>

In most of cases DMA addresses can be performed using offset value of
 Bus address space relatively to physical address space as following:

PFN->DMA:
 __pfn_to_phys(pfn + [-]dma_pfn_offset)

DMA->PFN:
 __phys_to_pfn(dma_addr) + [-]dma_pfn_offset

This patch introduces new field dma_pfn_offset in ARM dev_archdata
structure which has to be filed per-device at arch init time
(simplest way is to use Platform bus notifier to handle
BUS_NOTIFY_ADD_DEVICE event) and updates DMA address translation
routines in order to accommodate bus offset value by default.

Cc: Russell King <linux@arm.linux.org.uk>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Olof Johansson <olof@lixom.net>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/include/asm/device.h      |    1 +
 arch/arm/include/asm/dma-mapping.h |   17 +++++++++++++----
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/arch/arm/include/asm/device.h b/arch/arm/include/asm/device.h
index dc662fc..861961c 100644
--- a/arch/arm/include/asm/device.h
+++ b/arch/arm/include/asm/device.h
@@ -8,6 +8,7 @@
 
 struct dev_archdata {
 	struct dma_map_ops	*dma_ops;
+	unsigned long		dma_pfn_offset;
 #ifdef CONFIG_DMABOUNCE
 	struct dmabounce_device_info *dmabounce;
 #endif
diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h
index e701a4d..247ed72 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -58,22 +58,31 @@ static inline int dma_set_mask(struct device *dev, u64 mask)
 #ifndef __arch_pfn_to_dma
 static inline dma_addr_t pfn_to_dma(struct device *dev, unsigned long pfn)
 {
-	return (dma_addr_t)__pfn_to_bus(pfn);
+	if (!dev)
+		return DMA_ERROR_CODE;
+	return (dma_addr_t)__pfn_to_bus(pfn - dev->archdata.dma_pfn_offset);
 }
 
 static inline unsigned long dma_to_pfn(struct device *dev, dma_addr_t addr)
 {
-	return __bus_to_pfn(addr);
+	if (!dev)
+		return 0;
+	return __bus_to_pfn(addr) + dev->archdata.dma_pfn_offset;
 }
 
 static inline void *dma_to_virt(struct device *dev, dma_addr_t addr)
 {
-	return (void *)__bus_to_virt((unsigned long)addr);
+	if (!dev)
+		return NULL;
+	return (void *)__bus_to_virt(__pfn_to_bus(dma_to_pfn(dev, addr)));
 }
 
 static inline dma_addr_t virt_to_dma(struct device *dev, void *addr)
 {
-	return (dma_addr_t)__virt_to_bus((unsigned long)(addr));
+	if (!dev)
+		return DMA_ERROR_CODE;
+	return pfn_to_dma(dev,
+			   __bus_to_pfn(__virt_to_bus((unsigned long)(addr))));
 }
 
 #else
-- 
1.7.9.5

Re: [PATCH v2 1/7] ARM: mm: Introduce archdata.dma_pfn_offset

[Arnd Bergmann]

On Thursday 27 February 2014 16:17:46 Santosh Shilimkar wrote:
> From: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
> 
> In most of cases DMA addresses can be performed using offset value of
>  Bus address space relatively to physical address space as following:
> 
> PFN->DMA:
>  __pfn_to_phys(pfn + [-]dma_pfn_offset)
> 
> DMA->PFN:
>  __phys_to_pfn(dma_addr) + [-]dma_pfn_offset
> 
> This patch introduces new field dma_pfn_offset in ARM dev_archdata
> structure which has to be filed per-device at arch init time
> (simplest way is to use Platform bus notifier to handle
> BUS_NOTIFY_ADD_DEVICE event) and updates DMA address translation
> routines in order to accommodate bus offset value by default.
> 
> Cc: Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
> Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> Cc: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>
> Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>

This was my suggestion, and the implementation is trivial, so

Acked-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>

This should be most of what LinusW needs to generalize the
mach-integrator PCI DMA mapping.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/7] ARM: mm: Introduce archdata.dma_pfn_offset

[Linus Walleij]

On Fri, Feb 28, 2014 at 5:17 AM, Santosh Shilimkar
<santosh.shilimkar-l0cyMroinI0@public.gmane.org> wrote:

> From: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
>
> In most of cases DMA addresses can be performed using offset value of
>  Bus address space relatively to physical address space as following:
>
> PFN->DMA:
>  __pfn_to_phys(pfn + [-]dma_pfn_offset)
>
> DMA->PFN:
>  __phys_to_pfn(dma_addr) + [-]dma_pfn_offset
>
> This patch introduces new field dma_pfn_offset in ARM dev_archdata
> structure which has to be filed per-device at arch init time
> (simplest way is to use Platform bus notifier to handle
> BUS_NOTIFY_ADD_DEVICE event) and updates DMA address translation
> routines in order to accommodate bus offset value by default.
>
> Cc: Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
> Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> Cc: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>
> Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>

This looks like exactly what I need to proceed to multiplatform on
Integrator.
Acked-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/7] ARM: mm: Introduce archdata.dma_pfn_offset

[Santosh Shilimkar]

On Wednesday 05 March 2014 12:45 PM, Linus Walleij wrote:
> On Fri, Feb 28, 2014 at 5:17 AM, Santosh Shilimkar
> <santosh.shilimkar-l0cyMroinI0@public.gmane.org> wrote:
> 
>> From: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
>>
>> In most of cases DMA addresses can be performed using offset value of
>>  Bus address space relatively to physical address space as following:
>>
>> PFN->DMA:
>>  __pfn_to_phys(pfn + [-]dma_pfn_offset)
>>
>> DMA->PFN:
>>  __phys_to_pfn(dma_addr) + [-]dma_pfn_offset
>>
>> This patch introduces new field dma_pfn_offset in ARM dev_archdata
>> structure which has to be filed per-device at arch init time
>> (simplest way is to use Platform bus notifier to handle
>> BUS_NOTIFY_ADD_DEVICE event) and updates DMA address translation
>> routines in order to accommodate bus offset value by default.
>>
>> Cc: Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
>> Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
>> Cc: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>
> 
> This looks like exactly what I need to proceed to multiplatform on
> Integrator.
> Acked-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 
Thanks Linus.

Just to make folks aware on the thread, after some more inputs and discussion
at connect, looks like we need to make this work for other arch's too. So am
going to move dma_pfn_offset from archdata to device_dma_parameters  so that
arch code as well as driver code should be able to access/modify the pfn
offset.

Regards,
Santosh





--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/7] ARM: mm: Remove unsed dma_to_virt()

[Santosh Shilimkar]

From: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>

Remove dma_to_virt() as there are no in-tree users of it.

Cc: Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Cc: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>
Cc: Greg Ungerer <gerg-JBU5SbJe1FlAfugRpC6u6w@public.gmane.org>
Cc: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>
---
 arch/arm/include/asm/dma-mapping.h          |   14 +-------------
 arch/arm/mach-iop13xx/include/mach/memory.h |   11 -----------
 arch/arm/mach-ks8695/include/mach/memory.h  |    2 --
 arch/arm/mach-omap1/include/mach/memory.h   |    4 ----
 4 files changed, 1 insertion(+), 30 deletions(-)

diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h
index 247ed72..e365158 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -51,7 +51,7 @@ static inline int dma_set_mask(struct device *dev, u64 mask)
 #endif
 
 /*
- * dma_to_pfn/pfn_to_dma/dma_to_virt/virt_to_dma are architecture private
+ * dma_to_pfn/pfn_to_dma/virt_to_dma are architecture private
  * functions used internally by the DMA-mapping API to provide DMA
  * addresses. They must not be used by drivers.
  */
@@ -70,13 +70,6 @@ static inline unsigned long dma_to_pfn(struct device *dev, dma_addr_t addr)
 	return __bus_to_pfn(addr) + dev->archdata.dma_pfn_offset;
 }
 
-static inline void *dma_to_virt(struct device *dev, dma_addr_t addr)
-{
-	if (!dev)
-		return NULL;
-	return (void *)__bus_to_virt(__pfn_to_bus(dma_to_pfn(dev, addr)));
-}
-
 static inline dma_addr_t virt_to_dma(struct device *dev, void *addr)
 {
 	if (!dev)
@@ -96,11 +89,6 @@ static inline unsigned long dma_to_pfn(struct device *dev, dma_addr_t addr)
 	return __arch_dma_to_pfn(dev, addr);
 }
 
-static inline void *dma_to_virt(struct device *dev, dma_addr_t addr)
-{
-	return __arch_dma_to_virt(dev, addr);
-}
-
 static inline dma_addr_t virt_to_dma(struct device *dev, void *addr)
 {
 	return __arch_virt_to_dma(dev, addr);
diff --git a/arch/arm/mach-iop13xx/include/mach/memory.h b/arch/arm/mach-iop13xx/include/mach/memory.h
index 7c032d0..1223c85 100644
--- a/arch/arm/mach-iop13xx/include/mach/memory.h
+++ b/arch/arm/mach-iop13xx/include/mach/memory.h
@@ -36,17 +36,6 @@ static inline void __iomem *__lbus_to_virt(dma_addr_t x)
 #define is_lbus_device(dev) 				\
 	(dev && strncmp(dev->bus->name, "platform", 8) == 0)
 
-#define __arch_dma_to_virt(dev, addr)					\
-	({								\
-		void * __virt;						\
-		dma_addr_t __dma = addr;				\
-		if (is_lbus_device(dev) && __is_lbus_dma(__dma))	\
-			__virt = __lbus_to_virt(__dma);			\
-		else							\
-			__virt = (void *)__phys_to_virt(__dma);		\
-		__virt;							\
-	})
-
 #define __arch_virt_to_dma(dev, addr)					\
 	({								\
 		void * __virt = addr;					\
diff --git a/arch/arm/mach-ks8695/include/mach/memory.h b/arch/arm/mach-ks8695/include/mach/memory.h
index 95e731a..f42477c 100644
--- a/arch/arm/mach-ks8695/include/mach/memory.h
+++ b/arch/arm/mach-ks8695/include/mach/memory.h
@@ -31,8 +31,6 @@
 /* Platform-bus mapping */
 extern struct bus_type platform_bus_type;
 #define is_lbus_device(dev)		(dev && dev->bus == &platform_bus_type)
-#define __arch_dma_to_virt(dev, x)	({ (void *) (is_lbus_device(dev) ? \
-					__phys_to_virt(x) : __bus_to_virt(x)); })
 #define __arch_virt_to_dma(dev, x)	({ is_lbus_device(dev) ? \
 					(dma_addr_t)__virt_to_phys((unsigned long)x) \
 					: (dma_addr_t)__virt_to_bus(x); })
diff --git a/arch/arm/mach-omap1/include/mach/memory.h b/arch/arm/mach-omap1/include/mach/memory.h
index 3c25305..73f86db 100644
--- a/arch/arm/mach-omap1/include/mach/memory.h
+++ b/arch/arm/mach-omap1/include/mach/memory.h
@@ -43,10 +43,6 @@
 	   __phys_to_pfn(__dma);				\
 	})
 
-#define __arch_dma_to_virt(dev, addr)	({ (void *) (is_lbus_device(dev) ? \
-						lbus_to_virt(addr) : \
-						__phys_to_virt(addr)); })
-
 #define __arch_virt_to_dma(dev, addr)	({ unsigned long __addr = (unsigned long)(addr); \
 					   (dma_addr_t) (is_lbus_device(dev) ? \
 						virt_to_lbus(__addr) : \
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/7] ARM: mm: Remove unsed dma_to_virt()

[Arnd Bergmann]

On Thursday 27 February 2014 16:17:47 Santosh Shilimkar wrote:
> From: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
> 
> Remove dma_to_virt() as there are no in-tree users of it.
> 
> Cc: Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
> Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> Cc: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>
> Cc: Greg Ungerer <gerg-JBU5SbJe1FlAfugRpC6u6w@public.gmane.org>
> Cc: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>

Acked-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/7] ARM: mm: Remove unsed dma_to_virt()

[Greg Ungerer]

Hi Santosh,

On 28/02/14 07:17, Santosh Shilimkar wrote:
> From: Grygorii Strashko <grygorii.strashko@ti.com>
> 
> Remove dma_to_virt() as there are no in-tree users of it.
> 
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Olof Johansson <olof@lixom.net>
> Cc: Greg Ungerer <gerg@uclinux.org>
> Cc: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

I am fine with the ks8695 change:

Acked-by: Greg Ungerer <gerg@uclinux.org>


> ---
>  arch/arm/include/asm/dma-mapping.h          |   14 +-------------
>  arch/arm/mach-iop13xx/include/mach/memory.h |   11 -----------
>  arch/arm/mach-ks8695/include/mach/memory.h  |    2 --
>  arch/arm/mach-omap1/include/mach/memory.h   |    4 ----
>  4 files changed, 1 insertion(+), 30 deletions(-)
> 
> diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h
> index 247ed72..e365158 100644
> --- a/arch/arm/include/asm/dma-mapping.h
> +++ b/arch/arm/include/asm/dma-mapping.h
> @@ -51,7 +51,7 @@ static inline int dma_set_mask(struct device *dev, u64 mask)
>  #endif
>  
>  /*
> - * dma_to_pfn/pfn_to_dma/dma_to_virt/virt_to_dma are architecture private
> + * dma_to_pfn/pfn_to_dma/virt_to_dma are architecture private
>   * functions used internally by the DMA-mapping API to provide DMA
>   * addresses. They must not be used by drivers.
>   */
> @@ -70,13 +70,6 @@ static inline unsigned long dma_to_pfn(struct device *dev, dma_addr_t addr)
>  	return __bus_to_pfn(addr) + dev->archdata.dma_pfn_offset;
>  }
>  
> -static inline void *dma_to_virt(struct device *dev, dma_addr_t addr)
> -{
> -	if (!dev)
> -		return NULL;
> -	return (void *)__bus_to_virt(__pfn_to_bus(dma_to_pfn(dev, addr)));
> -}
> -
>  static inline dma_addr_t virt_to_dma(struct device *dev, void *addr)
>  {
>  	if (!dev)
> @@ -96,11 +89,6 @@ static inline unsigned long dma_to_pfn(struct device *dev, dma_addr_t addr)
>  	return __arch_dma_to_pfn(dev, addr);
>  }
>  
> -static inline void *dma_to_virt(struct device *dev, dma_addr_t addr)
> -{
> -	return __arch_dma_to_virt(dev, addr);
> -}
> -
>  static inline dma_addr_t virt_to_dma(struct device *dev, void *addr)
>  {
>  	return __arch_virt_to_dma(dev, addr);
> diff --git a/arch/arm/mach-iop13xx/include/mach/memory.h b/arch/arm/mach-iop13xx/include/mach/memory.h
> index 7c032d0..1223c85 100644
> --- a/arch/arm/mach-iop13xx/include/mach/memory.h
> +++ b/arch/arm/mach-iop13xx/include/mach/memory.h
> @@ -36,17 +36,6 @@ static inline void __iomem *__lbus_to_virt(dma_addr_t x)
>  #define is_lbus_device(dev) 				\
>  	(dev && strncmp(dev->bus->name, "platform", 8) == 0)
>  
> -#define __arch_dma_to_virt(dev, addr)					\
> -	({								\
> -		void * __virt;						\
> -		dma_addr_t __dma = addr;				\
> -		if (is_lbus_device(dev) && __is_lbus_dma(__dma))	\
> -			__virt = __lbus_to_virt(__dma);			\
> -		else							\
> -			__virt = (void *)__phys_to_virt(__dma);		\
> -		__virt;							\
> -	})
> -
>  #define __arch_virt_to_dma(dev, addr)					\
>  	({								\
>  		void * __virt = addr;					\
> diff --git a/arch/arm/mach-ks8695/include/mach/memory.h b/arch/arm/mach-ks8695/include/mach/memory.h
> index 95e731a..f42477c 100644
> --- a/arch/arm/mach-ks8695/include/mach/memory.h
> +++ b/arch/arm/mach-ks8695/include/mach/memory.h
> @@ -31,8 +31,6 @@
>  /* Platform-bus mapping */
>  extern struct bus_type platform_bus_type;
>  #define is_lbus_device(dev)		(dev && dev->bus == &platform_bus_type)
> -#define __arch_dma_to_virt(dev, x)	({ (void *) (is_lbus_device(dev) ? \
> -					__phys_to_virt(x) : __bus_to_virt(x)); })
>  #define __arch_virt_to_dma(dev, x)	({ is_lbus_device(dev) ? \
>  					(dma_addr_t)__virt_to_phys((unsigned long)x) \
>  					: (dma_addr_t)__virt_to_bus(x); })
> diff --git a/arch/arm/mach-omap1/include/mach/memory.h b/arch/arm/mach-omap1/include/mach/memory.h
> index 3c25305..73f86db 100644
> --- a/arch/arm/mach-omap1/include/mach/memory.h
> +++ b/arch/arm/mach-omap1/include/mach/memory.h
> @@ -43,10 +43,6 @@
>  	   __phys_to_pfn(__dma);				\
>  	})
>  
> -#define __arch_dma_to_virt(dev, addr)	({ (void *) (is_lbus_device(dev) ? \
> -						lbus_to_virt(addr) : \
> -						__phys_to_virt(addr)); })
> -
>  #define __arch_virt_to_dma(dev, addr)	({ unsigned long __addr = (unsigned long)(addr); \
>  					   (dma_addr_t) (is_lbus_device(dev) ? \
>  						virt_to_lbus(__addr) : \
> 

[PATCH v2 3/7] dma: of: introduce of_dma_get_range() helper

[Santosh Shilimkar]

From: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>

The of_dma_get_range() allows to find "dma-range" property for
the specified device and parse it.
 dma-ranges format:
   DMA addr (dma_addr)		: naddr cells
   CPU addr (phys_addr_t)	: pna cells
   size				: nsize cells

Cc: Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Cc: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>
Cc: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>
---
 drivers/dma/of-dma.c   |   86 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of_dma.h |    8 +++++
 2 files changed, 94 insertions(+)

diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
index e8fe9dc..9b51768 100644
--- a/drivers/dma/of-dma.c
+++ b/drivers/dma/of-dma.c
@@ -17,6 +17,7 @@
 #include <linux/slab.h>
 #include <linux/of.h>
 #include <linux/of_dma.h>
+#include <linux/of_address.h>
 
 static LIST_HEAD(of_dma_list);
 static DEFINE_MUTEX(of_dma_lock);
@@ -218,3 +219,88 @@ struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
 			&dma_spec->args[0]);
 }
 EXPORT_SYMBOL_GPL(of_dma_simple_xlate);
+
+/**
+ * of_dma_get_range - Get DMA range info
+ * @np:		device node to get DMA range info
+ * @dma_addr:	pointer to store initial DMA address of DMA range
+ * @paddr:	pointer to store initial CPU address of DMA range
+ * @size:	pointer to store size of DMA range
+ *
+ * Look in bottom up direction for the first "dma-range" property
+ * and parse it.
+ *  dma-ranges format:
+ *	DMA addr (dma_addr)	: naddr cells
+ *	CPU addr (phys_addr_t)	: pna cells
+ *	size			: nsize cells
+ *
+ * It returns -ENODEV if "dma-ranges" property was not found
+ * for this device in DT.
+ */
+extern int of_dma_get_range(struct device_node *np, dma_addr_t *dma_addr,
+		phys_addr_t *paddr, phys_addr_t *size)
+{
+	struct device_node *node = np;
+	const u32 *ranges = NULL;
+	int len, naddr, nsize, pna;
+	int ret = 0;
+
+	if (!node)
+		return -EINVAL;
+
+	while (1) {
+		naddr = of_n_addr_cells(node);
+		nsize = of_n_size_cells(node);
+		node = of_get_next_parent(node);
+		if (!node)
+			break;
+
+		ranges = of_get_property(node, "dma-ranges", &len);
+
+		/* Ignore empty ranges, they imply no translation required */
+		if (ranges && len > 0)
+			break;
+
+		/*
+		 * At least empty ranges has to be defined for parent node if
+		 * DMA is supported
+		 */
+		if (!ranges)
+			break;
+	}
+
+	if (!ranges) {
+		pr_debug("%s: no dma-ranges found for node(%s)\n",
+			 __func__, np->full_name);
+		ret = -ENODEV;
+		goto out;
+	}
+
+	len /= sizeof(u32);
+
+	pna = of_n_addr_cells(node);
+
+	/* dma-ranges format:
+	 * DMA addr	: naddr cells
+	 * CPU addr	: pna cells
+	 * size		: nsize cells
+	 */
+	*dma_addr = of_read_number(ranges, naddr);
+	*paddr = of_translate_dma_address(np, ranges);
+	if (*paddr == OF_BAD_ADDR) {
+		pr_err("%s: translation of DMA address(%#08x) to CPU address failed node(%s)\n",
+		       __func__, *dma_addr, np->full_name);
+		ret = -EINVAL;
+	}
+
+	*size = of_read_number(ranges + naddr + pna, nsize);
+
+	pr_debug("dma_addr(%08x) cpu_addr(%pa) size(%pa)\n",
+		 *dma_addr, paddr, size);
+
+out:
+	of_node_put(node);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(of_dma_get_range);
diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
index ae36298..f04171a 100644
--- a/include/linux/of_dma.h
+++ b/include/linux/of_dma.h
@@ -41,6 +41,9 @@ extern struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
 						     const char *name);
 extern struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
 		struct of_dma *ofdma);
+
+extern int of_dma_get_range(struct device_node *np, dma_addr_t *dma_addr,
+		phys_addr_t *paddr, phys_addr_t *size);
 #else
 static inline int of_dma_controller_register(struct device_node *np,
 		struct dma_chan *(*of_dma_xlate)
@@ -66,6 +69,11 @@ static inline struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_s
 	return NULL;
 }
 
+static inline int of_dma_get_range(struct device_node *np, dma_addr_t *dma_addr,
+		phys_addr_t *paddr, phys_addr_t *size);
+{
+	return -ENODEV;
+}
 #endif
 
 #endif /* __LINUX_OF_DMA_H */
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 4/7] dma: of: introduce of_dma_is_coherent() helper

[Santosh Shilimkar]

The of_dma_is_coherent() helper parses the given DT device
node to see if the "dma-coherent" property is supported and
returns true or false accordingly.

Cc: Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Cc: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>
Cc: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>
---
 drivers/dma/of-dma.c   |   22 ++++++++++++++++++++++
 include/linux/of_dma.h |    5 +++++
 2 files changed, 27 insertions(+)

diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
index 9b51768..c5958d1 100644
--- a/drivers/dma/of-dma.c
+++ b/drivers/dma/of-dma.c
@@ -304,3 +304,25 @@ out:
 	return ret;
 }
 EXPORT_SYMBOL_GPL(of_dma_get_range);
+
+/**
+ * of_dma_is_coherent - Check if device is coherent
+ * @np:	device node
+ *
+ * It returns true if "dma-coherent" property was found
+ * for this device in DT.
+ */
+bool of_dma_is_coherent(struct device_node *np)
+{
+	struct device_node *node = np;
+
+	while (node) {
+		if (of_property_read_bool(node, "dma-coherent")) {
+			of_node_put(node);
+			return true;
+		}
+		node = of_get_next_parent(node);
+	}
+	return false;
+}
+EXPORT_SYMBOL_GPL(of_dma_is_coherent);
diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
index f04171a..6191f02 100644
--- a/include/linux/of_dma.h
+++ b/include/linux/of_dma.h
@@ -44,6 +44,7 @@ extern struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
 
 extern int of_dma_get_range(struct device_node *np, dma_addr_t *dma_addr,
 		phys_addr_t *paddr, phys_addr_t *size);
+extern bool of_dma_is_coherent(struct device_node *np);
 #else
 static inline int of_dma_controller_register(struct device_node *np,
 		struct dma_chan *(*of_dma_xlate)
@@ -74,6 +75,10 @@ static inline int of_dma_get_range(struct device_node *np, dma_addr_t *dma_addr,
 {
 	return -ENODEV;
 }
+static inline bool of_dma_is_coherent(struct device_node *np)
+{
+	return false;
+}
 #endif
 
 #endif /* __LINUX_OF_DMA_H */
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 4/7] dma: of: introduce of_dma_is_coherent() helper

[Arnd Bergmann]

On Thursday 27 February 2014 16:17:49 Santosh Shilimkar wrote:
> +
> +/**
> + * of_dma_is_coherent - Check if device is coherent
> + * @np:        device node
> + *
> + * It returns true if "dma-coherent" property was found
> + * for this device in DT.
> + */
> +bool of_dma_is_coherent(struct device_node *np)
> +{
> +       struct device_node *node = np;
> +
> +       while (node) {
> +               if (of_property_read_bool(node, "dma-coherent")) {
> +                       of_node_put(node);
> +                       return true;
> +               }
> +               node = of_get_next_parent(node);
> +       }
> +       return false;
> +}
> +EXPORT_SYMBOL_GPL(of_dma_is_coherent);
> 

This won't work on architectures that are always coherent and
did not need 'dma-coherent' properties before, such as IBM
Power servers.

That said, I think the property makes sense, and we already have
platforms using it (highbank is the one I'm aware of).

We probably need ways to override this function in both ways:
"always coherent" (powerpc, x86), and "never coherent" (keystone
without LPAE) from platform code, and it would be nice to put
either option into DT in a global location as well. We may have
to go through a few iterations of this patch to get the best
algorithm, but I think the interface is good at least.

	Arnd

Re: [PATCH v2 4/7] dma: of: introduce of_dma_is_coherent() helper

[Santosh Shilimkar]

On Friday 28 February 2014 04:39 AM, Arnd Bergmann wrote:
> On Thursday 27 February 2014 16:17:49 Santosh Shilimkar wrote:
>> +
>> +/**
>> + * of_dma_is_coherent - Check if device is coherent
>> + * @np:        device node
>> + *
>> + * It returns true if "dma-coherent" property was found
>> + * for this device in DT.
>> + */
>> +bool of_dma_is_coherent(struct device_node *np)
>> +{
>> +       struct device_node *node = np;
>> +
>> +       while (node) {
>> +               if (of_property_read_bool(node, "dma-coherent")) {
>> +                       of_node_put(node);
>> +                       return true;
>> +               }
>> +               node = of_get_next_parent(node);
>> +       }
>> +       return false;
>> +}
>> +EXPORT_SYMBOL_GPL(of_dma_is_coherent);
>>
> 
> This won't work on architectures that are always coherent and
> did not need 'dma-coherent' properties before, such as IBM
> Power servers.
> 
> That said, I think the property makes sense, and we already have
> platforms using it (highbank is the one I'm aware of).
> 
> We probably need ways to override this function in both ways:
> "always coherent" (powerpc, x86), and "never coherent" (keystone
> without LPAE) from platform code, and it would be nice to put
> either option into DT in a global location as well. We may have
> to go through a few iterations of this patch to get the best
> algorithm, but I think the interface is good at least.
> 
Probably we should discuss bit more next week at connect. The
current 'dma-coherent' is a per device property. For arch's
which are always coherent, the per device property doesn't make
sense.

BTW, the current users of this API is only ARM32 bit port
and if this satisfies the ARM platforms, we should get
this in kernel and then address other cases on need
basis.

Regards,
Santosh

Re: [PATCH v2 4/7] dma: of: introduce of_dma_is_coherent() helper

[Catalin Marinas]

On Fri, Feb 28, 2014 at 09:17:08AM -0500, Santosh Shilimkar wrote:
> On Friday 28 February 2014 04:39 AM, Arnd Bergmann wrote:
> > On Thursday 27 February 2014 16:17:49 Santosh Shilimkar wrote:
> >> +
> >> +/**
> >> + * of_dma_is_coherent - Check if device is coherent
> >> + * @np:        device node
> >> + *
> >> + * It returns true if "dma-coherent" property was found
> >> + * for this device in DT.
> >> + */
> >> +bool of_dma_is_coherent(struct device_node *np)
> >> +{
> >> +       struct device_node *node = np;
> >> +
> >> +       while (node) {
> >> +               if (of_property_read_bool(node, "dma-coherent")) {
> >> +                       of_node_put(node);
> >> +                       return true;
> >> +               }
> >> +               node = of_get_next_parent(node);
> >> +       }
> >> +       return false;
> >> +}
> >> +EXPORT_SYMBOL_GPL(of_dma_is_coherent);
> >>
> > 
> > This won't work on architectures that are always coherent and
> > did not need 'dma-coherent' properties before, such as IBM
> > Power servers.
> > 
> > That said, I think the property makes sense, and we already have
> > platforms using it (highbank is the one I'm aware of).
> > 
> > We probably need ways to override this function in both ways:
> > "always coherent" (powerpc, x86), and "never coherent" (keystone
> > without LPAE) from platform code, and it would be nice to put
> > either option into DT in a global location as well. We may have
> > to go through a few iterations of this patch to get the best
> > algorithm, but I think the interface is good at least.
> 
> Probably we should discuss bit more next week at connect. The
> current 'dma-coherent' is a per device property. For arch's
> which are always coherent, the per device property doesn't make
> sense.
> 
> BTW, the current users of this API is only ARM32 bit port
> and if this satisfies the ARM platforms, we should get
> this in kernel and then address other cases on need
> basis.

I have a suspicion that we'll need this API on arm64 at some point as
well. I get regular questions about DMA cache maintenance for arm64 and
I carried a patch in my tree for a long time (now in -next). While full
coherency is nice, there are some devices on certain SoCs that are more
efficient (power, speed) when they don't have to snoop the CPU caches.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 4/7] dma: of: introduce of_dma_is_coherent() helper

[Rob Herring]

On Fri, Feb 28, 2014 at 3:39 AM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> On Thursday 27 February 2014 16:17:49 Santosh Shilimkar wrote:
>> +
>> +/**
>> + * of_dma_is_coherent - Check if device is coherent
>> + * @np:        device node
>> + *
>> + * It returns true if "dma-coherent" property was found
>> + * for this device in DT.
>> + */
>> +bool of_dma_is_coherent(struct device_node *np)
>> +{
>> +       struct device_node *node = np;
>> +
>> +       while (node) {
>> +               if (of_property_read_bool(node, "dma-coherent")) {
>> +                       of_node_put(node);
>> +                       return true;
>> +               }
>> +               node = of_get_next_parent(node);
>> +       }
>> +       return false;
>> +}
>> +EXPORT_SYMBOL_GPL(of_dma_is_coherent);
>>
>
> This won't work on architectures that are always coherent and
> did not need 'dma-coherent' properties before, such as IBM
> Power servers.
>
> That said, I think the property makes sense, and we already have
> platforms using it (highbank is the one I'm aware of).
>
> We probably need ways to override this function in both ways:
> "always coherent" (powerpc, x86), and "never coherent" (keystone
> without LPAE) from platform code, and it would be nice to put
> either option into DT in a global location as well. We may have
> to go through a few iterations of this patch to get the best
> algorithm, but I think the interface is good at least.

I know Will D was not a fan of this property. Primarily I believe
because you may need to describe more than just a boolean in more
complex bus topologies.

Effectively, highbank is always coherent. It was only PCI that is
non-coherent, but I can safely say PCI will never be enabled at this
point. There are no designs with PCI beyond 1 or 2 validation boards
(total boards, not designs), and getting PCI to work was quite hacky
due to only a 1MB window. The other masters are programmable, but only
the coherent path is used as the non-coherent path actually has some
issues. I had expected the opposite believing the ACP port would
actually have issues which is also why I made it configurable.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 4/7] dma: of: introduce of_dma_is_coherent() helper

[Santosh Shilimkar]

On Friday 28 February 2014 10:14 AM, Rob Herring wrote:
> On Fri, Feb 28, 2014 at 3:39 AM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
>> On Thursday 27 February 2014 16:17:49 Santosh Shilimkar wrote:
>>> +
>>> +/**
>>> + * of_dma_is_coherent - Check if device is coherent
>>> + * @np:        device node
>>> + *
>>> + * It returns true if "dma-coherent" property was found
>>> + * for this device in DT.
>>> + */
>>> +bool of_dma_is_coherent(struct device_node *np)
>>> +{
>>> +       struct device_node *node = np;
>>> +
>>> +       while (node) {
>>> +               if (of_property_read_bool(node, "dma-coherent")) {
>>> +                       of_node_put(node);
>>> +                       return true;
>>> +               }
>>> +               node = of_get_next_parent(node);
>>> +       }
>>> +       return false;
>>> +}
>>> +EXPORT_SYMBOL_GPL(of_dma_is_coherent);
>>>
>>
>> This won't work on architectures that are always coherent and
>> did not need 'dma-coherent' properties before, such as IBM
>> Power servers.
>>
>> That said, I think the property makes sense, and we already have
>> platforms using it (highbank is the one I'm aware of).
>>
>> We probably need ways to override this function in both ways:
>> "always coherent" (powerpc, x86), and "never coherent" (keystone
>> without LPAE) from platform code, and it would be nice to put
>> either option into DT in a global location as well. We may have
>> to go through a few iterations of this patch to get the best
>> algorithm, but I think the interface is good at least.
> 
> I know Will D was not a fan of this property. Primarily I believe
> because you may need to describe more than just a boolean in more
> complex bus topologies.
> 
> Effectively, highbank is always coherent. It was only PCI that is
> non-coherent, but I can safely say PCI will never be enabled at this
> point. There are no designs with PCI beyond 1 or 2 validation boards
> (total boards, not designs), and getting PCI to work was quite hacky
> due to only a 1MB window. The other masters are programmable, but only
> the coherent path is used as the non-coherent path actually has some
> issues. I had expected the opposite believing the ACP port would
> actually have issues which is also why I made it configurable.
> 
I also like it to be configurable per device since you can handle
the broken masters if any. On Keystone we have one of the master
broken(non-PCI) and this helps to take care of that case.

Regards,
Santosh


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 4/7] dma: of: introduce of_dma_is_coherent() helper

[Arnd Bergmann]

On Friday 28 February 2014 09:14:19 Rob Herring wrote:
> 
> I know Will D was not a fan of this property. Primarily I believe
> because you may need to describe more than just a boolean in more
> complex bus topologies.

I can't think of any example where it's not per-device. Do you
think we can end up with a device that has multiple bus master
ports, only some of which are coherent, or is there a different
concern?

> Effectively, highbank is always coherent. It was only PCI that is
> non-coherent, but I can safely say PCI will never be enabled at this
> point. There are no designs with PCI beyond 1 or 2 validation boards
> (total boards, not designs), and getting PCI to work was quite hacky
> due to only a 1MB window. The other masters are programmable, but only
> the coherent path is used as the non-coherent path actually has some
> issues. I had expected the opposite believing the ACP port would
> actually have issues which is also why I made it configurable.

Ok, I see. I still expect that we will see systems that are only
partially coherent in the future, but it's good to know we don't really
have to deal with backwards-compatibility as long as we can just
hardcode highbank to be always coherent.

I'm especially thankful we don't have to deal with the PCI implementation.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 4/7] dma: of: introduce of_dma_is_coherent() helper

[Rob Herring]

Adding Will...

On Fri, Feb 28, 2014 at 9:24 AM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> On Friday 28 February 2014 09:14:19 Rob Herring wrote:
>>
>> I know Will D was not a fan of this property. Primarily I believe
>> because you may need to describe more than just a boolean in more
>> complex bus topologies.
>
> I can't think of any example where it's not per-device. Do you
> think we can end up with a device that has multiple bus master
> ports, only some of which are coherent, or is there a different
> concern?

Perhaps Will can post his slides from the ARM kernel summit or chime
in here, but I believe it was more that coherency is just one aspect
of bus master bus topology. The DT models the slave bus hierarchy and
doesn't model the master side which is becoming more complex and
needing to be described. In the case of highbank, you have an ACP bus
with some number of masters on it. They are configurable, but it is
not the device that is configurable, but really which bus they are
connected to.

>
>> Effectively, highbank is always coherent. It was only PCI that is
>> non-coherent, but I can safely say PCI will never be enabled at this
>> point. There are no designs with PCI beyond 1 or 2 validation boards
>> (total boards, not designs), and getting PCI to work was quite hacky
>> due to only a 1MB window. The other masters are programmable, but only
>> the coherent path is used as the non-coherent path actually has some
>> issues. I had expected the opposite believing the ACP port would
>> actually have issues which is also why I made it configurable.
>
> Ok, I see. I still expect that we will see systems that are only
> partially coherent in the future, but it's good to know we don't really
> have to deal with backwards-compatibility as long as we can just
> hardcode highbank to be always coherent.
>
> I'm especially thankful we don't have to deal with the PCI implementation.

Yeah, you and me both.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 4/7] dma: of: introduce of_dma_is_coherent() helper

[Will Deacon]

On Mon, Mar 03, 2014 at 02:04:17PM +0000, Rob Herring wrote:
> Adding Will...

Thanks Rob!

> On Fri, Feb 28, 2014 at 9:24 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Friday 28 February 2014 09:14:19 Rob Herring wrote:
> >>
> >> I know Will D was not a fan of this property. Primarily I believe
> >> because you may need to describe more than just a boolean in more
> >> complex bus topologies.
> >
> > I can't think of any example where it's not per-device. Do you
> > think we can end up with a device that has multiple bus master
> > ports, only some of which are coherent, or is there a different
> > concern?
> 
> Perhaps Will can post his slides from the ARM kernel summit or chime
> in here, but I believe it was more that coherency is just one aspect
> of bus master bus topology. The DT models the slave bus hierarchy and
> doesn't model the master side which is becoming more complex and
> needing to be described. In the case of highbank, you have an ACP bus
> with some number of masters on it. They are configurable, but it is
> not the device that is configurable, but really which bus they are
> connected to.

I'm not sure what happened to this slides but, yes, you have the right idea.
It's not only DMA coherency that's directly related to master topology, but
also SMMU configuration and MSI routing will depend on this information.

Using a per-device, boolean property to describe DMA coherency may work for a
simple, statically configured system, but it really just avoids the bigger
issue.

Will

[PATCH v2 5/7] ARM: of: introduce common routine for DMA configuration

[Santosh Shilimkar]

From: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>

This patch introduces ARM specific function arm_dt_dma_configure()
which intended to retrieve DMA configuration from DT and setup Platform
device's DMA parameters.

The DMA configuration in DT has to be specified using "dma-ranges"
and "dam-coherent" properties if supported. The DMA configuration applied
by arm_dt_dma_configure() as following:
 - call of_get_dma_range() and get supported DMA range info
   (dma_addr, cpu_addr, dma_size);
 - if "not found" then fill dma_mask as DMA_BIT_MASK(32)
   (this is default behaviour);
 - if "failed" then clean up dma_mask (DMA not supported)
 - if ok then update devices DMA configuration:
      set dma_mask to (dma_addr + dma_size - 1)
      set dma_pfn_offset to PFN_DOWN(cpu_addr - dma_addr)

Cc: Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Cc: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>
Cc: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>
---
 arch/arm/include/asm/prom.h |    3 +++
 arch/arm/kernel/devtree.c   |   61 +++++++++++++++++++++++++++++++++++++++++++
 drivers/of/platform.c       |    5 +++-
 include/linux/of.h          |    2 +-
 4 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/prom.h b/arch/arm/include/asm/prom.h
index b681575..1acb732 100644
--- a/arch/arm/include/asm/prom.h
+++ b/arch/arm/include/asm/prom.h
@@ -11,11 +11,13 @@
 #ifndef __ASMARM_PROM_H
 #define __ASMARM_PROM_H
 
+struct device;
 #ifdef CONFIG_OF
 
 extern const struct machine_desc *setup_machine_fdt(unsigned int dt_phys);
 extern void arm_dt_memblock_reserve(void);
 extern void __init arm_dt_init_cpu_maps(void);
+extern void arm_dt_dma_configure(struct device *dev);
 
 #else /* CONFIG_OF */
 
@@ -26,6 +28,7 @@ static inline const struct machine_desc *setup_machine_fdt(unsigned int dt_phys)
 
 static inline void arm_dt_memblock_reserve(void) { }
 static inline void arm_dt_init_cpu_maps(void) { }
+static inline void arm_dt_dma_configure(struct device *dev) { }
 
 #endif /* CONFIG_OF */
 #endif /* ASMARM_PROM_H */
diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
index f751714..926b5dd 100644
--- a/arch/arm/kernel/devtree.c
+++ b/arch/arm/kernel/devtree.c
@@ -18,6 +18,9 @@
 #include <linux/of_fdt.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
+#include <linux/of_dma.h>
+#include <linux/dma-mapping.h>
+#include <linux/slab.h>
 
 #include <asm/cputype.h>
 #include <asm/setup.h>
@@ -235,3 +238,61 @@ const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
 
 	return mdesc;
 }
+
+void arm_dt_dma_configure(struct device *dev)
+{
+	dma_addr_t dma_addr;
+	phys_addr_t paddr, size;
+	dma_addr_t dma_mask;
+	int ret;
+
+	/*
+	 * if dma-ranges property doesn't exist - use 32 bits DMA mask
+	 * by default and don't set skip archdata.dma_pfn_offset
+	 */
+	ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size);
+	if (ret == -ENODEV) {
+		dev->coherent_dma_mask = DMA_BIT_MASK(32);
+		if (!dev->dma_mask)
+			dev->dma_mask = &dev->coherent_dma_mask;
+		return;
+	}
+
+	/* if failed - disable DMA for device */
+	if (ret < 0) {
+		dev_err(dev, "failed to configure DMA\n");
+		return;
+	}
+
+	/* DMA ranges found. Calculate and set dma_pfn_offset */
+	dev->archdata.dma_pfn_offset = PFN_DOWN(paddr - dma_addr);
+
+	/* Configure DMA mask */
+	dev->dma_mask = kmalloc(sizeof(*dev->dma_mask), GFP_KERNEL);
+	if (!dev->dma_mask)
+		return;
+
+	dma_mask = dma_addr + size - 1;
+
+	ret = arm_dma_set_mask(dev, dma_mask);
+	if (ret < 0) {
+		dev_err(dev, "failed to set DMA mask %#08x\n", dma_mask);
+		kfree(dev->dma_mask);
+		dev->dma_mask = NULL;
+		return;
+	}
+
+	dev_dbg(dev, "dma_pfn_offset(%#08lx) dma_mask(%#016llx)\n",
+		dev->archdata.dma_pfn_offset, *dev->dma_mask);
+
+	if (of_dma_is_coherent(dev->of_node)) {
+		set_dma_ops(dev, &arm_coherent_dma_ops);
+		dev_dbg(dev, "device is dma coherent\n");
+	}
+
+	ret = dma_set_coherent_mask(dev, dma_mask);
+	if (ret < 0) {
+		dev_err(dev, "failed to set coherent DMA mask %#08x\n",
+			dma_mask);
+	}
+}
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 404d1da..97d5533 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -213,10 +213,13 @@ static struct platform_device *of_platform_device_create_pdata(
 
 #if defined(CONFIG_MICROBLAZE)
 	dev->archdata.dma_mask = 0xffffffffUL;
-#endif
+#elif defined(CONFIG_ARM_LPAE)
+	arm_dt_dma_configure(&dev->dev);
+#else
 	dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
 	if (!dev->dev.dma_mask)
 		dev->dev.dma_mask = &dev->dev.coherent_dma_mask;
+#endif
 	dev->dev.bus = &platform_bus_type;
 	dev->dev.platform_data = platform_data;
 
diff --git a/include/linux/of.h b/include/linux/of.h
index 70c64ba..a321058 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -136,7 +136,7 @@ static inline unsigned long of_read_ulong(const __be32 *cell, int size)
 	return of_read_number(cell, size);
 }
 
-#if defined(CONFIG_SPARC)
+#if defined(CONFIG_SPARC) || defined(CONFIG_ARM_LPAE)
 #include <asm/prom.h>
 #endif
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 5/7] ARM: of: introduce common routine for DMA configuration

[Arnd Bergmann]

On Thursday 27 February 2014 16:17:50 Santosh Shilimkar wrote:
> diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
> index f751714..926b5dd 100644
> --- a/arch/arm/kernel/devtree.c
> +++ b/arch/arm/kernel/devtree.c
> @@ -235,3 +238,61 @@ const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
>  
>  	return mdesc;
>  }
> +
> +void arm_dt_dma_configure(struct device *dev)
> +{
> +	dma_addr_t dma_addr;
> +	phys_addr_t paddr, size;
> +	dma_addr_t dma_mask;
> +	int ret;
> +
> +	/*
> +	 * if dma-ranges property doesn't exist - use 32 bits DMA mask
> +	 * by default and don't set skip archdata.dma_pfn_offset
> +	 */
> +	ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size);
> +	if (ret == -ENODEV) {
> +		dev->coherent_dma_mask = DMA_BIT_MASK(32);
> +		if (!dev->dma_mask)
> +			dev->dma_mask = &dev->coherent_dma_mask;
> +		return;
> +	}

I think this is a reasonable default, but I also want Russell's
opinion on this, since I suspect he will argue that we shouldn't
default to setting a DMA mask for devices that are not DMA capable.

Maybe someone has an idea how we can detect all three important cases:

a) A device is marked as DMA capable using a dma-ranges property
b) A device is known not to be DMA capable
c) we don't have any dma-ranges properties in an old dtb file
   but still want 32 bit masks by default.

> +	/* if failed - disable DMA for device */
> +	if (ret < 0) {
> +		dev_err(dev, "failed to configure DMA\n");
> +		return;
> +	}

I guess this is also where other platforms (shmobile, highbank, ...)
will want the IOMMU detection to happen.

> +	/* DMA ranges found. Calculate and set dma_pfn_offset */
> +	dev->archdata.dma_pfn_offset = PFN_DOWN(paddr - dma_addr);
> +
> +	/* Configure DMA mask */
> +	dev->dma_mask = kmalloc(sizeof(*dev->dma_mask), GFP_KERNEL);
> +	if (!dev->dma_mask)
> +		return;

Do we have to worry about freeing this? We could in theory put the
mask into pdev_archdata (as on microblaze), or point to
coherent_dma_mask (as of_platform_device_create_pdata does).
I can't think of a case where the latter won't actually work,
since coherent_dma_mask!=*dma_mask doesn't happen on any platform
device I have ever seen. coherent_dma_mask was introduced to handle
some special requirements of PCI devices on ia64 or parisc.

> +       dma_mask = dma_addr + size - 1;

I can never remember if this is actually correct, or if it would have to
be 

	dma_mask = size - 1;

instead. Russell knows.

> +	ret = arm_dma_set_mask(dev, dma_mask);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to set DMA mask %#08x\n", dma_mask);
> +		kfree(dev->dma_mask);
> +		dev->dma_mask = NULL;
> +		return;
> +	}

Again I'm hoping for Russell to provide the correct answer: Should we
set the correct mask initially for the device here, or should we
rely on dma_set_mask() to refuse a mask that is larger than we
can handle?

For PCI devices, we normally assume that we can always set a 32-bit
DMA mask, but drivers can set a smaller mask if the device can
support a smaller space than the bus can. In this case, the mask
is already the intersection of what the device and all the parent
buses support, and I'm not sure how the API describe in
Documentation/DMA-API-HOWTO.txt would deal with this.

> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 404d1da..97d5533 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -213,10 +213,13 @@ static struct platform_device *of_platform_device_create_pdata(
>  
>  #if defined(CONFIG_MICROBLAZE)
>  	dev->archdata.dma_mask = 0xffffffffUL;
> -#endif
> +#elif defined(CONFIG_ARM_LPAE)
> +	arm_dt_dma_configure(&dev->dev);
> +#else
>  	dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
>  	if (!dev->dev.dma_mask)
>  		dev->dev.dma_mask = &dev->dev.coherent_dma_mask;
> +#endif

The dependency on CONFIG_ARM_LPAE is not correct the general case,
that would be a special case on keystone. I'd suggest using
CONFIG_ARM here, and finding a different way to return false
for dma_is_coherent() on keystone with LPAE disabled.

	Arnd

Re: [PATCH v2 5/7] ARM: of: introduce common routine for DMA configuration

[Grygorii Strashko]

Hi Arnd,

On 02/28/2014 12:00 PM, Arnd Bergmann wrote:
> On Thursday 27 February 2014 16:17:50 Santosh Shilimkar wrote:
>> diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
>> index f751714..926b5dd 100644
>> --- a/arch/arm/kernel/devtree.c
>> +++ b/arch/arm/kernel/devtree.c
>> @@ -235,3 +238,61 @@ const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
>>   
>>   	return mdesc;
>>   }
>> +
>> +void arm_dt_dma_configure(struct device *dev)
>> +{
>> +	dma_addr_t dma_addr;
>> +	phys_addr_t paddr, size;
>> +	dma_addr_t dma_mask;
>> +	int ret;
>> +
>> +	/*
>> +	 * if dma-ranges property doesn't exist - use 32 bits DMA mask
>> +	 * by default and don't set skip archdata.dma_pfn_offset
>> +	 */
>> +	ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size);
>> +	if (ret == -ENODEV) {
>> +		dev->coherent_dma_mask = DMA_BIT_MASK(32);
>> +		if (!dev->dma_mask)
>> +			dev->dma_mask = &dev->coherent_dma_mask;
>> +		return;
>> +	}
> 
> I think this is a reasonable default, but I also want Russell's
> opinion on this, since I suspect he will argue that we shouldn't
> default to setting a DMA mask for devices that are not DMA capable.

Just to note, that's current default behavior used in of_platform_device_create_pdata()

> 
> Maybe someone has an idea how we can detect all three important cases:
> 
> a) A device is marked as DMA capable using a dma-ranges property
> b) A device is known not to be DMA capable
> c) we don't have any dma-ranges properties in an old dtb file
>     but still want 32 bit masks by default.

Yep, This patch set supports [a, c]. But, case be [b] can be patched 
by arch/mach code using Platform Bus notifier if needed.
(Platform Bus notifiers will be called after arm_dt_dma_configure is 
finished).

> 
>> +	/* if failed - disable DMA for device */
>> +	if (ret < 0) {
>> +		dev_err(dev, "failed to configure DMA\n");
>> +		return;
>> +	}
> 
> I guess this is also where other platforms (shmobile, highbank, ...)
> will want the IOMMU detection to happen.

This error path handling - means, DT contains wrong data :)

> 
>> +	/* DMA ranges found. Calculate and set dma_pfn_offset */
>> +	dev->archdata.dma_pfn_offset = PFN_DOWN(paddr - dma_addr);
>> +
>> +	/* Configure DMA mask */
>> +	dev->dma_mask = kmalloc(sizeof(*dev->dma_mask), GFP_KERNEL);
>> +	if (!dev->dma_mask)
>> +		return;
> 
> Do we have to worry about freeing this? We could in theory put the
> mask into pdev_archdata (as on microblaze), or point to
> coherent_dma_mask (as of_platform_device_create_pdata does).
> I can't think of a case where the latter won't actually work,
> since coherent_dma_mask!=*dma_mask doesn't happen on any platform
> device I have ever seen. coherent_dma_mask was introduced to handle
> some special requirements of PCI devices on ia64 or parisc.

I've used platform_device_register_full() as ref here. It actually contains
good comment regarding this mem leak issue:
/*
 * This memory isn't freed when the device is put,
 * I don't have a nice idea for that though.  Conceptually
 * dma_mask in struct device should not be a pointer.
 * See http://thread.gmane.org/gmane.linux.kernel.pci/9081
 */


> 
>> +       dma_mask = dma_addr + size - 1;
> 
> I can never remember if this is actually correct, or if it would have to
> be
> 
> 	dma_mask = size - 1;
> 
> instead. Russell knows.
> 
>> +	ret = arm_dma_set_mask(dev, dma_mask);
>> +	if (ret < 0) {
>> +		dev_err(dev, "failed to set DMA mask %#08x\n", dma_mask);
>> +		kfree(dev->dma_mask);
>> +		dev->dma_mask = NULL;
>> +		return;
>> +	}
> 
> Again I'm hoping for Russell to provide the correct answer: Should we
> set the correct mask initially for the device here, or should we
> rely on dma_set_mask() to refuse a mask that is larger than we
> can handle?
> 
> For PCI devices, we normally assume that we can always set a 32-bit
> DMA mask, but drivers can set a smaller mask if the device can
> support a smaller space than the bus can. In this case, the mask
> is already the intersection of what the device and all the parent
> buses support, and I'm not sure how the API describe in
> Documentation/DMA-API-HOWTO.txt would deal with this.

As mentioned by Santosh in cover letter,
PCI (and other buses) is problem here as they may have different "dma-ranges"
prop format (PCI #address-cells = <3>) and need to handled in different way. 

May be, this code can be limited to platform_bus_type devices only somehow.

> 
>> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
>> index 404d1da..97d5533 100644
>> --- a/drivers/of/platform.c
>> +++ b/drivers/of/platform.c
>> @@ -213,10 +213,13 @@ static struct platform_device *of_platform_device_create_pdata(
>>   
>>   #if defined(CONFIG_MICROBLAZE)
>>   	dev->archdata.dma_mask = 0xffffffffUL;
>> -#endif
>> +#elif defined(CONFIG_ARM_LPAE)

ops, should be:
#endif
+#if defined(CONFIG_ARM_LPAE)

>> +	arm_dt_dma_configure(&dev->dev);
>> +#else
>>   	dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
>>   	if (!dev->dev.dma_mask)
>>   		dev->dev.dma_mask = &dev->dev.coherent_dma_mask;
>> +#endif
> 
> The dependency on CONFIG_ARM_LPAE is not correct the general case,
> that would be a special case on keystone. I'd suggest using
> CONFIG_ARM here, and finding a different way to return false
> for dma_is_coherent() on keystone with LPAE disabled.
> 

Thanks, for your comments.

Regards,
-grygorii

Re: [PATCH v2 5/7] ARM: of: introduce common routine for DMA configuration

[Arnd Bergmann]

On Friday 28 February 2014 13:49:34 Grygorii Strashko wrote:
> On 02/28/2014 12:00 PM, Arnd Bergmann wrote:
> > On Thursday 27 February 2014 16:17:50 Santosh Shilimkar wrote:
> >> diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
> >> +
> >> +void arm_dt_dma_configure(struct device *dev)
> >> +{
> >> +	dma_addr_t dma_addr;
> >> +	phys_addr_t paddr, size;
> >> +	dma_addr_t dma_mask;
> >> +	int ret;
> >> +
> >> +	/*
> >> +	 * if dma-ranges property doesn't exist - use 32 bits DMA mask
> >> +	 * by default and don't set skip archdata.dma_pfn_offset
> >> +	 */
> >> +	ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size);
> >> +	if (ret == -ENODEV) {
> >> +		dev->coherent_dma_mask = DMA_BIT_MASK(32);
> >> +		if (!dev->dma_mask)
> >> +			dev->dma_mask = &dev->coherent_dma_mask;
> >> +		return;
> >> +	}
> > 
> > I think this is a reasonable default, but I also want Russell's
> > opinion on this, since I suspect he will argue that we shouldn't
> > default to setting a DMA mask for devices that are not DMA capable.
> 
> Just to note, that's current default behavior used in of_platform_device_create_pdata()

Right, I realized that later.

> > Maybe someone has an idea how we can detect all three important cases:
> > 
> > a) A device is marked as DMA capable using a dma-ranges property
> > b) A device is known not to be DMA capable
> > c) we don't have any dma-ranges properties in an old dtb file
> >     but still want 32 bit masks by default.
> 
> Yep, This patch set supports [a, c]. But, case be [b] can be patched 
> by arch/mach code using Platform Bus notifier if needed.
> (Platform Bus notifiers will be called after arm_dt_dma_configure is 
> finished).

It would be nice to have a way to do it without a platform specific
notifier, I just haven't found a nice way to express that in DT.

> >> +	/* if failed - disable DMA for device */
> >> +	if (ret < 0) {
> >> +		dev_err(dev, "failed to configure DMA\n");
> >> +		return;
> >> +	}
> > 
> > I guess this is also where other platforms (shmobile, highbank, ...)
> > will want the IOMMU detection to happen.
> 
> This error path handling - means, DT contains wrong data :)

I wasn't referring to the error path here, sorry for being
ambiguous. What I meant that we could add code after this line
to look for an IOMMU.

> >> +	/* DMA ranges found. Calculate and set dma_pfn_offset */
> >> +	dev->archdata.dma_pfn_offset = PFN_DOWN(paddr - dma_addr);
> >> +
> >> +	/* Configure DMA mask */
> >> +	dev->dma_mask = kmalloc(sizeof(*dev->dma_mask), GFP_KERNEL);
> >> +	if (!dev->dma_mask)
> >> +		return;
> > 
> > Do we have to worry about freeing this? We could in theory put the
> > mask into pdev_archdata (as on microblaze), or point to
> > coherent_dma_mask (as of_platform_device_create_pdata does).
> > I can't think of a case where the latter won't actually work,
> > since coherent_dma_mask!=*dma_mask doesn't happen on any platform
> > device I have ever seen. coherent_dma_mask was introduced to handle
> > some special requirements of PCI devices on ia64 or parisc.
> 
> I've used platform_device_register_full() as ref here. It actually contains
> good comment regarding this mem leak issue:
> /*
>  * This memory isn't freed when the device is put,
>  * I don't have a nice idea for that though.  Conceptually
>  * dma_mask in struct device should not be a pointer.
>  * See http://thread.gmane.org/gmane.linux.kernel.pci/9081
>  */

Right. Maybe the best solution for that code path however is to
make it the same as the of_platform code where we today set the
mask pointer to &dev->coherent_mask.

> > Again I'm hoping for Russell to provide the correct answer: Should we
> > set the correct mask initially for the device here, or should we
> > rely on dma_set_mask() to refuse a mask that is larger than we
> > can handle?
> > 
> > For PCI devices, we normally assume that we can always set a 32-bit
> > DMA mask, but drivers can set a smaller mask if the device can
> > support a smaller space than the bus can. In this case, the mask
> > is already the intersection of what the device and all the parent
> > buses support, and I'm not sure how the API describe in
> > Documentation/DMA-API-HOWTO.txt would deal with this.
> 
> As mentioned by Santosh in cover letter,
> PCI (and other buses) is problem here as they may have different "dma-ranges"
> prop format (PCI #address-cells = <3>) and need to handled in different way. 
> 
> May be, this code can be limited to platform_bus_type devices only somehow.

Doesn't that already get handled correctly by of_bus_pci_translate()?
We have bus specific translation functions that should work for
both 'ranges' and 'dma-ranges'.

	Arnd

Re: [PATCH v2 5/7] ARM: of: introduce common routine for DMA configuration

[Santosh Shilimkar]

On Friday 28 February 2014 05:00 AM, Arnd Bergmann wrote:
>> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
>> > index 404d1da..97d5533 100644
>> > --- a/drivers/of/platform.c
>> > +++ b/drivers/of/platform.c
>> > @@ -213,10 +213,13 @@ static struct platform_device *of_platform_device_create_pdata(
>> >  
>> >  #if defined(CONFIG_MICROBLAZE)
>> >  	dev->archdata.dma_mask = 0xffffffffUL;
>> > -#endif
>> > +#elif defined(CONFIG_ARM_LPAE)
>> > +	arm_dt_dma_configure(&dev->dev);
>> > +#else
>> >  	dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
>> >  	if (!dev->dev.dma_mask)
>> >  		dev->dev.dma_mask = &dev->dev.coherent_dma_mask;
>> > +#endif
> The dependency on CONFIG_ARM_LPAE is not correct the general case,
> that would be a special case on keystone. I'd suggest using
> CONFIG_ARM here, and finding a different way to return false
> for dma_is_coherent() on keystone with LPAE disabled.
> 
I made that LPAE specific assuming the 32 machines anyway are
happy with default as they are today. We can keep CONFIG_ARM
and handle the special case in machine platform notifier.

Regards,
Santosh

Re: [PATCH v2 5/7] ARM: of: introduce common routine for DMA configuration

[Arnd Bergmann]

On Friday 28 February 2014 10:06:25 Santosh Shilimkar wrote:
> I made that LPAE specific assuming the 32 machines anyway are
> happy with default as they are today. We can keep CONFIG_ARM
> and handle the special case in machine platform notifier.

I think all other machines that support LPAE can also run with
LPAE disabled and still use coherent DMA, they may just not
support all the available RAM.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 5/7] ARM: of: introduce common routine for DMA configuration

[Santosh Shilimkar]

On Friday 28 February 2014 10:31 AM, Arnd Bergmann wrote:
> On Friday 28 February 2014 10:06:25 Santosh Shilimkar wrote:
>> I made that LPAE specific assuming the 32 machines anyway are
>> happy with default as they are today. We can keep CONFIG_ARM
>> and handle the special case in machine platform notifier.
> 
> I think all other machines that support LPAE can also run with
> LPAE disabled and still use coherent DMA, they may just not
> support all the available RAM.
> 
Fair enough.

Regards,
Santosh

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 5/7] ARM: of: introduce common routine for DMA configuration

[Linus Walleij]

On Fri, Feb 28, 2014 at 6:00 PM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> On Thursday 27 February 2014 16:17:50 Santosh Shilimkar wrote:
>> diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
>> index f751714..926b5dd 100644
>> --- a/arch/arm/kernel/devtree.c
>> +++ b/arch/arm/kernel/devtree.c
>> @@ -235,3 +238,61 @@ const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
>>
>>       return mdesc;
>>  }
>> +
>> +void arm_dt_dma_configure(struct device *dev)
>> +{
>> +     dma_addr_t dma_addr;
>> +     phys_addr_t paddr, size;
>> +     dma_addr_t dma_mask;
>> +     int ret;
>> +
>> +     /*
>> +      * if dma-ranges property doesn't exist - use 32 bits DMA mask
>> +      * by default and don't set skip archdata.dma_pfn_offset
>> +      */
>> +     ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size);
>> +     if (ret == -ENODEV) {
>> +             dev->coherent_dma_mask = DMA_BIT_MASK(32);
>> +             if (!dev->dma_mask)
>> +                     dev->dma_mask = &dev->coherent_dma_mask;
>> +             return;
>> +     }
>
> I think this is a reasonable default, but I also want Russell's
> opinion on this, since I suspect he will argue that we shouldn't
> default to setting a DMA mask for devices that are not DMA capable.

of_platform_device_create_pdata() in drivers/of/platform.c
does the same assumption since ages.

> Maybe someone has an idea how we can detect all three important cases:
>
> a) A device is marked as DMA capable using a dma-ranges property
> b) A device is known not to be DMA capable
> c) we don't have any dma-ranges properties in an old dtb file
>    but still want 32 bit masks by default.

The exact same solution would apply to the of core right?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 5/7] ARM: of: introduce common routine for DMA configuration

[Rob Herring]

On Thu, Feb 27, 2014 at 3:17 PM, Santosh Shilimkar
<santosh.shilimkar-l0cyMroinI0@public.gmane.org> wrote:
> From: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
>
> This patch introduces ARM specific function arm_dt_dma_configure()
> which intended to retrieve DMA configuration from DT and setup Platform
> device's DMA parameters.
>
> The DMA configuration in DT has to be specified using "dma-ranges"
> and "dam-coherent" properties if supported. The DMA configuration applied

s/dam/dma/

> by arm_dt_dma_configure() as following:
>  - call of_get_dma_range() and get supported DMA range info
>    (dma_addr, cpu_addr, dma_size);
>  - if "not found" then fill dma_mask as DMA_BIT_MASK(32)
>    (this is default behaviour);
>  - if "failed" then clean up dma_mask (DMA not supported)
>  - if ok then update devices DMA configuration:
>       set dma_mask to (dma_addr + dma_size - 1)
>       set dma_pfn_offset to PFN_DOWN(cpu_addr - dma_addr)
>
> Cc: Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
> Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> Cc: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>
> Cc: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>
> ---
>  arch/arm/include/asm/prom.h |    3 +++
>  arch/arm/kernel/devtree.c   |   61 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/of/platform.c       |    5 +++-
>  include/linux/of.h          |    2 +-
>  4 files changed, 69 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/include/asm/prom.h b/arch/arm/include/asm/prom.h
> index b681575..1acb732 100644
> --- a/arch/arm/include/asm/prom.h
> +++ b/arch/arm/include/asm/prom.h
> @@ -11,11 +11,13 @@
>  #ifndef __ASMARM_PROM_H
>  #define __ASMARM_PROM_H
>
> +struct device;
>  #ifdef CONFIG_OF
>
>  extern const struct machine_desc *setup_machine_fdt(unsigned int dt_phys);
>  extern void arm_dt_memblock_reserve(void);
>  extern void __init arm_dt_init_cpu_maps(void);
> +extern void arm_dt_dma_configure(struct device *dev);
>
>  #else /* CONFIG_OF */
>
> @@ -26,6 +28,7 @@ static inline const struct machine_desc *setup_machine_fdt(unsigned int dt_phys)
>
>  static inline void arm_dt_memblock_reserve(void) { }
>  static inline void arm_dt_init_cpu_maps(void) { }
> +static inline void arm_dt_dma_configure(struct device *dev) { }
>
>  #endif /* CONFIG_OF */
>  #endif /* ASMARM_PROM_H */
> diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
> index f751714..926b5dd 100644
> --- a/arch/arm/kernel/devtree.c
> +++ b/arch/arm/kernel/devtree.c
> @@ -18,6 +18,9 @@
>  #include <linux/of_fdt.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_platform.h>
> +#include <linux/of_dma.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/slab.h>
>
>  #include <asm/cputype.h>
>  #include <asm/setup.h>
> @@ -235,3 +238,61 @@ const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
>
>         return mdesc;
>  }
> +
> +void arm_dt_dma_configure(struct device *dev)

The implementation may be ARM specific, but the need for the function
should not be.

> +{
> +       dma_addr_t dma_addr;
> +       phys_addr_t paddr, size;
> +       dma_addr_t dma_mask;
> +       int ret;
> +
> +       /*
> +        * if dma-ranges property doesn't exist - use 32 bits DMA mask
> +        * by default and don't set skip archdata.dma_pfn_offset
> +        */
> +       ret = of_dma_get_range(dev->of_node, &dma_addr, &paddr, &size);
> +       if (ret == -ENODEV) {
> +               dev->coherent_dma_mask = DMA_BIT_MASK(32);
> +               if (!dev->dma_mask)
> +                       dev->dma_mask = &dev->coherent_dma_mask;
> +               return;
> +       }
> +
> +       /* if failed - disable DMA for device */
> +       if (ret < 0) {
> +               dev_err(dev, "failed to configure DMA\n");
> +               return;
> +       }
> +
> +       /* DMA ranges found. Calculate and set dma_pfn_offset */
> +       dev->archdata.dma_pfn_offset = PFN_DOWN(paddr - dma_addr);
> +
> +       /* Configure DMA mask */
> +       dev->dma_mask = kmalloc(sizeof(*dev->dma_mask), GFP_KERNEL);

I don't believe that any ARM platform needs dma_mask to be different
from coherent_dma_mask. I traced the history to when 2 masks were
defined and this was my conclusion. Russell has also recently stated
the same:

https://lkml.org/lkml/2013/8/9/205

> +       if (!dev->dma_mask)
> +               return;
> +
> +       dma_mask = dma_addr + size - 1;
> +
> +       ret = arm_dma_set_mask(dev, dma_mask);
> +       if (ret < 0) {
> +               dev_err(dev, "failed to set DMA mask %#08x\n", dma_mask);
> +               kfree(dev->dma_mask);
> +               dev->dma_mask = NULL;
> +               return;
> +       }
> +
> +       dev_dbg(dev, "dma_pfn_offset(%#08lx) dma_mask(%#016llx)\n",
> +               dev->archdata.dma_pfn_offset, *dev->dma_mask);
> +
> +       if (of_dma_is_coherent(dev->of_node)) {
> +               set_dma_ops(dev, &arm_coherent_dma_ops);

All but this line could be in a common function. So make an arm
version of the function that calls the common function and then does
this. Or perhaps a per arch "get dma ops" function is all that is
needed.

> +               dev_dbg(dev, "device is dma coherent\n");
> +       }
> +
> +       ret = dma_set_coherent_mask(dev, dma_mask);
> +       if (ret < 0) {
> +               dev_err(dev, "failed to set coherent DMA mask %#08x\n",
> +                       dma_mask);
> +       }
> +}
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 404d1da..97d5533 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -213,10 +213,13 @@ static struct platform_device *of_platform_device_create_pdata(
>
>  #if defined(CONFIG_MICROBLAZE)
>         dev->archdata.dma_mask = 0xffffffffUL;

This should be moved into a microblaze specific version of the function.

> -#endif
> +#elif defined(CONFIG_ARM_LPAE)
> +       arm_dt_dma_configure(&dev->dev);
> +#else
>         dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
>         if (!dev->dev.dma_mask)
>                 dev->dev.dma_mask = &dev->dev.coherent_dma_mask;
> +#endif
>         dev->dev.bus = &platform_bus_type;
>         dev->dev.platform_data = platform_data;
>
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 70c64ba..a321058 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -136,7 +136,7 @@ static inline unsigned long of_read_ulong(const __be32 *cell, int size)
>         return of_read_number(cell, size);
>  }
>
> -#if defined(CONFIG_SPARC)
> +#if defined(CONFIG_SPARC) || defined(CONFIG_ARM_LPAE)
>  #include <asm/prom.h>

No. The idea here is to get rid of including prom.h.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 6/7] ARM: dts: keystone: Use dma-ranges property

[Santosh Shilimkar]

From: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>

The dma-ranges property has to be specified per bus and has format:
 < DMA addr > - Base DMA address for Bus (Bus format 32-bits)
 < CPU addr > - Corresponding base CPU address (CPU format 64-bits)
 < DMA range size > - Size of supported DMA range

Cc: Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Cc: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>
Cc: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>
---
 arch/arm/boot/dts/keystone.dtsi |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/keystone.dtsi b/arch/arm/boot/dts/keystone.dtsi
index b420290..171579d 100644
--- a/arch/arm/boot/dts/keystone.dtsi
+++ b/arch/arm/boot/dts/keystone.dtsi
@@ -96,6 +96,7 @@
 		compatible = "ti,keystone","simple-bus";
 		interrupt-parent = <&gic>;
 		ranges = <0x0 0x0 0x0 0xc0000000>;
+		dma-ranges = <0x80000000 0x8 0x00000000 0x80000000>;
 
 		rstctrl: reset-controller {
 			compatible = "ti,keystone-reset";
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 7/7] ARM: dts: keystone: Udate USB node for dma properties

[Santosh Shilimkar]

Keystone supports dma-coherent on USB master and also needs
dma-ranges to specify the hardware alias memory range in which DMA
can be operational.

Cc: Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Cc: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>
Cc: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>
---
 arch/arm/boot/dts/keystone.dtsi |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/keystone.dtsi b/arch/arm/boot/dts/keystone.dtsi
index 171579d..de155c0 100644
--- a/arch/arm/boot/dts/keystone.dtsi
+++ b/arch/arm/boot/dts/keystone.dtsi
@@ -200,6 +200,8 @@
 			clock-names = "usb";
 			interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
 			ranges;
+			dma-coherent;
+			dma-ranges;
 			status = "disabled";
 
 			dwc3@2690000 {
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html