2008-01-30 05:52:20

by Arthur Kepner

[permalink] [raw]
Subject: [PATCH 1/4] dma: document dma_{un}map_{single|sg}_attrs() interface


Document the new dma_{un}map_{single|sg}_attrs() functions.

Signed-off-by: Arthur Kepner <[email protected]>

--

DMA-API.txt | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 63 insertions(+)

diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
index b939ebb..fad05e0 100644
--- a/Documentation/DMA-API.txt
+++ b/Documentation/DMA-API.txt
@@ -395,6 +395,69 @@ Notes: You must do this:

See also dma_map_single().

+dma_addr_t
+dma_map_single_attrs(struct device *dev, void *cpu_addr, size_t size,
+ enum dma_data_direction dir,
+ struct dma_attrs* attrs)
+
+void
+dma_unmap_single_attrs(struct device *dev, dma_addr_t dma_addr,
+ size_t size, enum dma_data_direction dir,
+ struct dma_attrs* attrs)
+
+int
+dma_map_sg_attrs(struct device *dev, struct scatterlist *sgl,
+ int nents, enum dma_data_direction dir,
+ struct dma_attrs *attrs)
+
+void
+dma_unmap_sg_attrs(struct device *dev, struct scatterlist *sgl,
+ int nents, enum dma_data_direction dir,
+ struct dma_attrs *attrs)
+
+The four functions above are just like the counterpart functions
+without the _attrs suffixes, except that they pass an optional
+struct dma_attrs*.
+
+struct dma_attrs encapsulates a set of "dma attributes". For the
+definition of struct dma_attrs see linux/dma-attrs.h.
+
+The interpretation of dma attributes is architecture-specific.
+
+If struct dma_attrs* is NULL, the semantics of each of these
+functions is identical to those of the corresponding function
+without the _attrs suffix. As a result dma_map_single_attrs()
+can generally replace dma_map_single(), etc.
+
+As an example of the use of the *_attrs functions, here's how
+you could pass an attribute DMA_ATTR_FOO when mapping memory
+for DMA:
+
+#include <linux/dma-attrs.h>
+/* DMA_ATTR_FOO should be defined in linux/dma-attrs.h */
+...
+
+ DECLARE_DMA_ATTRS(attrs);
+ dma_set_attr(&attrs, DMA_ATTR_FOO);
+ ....
+ n = dma_map_sg_attrs(dev, sg, nents, DMA_TO_DEVICE, &attr);
+ ....
+
+Architectures that care about DMA_ATTR_FOO would check for its
+presence in their implementations of the mapping and unmapping
+routines, e.g.:
+
+void whizco_dma_map_sg_attrs(struct device *dev, dma_addr_t dma_addr,
+ size_t size, enum dma_data_direction dir,
+ struct dma_attrs* attrs)
+{
+ ....
+ int foo = dma_get_attr(attrs, DMA_ATTR_FOO);
+ ....
+ if (foo)
+ /* twizzle the frobnozzle */
+ ....
+

Part II - Advanced dma_ usage
-----------------------------


2008-01-31 22:39:17

by Grant Grundler

[permalink] [raw]
Subject: Re: [PATCH 1/4] dma: document dma_{un}map_{single|sg}_attrs() interface

On Tue, Jan 29, 2008 at 09:50:40PM -0800, [email protected] wrote:
>
> Document the new dma_{un}map_{single|sg}_attrs() functions.

Thank you!

Looks good to me...so far I've only found one nit.

...
> +struct dma_attrs encapsulates a set of "dma attributes". For the
> +definition of struct dma_attrs see linux/dma-attrs.h.

Because all architectures will share the set of attrs definitions,
would it be reasonable to document the intent of each attr?

Two reasons for doing this:
1) people reading the driver code should understand WHY the dma attr was added.
2) other arch maintainers need to know in order to implement the same attr
for their shiny new toys.


> +The interpretation of dma attributes is architecture-specific.

This statement is really important....but:
Could we add a reference to arch documentation for each attr?

ie something public (doesn't have to be in linux source tree)
which explains the subtlies of how that DMA attr actually works.

Having worked on HP chipsets for 10+ years, I know releasing
original HW docs is often not possible. I'm not asking for the
impossible. Please don't flame me for that. But if the company
is willing to publish the existance of a feature, a paragraph or
two would be good marketing too. Maybe just include comments in
the arch/ code that implements the feature and reference those
comments in DMA-API.txt.


> +If struct dma_attrs* is NULL, the semantics of each of these
> +functions is identical to those of the corresponding function
> +without the _attrs suffix. As a result dma_map_single_attrs()
> +can generally replace dma_map_single(), etc.
> +
> +As an example of the use of the *_attrs functions, here's how
> +you could pass an attribute DMA_ATTR_FOO when mapping memory
> +for DMA:
> +
> +#include <linux/dma-attrs.h>
> +/* DMA_ATTR_FOO should be defined in linux/dma-attrs.h */
> +...
> +
> + DECLARE_DMA_ATTRS(attrs);
> + dma_set_attr(&attrs, DMA_ATTR_FOO);
> + ....
> + n = dma_map_sg_attrs(dev, sg, nents, DMA_TO_DEVICE, &attr);
> + ....
> +
> +Architectures that care about DMA_ATTR_FOO would check for its
> +presence in their implementations of the mapping and unmapping
> +routines, e.g.:
> +
> +void whizco_dma_map_sg_attrs(struct device *dev, dma_addr_t dma_addr,
> + size_t size, enum dma_data_direction dir,
> + struct dma_attrs* attrs)
> +{
> + ....
> + int foo = dma_get_attr(attrs, DMA_ATTR_FOO);
> + ....
> + if (foo)
> + /* twizzle the frobnozzle */
> + ....
> +

Excellent example.

thanks again for remembering DMA-API.txt.

cheers,
grant

2008-02-01 03:09:26

by Arthur Kepner

[permalink] [raw]
Subject: Re: [PATCH 1/4] dma: document dma_{un}map_{single|sg}_attrs() interface

On Thu, Jan 31, 2008 at 03:38:48PM -0700, Grant Grundler wrote:

Thanks for looking through this.

I'll send an updated patchset that addresses your comments
as soon as I can - probably around Monday.

> ...
> > +struct dma_attrs encapsulates a set of "dma attributes". For the
> > +definition of struct dma_attrs see linux/dma-attrs.h.
>
> Because all architectures will share the set of attrs definitions,
> would it be reasonable to document the intent of each attr?
>
> Two reasons for doing this:
> 1) people reading the driver code should understand WHY the dma attr was added.
> 2) other arch maintainers need to know in order to implement the same attr
> for their shiny new toys.
>
>
> > +The interpretation of dma attributes is architecture-specific.
>
> This statement is really important....but:
> Could we add a reference to arch documentation for each attr?
>
> ie something public (doesn't have to be in linux source tree)
> which explains the subtlies of how that DMA attr actually works.
>
> Having worked on HP chipsets for 10+ years, I know releasing
> original HW docs is often not possible. I'm not asking for the
> impossible. Please don't flame me for that. But if the company
> is willing to publish the existance of a feature, a paragraph or
> two would be good marketing too. Maybe just include comments in
> the arch/ code that implements the feature and reference those
> comments in DMA-API.txt.
> ....

OK, I can certainly add an explanation of the attribute that
I'm worried about.

I think there's a reference to this behavior in our driver
porting guide - I can add a pointer to that, too.

This might also address your point above about documenting
each attribute - let me know once the next patch is out.

--
Arthur