* [PATCH 0/4] POWERPC: 32/64-bit DMA code merge and cleanup @ 2008-09-08 19:09 Becky Bruce 2008-09-08 19:09 ` [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c Becky Bruce 2008-09-08 21:56 ` [PATCH 0/4] POWERPC: 32/64-bit DMA code merge and cleanup Christoph Hellwig 0 siblings, 2 replies; 39+ messages in thread From: Becky Bruce @ 2008-09-08 19:09 UTC (permalink / raw) To: linuxppc-dev 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c 2008-09-08 19:09 [PATCH 0/4] POWERPC: 32/64-bit DMA code merge and cleanup Becky Bruce @ 2008-09-08 19:09 ` Becky Bruce 2008-09-08 19:09 ` [PATCH 2/4] POWERPC: Move iommu dma ops from dma.c to dma-iommu.c Becky Bruce 2008-09-08 19:18 ` [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c Scott Wood 2008-09-08 21:56 ` [PATCH 0/4] POWERPC: 32/64-bit DMA code merge and cleanup Christoph Hellwig 1 sibling, 2 replies; 39+ messages in thread From: Becky Bruce @ 2008-09-08 19:09 UTC (permalink / raw) To: linuxppc-dev 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 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 2/4] POWERPC: Move iommu dma ops from dma.c to dma-iommu.c 2008-09-08 19:09 ` [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c Becky Bruce @ 2008-09-08 19:09 ` Becky Bruce 2008-09-08 19:09 ` [PATCH 3/4] POWERPC: Drop archdata numa_node Becky Bruce 2008-09-08 21:57 ` [PATCH 2/4] POWERPC: Move iommu dma ops from dma.c to dma-iommu.c Christoph Hellwig 2008-09-08 19:18 ` [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c Scott Wood 1 sibling, 2 replies; 39+ messages in thread From: Becky Bruce @ 2008-09-08 19:09 UTC (permalink / raw) To: linuxppc-dev 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 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 3/4] POWERPC: Drop archdata numa_node 2008-09-08 19:09 ` [PATCH 2/4] POWERPC: Move iommu dma ops from dma.c to dma-iommu.c Becky Bruce @ 2008-09-08 19:09 ` Becky Bruce 2008-09-08 19:09 ` [PATCH 4/4] POWERPC: Merge 32 and 64-bit dma code Becky Bruce 2008-09-08 21:57 ` [PATCH 2/4] POWERPC: Move iommu dma ops from dma.c to dma-iommu.c Christoph Hellwig 1 sibling, 1 reply; 39+ messages in thread From: Becky Bruce @ 2008-09-08 19:09 UTC (permalink / raw) To: linuxppc-dev 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 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 4/4] POWERPC: Merge 32 and 64-bit dma code 2008-09-08 19:09 ` [PATCH 3/4] POWERPC: Drop archdata numa_node Becky Bruce @ 2008-09-08 19:09 ` Becky Bruce 2008-09-08 22:03 ` Christoph Hellwig 2008-09-12 20:34 ` [PATCH V2 " Becky Bruce 0 siblings, 2 replies; 39+ messages in thread From: Becky Bruce @ 2008-09-08 19:09 UTC (permalink / raw) To: linuxppc-dev 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 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 4/4] POWERPC: Merge 32 and 64-bit dma code 2008-09-08 19:09 ` [PATCH 4/4] POWERPC: Merge 32 and 64-bit dma code Becky Bruce @ 2008-09-08 22:03 ` Christoph Hellwig 2008-09-09 10:54 ` Benjamin Herrenschmidt 2008-09-09 14:39 ` Becky Bruce 2008-09-12 20:34 ` [PATCH V2 " Becky Bruce 1 sibling, 2 replies; 39+ messages in thread From: Christoph Hellwig @ 2008-09-08 22:03 UTC (permalink / raw) To: Becky Bruce; +Cc: linuxppc-dev > - 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? ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 4/4] POWERPC: Merge 32 and 64-bit dma code 2008-09-08 22:03 ` Christoph Hellwig @ 2008-09-09 10:54 ` Benjamin Herrenschmidt 2008-09-09 14:39 ` Becky Bruce 1 sibling, 0 replies; 39+ messages in thread From: Benjamin Herrenschmidt @ 2008-09-09 10:54 UTC (permalink / raw) To: Christoph Hellwig; +Cc: linuxppc-dev 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 4/4] POWERPC: Merge 32 and 64-bit dma code 2008-09-08 22:03 ` Christoph Hellwig 2008-09-09 10:54 ` Benjamin Herrenschmidt @ 2008-09-09 14:39 ` Becky Bruce 2008-09-09 20:45 ` Christoph Hellwig 1 sibling, 1 reply; 39+ messages in thread From: Becky Bruce @ 2008-09-09 14:39 UTC (permalink / raw) To: Christoph Hellwig; +Cc: linuxppc-dev 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 4/4] POWERPC: Merge 32 and 64-bit dma code 2008-09-09 14:39 ` Becky Bruce @ 2008-09-09 20:45 ` Christoph Hellwig 2008-09-09 22:10 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 39+ messages in thread From: Christoph Hellwig @ 2008-09-09 20:45 UTC (permalink / raw) To: Becky Bruce; +Cc: linuxppc-dev, 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 4/4] POWERPC: Merge 32 and 64-bit dma code 2008-09-09 20:45 ` Christoph Hellwig @ 2008-09-09 22:10 ` Benjamin Herrenschmidt 2008-09-09 22:13 ` Christoph Hellwig 0 siblings, 1 reply; 39+ messages in thread From: Benjamin Herrenschmidt @ 2008-09-09 22:10 UTC (permalink / raw) To: Christoph Hellwig; +Cc: linuxppc-dev > 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 4/4] POWERPC: Merge 32 and 64-bit dma code 2008-09-09 22:10 ` Benjamin Herrenschmidt @ 2008-09-09 22:13 ` Christoph Hellwig 2008-09-09 22:17 ` Becky Bruce 0 siblings, 1 reply; 39+ messages in thread From: Christoph Hellwig @ 2008-09-09 22:13 UTC (permalink / raw) To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, 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--- ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 4/4] POWERPC: Merge 32 and 64-bit dma code 2008-09-09 22:13 ` Christoph Hellwig @ 2008-09-09 22:17 ` Becky Bruce 0 siblings, 0 replies; 39+ messages in thread From: Becky Bruce @ 2008-09-09 22:17 UTC (permalink / raw) To: Christoph Hellwig; +Cc: linuxppc-dev 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code 2008-09-08 19:09 ` [PATCH 4/4] POWERPC: Merge 32 and 64-bit dma code Becky Bruce 2008-09-08 22:03 ` Christoph Hellwig @ 2008-09-12 20:34 ` Becky Bruce 2008-10-13 14:49 ` Josh Boyer 1 sibling, 1 reply; 39+ messages in thread From: Becky Bruce @ 2008-09-12 20:34 UTC (permalink / raw) To: linuxppc-dev 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 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code 2008-09-12 20:34 ` [PATCH V2 " Becky Bruce @ 2008-10-13 14:49 ` Josh Boyer 2008-10-13 15:41 ` Josh Boyer 0 siblings, 1 reply; 39+ messages in thread From: Josh Boyer @ 2008-10-13 14:49 UTC (permalink / raw) To: Becky Bruce; +Cc: linuxppc-dev 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code 2008-10-13 14:49 ` Josh Boyer @ 2008-10-13 15:41 ` Josh Boyer 2008-10-13 18:06 ` Kumar Gala 0 siblings, 1 reply; 39+ messages in thread From: Josh Boyer @ 2008-10-13 15:41 UTC (permalink / raw) To: Becky Bruce; +Cc: linuxppc-dev 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code 2008-10-13 15:41 ` Josh Boyer @ 2008-10-13 18:06 ` Kumar Gala 2008-10-13 18:21 ` Josh Boyer 0 siblings, 1 reply; 39+ messages in thread From: Kumar Gala @ 2008-10-13 18:06 UTC (permalink / raw) To: Josh Boyer; +Cc: linuxppc-dev 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code 2008-10-13 18:06 ` Kumar Gala @ 2008-10-13 18:21 ` Josh Boyer 2008-10-13 18:22 ` Kumar Gala 2008-10-13 23:30 ` Benjamin Herrenschmidt 0 siblings, 2 replies; 39+ messages in thread From: Josh Boyer @ 2008-10-13 18:21 UTC (permalink / raw) To: Kumar Gala; +Cc: linuxppc-dev 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code 2008-10-13 18:21 ` Josh Boyer @ 2008-10-13 18:22 ` Kumar Gala 2008-10-13 23:30 ` Benjamin Herrenschmidt 2008-10-13 23:30 ` Benjamin Herrenschmidt 1 sibling, 1 reply; 39+ messages in thread From: Kumar Gala @ 2008-10-13 18:22 UTC (permalink / raw) To: Josh Boyer; +Cc: linuxppc-dev >>>>> >>>> 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code 2008-10-13 18:22 ` Kumar Gala @ 2008-10-13 23:30 ` Benjamin Herrenschmidt 2008-10-14 1:52 ` Kumar Gala 0 siblings, 1 reply; 39+ messages in thread From: Benjamin Herrenschmidt @ 2008-10-13 23:30 UTC (permalink / raw) To: Kumar Gala; +Cc: linuxppc-dev 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code 2008-10-13 23:30 ` Benjamin Herrenschmidt @ 2008-10-14 1:52 ` Kumar Gala 2008-10-14 10:24 ` Josh Boyer 0 siblings, 1 reply; 39+ messages in thread From: Kumar Gala @ 2008-10-14 1:52 UTC (permalink / raw) To: benh; +Cc: linuxppc-dev 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code 2008-10-14 1:52 ` Kumar Gala @ 2008-10-14 10:24 ` Josh Boyer 2008-10-14 12:05 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 39+ messages in thread From: Josh Boyer @ 2008-10-14 10:24 UTC (permalink / raw) To: Kumar Gala; +Cc: linuxppc-dev 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code 2008-10-14 10:24 ` Josh Boyer @ 2008-10-14 12:05 ` Benjamin Herrenschmidt 2008-10-14 15:45 ` Becky Bruce 0 siblings, 1 reply; 39+ messages in thread From: Benjamin Herrenschmidt @ 2008-10-14 12:05 UTC (permalink / raw) To: Josh Boyer; +Cc: linuxppc-dev 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code 2008-10-14 12:05 ` Benjamin Herrenschmidt @ 2008-10-14 15:45 ` Becky Bruce 0 siblings, 0 replies; 39+ messages in thread From: Becky Bruce @ 2008-10-14 15:45 UTC (permalink / raw) To: benh; +Cc: linuxppc-dev 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH V2 4/4] POWERPC: Merge 32 and 64-bit dma code 2008-10-13 18:21 ` Josh Boyer 2008-10-13 18:22 ` Kumar Gala @ 2008-10-13 23:30 ` Benjamin Herrenschmidt 1 sibling, 0 replies; 39+ messages in thread From: Benjamin Herrenschmidt @ 2008-10-13 23:30 UTC (permalink / raw) To: Josh Boyer; +Cc: linuxppc-dev 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/4] POWERPC: Move iommu dma ops from dma.c to dma-iommu.c 2008-09-08 19:09 ` [PATCH 2/4] POWERPC: Move iommu dma ops from dma.c to dma-iommu.c Becky Bruce 2008-09-08 19:09 ` [PATCH 3/4] POWERPC: Drop archdata numa_node Becky Bruce @ 2008-09-08 21:57 ` Christoph Hellwig 2008-09-12 15:32 ` Becky Bruce 1 sibling, 1 reply; 39+ messages in thread From: Christoph Hellwig @ 2008-09-08 21:57 UTC (permalink / raw) To: Becky Bruce; +Cc: linuxppc-dev 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? ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/4] POWERPC: Move iommu dma ops from dma.c to dma-iommu.c 2008-09-08 21:57 ` [PATCH 2/4] POWERPC: Move iommu dma ops from dma.c to dma-iommu.c Christoph Hellwig @ 2008-09-12 15:32 ` Becky Bruce 0 siblings, 0 replies; 39+ messages in thread From: Becky Bruce @ 2008-09-12 15:32 UTC (permalink / raw) To: Christoph Hellwig; +Cc: linuxppc-dev 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c 2008-09-08 19:09 ` [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c Becky Bruce 2008-09-08 19:09 ` [PATCH 2/4] POWERPC: Move iommu dma ops from dma.c to dma-iommu.c Becky Bruce @ 2008-09-08 19:18 ` Scott Wood 2008-09-08 21:27 ` git apply vs. renamed files index mismatch (was: Re: [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c) Anton Vorontsov 1 sibling, 1 reply; 39+ messages in thread From: Scott Wood @ 2008-09-08 19:18 UTC (permalink / raw) To: Becky Bruce; +Cc: linuxppc-dev 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* git apply vs. renamed files index mismatch (was: Re: [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c) 2008-09-08 19:18 ` [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c Scott Wood @ 2008-09-08 21:27 ` Anton Vorontsov 2008-09-08 21:38 ` git apply vs. renamed files index mismatch Scott Wood ` (2 more replies) 0 siblings, 3 replies; 39+ messages in thread From: Anton Vorontsov @ 2008-09-08 21:27 UTC (permalink / raw) To: Scott Wood; +Cc: git, linuxppc-dev 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: git apply vs. renamed files index mismatch 2008-09-08 21:27 ` git apply vs. renamed files index mismatch (was: Re: [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c) Anton Vorontsov @ 2008-09-08 21:38 ` Scott Wood 2008-09-08 21:54 ` Anton Vorontsov 2008-09-08 21:58 ` git apply vs. renamed files index mismatch (was: Re: [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c) Christoph Hellwig 2008-09-09 0:53 ` git apply vs. renamed files index mismatch Junio C Hamano 2 siblings, 1 reply; 39+ messages in thread From: Scott Wood @ 2008-09-08 21:38 UTC (permalink / raw) To: avorontsov; +Cc: git, linuxppc-dev 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: git apply vs. renamed files index mismatch 2008-09-08 21:38 ` git apply vs. renamed files index mismatch Scott Wood @ 2008-09-08 21:54 ` Anton Vorontsov 2008-09-09 0:55 ` Junio C Hamano 2008-09-09 9:06 ` Geert Uytterhoeven 0 siblings, 2 replies; 39+ messages in thread From: Anton Vorontsov @ 2008-09-08 21:54 UTC (permalink / raw) To: Scott Wood; +Cc: git, linuxppc-dev 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: git apply vs. renamed files index mismatch 2008-09-08 21:54 ` Anton Vorontsov @ 2008-09-09 0:55 ` Junio C Hamano 2008-09-09 9:06 ` Geert Uytterhoeven 1 sibling, 0 replies; 39+ messages in thread From: Junio C Hamano @ 2008-09-09 0:55 UTC (permalink / raw) To: avorontsov; +Cc: Scott Wood, git, linuxppc-dev 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: git apply vs. renamed files index mismatch 2008-09-08 21:54 ` Anton Vorontsov 2008-09-09 0:55 ` Junio C Hamano @ 2008-09-09 9:06 ` Geert Uytterhoeven 1 sibling, 0 replies; 39+ messages in thread From: Geert Uytterhoeven @ 2008-09-09 9:06 UTC (permalink / raw) To: Anton Vorontsov; +Cc: Scott Wood, linuxppc-dev, git [-- 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: git apply vs. renamed files index mismatch (was: Re: [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c) 2008-09-08 21:27 ` git apply vs. renamed files index mismatch (was: Re: [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c) Anton Vorontsov 2008-09-08 21:38 ` git apply vs. renamed files index mismatch Scott Wood @ 2008-09-08 21:58 ` Christoph Hellwig 2008-09-09 0:53 ` git apply vs. renamed files index mismatch Junio C Hamano 2 siblings, 0 replies; 39+ messages in thread From: Christoph Hellwig @ 2008-09-08 21:58 UTC (permalink / raw) To: Anton Vorontsov; +Cc: Scott Wood, linuxppc-dev, git 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: git apply vs. renamed files index mismatch 2008-09-08 21:27 ` git apply vs. renamed files index mismatch (was: Re: [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c) Anton Vorontsov 2008-09-08 21:38 ` git apply vs. renamed files index mismatch Scott Wood 2008-09-08 21:58 ` git apply vs. renamed files index mismatch (was: Re: [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c) Christoph Hellwig @ 2008-09-09 0:53 ` Junio C Hamano 2008-09-09 10:06 ` Anton Vorontsov 2 siblings, 1 reply; 39+ messages in thread From: Junio C Hamano @ 2008-09-09 0:53 UTC (permalink / raw) To: avorontsov; +Cc: Scott Wood, git, linuxppc-dev 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? ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: git apply vs. renamed files index mismatch 2008-09-09 0:53 ` git apply vs. renamed files index mismatch Junio C Hamano @ 2008-09-09 10:06 ` Anton Vorontsov 2008-09-09 14:45 ` Junio C Hamano 0 siblings, 1 reply; 39+ messages in thread From: Anton Vorontsov @ 2008-09-09 10:06 UTC (permalink / raw) To: Junio C Hamano; +Cc: Scott Wood, git, linuxppc-dev 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 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: git apply vs. renamed files index mismatch 2008-09-09 10:06 ` Anton Vorontsov @ 2008-09-09 14:45 ` Junio C Hamano 2008-09-09 15:14 ` Anton Vorontsov 0 siblings, 1 reply; 39+ messages in thread From: Junio C Hamano @ 2008-09-09 14:45 UTC (permalink / raw) To: avorontsov; +Cc: Scott Wood, git, linuxppc-dev 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: git apply vs. renamed files index mismatch 2008-09-09 14:45 ` Junio C Hamano @ 2008-09-09 15:14 ` Anton Vorontsov 2008-09-10 3:31 ` Junio C Hamano 0 siblings, 1 reply; 39+ messages in thread From: Anton Vorontsov @ 2008-09-09 15:14 UTC (permalink / raw) To: Junio C Hamano; +Cc: Scott Wood, git, linuxppc-dev 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: git apply vs. renamed files index mismatch 2008-09-09 15:14 ` Anton Vorontsov @ 2008-09-10 3:31 ` Junio C Hamano 0 siblings, 0 replies; 39+ messages in thread From: Junio C Hamano @ 2008-09-10 3:31 UTC (permalink / raw) To: avorontsov; +Cc: Scott Wood, git, linuxppc-dev 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 0/4] POWERPC: 32/64-bit DMA code merge and cleanup 2008-09-08 19:09 [PATCH 0/4] POWERPC: 32/64-bit DMA code merge and cleanup Becky Bruce 2008-09-08 19:09 ` [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c Becky Bruce @ 2008-09-08 21:56 ` Christoph Hellwig 1 sibling, 0 replies; 39+ messages in thread From: Christoph Hellwig @ 2008-09-08 21:56 UTC (permalink / raw) To: Becky Bruce; +Cc: linuxppc-dev 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2008-10-15 0:05 UTC | newest] Thread overview: 39+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-09-08 19:09 [PATCH 0/4] POWERPC: 32/64-bit DMA code merge and cleanup Becky Bruce 2008-09-08 19:09 ` [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c Becky Bruce 2008-09-08 19:09 ` [PATCH 2/4] POWERPC: Move iommu dma ops from dma.c to dma-iommu.c Becky Bruce 2008-09-08 19:09 ` [PATCH 3/4] POWERPC: Drop archdata numa_node Becky Bruce 2008-09-08 19:09 ` [PATCH 4/4] POWERPC: Merge 32 and 64-bit dma code Becky Bruce 2008-09-08 22:03 ` Christoph Hellwig 2008-09-09 10:54 ` Benjamin Herrenschmidt 2008-09-09 14:39 ` Becky Bruce 2008-09-09 20:45 ` Christoph Hellwig 2008-09-09 22:10 ` Benjamin Herrenschmidt 2008-09-09 22:13 ` Christoph Hellwig 2008-09-09 22:17 ` Becky Bruce 2008-09-12 20:34 ` [PATCH V2 " Becky Bruce 2008-10-13 14:49 ` Josh Boyer 2008-10-13 15:41 ` Josh Boyer 2008-10-13 18:06 ` Kumar Gala 2008-10-13 18:21 ` Josh Boyer 2008-10-13 18:22 ` Kumar Gala 2008-10-13 23:30 ` Benjamin Herrenschmidt 2008-10-14 1:52 ` Kumar Gala 2008-10-14 10:24 ` Josh Boyer 2008-10-14 12:05 ` Benjamin Herrenschmidt 2008-10-14 15:45 ` Becky Bruce 2008-10-13 23:30 ` Benjamin Herrenschmidt 2008-09-08 21:57 ` [PATCH 2/4] POWERPC: Move iommu dma ops from dma.c to dma-iommu.c Christoph Hellwig 2008-09-12 15:32 ` Becky Bruce 2008-09-08 19:18 ` [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c Scott Wood 2008-09-08 21:27 ` git apply vs. renamed files index mismatch (was: Re: [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c) Anton Vorontsov 2008-09-08 21:38 ` git apply vs. renamed files index mismatch Scott Wood 2008-09-08 21:54 ` Anton Vorontsov 2008-09-09 0:55 ` Junio C Hamano 2008-09-09 9:06 ` Geert Uytterhoeven 2008-09-08 21:58 ` git apply vs. renamed files index mismatch (was: Re: [PATCH 1/4] POWERPC: Rename dma_64.c to dma.c) Christoph Hellwig 2008-09-09 0:53 ` git apply vs. renamed files index mismatch Junio C Hamano 2008-09-09 10:06 ` Anton Vorontsov 2008-09-09 14:45 ` Junio C Hamano 2008-09-09 15:14 ` Anton Vorontsov 2008-09-10 3:31 ` Junio C Hamano 2008-09-08 21:56 ` [PATCH 0/4] POWERPC: 32/64-bit DMA code merge and cleanup Christoph Hellwig
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).