Re: [PATCH 06/22] arc: use generic dma_noncoherent_ops

2018-04-26 Thread h...@lst.de 
On Thu, Apr 26, 2018 at 08:45:00AM +0200, h...@lst.de wrote:
> On Wed, Apr 25, 2018 at 11:17:01AM +, Alexey Brodkin wrote:
> > Which is actually strange as I would expect ARC code to be built by bots.
> 
> I don't think I got any notification.  Thank for the fixes!
> 
> I think I found the bug, based on the fact that so far all tests for
> architectures that also need a cache op for device to cpu transitions
> failed.  I did a stupid typo when changing kconfig symbols, so please
> try the patch below.

Confirmed to work for nds32, so here is a git tree with the core, arc
and nds32 fixes folded in, feel free to test that one:

git://git.infradead.org/users/hch/misc.git generic-dma-noncoherent
--
To unsubscribe from this list: send the line "unsubscribe linux-alpha" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 06/22] arc: use generic dma_noncoherent_ops

2018-04-26 Thread h...@lst.de 
On Wed, Apr 25, 2018 at 11:17:01AM +, Alexey Brodkin wrote:
> Which is actually strange as I would expect ARC code to be built by bots.

I don't think I got any notification.  Thank for the fixes!

I think I found the bug, based on the fact that so far all tests for
architectures that also need a cache op for device to cpu transitions
failed.  I did a stupid typo when changing kconfig symbols, so please
try the patch below.

>  
>  static int l2_line_sz;
>  static int ioc_exists;
> -int slc_enable = 1, ioc_enable = 1;
> +int slc_enable = 1, ioc_enable = 0;

Hmm.  It seems if ioc_enable is 0 we should simply be using
dma_direct_ops on arc, but that is a different discussion.

---
diff --git a/lib/dma-noncoherent.c b/lib/dma-noncoherent.c
index f4b8532c20ac..a2c192b3508d 100644
--- a/lib/dma-noncoherent.c
+++ b/lib/dma-noncoherent.c
@@ -48,7 +48,7 @@ static int dma_noncoherent_map_sg(struct device *dev, struct 
scatterlist *sgl,
return nents;
 }
 
-#ifdef CONFIG_DMA_NONCOHERENT_SYNC_FOR_CPU
+#ifdef CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU
 static void dma_noncoherent_sync_single_for_cpu(struct device *dev,
dma_addr_t addr, size_t size, enum dma_data_direction dir)
 {
@@ -88,7 +88,7 @@ const struct dma_map_ops dma_noncoherent_ops = {
.sync_sg_for_device = dma_noncoherent_sync_sg_for_device,
.map_page   = dma_noncoherent_map_page,
.map_sg = dma_noncoherent_map_sg,
-#ifdef CONFIG_DMA_NONCOHERENT_SYNC_FOR_CPU
+#ifdef CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU
.sync_single_for_cpu= dma_noncoherent_sync_single_for_cpu,
.sync_sg_for_cpu= dma_noncoherent_sync_sg_for_cpu,
.unmap_page = dma_noncoherent_unmap_page,
--
To unsubscribe from this list: send the line "unsubscribe linux-alpha" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 06/22] arc: use generic dma_noncoherent_ops

2018-04-25 Thread Alexey Brodkin 
Hi Christoph,

On Fri, 2018-04-20 at 10:02 +0200, Christoph Hellwig wrote:
> Switch to the generic noncoherent direct mapping implementation.
> 
> Signed-off-by: Christoph Hellwig 
> ---
>  arch/arc/Kconfig   |   4 +
>  arch/arc/include/asm/Kbuild|   1 +
>  arch/arc/include/asm/dma-mapping.h |  21 -
>  arch/arc/mm/dma.c  | 141 +++--
>  4 files changed, 19 insertions(+), 148 deletions(-)
>  delete mode 100644 arch/arc/include/asm/dma-mapping.h
> 

[snip]

> @@ -135,7 +134,7 @@ static int arc_dma_mmap(struct device *dev, struct 
> vm_area_struct *vma,
>   * CPU accesses page via normal paddr, thus needs to explicitly made
>   * consistent before each use
>   */
> -static void _dma_cache_sync(phys_addr_t paddr, size_t size,
> +static void _dma_cache_sync(struct device *dev, phys_addr_t paddr size_t 
> size,
>   enum dma_data_direction dir)

Seems like there's a missing comma:
--->8
--- a/arch/arc/mm/dma.c
+++ b/arch/arc/mm/dma.c
@@ -134,7 +134,7 @@ int arch_dma_mmap(struct device *dev, struct vm_area_struct 
*vma,
  * CPU accesses page via normal paddr, thus needs to explicitly made
  * consistent before each use
  */
-static void _dma_cache_sync(struct device *dev, phys_addr_t paddr size_t size,
+static void _dma_cache_sync(struct device *dev, phys_addr_t paddr, size_t size,
enum dma_data_direction dir)
 {
switch (dir) {
--->8

Which is actually strange as I would expect ARC code to be built by bots.

Anyways with above fix I do see problems with both USB and Ethernet controllers
on ARC HSDK board.
--->8
usb 1-1: new high-speed USB device number 2 using ehci-platform
usb 1-1: device descriptor read/64, error -32
usb 1-1: device descriptor read/64, error -32
usb 1-1: new high-speed USB device number 3 using ehci-platform
usb 1-1: device descriptor read/64, error -32
usb 1-1: device descriptor read/64, error -32
usb usb1-port1: attempt power cycle
usb 1-1: new high-speed USB device number 4 using ehci-platform
usb 1-1: device not accepting address 4, error -32
usb 1-1: new high-speed USB device number 5 using ehci-platform
usb 1-1: device not accepting address 5, error -32

...

# wget ftp://ftp.denx.de/pub/u-boot/u-boot-1.0.0.tar.bz2
Connecting to ftp.denx.de (81.169.202.6:21)
wget: can't connect to remote host (81.169.202.6): No route to host
--->8

Will all patches from the series reverted (i.e. with
your base-line) all issues go away.

I'll need to spend more time on checking what's actually wrong.

-Alexey

P.S. Note to my ARC colleagues - it's required to disable IO Coherency
to get DMA ops really used, it could be obviously done with:
--->8
--- a/arch/arc/mm/cache.c
+++ b/arch/arc/mm/cache.c
@@ -27,7 +27,7 @@
 
 static int l2_line_sz;
 static int ioc_exists;
-int slc_enable = 1, ioc_enable = 1;
+int slc_enable = 1, ioc_enable = 0;
 unsigned long perip_base = ARC_UNCACHED_ADDR_SPACE; /* legacy value for boot */
 unsigned long perip_end = 0x; /* legacy value */
--->8

[PATCH 06/22] arc: use generic dma_noncoherent_ops

2018-04-20 Thread Christoph Hellwig 
Switch to the generic noncoherent direct mapping implementation.

Signed-off-by: Christoph Hellwig 
---
 arch/arc/Kconfig   |   4 +
 arch/arc/include/asm/Kbuild|   1 +
 arch/arc/include/asm/dma-mapping.h |  21 -
 arch/arc/mm/dma.c  | 141 +++--
 4 files changed, 19 insertions(+), 148 deletions(-)
 delete mode 100644 arch/arc/include/asm/dma-mapping.h

diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
index 7498aca4b887..89d47eac18b2 100644
--- a/arch/arc/Kconfig
+++ b/arch/arc/Kconfig
@@ -9,11 +9,15 @@
 config ARC
def_bool y
select ARC_TIMERS
+   select ARCH_HAS_SYNC_DMA_FOR_CPU
+   select ARCH_HAS_SYNC_DMA_FOR_DEVICE
select ARCH_HAS_SG_CHAIN
select ARCH_SUPPORTS_ATOMIC_RMW if ARC_HAS_LLSC
select BUILDTIME_EXTABLE_SORT
select CLONE_BACKWARDS
select COMMON_CLK
+   select DMA_NONCOHERENT_OPS
+   select DMA_NONCOHERENT_MMAP
select GENERIC_ATOMIC64 if !ISA_ARCV2 || !(ARC_HAS_LL64 && ARC_HAS_LLSC)
select GENERIC_CLOCKEVENTS
select GENERIC_FIND_FIRST_BIT
diff --git a/arch/arc/include/asm/Kbuild b/arch/arc/include/asm/Kbuild
index 4bd5d4369e05..bbdcb955e18f 100644
--- a/arch/arc/include/asm/Kbuild
+++ b/arch/arc/include/asm/Kbuild
@@ -2,6 +2,7 @@
 generic-y += bugs.h
 generic-y += device.h
 generic-y += div64.h
+generic-y += dma-mapping.h
 generic-y += emergency-restart.h
 generic-y += extable.h
 generic-y += fb.h
diff --git a/arch/arc/include/asm/dma-mapping.h 
b/arch/arc/include/asm/dma-mapping.h
deleted file mode 100644
index 7a16824bfe98..
--- a/arch/arc/include/asm/dma-mapping.h
+++ /dev/null
@@ -1,21 +0,0 @@
-/*
- * DMA Mapping glue for ARC
- *
- * Copyright (C) 2004, 2007-2010, 2011-2012 Synopsys, Inc. (www.synopsys.com)
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#ifndef ASM_ARC_DMA_MAPPING_H
-#define ASM_ARC_DMA_MAPPING_H
-
-extern const struct dma_map_ops arc_dma_ops;
-
-static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
-{
-   return _dma_ops;
-}
-
-#endif
diff --git a/arch/arc/mm/dma.c b/arch/arc/mm/dma.c
index 1dcc404b5aec..ecfeb5585cc7 100644
--- a/arch/arc/mm/dma.c
+++ b/arch/arc/mm/dma.c
@@ -16,13 +16,12 @@
  * The default DMA address == Phy address which is 0x8000_ based.
  */
 
-#include 
+#include 
 #include 
 #include 
 
-
-static void *arc_dma_alloc(struct device *dev, size_t size,
-   dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
+void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
+   gfp_t gfp, unsigned long attrs)
 {
unsigned long order = get_order(size);
struct page *page;
@@ -89,7 +88,7 @@ static void *arc_dma_alloc(struct device *dev, size_t size,
return kvaddr;
 }
 
-static void arc_dma_free(struct device *dev, size_t size, void *vaddr,
+void arch_dma_free(struct device *dev, size_t size, void *vaddr,
dma_addr_t dma_handle, unsigned long attrs)
 {
phys_addr_t paddr = dma_handle;
@@ -105,9 +104,9 @@ static void arc_dma_free(struct device *dev, size_t size, 
void *vaddr,
__free_pages(page, get_order(size));
 }
 
-static int arc_dma_mmap(struct device *dev, struct vm_area_struct *vma,
-   void *cpu_addr, dma_addr_t dma_addr, size_t size,
-   unsigned long attrs)
+int arch_dma_mmap(struct device *dev, struct vm_area_struct *vma,
+   void *cpu_addr, dma_addr_t dma_addr, size_t size,
+   unsigned long attrs)
 {
unsigned long user_count = vma_pages(vma);
unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT;
@@ -135,7 +134,7 @@ static int arc_dma_mmap(struct device *dev, struct 
vm_area_struct *vma,
  * CPU accesses page via normal paddr, thus needs to explicitly made
  * consistent before each use
  */
-static void _dma_cache_sync(phys_addr_t paddr, size_t size,
+static void _dma_cache_sync(struct device *dev, phys_addr_t paddr size_t size,
enum dma_data_direction dir)
 {
switch (dir) {
@@ -153,126 +152,14 @@ static void _dma_cache_sync(phys_addr_t paddr, size_t 
size,
}
 }
 
-/*
- * arc_dma_map_page - map a portion of a page for streaming DMA
- *
- * Ensure that any data held in the cache is appropriately discarded
- * or written back.
- *
- * The device owns this memory once this call has completed.  The CPU
- * can regain ownership by calling dma_unmap_page().
- *
- * Note: while it takes struct page as arg, caller can "abuse" it to pass
- * a region larger than PAGE_SIZE, provided it is physically contiguous
- * and this still works correctly
- */
-static dma_addr_t arc_dma_map_page(struct device *dev, struct page *page,
-   unsigned long offset, size_t size, enum