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@xxxxxx> > > --- > > > > 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/ -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
