Re: [PATCH 0/3] dmaengine: Stear users towards dma_request_slave_chan()

[Andy Shevchenko]

On Mon, Feb 3, 2020 at 2:09 PM Peter Ujfalusi <peter.ujfalusi@xxxxxx> wrote:
> On 03/02/2020 13.16, Andy Shevchenko wrote:
> > On Mon, Feb 3, 2020 at 12:59 PM Peter Ujfalusi <peter.ujfalusi@xxxxxx> wrote:
> >> On 03/02/2020 12.37, Andy Shevchenko wrote:
> >>> On Mon, Feb 3, 2020 at 12:32 PM Peter Ujfalusi <peter.ujfalusi@xxxxxx> wrote:
> >
> >>>> Advise users of dma_request_slave_channel() and
> >>>> dma_request_slave_channel_compat() to move to dma_request_slave_chan()
> >>>
> >>> How? There are legacy ARM boards you have to care / remove before.
> >>> DMAengine subsystem makes a p*s off decisions
> >>
> >> The dma_slave_map support is added few years back for the legacy ARM
> >> boards, because we do care.
> >> daVinci, OMAP1, pxa, s3cx4xx and even m68k/coldfire moved over.
> >
> > Then why simple not to convert (start converting) those few drivers to
> > new API and simple remove the old one?
>
> _if_ the dma_request_slave_channel_compat() is really falling back after
> dma_request_chan() - this is what it calls first, then it is not a
> simple convert to a new API.
> 1. The arch needs to define the dma_slave_map for the SoC.
> 2. The dma driver needs few lines to add it to DMAengine core (+pdata
> change).
> After this the _compat() can be replaced with dma_request_chan()
>

> In most cases I do not have access to documentation and boards to test.

So, why everyone with the boards outside of your scope has to suffer?

...

> >> Imho it is confusing to have 4+ APIs to do the same thing, but in a
> >> slightly different way.
> >
> > It was always an excuse by authors "that too many drivers to convert..."
>
> Sure, but before you accuse anyone with neglect, can you check the git
> log for 'dma_request_slave_channel' in the commit messages?

It's good people, and you, are taking care of it, but make user suffer
because somebody wants to call for *developers* is a bad idea. And it
happened not first time in the DMA engine subsystem.

...

> > No, it's a reason when you first take care of existing users and
> > decide to obsolete an API followed by removal few releases later.
>
> I'm fine to drop the pr_info()

Yes, please do.

> and the __deprecated mark for
> dma_request_slave_channel.

This OTOH is for *developers* and its scope won't scary people. So,
it's more or less fine.

> > But
> > I see no reason to keep such APIs at all, so, instead of this
> > *wonderful* messages perhaps somebody should do better work?
>
> To touch the _compat() variant one needs to have access to the
> documentation of the SoC on which the code falls back. It is not a
> matter of sloppy/poor/ignorant/etc work attitude.
> I have kept clear on touching those few drivers using it [1] as I don't
> have documentation.
>
> >> New drivers should not use the old API i new code and developers tend to
> >> pick the API they use after a quick 'git grep dma_request_' and see what
> >> the majority is using.
> >
> > Isn't it a point to do better review rather than scary end users?
>
> Sure, but we rarely CCd on new client drivers for DMAengine API usage.

Linux Next allows people to `git grep -n dma_slave_DO_NOT_CALL` and
tell developers. Isn't it the job good maintainer can do?

> [1] fwiw, there are drivers using dma_request_channel() and by the look
> their use is either _compat() or the dma_request_chan_by_mask() style
> and some even have a twist here and there...

Bottom line, if you need to tell *developers* about something, please
don't bother *end users*.

-- 
With Best Regards,
Andy Shevchenko