login
Header Space

 
 

Re: [patch v2] doc: DMA-mapping.txt has undeclared variables [Bug 10397]

Previous thread: [patch 22/22 -v3] xtensa: Use generic show_mem() by Johannes Weiner on Monday, April 7, 2008 - 10:40 am. (1 message)

Next thread: [PATCH] Trivial: change %{_smp_flags} to {?_smp_mflags} in RPM spec file by Peter Samuelson on Monday, April 7, 2008 - 12:18 pm. (1 message)
To: Grant Grundler <grundler@...>, <gregkh@...>
Cc: <linux-kernel@...>, <linux-pci@...>
Date: Monday, April 7, 2008 - 12:01 pm

From: Matti Linnanvuori &lt;mattilinnanvuori@yahoo.com&gt;

Fix undeclared variables in code examples.
Consistently use pdev as the variable of type struct pci_dev *.
Bug 10397.

Signed-off-by: Matti Linnanvuori &lt;mattilinnanvuori@yahoo.com&gt;

---

--- linux-2.6/Documentation/DMA-mapping.txt	2008-03-23 09:01:06.304511500 +0200
+++ linux/Documentation/DMA-mapping.txt	2008-04-06 06:56:11.821314500 +0300
@@ -315,9 +315,9 @@
 
 	dma_addr_t dma_handle;
 
-	cpu_addr = pci_alloc_consistent(dev, size, &amp;dma_handle);
+	cpu_addr = pci_alloc_consistent(pdev, size, &amp;dma_handle);
 
-where dev is a struct pci_dev *. You should pass NULL for PCI like buses
+where pdev is a struct pci_dev *. You should pass NULL for PCI like buses
 where devices don't have struct pci_dev (like ISA, EISA).  This may be
 called in interrupt context. 
 
@@ -354,9 +354,9 @@
 
 To unmap and free such a DMA region, you call:
 
-	pci_free_consistent(dev, size, cpu_addr, dma_handle);
+	pci_free_consistent(pdev, size, cpu_addr, dma_handle);
 
-where dev, size are the same as in the above call and cpu_addr and
+where pdev, size are the same as in the above call and cpu_addr and
 dma_handle are the values pci_alloc_consistent returned to you.
 This function may not be called in interrupt context.
 
@@ -371,9 +371,9 @@
 
 	struct pci_pool *pool;
 
-	pool = pci_pool_create(name, dev, size, align, alloc);
+	pool = pci_pool_create(name, pdev, size, align, alloc);
 
-The "name" is for diagnostics (like a kmem_cache name); dev and size
+The "name" is for diagnostics (like a kmem_cache name); pdev and size
 are as above.  The device's hardware alignment requirement for this
 type of data is "align" (which is expressed in bytes, and must be a
 power of two).  If your device has no boundary crossing restrictions,
@@ -472,11 +472,11 @@
 	void *addr = buffer-&gt;ptr;
 	size_t size = buffer-&gt;len;
 
-	dma_handle = pci_map_single(dev, addr, size, direction);
+	dma_handle = pci_map_single(pdev, addr, size...
To: <linux-pci@...>
Cc: Matti Linnanvuori <mattilinnanvuori@...>, Grant Grundler <grundler@...>, <gregkh@...>, <linux-kernel@...>
Date: Monday, April 7, 2008 - 12:05 pm

I think Grant's comment still stands, "fix undeclared variables" isn't an 
accurate description of what you're doing here...

That said, the patch itself looks fine to me, if a bit pedantic.  The compiler 
will catch any mismatched arguments in this case, and the rest of the 
documentation is pretty clear that these routines take struct pci_dev not 
struct device, so I'd be inclined to just leave it the way it is to avoid 
churn.

Jesse
--
To: Jesse Barnes <jbarnes@...>
Cc: <linux-pci@...>, Matti Linnanvuori <mattilinnanvuori@...>, Grant Grundler <grundler@...>, <gregkh@...>, <linux-kernel@...>
Date: Wednesday, April 9, 2008 - 2:45 pm

I think it's a good habit to use "pdev" in device drivers and would
like the documentation to incorporate that.

I'm ok with the churn since it's not conflicting with anything else AFAIK.

thanks,
grant

--
To: Grant Grundler <grundler@...>
Cc: <linux-pci@...>, Matti Linnanvuori <mattilinnanvuori@...>, <linux-kernel@...>
Date: Wednesday, April 23, 2008 - 1:58 pm

Matti, can you post an update with Grant's comments addressed?  It's mainly 
about the changelog comment at this point, which should read something 
like "Update DMA mapping documentation to use 'pdev' rather than 'dev' in 
example code that calls routines expecting 'struct pci_device *', since 'dev' 
might make readers think they're passing 'struct device *' parameters."

Assuming it looks ok I'll go ahead and put it into the PCI tree.

Thanks,
Jesse

--
Previous thread: [patch 22/22 -v3] xtensa: Use generic show_mem() by Johannes Weiner on Monday, April 7, 2008 - 10:40 am. (1 message)

Next thread: [PATCH] Trivial: change %{_smp_flags} to {?_smp_mflags} in RPM spec file by Peter Samuelson on Monday, April 7, 2008 - 12:18 pm. (1 message)
speck-geostationary