linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Walleij <linus.walleij@linaro.org>
To: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Vinod Koul <vinod.koul@intel.com>,
	Russell King - ARM Linux <linux@arm.linux.org.uk>,
	linux-kernel@vger.kernel.org,
	Jassi Brar <jassisinghbrar@gmail.com>,
	Magnus Damm <magnus.damm@gmail.com>,
	Paul Mundt <lethal@linux-sh.org>
Subject: Re: [PATCH/RFC] dmaengine: add a slave parameter to __dma_request_channel()
Date: Thu, 8 Mar 2012 12:46:32 +0100	[thread overview]
Message-ID: <CACRpkdY4y6Dj+G0zNoeY5DGWW1CRXez4pVCju2SXc=9oNyzdGw@mail.gmail.com> (raw)
In-Reply-To: <Pine.LNX.4.64.1203080902170.29847@axis700.grange>

On Thu, Mar 8, 2012 at 11:16 AM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:

>(...) our case can be handled _very_ easily:
>
> 1. a client requests a channel of a specific type
> 2. one of channels of that type, residing on one of compatible
>   controllers, is allocated, configured and handed in to the client
>
> That's it. No filtering, no post-allocation configuration - at least so
> far. And penalising such a simple case by forcing it to use virtual
> channels and filter through some unnatural mappings doesn't seem very
> productive to me.

But you do realize that this increases the complexity of the
dmaengine and means more maintenance burden for the
subsystem maintainer that will after this have to think in two
different sematic ways about channel retrieveal - yeah this one
passes that as parameter and this one has a config struct
provided, then this one use a filter function still - etc.

That is, of course, unless you convert all the existing DMA
engines to do it the same way, then it's redesigning proper.

In that case I am much more positive to the change, even
if it doesn't take us all the way to the new channel mappings
we want to have.

You haven't stated whether you will go in and rewrite the other
drivers to use this scheme or whether you will just add this one
kludge to handle this one DMA controller, then just update
all others to ignore the parameter. (You'd obviously have to
do that to get this to even compile...)

So the *proper* way to refactor to using this scheme would
be to introduce this new scheme *and* remove the filter
function from all the other drivers and DMA engine at large,
so it's not needed anymore. Which means a bit of refactoring.

Currently drivers have to pass a filter function from
platform data to filter out relevant channels, and with
the new style (this patch plus removing all filter functions)
it will be passing something else instead. That's all
fine, and actually an improvement, because passing around
a filter function is not as good as passing some struct
with data.

So does RFC patch mean you will fix this up for everyone
or does it mean something else?

If you're not also planning to get rid of the filter functions
for all other drivers I don't see this going anywhere right
now. It is not beneficial for dmaengine, the only benefit
is one more kludgy driver to maintain.

Yours,
Linus Walleij

  parent reply	other threads:[~2012-03-08 11:46 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-01 15:26 [PATCH/RFC] dmaengine: add a slave parameter to __dma_request_channel() Guennadi Liakhovetski
2012-03-02 13:21 ` Guennadi Liakhovetski
2012-03-06  8:30   ` Vinod Koul
2012-03-06  8:53     ` Guennadi Liakhovetski
2012-03-06 12:08       ` Vinod Koul
2012-03-06 13:03         ` Guennadi Liakhovetski
2012-03-07  6:28           ` Vinod Koul
2012-03-07  9:18             ` Guennadi Liakhovetski
2012-03-07  9:30               ` Russell King - ARM Linux
2012-03-07  9:55                 ` Linus Walleij
2012-03-07 10:02                 ` Guennadi Liakhovetski
2012-03-07 10:31                   ` Russell King - ARM Linux
2012-03-07 12:30                     ` Guennadi Liakhovetski
2012-03-07 12:45                       ` Guennadi Liakhovetski
2012-03-07 12:46                       ` Russell King - ARM Linux
2012-03-07 13:49                         ` Guennadi Liakhovetski
2012-03-07 14:26                           ` Russell King - ARM Linux
2012-03-07 15:44                             ` Guennadi Liakhovetski
2012-03-07 16:27                               ` Russell King - ARM Linux
2012-03-07 18:21                                 ` Guennadi Liakhovetski
2012-03-08  6:30                                   ` Vinod Koul
2012-03-08 10:16                                     ` Guennadi Liakhovetski
2012-03-08 10:55                                       ` Vinod Koul
2012-03-08 11:22                                         ` Guennadi Liakhovetski
2012-03-08 11:34                                           ` Vinod Koul
2012-03-08 12:58                                             ` Vinod Koul
2012-03-08 13:18                                               ` Guennadi Liakhovetski
2012-03-09  9:21                                                 ` Vinod Koul
2012-03-09  9:24                                                   ` Guennadi Liakhovetski
2012-03-09  9:39                                                     ` Vinod Koul
2012-03-09 12:20                                                       ` Guennadi Liakhovetski
2012-03-09 14:07                                                         ` Russell King - ARM Linux
2012-03-09 14:15                                                           ` Guennadi Liakhovetski
2012-03-12  2:47                                                         ` Vinod Koul
2012-03-12 19:47                                                           ` Linus Walleij
2012-03-16  9:36                                                           ` Guennadi Liakhovetski
2012-03-16 10:16                                                             ` Linus Walleij
2012-03-16 10:31                                                               ` Russell King - ARM Linux
2012-03-16 11:09                                                               ` Guennadi Liakhovetski
2012-03-16 14:11                                                                 ` Linus Walleij
2012-03-16 14:28                                                                   ` Guennadi Liakhovetski
2012-03-30  5:44                                                                     ` Linus Walleij
2012-03-30  6:40                                                                       ` Guennadi Liakhovetski
2012-03-30 10:38                                                                       ` Russell King - ARM Linux
2012-04-03 20:36                                                                         ` Linus Walleij
2012-04-03 20:44                                                                         ` Linus Walleij
2012-04-12 21:33                                                                           ` Guennadi Liakhovetski
2012-04-12 23:48                                                                             ` Russell King - ARM Linux
2012-03-30 10:29                                                                     ` Russell King - ARM Linux
2012-03-30 10:40                                                                       ` Guennadi Liakhovetski
2012-03-30 10:43                                                                         ` Russell King - ARM Linux
2012-03-19 11:58                                                                   ` Vinod Koul
2012-03-30 10:25                                                                 ` Russell King - ARM Linux
2012-03-19 11:39                                                               ` Vinod Koul
2012-03-19 11:37                                                             ` Vinod Koul
2012-03-19 11:47                                                               ` Guennadi Liakhovetski
2012-03-19 13:34                                                                 ` Vinod Koul
2012-03-19 13:38                                                                   ` Guennadi Liakhovetski
2012-03-19 14:00                                                                     ` Vinod Koul
2012-03-19 14:09                                                                       ` Guennadi Liakhovetski
2012-03-19 14:22                                                                         ` Vinod Koul
2012-03-19 14:45                                                                           ` Guennadi Liakhovetski
2012-03-19 16:20                                                                             ` Vinod Koul
2012-03-19 16:32                                                                               ` Guennadi Liakhovetski
2012-03-20  7:11                                                                                 ` Vinod Koul
2012-03-08 11:46                                       ` Linus Walleij [this message]
2012-03-08 12:36                                         ` Guennadi Liakhovetski
2012-03-07 16:31                         ` Linus Walleij
2012-03-07 16:20                     ` Linus Walleij
2012-03-07  9:46               ` Linus Walleij

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CACRpkdY4y6Dj+G0zNoeY5DGWW1CRXez4pVCju2SXc=9oNyzdGw@mail.gmail.com' \
    --to=linus.walleij@linaro.org \
    --cc=g.liakhovetski@gmx.de \
    --cc=jassisinghbrar@gmail.com \
    --cc=lethal@linux-sh.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=magnus.damm@gmail.com \
    --cc=vinod.koul@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).