linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
To: Linus Walleij <linus.walleij@linaro.org>
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: Fri, 16 Mar 2012 12:09:55 +0100 (CET)	[thread overview]
Message-ID: <Pine.LNX.4.64.1203161128120.13465@axis700.grange> (raw)
In-Reply-To: <CACRpkdYiuERV08=UhBRnj2AjC_RxF+EY+zHAtEp6KYf2W10C_Q@mail.gmail.com>

On Fri, 16 Mar 2012, Linus Walleij wrote:

> On Fri, Mar 16, 2012 at 10:36 AM, Guennadi Liakhovetski
> <g.liakhovetski@gmx.de> wrote:
> 
> > Ok, let me try to summarise, what this would mean for sh-mobile:
> >
> > 1. this proposal introduces a new special case: with or without a mapping,
> > that will have to be handled in affected client and DMA controller
> > drivers. E.g., on sh-mobile some devices might on some systems use
> > channels from "general purpose" DMA controllers (no mapping), on other
> > systems it will be a dedicated controller (fixed mapping).
> >
> > 2. this will break, if we get more than 1 "general purpose" type with
> > different supported client sets. So, we develop a new API with a
> > pre-programmed limitation.
> 
> I fail to see why this would not be solved by a one-to-many mapping?
> 
> Flag for each device which channels it may use in a mapping
> table in platform data or device tree, I don't see the problem.
> 
> You don't even have to specify that on a per-channel basis if
> you can come up with something more clever in the mapping
> table, such as "this device can use any channel on this DMAC,
> and channels 1-7 on that DMAC" - no problem?

Sure, everything is possible. So, would something like this make you 
happy:

struct dma_channel_range {
	const char *dma_device;
	int channel_start;
	int channel_end;
};

struct dma_map {
	const char *name;
	const char *client;
	const struct dma_channel_range *chan_range;
	int chan_range_num;
};

You really want to do this?...

And the least important question: who and when will implement the core 
support for this?

> > 3. this will mean a substantial driver and platform code modification.
> > Nothing super-complex, but still some.
> 
> Big deal. Refactoring is fun... ;-)
> 
> > 4. we'll need a 3-stage channel allocation / configuration: request,
> > filter, config.
> 
> In my world: channel request with *NO* filter function.

How??? Again:

1. the client issues a dma_request_channel() with _just_ a capability mask 
and a filter and its argument as parameters - _nothing_ about channel 
restrictions.

2. you propose to eliminate a filter - the core has no way to know, which 
channel to pick up...

3. the wrapper, proposed by Russell, now calls dmaengine_slave_config(), 
which fails, because that's a wrong channel (hope I get this right this 
time - configuration has nothing to do with selection:-))

4. that's it, if you start again - the dmaengine core will enumerate the 
same channels again and propose the same unsuitable channel to you - 
there's no way to continue to the next channel / device.

What am I missing? How is the mapping going to be used, if you eliminate 
the filter function?

> Filter functions are part of the problem. So we refactor these
> away as part of this change. That's the whole point...
> 
> The core gathers information from the platform and the
> DMAC driver(s) to build up the constraints necessary to
> hand out workling channels to each device that request
> one.
> 
> And Russell IIRC already suggested a request-and-config
> channel inline for the simple cases, and if you still need to
> explicitly runtime-reconfigure then that's for a good
> reason.
> 
> > Whereas with my configuration-parameter proposal it's just
> > one stage: allocate-and-configure.
> 
> For one specific hardware, yes. For DMAengine at large
> and the majority of the drivers, no.

Sorry, why? I don't think I saw an answer to it apart from - maintenance 
burden... You can use that parameter to actually pass information to be 
used by the core to scan your mapping tables, I really don't see how you 
want to use those tables with the existing dmaengine channel-allocation 
API.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

  parent reply	other threads:[~2012-03-16 11:10 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 [this message]
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
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=Pine.LNX.4.64.1203161128120.13465@axis700.grange \
    --to=g.liakhovetski@gmx.de \
    --cc=jassisinghbrar@gmail.com \
    --cc=lethal@linux-sh.org \
    --cc=linus.walleij@linaro.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).