On 26-07-21, 23:14, Adrian Larumbe wrote:
> Hi Vinod, I'm the same person who authored this patch. I left my previous
> employer so no longer have access to their company email address. However I've
> signed this email with the same GPG key to confirm my identity.
>
> On 14.07.2021 10:28, Vinod Koul wrote:
> > On 07-07-21, 00:43, Adrian Larumbe wrote:
> > > This is the old DMA_SG interface that was removed in commit
> > > c678fa66341c ("dmaengine: remove DMA_SG as it is dead code in kernel"). It
> > > has been renamed to DMA_MEMCPY_SG to better match the MEMSET and MEMSET_SG
> > > naming convention.
> > >
> > > It should only be used for mem2mem copies, either main system memory or
> > > CPU-addressable device memory (like video memory on a PCI graphics card).
> > >
> > > Bringing back this interface was prompted by the need to use the Xilinx
> > > CDMA device for mem2mem SG transfers. The current CDMA binding for
> > > device_prep_dma_memcpy_sg was partially borrowed from xlnx kernel tree, and
> > > expanded with extended address space support when linking descriptor
> > > segments and checking for incorrect zero transfer size.
> > >
> > > Signed-off-by: Adrian Larumbe <adrian.martinezlarumbe@xxxxxxxxxx>
> > > ---
> > > .../driver-api/dmaengine/provider.rst | 11 ++
> > > drivers/dma/dmaengine.c | 7 +
> > > drivers/dma/xilinx/xilinx_dma.c | 122 ++++++++++++++++++
> >
> > Can you make this split... documentation patch, core change and then
> > driver
>
> I understand you'd like these in three different patches, is that right? Or
Correct
> maybe one patch for the core change and its associated documentation, and
doc and core should be different
> another one for the consumer.
>
> > > include/linux/dmaengine.h | 20 +++
> > > 4 files changed, 160 insertions(+)
> > >
> > > diff --git a/Documentation/driver-api/dmaengine/provider.rst b/Documentation/driver-api/dmaengine/provider.rst
> > > index ddb0a81a796c..9f0efe9e9952 100644
> > > --- a/Documentation/driver-api/dmaengine/provider.rst
> > > +++ b/Documentation/driver-api/dmaengine/provider.rst
> > > @@ -162,6 +162,17 @@ Currently, the types available are:
> > >
> > > - The device is able to do memory to memory copies
> > >
> > > +- - DMA_MEMCPY_SG
> > > +
> > > + - The device supports memory to memory scatter-gather transfers.
> > > +
> > > + - Even though a plain memcpy can look like a particular case of a
> > > + scatter-gather transfer, with a single chunk to transfer, it's a
> > > + distinct transaction type in the mem2mem transfer case. This is
> > > + because some very simple devices might be able to do contiguous
> > > + single-chunk memory copies, but have no support for more
> > > + complex SG transfers.
> >
> > How does one deal with cases where
> > - src_sg_len and dstn_sg_len are different?
>
> Then only as many bytes as the smallest of the scattered buffers will be copied.
Is that not a restriction and that needs to be documented with examples!
>
> > > - src_sg and dstn_sg are different lists (maybe different number of
> > > entries with different lengths..)
> > >
> > > I think we need to document these cases or limitations..
>
> I don't think we should place any restrictions on the number of scatterlist
> entries or their length, and the consumer driver should ensure that these get
> translated into a device-specific descriptor chain. However the previous
> semantic should always be observed, which effectively turns the operation into
> sort of a strncpy.
That sounds right, but someone needs to know how to handle such cases
with this API, that needs to be explained in detail on expected
behaviour when src_sg_len & dstn_sg_len and same, less or more!
--
~Vinod
