[PATCH 0/4] POWERPC: 32/64-bit DMA code merge and cleanup

[PATCH 0/4] POWERPC: 32/64-bit DMA code merge and cleanup

[Becky Bruce]

This patch series merges the 32 and 64-bit dma code and does some
other cleanup.  This has changed a bit since the RFC was submitted
a few months ago - I incorporated most of the comments from Christoph,
including moving the iommu-specific code into its own file and dropping
numa_node from archdata - we now use the version in the struct
device.

Now that the 32-bit code has a dma_ops structure that is used to
access the dma functions, we will be more easily able to implement
swiotlb or iommu-type dma by setting dma_ops up to use an alternate
set of operations.

I have tested this code on 8641 and 8572 platforms using the popular
kernel build as well as full runs of LTP.  I have built 64-bit
platforms, but have no means to test there, so I would greatly
appreciate any feedback you can provide on possible issues. 

Cheers,
Becky

[PATCH 1/4] POWERPC: Rename dma_64.c to dma.c

[Becky Bruce]

This is in preparation for the merge of the 32 and 64-bit
dma code in arch/powerpc.

Signed-off-by: Becky Bruce <becky.bruce@freescale.com>
---
 arch/powerpc/kernel/Makefile |    2 +-
 arch/powerpc/kernel/dma.c    |  200 ++++++++++++++++++++++++++++++++++++++++++
 arch/powerpc/kernel/dma_64.c |  200 ------------------------------------------
 3 files changed, 201 insertions(+), 201 deletions(-)
 create mode 100644 arch/powerpc/kernel/dma.c
 delete mode 100644 arch/powerpc/kernel/dma_64.c

diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index f17b579..a8a6724 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -71,7 +71,7 @@ obj-y				+= time.o prom.o traps.o setup-common.o \
 				   udbg.o misc.o io.o \
 				   misc_$(CONFIG_WORD_SIZE).o
 obj-$(CONFIG_PPC32)		+= entry_32.o setup_32.o
-obj-$(CONFIG_PPC64)		+= dma_64.o iommu.o
+obj-$(CONFIG_PPC64)		+= dma.o iommu.o
 obj-$(CONFIG_KGDB)		+= kgdb.o
 obj-$(CONFIG_PPC_MULTIPLATFORM)	+= prom_init.o
 obj-$(CONFIG_MODULES)		+= ppc_ksyms.o
diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
new file mode 100644
index 0000000..ae5708e
--- /dev/null
+++ b/arch/powerpc/kernel/dma.c
@@ -0,0 +1,200 @@
+/*
+ * Copyright (C) 2006 Benjamin Herrenschmidt, IBM Corporation
+ *
+ * Provide default implementations of the DMA mapping callbacks for
+ * directly mapped busses and busses using the iommu infrastructure
+ */
+
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <asm/bug.h>
+#include <asm/iommu.h>
+#include <asm/abs_addr.h>
+
+/*
+ * Generic iommu implementation
+ */
+
+/* Allocates a contiguous real buffer and creates mappings over it.
+ * Returns the virtual address of the buffer and sets dma_handle
+ * to the dma address (mapping) of the first page.
+ */
+static void *dma_iommu_alloc_coherent(struct device *dev, size_t size,
+				      dma_addr_t *dma_handle, gfp_t flag)
+{
+	return iommu_alloc_coherent(dev, dev->archdata.dma_data, size,
+				    dma_handle, device_to_mask(dev), flag,
+				    dev->archdata.numa_node);
+}
+
+static void dma_iommu_free_coherent(struct device *dev, size_t size,
+				    void *vaddr, dma_addr_t dma_handle)
+{
+	iommu_free_coherent(dev->archdata.dma_data, size, vaddr, dma_handle);
+}
+
+/* Creates TCEs for a user provided buffer.  The user buffer must be
+ * contiguous real kernel storage (not vmalloc).  The address of the buffer
+ * passed here is the kernel (virtual) address of the buffer.  The buffer
+ * need not be page aligned, the dma_addr_t returned will point to the same
+ * byte within the page as vaddr.
+ */
+static dma_addr_t dma_iommu_map_single(struct device *dev, void *vaddr,
+				       size_t size,
+				       enum dma_data_direction direction,
+				       struct dma_attrs *attrs)
+{
+	return iommu_map_single(dev, dev->archdata.dma_data, vaddr, size,
+				device_to_mask(dev), direction, attrs);
+}
+
+
+static void dma_iommu_unmap_single(struct device *dev, dma_addr_t dma_handle,
+				   size_t size,
+				   enum dma_data_direction direction,
+				   struct dma_attrs *attrs)
+{
+	iommu_unmap_single(dev->archdata.dma_data, dma_handle, size, direction,
+			   attrs);
+}
+
+
+static int dma_iommu_map_sg(struct device *dev, struct scatterlist *sglist,
+			    int nelems, enum dma_data_direction direction,
+			    struct dma_attrs *attrs)
+{
+	return iommu_map_sg(dev, dev->archdata.dma_data, sglist, nelems,
+			    device_to_mask(dev), direction, attrs);
+}
+
+static void dma_iommu_unmap_sg(struct device *dev, struct scatterlist *sglist,
+		int nelems, enum dma_data_direction direction,
+		struct dma_attrs *attrs)
+{
+	iommu_unmap_sg(dev->archdata.dma_data, sglist, nelems, direction,
+		       attrs);
+}
+
+/* We support DMA to/from any memory page via the iommu */
+static int dma_iommu_dma_supported(struct device *dev, u64 mask)
+{
+	struct iommu_table *tbl = dev->archdata.dma_data;
+
+	if (!tbl || tbl->it_offset > mask) {
+		printk(KERN_INFO
+		       "Warning: IOMMU offset too big for device mask\n");
+		if (tbl)
+			printk(KERN_INFO
+			       "mask: 0x%08lx, table offset: 0x%08lx\n",
+				mask, tbl->it_offset);
+		else
+			printk(KERN_INFO "mask: 0x%08lx, table unavailable\n",
+				mask);
+		return 0;
+	} else
+		return 1;
+}
+
+struct dma_mapping_ops dma_iommu_ops = {
+	.alloc_coherent	= dma_iommu_alloc_coherent,
+	.free_coherent	= dma_iommu_free_coherent,
+	.map_single	= dma_iommu_map_single,
+	.unmap_single	= dma_iommu_unmap_single,
+	.map_sg		= dma_iommu_map_sg,
+	.unmap_sg	= dma_iommu_unmap_sg,
+	.dma_supported	= dma_iommu_dma_supported,
+};
+EXPORT_SYMBOL(dma_iommu_ops);
+
+/*
+ * Generic direct DMA implementation
+ *
+ * This implementation supports a per-device offset that can be applied if
+ * the address at which memory is visible to devices is not 0. Platform code
+ * can set archdata.dma_data to an unsigned long holding the offset. By
+ * default the offset is zero.
+ */
+
+static unsigned long get_dma_direct_offset(struct device *dev)
+{
+	return (unsigned long)dev->archdata.dma_data;
+}
+
+static void *dma_direct_alloc_coherent(struct device *dev, size_t size,
+				       dma_addr_t *dma_handle, gfp_t flag)
+{
+	struct page *page;
+	void *ret;
+	int node = dev->archdata.numa_node;
+
+	page = alloc_pages_node(node, flag, get_order(size));
+	if (page == NULL)
+		return NULL;
+	ret = page_address(page);
+	memset(ret, 0, size);
+	*dma_handle = virt_to_abs(ret) + get_dma_direct_offset(dev);
+
+	return ret;
+}
+
+static void dma_direct_free_coherent(struct device *dev, size_t size,
+				     void *vaddr, dma_addr_t dma_handle)
+{
+	free_pages((unsigned long)vaddr, get_order(size));
+}
+
+static dma_addr_t dma_direct_map_single(struct device *dev, void *ptr,
+					size_t size,
+					enum dma_data_direction direction,
+					struct dma_attrs *attrs)
+{
+	return virt_to_abs(ptr) + get_dma_direct_offset(dev);
+}
+
+static void dma_direct_unmap_single(struct device *dev, dma_addr_t dma_addr,
+				    size_t size,
+				    enum dma_data_direction direction,
+				    struct dma_attrs *attrs)
+{
+}
+
+static int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl,
+			     int nents, enum dma_data_direction direction,
+			     struct dma_attrs *attrs)
+{
+	struct scatterlist *sg;
+	int i;
+
+	for_each_sg(sgl, sg, nents, i) {
+		sg->dma_address = sg_phys(sg) + get_dma_direct_offset(dev);
+		sg->dma_length = sg->length;
+	}
+
+	return nents;
+}
+
+static void dma_direct_unmap_sg(struct device *dev, struct scatterlist *sg,
+				int nents, enum dma_data_direction direction,
+				struct dma_attrs *attrs)
+{
+}
+
+static int dma_direct_dma_supported(struct device *dev, u64 mask)
+{
+	/* Could be improved to check for memory though it better be
+	 * done via some global so platforms can set the limit in case
+	 * they have limited DMA windows
+	 */
+	return mask >= DMA_32BIT_MASK;
+}
+
+struct dma_mapping_ops dma_direct_ops = {
+	.alloc_coherent	= dma_direct_alloc_coherent,
+	.free_coherent	= dma_direct_free_coherent,
+	.map_single	= dma_direct_map_single,
+	.unmap_single	= dma_direct_unmap_single,
+	.map_sg		= dma_direct_map_sg,
+	.unmap_sg	= dma_direct_unmap_sg,
+	.dma_supported	= dma_direct_dma_supported,
+};
+EXPORT_SYMBOL(dma_direct_ops);
diff --git a/arch/powerpc/kernel/dma_64.c b/arch/powerpc/kernel/dma_64.c
deleted file mode 100644
index ae5708e..0000000
--- a/arch/powerpc/kernel/dma_64.c
+++ /dev/null
@@ -1,200 +0,0 @@
-/*
- * Copyright (C) 2006 Benjamin Herrenschmidt, IBM Corporation
- *
- * Provide default implementations of the DMA mapping callbacks for
- * directly mapped busses and busses using the iommu infrastructure
- */
-
-#include <linux/device.h>
-#include <linux/dma-mapping.h>
-#include <asm/bug.h>
-#include <asm/iommu.h>
-#include <asm/abs_addr.h>
-
-/*
- * Generic iommu implementation
- */
-
-/* Allocates a contiguous real buffer and creates mappings over it.
- * Returns the virtual address of the buffer and sets dma_handle
- * to the dma address (mapping) of the first page.
- */
-static void *dma_iommu_alloc_coherent(struct device *dev, size_t size,
-				      dma_addr_t *dma_handle, gfp_t flag)
-{
-	return iommu_alloc_coherent(dev, dev->archdata.dma_data, size,
-				    dma_handle, device_to_mask(dev), flag,
-				    dev->archdata.numa_node);
-}
-
-static void dma_iommu_free_coherent(struct device *dev, size_t size,
-				    void *vaddr, dma_addr_t dma_handle)
-{
-	iommu_free_coherent(dev->archdata.dma_data, size, vaddr, dma_handle);
-}
-
-/* Creates TCEs for a user provided buffer.  The user buffer must be
- * contiguous real kernel storage (not vmalloc).  The address of the buffer
- * passed here is the kernel (virtual) address of the buffer.  The buffer
- * need not be page aligned, the dma_addr_t returned will point to the same
- * byte within the page as vaddr.
- */
-static dma_addr_t dma_iommu_map_single(struct device *dev, void *vaddr,
-				       size_t size,
-				       enum dma_data_direction direction,
-				       struct dma_attrs *attrs)
-{
-	return iommu_map_single(dev, dev->archdata.dma_data, vaddr, size,
-				device_to_mask(dev), direction, attrs);
-}
-
-
-static void dma_iommu_unmap_single(struct device *dev, dma_addr_t dma_handle,
-				   size_t size,
-				   enum dma_data_direction direction,
-				   struct dma_attrs *attrs)
-{
-	iommu_unmap_single(dev->archdata.dma_data, dma_handle, size, direction,
-			   attrs);
-}
-
-
-static int dma_iommu_map_sg(struct device *dev, struct scatterlist *sglist,
-			    int nelems, enum dma_data_direction direction,
-			    struct dma_attrs *attrs)
-{
-	return iommu_map_sg(dev, dev->archdata.dma_data, sglist, nelems,
-			    device_to_mask(dev), direction, attrs);
-}
-
-static void dma_iommu_unmap_sg(struct device *dev, struct scatterlist *sglist,
-		int nelems, enum dma_data_direction direction,
-		struct dma_attrs *attrs)
-{
-	iommu_unmap_sg(dev->archdata.dma_data, sglist, nelems, direction,
-		       attrs);
-}
-
-/* We support DMA to/from any memory page via the iommu */
-static int dma_iommu_dma_supported(struct device *dev, u64 mask)
-{
-	struct iommu_table *tbl = dev->archdata.dma_data;
-
-	if (!tbl || tbl->it_offset > mask) {
-		printk(KERN_INFO
-		       "Warning: IOMMU offset too big for device mask\n");
-		if (tbl)
-			printk(KERN_INFO
-			       "mask: 0x%08lx, table offset: 0x%08lx\n",
-				mask, tbl->it_offset);
-		else
-			printk(KERN_INFO "mask: 0x%08lx, table unavailable\n",
-				mask);
-		return 0;
-	} else
-		return 1;
-}
-
-struct dma_mapping_ops dma_iommu_ops = {
-	.alloc_coherent	= dma_iommu_alloc_coherent,
-	.free_coherent	= dma_iommu_free_coherent,
-	.map_single	= dma_iommu_map_single,
-	.unmap_single	= dma_iommu_unmap_single,
-	.map_sg		= dma_iommu_map_sg,
-	.unmap_sg	= dma_iommu_unmap_sg,
-	.dma_supported	= dma_iommu_dma_supported,
-};
-EXPORT_SYMBOL(dma_iommu_ops);
-
-/*
- * Generic direct DMA implementation
- *
- * This implementation supports a per-device offset that can be applied if
- * the address at which memory is visible to devices is not 0. Platform code
- * can set archdata.dma_data to an unsigned long holding the offset. By
- * default the offset is zero.
- */
-
-static unsigned long get_dma_direct_offset(struct device *dev)
-{
-	return (unsigned long)dev->archdata.dma_data;
-}
-
-static void *dma_direct_alloc_coherent(struct device *dev, size_t size,
-				       dma_addr_t *dma_handle, gfp_t flag)
-{
-	struct page *page;
-	void *ret;
-	int node = dev->archdata.numa_node;
-
-	page = alloc_pages_node(node, flag, get_order(size));
-	if (page == NULL)
-		return NULL;
-	ret = page_address(page);
-	memset(ret, 0, size);
-	*dma_handle = virt_to_abs(ret) + get_dma_direct_offset(dev);
-
-	return ret;
-}
-
-static void dma_direct_free_coherent(struct device *dev, size_t size,
-				     void *vaddr, dma_addr_t dma_handle)
-{
-	free_pages((unsigned long)vaddr, get_order(size));
-}
-
-static dma_addr_t dma_direct_map_single(struct device *dev, void *ptr,
-					size_t size,
-					enum dma_data_direction direction,
-					struct dma_attrs *attrs)
-{
-	return virt_to_abs(ptr) + get_dma_direct_offset(dev);
-}
-
-static void dma_direct_unmap_single(struct device *dev, dma_addr_t dma_addr,
-				    size_t size,
-				    enum dma_data_direction direction,
-				    struct dma_attrs *attrs)
-{
-}
-
-static int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl,
-			     int nents, enum dma_data_direction direction,
-			     struct dma_attrs *attrs)
-{
-	struct scatterlist *sg;
-	int i;
-
-	for_each_sg(sgl, sg, nents, i) {
-		sg->dma_address = sg_phys(sg) + get_dma_direct_offset(dev);
-		sg->dma_length = sg->length;
-	}
-
-	return nents;
-}
-
-static void dma_direct_unmap_sg(struct device *dev, struct scatterlist *sg,
-				int nents, enum dma_data_direction direction,
-				struct dma_attrs *attrs)
-{
-}
-
-static int dma_direct_dma_supported(struct device *dev, u64 mask)
-{
-	/* Could be improved to check for memory though it better be
-	 * done via some global so platforms can set the limit in case
-	 * they have limited DMA windows
-	 */
-	return mask >= DMA_32BIT_MASK;
-}
-
-struct dma_mapping_ops dma_direct_ops = {
-	.alloc_coherent	= dma_direct_alloc_coherent,
-	.free_coherent	= dma_direct_free_coherent,
-	.map_single	= dma_direct_map_single,
-	.unmap_single	= dma_direct_unmap_single,
-	.map_sg		= dma_direct_map_sg,
-	.unmap_sg	= dma_direct_unmap_sg,
-	.dma_supported	= dma_direct_dma_supported,
-};
-EXPORT_SYMBOL(dma_direct_ops);
-- 
1.5.5.1

[PATCH 2/4] POWERPC: Move iommu dma ops from dma.c to dma-iommu.c

[Becky Bruce]

32-bit platforms are about to start using dma.c; move the iommu
dma ops into their own file to make this a bit cleaner.

Signed-off-by: Becky Bruce <becky.bruce@freescale.com>
---
 arch/powerpc/kernel/Makefile    |    2 +-
 arch/powerpc/kernel/dma-iommu.c |  103 +++++++++++++++++++++++++++++++++++++++
 arch/powerpc/kernel/dma.c       |   98 +------------------------------------
 3 files changed, 105 insertions(+), 98 deletions(-)
 create mode 100644 arch/powerpc/kernel/dma-iommu.c

diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index a8a6724..45570fe 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -71,7 +71,7 @@ obj-y				+= time.o prom.o traps.o setup-common.o \
 				   udbg.o misc.o io.o \
 				   misc_$(CONFIG_WORD_SIZE).o
 obj-$(CONFIG_PPC32)		+= entry_32.o setup_32.o
-obj-$(CONFIG_PPC64)		+= dma.o iommu.o
+obj-$(CONFIG_PPC64)		+= dma.o dma-iommu.o iommu.o
 obj-$(CONFIG_KGDB)		+= kgdb.o
 obj-$(CONFIG_PPC_MULTIPLATFORM)	+= prom_init.o
 obj-$(CONFIG_MODULES)		+= ppc_ksyms.o
diff --git a/arch/powerpc/kernel/dma-iommu.c b/arch/powerpc/kernel/dma-iommu.c
new file mode 100644
index 0000000..01091f1
--- /dev/null
+++ b/arch/powerpc/kernel/dma-iommu.c
@@ -0,0 +1,103 @@
+/*
+ * Copyright (C) 2006 Benjamin Herrenschmidt, IBM Corporation
+ *
+ * Provide default implementations of the DMA mapping callbacks for
+ * busses using the iommu infrastructure
+ */
+
+#include <asm/iommu.h>
+
+/*
+ * Generic iommu implementation
+ */
+
+/* Allocates a contiguous real buffer and creates mappings over it.
+ * Returns the virtual address of the buffer and sets dma_handle
+ * to the dma address (mapping) of the first page.
+ */
+static void *dma_iommu_alloc_coherent(struct device *dev, size_t size,
+				      dma_addr_t *dma_handle, gfp_t flag)
+{
+	return iommu_alloc_coherent(dev, dev->archdata.dma_data, size,
+				    dma_handle, device_to_mask(dev), flag,
+				    dev->archdata.numa_node);
+}
+
+static void dma_iommu_free_coherent(struct device *dev, size_t size,
+				    void *vaddr, dma_addr_t dma_handle)
+{
+	iommu_free_coherent(dev->archdata.dma_data, size, vaddr, dma_handle);
+}
+
+/* Creates TCEs for a user provided buffer.  The user buffer must be
+ * contiguous real kernel storage (not vmalloc).  The address of the buffer
+ * passed here is the kernel (virtual) address of the buffer.  The buffer
+ * need not be page aligned, the dma_addr_t returned will point to the same
+ * byte within the page as vaddr.
+ */
+static dma_addr_t dma_iommu_map_single(struct device *dev, void *vaddr,
+				       size_t size,
+				       enum dma_data_direction direction,
+				       struct dma_attrs *attrs)
+{
+	return iommu_map_single(dev, dev->archdata.dma_data, vaddr, size,
+				device_to_mask(dev), direction, attrs);
+}
+
+
+static void dma_iommu_unmap_single(struct device *dev, dma_addr_t dma_handle,
+				   size_t size,
+				   enum dma_data_direction direction,
+				   struct dma_attrs *attrs)
+{
+	iommu_unmap_single(dev->archdata.dma_data, dma_handle, size, direction,
+			   attrs);
+}
+
+
+static int dma_iommu_map_sg(struct device *dev, struct scatterlist *sglist,
+			    int nelems, enum dma_data_direction direction,
+			    struct dma_attrs *attrs)
+{
+	return iommu_map_sg(dev, dev->archdata.dma_data, sglist, nelems,
+			    device_to_mask(dev), direction, attrs);
+}
+
+static void dma_iommu_unmap_sg(struct device *dev, struct scatterlist *sglist,
+		int nelems, enum dma_data_direction direction,
+		struct dma_attrs *attrs)
+{
+	iommu_unmap_sg(dev->archdata.dma_data, sglist, nelems, direction,
+		       attrs);
+}
+
+/* We support DMA to/from any memory page via the iommu */
+static int dma_iommu_dma_supported(struct device *dev, u64 mask)
+{
+	struct iommu_table *tbl = dev->archdata.dma_data;
+
+	if (!tbl || tbl->it_offset > mask) {
+		printk(KERN_INFO
+		       "Warning: IOMMU offset too big for device mask\n");
+		if (tbl)
+			printk(KERN_INFO
+			       "mask: 0x%08lx, table offset: 0x%08lx\n",
+				mask, tbl->it_offset);
+		else
+			printk(KERN_INFO "mask: 0x%08lx, table unavailable\n",
+				mask);
+		return 0;
+	} else
+		return 1;
+}
+
+struct dma_mapping_ops dma_iommu_ops = {
+	.alloc_coherent	= dma_iommu_alloc_coherent,
+	.free_coherent	= dma_iommu_free_coherent,
+	.map_single	= dma_iommu_map_single,
+	.unmap_single	= dma_iommu_unmap_single,
+	.map_sg		= dma_iommu_map_sg,
+	.unmap_sg	= dma_iommu_unmap_sg,
+	.dma_supported	= dma_iommu_dma_supported,
+};
+EXPORT_SYMBOL(dma_iommu_ops);
diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
index ae5708e..44e3486 100644
--- a/arch/powerpc/kernel/dma.c
+++ b/arch/powerpc/kernel/dma.c
@@ -2,111 +2,15 @@
  * Copyright (C) 2006 Benjamin Herrenschmidt, IBM Corporation
  *
  * Provide default implementations of the DMA mapping callbacks for
- * directly mapped busses and busses using the iommu infrastructure
+ * directly mapped busses.
  */
 
 #include <linux/device.h>
 #include <linux/dma-mapping.h>
 #include <asm/bug.h>
-#include <asm/iommu.h>
 #include <asm/abs_addr.h>
 
 /*
- * Generic iommu implementation
- */
-
-/* Allocates a contiguous real buffer and creates mappings over it.
- * Returns the virtual address of the buffer and sets dma_handle
- * to the dma address (mapping) of the first page.
- */
-static void *dma_iommu_alloc_coherent(struct device *dev, size_t size,
-				      dma_addr_t *dma_handle, gfp_t flag)
-{
-	return iommu_alloc_coherent(dev, dev->archdata.dma_data, size,
-				    dma_handle, device_to_mask(dev), flag,
-				    dev->archdata.numa_node);
-}
-
-static void dma_iommu_free_coherent(struct device *dev, size_t size,
-				    void *vaddr, dma_addr_t dma_handle)
-{
-	iommu_free_coherent(dev->archdata.dma_data, size, vaddr, dma_handle);
-}
-
-/* Creates TCEs for a user provided buffer.  The user buffer must be
- * contiguous real kernel storage (not vmalloc).  The address of the buffer
- * passed here is the kernel (virtual) address of the buffer.  The buffer
- * need not be page aligned, the dma_addr_t returned will point to the same
- * byte within the page as vaddr.
- */
-static dma_addr_t dma_iommu_map_single(struct device *dev, void *vaddr,
-				       size_t size,
-				       enum dma_data_direction direction,
-				       struct dma_attrs *attrs)
-{
-	return iommu_map_single(dev, dev->archdata.dma_data, vaddr, size,
-				device_to_mask(dev), direction, attrs);
-}
-
-
-static void dma_iommu_unmap_single(struct device *dev, dma_addr_t dma_handle,
-				   size_t size,
-				   enum dma_data_direction direction,
-				   struct dma_attrs *attrs)
-{
-	iommu_unmap_single(dev->archdata.dma_data, dma_handle, size, direction,
-			   attrs);
-}
-
-
-static int dma_iommu_map_sg(struct device *dev, struct scatterlist *sglist,
-			    int nelems, enum dma_data_direction direction,
-			    struct dma_attrs *attrs)
-{
-	return iommu_map_sg(dev, dev->archdata.dma_data, sglist, nelems,
-			    device_to_mask(dev), direction, attrs);
-}
-
-static void dma_iommu_unmap_sg(struct device *dev, struct scatterlist *sglist,
-		int nelems, enum dma_data_direction direction,
-		struct dma_attrs *attrs)
-{
-	iommu_unmap_sg(dev->archdata.dma_data, sglist, nelems, direction,
-		       attrs);
-}
-
-/* We support DMA to/from any memory page via the iommu */
-static int dma_iommu_dma_supported(struct device *dev, u64 mask)
-{
-	struct iommu_table *tbl = dev->archdata.dma_data;
-
-	if (!tbl || tbl->it_offset > mask) {
-		printk(KERN_INFO
-		       "Warning: IOMMU offset too big for device mask\n");
-		if (tbl)
-			printk(KERN_INFO
-			       "mask: 0x%08lx, table offset: 0x%08lx\n",
-				mask, tbl->it_offset);
-		else
-			printk(KERN_INFO "mask: 0x%08lx, table unavailable\n",
-				mask);
-		return 0;
-	} else
-		return 1;
-}
-
-struct dma_mapping_ops dma_iommu_ops = {
-	.alloc_coherent	= dma_iommu_alloc_coherent,
-	.free_coherent	= dma_iommu_free_coherent,
-	.map_single	= dma_iommu_map_single,
-	.unmap_single	= dma_iommu_unmap_single,
-	.map_sg		= dma_iommu_map_sg,
-	.unmap_sg	= dma_iommu_unmap_sg,
-	.dma_supported	= dma_iommu_dma_supported,
-};
-EXPORT_SYMBOL(dma_iommu_ops);
-
-/*
  * Generic direct DMA implementation
  *
  * This implementation supports a per-device offset that can be applied if
-- 
1.5.5.1

[PATCH 3/4] POWERPC: Drop archdata numa_node

[Becky Bruce]

Use the struct device's numa_node instead; use accessor functions
to get/set numa_node.

Signed-off-by: Becky Bruce <becky.bruce@freescale.com>
---
64-bit folks: Please test when you get a moment.  I've build-tested this,
but I do not have the ability to actually boot it on anything.  Yes, I know
this fundamentally doesn't have much relation to the dma merge, except that
doing this saves me some ifdeffing in the code in a later patch.  Also,
since I've renamed dma_64.c to dma.c, I wanted to do this after the rename
so it's easier to see the history in git.

Thanks!
-Becky

 arch/powerpc/include/asm/device.h       |    3 ---
 arch/powerpc/kernel/dma-iommu.c         |    2 +-
 arch/powerpc/kernel/dma.c               |    2 +-
 arch/powerpc/kernel/of_device.c         |    2 +-
 arch/powerpc/kernel/pci_64.c            |    7 ++-----
 arch/powerpc/kernel/vio.c               |    2 +-
 arch/powerpc/platforms/cell/iommu.c     |    6 +++---
 arch/powerpc/platforms/ps3/system-bus.c |    2 +-
 8 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/arch/powerpc/include/asm/device.h b/arch/powerpc/include/asm/device.h
index 228ab2a..dfd504c 100644
--- a/arch/powerpc/include/asm/device.h
+++ b/arch/powerpc/include/asm/device.h
@@ -16,9 +16,6 @@ struct dev_archdata {
 	/* DMA operations on that device */
 	struct dma_mapping_ops	*dma_ops;
 	void			*dma_data;
-
-	/* NUMA node if applicable */
-	int			numa_node;
 };
 
 #endif /* _ASM_POWERPC_DEVICE_H */
diff --git a/arch/powerpc/kernel/dma-iommu.c b/arch/powerpc/kernel/dma-iommu.c
index 01091f1..49248f8 100644
--- a/arch/powerpc/kernel/dma-iommu.c
+++ b/arch/powerpc/kernel/dma-iommu.c
@@ -20,7 +20,7 @@ static void *dma_iommu_alloc_coherent(struct device *dev, size_t size,
 {
 	return iommu_alloc_coherent(dev, dev->archdata.dma_data, size,
 				    dma_handle, device_to_mask(dev), flag,
-				    dev->archdata.numa_node);
+				    dev_to_node(dev));
 }
 
 static void dma_iommu_free_coherent(struct device *dev, size_t size,
diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
index 44e3486..124f867 100644
--- a/arch/powerpc/kernel/dma.c
+++ b/arch/powerpc/kernel/dma.c
@@ -29,7 +29,7 @@ static void *dma_direct_alloc_coherent(struct device *dev, size_t size,
 {
 	struct page *page;
 	void *ret;
-	int node = dev->archdata.numa_node;
+	int node = dev_to_node(dev);
 
 	page = alloc_pages_node(node, flag, get_order(size));
 	if (page == NULL)
diff --git a/arch/powerpc/kernel/of_device.c b/arch/powerpc/kernel/of_device.c
index e9be908..93ae5b1 100644
--- a/arch/powerpc/kernel/of_device.c
+++ b/arch/powerpc/kernel/of_device.c
@@ -78,7 +78,7 @@ struct of_device *of_device_alloc(struct device_node *np,
 	dev->dev.parent = parent;
 	dev->dev.release = of_release_dev;
 	dev->dev.archdata.of_node = np;
-	dev->dev.archdata.numa_node = of_node_to_nid(np);
+	set_dev_node(&dev->dev, of_node_to_nid(np));
 
 	if (bus_id)
 		strlcpy(dev->dev.bus_id, bus_id, BUS_ID_SIZE);
diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index 30eedfc..1f75bf0 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -558,11 +558,8 @@ void __devinit pcibios_setup_new_device(struct pci_dev *dev)
 	    sd->of_node ? sd->of_node->full_name : "<none>");
 
 	sd->dma_ops = pci_dma_ops;
-#ifdef CONFIG_NUMA
-	sd->numa_node = pcibus_to_node(dev->bus);
-#else
-	sd->numa_node = -1;
-#endif
+	set_dev_node(&dev->dev, pcibus_to_node(dev->bus));
+
 	if (ppc_md.pci_dma_dev_setup)
 		ppc_md.pci_dma_dev_setup(dev);
 }
diff --git a/arch/powerpc/kernel/vio.c b/arch/powerpc/kernel/vio.c
index 2750fba..434c92a 100644
--- a/arch/powerpc/kernel/vio.c
+++ b/arch/powerpc/kernel/vio.c
@@ -1232,7 +1232,7 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node)
 	else
 		viodev->dev.archdata.dma_ops = &dma_iommu_ops;
 	viodev->dev.archdata.dma_data = vio_build_iommu_table(viodev);
-	viodev->dev.archdata.numa_node = of_node_to_nid(of_node);
+	set_dev_node(&viodev->dev, of_node_to_nid(of_node));
 
 	/* init generic 'struct device' fields: */
 	viodev->dev.parent = &vio_bus_device.dev;
diff --git a/arch/powerpc/platforms/cell/iommu.c b/arch/powerpc/platforms/cell/iommu.c
index e06420a..ef92e71 100644
--- a/arch/powerpc/platforms/cell/iommu.c
+++ b/arch/powerpc/platforms/cell/iommu.c
@@ -556,11 +556,11 @@ static struct iommu_table *cell_get_iommu_table(struct device *dev)
 	 * node's iommu. We -might- do something smarter later though it may
 	 * never be necessary
 	 */
-	iommu = cell_iommu_for_node(archdata->numa_node);
+	iommu = cell_iommu_for_node(dev_to_node(dev));
 	if (iommu == NULL || list_empty(&iommu->windows)) {
 		printk(KERN_ERR "iommu: missing iommu for %s (node %d)\n",
 		       archdata->of_node ? archdata->of_node->full_name : "?",
-		       archdata->numa_node);
+		       dev_to_node(dev));
 		return NULL;
 	}
 	window = list_entry(iommu->windows.next, struct iommu_window, list);
@@ -577,7 +577,7 @@ static void *dma_fixed_alloc_coherent(struct device *dev, size_t size,
 		return iommu_alloc_coherent(dev, cell_get_iommu_table(dev),
 					    size, dma_handle,
 					    device_to_mask(dev), flag,
-					    dev->archdata.numa_node);
+					    dev_to_node(dev));
 	else
 		return dma_direct_ops.alloc_coherent(dev, size, dma_handle,
 						     flag);
diff --git a/arch/powerpc/platforms/ps3/system-bus.c b/arch/powerpc/platforms/ps3/system-bus.c
index 280ee88..a789bf5 100644
--- a/arch/powerpc/platforms/ps3/system-bus.c
+++ b/arch/powerpc/platforms/ps3/system-bus.c
@@ -762,7 +762,7 @@ int ps3_system_bus_device_register(struct ps3_system_bus_device *dev)
 	};
 
 	dev->core.archdata.of_node = NULL;
-	dev->core.archdata.numa_node = 0;
+	set_dev_node(&dev->core, 0);
 
 	pr_debug("%s:%d add %s\n", __func__, __LINE__, dev->core.bus_id);
 
-- 
1.5.5.1

[PATCH 4/4] POWERPC: Merge 32 and 64-bit dma code

[Becky Bruce]

We essentially adopt the 64-bit dma code, with some changes to support
32-bit systems, including HIGHMEM.  dma functions on 32-bit are now
invoked via accessor functions which call the correct op for a device based
on archdata dma_ops.  If there is no archdata dma_ops, this defaults
to dma_direct_ops.

In addition, the dma_map/unmap_page functions are added to dma_ops
because we can't just fall back on map/unmap_single when HIGHMEM is
enabled. In the case of dma_direct_*, we stop using map/unmap_single
and just use the page version - this saves a lot of ugly
ifdeffing.  We leave map/unmap_single in the dma_ops definition,
though, because they are needed by the iommu code, which does not
implement map/unmap_page.

Signed-off-by: Becky Bruce <becky.bruce@freescale.com>
---
 arch/powerpc/include/asm/dma-mapping.h |  178 +++++++++----------------------
 arch/powerpc/include/asm/machdep.h     |    5 +-
 arch/powerpc/include/asm/pci.h         |   14 ++-
 arch/powerpc/kernel/Makefile           |    4 +-
 arch/powerpc/kernel/dma.c              |   69 ++++++++----
 arch/powerpc/kernel/pci-common.c       |   48 +++++++++
 arch/powerpc/kernel/pci_32.c           |    7 ++
 arch/powerpc/kernel/pci_64.c           |   46 --------
 8 files changed, 166 insertions(+), 205 deletions(-)

diff --git a/arch/powerpc/include/asm/dma-mapping.h b/arch/powerpc/include/asm/dma-mapping.h
index c7ca45f..bd2b2b7 100644
--- a/arch/powerpc/include/asm/dma-mapping.h
+++ b/arch/powerpc/include/asm/dma-mapping.h
@@ -44,8 +44,6 @@ extern void __dma_sync_page(struct page *page, unsigned long offset,
 
 #endif /* ! CONFIG_NOT_COHERENT_CACHE */
 
-#ifdef CONFIG_PPC64
-
 static inline unsigned long device_to_mask(struct device *dev)
 {
 	if (dev->dma_mask && *dev->dma_mask)
@@ -76,8 +74,24 @@ struct dma_mapping_ops {
 				struct dma_attrs *attrs);
 	int		(*dma_supported)(struct device *dev, u64 mask);
 	int		(*set_dma_mask)(struct device *dev, u64 dma_mask);
+	dma_addr_t 	(*map_page)(struct device *dev, struct page *page,
+				unsigned long offset, size_t size,
+				enum dma_data_direction direction,
+				struct dma_attrs *attrs);
+	void		(*unmap_page)(struct device *dev,
+				dma_addr_t dma_address, size_t size,
+				enum dma_data_direction direction,
+				struct dma_attrs *attrs);
 };
 
+/*
+ * Available generic sets of operations
+ */
+#ifdef CONFIG_PPC64
+extern struct dma_mapping_ops dma_iommu_ops;
+#endif
+extern struct dma_mapping_ops dma_direct_ops;
+
 static inline struct dma_mapping_ops *get_dma_ops(struct device *dev)
 {
 	/* We don't handle the NULL dev case for ISA for now. We could
@@ -85,8 +99,16 @@ static inline struct dma_mapping_ops *get_dma_ops(struct device *dev)
 	 * only ISA DMA device we support is the floppy and we have a hack
 	 * in the floppy driver directly to get a device for us.
 	 */
-	if (unlikely(dev == NULL || dev->archdata.dma_ops == NULL))
+
+	if (unlikely(dev == NULL) || dev->archdata.dma_ops == NULL) {
+#ifdef CONFIG_PPC64
 		return NULL;
+#else
+		/* Use default on 32-bit if dma_ops is not set up */
+		return &dma_direct_ops;
+#endif
+	}
+
 	return dev->archdata.dma_ops;
 }
 
@@ -132,7 +154,14 @@ static inline dma_addr_t dma_map_single_attrs(struct device *dev,
 	struct dma_mapping_ops *dma_ops = get_dma_ops(dev);
 
 	BUG_ON(!dma_ops);
-	return dma_ops->map_single(dev, cpu_addr, size, direction, attrs);
+
+	if (dma_ops->map_single)
+		return dma_ops->map_single(dev, cpu_addr, size, direction,
+					   attrs);
+
+	return dma_ops->map_page(dev, virt_to_page(cpu_addr),
+				 (unsigned long)cpu_addr % PAGE_SIZE, size,
+				 direction, attrs);
 }
 
 static inline void dma_unmap_single_attrs(struct device *dev,
@@ -144,7 +173,13 @@ static inline void dma_unmap_single_attrs(struct device *dev,
 	struct dma_mapping_ops *dma_ops = get_dma_ops(dev);
 
 	BUG_ON(!dma_ops);
-	dma_ops->unmap_single(dev, dma_addr, size, direction, attrs);
+
+	if (dma_ops->unmap_single) {
+		dma_ops->unmap_single(dev, dma_addr, size, direction, attrs);
+		return;
+	}
+
+	dma_ops->unmap_page(dev, dma_addr, size, direction, attrs);
 }
 
 static inline dma_addr_t dma_map_page_attrs(struct device *dev,
@@ -156,8 +191,13 @@ static inline dma_addr_t dma_map_page_attrs(struct device *dev,
 	struct dma_mapping_ops *dma_ops = get_dma_ops(dev);
 
 	BUG_ON(!dma_ops);
+
+	if (dma_ops->map_page)
+		return dma_ops->map_page(dev, page, offset, size, direction,
+					 attrs);
+
 	return dma_ops->map_single(dev, page_address(page) + offset, size,
-			direction, attrs);
+				   direction, attrs);
 }
 
 static inline void dma_unmap_page_attrs(struct device *dev,
@@ -169,6 +209,12 @@ static inline void dma_unmap_page_attrs(struct device *dev,
 	struct dma_mapping_ops *dma_ops = get_dma_ops(dev);
 
 	BUG_ON(!dma_ops);
+
+	if (dma_ops->unmap_page) {
+		dma_ops->unmap_page(dev, dma_address, size, direction, attrs);
+		return;
+	}
+
 	dma_ops->unmap_single(dev, dma_address, size, direction, attrs);
 }
 
@@ -253,126 +299,6 @@ static inline void dma_unmap_sg(struct device *dev, struct scatterlist *sg,
 	dma_unmap_sg_attrs(dev, sg, nhwentries, direction, NULL);
 }
 
-/*
- * Available generic sets of operations
- */
-extern struct dma_mapping_ops dma_iommu_ops;
-extern struct dma_mapping_ops dma_direct_ops;
-
-#else /* CONFIG_PPC64 */
-
-#define dma_supported(dev, mask)	(1)
-
-static inline int dma_set_mask(struct device *dev, u64 dma_mask)
-{
-	if (!dev->dma_mask || !dma_supported(dev, mask))
-		return -EIO;
-
-	*dev->dma_mask = dma_mask;
-
-	return 0;
-}
-
-static inline void *dma_alloc_coherent(struct device *dev, size_t size,
-				       dma_addr_t * dma_handle,
-				       gfp_t gfp)
-{
-#ifdef CONFIG_NOT_COHERENT_CACHE
-	return __dma_alloc_coherent(size, dma_handle, gfp);
-#else
-	void *ret;
-	/* ignore region specifiers */
-	gfp &= ~(__GFP_DMA | __GFP_HIGHMEM);
-
-	if (dev == NULL || dev->coherent_dma_mask < 0xffffffff)
-		gfp |= GFP_DMA;
-
-	ret = (void *)__get_free_pages(gfp, get_order(size));
-
-	if (ret != NULL) {
-		memset(ret, 0, size);
-		*dma_handle = virt_to_bus(ret);
-	}
-
-	return ret;
-#endif
-}
-
-static inline void
-dma_free_coherent(struct device *dev, size_t size, void *vaddr,
-		  dma_addr_t dma_handle)
-{
-#ifdef CONFIG_NOT_COHERENT_CACHE
-	__dma_free_coherent(size, vaddr);
-#else
-	free_pages((unsigned long)vaddr, get_order(size));
-#endif
-}
-
-static inline dma_addr_t
-dma_map_single(struct device *dev, void *ptr, size_t size,
-	       enum dma_data_direction direction)
-{
-	BUG_ON(direction == DMA_NONE);
-
-	__dma_sync(ptr, size, direction);
-
-	return virt_to_bus(ptr);
-}
-
-static inline void dma_unmap_single(struct device *dev, dma_addr_t dma_addr,
-				    size_t size,
-				    enum dma_data_direction direction)
-{
-	/* We do nothing. */
-}
-
-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_ON(direction == DMA_NONE);
-
-	__dma_sync_page(page, offset, size, direction);
-
-	return page_to_bus(page) + offset;
-}
-
-static inline void dma_unmap_page(struct device *dev, dma_addr_t dma_address,
-				  size_t size,
-				  enum dma_data_direction direction)
-{
-	/* We do nothing. */
-}
-
-static inline int
-dma_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
-	   enum dma_data_direction direction)
-{
-	struct scatterlist *sg;
-	int i;
-
-	BUG_ON(direction == DMA_NONE);
-
-	for_each_sg(sgl, sg, nents, i) {
-		BUG_ON(!sg_page(sg));
-		__dma_sync_page(sg_page(sg), sg->offset, sg->length, direction);
-		sg->dma_address = page_to_bus(sg_page(sg)) + sg->offset;
-	}
-
-	return nents;
-}
-
-static inline void dma_unmap_sg(struct device *dev, struct scatterlist *sg,
-				int nhwentries,
-				enum dma_data_direction direction)
-{
-	/* We don't do anything here. */
-}
-
-#endif /* CONFIG_PPC64 */
-
 static inline void dma_sync_single_for_cpu(struct device *dev,
 		dma_addr_t dma_handle, size_t size,
 		enum dma_data_direction direction)
diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
index 893aafd..2740c44 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -88,8 +88,6 @@ struct machdep_calls {
 	unsigned long	(*tce_get)(struct iommu_table *tbl,
 				    long index);
 	void		(*tce_flush)(struct iommu_table *tbl);
-	void		(*pci_dma_dev_setup)(struct pci_dev *dev);
-	void		(*pci_dma_bus_setup)(struct pci_bus *bus);
 
 	void __iomem *	(*ioremap)(phys_addr_t addr, unsigned long size,
 				   unsigned long flags);
@@ -101,6 +99,9 @@ struct machdep_calls {
 #endif
 #endif /* CONFIG_PPC64 */
 
+	void		(*pci_dma_dev_setup)(struct pci_dev *dev);
+	void		(*pci_dma_bus_setup)(struct pci_bus *bus);
+
 	int		(*probe)(void);
 	void		(*setup_arch)(void); /* Optional, may be NULL */
 	void		(*init_early)(void);
diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
index a05a942..0e52c78 100644
--- a/arch/powerpc/include/asm/pci.h
+++ b/arch/powerpc/include/asm/pci.h
@@ -60,6 +60,14 @@ static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
 	return channel ? 15 : 14;
 }
 
+#ifdef CONFIG_PCI
+extern void set_pci_dma_ops(struct dma_mapping_ops *dma_ops);
+extern struct dma_mapping_ops *get_pci_dma_ops(void);
+#else	/* CONFIG_PCI */
+#define set_pci_dma_ops(d)
+#define get_pci_dma_ops()	NULL
+#endif
+
 #ifdef CONFIG_PPC64
 
 /*
@@ -70,9 +78,6 @@ static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
 #define PCI_DISABLE_MWI
 
 #ifdef CONFIG_PCI
-extern void set_pci_dma_ops(struct dma_mapping_ops *dma_ops);
-extern struct dma_mapping_ops *get_pci_dma_ops(void);
-
 static inline void pci_dma_burst_advice(struct pci_dev *pdev,
 					enum pci_dma_burst_strategy *strat,
 					unsigned long *strategy_parameter)
@@ -89,9 +94,6 @@ static inline void pci_dma_burst_advice(struct pci_dev *pdev,
 	*strat = PCI_DMA_BURST_MULTIPLE;
 	*strategy_parameter = cacheline_size;
 }
-#else	/* CONFIG_PCI */
-#define set_pci_dma_ops(d)
-#define get_pci_dma_ops()	NULL
 #endif
 
 #else /* 32-bit */
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 45570fe..98f5282 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -68,10 +68,10 @@ extra-$(CONFIG_8xx)		:= head_8xx.o
 extra-y				+= vmlinux.lds
 
 obj-y				+= time.o prom.o traps.o setup-common.o \
-				   udbg.o misc.o io.o \
+				   udbg.o misc.o io.o dma.o \
 				   misc_$(CONFIG_WORD_SIZE).o
 obj-$(CONFIG_PPC32)		+= entry_32.o setup_32.o
-obj-$(CONFIG_PPC64)		+= dma.o dma-iommu.o iommu.o
+obj-$(CONFIG_PPC64)		+= dma-iommu.o iommu.o
 obj-$(CONFIG_KGDB)		+= kgdb.o
 obj-$(CONFIG_PPC_MULTIPLATFORM)	+= prom_init.o
 obj-$(CONFIG_MODULES)		+= ppc_ksyms.o
diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
index 124f867..41fdd48 100644
--- a/arch/powerpc/kernel/dma.c
+++ b/arch/powerpc/kernel/dma.c
@@ -16,21 +16,30 @@
  * This implementation supports a per-device offset that can be applied if
  * the address at which memory is visible to devices is not 0. Platform code
  * can set archdata.dma_data to an unsigned long holding the offset. By
- * default the offset is zero.
+ * default the offset is PCI_DRAM_OFFSET.
  */
 
 static unsigned long get_dma_direct_offset(struct device *dev)
 {
-	return (unsigned long)dev->archdata.dma_data;
+	if (dev)
+		return (unsigned long)dev->archdata.dma_data;
+
+	return PCI_DRAM_OFFSET;
 }
 
-static void *dma_direct_alloc_coherent(struct device *dev, size_t size,
-				       dma_addr_t *dma_handle, gfp_t flag)
+void *dma_direct_alloc_coherent(struct device *dev, size_t size,
+				dma_addr_t *dma_handle, gfp_t flag)
 {
+#ifdef CONFIG_NOT_COHERENT_CACHE
+	return __dma_alloc_coherent(size, dma_handle, flag);
+#else
 	struct page *page;
 	void *ret;
 	int node = dev_to_node(dev);
 
+	/* ignore region specifiers */
+	flag  &= ~(__GFP_HIGHMEM);
+
 	page = alloc_pages_node(node, flag, get_order(size));
 	if (page == NULL)
 		return NULL;
@@ -39,27 +48,17 @@ static void *dma_direct_alloc_coherent(struct device *dev, size_t size,
 	*dma_handle = virt_to_abs(ret) + get_dma_direct_offset(dev);
 
 	return ret;
+#endif
 }
 
-static void dma_direct_free_coherent(struct device *dev, size_t size,
-				     void *vaddr, dma_addr_t dma_handle)
+void dma_direct_free_coherent(struct device *dev, size_t size,
+			      void *vaddr, dma_addr_t dma_handle)
 {
+#ifdef CONFIG_NOT_COHERENT_CACHE
+	__dma_free_coherent(size, vaddr);
+#else
 	free_pages((unsigned long)vaddr, get_order(size));
-}
-
-static dma_addr_t dma_direct_map_single(struct device *dev, void *ptr,
-					size_t size,
-					enum dma_data_direction direction,
-					struct dma_attrs *attrs)
-{
-	return virt_to_abs(ptr) + get_dma_direct_offset(dev);
-}
-
-static void dma_direct_unmap_single(struct device *dev, dma_addr_t dma_addr,
-				    size_t size,
-				    enum dma_data_direction direction,
-				    struct dma_attrs *attrs)
-{
+#endif
 }
 
 static int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl,
@@ -85,20 +84,44 @@ static void dma_direct_unmap_sg(struct device *dev, struct scatterlist *sg,
 
 static int dma_direct_dma_supported(struct device *dev, u64 mask)
 {
+#ifdef CONFIG_PPC64
 	/* Could be improved to check for memory though it better be
 	 * done via some global so platforms can set the limit in case
 	 * they have limited DMA windows
 	 */
 	return mask >= DMA_32BIT_MASK;
+#else
+	return 1;
+#endif
+}
+
+static inline dma_addr_t dma_direct_map_page(struct device *dev,
+					     struct page *page,
+					     unsigned long offset,
+					     size_t size,
+					     enum dma_data_direction dir,
+					     struct dma_attrs *attrs)
+{
+	BUG_ON(dir == DMA_NONE);
+	__dma_sync_page(page, offset, size, dir);
+	return page_to_phys(page) + offset + get_dma_direct_offset(dev);
+}
+
+static inline void dma_direct_unmap_page(struct device *dev,
+					 dma_addr_t dma_address,
+					 size_t size,
+					 enum dma_data_direction direction,
+					 struct dma_attrs *attrs)
+{
 }
 
 struct dma_mapping_ops dma_direct_ops = {
 	.alloc_coherent	= dma_direct_alloc_coherent,
 	.free_coherent	= dma_direct_free_coherent,
-	.map_single	= dma_direct_map_single,
-	.unmap_single	= dma_direct_unmap_single,
 	.map_sg		= dma_direct_map_sg,
 	.unmap_sg	= dma_direct_unmap_sg,
 	.dma_supported	= dma_direct_dma_supported,
+	.map_page	= dma_direct_map_page,
+	.unmap_page	= dma_direct_unmap_page,
 };
 EXPORT_SYMBOL(dma_direct_ops);
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index ea0c61e..52ccfed 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -56,6 +56,34 @@ resource_size_t isa_mem_base;
 /* Default PCI flags is 0 */
 unsigned int ppc_pci_flags;
 
+static struct dma_mapping_ops *pci_dma_ops;
+
+void set_pci_dma_ops(struct dma_mapping_ops *dma_ops)
+{
+	pci_dma_ops = dma_ops;
+}
+
+struct dma_mapping_ops *get_pci_dma_ops(void)
+{
+	return pci_dma_ops;
+}
+EXPORT_SYMBOL(get_pci_dma_ops);
+
+int pci_set_dma_mask(struct pci_dev *dev, u64 mask)
+{
+	return dma_set_mask(&dev->dev, mask);
+}
+
+int pci_set_consistent_dma_mask(struct pci_dev *dev, u64 mask)
+{
+	int rc;
+
+	rc = dma_set_mask(&dev->dev, mask);
+	dev->dev.coherent_dma_mask = dev->dma_mask;
+
+	return rc;
+}
+
 struct pci_controller *pcibios_alloc_controller(struct device_node *dev)
 {
 	struct pci_controller *phb;
@@ -180,6 +208,26 @@ char __devinit *pcibios_setup(char *str)
 	return str;
 }
 
+void __devinit pcibios_setup_new_device(struct pci_dev *dev)
+{
+	struct dev_archdata *sd = &dev->dev.archdata;
+
+	sd->of_node = pci_device_to_OF_node(dev);
+
+	DBG("PCI: device %s OF node: %s\n", pci_name(dev),
+	    sd->of_node ? sd->of_node->full_name : "<none>");
+
+	sd->dma_ops = pci_dma_ops;
+#ifdef CONFIG_PPC32
+	sd->dma_data = (void *)PCI_DRAM_OFFSET;
+#endif
+	set_dev_node(&dev->dev, pcibus_to_node(dev->bus));
+
+	if (ppc_md.pci_dma_dev_setup)
+		ppc_md.pci_dma_dev_setup(dev);
+}
+EXPORT_SYMBOL(pcibios_setup_new_device);
+
 /*
  * Reads the interrupt pin to determine if interrupt is use by card.
  * If the interrupt is used, then gets the interrupt line from the
diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
index 88db4ff..174b77e 100644
--- a/arch/powerpc/kernel/pci_32.c
+++ b/arch/powerpc/kernel/pci_32.c
@@ -424,6 +424,7 @@ void __devinit pcibios_do_bus_setup(struct pci_bus *bus)
 	unsigned long io_offset;
 	struct resource *res;
 	int i;
+	struct pci_dev *dev;
 
 	/* Hookup PHB resources */
 	io_offset = (unsigned long)hose->io_base_virt - isa_io_base;
@@ -457,6 +458,12 @@ void __devinit pcibios_do_bus_setup(struct pci_bus *bus)
 			bus->resource[i+1] = res;
 		}
 	}
+
+	if (ppc_md.pci_dma_bus_setup)
+		ppc_md.pci_dma_bus_setup(bus);
+
+	list_for_each_entry(dev, &bus->devices, bus_list)
+		pcibios_setup_new_device(dev);
 }
 
 /* the next one is stolen from the alpha port... */
diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index 1f75bf0..8247cff 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -52,35 +52,6 @@ EXPORT_SYMBOL(pci_io_base);
 
 LIST_HEAD(hose_list);
 
-static struct dma_mapping_ops *pci_dma_ops;
-
-void set_pci_dma_ops(struct dma_mapping_ops *dma_ops)
-{
-	pci_dma_ops = dma_ops;
-}
-
-struct dma_mapping_ops *get_pci_dma_ops(void)
-{
-	return pci_dma_ops;
-}
-EXPORT_SYMBOL(get_pci_dma_ops);
-
-
-int pci_set_dma_mask(struct pci_dev *dev, u64 mask)
-{
-	return dma_set_mask(&dev->dev, mask);
-}
-
-int pci_set_consistent_dma_mask(struct pci_dev *dev, u64 mask)
-{
-	int rc;
-
-	rc = dma_set_mask(&dev->dev, mask);
-	dev->dev.coherent_dma_mask = dev->dma_mask;
-
-	return rc;
-}
-
 static void fixup_broken_pcnet32(struct pci_dev* dev)
 {
 	if ((dev->class>>8 == PCI_CLASS_NETWORK_ETHERNET)) {
@@ -548,23 +519,6 @@ int __devinit pcibios_map_io_space(struct pci_bus *bus)
 }
 EXPORT_SYMBOL_GPL(pcibios_map_io_space);
 
-void __devinit pcibios_setup_new_device(struct pci_dev *dev)
-{
-	struct dev_archdata *sd = &dev->dev.archdata;
-
-	sd->of_node = pci_device_to_OF_node(dev);
-
-	DBG("PCI: device %s OF node: %s\n", pci_name(dev),
-	    sd->of_node ? sd->of_node->full_name : "<none>");
-
-	sd->dma_ops = pci_dma_ops;
-	set_dev_node(&dev->dev, pcibus_to_node(dev->bus));
-
-	if (ppc_md.pci_dma_dev_setup)
-		ppc_md.pci_dma_dev_setup(dev);
-}
-EXPORT_SYMBOL(pcibios_setup_new_device);
-
 void __devinit pcibios_do_bus_setup(struct pci_bus *bus)
 {
 	struct pci_dev *dev;
-- 
1.5.5.1

Re: [PATCH 4/4] POWERPC: Merge 32 and 64-bit dma code

[Christoph Hellwig]

> -	if (unlikely(dev == NULL || dev->archdata.dma_ops == NULL))
> +
> +	if (unlikely(dev == NULL) || dev->archdata.dma_ops == NULL) {
> +#ifdef CONFIG_PPC64
>  		return NULL;
> +#else
> +		/* Use default on 32-bit if dma_ops is not set up */
> +		return &dma_direct_ops;
> +#endif
> +	}
> +

This is okay for the transition, but I think long-term it should
be setup for all busses.

>  }
>  
> @@ -132,7 +154,14 @@ static inline dma_addr_t dma_map_single_attrs(struct device *dev,
>  	struct dma_mapping_ops *dma_ops = get_dma_ops(dev);
>  
>  	BUG_ON(!dma_ops);
> -	return dma_ops->map_single(dev, cpu_addr, size, direction, attrs);
> +
> +	if (dma_ops->map_single)
> +		return dma_ops->map_single(dev, cpu_addr, size, direction,
> +					   attrs);
> +
> +	return dma_ops->map_page(dev, virt_to_page(cpu_addr),
> +				 (unsigned long)cpu_addr % PAGE_SIZE, size,
> +				 direction, attrs);

Why would a dma ops implementation not provide map_single/unmap_single?

Re: [PATCH 4/4] POWERPC: Merge 32 and 64-bit dma code

[Benjamin Herrenschmidt]

On Tue, 2008-09-09 at 00:03 +0200, Christoph Hellwig wrote:
> > -	if (unlikely(dev == NULL || dev->archdata.dma_ops == NULL))
> > +
> > +	if (unlikely(dev == NULL) || dev->archdata.dma_ops == NULL) {
> > +#ifdef CONFIG_PPC64
> >  		return NULL;
> > +#else
> > +		/* Use default on 32-bit if dma_ops is not set up */
> > +		return &dma_direct_ops;
> > +#endif
> > +	}
> > +
> 
> This is okay for the transition, but I think long-term it should
> be setup for all busses.

Agreed.

Ben.

Re: [PATCH 4/4] POWERPC: Merge 32 and 64-bit dma code

[Becky Bruce]

On Sep 8, 2008, at 5:03 PM, Christoph Hellwig wrote:

>> -	if (unlikely(dev == NULL || dev->archdata.dma_ops == NULL))
>> +
>> +	if (unlikely(dev == NULL) || dev->archdata.dma_ops == NULL) {
>> +#ifdef CONFIG_PPC64
>> 		return NULL;
>> +#else
>> +		/* Use default on 32-bit if dma_ops is not set up */
>> +		return &dma_direct_ops;
>> +#endif
>> +	}
>> +
>
> This is okay for the transition, but I think long-term it should
> be setup for all busses.

Agreed!

>
>
>> }
>>
>> @@ -132,7 +154,14 @@ static inline dma_addr_t  
>> dma_map_single_attrs(struct device *dev,
>> 	struct dma_mapping_ops *dma_ops = get_dma_ops(dev);
>>
>> 	BUG_ON(!dma_ops);
>> -	return dma_ops->map_single(dev, cpu_addr, size, direction, attrs);
>> +
>> +	if (dma_ops->map_single)
>> +		return dma_ops->map_single(dev, cpu_addr, size, direction,
>> +					   attrs);
>> +
>> +	return dma_ops->map_page(dev, virt_to_page(cpu_addr),
>> +				 (unsigned long)cpu_addr % PAGE_SIZE, size,
>> +				 direction, attrs);
>
> Why would a dma ops implementation not provide map_single/ 
> unmap_single?

.... because you asked me to have just map/unmap_page in your review  
of an earlier rev of this patch series in May? :)  I don't actually  
expect you to remember this, because it was a long time ago, but  
here's the relevant chunk of the conversation:

On May 23, 2008, at 4:51 AM, Christoph Hellwig wrote:

> On Wed, Apr 30, 2008 at 06:36:43PM -0500, Becky Bruce wrote:
>> In addition, the dma_map/unmap_page functions are added to dma_ops on
>> HIGHMEM-enabled configs because we can't just fall back on map/ 
>> unmap_single
>> when HIGHMEM is enabled.  Adding these to dma_ops makes it cleaner to
>> substitute different functionality once we have iommu/swiotlb  
>> support.
>
> Maybe I'm missing something but we should only have the page ones.
> virt_to_page is cheap and we'll most likely need the phys address  
> which
> we get from the page anyway.


So I did that for the dma_direct_* ops, but discovered that doing it  
for the iommu case (which I can't test and don't fully understand)  
would be a bit more complicated.  The above code (in conjunction with  
the same code for map_page) allows you to have map/unmap_page, map/ 
unmap_single, or both.  I'm happy to change it again, though.  We  
could just do the above in map_page/unmap_page, calling map/ 
unmap_single if there is no map/unmap_page, and provide both a  
dma_direct_map/unmap_single and a dma_direct_map/unmap_page for the  
dma_direct* ops.

Thanks for taking the time to review - I appreciate it!
-Becky

Re: [PATCH 4/4] POWERPC: Merge 32 and 64-bit dma code

[Christoph Hellwig]

On Tue, Sep 09, 2008 at 09:39:20AM -0500, Becky Bruce wrote:
> .... because you asked me to have just map/unmap_page in your review  
> of an earlier rev of this patch series in May? :)  I don't actually  
> expect you to remember this, because it was a long time ago, but  
> here's the relevant chunk of the conversation:
> 
> On May 23, 2008, at 4:51 AM, Christoph Hellwig wrote:
> 
> >On Wed, Apr 30, 2008 at 06:36:43PM -0500, Becky Bruce wrote:
> >>In addition, the dma_map/unmap_page functions are added to dma_ops on
> >>HIGHMEM-enabled configs because we can't just fall back on map/ 
> >>unmap_single
> >>when HIGHMEM is enabled.  Adding these to dma_ops makes it cleaner to
> >>substitute different functionality once we have iommu/swiotlb  
> >>support.
> >
> >Maybe I'm missing something but we should only have the page ones.
> >virt_to_page is cheap and we'll most likely need the phys address  
> >which
> >we get from the page anyway.
> 
> 
> So I did that for the dma_direct_* ops, but discovered that doing it  
> for the iommu case (which I can't test and don't fully understand)  
> would be a bit more complicated.  The above code (in conjunction with  
> the same code for map_page) allows you to have map/unmap_page, map/ 
> unmap_single, or both.  I'm happy to change it again, though.  We  
> could just do the above in map_page/unmap_page, calling map/ 
> unmap_single if there is no map/unmap_page, and provide both a  
> dma_direct_map/unmap_single and a dma_direct_map/unmap_page for the  
> dma_direct* ops.

Yeah, the statement this time should be why do you keep _single :)
It don't really mind which one we keep, but having both and both
optional seems rather odd.

Re: [PATCH 4/4] POWERPC: Merge 32 and 64-bit dma code

[Benjamin Herrenschmidt]

> Yeah, the statement this time should be why do you keep _single :)
> It don't really mind which one we keep, but having both and both
> optional seems rather odd.

It would be more logic to implement _single on top of _page.. as long as
we can still do sub-page mappings (needed on ppc64) which I think we can
using the offset / size args of _page...

But that means more work changing the existing ppc64 backends :-) So
probably a worthwhile long term goal but what Becky did is ok for the
transition. I can look at fixing up the ppc64 ones to implement page
(and see if that causes any problem) some time after Plumbers conf.

Cheers,
Ben.
 

Re: [PATCH 4/4] POWERPC: Merge 32 and 64-bit dma code

[Christoph Hellwig]

On Wed, Sep 10, 2008 at 08:10:31AM +1000, Benjamin Herrenschmidt wrote:
> 
> > Yeah, the statement this time should be why do you keep _single :)
> > It don't really mind which one we keep, but having both and both
> > optional seems rather odd.
> 
> It would be more logic to implement _single on top of _page.. as long as
> we can still do sub-page mappings (needed on ppc64) which I think we can
> using the offset / size args of _page...
> 
> But that means more work changing the existing ppc64 backends :-) So
> probably a worthwhile long term goal but what Becky did is ok for the
> transition. I can look at fixing up the ppc64 ones to implement page
> (and see if that causes any problem) some time after Plumbers conf.

Ok.  But it would be good if we could add comments in the code for the
to be done later issues we noted so it's harder to forget it.

> 
> Cheers,
> Ben.
>  
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
---end quoted text---

Re: [PATCH 4/4] POWERPC: Merge 32 and 64-bit dma code

[Becky Bruce]

On Sep 9, 2008, at 5:13 PM, Christoph Hellwig wrote:

> On Wed, Sep 10, 2008 at 08:10:31AM +1000, Benjamin Herrenschmidt  
> wrote:
>>
>>> Yeah, the statement this time should be why do you keep _single :)
>>> It don't really mind which one we keep, but having both and both
>>> optional seems rather odd.
>>
>> It would be more logic to implement _single on top of _page.. as  
>> long as
>> we can still do sub-page mappings (needed on ppc64) which I think  
>> we can
>> using the offset / size args of _page...
>>
>> But that means more work changing the existing ppc64 backends :-) So
>> probably a worthwhile long term goal but what Becky did is ok for the
>> transition. I can look at fixing up the ppc64 ones to implement page
>> (and see if that causes any problem) some time after Plumbers conf.
>
> Ok.  But it would be good if we could add comments in the code for the
> to be done later issues we noted so it's harder to forget it.

Will do as I respin.

Thanks!
-Becky

[PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code

[Becky Bruce]

We essentially adopt the 64-bit dma code, with some changes to support
32-bit systems, including HIGHMEM.  dma functions on 32-bit are now
invoked via accessor functions which call the correct op for a device based
on archdata dma_ops.  If there is no archdata dma_ops, this defaults
to dma_direct_ops.

In addition, the dma_map/unmap_page functions are added to dma_ops
because we can't just fall back on map/unmap_single when HIGHMEM is
enabled. In the case of dma_direct_*, we stop using map/unmap_single
and just use the page version - this saves a lot of ugly
ifdeffing.  We leave map/unmap_single in the dma_ops definition,
though, because they are needed by the iommu code, which does not
implement map/unmap_page.  Ideally, going forward, we will completely
eliminate map/unmap_single and just have map/unmap_page, if it's
workable for 64-bit.

Signed-off-by: Becky Bruce <becky.bruce@freescale.com>
---
Added comments to the effect that we would like devices to properly
set up the dev and archdata in the future so we can remove the the
fallback behavior on 32-bit.

 arch/powerpc/include/asm/dma-mapping.h |  187 +++++++++++---------------------
 arch/powerpc/include/asm/machdep.h     |    5 +-
 arch/powerpc/include/asm/pci.h         |   14 ++-
 arch/powerpc/kernel/Makefile           |    4 +-
 arch/powerpc/kernel/dma.c              |   69 ++++++++----
 arch/powerpc/kernel/pci-common.c       |   48 ++++++++
 arch/powerpc/kernel/pci_32.c           |    7 ++
 arch/powerpc/kernel/pci_64.c           |   46 --------
 8 files changed, 175 insertions(+), 205 deletions(-)

diff --git a/arch/powerpc/include/asm/dma-mapping.h b/arch/powerpc/include/asm/dma-mapping.h
index c7ca45f..fddb229 100644
--- a/arch/powerpc/include/asm/dma-mapping.h
+++ b/arch/powerpc/include/asm/dma-mapping.h
@@ -44,8 +44,6 @@ extern void __dma_sync_page(struct page *page, unsigned long offset,
 
 #endif /* ! CONFIG_NOT_COHERENT_CACHE */
 
-#ifdef CONFIG_PPC64
-
 static inline unsigned long device_to_mask(struct device *dev)
 {
 	if (dev->dma_mask && *dev->dma_mask)
@@ -76,8 +74,24 @@ struct dma_mapping_ops {
 				struct dma_attrs *attrs);
 	int		(*dma_supported)(struct device *dev, u64 mask);
 	int		(*set_dma_mask)(struct device *dev, u64 dma_mask);
+	dma_addr_t 	(*map_page)(struct device *dev, struct page *page,
+				unsigned long offset, size_t size,
+				enum dma_data_direction direction,
+				struct dma_attrs *attrs);
+	void		(*unmap_page)(struct device *dev,
+				dma_addr_t dma_address, size_t size,
+				enum dma_data_direction direction,
+				struct dma_attrs *attrs);
 };
 
+/*
+ * Available generic sets of operations
+ */
+#ifdef CONFIG_PPC64
+extern struct dma_mapping_ops dma_iommu_ops;
+#endif
+extern struct dma_mapping_ops dma_direct_ops;
+
 static inline struct dma_mapping_ops *get_dma_ops(struct device *dev)
 {
 	/* We don't handle the NULL dev case for ISA for now. We could
@@ -85,8 +99,19 @@ static inline struct dma_mapping_ops *get_dma_ops(struct device *dev)
 	 * only ISA DMA device we support is the floppy and we have a hack
 	 * in the floppy driver directly to get a device for us.
 	 */
-	if (unlikely(dev == NULL || dev->archdata.dma_ops == NULL))
+
+	if (unlikely(dev == NULL) || dev->archdata.dma_ops == NULL) {
+#ifdef CONFIG_PPC64
 		return NULL;
+#else
+		/* Use default on 32-bit if dma_ops is not set up */
+		/* TODO: Long term, we should fix drivers so that dev and
+		 * archdata dma_ops are set up for all buses.
+		 */
+		return &dma_direct_ops;
+#endif
+	}
+
 	return dev->archdata.dma_ops;
 }
 
@@ -123,6 +148,12 @@ static inline int dma_set_mask(struct device *dev, u64 dma_mask)
 	return 0;
 }
 
+/*
+ * TODO: map_/unmap_single will ideally go away, to be completely
+ * replaced by map/unmap_page.   Until then, we allow dma_ops to have
+ * one or the other, or both by checking to see if the specific
+ * function requested exists; and if not, falling back on the other set.
+ */
 static inline dma_addr_t dma_map_single_attrs(struct device *dev,
 					      void *cpu_addr,
 					      size_t size,
@@ -132,7 +163,14 @@ static inline dma_addr_t dma_map_single_attrs(struct device *dev,
 	struct dma_mapping_ops *dma_ops = get_dma_ops(dev);
 
 	BUG_ON(!dma_ops);
-	return dma_ops->map_single(dev, cpu_addr, size, direction, attrs);
+
+	if (dma_ops->map_single)
+		return dma_ops->map_single(dev, cpu_addr, size, direction,
+					   attrs);
+
+	return dma_ops->map_page(dev, virt_to_page(cpu_addr),
+				 (unsigned long)cpu_addr % PAGE_SIZE, size,
+				 direction, attrs);
 }
 
 static inline void dma_unmap_single_attrs(struct device *dev,
@@ -144,7 +182,13 @@ static inline void dma_unmap_single_attrs(struct device *dev,
 	struct dma_mapping_ops *dma_ops = get_dma_ops(dev);
 
 	BUG_ON(!dma_ops);
-	dma_ops->unmap_single(dev, dma_addr, size, direction, attrs);
+
+	if (dma_ops->unmap_single) {
+		dma_ops->unmap_single(dev, dma_addr, size, direction, attrs);
+		return;
+	}
+
+	dma_ops->unmap_page(dev, dma_addr, size, direction, attrs);
 }
 
 static inline dma_addr_t dma_map_page_attrs(struct device *dev,
@@ -156,8 +200,13 @@ static inline dma_addr_t dma_map_page_attrs(struct device *dev,
 	struct dma_mapping_ops *dma_ops = get_dma_ops(dev);
 
 	BUG_ON(!dma_ops);
+
+	if (dma_ops->map_page)
+		return dma_ops->map_page(dev, page, offset, size, direction,
+					 attrs);
+
 	return dma_ops->map_single(dev, page_address(page) + offset, size,
-			direction, attrs);
+				   direction, attrs);
 }
 
 static inline void dma_unmap_page_attrs(struct device *dev,
@@ -169,6 +218,12 @@ static inline void dma_unmap_page_attrs(struct device *dev,
 	struct dma_mapping_ops *dma_ops = get_dma_ops(dev);
 
 	BUG_ON(!dma_ops);
+
+	if (dma_ops->unmap_page) {
+		dma_ops->unmap_page(dev, dma_address, size, direction, attrs);
+		return;
+	}
+
 	dma_ops->unmap_single(dev, dma_address, size, direction, attrs);
 }
 
@@ -253,126 +308,6 @@ static inline void dma_unmap_sg(struct device *dev, struct scatterlist *sg,
 	dma_unmap_sg_attrs(dev, sg, nhwentries, direction, NULL);
 }
 
-/*
- * Available generic sets of operations
- */
-extern struct dma_mapping_ops dma_iommu_ops;
-extern struct dma_mapping_ops dma_direct_ops;
-
-#else /* CONFIG_PPC64 */
-
-#define dma_supported(dev, mask)	(1)
-
-static inline int dma_set_mask(struct device *dev, u64 dma_mask)
-{
-	if (!dev->dma_mask || !dma_supported(dev, mask))
-		return -EIO;
-
-	*dev->dma_mask = dma_mask;
-
-	return 0;
-}
-
-static inline void *dma_alloc_coherent(struct device *dev, size_t size,
-				       dma_addr_t * dma_handle,
-				       gfp_t gfp)
-{
-#ifdef CONFIG_NOT_COHERENT_CACHE
-	return __dma_alloc_coherent(size, dma_handle, gfp);
-#else
-	void *ret;
-	/* ignore region specifiers */
-	gfp &= ~(__GFP_DMA | __GFP_HIGHMEM);
-
-	if (dev == NULL || dev->coherent_dma_mask < 0xffffffff)
-		gfp |= GFP_DMA;
-
-	ret = (void *)__get_free_pages(gfp, get_order(size));
-
-	if (ret != NULL) {
-		memset(ret, 0, size);
-		*dma_handle = virt_to_bus(ret);
-	}
-
-	return ret;
-#endif
-}
-
-static inline void
-dma_free_coherent(struct device *dev, size_t size, void *vaddr,
-		  dma_addr_t dma_handle)
-{
-#ifdef CONFIG_NOT_COHERENT_CACHE
-	__dma_free_coherent(size, vaddr);
-#else
-	free_pages((unsigned long)vaddr, get_order(size));
-#endif
-}
-
-static inline dma_addr_t
-dma_map_single(struct device *dev, void *ptr, size_t size,
-	       enum dma_data_direction direction)
-{
-	BUG_ON(direction == DMA_NONE);
-
-	__dma_sync(ptr, size, direction);
-
-	return virt_to_bus(ptr);
-}
-
-static inline void dma_unmap_single(struct device *dev, dma_addr_t dma_addr,
-				    size_t size,
-				    enum dma_data_direction direction)
-{
-	/* We do nothing. */
-}
-
-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_ON(direction == DMA_NONE);
-
-	__dma_sync_page(page, offset, size, direction);
-
-	return page_to_bus(page) + offset;
-}
-
-static inline void dma_unmap_page(struct device *dev, dma_addr_t dma_address,
-				  size_t size,
-				  enum dma_data_direction direction)
-{
-	/* We do nothing. */
-}
-
-static inline int
-dma_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
-	   enum dma_data_direction direction)
-{
-	struct scatterlist *sg;
-	int i;
-
-	BUG_ON(direction == DMA_NONE);
-
-	for_each_sg(sgl, sg, nents, i) {
-		BUG_ON(!sg_page(sg));
-		__dma_sync_page(sg_page(sg), sg->offset, sg->length, direction);
-		sg->dma_address = page_to_bus(sg_page(sg)) + sg->offset;
-	}
-
-	return nents;
-}
-
-static inline void dma_unmap_sg(struct device *dev, struct scatterlist *sg,
-				int nhwentries,
-				enum dma_data_direction direction)
-{
-	/* We don't do anything here. */
-}
-
-#endif /* CONFIG_PPC64 */
-
 static inline void dma_sync_single_for_cpu(struct device *dev,
 		dma_addr_t dma_handle, size_t size,
 		enum dma_data_direction direction)
diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
index 893aafd..2740c44 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -88,8 +88,6 @@ struct machdep_calls {
 	unsigned long	(*tce_get)(struct iommu_table *tbl,
 				    long index);
 	void		(*tce_flush)(struct iommu_table *tbl);
-	void		(*pci_dma_dev_setup)(struct pci_dev *dev);
-	void		(*pci_dma_bus_setup)(struct pci_bus *bus);
 
 	void __iomem *	(*ioremap)(phys_addr_t addr, unsigned long size,
 				   unsigned long flags);
@@ -101,6 +99,9 @@ struct machdep_calls {
 #endif
 #endif /* CONFIG_PPC64 */
 
+	void		(*pci_dma_dev_setup)(struct pci_dev *dev);
+	void		(*pci_dma_bus_setup)(struct pci_bus *bus);
+
 	int		(*probe)(void);
 	void		(*setup_arch)(void); /* Optional, may be NULL */
 	void		(*init_early)(void);
diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
index a05a942..0e52c78 100644
--- a/arch/powerpc/include/asm/pci.h
+++ b/arch/powerpc/include/asm/pci.h
@@ -60,6 +60,14 @@ static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
 	return channel ? 15 : 14;
 }
 
+#ifdef CONFIG_PCI
+extern void set_pci_dma_ops(struct dma_mapping_ops *dma_ops);
+extern struct dma_mapping_ops *get_pci_dma_ops(void);
+#else	/* CONFIG_PCI */
+#define set_pci_dma_ops(d)
+#define get_pci_dma_ops()	NULL
+#endif
+
 #ifdef CONFIG_PPC64
 
 /*
@@ -70,9 +78,6 @@ static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
 #define PCI_DISABLE_MWI
 
 #ifdef CONFIG_PCI
-extern void set_pci_dma_ops(struct dma_mapping_ops *dma_ops);
-extern struct dma_mapping_ops *get_pci_dma_ops(void);
-
 static inline void pci_dma_burst_advice(struct pci_dev *pdev,
 					enum pci_dma_burst_strategy *strat,
 					unsigned long *strategy_parameter)
@@ -89,9 +94,6 @@ static inline void pci_dma_burst_advice(struct pci_dev *pdev,
 	*strat = PCI_DMA_BURST_MULTIPLE;
 	*strategy_parameter = cacheline_size;
 }
-#else	/* CONFIG_PCI */
-#define set_pci_dma_ops(d)
-#define get_pci_dma_ops()	NULL
 #endif
 
 #else /* 32-bit */
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 45570fe..98f5282 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -68,10 +68,10 @@ extra-$(CONFIG_8xx)		:= head_8xx.o
 extra-y				+= vmlinux.lds
 
 obj-y				+= time.o prom.o traps.o setup-common.o \
-				   udbg.o misc.o io.o \
+				   udbg.o misc.o io.o dma.o \
 				   misc_$(CONFIG_WORD_SIZE).o
 obj-$(CONFIG_PPC32)		+= entry_32.o setup_32.o
-obj-$(CONFIG_PPC64)		+= dma.o dma-iommu.o iommu.o
+obj-$(CONFIG_PPC64)		+= dma-iommu.o iommu.o
 obj-$(CONFIG_KGDB)		+= kgdb.o
 obj-$(CONFIG_PPC_MULTIPLATFORM)	+= prom_init.o
 obj-$(CONFIG_MODULES)		+= ppc_ksyms.o
diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
index 124f867..41fdd48 100644
--- a/arch/powerpc/kernel/dma.c
+++ b/arch/powerpc/kernel/dma.c
@@ -16,21 +16,30 @@
  * This implementation supports a per-device offset that can be applied if
  * the address at which memory is visible to devices is not 0. Platform code
  * can set archdata.dma_data to an unsigned long holding the offset. By
- * default the offset is zero.
+ * default the offset is PCI_DRAM_OFFSET.
  */
 
 static unsigned long get_dma_direct_offset(struct device *dev)
 {
-	return (unsigned long)dev->archdata.dma_data;
+	if (dev)
+		return (unsigned long)dev->archdata.dma_data;
+
+	return PCI_DRAM_OFFSET;
 }
 
-static void *dma_direct_alloc_coherent(struct device *dev, size_t size,
-				       dma_addr_t *dma_handle, gfp_t flag)
+void *dma_direct_alloc_coherent(struct device *dev, size_t size,
+				dma_addr_t *dma_handle, gfp_t flag)
 {
+#ifdef CONFIG_NOT_COHERENT_CACHE
+	return __dma_alloc_coherent(size, dma_handle, flag);
+#else
 	struct page *page;
 	void *ret;
 	int node = dev_to_node(dev);
 
+	/* ignore region specifiers */
+	flag  &= ~(__GFP_HIGHMEM);
+
 	page = alloc_pages_node(node, flag, get_order(size));
 	if (page == NULL)
 		return NULL;
@@ -39,27 +48,17 @@ static void *dma_direct_alloc_coherent(struct device *dev, size_t size,
 	*dma_handle = virt_to_abs(ret) + get_dma_direct_offset(dev);
 
 	return ret;
+#endif
 }
 
-static void dma_direct_free_coherent(struct device *dev, size_t size,
-				     void *vaddr, dma_addr_t dma_handle)
+void dma_direct_free_coherent(struct device *dev, size_t size,
+			      void *vaddr, dma_addr_t dma_handle)
 {
+#ifdef CONFIG_NOT_COHERENT_CACHE
+	__dma_free_coherent(size, vaddr);
+#else
 	free_pages((unsigned long)vaddr, get_order(size));
-}
-
-static dma_addr_t dma_direct_map_single(struct device *dev, void *ptr,
-					size_t size,
-					enum dma_data_direction direction,
-					struct dma_attrs *attrs)
-{
-	return virt_to_abs(ptr) + get_dma_direct_offset(dev);
-}
-
-static void dma_direct_unmap_single(struct device *dev, dma_addr_t dma_addr,
-				    size_t size,
-				    enum dma_data_direction direction,
-				    struct dma_attrs *attrs)
-{
+#endif
 }
 
 static int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl,
@@ -85,20 +84,44 @@ static void dma_direct_unmap_sg(struct device *dev, struct scatterlist *sg,
 
 static int dma_direct_dma_supported(struct device *dev, u64 mask)
 {
+#ifdef CONFIG_PPC64
 	/* Could be improved to check for memory though it better be
 	 * done via some global so platforms can set the limit in case
 	 * they have limited DMA windows
 	 */
 	return mask >= DMA_32BIT_MASK;
+#else
+	return 1;
+#endif
+}
+
+static inline dma_addr_t dma_direct_map_page(struct device *dev,
+					     struct page *page,
+					     unsigned long offset,
+					     size_t size,
+					     enum dma_data_direction dir,
+					     struct dma_attrs *attrs)
+{
+	BUG_ON(dir == DMA_NONE);
+	__dma_sync_page(page, offset, size, dir);
+	return page_to_phys(page) + offset + get_dma_direct_offset(dev);
+}
+
+static inline void dma_direct_unmap_page(struct device *dev,
+					 dma_addr_t dma_address,
+					 size_t size,
+					 enum dma_data_direction direction,
+					 struct dma_attrs *attrs)
+{
 }
 
 struct dma_mapping_ops dma_direct_ops = {
 	.alloc_coherent	= dma_direct_alloc_coherent,
 	.free_coherent	= dma_direct_free_coherent,
-	.map_single	= dma_direct_map_single,
-	.unmap_single	= dma_direct_unmap_single,
 	.map_sg		= dma_direct_map_sg,
 	.unmap_sg	= dma_direct_unmap_sg,
 	.dma_supported	= dma_direct_dma_supported,
+	.map_page	= dma_direct_map_page,
+	.unmap_page	= dma_direct_unmap_page,
 };
 EXPORT_SYMBOL(dma_direct_ops);
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index ea0c61e..52ccfed 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -56,6 +56,34 @@ resource_size_t isa_mem_base;
 /* Default PCI flags is 0 */
 unsigned int ppc_pci_flags;
 
+static struct dma_mapping_ops *pci_dma_ops;
+
+void set_pci_dma_ops(struct dma_mapping_ops *dma_ops)
+{
+	pci_dma_ops = dma_ops;
+}
+
+struct dma_mapping_ops *get_pci_dma_ops(void)
+{
+	return pci_dma_ops;
+}
+EXPORT_SYMBOL(get_pci_dma_ops);
+
+int pci_set_dma_mask(struct pci_dev *dev, u64 mask)
+{
+	return dma_set_mask(&dev->dev, mask);
+}
+
+int pci_set_consistent_dma_mask(struct pci_dev *dev, u64 mask)
+{
+	int rc;
+
+	rc = dma_set_mask(&dev->dev, mask);
+	dev->dev.coherent_dma_mask = dev->dma_mask;
+
+	return rc;
+}
+
 struct pci_controller *pcibios_alloc_controller(struct device_node *dev)
 {
 	struct pci_controller *phb;
@@ -180,6 +208,26 @@ char __devinit *pcibios_setup(char *str)
 	return str;
 }
 
+void __devinit pcibios_setup_new_device(struct pci_dev *dev)
+{
+	struct dev_archdata *sd = &dev->dev.archdata;
+
+	sd->of_node = pci_device_to_OF_node(dev);
+
+	DBG("PCI: device %s OF node: %s\n", pci_name(dev),
+	    sd->of_node ? sd->of_node->full_name : "<none>");
+
+	sd->dma_ops = pci_dma_ops;
+#ifdef CONFIG_PPC32
+	sd->dma_data = (void *)PCI_DRAM_OFFSET;
+#endif
+	set_dev_node(&dev->dev, pcibus_to_node(dev->bus));
+
+	if (ppc_md.pci_dma_dev_setup)
+		ppc_md.pci_dma_dev_setup(dev);
+}
+EXPORT_SYMBOL(pcibios_setup_new_device);
+
 /*
  * Reads the interrupt pin to determine if interrupt is use by card.
  * If the interrupt is used, then gets the interrupt line from the
diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
index 88db4ff..174b77e 100644
--- a/arch/powerpc/kernel/pci_32.c
+++ b/arch/powerpc/kernel/pci_32.c
@@ -424,6 +424,7 @@ void __devinit pcibios_do_bus_setup(struct pci_bus *bus)
 	unsigned long io_offset;
 	struct resource *res;
 	int i;
+	struct pci_dev *dev;
 
 	/* Hookup PHB resources */
 	io_offset = (unsigned long)hose->io_base_virt - isa_io_base;
@@ -457,6 +458,12 @@ void __devinit pcibios_do_bus_setup(struct pci_bus *bus)
 			bus->resource[i+1] = res;
 		}
 	}
+
+	if (ppc_md.pci_dma_bus_setup)
+		ppc_md.pci_dma_bus_setup(bus);
+
+	list_for_each_entry(dev, &bus->devices, bus_list)
+		pcibios_setup_new_device(dev);
 }
 
 /* the next one is stolen from the alpha port... */
diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index 1f75bf0..8247cff 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -52,35 +52,6 @@ EXPORT_SYMBOL(pci_io_base);
 
 LIST_HEAD(hose_list);
 
-static struct dma_mapping_ops *pci_dma_ops;
-
-void set_pci_dma_ops(struct dma_mapping_ops *dma_ops)
-{
-	pci_dma_ops = dma_ops;
-}
-
-struct dma_mapping_ops *get_pci_dma_ops(void)
-{
-	return pci_dma_ops;
-}
-EXPORT_SYMBOL(get_pci_dma_ops);
-
-
-int pci_set_dma_mask(struct pci_dev *dev, u64 mask)
-{
-	return dma_set_mask(&dev->dev, mask);
-}
-
-int pci_set_consistent_dma_mask(struct pci_dev *dev, u64 mask)
-{
-	int rc;
-
-	rc = dma_set_mask(&dev->dev, mask);
-	dev->dev.coherent_dma_mask = dev->dma_mask;
-
-	return rc;
-}
-
 static void fixup_broken_pcnet32(struct pci_dev* dev)
 {
 	if ((dev->class>>8 == PCI_CLASS_NETWORK_ETHERNET)) {
@@ -548,23 +519,6 @@ int __devinit pcibios_map_io_space(struct pci_bus *bus)
 }
 EXPORT_SYMBOL_GPL(pcibios_map_io_space);
 
-void __devinit pcibios_setup_new_device(struct pci_dev *dev)
-{
-	struct dev_archdata *sd = &dev->dev.archdata;
-
-	sd->of_node = pci_device_to_OF_node(dev);
-
-	DBG("PCI: device %s OF node: %s\n", pci_name(dev),
-	    sd->of_node ? sd->of_node->full_name : "<none>");
-
-	sd->dma_ops = pci_dma_ops;
-	set_dev_node(&dev->dev, pcibus_to_node(dev->bus));
-
-	if (ppc_md.pci_dma_dev_setup)
-		ppc_md.pci_dma_dev_setup(dev);
-}
-EXPORT_SYMBOL(pcibios_setup_new_device);
-
 void __devinit pcibios_do_bus_setup(struct pci_bus *bus)
 {
 	struct pci_dev *dev;
-- 
1.5.5.1

Re: [PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code

[Josh Boyer]

On Fri, Sep 12, 2008 at 03:34:46PM -0500, Becky Bruce wrote:
>We essentially adopt the 64-bit dma code, with some changes to support
>32-bit systems, including HIGHMEM.  dma functions on 32-bit are now
>invoked via accessor functions which call the correct op for a device based
>on archdata dma_ops.  If there is no archdata dma_ops, this defaults
>to dma_direct_ops.
>
>In addition, the dma_map/unmap_page functions are added to dma_ops
>because we can't just fall back on map/unmap_single when HIGHMEM is
>enabled. In the case of dma_direct_*, we stop using map/unmap_single
>and just use the page version - this saves a lot of ugly
>ifdeffing.  We leave map/unmap_single in the dma_ops definition,
>though, because they are needed by the iommu code, which does not
>implement map/unmap_page.  Ideally, going forward, we will completely
>eliminate map/unmap_single and just have map/unmap_page, if it's
>workable for 64-bit.
>
>Signed-off-by: Becky Bruce <becky.bruce@freescale.com>

While doing a buildall this morning, I notice chrp32_defconfig fails
to build with:

drivers/built-in.o: In function `hard_dma_setup':
floppy.c:(.text+0x6e40e): undefined reference to `isa_bridge_pcidev'
floppy.c:(.text+0x6e412): undefined reference to `isa_bridge_pcidev'
floppy.c:(.text+0x6e53e): undefined reference to `isa_bridge_pcidev'
floppy.c:(.text+0x6e546): undefined reference to `isa_bridge_pcidev'
floppy.c:(.text+0x6e54a): undefined reference to `isa_bridge_pcidev'
make[1]: *** [.tmp_vmlinux1] Error 1

(the hard_dma_setup thing is in arch/powerpc/include/asm/floppy.h).

I did a git bisect and it pointed at this commit as causing the build
to fail.  Why, I have no idea.

josh

Re: [PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code

[Josh Boyer]

On Mon, Oct 13, 2008 at 10:49:04AM -0400, Josh Boyer wrote:
>On Fri, Sep 12, 2008 at 03:34:46PM -0500, Becky Bruce wrote:
>>We essentially adopt the 64-bit dma code, with some changes to support
>>32-bit systems, including HIGHMEM.  dma functions on 32-bit are now
>>invoked via accessor functions which call the correct op for a device based
>>on archdata dma_ops.  If there is no archdata dma_ops, this defaults
>>to dma_direct_ops.
>>
>>In addition, the dma_map/unmap_page functions are added to dma_ops
>>because we can't just fall back on map/unmap_single when HIGHMEM is
>>enabled. In the case of dma_direct_*, we stop using map/unmap_single
>>and just use the page version - this saves a lot of ugly
>>ifdeffing.  We leave map/unmap_single in the dma_ops definition,
>>though, because they are needed by the iommu code, which does not
>>implement map/unmap_page.  Ideally, going forward, we will completely
>>eliminate map/unmap_single and just have map/unmap_page, if it's
>>workable for 64-bit.
>>
>>Signed-off-by: Becky Bruce <becky.bruce@freescale.com>
>
>While doing a buildall this morning, I notice chrp32_defconfig fails
>to build with:
>
>drivers/built-in.o: In function `hard_dma_setup':
>floppy.c:(.text+0x6e40e): undefined reference to `isa_bridge_pcidev'
>floppy.c:(.text+0x6e412): undefined reference to `isa_bridge_pcidev'
>floppy.c:(.text+0x6e53e): undefined reference to `isa_bridge_pcidev'
>floppy.c:(.text+0x6e546): undefined reference to `isa_bridge_pcidev'
>floppy.c:(.text+0x6e54a): undefined reference to `isa_bridge_pcidev'
>make[1]: *** [.tmp_vmlinux1] Error 1
>
>(the hard_dma_setup thing is in arch/powerpc/include/asm/floppy.h).
>
>I did a git bisect and it pointed at this commit as causing the build
>to fail.  Why, I have no idea.

Ok, I was annoyed enough to look at why.

Basically, before this patch pci_map_single on 32-bit PPC seemed to
be compiled down to __dma_sync(ptr, size, direction); and the "dev"
parameter to the function was never actually used.  The compiler
seems to have optimized this out entirely, so we don't get the odd
link reference to isa_bridge_pcidev at all.  (Neither pci_map_single
or isa_bridge_pcidev are present in the vmlinux at all).

With the patch, the compiler doesn't do this code elimination
because pci_map_single boils down to dma_map_page, which calls
get_dma_direct_offset with the "dev" parameter.  So since it is
still used, the compiler can't eliminate it and hence FAIL.

I have no patch for this at the moment.  Someone should look at
it more closely, because this is causing the 5 chrp32_defconfig
users to weep.

josh

Re: [PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code

[Kumar Gala]

On Oct 13, 2008, at 10:41 AM, Josh Boyer wrote:

> On Mon, Oct 13, 2008 at 10:49:04AM -0400, Josh Boyer wrote:
>> On Fri, Sep 12, 2008 at 03:34:46PM -0500, Becky Bruce wrote:
>>> We essentially adopt the 64-bit dma code, with some changes to  
>>> support
>>> 32-bit systems, including HIGHMEM.  dma functions on 32-bit are now
>>> invoked via accessor functions which call the correct op for a  
>>> device based
>>> on archdata dma_ops.  If there is no archdata dma_ops, this defaults
>>> to dma_direct_ops.
>>>
>>> In addition, the dma_map/unmap_page functions are added to dma_ops
>>> because we can't just fall back on map/unmap_single when HIGHMEM is
>>> enabled. In the case of dma_direct_*, we stop using map/unmap_single
>>> and just use the page version - this saves a lot of ugly
>>> ifdeffing.  We leave map/unmap_single in the dma_ops definition,
>>> though, because they are needed by the iommu code, which does not
>>> implement map/unmap_page.  Ideally, going forward, we will  
>>> completely
>>> eliminate map/unmap_single and just have map/unmap_page, if it's
>>> workable for 64-bit.
>>>
>>> Signed-off-by: Becky Bruce <becky.bruce@freescale.com>
>>
>> While doing a buildall this morning, I notice chrp32_defconfig fails
>> to build with:
>>
>> drivers/built-in.o: In function `hard_dma_setup':
>> floppy.c:(.text+0x6e40e): undefined reference to `isa_bridge_pcidev'
>> floppy.c:(.text+0x6e412): undefined reference to `isa_bridge_pcidev'
>> floppy.c:(.text+0x6e53e): undefined reference to `isa_bridge_pcidev'
>> floppy.c:(.text+0x6e546): undefined reference to `isa_bridge_pcidev'
>> floppy.c:(.text+0x6e54a): undefined reference to `isa_bridge_pcidev'
>> make[1]: *** [.tmp_vmlinux1] Error 1
>>
>> (the hard_dma_setup thing is in arch/powerpc/include/asm/floppy.h).
>>
>> I did a git bisect and it pointed at this commit as causing the build
>> to fail.  Why, I have no idea.
>
> Ok, I was annoyed enough to look at why.
>
> Basically, before this patch pci_map_single on 32-bit PPC seemed to
> be compiled down to __dma_sync(ptr, size, direction); and the "dev"
> parameter to the function was never actually used.  The compiler
> seems to have optimized this out entirely, so we don't get the odd
> link reference to isa_bridge_pcidev at all.  (Neither pci_map_single
> or isa_bridge_pcidev are present in the vmlinux at all).
>
> With the patch, the compiler doesn't do this code elimination
> because pci_map_single boils down to dma_map_page, which calls
> get_dma_direct_offset with the "dev" parameter.  So since it is
> still used, the compiler can't eliminate it and hence FAIL.
>
> I have no patch for this at the moment.  Someone should look at
> it more closely, because this is causing the 5 chrp32_defconfig
> users to weep.

Isn't this the type of regression we should fix post -rc1 :)

- k

Re: [PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code

[Josh Boyer]

On Mon, 13 Oct 2008 13:06:36 -0500
Kumar Gala <galak@kernel.crashing.org> wrote:

> 
> On Oct 13, 2008, at 10:41 AM, Josh Boyer wrote:
> 
> > On Mon, Oct 13, 2008 at 10:49:04AM -0400, Josh Boyer wrote:
> >> On Fri, Sep 12, 2008 at 03:34:46PM -0500, Becky Bruce wrote:
> >>> We essentially adopt the 64-bit dma code, with some changes to  
> >>> support
> >>> 32-bit systems, including HIGHMEM.  dma functions on 32-bit are now
> >>> invoked via accessor functions which call the correct op for a  
> >>> device based
> >>> on archdata dma_ops.  If there is no archdata dma_ops, this defaults
> >>> to dma_direct_ops.
> >>>
> >>> In addition, the dma_map/unmap_page functions are added to dma_ops
> >>> because we can't just fall back on map/unmap_single when HIGHMEM is
> >>> enabled. In the case of dma_direct_*, we stop using map/unmap_single
> >>> and just use the page version - this saves a lot of ugly
> >>> ifdeffing.  We leave map/unmap_single in the dma_ops definition,
> >>> though, because they are needed by the iommu code, which does not
> >>> implement map/unmap_page.  Ideally, going forward, we will  
> >>> completely
> >>> eliminate map/unmap_single and just have map/unmap_page, if it's
> >>> workable for 64-bit.
> >>>
> >>> Signed-off-by: Becky Bruce <becky.bruce@freescale.com>
> >>
> >> While doing a buildall this morning, I notice chrp32_defconfig fails
> >> to build with:
> >>
> >> drivers/built-in.o: In function `hard_dma_setup':
> >> floppy.c:(.text+0x6e40e): undefined reference to `isa_bridge_pcidev'
> >> floppy.c:(.text+0x6e412): undefined reference to `isa_bridge_pcidev'
> >> floppy.c:(.text+0x6e53e): undefined reference to `isa_bridge_pcidev'
> >> floppy.c:(.text+0x6e546): undefined reference to `isa_bridge_pcidev'
> >> floppy.c:(.text+0x6e54a): undefined reference to `isa_bridge_pcidev'
> >> make[1]: *** [.tmp_vmlinux1] Error 1
> >>
> >> (the hard_dma_setup thing is in arch/powerpc/include/asm/floppy.h).
> >>
> >> I did a git bisect and it pointed at this commit as causing the build
> >> to fail.  Why, I have no idea.
> >
> > Ok, I was annoyed enough to look at why.
> >
> > Basically, before this patch pci_map_single on 32-bit PPC seemed to
> > be compiled down to __dma_sync(ptr, size, direction); and the "dev"
> > parameter to the function was never actually used.  The compiler
> > seems to have optimized this out entirely, so we don't get the odd
> > link reference to isa_bridge_pcidev at all.  (Neither pci_map_single
> > or isa_bridge_pcidev are present in the vmlinux at all).
> >
> > With the patch, the compiler doesn't do this code elimination
> > because pci_map_single boils down to dma_map_page, which calls
> > get_dma_direct_offset with the "dev" parameter.  So since it is
> > still used, the compiler can't eliminate it and hence FAIL.
> >
> > I have no patch for this at the moment.  Someone should look at
> > it more closely, because this is causing the 5 chrp32_defconfig
> > users to weep.
> 
> Isn't this the type of regression we should fix post -rc1 :)

I don't think it matters much when it gets fixed, pre or post -rc1.  But
it should probably get fixed.  My hack was to pull isa_bridge_pcidev
into pci-common.c and export it from there.  The 64-bit PCI code can
initialized it, and the 32-bit can leave it NULL.  But I have no idea
if that is sane.  If so, I can probably submit a patch for it.

josh

Re: [PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code

[Kumar Gala]

>>>>>
>>>> While doing a buildall this morning, I notice chrp32_defconfig  
>>>> fails
>>>> to build with:
>>>>
>>>> drivers/built-in.o: In function `hard_dma_setup':
>>>> floppy.c:(.text+0x6e40e): undefined reference to  
>>>> `isa_bridge_pcidev'
>>>> floppy.c:(.text+0x6e412): undefined reference to  
>>>> `isa_bridge_pcidev'
>>>> floppy.c:(.text+0x6e53e): undefined reference to  
>>>> `isa_bridge_pcidev'
>>>> floppy.c:(.text+0x6e546): undefined reference to  
>>>> `isa_bridge_pcidev'
>>>> floppy.c:(.text+0x6e54a): undefined reference to  
>>>> `isa_bridge_pcidev'
>>>> make[1]: *** [.tmp_vmlinux1] Error 1
>>>>
>>>> (the hard_dma_setup thing is in arch/powerpc/include/asm/floppy.h).
>>>>
>>>> I did a git bisect and it pointed at this commit as causing the  
>>>> build
>>>> to fail.  Why, I have no idea.
>>>
>>> Ok, I was annoyed enough to look at why.
>>>
>>> Basically, before this patch pci_map_single on 32-bit PPC seemed to
>>> be compiled down to __dma_sync(ptr, size, direction); and the "dev"
>>> parameter to the function was never actually used.  The compiler
>>> seems to have optimized this out entirely, so we don't get the odd
>>> link reference to isa_bridge_pcidev at all.  (Neither pci_map_single
>>> or isa_bridge_pcidev are present in the vmlinux at all).
>>>
>>> With the patch, the compiler doesn't do this code elimination
>>> because pci_map_single boils down to dma_map_page, which calls
>>> get_dma_direct_offset with the "dev" parameter.  So since it is
>>> still used, the compiler can't eliminate it and hence FAIL.
>>>
>>> I have no patch for this at the moment.  Someone should look at
>>> it more closely, because this is causing the 5 chrp32_defconfig
>>> users to weep.
>>
>> Isn't this the type of regression we should fix post -rc1 :)
>
> I don't think it matters much when it gets fixed, pre or post -rc1.   
> But
> it should probably get fixed.  My hack was to pull isa_bridge_pcidev
> into pci-common.c and export it from there.  The 64-bit PCI code can
> initialized it, and the 32-bit can leave it NULL.  But I have no idea
> if that is sane.  If so, I can probably submit a patch for it.

I was just joking around about our "new" regression policy.. anyways I  
hope ben or maybe anton can comment about the ISA code.

- k

Re: [PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code

[Benjamin Herrenschmidt]

On Mon, 2008-10-13 at 13:22 -0500, Kumar Gala wrote:
> > I don't think it matters much when it gets fixed, pre or post -rc1.   
> > But
> > it should probably get fixed.  My hack was to pull isa_bridge_pcidev
> > into pci-common.c and export it from there.  The 64-bit PCI code can
> > initialized it, and the 32-bit can leave it NULL.  But I have no idea
> > if that is sane.  If so, I can probably submit a patch for it.
> 
> I was just joking around about our "new" regression policy.. anyways I  
> hope ben or maybe anton can comment about the ISA code.

My policy for that sort of bug is fix ASAP. I'll give it a go when I do
my test builds. Funny I didn't catch it, I might be lacking a chrp32
defconfig in my build tests :-)

Cheers,
Ben.

Re: [PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code

[Kumar Gala]

On Oct 13, 2008, at 6:30 PM, Benjamin Herrenschmidt wrote:

> On Mon, 2008-10-13 at 13:22 -0500, Kumar Gala wrote:
>>> I don't think it matters much when it gets fixed, pre or post -rc1.
>>> But
>>> it should probably get fixed.  My hack was to pull isa_bridge_pcidev
>>> into pci-common.c and export it from there.  The 64-bit PCI code can
>>> initialized it, and the 32-bit can leave it NULL.  But I have no  
>>> idea
>>> if that is sane.  If so, I can probably submit a patch for it.
>>
>> I was just joking around about our "new" regression policy..  
>> anyways I
>> hope ben or maybe anton can comment about the ISA code.
>
> My policy for that sort of bug is fix ASAP. I'll give it a go when I  
> do
> my test builds. Funny I didn't catch it, I might be lacking a chrp32
> defconfig in my build tests :-)

I pointed out to Stephen that kisskb.ellerman.id.au hasn't been  
updating.. there is a chrp32 defconfig in there that would normally  
catch something like this.

- k

Re: [PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code

[Josh Boyer]

On Mon, 13 Oct 2008 20:52:25 -0500
Kumar Gala <galak@kernel.crashing.org> wrote:

> 
> On Oct 13, 2008, at 6:30 PM, Benjamin Herrenschmidt wrote:
> 
> > On Mon, 2008-10-13 at 13:22 -0500, Kumar Gala wrote:
> >>> I don't think it matters much when it gets fixed, pre or post -rc1.
> >>> But
> >>> it should probably get fixed.  My hack was to pull isa_bridge_pcidev
> >>> into pci-common.c and export it from there.  The 64-bit PCI code can
> >>> initialized it, and the 32-bit can leave it NULL.  But I have no  
> >>> idea
> >>> if that is sane.  If so, I can probably submit a patch for it.
> >>
> >> I was just joking around about our "new" regression policy..  
> >> anyways I
> >> hope ben or maybe anton can comment about the ISA code.
> >
> > My policy for that sort of bug is fix ASAP. I'll give it a go when I  
> > do
> > my test builds. Funny I didn't catch it, I might be lacking a chrp32
> > defconfig in my build tests :-)
> 
> I pointed out to Stephen that kisskb.ellerman.id.au hasn't been  
> updating.. there is a chrp32 defconfig in there that would normally  
> catch something like this.

Except if kissb catches it, it's already committed in tree.  Better
than nothing, but it would be nice if people did a buildall before they
commit.

josh

Re: [PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code

[Benjamin Herrenschmidt]

On Tue, 2008-10-14 at 06:24 -0400, Josh Boyer wrote:
> > I pointed out to Stephen that kisskb.ellerman.id.au hasn't been  
> > updating.. there is a chrp32 defconfig in there that would
> normally  
> > catch something like this.
> 
> Except if kissb catches it, it's already committed in tree.  Better
> than nothing, but it would be nice if people did a buildall before
> they
> commit.

I have a script that builds a dozen of config's, it looks like it didn't
catch that one. I'm going to add a chrp32_defconfig to it. Mistakes
happen, hopefully we can at least make sure the common configs are well
tested.

Cheers,
Ben.

Re: [PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code

[Becky Bruce]

On Oct 14, 2008, at 7:05 AM, Benjamin Herrenschmidt wrote:

> On Tue, 2008-10-14 at 06:24 -0400, Josh Boyer wrote:
>>> I pointed out to Stephen that kisskb.ellerman.id.au hasn't been
>>> updating.. there is a chrp32 defconfig in there that would
>> normally
>>> catch something like this.
>>
>> Except if kissb catches it, it's already committed in tree.  Better
>> than nothing, but it would be nice if people did a buildall before
>> they
>> commit.
>
> I have a script that builds a dozen of config's, it looks like it  
> didn't
> catch that one. I'm going to add a chrp32_defconfig to it. Mistakes
> happen, hopefully we can at least make sure the common configs are  
> well
> tested.

Blah. I thought I had built everything before I pushed this stuff  
out.  My apologies (and also for being offline for when the fun  
occurred here.... I just got back from vacation this morning.)  Ben,  
thanks for fixing - let me know if there's anything you need from my  
end.

-B

Re: [PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code

[Benjamin Herrenschmidt]

On Mon, 2008-10-13 at 14:21 -0400, Josh Boyer wrote:
> I don't think it matters much when it gets fixed, pre or post -rc1.  But
> it should probably get fixed.  My hack was to pull isa_bridge_pcidev
> into pci-common.c and export it from there.  The 64-bit PCI code can
> initialized it, and the 32-bit can leave it NULL.  But I have no idea
> if that is sane.  If so, I can probably submit a patch for it.

I think the proper fix in the long run is to make the isa bridge
handling code in pci-64 common, but that's going to be after I rework
the IO space allocation/mapping mechanisms for pci-32 to also look like
pci-64 so it will take a little while.

In the meantime, I agree, just leave it to NULL, we can do a better
fixup on top of that. As it is, it's going to hurt Pegasos too (the
other 5 users :-)

Cheers,
Ben.

Re: [PATCH 2/4] POWERPC: Move iommu dma ops from dma.c to dma-iommu.c

[Christoph Hellwig]

On Mon, Sep 08, 2008 at 02:09:53PM -0500, Becky Bruce wrote:
> +/* We support DMA to/from any memory page via the iommu */
> +static int dma_iommu_dma_supported(struct device *dev, u64 mask)
> +{
> +	struct iommu_table *tbl = dev->archdata.dma_data;
> +
> +	if (!tbl || tbl->it_offset > mask) {
> +		printk(KERN_INFO
> +		       "Warning: IOMMU offset too big for device mask\n");
> +		if (tbl)
> +			printk(KERN_INFO
> +			       "mask: 0x%08lx, table offset: 0x%08lx\n",
> +				mask, tbl->it_offset);
> +		else
> +			printk(KERN_INFO "mask: 0x%08lx, table unavailable\n",
> +				mask);
> +		return 0;
> +	} else
> +		return 1;
> +}

Small nitpick, but wouldn't a

	if (tbl && tbl->it_offset <= mask)
		return 1;

	< the rest>

be much easier to read?

Re: [PATCH 2/4] POWERPC: Move iommu dma ops from dma.c to dma-iommu.c

[Becky Bruce]

On Sep 8, 2008, at 4:57 PM, Christoph Hellwig wrote:

> On Mon, Sep 08, 2008 at 02:09:53PM -0500, Becky Bruce wrote:
>> +/* We support DMA to/from any memory page via the iommu */
>> +static int dma_iommu_dma_supported(struct device *dev, u64 mask)
>> +{
>> +	struct iommu_table *tbl = dev->archdata.dma_data;
>> +
>> +	if (!tbl || tbl->it_offset > mask) {
>> +		printk(KERN_INFO
>> +		       "Warning: IOMMU offset too big for device mask\n");
>> +		if (tbl)
>> +			printk(KERN_INFO
>> +			       "mask: 0x%08lx, table offset: 0x%08lx\n",
>> +				mask, tbl->it_offset);
>> +		else
>> +			printk(KERN_INFO "mask: 0x%08lx, table unavailable\n",
>> +				mask);
>> +		return 0;
>> +	} else
>> +		return 1;
>> +}
>
> Small nitpick, but wouldn't a
>
> 	if (tbl && tbl->it_offset <= mask)
> 		return 1;
>
> 	< the rest>
>
> be much easier to read?

Seems cleaner to me..... but I want this patch, which is a move of the  
iommu code, to be *just* a move.  So I'm going to leave this as-is for  
now.

Thanks!
-Becky

Re: [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c

[Scott Wood]

Becky Bruce wrote:
> This is in preparation for the merge of the 32 and 64-bit
> dma code in arch/powerpc.
> 
> Signed-off-by: Becky Bruce <becky.bruce@freescale.com>
> ---
>  arch/powerpc/kernel/Makefile |    2 +-
>  arch/powerpc/kernel/dma.c    |  200 ++++++++++++++++++++++++++++++++++++++++++
>  arch/powerpc/kernel/dma_64.c |  200 ------------------------------------------
>  3 files changed, 201 insertions(+), 201 deletions(-)
>  create mode 100644 arch/powerpc/kernel/dma.c
>  delete mode 100644 arch/powerpc/kernel/dma_64.c

Passing -M to git format-patch makes it much easier to see whether 
anything changed between the old file and the new file.

-Scott

git apply vs. renamed files index mismatch (was: Re: [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c)

[Anton Vorontsov]

On Mon, Sep 08, 2008 at 02:18:42PM -0500, Scott Wood wrote:
> Becky Bruce wrote:
>> This is in preparation for the merge of the 32 and 64-bit
>> dma code in arch/powerpc.
>>
>> Signed-off-by: Becky Bruce <becky.bruce@freescale.com>
>> ---
>>  arch/powerpc/kernel/Makefile |    2 +-
>>  arch/powerpc/kernel/dma.c    |  200 ++++++++++++++++++++++++++++++++++++++++++
>>  arch/powerpc/kernel/dma_64.c |  200 ------------------------------------------
>>  3 files changed, 201 insertions(+), 201 deletions(-)
>>  create mode 100644 arch/powerpc/kernel/dma.c
>>  delete mode 100644 arch/powerpc/kernel/dma_64.c
>
> Passing -M to git format-patch makes it much easier

I always thought that posting "-M" patches to the public lists is
discouraged since it is quite difficult to apply them via patch(1).
Also think of non-git users...

> to see whether  
> anything changed between the old file and the new file.

This is still possible by comparing the hashes:

diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
new file mode 100644
index 0000000..ae5708e
[...]
diff --git a/arch/powerpc/kernel/dma_64.c b/arch/powerpc/kernel/dma_64.c
deleted file mode 100644
index ae5708e..0000000

That is, if hashes match then it was pure rename.

Though, too bad git {apply,am} does not produce any warnings if there
are any hidden changes...

Cc'ing git mailing list.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: git apply vs. renamed files index mismatch

[Scott Wood]

Anton Vorontsov wrote:
> I always thought that posting "-M" patches to the public lists is
> discouraged since it is quite difficult to apply them via patch(1).
> Also think of non-git users...

I think the substantially enhanced reviewability trumps non-git-users 
who can follow the rename instructions manually (or fix up their patch 
utility) if they insist on shunning tools that would make their life easier.

> diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
> new file mode 100644
> index 0000000..ae5708e
> [...]
> diff --git a/arch/powerpc/kernel/dma_64.c b/arch/powerpc/kernel/dma_64.c
> deleted file mode 100644
> index ae5708e..0000000
> 
> That is, if hashes match then it was pure rename.

I suppose, though it's not as easy to spot, and won't help in showing 
what the differences are if there are any.

-Scott

Re: git apply vs. renamed files index mismatch

[Anton Vorontsov]

On Mon, Sep 08, 2008 at 04:38:57PM -0500, Scott Wood wrote:
> Anton Vorontsov wrote:
>> I always thought that posting "-M" patches to the public lists is
>> discouraged since it is quite difficult to apply them via patch(1).
>> Also think of non-git users...
>
> I think the substantially enhanced reviewability trumps non-git-users  
> who can follow the rename instructions manually (or fix up their patch  
> utility) if they insist on shunning tools that would make their life 
> easier.

Can't disagree, I myself use git. ;-) But I'm also using standard
patch(1) to test occasional patches... And sometimes linux-X tree
isn't tracked by git (e.g. -mm otm snapshots).

>> diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
>> new file mode 100644
>> index 0000000..ae5708e
>> [...]
>> diff --git a/arch/powerpc/kernel/dma_64.c b/arch/powerpc/kernel/dma_64.c
>> deleted file mode 100644
>> index ae5708e..0000000
>>
>> That is, if hashes match then it was pure rename.
>
> I suppose, though it's not as easy to spot, and won't help in showing  
> what the differences are if there are any.

..rename and changes ideally go in separate patches.

IIRC this also helps git to track renames (it can easily compare
hashes instead of guessing).

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: git apply vs. renamed files index mismatch

[Junio C Hamano]

Anton Vorontsov <avorontsov@ru.mvista.com> writes:

> ..rename and changes ideally go in separate patches.
>
> IIRC this also helps git to track renames (it can easily compare
> hashes instead of guessing).

It does not help much, and it is frowned upon (at least by well educated
users in git circle) because such a split patch hurts reviewability.

Re: git apply vs. renamed files index mismatch

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1975 bytes --]

On Tue, 9 Sep 2008, Anton Vorontsov wrote:
> On Mon, Sep 08, 2008 at 04:38:57PM -0500, Scott Wood wrote:
> > Anton Vorontsov wrote:
> >> I always thought that posting "-M" patches to the public lists is
> >> discouraged since it is quite difficult to apply them via patch(1).
> >> Also think of non-git users...
> >
> > I think the substantially enhanced reviewability trumps non-git-users  
> > who can follow the rename instructions manually (or fix up their patch  
> > utility) if they insist on shunning tools that would make their life 
> > easier.
> 
> Can't disagree, I myself use git. ;-) But I'm also using standard
> patch(1) to test occasional patches... And sometimes linux-X tree
> isn't tracked by git (e.g. -mm otm snapshots).
> 
> >> diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
> >> new file mode 100644
> >> index 0000000..ae5708e
> >> [...]
> >> diff --git a/arch/powerpc/kernel/dma_64.c b/arch/powerpc/kernel/dma_64.c
> >> deleted file mode 100644
> >> index ae5708e..0000000
> >>
> >> That is, if hashes match then it was pure rename.
> >
> > I suppose, though it's not as easy to spot, and won't help in showing  
> > what the differences are if there are any.
> 
> ..rename and changes ideally go in separate patches.

Except if the rename also requires some small changes (cfr. the move of
include/asm-*). But if no changes are required to fix breakage caused by the
rename, then make the changes separate.

> IIRC this also helps git to track renames (it can easily compare
> hashes instead of guessing).

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010

Re: git apply vs. renamed files index mismatch (was: Re: [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c)

[Christoph Hellwig]

On Tue, Sep 09, 2008 at 01:27:17AM +0400, Anton Vorontsov wrote:
> I always thought that posting "-M" patches to the public lists is
> discouraged since it is quite difficult to apply them via patch(1).
> Also think of non-git users...

Yes, it's a horrible idea.

Re: git apply vs. renamed files index mismatch

[Junio C Hamano]

Anton Vorontsov <avorontsov@ru.mvista.com> writes:

>>>  3 files changed, 201 insertions(+), 201 deletions(-)
>>>  create mode 100644 arch/powerpc/kernel/dma.c
>>>  delete mode 100644 arch/powerpc/kernel/dma_64.c
>>
>> Passing -M to git format-patch makes it much easier
>
> I always thought that posting "-M" patches to the public lists is
> discouraged since it is quite difficult to apply them via patch(1).
> Also think of non-git users...

My understanding has been that it is encouraged on the kernel mailing
list, because the rename format is far easier to review by showing the
differences that matter to reviewers, than showing a big chunk of text
deleted and another big chunk of text that is similar added elsewhere.

I won't comment on this any further; the use of it is strictly a list and
community policy issue.

> This is still possible by comparing the hashes:
> ...
> That is, if hashes match then it was pure rename.
>
> Though, too bad git {apply,am} does not produce any warnings if there
> are any hidden changes...

But I _do_ want to know what you mean by this comment.  Your statement
makes it sounds as if apply/am happily and silently accept "hidden
changes" and it is a bad thing.

Now what do you exactly mean by "any hidden changes"?  Do you mean "the
sender did not use renaming format, the patch you fed was a one that
removes a huge chunk of text from one file, and adds a similarly huge
chunk of text to another file.  The changes to these files looked similar
but was not quite the same"?  It is all there for you to review, and
especially if you prefer non-renaming format, then that is what you get.
So I do not think that is what you are complaining about.  It must be
something else --- what is it?

Re: git apply vs. renamed files index mismatch

[Anton Vorontsov]

On Mon, Sep 08, 2008 at 05:53:41PM -0700, Junio C Hamano wrote:
> Anton Vorontsov <avorontsov@ru.mvista.com> writes:
> 
> >>>  3 files changed, 201 insertions(+), 201 deletions(-)
> >>>  create mode 100644 arch/powerpc/kernel/dma.c
> >>>  delete mode 100644 arch/powerpc/kernel/dma_64.c
> >>
> >> Passing -M to git format-patch makes it much easier
> >
> > I always thought that posting "-M" patches to the public lists is
> > discouraged since it is quite difficult to apply them via patch(1).
> > Also think of non-git users...
> 
> My understanding has been that it is encouraged on the kernel mailing
> list, because the rename format is far easier to review by showing the
> differences that matter to reviewers, than showing a big chunk of text
> deleted and another big chunk of text that is similar added elsewhere.
> 
> I won't comment on this any further; the use of it is strictly a list and
> community policy issue.
> 
> > This is still possible by comparing the hashes:
> > ...
> > That is, if hashes match then it was pure rename.
> >
> > Though, too bad git {apply,am} does not produce any warnings if there
> > are any hidden changes...
> 
> But I _do_ want to know what you mean by this comment.  Your statement
> makes it sounds as if apply/am happily and silently accept "hidden
> changes" and it is a bad thing.
> 
> Now what do you exactly mean by "any hidden changes"?  Do you mean "the
> sender did not use renaming format, the patch you fed was a one that
> removes a huge chunk of text from one file, and adds a similarly huge
> chunk of text to another file.  The changes to these files looked similar
> but was not quite the same"?  It is all there for you to review, and
> especially if you prefer non-renaming format, then that is what you get.

As I said, "index .." lines that git puts into patches are useful to see
if there any changes has been made to a renamed file. So usually I don't
have to look through the whole patch to see if there are any changes,
I can just look into the patch and conclude: "this is git patch, and the
overhead information says that it is rename-only patch. It should
be safe."

Now consider the following patch (modified by hand: it should say
+foo, but I changed it to +bar).

diff --git a/file b/file
deleted file mode 100644
index 257cc56..0000000
--- a/file
+++ /dev/null
@@ -1 +0,0 @@
-foo
diff --git a/file_renamed b/file_renamed
new file mode 100644
index 0000000..257cc56
--- /dev/null
+++ b/file_renamed
@@ -0,0 +1 @@
+bar


The "index ..." stuff says that there are no changes and it is
pure rename, but obviously there is a change.

What would be great is to have is some warning (or error), that
is: "git-am: patch claims that it would only rename the file %s,
but it also changes things in that file. Somebody tried to make
a fool of you."

Makes sense?

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: git apply vs. renamed files index mismatch

[Junio C Hamano]

Anton Vorontsov <avorontsov@ru.mvista.com> writes:

> Now consider the following patch (modified by hand: it should say
> +foo, but I changed it to +bar).
> ...
> The "index ..." stuff says that there are no changes and it is
> pure rename, but obviously there is a change.

Ah, I see what you mean.  But in general, it is not obvious at all.

If you have the identical preimage (recorded on the LHS of the index line)
or the patch reproduces the postimage in full (i.e. "create a new file"),
you *could* notice.  It's an interesting idea from git person's point of
view (i.e. "would be fun to implement"), but I doubt it would be useful in
practice, because:

 (1) You often do not have the identically matching preimage;

 (2) More importantly, it is not unusual for people to *edit* the patch in
     their MUA (think of typofixes), after getting it out of git.

 (3) Even more importantly, even if you notice there is some difference,
     you cannot produce the postimage by only looking at the hash (this is
     obvious -- otherwise by definition you broke SHA-1), so you cannot
     tell *how* the patch was modified.

What is of much more practical value to learn here in the context of this
topic would be that after accepting such a patch that does not use -M (so
that non-git people can use patch(1) to apply), a git person can still
verify the result with "git show -M" to see what changes other than a pure
rename was made by the patch.

Re: git apply vs. renamed files index mismatch

[Anton Vorontsov]

On Tue, Sep 09, 2008 at 07:45:19AM -0700, Junio C Hamano wrote:
> Anton Vorontsov <avorontsov@ru.mvista.com> writes:
> 
> > Now consider the following patch (modified by hand: it should say
> > +foo, but I changed it to +bar).
> > ...
> > The "index ..." stuff says that there are no changes and it is
> > pure rename, but obviously there is a change.
> 
> Ah, I see what you mean.  But in general, it is not obvious at all.
> 
> If you have the identical preimage (recorded on the LHS of the index line)
> or the patch reproduces the postimage in full (i.e. "create a new file"),
> you *could* notice.  It's an interesting idea from git person's point of
> view (i.e. "would be fun to implement"), but I doubt it would be useful in
> practice, because:
> 
>  (1) You often do not have the identically matching preimage;
> 
>  (2) More importantly, it is not unusual for people to *edit* the patch in
>      their MUA (think of typofixes), after getting it out of git.

Not for rename patches...

>  (3) Even more importantly, even if you notice there is some difference,

Just noticing that there is a difference is enough.

As for implementing, isn't this as simple as this pseudo code:

if (index_deleted_file == index_new_file)
	if (deleted_file != new_file)
		printk("warning\n");

In the git-apply?

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: git apply vs. renamed files index mismatch

[Junio C Hamano]

Anton Vorontsov <avorontsov@ru.mvista.com> writes:

> On Tue, Sep 09, 2008 at 07:45:19AM -0700, Junio C Hamano wrote:
> ...
>> ...  It's an interesting idea from git person's point of
>> view (i.e. "would be fun to implement"), but I doubt it would be useful in
>> practice, because:
>> 
>>  (1) You often do not have the identically matching preimage;
>> 
>>  (2) More importantly, it is not unusual for people to *edit* the patch in
>>      their MUA (think of typofixes), after getting it out of git.
>
> Not for rename patches...

a. Why not?  Even if your patch is (totally uninteresting) pure rename, it
   is natural to review the patch before you send out, and it also is
   natural to get tempted to fix typoes, just for a straight normal patch.

b. If you can expect good behaviour out of people, by declaring "Not for
   rename patches" as your guarantee, what's the point of this discussion?

> As for implementing, isn't this as simple as this pseudo code:
>
> if (index_deleted_file == index_new_file)
> 	if (deleted_file != new_file)
> 		printk("warning\n");
>
> In the git-apply?

Implementation is easy (I said "would be fun to code", didn't I? --- by the
way, how did you match "index_deleted_file" with "index_new_file"?).

My point was that it would not be reliable enough to be useful in
practice.

Re: [PATCH 0/4] POWERPC: 32/64-bit DMA code merge and cleanup

[Christoph Hellwig]

On Mon, Sep 08, 2008 at 02:09:51PM -0500, Becky Bruce wrote:
> I have tested this code on 8641 and 8572 platforms using the popular
> kernel build as well as full runs of LTP.  I have built 64-bit
> platforms, but have no means to test there, so I would greatly
> appreciate any feedback you can provide on possible issues. 

I'll give it a spin on Cell.