Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752244AbZGNAuL (ORCPT ); Mon, 13 Jul 2009 20:50:11 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752193AbZGNAuJ (ORCPT ); Mon, 13 Jul 2009 20:50:09 -0400 Received: from sh.osrg.net ([192.16.179.4]:54830 "EHLO sh.osrg.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751990AbZGNAuG (ORCPT ); Mon, 13 Jul 2009 20:50:06 -0400 Date: Tue, 14 Jul 2009 09:49:55 +0900 To: beckyb@kernel.crashing.org Cc: fujita.tomonori@lab.ntt.co.jp, linuxppc-dev@lists.ozlabs.org, benh@kernel.crashing.org, galak@kernel.crashing.org, linux-kernel@vger.kernel.org Subject: Re: removing addr_needs_map in struct dma_mapping_ops From: FUJITA Tomonori In-Reply-To: References: <20090710104706I.fujita.tomonori@lab.ntt.co.jp> Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-Id: <20090714094919V.fujita.tomonori@lab.ntt.co.jp> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-3.0 (sh.osrg.net [192.16.179.4]); Tue, 14 Jul 2009 09:49:56 +0900 (JST) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8424 Lines: 240 On Mon, 13 Jul 2009 16:50:43 -0500 Becky Bruce wrote: > > talked about defining something like struct dma_data. Then we could > > > > struct dev_archdata { > > ... > > > > struct dma_data *ddata; > > }; > > > > or > > > > struct dev_archdata { > > ... > > > > struct dma_data ddata; > > }; > > > > > > struct dma_data needs dma_direct_offset, iommu_table, dma_base, and > > dma_window_size, anything else? > > IIRC, what we had talked about was simpler - we talked about changing > the current dev_archdata from this: > > struct dev_archdata { > struct device_node *of_node; > struct dma_mapping_ops *dma_ops; > void *dma_data; > }; > > to this: > > struct dev_archdata { > struct device_node *of_node; > struct dma_mapping_ops *dma_ops; > unsigned long long dma_data; > #ifdef CONFIG_SWIOTLB > dma_addr_t max_direct_dma_addr; > #endif > }; > > Where max_direct_dma_addr is the address beyond which a specific > device must use swiotlb, and dma_data is the offset like it is now > (but wider on 32-bit systems than void *). I believe Ben had mentioned > wanting to make the max_direct_dma_addr part conditional so we don't > bloat archdata on platforms that don't ever bounce. Only maximum address is enough? The minimum (dma_window_base_cur in swiotlb_pci_addr_needs_map) is not necessary? > The change to the type of dma_data is actually in preparation for an > optimization I have planned for 64-bit PCI devices (and which probably > requires more discussion), so that doesn't need to happen now - just > leave it as a void *, and I can post a followup patch. > > Let me know if I can help or do any testing - I've been meaning to > look into switching to dma_map_ops for a while now but it hasn't > managed to pop off my todo stack. Ok, how about this? I'm not familiar with POWERPC so I might misunderstand something. diff --git a/arch/powerpc/include/asm/device.h b/arch/powerpc/include/asm/device.h index 7d2277c..0086f8d 100644 --- a/arch/powerpc/include/asm/device.h +++ b/arch/powerpc/include/asm/device.h @@ -16,6 +16,9 @@ struct dev_archdata { /* DMA operations on that device */ struct dma_mapping_ops *dma_ops; void *dma_data; +#ifdef CONFIG_SWIOTLB + dma_addr_t max_direct_dma_addr; +#endif }; static inline void dev_archdata_set_node(struct dev_archdata *ad, diff --git a/arch/powerpc/include/asm/swiotlb.h b/arch/powerpc/include/asm/swiotlb.h index 30891d6..b23a4f1 100644 --- a/arch/powerpc/include/asm/swiotlb.h +++ b/arch/powerpc/include/asm/swiotlb.h @@ -24,4 +24,6 @@ static inline void dma_mark_clean(void *addr, size_t size) {} extern unsigned int ppc_swiotlb_enable; int __init swiotlb_setup_bus_notifier(void); +extern void pci_dma_dev_setup_swiotlb(struct pci_dev *pdev); + #endif /* __ASM_SWIOTLB_H */ diff --git a/arch/powerpc/kernel/dma-swiotlb.c b/arch/powerpc/kernel/dma-swiotlb.c index 68ccf11..e21359e 100644 --- a/arch/powerpc/kernel/dma-swiotlb.c +++ b/arch/powerpc/kernel/dma-swiotlb.c @@ -56,39 +56,16 @@ swiotlb_arch_address_needs_mapping(struct device *hwdev, dma_addr_t addr, size_t size) { struct dma_mapping_ops *dma_ops = get_dma_ops(hwdev); + struct dev_archdata *sd = &hwdev->archdata; BUG_ON(!dma_ops); - return dma_ops->addr_needs_map(hwdev, addr, size); -} -/* - * Determine if an address is reachable by a pci device, or if we must bounce. - */ -static int -swiotlb_pci_addr_needs_map(struct device *hwdev, dma_addr_t addr, size_t size) -{ - u64 mask = dma_get_mask(hwdev); - dma_addr_t max; - struct pci_controller *hose; - struct pci_dev *pdev = to_pci_dev(hwdev); - - hose = pci_bus_to_host(pdev->bus); - max = hose->dma_window_base_cur + hose->dma_window_size; - - /* check that we're within mapped pci window space */ - if ((addr + size > max) | (addr < hose->dma_window_base_cur)) + if (sd->max_direct_dma_addr && addr + size > sd->max_direct_dma_addr) return 1; - return !is_buffer_dma_capable(mask, addr, size); -} - -static int -swiotlb_addr_needs_map(struct device *hwdev, dma_addr_t addr, size_t size) -{ return !is_buffer_dma_capable(dma_get_mask(hwdev), addr, size); } - /* * At the moment, all platforms that use this code only require * swiotlb to be used if we're operating on HIGHMEM. Since @@ -104,7 +81,6 @@ struct dma_mapping_ops swiotlb_dma_ops = { .dma_supported = swiotlb_dma_supported, .map_page = swiotlb_map_page, .unmap_page = swiotlb_unmap_page, - .addr_needs_map = swiotlb_addr_needs_map, .sync_single_range_for_cpu = swiotlb_sync_single_range_for_cpu, .sync_single_range_for_device = swiotlb_sync_single_range_for_device, .sync_sg_for_cpu = swiotlb_sync_sg_for_cpu, @@ -119,13 +95,23 @@ struct dma_mapping_ops swiotlb_pci_dma_ops = { .dma_supported = swiotlb_dma_supported, .map_page = swiotlb_map_page, .unmap_page = swiotlb_unmap_page, - .addr_needs_map = swiotlb_pci_addr_needs_map, .sync_single_range_for_cpu = swiotlb_sync_single_range_for_cpu, .sync_single_range_for_device = swiotlb_sync_single_range_for_device, .sync_sg_for_cpu = swiotlb_sync_sg_for_cpu, .sync_sg_for_device = swiotlb_sync_sg_for_device }; +void pci_dma_dev_setup_swiotlb(struct pci_dev *pdev) +{ + struct pci_controller *hose; + struct dev_archdata *sd; + + hose = pci_bus_to_host(pdev->bus); + sd = &pdev->dev.archdata; + sd->max_direct_dma_addr = + hose->dma_window_base_cur + hose->dma_window_size; +} + static int ppc_swiotlb_bus_notify(struct notifier_block *nb, unsigned long action, void *data) { diff --git a/arch/powerpc/platforms/85xx/mpc8536_ds.c b/arch/powerpc/platforms/85xx/mpc8536_ds.c index 055ff41..401751b 100644 --- a/arch/powerpc/platforms/85xx/mpc8536_ds.c +++ b/arch/powerpc/platforms/85xx/mpc8536_ds.c @@ -136,6 +136,7 @@ define_machine(mpc8536_ds) { .init_IRQ = mpc8536_ds_pic_init, #ifdef CONFIG_PCI .pcibios_fixup_bus = fsl_pcibios_fixup_bus, + .pci_dma_dev_setup = pci_dma_dev_setup_swiotlb, #endif .get_irq = mpic_get_irq, .restart = fsl_rstcr_restart, diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/mpc85xx_ds.c index 849c0ac..1ba8e38 100644 --- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c +++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c @@ -277,6 +277,7 @@ define_machine(mpc8544_ds) { .init_IRQ = mpc85xx_ds_pic_init, #ifdef CONFIG_PCI .pcibios_fixup_bus = fsl_pcibios_fixup_bus, + .pci_dma_dev_setup = pci_dma_dev_setup_swiotlb, #endif .get_irq = mpic_get_irq, .restart = fsl_rstcr_restart, @@ -291,6 +292,7 @@ define_machine(mpc8572_ds) { .init_IRQ = mpc85xx_ds_pic_init, #ifdef CONFIG_PCI .pcibios_fixup_bus = fsl_pcibios_fixup_bus, + .pci_dma_dev_setup = pci_dma_dev_setup_swiotlb, #endif .get_irq = mpic_get_irq, .restart = fsl_rstcr_restart, @@ -305,6 +307,7 @@ define_machine(p2020_ds) { .init_IRQ = mpc85xx_ds_pic_init, #ifdef CONFIG_PCI .pcibios_fixup_bus = fsl_pcibios_fixup_bus, + .pci_dma_dev_setup = pci_dma_dev_setup_swiotlb, #endif .get_irq = mpic_get_irq, .restart = fsl_rstcr_restart, diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c index 60ed9c0..165a2de 100644 --- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c +++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c @@ -356,6 +356,7 @@ define_machine(mpc8568_mds) { .progress = udbg_progress, #ifdef CONFIG_PCI .pcibios_fixup_bus = fsl_pcibios_fixup_bus, + .pci_dma_dev_setup = pci_dma_dev_setup_swiotlb, #endif }; @@ -377,5 +378,6 @@ define_machine(mpc8569_mds) { .progress = udbg_progress, #ifdef CONFIG_PCI .pcibios_fixup_bus = fsl_pcibios_fixup_bus, + .pci_dma_dev_setup = pci_dma_dev_setup_swiotlb, #endif }; diff --git a/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c b/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c index 6632702..d1878f3 100644 --- a/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c +++ b/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c @@ -187,5 +187,6 @@ define_machine(mpc86xx_hpcn) { .progress = udbg_progress, #ifdef CONFIG_PCI .pcibios_fixup_bus = fsl_pcibios_fixup_bus, + .pci_dma_dev_setup = pci_dma_dev_setup_swiotlb, #endif }; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/