All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] dma-mapping-broken.h: flesh-out DMA API stubs
@ 2007-02-23  0:59 John W. Linville
  2007-02-23  3:42 ` Heiko Carstens
  0 siblings, 1 reply; 12+ messages in thread
From: John W. Linville @ 2007-02-23  0:59 UTC (permalink / raw)
  To: linux-kernel, James.Bottomley
  Cc: Tejun Heo, Jeff Garzik, Martin Schwidefsky, Heiko Carstens

This allows some drivers compile on arches that don't support DMA
(e.g. s390).

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
Is there any reason why this header should not cover the whole DMA API?

Compile-tested only...

 include/asm-generic/dma-mapping-broken.h |  134 ++++++++++++++++++++++++++++++
 1 files changed, 134 insertions(+), 0 deletions(-)

diff --git a/include/asm-generic/dma-mapping-broken.h b/include/asm-generic/dma-mapping-broken.h
index 29413d3..84812dd 100644
--- a/include/asm-generic/dma-mapping-broken.h
+++ b/include/asm-generic/dma-mapping-broken.h
@@ -21,4 +21,116 @@ dma_free_coherent(struct device *dev, size_t size, void *cpu_addr,
 #define dma_alloc_noncoherent(d, s, h, f) dma_alloc_coherent(d, s, h, f)
 #define dma_free_noncoherent(d, s, v, h) dma_free_coherent(d, s, v, h)
 
+static inline dma_addr_t
+dma_map_single(struct device *dev, void *ptr, size_t size,
+	       enum dma_data_direction direction)
+{
+	BUG();
+	return (dma_addr_t)0;
+}
+
+static inline void
+dma_unmap_single(struct device *dev, dma_addr_t dma_addr, size_t size,
+		 enum dma_data_direction direction)
+{
+	BUG();
+}
+
+static inline int
+dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
+	   enum dma_data_direction direction)
+{
+	BUG();
+	return 0;
+}
+
+static inline void
+dma_unmap_sg(struct device *dev, struct scatterlist *sg, int nhwentries,
+	     enum dma_data_direction direction)
+{
+	BUG();
+}
+
+static inline dma_addr_t
+dma_map_page(struct device *dev, struct page *page, unsigned long offset,
+	     size_t size, enum dma_data_direction direction)
+{
+	BUG();
+	return (dma_addr_t)0;
+}
+
+static inline void
+dma_unmap_page(struct device *dev, dma_addr_t dma_address, size_t size,
+	       enum dma_data_direction direction)
+{
+	BUG();
+}
+
+static inline void
+dma_sync_single_for_cpu(struct device *dev, dma_addr_t dma_handle, size_t size,
+			enum dma_data_direction direction)
+{
+	BUG();
+}
+
+static inline void
+dma_sync_single_range_for_cpu(struct device *dev, dma_addr_t dma_handle,
+			      unsigned long offset, size_t size,
+			      enum dma_data_direction direction)
+{
+	BUG();
+}
+
+static inline void
+dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg, int nelems,
+		    enum dma_data_direction direction)
+{
+	BUG();
+}
+
+#define dma_sync_single_for_device dma_sync_single_for_cpu
+#define dma_sync_single_range_for_device dma_sync_single_range_for_cpu
+#define dma_sync_sg_for_device dma_sync_sg_for_cpu
+
+static inline int
+dma_mapping_error(dma_addr_t dma_addr)
+{
+	BUG();
+	return 1;
+}
+
+static inline int
+dma_supported(struct device *dev, u64 mask)
+{
+	return 0;
+}
+
+static inline int
+dma_set_mask(struct device *dev, u64 mask)
+{
+	BUG();
+	return -EIO;
+}
+
+static inline int
+dma_get_cache_alignment(void)
+{
+	BUG();
+	return 0;
+}
+
+int
+dma_is_consistent(struct device *dev, dma_addr_t dma_handle)
+{
+	BUG();
+	return 0;
+}
+
+static inline void
+dma_cache_sync(struct device *dev, void *vaddr, size_t size,
+	       enum dma_data_direction direction)
+{
+	BUG();
+}
+
 #endif /* _ASM_GENERIC_DMA_MAPPING_H */
-- 
1.4.4.2

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] dma-mapping-broken.h: flesh-out DMA API stubs
  2007-02-23  0:59 [PATCH] dma-mapping-broken.h: flesh-out DMA API stubs John W. Linville
@ 2007-02-23  3:42 ` Heiko Carstens
  2007-02-23  6:13   ` Heiko Carstens
  0 siblings, 1 reply; 12+ messages in thread
From: Heiko Carstens @ 2007-02-23  3:42 UTC (permalink / raw)
  To: John W. Linville
  Cc: linux-kernel, James.Bottomley, Tejun Heo, Jeff Garzik,
	Martin Schwidefsky

On Thu, Feb 22, 2007 at 07:59:45PM -0500, John W. Linville wrote:
> This allows some drivers compile on arches that don't support DMA
> (e.g. s390).

Which drivers are we talking about? Last time I checked an allmodconfig
just compiled.
I'd rather like to the opposite: rip out everything that depends on DMA
(via e.g. CONFIG_HAS_DMA), so we don't have to compile in all the unused
code. If that is is possible...

-- 
Heiko Carstens
Linux on System z Development

IBM Deutschland Entwicklung GmbH
Vorsitzender des Aufsichtsrats: Johann Weihen
Geschaeftsfuehrung : Herbert Kircher
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] dma-mapping-broken.h: flesh-out DMA API stubs
  2007-02-23  3:42 ` Heiko Carstens
@ 2007-02-23  6:13   ` Heiko Carstens
  2007-02-23  9:50     ` Heiko Carstens
  2007-02-23 13:57     ` Kyle McMartin
  0 siblings, 2 replies; 12+ messages in thread
From: Heiko Carstens @ 2007-02-23  6:13 UTC (permalink / raw)
  To: John W. Linville
  Cc: linux-kernel, James.Bottomley, Tejun Heo, Jeff Garzik,
	Martin Schwidefsky

How about this for telling that an architecture doesn't support DMA?
At least we could get rid of dma-mapping-broken.h and don't need to
compile some afterwards dead code.

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 arch/s390/Kconfig              |    3 +++
 drivers/base/Makefile          |    4 ++--
 include/asm-s390/dma-mapping.h |    2 --
 lib/Kconfig                    |    5 +++++
 4 files changed, 10 insertions(+), 4 deletions(-)

Index: linux-2.6/arch/s390/Kconfig
===================================================================
--- linux-2.6.orig/arch/s390/Kconfig
+++ linux-2.6/arch/s390/Kconfig
@@ -44,6 +44,9 @@ config GENERIC_TIME
 config NO_IOMEM
 	def_bool y
 
+config NO_DMA
+	def_bool y
+
 mainmenu "Linux Kernel Configuration"
 
 config S390
Index: linux-2.6/drivers/base/Makefile
===================================================================
--- linux-2.6.orig/drivers/base/Makefile
+++ linux-2.6/drivers/base/Makefile
@@ -2,10 +2,10 @@
 
 obj-y			:= core.o sys.o bus.o dd.o \
 			   driver.o class.o platform.o \
-			   cpu.o firmware.o init.o map.o dmapool.o \
-			   dma-mapping.o devres.o \
+			   cpu.o firmware.o init.o map.o devres.o \
 			   attribute_container.o transport_class.o
 obj-y			+= power/
+obj-$(CONFIG_DMA)	+= dma-mapping.o dmapool.o
 obj-$(CONFIG_ISA)	+= isa.o
 obj-$(CONFIG_FW_LOADER)	+= firmware_class.o
 obj-$(CONFIG_NUMA)	+= node.o
Index: linux-2.6/include/asm-s390/dma-mapping.h
===================================================================
--- linux-2.6.orig/include/asm-s390/dma-mapping.h
+++ linux-2.6/include/asm-s390/dma-mapping.h
@@ -9,6 +9,4 @@
 #ifndef _ASM_DMA_MAPPING_H
 #define _ASM_DMA_MAPPING_H
 
-#include <asm-generic/dma-mapping-broken.h>
-
 #endif /* _ASM_DMA_MAPPING_H */
Index: linux-2.6/lib/Kconfig
===================================================================
--- linux-2.6.orig/lib/Kconfig
+++ linux-2.6/lib/Kconfig
@@ -111,4 +111,9 @@ config HAS_IOPORT
 	depends on HAS_IOMEM && !NO_IOPORT
 	default y
 
+config HAS_DMA
+	boolean
+	depends on !NO_DMA
+	default y
+
 endmenu

-- 
Heiko Carstens
Linux on System z Development

IBM Deutschland Entwicklung GmbH
Vorsitzender des Aufsichtsrats: Johann Weihen
Geschaeftsfuehrung : Herbert Kircher
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] dma-mapping-broken.h: flesh-out DMA API stubs
  2007-02-23  6:13   ` Heiko Carstens
@ 2007-02-23  9:50     ` Heiko Carstens
  2007-02-23 13:57     ` Kyle McMartin
  1 sibling, 0 replies; 12+ messages in thread
From: Heiko Carstens @ 2007-02-23  9:50 UTC (permalink / raw)
  To: John W. Linville
  Cc: linux-kernel, James.Bottomley, Tejun Heo, Jeff Garzik,
	Martin Schwidefsky

On Fri, Feb 23, 2007 at 07:13:32AM +0100, Heiko Carstens wrote:
> How about this for telling that an architecture doesn't support DMA?
> At least we could get rid of dma-mapping-broken.h and don't need to
> compile some afterwards dead code.
> 
> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> ---
>  arch/s390/Kconfig              |    3 +++
>  drivers/base/Makefile          |    4 ++--
>  include/asm-s390/dma-mapping.h |    2 --
>  lib/Kconfig                    |    5 +++++
>  4 files changed, 10 insertions(+), 4 deletions(-)
> 
> Index: linux-2.6/arch/s390/Kconfig
> ===================================================================
> --- linux-2.6.orig/arch/s390/Kconfig
> +++ linux-2.6/arch/s390/Kconfig
> @@ -44,6 +44,9 @@ config GENERIC_TIME
>  config NO_IOMEM
>  	def_bool y
> 
> +config NO_DMA
> +	def_bool y
> +
>  mainmenu "Linux Kernel Configuration"
> 
>  config S390
> Index: linux-2.6/drivers/base/Makefile
> ===================================================================
> --- linux-2.6.orig/drivers/base/Makefile
> +++ linux-2.6/drivers/base/Makefile
> @@ -2,10 +2,10 @@
> 
>  obj-y			:= core.o sys.o bus.o dd.o \
>  			   driver.o class.o platform.o \
> -			   cpu.o firmware.o init.o map.o dmapool.o \
> -			   dma-mapping.o devres.o \
> +			   cpu.o firmware.o init.o map.o devres.o \
>  			   attribute_container.o transport_class.o
>  obj-y			+= power/
> +obj-$(CONFIG_DMA)	+= dma-mapping.o dmapool.o

This should have been CONFIG_HAS_DMA. Anyway I'm going to resend this one
if nobody objects.



>  obj-$(CONFIG_ISA)	+= isa.o
>  obj-$(CONFIG_FW_LOADER)	+= firmware_class.o
>  obj-$(CONFIG_NUMA)	+= node.o
> Index: linux-2.6/include/asm-s390/dma-mapping.h
> ===================================================================
> --- linux-2.6.orig/include/asm-s390/dma-mapping.h
> +++ linux-2.6/include/asm-s390/dma-mapping.h
> @@ -9,6 +9,4 @@
>  #ifndef _ASM_DMA_MAPPING_H
>  #define _ASM_DMA_MAPPING_H
> 
> -#include <asm-generic/dma-mapping-broken.h>
> -
>  #endif /* _ASM_DMA_MAPPING_H */
> Index: linux-2.6/lib/Kconfig
> ===================================================================
> --- linux-2.6.orig/lib/Kconfig
> +++ linux-2.6/lib/Kconfig
> @@ -111,4 +111,9 @@ config HAS_IOPORT
>  	depends on HAS_IOMEM && !NO_IOPORT
>  	default y
> 
> +config HAS_DMA
> +	boolean
> +	depends on !NO_DMA
> +	default y
> +
>  endmenu

-- 
Heiko Carstens
Linux on System z Development

IBM Deutschland Entwicklung GmbH
Vorsitzender des Aufsichtsrats: Johann Weihen
Geschaeftsfuehrung : Herbert Kircher
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] dma-mapping-broken.h: flesh-out DMA API stubs
  2007-02-23  6:13   ` Heiko Carstens
  2007-02-23  9:50     ` Heiko Carstens
@ 2007-02-23 13:57     ` Kyle McMartin
  2007-02-23 14:39       ` John W. Linville
  1 sibling, 1 reply; 12+ messages in thread
From: Kyle McMartin @ 2007-02-23 13:57 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: John W. Linville, linux-kernel, James.Bottomley, Tejun Heo,
	Jeff Garzik, Martin Schwidefsky

On Fri, Feb 23, 2007 at 07:13:32AM +0100, Heiko Carstens wrote:
> How about this for telling that an architecture doesn't support DMA?
> At least we could get rid of dma-mapping-broken.h and don't need to
> compile some afterwards dead code.
> 
> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>

This like a lot better of an idea than stubbing out things which should never
be built in the first place...

Cheers,
	Kyle

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] dma-mapping-broken.h: flesh-out DMA API stubs
  2007-02-23 13:57     ` Kyle McMartin
@ 2007-02-23 14:39       ` John W. Linville
  2007-02-26 13:57         ` [patch] Introduce CONFIG_HAS_DMA Heiko Carstens
  0 siblings, 1 reply; 12+ messages in thread
From: John W. Linville @ 2007-02-23 14:39 UTC (permalink / raw)
  To: Kyle McMartin
  Cc: Heiko Carstens, linux-kernel, James.Bottomley, Tejun Heo,
	Jeff Garzik, Martin Schwidefsky

On Fri, Feb 23, 2007 at 08:57:41AM -0500, Kyle McMartin wrote:
> On Fri, Feb 23, 2007 at 07:13:32AM +0100, Heiko Carstens wrote:
> > How about this for telling that an architecture doesn't support DMA?
> > At least we could get rid of dma-mapping-broken.h and don't need to
> > compile some afterwards dead code.
> > 
> > Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> 
> This like a lot better of an idea than stubbing out things which should never
> be built in the first place...

My only thought was that the intent must have been to allow compilation of a driver
that checks for DMA at runtime (via dma_supported).  But, I guess dma_supported is intended
as a platform-level check?

If the DMA availability is at the arch level then such a driver could
use (albeit ugly) compile flags to cover that situation...?

Well, either way is fine with me.

John
-- 
John W. Linville
linville@tuxdriver.com

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [patch] Introduce CONFIG_HAS_DMA.
  2007-02-23 14:39       ` John W. Linville
@ 2007-02-26 13:57         ` Heiko Carstens
  2007-05-25 16:36           ` Dan Williams
  0 siblings, 1 reply; 12+ messages in thread
From: Heiko Carstens @ 2007-02-26 13:57 UTC (permalink / raw)
  To: Andrew Morton, John W. Linville
  Cc: Kyle McMartin, linux-kernel, James.Bottomley, Tejun Heo,
	Jeff Garzik, Martin Schwidefsky, geert, zippel, spyro,
	uclinux-v850, ysato

From: Heiko Carstens <heiko.carstens@de.ibm.com>

Architectures that don't support DMA can say so by adding a
config NO_DMA to their Kconfig file. This will prevent compilation
of some dma specific driver code. Also dma-mapping-broken.h isn't
needed anymore on at least s390.
This avoids compilation and linking of otherwise dead/broken code.

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---

Other architectures that include dma-mapping-broken.h are
arm26, h8300, m68k, m68knommu and v850. If these could be converted
as well we could get rid of the header file.

 arch/s390/Kconfig              |    3 +++
 drivers/base/Makefile          |    4 ++--
 include/asm-s390/dma-mapping.h |    2 --
 lib/Kconfig                    |    5 +++++
 4 files changed, 10 insertions(+), 4 deletions(-)

Index: linux-2.6/arch/s390/Kconfig
===================================================================
--- linux-2.6.orig/arch/s390/Kconfig
+++ linux-2.6/arch/s390/Kconfig
@@ -44,6 +44,9 @@ config GENERIC_TIME
 config NO_IOMEM
 	def_bool y
 
+config NO_DMA
+	def_bool y
+
 mainmenu "Linux Kernel Configuration"
 
 config S390
Index: linux-2.6/drivers/base/Makefile
===================================================================
--- linux-2.6.orig/drivers/base/Makefile
+++ linux-2.6/drivers/base/Makefile
@@ -2,10 +2,10 @@
 
 obj-y			:= core.o sys.o bus.o dd.o \
 			   driver.o class.o platform.o \
-			   cpu.o firmware.o init.o map.o dmapool.o \
-			   dma-mapping.o devres.o \
+			   cpu.o firmware.o init.o map.o devres.o \
 			   attribute_container.o transport_class.o
 obj-y			+= power/
+obj-$(CONFIG_HAS_DMA)	+= dma-mapping.o dmapool.o
 obj-$(CONFIG_ISA)	+= isa.o
 obj-$(CONFIG_FW_LOADER)	+= firmware_class.o
 obj-$(CONFIG_NUMA)	+= node.o
Index: linux-2.6/include/asm-s390/dma-mapping.h
===================================================================
--- linux-2.6.orig/include/asm-s390/dma-mapping.h
+++ linux-2.6/include/asm-s390/dma-mapping.h
@@ -9,6 +9,4 @@
 #ifndef _ASM_DMA_MAPPING_H
 #define _ASM_DMA_MAPPING_H
 
-#include <asm-generic/dma-mapping-broken.h>
-
 #endif /* _ASM_DMA_MAPPING_H */
Index: linux-2.6/lib/Kconfig
===================================================================
--- linux-2.6.orig/lib/Kconfig
+++ linux-2.6/lib/Kconfig
@@ -111,4 +111,9 @@ config HAS_IOPORT
 	depends on HAS_IOMEM && !NO_IOPORT
 	default y
 
+config HAS_DMA
+	boolean
+	depends on !NO_DMA
+	default y
+
 endmenu

-- 
Heiko Carstens
Linux on System z Development

IBM Deutschland Entwicklung GmbH
Vorsitzender des Aufsichtsrats: Johann Weihen
Geschaeftsfuehrung : Herbert Kircher
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] Introduce CONFIG_HAS_DMA.
  2007-02-26 13:57         ` [patch] Introduce CONFIG_HAS_DMA Heiko Carstens
@ 2007-05-25 16:36           ` Dan Williams
  2007-05-30  9:10             ` Cornelia Huck
  0 siblings, 1 reply; 12+ messages in thread
From: Dan Williams @ 2007-05-25 16:36 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Andrew Morton, John W. Linville, Kyle McMartin, linux-kernel,
	James.Bottomley, Tejun Heo, Jeff Garzik, Martin Schwidefsky,
	geert, zippel, spyro, uclinux-v850, ysato, Cornelia Huck

[ apologies for reviving an old conversation ]

On 2/26/07, Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> From: Heiko Carstens <heiko.carstens@de.ibm.com>
>
> Architectures that don't support DMA can say so by adding a
> config NO_DMA to their Kconfig file. This will prevent compilation
> of some dma specific driver code. Also dma-mapping-broken.h isn't
> needed anymore on at least s390.
> This avoids compilation and linking of otherwise dead/broken code.
>

I went back and read the thread leading up to this patch and I am of
the opinion that John's approach (adding more stubs to
dma-mapping-broken.h:
http://marc.info/?l=linux-kernel&m=117219377712232&w=2)  is needed in
_addition_ to this Kconfig option.  In my particular case I have an
API with a DMA and a non-DMA path written in such a way that the DMA
path can be compiled away.  Without dma-mapping-broken.h support the
API is unnecessarily forced to violate point 2 of
Documentation/SubmittingPatches (#ifdefs are ugly).

In other words let CONFIG_HAS_DMA prevent pure DMA code from being
built, but do not preclude "clever" implementations from calling
broken code.

If there are no objections I plan to resubmit John's changes with the
addition below, which allows CONFIG_HAS_DMA=n architectures to delete
their respective dma-mapping.h file.

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 9a663c6..2dc21cb 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -31,7 +31,11 @@ static inline int valid_dma_direction(int dma_direction)
                (dma_direction == DMA_FROM_DEVICE));
 }

+#ifdef CONFIG_HAS_DMA
 #include <asm/dma-mapping.h>
+#else
+#include <asm-generic/dma-mapping-broken.h>
+#endif

 /* Backwards compat, remove in 2.7.x */
 #define dma_sync_single                dma_sync_single_for_cpu

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [patch] Introduce CONFIG_HAS_DMA.
  2007-05-25 16:36           ` Dan Williams
@ 2007-05-30  9:10             ` Cornelia Huck
  2007-05-30 21:40               ` Williams, Dan J
  0 siblings, 1 reply; 12+ messages in thread
From: Cornelia Huck @ 2007-05-30  9:10 UTC (permalink / raw)
  To: Dan Williams
  Cc: Heiko Carstens, Andrew Morton, John W. Linville, Kyle McMartin,
	linux-kernel, James.Bottomley, Tejun Heo, Jeff Garzik,
	Martin Schwidefsky, geert, zippel, spyro, uclinux-v850, ysato

On Fri, 25 May 2007 09:36:31 -0700,
"Dan Williams" <dan.j.williams@intel.com> wrote:

> I went back and read the thread leading up to this patch and I am of
> the opinion that John's approach (adding more stubs to
> dma-mapping-broken.h:
> http://marc.info/?l=linux-kernel&m=117219377712232&w=2)  is needed in
> _addition_ to this Kconfig option.  In my particular case I have an
> API with a DMA and a non-DMA path written in such a way that the DMA
> path can be compiled away.  Without dma-mapping-broken.h support the
> API is unnecessarily forced to violate point 2 of
> Documentation/SubmittingPatches (#ifdefs are ugly).

IMO, well-placed #ifdefs are preferrable to dragging non-working code
around. Like:

- put the DMA path in a file only build for MY_STUFF_USE_DMA
- let MY_STUFF select MY_STUFF_USE_DMA if HAS_DMA
- have a header file that points to the implementation for
  MY_STUFF_USE_DMA and uses well defined stubs for !MY_STUFF_USE_DMA,
  like returning ICannotDoThat for check_if_can_do() (kind of like what
  include/linux/sysfs.h does, for example)

This contains the #ifdefs in a header, doesn't compile stuff that won't
work anyway on !HAS_DMA, and adds the ability to disable
MY_STUFF_USE_DMA even if HAS_DMA at a later time if someone wants it.

> In other words let CONFIG_HAS_DMA prevent pure DMA code from being
> built, but do not preclude "clever" implementations from calling
> broken code.

If it calls broken code, it may not be so "clever" after all :)

What I don't like about this is

- compiles stuff that is not needed on !HAS_DMA
- worse, compiles stuff that will not work on !HAS_DMA
- does not encourage to split code properly into a DMA and a non-DMA
  part

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [patch] Introduce CONFIG_HAS_DMA.
  2007-05-30  9:10             ` Cornelia Huck
@ 2007-05-30 21:40               ` Williams, Dan J
  2007-05-30 22:29                 ` [PATCH] dma-mapping: prevent dma dependent code from linking on !HAS_DMA archs Dan Williams
  2007-05-31  6:53                 ` [patch] Introduce CONFIG_HAS_DMA Cornelia Huck
  0 siblings, 2 replies; 12+ messages in thread
From: Williams, Dan J @ 2007-05-30 21:40 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: Heiko Carstens, Andrew Morton, John W. Linville, Kyle McMartin,
	linux-kernel, James.Bottomley, Tejun Heo, Jeff Garzik,
	Martin Schwidefsky, geert, zippel, spyro, uclinux-v850, ysato

[ please let me know if you want to be dropped from the cc ]

> From: Cornelia Huck [mailto:cornelia.huck@de.ibm.com]
> IMO, well-placed #ifdefs are preferrable to dragging non-working code
> around. Like:
> 
> - put the DMA path in a file only build for MY_STUFF_USE_DMA
> - let MY_STUFF select MY_STUFF_USE_DMA if HAS_DMA
> - have a header file that points to the implementation for
>   MY_STUFF_USE_DMA and uses well defined stubs for !MY_STUFF_USE_DMA,
>   like returning ICannotDoThat for check_if_can_do() (kind of like
what
>   include/linux/sysfs.h does, for example)
> 
> This contains the #ifdefs in a header, doesn't compile stuff that
won't
> work anyway on !HAS_DMA, and adds the ability to disable
> MY_STUFF_USE_DMA even if HAS_DMA at a later time if someone wants it.
> 
> > In other words let CONFIG_HAS_DMA prevent pure DMA code from being
> > built, but do not preclude "clever" implementations from calling
> > broken code.
> 
> If it calls broken code, it may not be so "clever" after all :)
> 
Yes, at run time, but not necessarily at compile time.  

> What I don't like about this is
> 
> - compiles stuff that is not needed on !HAS_DMA
> - worse, compiles stuff that will not work on !HAS_DMA
> - does not encourage to split code properly into a DMA and a non-DMA
>   part

I came to the same conclusions as I started to implement the bug-stubs.
The patch (to follow) attempts to satisfy all the concerns you outline
as well as my observation that async_tx requires no factoring for the
!HAS_DMA case.

With the patch non-dma-architectures that try to build code with true
dependencies on the DMA api will fail to link i.e.:

CONFIG_DMA_ENGINE=y CONFIG_HAS_DMA=n ASYNC_MEMCPY=y
  CC      init/version.o
  LD      init/built-in.o
  LD      .tmp_vmlinux1
async_tx/built-in.o: In function `async_memcpy':
xor.c:(.text+0x770): undefined reference to `dma_map_page'
xor.c:(.text+0x798): undefined reference to `dma_map_page'
xor.c:(.text+0x968): undefined reference to `dma_map_page'

Now changing CONFIG_DMA_ENGINE to depend on HAS_DMA (which is more
correct than saying !S390). Results in:
CONFIG_DMA_ENGINE=n CONFIG_HAS_DMA=n ASYNC_MEMCPY=y
  CC      init/version.o
  LD      init/built-in.o
  LD      .tmp_vmlinux1
  KSYM    .tmp_kallsyms1.S
  AS      .tmp_kallsyms1.o
  LD      .tmp_vmlinux2
  KSYM    .tmp_kallsyms2.S
  AS      .tmp_kallsyms2.o
  LD      vmlinux

This also allows include/asm-s390/dma-mapping.h to be dropped.

Dan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] dma-mapping: prevent dma dependent code from linking on !HAS_DMA archs
  2007-05-30 21:40               ` Williams, Dan J
@ 2007-05-30 22:29                 ` Dan Williams
  2007-05-31  6:53                 ` [patch] Introduce CONFIG_HAS_DMA Cornelia Huck
  1 sibling, 0 replies; 12+ messages in thread
From: Dan Williams @ 2007-05-30 22:29 UTC (permalink / raw)
  To: akpm
  Cc: linux-kernel, cornelia.huck, schwidefsky, heiko.carstens,
	linville, kyle, James.Bottomley, htejun, jeff, geert, zippel,
	spyro, ysato

Continuing the work started in 411f0f3edc141a582190d3605cadd1d993abb6df ...

This enables code with a dma path, that compiles away, to build without
requiring additional code factoring.  It also prevents code that calls
dma_alloc_coherent and dma_free_coherent from linking whereas previously
the code would hit a BUG() at run time.  Finally, it allows archs that set
!HAS_DMA to delete their asm/dma-mapping.h file.

Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: John W. Linville <linville@tuxdriver.com>
Cc: Kyle McMartin <kyle@parisc-linux.org>
Cc: James Bottomley <James.Bottomley@SteelEye.com>
Cc: Tejun Heo <htejun@gmail.com>
Cc: Jeff Garzik <jeff@garzik.org>
Cc: <geert@linux-m68k.org>
Cc: <zippel@linux-m68k.org>
Cc: <spyro@f2s.com>
Cc: <ysato@users.sourceforge.jp>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---

 arch/arm26/Kconfig                       |    3 +
 arch/h8300/Kconfig                       |    3 +
 arch/m32r/Kconfig                        |    3 +
 drivers/dma/Kconfig                      |    2 -
 include/asm-arm26/dma-mapping.h          |    2 -
 include/asm-generic/dma-mapping-broken.h |   82 ++++++++++++++++++++++++++----
 include/asm-h8300/dma-mapping.h          |    1 
 include/asm-m32r/dma-mapping.h           |    6 --
 include/asm-s390/dma-mapping.h           |   12 ----
 include/linux/dma-mapping.h              |    4 +
 10 files changed, 84 insertions(+), 34 deletions(-)

diff --git a/arch/arm26/Kconfig b/arch/arm26/Kconfig
index 20688bc..9044f33 100644
--- a/arch/arm26/Kconfig
+++ b/arch/arm26/Kconfig
@@ -17,6 +17,9 @@ config MMU
 	bool
 	default y
 
+config NO_DMA
+	def_bool y
+
 config ARCH_ACORN
         bool
         default y
diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig
index 618dbad..e35f74e 100644
--- a/arch/h8300/Kconfig
+++ b/arch/h8300/Kconfig
@@ -68,6 +68,9 @@ config TIME_LOW_RES
 config NO_IOPORT
 	def_bool y
 
+config NO_DMA
+	def_bool y
+
 config ISA
 	bool
 	default y
diff --git a/arch/m32r/Kconfig b/arch/m32r/Kconfig
index c3bb8a7..8ccf3e4 100644
--- a/arch/m32r/Kconfig
+++ b/arch/m32r/Kconfig
@@ -31,6 +31,9 @@ config GENERIC_IRQ_PROBE
 config NO_IOPORT
 	def_bool y
 
+config NO_DMA
+	def_bool y
+
 source "init/Kconfig"
 
 
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 72be6c6..59305bc 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -3,7 +3,7 @@
 #
 
 menu "DMA Engine support"
-	depends on !S390
+	depends on HAS_DMA
 
 config DMA_ENGINE
 	bool "Support for DMA engines"
diff --git a/include/asm-arm26/dma-mapping.h b/include/asm-arm26/dma-mapping.h
deleted file mode 100644
index a95eae0..0000000
--- a/include/asm-arm26/dma-mapping.h
+++ /dev/null
@@ -1,2 +0,0 @@
-#include <asm-generic/dma-mapping-broken.h>
-
diff --git a/include/asm-generic/dma-mapping-broken.h b/include/asm-generic/dma-mapping-broken.h
index 29413d3..e2468f8 100644
--- a/include/asm-generic/dma-mapping-broken.h
+++ b/include/asm-generic/dma-mapping-broken.h
@@ -1,24 +1,82 @@
 #ifndef _ASM_GENERIC_DMA_MAPPING_H
 #define _ASM_GENERIC_DMA_MAPPING_H
 
-/* This is used for archs that do not support DMA */
+/* define the dma api to allow compilation but not linking of
+ * dma dependent code.  Code that depends on the dma-mapping
+ * API needs to set 'depends on HAS_DMA' in its Kconfig
+ */
 
-static inline void *
+struct scatterlist;
+
+extern void *
 dma_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_handle,
-		   gfp_t flag)
-{
-	BUG();
-	return NULL;
-}
+		   gfp_t flag);
 
-static inline void
+extern void
 dma_free_coherent(struct device *dev, size_t size, void *cpu_addr,
-		    dma_addr_t dma_handle)
-{
-	BUG();
-}
+		    dma_addr_t dma_handle);
 
 #define dma_alloc_noncoherent(d, s, h, f) dma_alloc_coherent(d, s, h, f)
 #define dma_free_noncoherent(d, s, v, h) dma_free_coherent(d, s, v, h)
 
+extern dma_addr_t
+dma_map_single(struct device *dev, void *ptr, size_t size,
+	       enum dma_data_direction direction);
+
+extern void
+dma_unmap_single(struct device *dev, dma_addr_t dma_addr, size_t size,
+		 enum dma_data_direction direction);
+
+extern int
+dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
+	   enum dma_data_direction direction);
+
+extern void
+dma_unmap_sg(struct device *dev, struct scatterlist *sg, int nhwentries,
+	     enum dma_data_direction direction);
+
+extern dma_addr_t
+dma_map_page(struct device *dev, struct page *page, unsigned long offset,
+	     size_t size, enum dma_data_direction direction);
+
+extern void
+dma_unmap_page(struct device *dev, dma_addr_t dma_address, size_t size,
+	       enum dma_data_direction direction);
+
+extern void
+dma_sync_single_for_cpu(struct device *dev, dma_addr_t dma_handle, size_t size,
+			enum dma_data_direction direction);
+
+extern void
+dma_sync_single_range_for_cpu(struct device *dev, dma_addr_t dma_handle,
+			      unsigned long offset, size_t size,
+			      enum dma_data_direction direction);
+
+extern void
+dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg, int nelems,
+		    enum dma_data_direction direction);
+
+#define dma_sync_single_for_device dma_sync_single_for_cpu
+#define dma_sync_single_range_for_device dma_sync_single_range_for_cpu
+#define dma_sync_sg_for_device dma_sync_sg_for_cpu
+
+extern int
+dma_mapping_error(dma_addr_t dma_addr);
+
+extern int
+dma_supported(struct device *dev, u64 mask);
+
+extern int
+dma_set_mask(struct device *dev, u64 mask);
+
+extern int
+dma_get_cache_alignment(void);
+
+extern int
+dma_is_consistent(struct device *dev, dma_addr_t dma_handle);
+
+extern void
+dma_cache_sync(struct device *dev, void *vaddr, size_t size,
+	       enum dma_data_direction direction);
+
 #endif /* _ASM_GENERIC_DMA_MAPPING_H */
diff --git a/include/asm-h8300/dma-mapping.h b/include/asm-h8300/dma-mapping.h
deleted file mode 100644
index d00e400..0000000
--- a/include/asm-h8300/dma-mapping.h
+++ /dev/null
@@ -1 +0,0 @@
-#include <asm-generic/dma-mapping-broken.h>
diff --git a/include/asm-m32r/dma-mapping.h b/include/asm-m32r/dma-mapping.h
deleted file mode 100644
index f9b58eb..0000000
--- a/include/asm-m32r/dma-mapping.h
+++ /dev/null
@@ -1,6 +0,0 @@
-#ifndef _ASM_M32R_DMA_MAPPING_H
-#define _ASM_M32R_DMA_MAPPING_H
-
-#include <asm-generic/dma-mapping-broken.h>
-
-#endif /* _ASM_M32R_DMA_MAPPING_H */
diff --git a/include/asm-s390/dma-mapping.h b/include/asm-s390/dma-mapping.h
deleted file mode 100644
index 3f8c12f..0000000
--- a/include/asm-s390/dma-mapping.h
+++ /dev/null
@@ -1,12 +0,0 @@
-/*
- *  include/asm-s390/dma-mapping.h
- *
- *  S390 version
- *
- *  This file exists so that #include <dma-mapping.h> doesn't break anything.
- */
-
-#ifndef _ASM_DMA_MAPPING_H
-#define _ASM_DMA_MAPPING_H
-
-#endif /* _ASM_DMA_MAPPING_H */
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 9a663c6..2dc21cb 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -31,7 +31,11 @@ static inline int valid_dma_direction(int dma_direction)
 		(dma_direction == DMA_FROM_DEVICE));
 }
 
+#ifdef CONFIG_HAS_DMA
 #include <asm/dma-mapping.h>
+#else
+#include <asm-generic/dma-mapping-broken.h>
+#endif
 
 /* Backwards compat, remove in 2.7.x */
 #define dma_sync_single		dma_sync_single_for_cpu

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [patch] Introduce CONFIG_HAS_DMA.
  2007-05-30 21:40               ` Williams, Dan J
  2007-05-30 22:29                 ` [PATCH] dma-mapping: prevent dma dependent code from linking on !HAS_DMA archs Dan Williams
@ 2007-05-31  6:53                 ` Cornelia Huck
  1 sibling, 0 replies; 12+ messages in thread
From: Cornelia Huck @ 2007-05-31  6:53 UTC (permalink / raw)
  To: Williams, Dan J
  Cc: Heiko Carstens, Andrew Morton, John W. Linville, Kyle McMartin,
	linux-kernel, James.Bottomley, Tejun Heo, Jeff Garzik,
	Martin Schwidefsky, geert, zippel, spyro, uclinux-v850, ysato

On Wed, 30 May 2007 14:40:02 -0700,
"Williams, Dan J" <dan.j.williams@intel.com> wrote:

> With the patch non-dma-architectures that try to build code with true
> dependencies on the DMA api will fail to link i.e.:
> 
> CONFIG_DMA_ENGINE=y CONFIG_HAS_DMA=n ASYNC_MEMCPY=y
>   CC      init/version.o
>   LD      init/built-in.o
>   LD      .tmp_vmlinux1
> async_tx/built-in.o: In function `async_memcpy':
> xor.c:(.text+0x770): undefined reference to `dma_map_page'
> xor.c:(.text+0x798): undefined reference to `dma_map_page'
> xor.c:(.text+0x968): undefined reference to `dma_map_page'
> 
> Now changing CONFIG_DMA_ENGINE to depend on HAS_DMA (which is more
> correct than saying !S390). 

Makes sense.

> Results in:
> CONFIG_DMA_ENGINE=n CONFIG_HAS_DMA=n ASYNC_MEMCPY=y
>   CC      init/version.o
>   LD      init/built-in.o
>   LD      .tmp_vmlinux1
>   KSYM    .tmp_kallsyms1.S
>   AS      .tmp_kallsyms1.o
>   LD      .tmp_vmlinux2
>   KSYM    .tmp_kallsyms2.S
>   AS      .tmp_kallsyms2.o
>   LD      vmlinux
> 
> This also allows include/asm-s390/dma-mapping.h to be dropped.

Cool. Thanks for looking into that.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2007-05-31  6:53 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-02-23  0:59 [PATCH] dma-mapping-broken.h: flesh-out DMA API stubs John W. Linville
2007-02-23  3:42 ` Heiko Carstens
2007-02-23  6:13   ` Heiko Carstens
2007-02-23  9:50     ` Heiko Carstens
2007-02-23 13:57     ` Kyle McMartin
2007-02-23 14:39       ` John W. Linville
2007-02-26 13:57         ` [patch] Introduce CONFIG_HAS_DMA Heiko Carstens
2007-05-25 16:36           ` Dan Williams
2007-05-30  9:10             ` Cornelia Huck
2007-05-30 21:40               ` Williams, Dan J
2007-05-30 22:29                 ` [PATCH] dma-mapping: prevent dma dependent code from linking on !HAS_DMA archs Dan Williams
2007-05-31  6:53                 ` [patch] Introduce CONFIG_HAS_DMA Cornelia Huck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.