[PATCH 0/3] dma-mapping:remove CONFIG_HAVE_DMA_ATTRS

[PATCH 0/3] dma-mapping:remove CONFIG_HAVE_DMA_ATTRS

[tom.leiming]

This patch sets  enable CONFIG_HAVE_DMA_ATTRS always, so remove
it in dma-mapping and Kconfig of X86 and IA64 arch, based on
the following ideas:

1,Currently asm-generic/dma-mapping-common.h has provided dma_map_*_attrs()
and dma_unmap_*_attrs(), and more ARCHs will move to asm-generic/dma-mapping-common.h;

2,Disabling CONFIG_HAVE_DMA_ATTRS may lead to a compile failure;

3,X86 and IA64 has taken asm-generic/dma-mapping-common.h and always enabled the
option now;

4,If CONFIG_HAVE_DMA_ATTRS is enabled, dma_set/get_attr support the passed attrs
is NULL.

[PATCH 1/3] dma-mapping:remove CONFIG_HAVE_DMA_ATTRS

[tom.leiming]

From: Ming Lei <tom.leiming@gmail.com>


Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 include/linux/dma-attrs.h   |   10 ----------
 include/linux/dma-mapping.h |   17 -----------------
 2 files changed, 0 insertions(+), 27 deletions(-)

diff --git a/include/linux/dma-attrs.h b/include/linux/dma-attrs.h
index 71ad34e..fce45cd 100644
--- a/include/linux/dma-attrs.h
+++ b/include/linux/dma-attrs.h
@@ -36,7 +36,6 @@ static inline void init_dma_attrs(struct dma_attrs *attrs)
 	bitmap_zero(attrs->flags, __DMA_ATTRS_LONGS);
 }
 
-#ifdef CONFIG_HAVE_DMA_ATTRS
 /**
  * dma_set_attr - set a specific attribute
  * @attr: attribute to set
@@ -62,14 +61,5 @@ static inline int dma_get_attr(enum dma_attr attr, struct dma_attrs *attrs)
 	BUG_ON(attr >= DMA_ATTR_MAX);
 	return test_bit(attr, attrs->flags);
 }
-#else /* !CONFIG_HAVE_DMA_ATTRS */
-static inline void dma_set_attr(enum dma_attr attr, struct dma_attrs *attrs)
-{
-}
 
-static inline int dma_get_attr(enum dma_attr attr, struct dma_attrs *attrs)
-{
-	return 0;
-}
-#endif /* CONFIG_HAVE_DMA_ATTRS */
 #endif /* _DMA_ATTR_H */
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 07dfd46..b83acee 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -219,21 +219,4 @@ static inline void dmam_release_declared_memory(struct device *dev)
 }
 #endif /* ARCH_HAS_DMA_DECLARE_COHERENT_MEMORY */
 
-#ifndef CONFIG_HAVE_DMA_ATTRS
-struct dma_attrs;
-
-#define dma_map_single_attrs(dev, cpu_addr, size, dir, attrs) \
-	dma_map_single(dev, cpu_addr, size, dir)
-
-#define dma_unmap_single_attrs(dev, dma_addr, size, dir, attrs) \
-	dma_unmap_single(dev, dma_addr, size, dir)
-
-#define dma_map_sg_attrs(dev, sgl, nents, dir, attrs) \
-	dma_map_sg(dev, sgl, nents, dir)
-
-#define dma_unmap_sg_attrs(dev, sgl, nents, dir, attrs) \
-	dma_unmap_sg(dev, sgl, nents, dir)
-
-#endif /* CONFIG_HAVE_DMA_ATTRS */
-
 #endif
-- 
1.6.0.GIT

[PATCH 2/3] dma-mapping:X86:remove CONFIG_HAVE_DMA_ATTRS

[tom.leiming]

From: Ming Lei <tom.leiming@gmail.com>


Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 arch/x86/Kconfig |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index c86a1af..a135d27 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -28,7 +28,6 @@ config X86
 	select HAVE_KPROBES
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	select ARCH_WANT_FRAME_POINTERS
-	select HAVE_DMA_ATTRS
 	select HAVE_KRETPROBES
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_DYNAMIC_FTRACE
-- 
1.6.0.GIT

[PATCH 3/3] dma-mapping:IA64:remove CONFIG_HAVE_DMA_ATTRS

[tom.leiming]

From: Ming Lei <tom.leiming@gmail.com>


Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 arch/ia64/Kconfig |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 328d2f8..07b7f68 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -25,7 +25,6 @@ config IA64
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_DYNAMIC_FTRACE if (!ITANIUM)
 	select HAVE_FUNCTION_TRACER
-	select HAVE_DMA_ATTRS
 	select HAVE_KVM
 	select HAVE_ARCH_TRACEHOOK
 	select HAVE_DMA_API_DEBUG
-- 
1.6.0.GIT

Re: [PATCH 0/3] dma-mapping:remove CONFIG_HAVE_DMA_ATTRS

[Arnd Bergmann]

On Saturday 04 July 2009, tom.leiming@gmail.com wrote:
> 2,Disabling CONFIG_HAVE_DMA_ATTRS may lead to a compile failure;

I'm not sure I understand this point. CONFIG_HAVE_DMA_ATTRS tells
the common code whether the architecture understands dma attributes.
If you enable it on all architectures, you will get new compile
failures on all those that don't understand them, while the current
code correctly falls back on the standard functions.

I think it makes sense to combine CONFIG_HAVE_DMA_ATTRS with the
use of dma-mapping-common.h, but the majority of the architectures
just uses a static mapping, where attributes make no sense.

You also missed powerpc64, which selects CONFIG_HAVE_DMA_ATTRS
but does not (yet) use dma-mapping-common.h.

	Arnd <><

Re: [PATCH 0/3] dma-mapping:remove CONFIG_HAVE_DMA_ATTRS

[Ming Lei]

On Sun, 5 Jul 2009 13:19:36 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Saturday 04 July 2009, tom.leiming@gmail.com wrote:
> > 2,Disabling CONFIG_HAVE_DMA_ATTRS may lead to a compile failure;
> 
> I'm not sure I understand this point. CONFIG_HAVE_DMA_ATTRS tells
> the common code whether the architecture understands dma attributes.

If a new arch does not define CONFIG_HAVE_DMA_ATTRS but uses
dma-mapping-common.h, it will lead to a compile failure. 

include/asm-generic/dma-mapping-common.h

	#define dma_map_single(d, a, s, r) dma_map_single_attrs(d, a,
	s, r, NULL)
	...

include/linux/dma-mapping.h
	
	#ifndef CONFIG_HAVE_DMA_ATTRS
	struct dma_attrs;

	#define dma_map_single_attrs(dev, cpu_addr, size, dir, attrs) \
        dma_map_single(dev, cpu_addr, size, dir)
	...
	#endif


> If you enable it on all architectures, you will get new compile
> failures on all those that don't understand them, while the current
> code correctly falls back on the standard functions.

You are right, the patch will lead to new compile failure for the
ARCHs, on which disables CONFIG_HAVE_DMA_ATTRS and doesn't use
dma-mapping-common.h. So the patch is not mature.

> 
> I think it makes sense to combine CONFIG_HAVE_DMA_ATTRS with the
> use of dma-mapping-common.h, but the majority of the architectures
> just uses a static mapping, where attributes make no sense.
> 
> You also missed powerpc64, which selects CONFIG_HAVE_DMA_ATTRS
> but does not (yet) use dma-mapping-common.h.

ppc64 doesn't use dma-mapping-common.h, so the patch doesn't consider
it.

-- 
Lei Ming

Re: [PATCH 0/3] dma-mapping:remove CONFIG_HAVE_DMA_ATTRS

[Ming Lei]

2009/7/5 Ming Lei <tom.leiming@gmail.com>:
> On Sun, 5 Jul 2009 13:19:36 +0200
> Arnd Bergmann <arnd@arndb.de> wrote:
>
>> On Saturday 04 July 2009, tom.leiming@gmail.com wrote:
>> > 2,Disabling CONFIG_HAVE_DMA_ATTRS may lead to a compile failure;
>>
>> I'm not sure I understand this point. CONFIG_HAVE_DMA_ATTRS tells
>> the common code whether the architecture understands dma attributes.
>
> If a new arch does not define CONFIG_HAVE_DMA_ATTRS but uses
> dma-mapping-common.h, it will lead to a compile failure.
>
> include/asm-generic/dma-mapping-common.h
>
>        #define dma_map_single(d, a, s, r) dma_map_single_attrs(d, a,
>        s, r, NULL)
>        ...
>
> include/linux/dma-mapping.h
>
>        #ifndef CONFIG_HAVE_DMA_ATTRS
>        struct dma_attrs;
>
>        #define dma_map_single_attrs(dev, cpu_addr, size, dir, attrs) \
>        dma_map_single(dev, cpu_addr, size, dir)
>        ...
>        #endif
>
>

I have another idea to fix the possible compile failure.  Does the
following patch
work?

diff --git a/include/asm-generic/dma-mapping-common.h
b/include/asm-generic/dma-mapping-common.h
index 5406a60..39e2ec5 100644
--- a/include/asm-generic/dma-mapping-common.h
+++ b/include/asm-generic/dma-mapping-common.h
@@ -6,6 +6,8 @@
 #include <linux/dma-debug.h>
 #include <linux/dma-attrs.h>

+#define ARCH_USE_DMA_MAPPING_COMMON
+
 static inline dma_addr_t dma_map_single_attrs(struct device *dev, void *ptr,
 					      size_t size,
 					      enum dma_data_direction dir,
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 07dfd46..4be1121 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -219,6 +219,7 @@ static inline void
dmam_release_declared_memory(struct device *dev)
 }
 #endif /* ARCH_HAS_DMA_DECLARE_COHERENT_MEMORY */

+#ifndef ARCH_USE_DMA_MAPPING_COMMON
 #ifndef CONFIG_HAVE_DMA_ATTRS
 struct dma_attrs;

@@ -235,5 +236,7 @@ struct dma_attrs;
 	dma_unmap_sg(dev, sgl, nents, dir)

 #endif /* CONFIG_HAVE_DMA_ATTRS */
+#endif /* ARCH_USE_DMA_MAPPING_COMMON */
+

 #endif



-- 
Lei Ming

Re: [PATCH 0/3] dma-mapping:remove CONFIG_HAVE_DMA_ATTRS

[Arnd Bergmann]

On Sunday 05 July 2009, Ming Lei wrote:
> If a new arch does not define CONFIG_HAVE_DMA_ATTRS but uses
> dma-mapping-common.h, it will lead to a compile failure. 

Right, but I think we should just mandate that every arch that wants
to use dma-mapping-common.h should define CONFIG_HAVE_DMA_ATTRS,
which is sort of implied by the definition of dma_map_ops anyway.

	Arnd <><

Re: [PATCH 0/3] dma-mapping:remove CONFIG_HAVE_DMA_ATTRS

[FUJITA Tomonori]

On Sun, 5 Jul 2009 20:44:18 +0800
Ming Lei <tom.leiming@gmail.com> wrote:

> On Sun, 5 Jul 2009 13:19:36 +0200
> Arnd Bergmann <arnd@arndb.de> wrote:
> 
> > On Saturday 04 July 2009, tom.leiming@gmail.com wrote:
> > > 2,Disabling CONFIG_HAVE_DMA_ATTRS may lead to a compile failure;
> > 
> > I'm not sure I understand this point. CONFIG_HAVE_DMA_ATTRS tells
> > the common code whether the architecture understands dma attributes.
> 
> If a new arch does not define CONFIG_HAVE_DMA_ATTRS but uses
> dma-mapping-common.h, it will lead to a compile failure. 

Yeah, architectures that use dma-mapping-common.h need to define
CONFIG_HAVE_DMA_ATTRS because dma-mapping-common.h needs to handle
architectures that need CONFIG_HAVE_DMA_ATTRS.


Your idea doesn't sound good to me.

I think that it's better to define CONFIG_HAVE_DMA_ATTRS in the
consistent way; defining arch's Kconfig. Defining
CONFIG_HAVE_DMA_ATTRS in two different ways and inventing another
define such as ARCH_USE_DMA_MAPPING_COMMON is just confusing.