Patchwork [01/51] DMA-API: provide a helper to set both DMA and coherent DMA masks

login
register
mail settings
Submitter Russell King
Date Sept. 19, 2013, 9:25 p.m.
Message ID <E1VMlja-0007fA-HU@rmk-PC.arm.linux.org.uk>
Download mbox | patch
Permalink /patch/276082/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Russell King - Sept. 19, 2013, 9:25 p.m.
Provide a helper to set both the DMA and coherent DMA masks to the
same value - this avoids duplicated code in a number of drivers,
sometimes with buggy error handling, and also allows us identify
which drivers do things differently.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 Documentation/DMA-API-HOWTO.txt |   37 ++++++++++++++++++++++---------------
 Documentation/DMA-API.txt       |    8 ++++++++
 include/linux/dma-mapping.h     |   14 ++++++++++++++
 3 files changed, 44 insertions(+), 15 deletions(-)
Ben Hutchings - Sept. 20, 2013, 1:21 a.m.
On Thu, 2013-09-19 at 22:25 +0100, Russell King wrote:
> Provide a helper to set both the DMA and coherent DMA masks to the
> same value - this avoids duplicated code in a number of drivers,
> sometimes with buggy error handling, and also allows us identify
> which drivers do things differently.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>  Documentation/DMA-API-HOWTO.txt |   37 ++++++++++++++++++++++---------------
>  Documentation/DMA-API.txt       |    8 ++++++++
>  include/linux/dma-mapping.h     |   14 ++++++++++++++
>  3 files changed, 44 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/DMA-API-HOWTO.txt b/Documentation/DMA-API-HOWTO.txt
> index 14129f1..5e98303 100644
> --- a/Documentation/DMA-API-HOWTO.txt
> +++ b/Documentation/DMA-API-HOWTO.txt
[...]
> -dma_set_coherent_mask() will always be able to set the same or a
> -smaller mask as dma_set_mask(). However for the rare case that a
> +The coherent coherent mask will always be able to set the same or a
> +smaller mask as the streaming mask. However for the rare case that a
[...]

The new wording doesn't make sense; a mask doesn't set itself.  I would
suggest:

"The coherent mask can always be set to the same or a smaller mask than
the streaming mask."

Ben.
Russell King - ARM Linux - Sept. 20, 2013, 2:12 p.m.
On Fri, Sep 20, 2013 at 02:21:37AM +0100, Ben Hutchings wrote:
> On Thu, 2013-09-19 at 22:25 +0100, Russell King wrote:
> [...]
> > -dma_set_coherent_mask() will always be able to set the same or a
> > -smaller mask as dma_set_mask(). However for the rare case that a
> > +The coherent coherent mask will always be able to set the same or a
> > +smaller mask as the streaming mask. However for the rare case that a
> [...]
> 
> The new wording doesn't make sense; a mask doesn't set itself.  I would
> suggest:
> 
> "The coherent mask can always be set to the same or a smaller mask than
> the streaming mask."

Yes, the original sentence is not particularly good, but I think even
your modified version can be interpreted as "a mask setting itself"
for all the same reasons that the original can be (which mask does "same"
refer to?)

Even so, I prefer your version.  Thanks. :)
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/Documentation/DMA-API-HOWTO.txt b/Documentation/DMA-API-HOWTO.txt
index 14129f1..5e98303 100644
--- a/Documentation/DMA-API-HOWTO.txt
+++ b/Documentation/DMA-API-HOWTO.txt
@@ -101,14 +101,23 @@  style to do this even if your device holds the default setting,
 because this shows that you did think about these issues wrt. your
 device.
 
-The query is performed via a call to dma_set_mask():
+The query is performed via a call to dma_set_mask_and_coherent():
 
-	int dma_set_mask(struct device *dev, u64 mask);
+	int dma_set_mask_and_coherent(struct device *dev, u64 mask);
 
-The query for consistent allocations is performed via a call to
-dma_set_coherent_mask():
+which will query the mask for both streaming and coherent APIs together.
+If you have some special requirements, then the following two separate
+queries can be used instead:
 
-	int dma_set_coherent_mask(struct device *dev, u64 mask);
+	The query for streaming mappings is performed via a call to
+	dma_set_mask():
+
+		int dma_set_mask(struct device *dev, u64 mask);
+
+	The query for consistent allocations is performed via a call
+	to dma_set_coherent_mask():
+
+		int dma_set_coherent_mask(struct device *dev, u64 mask);
 
 Here, dev is a pointer to the device struct of your device, and mask
 is a bit mask describing which bits of an address your device
@@ -137,7 +146,7 @@  exactly why.
 
 The standard 32-bit addressing device would do something like this:
 
-	if (dma_set_mask(dev, DMA_BIT_MASK(32))) {
+	if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32))) {
 		printk(KERN_WARNING
 		       "mydev: No suitable DMA available.\n");
 		goto ignore_this_device;
@@ -171,22 +180,20 @@  If a card is capable of using 64-bit consistent allocations as well,
 
 	int using_dac, consistent_using_dac;
 
-	if (!dma_set_mask(dev, DMA_BIT_MASK(64))) {
+	if (!dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64))) {
 		using_dac = 1;
 	   	consistent_using_dac = 1;
-		dma_set_coherent_mask(dev, DMA_BIT_MASK(64));
-	} else if (!dma_set_mask(dev, DMA_BIT_MASK(32))) {
+	} else if (!dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32))) {
 		using_dac = 0;
 		consistent_using_dac = 0;
-		dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
 	} else {
 		printk(KERN_WARNING
 		       "mydev: No suitable DMA available.\n");
 		goto ignore_this_device;
 	}
 
-dma_set_coherent_mask() will always be able to set the same or a
-smaller mask as dma_set_mask(). However for the rare case that a
+The coherent coherent mask will always be able to set the same or a
+smaller mask as the streaming mask. However for the rare case that a
 device driver only uses consistent allocations, one would have to
 check the return value from dma_set_coherent_mask().
 
@@ -199,9 +206,9 @@  Finally, if your device can only drive the low 24-bits of
 		goto ignore_this_device;
 	}
 
-When dma_set_mask() is successful, and returns zero, the kernel saves
-away this mask you have provided.  The kernel will use this
-information later when you make DMA mappings.
+When dma_set_mask() or dma_set_mask_and_coherent() is successful, and
+returns zero, the kernel saves away this mask you have provided.  The
+kernel will use this information later when you make DMA mappings.
 
 There is a case which we are aware of at this time, which is worth
 mentioning in this documentation.  If your device supports multiple
diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
index 78a6c56..e865279 100644
--- a/Documentation/DMA-API.txt
+++ b/Documentation/DMA-API.txt
@@ -142,6 +142,14 @@  internal API for use by the platform than an external API for use by
 driver writers.
 
 int
+dma_set_mask_and_coherent(struct device *dev, u64 mask)
+
+Checks to see if the mask is possible and updates the device
+streaming and coherent DMA mask parameters if it is.
+
+Returns: 0 if successful and a negative error if not.
+
+int
 dma_set_mask(struct device *dev, u64 mask)
 
 Checks to see if the mask is possible and updates the device
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 3a8d0a2..ec951f9 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -97,6 +97,20 @@  static inline int dma_set_coherent_mask(struct device *dev, u64 mask)
 }
 #endif
 
+/*
+ * Set both the DMA mask and the coherent DMA mask to the same thing.
+ * Note that we don't check the return value from dma_set_coherent_mask()
+ * as the DMA API guarantees that the coherent DMA mask can be set to
+ * the same or smaller than the streaming DMA mask.
+ */
+static inline int dma_set_mask_and_coherent(struct device *dev, u64 mask)
+{
+	int rc = dma_set_mask(dev, mask);
+	if (rc == 0)
+		dma_set_coherent_mask(dev, mask);
+	return rc;
+}
+
 extern u64 dma_get_required_mask(struct device *dev);
 
 static inline unsigned int dma_get_max_seg_size(struct device *dev)