[PATCH 0/4] Expand Xilinx CDMA functions

[Lars-Peter Clausen]

On 4/23/21 3:19 AM, Adrian Larumbe wrote:
> Recently at Imgtec we had to provide GLES API buffers with DMA transfer
> capabilities to device memory. We had access to a Xilinx CDMA IP module,
> but even though the hardware supports scatter-gather operations,
> the driver did not. This patch series' goal is to extend the driver
> to support SG transfers on CDMA devices.

Hi,

Thanks for the patch series. From an implementation point of view this 
looks good. But I'm a bit concerned about dmaengine API compliance.

The device_config() and device_prep_slave_sg() APIs are meant for 
memory-to-device or device-to-memory transfers. This means the device 
address is fixed and usually maps to a FIFO in the device.

What you are implementing is memory-to-memory, which means addresses are 
incremented on both sides of transfer. And this works if you know what 
you are doing, e.g. you know that you are using a specific dmaengine 
driver that implements the dmaengine API in a way that does not comply 
with the specification. But it breaks for generic dmaengine API clients 
that expect compliant behavior. And it is the reason why we have an API 
in the first place, to get consistent behavior. If you have to know 
about the driver specific semantics you might as well just bypass the 
framework and use driver specific functions.

And the CDMA seems to support the dmaengine API intended behavior 
through what it calls the keyhole feature where the address is not 
incremented. And if somebody wanted to use that feature they wouldn't be 
able to add support for it because it will break your usage of the 
prep_slave_sg() API.

It seems to me what we are missing from the DMAengine API is the 
equivalent of device_prep_dma_memcpy() that is able to take SG lists. 
There is already a memset_sg, it should be possible to add something 
similar for memcpy.

  - Lars