LKML Archive on
help / color / mirror / Atom feed
From: Haavard Skinnemoen <>
To: "Dan Williams" <>
	"Shannon Nelson" <>,
	"David Brownell" <>,, "Francis Moreau" <>,
	"Paul Mundt" <>,
	"Vladimir A. Barinov" <>,
	"Pierre Ossman" <>
Subject: Re: [RFC v3 4/7] dmaengine: Add slave DMA interface
Date: Mon, 18 Feb 2008 14:22:39 +0100	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Sat, 16 Feb 2008 13:06:54 -0700
"Dan Williams" <> wrote:

> I like the direction of the patch, i.e. splitting out separate
> functionality into separate structs.  However, I do not want to break
> the model of clients sourcing the operations and drivers sinking them
> which dma_slave_descriptor appears to do.  How about adding a
> scatterlist pointer and an 'unmap_type' to the common descriptor?
> Where unmap_type selects between,  page, single, sg, or no-unmap.
> Drivers already know the length and direction.

But there are currently no operations available for submitting
scatterlists as a single descriptor -- the client iterates over the
scatterlist and submits one descriptor for each entry. So there's no
way you can associated a scatterlist with a single descriptor. You can
perhaps attach it to the last one, but that may get you into trouble if
the transfer is terminated early for some reason.

I don't think the pure source/sink model is very realistic -- clients
can't just submit DMA transfers and then forget about them. They must
at the very least check the status and take appropriate action if there
was an error. They must also notify the driver that the descriptor can
be reused (although I guess if a client doesn't care about the result
it can do this immediately after submission, but I really think clients
_should_ care about the result.)

And since they need to do some sort of cleanup anyway, they might as
well unmap the buffers (or call some dma_memcpy_finish() type of
function that does it for them.)

The clients certainly know the length and direction too, but they don't
necessarily know the physical address since the mapping is done by a
"middle layer". I guess that's the main problem with the model I'm

How about we add a kind of "address cookie" struct like this (feel free
to suggest better names):

struct dma_buf_addr {
	void *cpu_addr;
	dma_addr_t dma_addr;

The client initializes the cpu_addr part and passes the struct to
dma_async_memcpy_buf_to_buf() or whatever, the middle layer sets the
dma_addr after mapping the buffer (or page), and the client passes the
same struct to dma_finish_memcpy_buf_to_buf(). Which will then be able
to unmap both buffers appropriately.

This will also eliminate the hack in crypto/async_tx/async_xor.c and
make HIGHMEM64G work again.

Scatterlists currently don't have any middle-layer support, so we can
ignore them for now and let the client take the full responsibility of
mapping and unmapping the buffers. If we were to add some middle-layer
functions for dealing with scatterlists, I don't think they would need
any special treatment -- the dma address is kept in the struct
scatterlist array passed by the client, so it would work pretty much
the same way without any special treatment.

Alternatively, we could convert the whole API to use scatterlists. But
that's probably overdoing it.

Btw, this discussion is a bit off-topic for the patch in question, but
it's an issue that needs to be resolved.


  reply	other threads:[~2008-02-18 13:22 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-12 16:43 [RFC v3 1/7] dmaengine: Couple DMA channels to their physical DMA device Haavard Skinnemoen
2008-02-12 16:43 ` [RFC v3 2/7] dmaengine: Add dma_client parameter to device_alloc_chan_resources Haavard Skinnemoen
2008-02-12 16:43   ` [RFC v3 3/7] dmaengine: Add dma_chan_is_in_use() function Haavard Skinnemoen
2008-02-12 16:43     ` [RFC v3 4/7] dmaengine: Add slave DMA interface Haavard Skinnemoen
2008-02-12 16:43       ` [RFC v3 5/7] dmaengine: Make DMA Engine menu visible for AVR32 users Haavard Skinnemoen
2008-02-12 16:43         ` [RFC v3 6/7] dmaengine: Driver for the Synopsys DesignWare DMA controller Haavard Skinnemoen
2008-02-12 16:43           ` [RFC v3 7/7] Atmel MCI: Driver for Atmel on-chip MMC controllers Haavard Skinnemoen
2008-02-12 20:43         ` [RFC v3 5/7] dmaengine: Make DMA Engine menu visible for AVR32 users Olof Johansson
2008-02-12 22:13           ` Haavard Skinnemoen
2008-02-12 22:27             ` Dan Williams
2008-02-13  8:44               ` Haavard Skinnemoen
2008-02-13  7:21       ` [RFC v3 4/7] dmaengine: Add slave DMA interface Dan Williams
2008-02-13  8:03         ` Haavard Skinnemoen
2008-02-13 19:07       ` Dan Williams
2008-02-13 19:24         ` Haavard Skinnemoen
2008-02-15  9:53           ` Haavard Skinnemoen
2008-02-15 17:12             ` Nelson, Shannon
2008-02-18 13:29               ` Haavard Skinnemoen
2008-02-19 18:46                 ` Nelson, Shannon
2008-02-16 20:06             ` Dan Williams
2008-02-18 13:22               ` Haavard Skinnemoen [this message]
2008-02-18 22:42                 ` Dan Williams

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:

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

  git send-email \ \ \ \ \ \ \ \ \ \ \ \
    --subject='Re: [RFC v3 4/7] dmaengine: Add slave DMA interface' \

* 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).