Re: [PATCH] lib/scatterlist: introduce sg_nents_for_dma() helper

[Ardelean, Alexandru]

On Wed, 2019-02-27 at 16:15 +0200, Andy Shevchenko wrote:
> [External]
> 
> 
> On Wed, Feb 27, 2019 at 2:26 PM Ardelean, Alexandru
> <Alex.Ardelean@xxxxxxxxxx> wrote:
> > 
> > On Wed, 2019-02-27 at 11:46 +0200, Andy Shevchenko wrote:
> > > [External]
> > > 
> > > 
> > > +Cc Vinod
> > > 
> > > On Wed, Feb 27, 2019 at 11:45 AM Andy Shevchenko
> > > <andy.shevchenko@xxxxxxxxx> wrote:
> > > > 
> > > > On Wed, Feb 27, 2019 at 10:51 AM Alexandru Ardelean
> > > > <alexandru.ardelean@xxxxxxxxxx> wrote:
> > > > > 
> > > > > From: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
> > > > > 
> > > > > Sometimes the user needs to split each entry on the mapped
> > > > > scatter
> > > > > list
> > > > > due to DMA length constrains. This helper returns a number of
> > > > > entities
> > > > > assuming that each of them is not bigger than supplied maximum
> > > > > length.
> > > > > 
> > > > > Signed-off-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
> > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx>
> > > > 
> > > > Hmm... Usually we don't accept generic API without users.
> > > > Do you have any use case in mind?
> > 
> > Yep: this one:
> > https://patchwork.kernel.org/patch/10814527/
> > 
> > But, I can rework this patch to work without the helper.
> 
> Ah, okay, worth to mention this in comments (in addition to what you
> put about the patch below).
> 
> > > > > Patch was sent in 2016 initially to the DMA engine sub-system.
> > > > > Link:
> > > > >   https://patchwork.kernel.org/patch/9389821/
> > > > > This was part of a larger series:
> > > > > 
> > > > > 
https://patchwork.kernel.org/project/linux-dmaengine/list/?q=sg_nents_for_dma&archive=both&series=&submitter=&delegate=&state=*
> > > > > 
> > > > > I'm not sure if this is supposed to go into DMAEngine or
> > > > > lib/scatterlist.
> > > > > It doesn't look like lib/scatterlist is managed by DMAEngine, so
> > > > > (by
> > > > > using
> > > > > the `get_maintainers.pl` script) I'm sending this patch to this
> > > > > group
> > > > > of
> > > > > parties.
> > > > 
> > > > The problem the patch tried to solve is much deeper and correct
> > > > solution should be more generic, i.e.
> > > > each channel should provide a set of parameters, such as DMA
> > > > segment
> > > > size, to the users (via DMA engine API) and users should prepare
> > > > the
> > > > SG list according to the limits of the channel.
> > > > In that case we don't need to re-split/re-allocate given SG list at
> > > > all, which would simplify DMA slave drivers and their users.
> > > > 
> > 
> > I don't think I managed to follow [or find] the full length of that
> > discussion. Or, the conclusion wasn't that obvious to me, from what I
> > found.
> > I assumed this may have been dropped/forgotten.
> > 
> > In any case, I am fine with just reworking.
> 
> It's fine to apply it, my point that it help to solve a symptom in a
> short-term (however this short-term can be easily a long one due to no
> guarantee that all drivers using SG for DMA will be converted).
> So, please, update the comments and resubmit as a new version.
> 

There's so much uncertainty about this patch, that it makes it easier [and
probably makes more sense] to just rework the AXI DMAC patch to not use
this helper.

This is also in the hope that one day we will get to the more generic API
that you mentioned.

> --
> With Best Regards,
> Andy Shevchenko