[PATCH] DMA: Correct invalid assumptions in the Kconfig text

Previous thread: Re: [PATCH] fs/partitions/check.c: add error handling for kobject_add by Borislav Petkov on Wednesday, October 24, 2007 - 2:32 am. (1 message)

Next thread: [RFC/git patch] move samples/ to Documentation/markers/ by Jeff Garzik on Wednesday, October 24, 2007 - 2:52 am. (9 messages)
From: Haavard Skinnemoen
Date: Wednesday, October 24, 2007 - 2:38 am

This patch corrects what I hope are invalid assumptions about the DMA
engine layer: Not only Intel(R) hardware can do DMA, and DMA can be
used for other things than memcpy and RAID offloading.

At the same time, make the DMA Engine menu visible again on AVR32. I'm
currently working on a driver for a DMA controller that can do
mem-to-mem transfers (which is supported by the framework) as well as
device-to-mem and mem-to-device transfers (not currently supported.)

Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com>
---
Don't get me wrong; I think Intel deserves lots of respect for
creating this framework. But this is also why I got a bit disappointed
when I discovered that it seems to be less generic than I initially
hoped.

DMA controllers, which may support plain memcpy acceleration in
addition to more traditional "slave DMA", are very common in SoC
devices, and I think Linux needs a common framework for it. The
existing DMA Engine framework seems to come pretty close already, but
I think it needs more input from the embedded crowd before it can be
completely usable on a large number of embedded systems.

I'm not going to suggest any changes to the actual framework for
2.6.24, but I think the _intention_ of the framework needs to be
clarified.

Haavard

 drivers/dma/Kconfig |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 9c91b0f..62a9fe5 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -3,11 +3,13 @@
 #
 
 menuconfig DMADEVICES
-	bool "DMA Offload Engine support"
-	depends on (PCI && X86) || ARCH_IOP32X || ARCH_IOP33X || ARCH_IOP13XX
+	bool "DMA Engine support"
+	depends on (PCI && X86) || ARCH_IOP32X || ARCH_IOP33X || ARCH_IOP13XX || AVR32
 	help
-	  Intel(R) offload engines enable offloading memory copies in the
-	  network stack and RAID operations in the MD driver.
+	  DMA engines can do asynchronous data transfers without
+	  involving the host CPU. This ...
From: Nelson, Shannon
Date: Wednesday, October 24, 2007 - 8:11 am

Acked-by: Shannon Nelson <shannon.nelson@intel.com>
-

From: Dan Williams
Date: Wednesday, October 24, 2007 - 8:55 am

Hi Haavard,



Part of the problem of supporting slave/device DMA along side generic
memcpy/xor/memset acceleration is that it adds a number of caveats and
restrictions to the interface.  One idea is to create another client
layer, similar to async_tx, that can handle the architecture specific
address, bus, and device pairing restrictions.  In other words make
device-dma a superset of the generic offload capabilities and move it

Should this patch wait until the framework has been extended?


Regards,
Dan
-

From: Haavard Skinnemoen
Date: Wednesday, October 24, 2007 - 11:16 am

On Wed, 24 Oct 2007 08:55:58 -0700

Sure, that's the plan, and this patch is a start, isn't it? I just
wanted to make sure that the generic-sounding DMA Engine API would
allow more "traditional" DMA controller functionality in addition to

Yes, I've been thinking along those lines, and
Documentation/crypto/async-tx-api.txt seems to suggest something like
that for platform-specific extensions. More specifically, I'm thinking
about adding a few new structs which wrap around some of the existing
ones, like struct dma_async_tx_descriptor, adding new ops and data
members. So it will be sort of a generic, optional extension of the API.

If that works out well, we may also consider moving some of the
async_tx-specific fields into an extended struct of its own. But that
will be more of an optimization or cleanup. I don't want to hurt any
existing users.

Then we're going to need some new hooks for creating those extended
descriptors. This can be done either by adding more hooks in struct
dma_device or by creating another "subclass".

Yes, I'm mostly handwaving at this point, but I intend to follow up

IMO, no. The first step I'm planning to do is to get the driver working
within the existing framework, i.e. as a pure memcpy offload
engine, and verify that it can indeed improve networking performance.
This would imply that the framework itself isn't Intel-specific even
though all the existing drivers are for Intel hardware. And I don't
think the proposed text makes any new promises that weren't there
before.

And I think it's important to clarify that what is meant to live under
drivers/dma isn't just RAID and networking acceleration engines -- it
is realy meant to be a generic DMA engine framework. If this isn't
true, I might as well stop writing the driver based on this framework
and instead focus on creating a new one.

But I'm pretty optimistic about being able to extend the API in a way
that will not hurt existing functionality and still provide what we
need to support ...
From: Haavard Skinnemoen
Date: Thursday, October 25, 2007 - 2:32 am

On Wed, 24 Oct 2007 20:16:16 +0200

Oh, and we definitely need a way to report errors. Looks like the
existing drivers want this as well -- I couldn't help but notice this
in the iop-adma driver:

static irqreturn_t iop_adma_err_handler(int irq, void *data)
{
	(...)

	BUG();
}

That's a panic waiting to happen, isn't it?

Håvard
-

From: Dan Williams
Date: Friday, October 26, 2007 - 10:02 am

Yes, and it should have a comment, because for now this is deliberate.
 This was primarily driven by the fact that MD has no way of
recovering from hardware errors during software-memcpy or
software-xor_blocks so there was no where to plug-in
accelerated-memcpy/xor error recovery.  I can foresee other clients
wanting to have this information reported but async_tx based clients
are supposed to be blissfully unaware of under the covers hardware
acceleration.

One idea is to pass an error-pointer as the parameter to callback
routines, but that would hamper the client's ability to recover the

--
Dan
-

From: Haavard Skinnemoen
Date: Saturday, October 27, 2007 - 6:58 am

On Fri, 26 Oct 2007 10:02:24 -0700

Yeah, it's a pretty serious bug if the DMA engine flags an error. But
wouldn't it be better to BUG() in the context of the caller? That way,

It's probably a good idea to dump the descriptors at some point. I
don't think the client has many options to deal with such failures
other than reset something or kill something, but if the client gets
notified, it at least has some chance of recovering.

Håvard
-

From: Dan Williams
Date: Saturday, October 27, 2007 - 11:10 am

I see your point...  We could track the caller's task_struct in
dma_async_tx_descriptor, and deliver a SIGBUS in the case of an error.
 It limits the client's recovery options, but at least the damaged is
localized to the correct process.  I need to go read up on what this
would imply for kernel threads like raid5d...
-

From: Dan Williams
Date: Friday, October 26, 2007 - 9:44 am

Yeah, I'll pick it up.  I'll leave off the AVR addition to DMADEVICES
because I assume it will come with the future patch implementing the

Thanks,
Dan
-

From: Haavard Skinnemoen
Date: Saturday, October 27, 2007 - 9:07 am

On Fri, 26 Oct 2007 09:44:44 -0700

Sure, either way is fine with me.

Thanks,
Håvard
-

Previous thread: Re: [PATCH] fs/partitions/check.c: add error handling for kobject_add by Borislav Petkov on Wednesday, October 24, 2007 - 2:32 am. (1 message)

Next thread: [RFC/git patch] move samples/ to Documentation/markers/ by Jeff Garzik on Wednesday, October 24, 2007 - 2:52 am. (9 messages)