Skip to contentSkip to conversation

move consistent_dma_mask to the generic device

PATCH In review James Bottomley 2 participants 3 messages Feb 26, 2004 – Feb 27, 2004

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 move consistent_dma_mask to the generic device

This is a reroll of the previous patch. It fixes up badness in the sound drivers which were accessing the dma_mask directly instead of using the accessor functions.

It also modifies the arch's that were using consistent_dma_mask:

x86_64, ia64 (both sn and hp) and parisc

Here's the previous description:

pci_dev.consistent_dma_mask was introduced to get around problems in the IA64 Altix machine.

Now, we have a use for it in x86: the aacraid needs coherent memory in a 31 bit address range (2GB). Unfortunately, x86 is converted to the dma model, so it can't see the pci_dev by the time coherent memory is allocated.

The solution to all of this is to move pci_dev.consistent_dma_mask to dev.coherent_dma_mask and make x86 use it in the dma_alloc_coherent() calls.

This should allow me to make the aacraid set the coherent mask instead of using it's current dma_mask juggling.

James

===== arch/i386/kernel/pci-dma.c 1.11 vs edited =====

arch/i386/kernel/pci-dma.c +2 -1
@@ -20,8 +20,9 @@
 	/* ignore region specifiers */
 	gfp &= ~(__GFP_DMA | __GFP_HIGHMEM);
 
-	if (dev == NULL || (*dev->dma_mask < 0xffffffff))
+	if (dev == NULL || (dev->coherent_dma_mask < 0xffffffff))
 		gfp |= GFP_DMA;
+
 	ret = (void *)__get_free_pages(gfp, get_order(size));
 
 	if (ret != NULL) {

===== arch/ia64/hp/common/sba_iommu.c 1.37 vs edited =====

arch/ia64/hp/common/sba_iommu.c +3 -3
@@ -1055,13 +1055,13 @@
 	*dma_handle = virt_to_phys(addr);
 
 #ifdef ALLOW_IOV_BYPASS
-	ASSERT(to_pci_dev(dev)->consistent_dma_mask);
+	ASSERT(dev->coherent_dma_mask);
 	/*
  	** Check if the PCI device can DMA to ptr... if so, just return ptr
  	*/
-	if (likely((*dma_handle & ~to_pci_dev(dev)->consistent_dma_mask) == 0)) {
+	if (likely((*dma_handle & ~dev->coherent_dma_mask) == 0)) {
 		DBG_BYPASS("sba_alloc_coherent() bypass mask/addr: 0x%lx/0x%lx\n",
-		           to_pci_dev(dev)->consistent_dma_mask, *dma_handle);
+		           dev->coherent_dma_mask, *dma_handle);
 
 		return addr;
 	}

===== arch/ia64/sn/io/machvec/pci_dma.c 1.26 vs edited =====

arch/ia64/sn/io/machvec/pci_dma.c +2 -2
@@ -152,7 +152,7 @@
 	 *   pcibr_dmatrans_addr ignores a missing PCIIO_DMA_A64 flag on
 	 *   PCI-X buses.
 	 */
-	if (hwdev->consistent_dma_mask == ~0UL)
+	if (hwdev->dev.coherent_dma_mask == ~0UL)
 		*dma_handle = pcibr_dmatrans_addr(vhdl, NULL, phys_addr, size,
 					  PCIIO_DMA_CMD | PCIIO_DMA_A64);
 	else {
@@ -169,7 +169,7 @@
 		}
 	}
 
-	if (!*dma_handle || *dma_handle > hwdev->consistent_dma_mask) {
+	if (!*dma_handle || *dma_handle > hwdev->dev.coherent_dma_mask) {
 		if (dma_map) {
 			pcibr_dmamap_done(dma_map);
 			pcibr_dmamap_free(dma_map);

===== arch/parisc/kernel/drivers.c 1.10 vs edited =====

arch/parisc/kernel/drivers.c +1 -0
@@ -618,6 +618,7 @@
 		 tmp1);
 	/* make the generic dma mask a pointer to the parisc one */
 	dev->dev.dma_mask = &dev->dma_mask;
+	dev->dev.coherent_dma_mask = dev->dma_mask;
 	pr_debug("device_register(%s)\n", dev->dev.bus_id);
 	device_register(&dev->dev);
 }

===== arch/parisc/kernel/pci-dma.c 1.7 vs edited =====

arch/parisc/kernel/pci-dma.c +1 -1
@@ -372,7 +372,7 @@
 ** ISA cards will certainly only support 24-bit DMA addressing.
 ** Not clear if we can, want, or need to support ISA.
 */
-	if (!dev || *dev->dma_mask != 0xffffffff)
+	if (!dev || *dev->coherent_dma_mask < 0xffffffff)
 		gfp |= GFP_DMA;
 #endif
 	return (void *)vaddr;

===== arch/x86_64/kernel/pci-gart.c 1.29 vs edited =====

arch/x86_64/kernel/pci-gart.c +1 -1
@@ -177,7 +177,7 @@
 		gfp |= GFP_DMA; 
 		dma_mask = 0xffffffff; 
 	} else {
-		dma_mask = hwdev->consistent_dma_mask; 
+		dma_mask = hwdev->dev.coherent_dma_mask; 
 	}
 
 	if (dma_mask == 0) 

===== drivers/eisa/eisa-bus.c 1.14 vs edited =====

drivers/eisa/eisa-bus.c +1 -0
@@ -187,6 +187,7 @@
 	edev->dev.parent = root->dev;
 	edev->dev.bus = &eisa_bus_type;
 	edev->dev.dma_mask = &edev->dma_mask;
+	edev->dev.coherent_dma_mask = edev->dma_mask;
 	sprintf (edev->dev.bus_id, "%02X:%02X", root->bus_nr, slot);
 
 	for (i = 0; i < EISA_MAX_RESOURCES; i++) {

===== drivers/mca/mca-bus.c 1.5 vs edited =====

drivers/mca/mca-bus.c +1 -0
@@ -106,6 +106,7 @@
 	sprintf (mca_dev->dev.bus_id, "%02d:%02X", bus, mca_dev->slot);
 	mca_dev->dma_mask = mca_bus->default_dma_mask;
 	mca_dev->dev.dma_mask = &mca_dev->dma_mask;
+	mca_dev->dev.coherent_dma_mask = mca_dev->dma_mask;
 
 	if (device_register(&mca_dev->dev))
 		return 0;

===== drivers/pci/pci.c 1.61 vs edited =====

drivers/pci/pci.c +1 -1
@@ -691,7 +691,7 @@
 	if (!pci_dma_supported(dev, mask))
 		return -EIO;
 
-	dev->consistent_dma_mask = mask;
+	dev->dev.coherent_dma_mask = mask;
 
 	return 0;
 }

===== drivers/pci/probe.c 1.59 vs edited =====

drivers/pci/probe.c +1 -1
@@ -566,7 +566,6 @@
 	/* Assume 32-bit PCI; let 64-bit PCI cards (which are far rarer)
 	   set this higher, assuming the system even supports it.  */
 	dev->dma_mask = 0xffffffff;
-	dev->consistent_dma_mask = 0xffffffff;
 	if (pci_setup_device(dev) < 0) {
 		kfree(dev);
 		return NULL;
@@ -578,6 +577,7 @@
 	pci_name_device(dev);
 
 	dev->dev.dma_mask = &dev->dma_mask;
+	dev->dev.coherent_dma_mask = 0xffffffffull;
 
 	return dev;
 }

===== include/linux/device.h 1.115 vs edited =====

include/linux/device.h +6 -0
@@ -285,6 +285,12 @@
 					   detached from its driver. */
 
 	u64		*dma_mask;	/* dma mask (if dma'able device) */
+	u64		coherent_dma_mask;/* Like dma_mask, but for
+					     alloc_coherent mappings as
+					     not all hardware supports
+					     64 bit addresses for consistent
+					     allocations such descriptors. */
+
 	struct list_head	dma_pools;	/* dma pools (if dma'ble) */
 
 	void	(*release)(struct device * dev);

===== include/linux/pci.h 1.117 vs edited =====

include/linux/pci.h +0 -5
@@ -392,11 +392,6 @@
 					   this if your device has broken DMA
 					   or supports 64-bit transfers.  */
 
-	u64		consistent_dma_mask;/* Like dma_mask, but for
-					       pci_alloc_consistent mappings as
-					       not all hardware supports
-					       64 bit addresses for consistent
-					       allocations such descriptors. */
 	u32             current_state;  /* Current operating state. In ACPI-speak,
 					   this is D0-D3, D0 being fully functional,
 					   and D3 being off. */

===== sound/core/memalloc.c 1.16 vs edited =====

sound/core/memalloc.c +4 -4
@@ -100,13 +100,13 @@
 	if (hwdev == NULL)
 		return pci_alloc_consistent(hwdev, size, dma_handle);
 	dma_mask = hwdev->dma_mask;
-	cdma_mask = hwdev->consistent_dma_mask;
+	cdma_mask = hwdev->dev.coherent_dma_mask;
 	mask = (unsigned long)dma_mask && (unsigned long)cdma_mask;
 	hwdev->dma_mask = 0xffffffff; /* do without masking */
-	hwdev->consistent_dma_mask = 0xffffffff; /* do without masking */
+	hwdev->dev.coherent_dma_mask = 0xffffffff; /* do without masking */
 	ret = pci_alloc_consistent(hwdev, size, dma_handle);
 	hwdev->dma_mask = dma_mask; /* restore */
-	hwdev->consistent_dma_mask = cdma_mask; /* restore */
+	hwdev->dev.coherent_dma_mask = cdma_mask; /* restore */
 	if (ret) {
 		/* obtained address is out of range? */
 		if (((unsigned long)*dma_handle + size - 1) & ~mask) {
@@ -648,7 +648,7 @@
 	dma_addr_t addr;
 	unsigned long mask;
 
-	mask = pci ? (unsigned long)pci->consistent_dma_mask : 0x00ffffffUL;
+	mask = pci ? (unsigned long)pci->dev.coherent_dma_mask : 0x00ffffffUL;
 	ptr = (void *)__get_free_page(GFP_KERNEL);
 	if (ptr) {
 		addr = virt_to_phys(ptr);
#1 lore raw

I haven't had a chance to try it yet, but it looks good.

If you're going to get rid of pci_dev.consistent_dma_mask in favor of pci_dev.dev.coherent_dma_mask, would you want to do the same with pci_dev.dma_mask?

Which brings to mind a second question; why is device.dma_mask a u64 * instead of u64? Does it typically point to pci_dev.dma_mask?

thanks

jeremy

Quoted 7 lines · James Bottomley

This is a reroll of the previous patch. It fixes up badness in the sound drivers which were accessing the dma_mask directly instead of using the accessor functions.

It also modifies the arch's that were using consistent_dma_mask:

x86_64, ia64 (both sn and hp) and parisc

#2 lore raw
Quoted 5 lines · Jeremy Higdon

I haven't had a chance to try it yet, but it looks good.

If you're going to get rid of pci_dev.consistent_dma_mask in favor of pci_dev.dev.coherent_dma_mask, would you want to do the same with pci_dev.dma_mask?

It's probably about time that was done, yes.

Quoted 2 lines

Which brings to mind a second question; why is device.dma_mask a u64 * instead of u64? Does it typically point to pci_dev.dma_mask?

That's a bad design decision that will forever haunt me. When I first proposed moving from the PCI DMA model to the generic device DMA model, the dma_mask was in the wrong place. Quite a few drivers touched it themselves outside of the accessor functions, so actually moving it in to struct device became quite involved, so I took the easy way out and simply made the entry in struct device a pointer to the real one so that anything that updated the mask outside of the accessors would still work.

I suppose I should really do the work as pennance, plus write out a hundred times "never sacrifice design integrity for expediency", sigh.

James