LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Grant Grundler <grundler@parisc-linux.org>
To: akepner@sgi.com
Cc: Tony Luck <tony.luck@intel.com>,
	Grant Grundler <grundler@parisc-linux.org>,
	Jesse Barnes <jbarnes@virtuousgeek.org>,
	Jes Sorensen <jes@sgi.com>,
	Randy Dunlap <randy.dunlap@oracle.com>,
	Roland Dreier <rdreier@cisco.com>,
	James Bottomley <James.Bottomley@HansenPartnership.com>,
	David Miller <davem@davemloft.net>,
	Muli Ben-Yehuda <muli@il.ibm.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/4] dma: document dma_{un}map_{single|sg}_attrs() interface
Date: Thu, 31 Jan 2008 15:38:48 -0700	[thread overview]
Message-ID: <20080131223847.GA11872@colo.lackof.org> (raw)
In-Reply-To: <20080130055040.GS30022@sgi.com>

On Tue, Jan 29, 2008 at 09:50:40PM -0800, akepner@sgi.com 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

  reply	other threads:[~2008-01-31 22:39 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-30  5:50 akepner
2008-01-31 22:38 ` Grant Grundler [this message]
2008-02-01  3:07   ` akepner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080131223847.GA11872@colo.lackof.org \
    --to=grundler@parisc-linux.org \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=akepner@sgi.com \
    --cc=davem@davemloft.net \
    --cc=jbarnes@virtuousgeek.org \
    --cc=jes@sgi.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=muli@il.ibm.com \
    --cc=randy.dunlap@oracle.com \
    --cc=rdreier@cisco.com \
    --cc=tony.luck@intel.com \
    --subject='Re: [PATCH 1/4] dma: document dma_{un}map_{single|sg}_attrs() interface' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).