Hi Vinod
Thanks for your email.
On Mon, 30 Jan 2012, Vinod Koul wrote:
> On Thu, 2012-01-26 at 15:56 +0100, Guennadi Liakhovetski wrote:
> > This patch adds a library of functions, helping to implement dmaengine
> > drivers for hardware, unable to handle scatter-gather lists natively.
> > The first version of this driver only supports memcpy and slave DMA
> > operation.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski at gmx.de>
> > ---
> >
> > v2:
> >
> > 1. switch from using a tasklet to threaded IRQ, which allowed to
> > 2. remove lock / unlock inline functions
> > 3. remove __devinit, __devexit annotations
> Sorry to join the discussion late, was on vacation, travel, long
> weekend...
>> I don't still comprehend the need for a library on top of dmaengine
> which gain is just a library between clients and dmacs. Surely we don't
> want to write another abstraction on top of one provided?
>> If the question is to handle scatter-gather even if the hardware doesn't
> have the capability, then why don't add that in dmaengine itself rather
> than one more layer?
Well, yes, adding new abstraction layers is always a decision, that has to
be well justified. In this case it does at least make the life easier for
two sh-mobile drivers: shdma and the new SUDMAC driver.
However, I did name the library in a generic way without reference to sh,
assuming, that it might with time become useful for other architectures
too. The reasons why I prefered to keep it as an optional addition to
dmaengine core, instead of tightly integrating it with it are, that (1) I
did not want to add useless code to drivers, that do not need it, (2) I am
not sure if and when this library will become useful for other drivers:
apart from sh I am only familiar with one more dmaengine driver:
ipu/ipu_idmac.c, and that one supports scatter-gather lists in a limited
way and has some further peculiarities, that would likely make it a bad
match for the simple DMA library, (3) keeping it separate makes its
further development easier.
OTOH, I'm certainly fine with a tighter library integration with the
dmaengine core. I think, it still would be better to keep it in a separate
file and only build it if needed, right? This woult also simplify code
debugging and further development. I can remove the "simple" notation,
which does make it look like an additional abstraction layer, and replace
it with, say, sgsoft (scatter-gather software implementation)? A more
interesting question is what to do with struct dma_simple_dev, struct
dma_simple_chan, struct dma_simple_desc, that embed struct dma_device,
struct dma_chan and struct dma_async_tx_descriptor respectively. I don't
think we want to merge all the additions from those wrapping structs back
into their dmaengine counterparts?
How would you like to do this? Don't you think, it would be good to allow
both: either implement a dmaengine driver directly, exactly as all drivers
are doing now, or use the additional helper library for suitable (simple)
hardware types? I see it similar to I2C, where you either implement an I2C
driver directly, or you use the bitbanging abstraction for simpler
hardware.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/