* [RFC] dmaengine/dma_slave: add context parameter to prep_slave_sg callback
@ 2012-01-26 21:22 Alexandre Bounine
2012-01-30 9:30 ` Vinod Koul
0 siblings, 1 reply; 8+ messages in thread
From: Alexandre Bounine @ 2012-01-26 21:22 UTC (permalink / raw)
To: akpm, linux-kernel, linuxppc-dev, vinod.koul, dan.j.williams
Cc: Jassi Brar, Alexandre Bounine, Russell King
As we agreed during our discussion about adding DMA Engine support for RapidIO
subsystem, RapidIO and similar clients may benefit from adding an extra context
parameter to device_prep_slave_sg() callback.
See https://lkml.org/lkml/2011/10/24/275 for more details.
Adding the context parameter will allow to pass client/target specific
information associated with an individual data transfer request.
In the case of RapidIO support this additional information consists of target
destination ID and its buffer address (which is not mapped into the local CPU
memory space). Because a single RapidIO-capable DMA channel may queue data
transfer requests to different target devices, the per-request configuration
is required.
The proposed change eliminates need for new subsystem-specific API.
Existing DMA_SLAVE clients will ignore the new parameter.
This RFC only demonstrates the API change and does not include corresponding
changes to existing DMA_SLAVE clients. Complete set of patches will be provided
after (if) this API change is accepted.
Signed-off-by: Alexandre Bounine <alexandre.bounine@idt.com>
Cc: Jassi Brar <jaswinder.singh@linaro.org>
Cc: Russell King <rmk@arm.linux.org.uk>
Cc: Kumar Gala <galak@kernel.crashing.org>
Cc: Matt Porter <mporter@kernel.crashing.org>
Cc: Li Yang <leoli@freescale.com>
---
include/linux/dmaengine.h | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 679b349..79d71bb 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -575,7 +575,7 @@ struct dma_device {
struct dma_async_tx_descriptor *(*device_prep_slave_sg)(
struct dma_chan *chan, struct scatterlist *sgl,
unsigned int sg_len, enum dma_transfer_direction direction,
- unsigned long flags);
+ unsigned long flags, void *context);
struct dma_async_tx_descriptor *(*device_prep_dma_cyclic)(
struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
size_t period_len, enum dma_transfer_direction direction);
@@ -607,12 +607,13 @@ static inline int dmaengine_slave_config(struct dma_chan *chan,
static inline struct dma_async_tx_descriptor *dmaengine_prep_slave_single(
struct dma_chan *chan, void *buf, size_t len,
- enum dma_transfer_direction dir, unsigned long flags)
+ enum dma_transfer_direction dir, unsigned long flags, void *context)
{
struct scatterlist sg;
sg_init_one(&sg, buf, len);
- return chan->device->device_prep_slave_sg(chan, &sg, 1, dir, flags);
+ return chan->device->device_prep_slave_sg(chan, &sg, 1, dir, flags,
+ context);
}
static inline int dmaengine_terminate_all(struct dma_chan *chan)
--
1.7.8.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC] dmaengine/dma_slave: add context parameter to prep_slave_sg callback
2012-01-26 21:22 [RFC] dmaengine/dma_slave: add context parameter to prep_slave_sg callback Alexandre Bounine
@ 2012-01-30 9:30 ` Vinod Koul
2012-01-30 16:55 ` Bounine, Alexandre
2012-02-01 0:09 ` Guennadi Liakhovetski
0 siblings, 2 replies; 8+ messages in thread
From: Vinod Koul @ 2012-01-30 9:30 UTC (permalink / raw)
To: Alexandre Bounine
Cc: linux-kernel, Jassi Brar, akpm, linuxppc-dev, dan.j.williams,
Russell King
On Thu, 2012-01-26 at 16:22 -0500, Alexandre Bounine wrote:
> As we agreed during our discussion about adding DMA Engine support for RapidIO
> subsystem, RapidIO and similar clients may benefit from adding an extra context
> parameter to device_prep_slave_sg() callback.
> See https://lkml.org/lkml/2011/10/24/275 for more details.
>
> Adding the context parameter will allow to pass client/target specific
> information associated with an individual data transfer request.
>
> In the case of RapidIO support this additional information consists of target
> destination ID and its buffer address (which is not mapped into the local CPU
> memory space). Because a single RapidIO-capable DMA channel may queue data
> transfer requests to different target devices, the per-request configuration
> is required.
>
> The proposed change eliminates need for new subsystem-specific API.
> Existing DMA_SLAVE clients will ignore the new parameter.
>
> This RFC only demonstrates the API change and does not include corresponding
> changes to existing DMA_SLAVE clients. Complete set of patches will be provided
> after (if) this API change is accepted.
This looks good to me. But was thinking if we need to add this new
parameter for other slave calls (circular, interleaved, memcpy...)
>
> Signed-off-by: Alexandre Bounine <alexandre.bounine@idt.com>
> Cc: Jassi Brar <jaswinder.singh@linaro.org>
> Cc: Russell King <rmk@arm.linux.org.uk>
> Cc: Kumar Gala <galak@kernel.crashing.org>
> Cc: Matt Porter <mporter@kernel.crashing.org>
> Cc: Li Yang <leoli@freescale.com>
> ---
> include/linux/dmaengine.h | 7 ++++---
> 1 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 679b349..79d71bb 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -575,7 +575,7 @@ struct dma_device {
> struct dma_async_tx_descriptor *(*device_prep_slave_sg)(
> struct dma_chan *chan, struct scatterlist *sgl,
> unsigned int sg_len, enum dma_transfer_direction direction,
> - unsigned long flags);
> + unsigned long flags, void *context);
> struct dma_async_tx_descriptor *(*device_prep_dma_cyclic)(
> struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
> size_t period_len, enum dma_transfer_direction direction);
> @@ -607,12 +607,13 @@ static inline int dmaengine_slave_config(struct dma_chan *chan,
>
> static inline struct dma_async_tx_descriptor *dmaengine_prep_slave_single(
> struct dma_chan *chan, void *buf, size_t len,
> - enum dma_transfer_direction dir, unsigned long flags)
> + enum dma_transfer_direction dir, unsigned long flags, void *context)
> {
> struct scatterlist sg;
> sg_init_one(&sg, buf, len);
>
> - return chan->device->device_prep_slave_sg(chan, &sg, 1, dir, flags);
> + return chan->device->device_prep_slave_sg(chan, &sg, 1, dir, flags,
> + context);
> }
>
> static inline int dmaengine_terminate_all(struct dma_chan *chan)
--
~Vinod
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [RFC] dmaengine/dma_slave: add context parameter to prep_slave_sg callback
2012-01-30 9:30 ` Vinod Koul
@ 2012-01-30 16:55 ` Bounine, Alexandre
2012-01-31 3:14 ` Vinod Koul
2012-02-01 0:09 ` Guennadi Liakhovetski
1 sibling, 1 reply; 8+ messages in thread
From: Bounine, Alexandre @ 2012-01-30 16:55 UTC (permalink / raw)
To: Vinod Koul
Cc: linux-kernel, Jassi Brar, akpm, linuxppc-dev, dan.j.williams,
Russell King
T24gTW9uZGF5LCBKYW51YXJ5IDMwLCAyMDEyIGF0IDQ6MzEgQU0sIFZpbm9kIEtvdWwgd3JvdGU6
DQo+IA0KPiBPbiBUaHUsIDIwMTItMDEtMjYgYXQgMTY6MjIgLTA1MDAsIEFsZXhhbmRyZSBCb3Vu
aW5lIHdyb3RlOg0KPiA+IEFzIHdlIGFncmVlZCBkdXJpbmcgb3VyIGRpc2N1c3Npb24gYWJvdXQg
YWRkaW5nIERNQSBFbmdpbmUgc3VwcG9ydCBmb3IgUmFwaWRJTw0KPiA+IHN1YnN5c3RlbSwgUmFw
aWRJTyBhbmQgc2ltaWxhciBjbGllbnRzIG1heSBiZW5lZml0IGZyb20gYWRkaW5nIGFuIGV4dHJh
IGNvbnRleHQNCj4gPiBwYXJhbWV0ZXIgdG8gZGV2aWNlX3ByZXBfc2xhdmVfc2coKSBjYWxsYmFj
ay4NCj4gPiBTZWUgaHR0cHM6Ly9sa21sLm9yZy9sa21sLzIwMTEvMTAvMjQvMjc1IGZvciBtb3Jl
IGRldGFpbHMuDQo+ID4NCj4gPiBBZGRpbmcgdGhlIGNvbnRleHQgcGFyYW1ldGVyIHdpbGwgYWxs
b3cgdG8gcGFzcyBjbGllbnQvdGFyZ2V0IHNwZWNpZmljDQo+ID4gaW5mb3JtYXRpb24gYXNzb2Np
YXRlZCB3aXRoIGFuIGluZGl2aWR1YWwgZGF0YSB0cmFuc2ZlciByZXF1ZXN0Lg0KPiA+DQo+ID4g
SW4gdGhlIGNhc2Ugb2YgUmFwaWRJTyBzdXBwb3J0IHRoaXMgYWRkaXRpb25hbCBpbmZvcm1hdGlv
biBjb25zaXN0cyBvZiB0YXJnZXQNCj4gPiBkZXN0aW5hdGlvbiBJRCBhbmQgaXRzIGJ1ZmZlciBh
ZGRyZXNzICh3aGljaCBpcyBub3QgbWFwcGVkIGludG8gdGhlIGxvY2FsIENQVQ0KPiA+IG1lbW9y
eSBzcGFjZSkuIEJlY2F1c2UgYSBzaW5nbGUgUmFwaWRJTy1jYXBhYmxlIERNQSBjaGFubmVsIG1h
eSBxdWV1ZSBkYXRhDQo+ID4gdHJhbnNmZXIgcmVxdWVzdHMgdG8gZGlmZmVyZW50IHRhcmdldCBk
ZXZpY2VzLCB0aGUgcGVyLXJlcXVlc3QgY29uZmlndXJhdGlvbg0KPiA+IGlzIHJlcXVpcmVkLg0K
PiA+DQo+ID4gVGhlIHByb3Bvc2VkIGNoYW5nZSBlbGltaW5hdGVzIG5lZWQgZm9yIG5ldyBzdWJz
eXN0ZW0tc3BlY2lmaWMgQVBJLg0KPiA+IEV4aXN0aW5nIERNQV9TTEFWRSBjbGllbnRzIHdpbGwg
aWdub3JlIHRoZSBuZXcgcGFyYW1ldGVyLg0KPiA+DQo+ID4gVGhpcyBSRkMgb25seSBkZW1vbnN0
cmF0ZXMgdGhlIEFQSSBjaGFuZ2UgYW5kIGRvZXMgbm90IGluY2x1ZGUgY29ycmVzcG9uZGluZw0K
PiA+IGNoYW5nZXMgdG8gZXhpc3RpbmcgRE1BX1NMQVZFIGNsaWVudHMuIENvbXBsZXRlIHNldCBv
ZiBwYXRjaGVzIHdpbGwgYmUgcHJvdmlkZWQNCj4gPiBhZnRlciAoaWYpIHRoaXMgQVBJIGNoYW5n
ZSBpcyBhY2NlcHRlZC4NCj4NCj4gVGhpcyBsb29rcyBnb29kIHRvIG1lLiBCdXQgd2FzIHRoaW5r
aW5nIGlmIHdlIG5lZWQgdG8gYWRkIHRoaXMgbmV3DQo+IHBhcmFtZXRlciBmb3Igb3RoZXIgc2xh
dmUgY2FsbHMgKGNpcmN1bGFyLCBpbnRlcmxlYXZlZCwgbWVtY3B5Li4uKQ0KPiANCg0KSSBhZ3Jl
ZSB0aGF0IGN5Y2xpYyBhbmQgaW50ZXJsZWF2ZWQgY2FsbHMgbWF5IGJlbmVmaXQgZnJvbSBhZGRp
bmcgdGhhdCBwYXJhbWV0ZXIgYXMgd2VsbC4NCkJlbmVmaXRzIHRvIHRoZSBjeWNsaWMgY2FsbCBh
cmUgc3RyYWlnaHRmb3J3YXJkIC0gc2FtZSBhcyBkbWFfc2xhdmUuDQpBZGRpbmcgYSBjb250ZXh0
IHBhcmFtZXRlciB0byB0aGUgaW50ZXJsZWF2ZWQgdHJhbnNmZXJzIG1heSBiZSBtb3JlIGZ1dHVy
ZSBwcm9vZmluZyBvcHRpb24NCnRoYW4gYW4gaW1tZWRpYXRlIG5lZWQuIE1lbWNvcHkgYW5kIG90
aGVyIGNhbGxzIHRoYXQgZGVhbCB3aXRoIGxvY2FsIG1lbW9yeSB0cmFuc2ZlcnMNCnByb2JhYmx5
IHNob3VsZCBiZSBsZWZ0IHVudG91Y2hlZC4NCg0KV2hhdCBpZiB3ZSBsaW1pdCBtb2RpZmljYXRp
b25zIHRvOg0KMSkgdGhyZWUgY2FsbHMgKHNsYXZlLCBjeWNsaWMgYW5kIGludGVybGVhdmVkKSBP
Ug0KMikgdHdvIChzbGF2ZSBhbmQgY3ljbGljKSBhdCB0aGlzIG1vbWVudD8NCg0KSSBhbSBqdXN0
IG1vcmUgZm9jdXNlZCBvbiBkbWFfc2xhdmUganVzdCBiZWNhdXNlIGl0IGZpdHMgd2VsbCB0byBw
cm92aWRlIFJETUENCm92ZXIgUmFwaWRJTyBmYWJyaWMuDQoNCklmIGV2ZXJ5Ym9keSBhZ3JlZXMs
IEkgY2FuIGdvIGFoZWFkIGFuZCBtYWtlIGNoYW5nZXMgdG8gYWxsIHRocmVlIGF0IG9uY2UuDQoN
CkFsZXguDQoNCg0K
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [RFC] dmaengine/dma_slave: add context parameter to prep_slave_sg callback
2012-01-30 16:55 ` Bounine, Alexandre
@ 2012-01-31 3:14 ` Vinod Koul
0 siblings, 0 replies; 8+ messages in thread
From: Vinod Koul @ 2012-01-31 3:14 UTC (permalink / raw)
To: Bounine, Alexandre
Cc: linux-kernel, Jassi Brar, akpm, linuxppc-dev, dan.j.williams,
Russell King
On Mon, 2012-01-30 at 08:55 -0800, Bounine, Alexandre wrote:
> On Monday, January 30, 2012 at 4:31 AM, Vinod Koul wrote:
> >
> > On Thu, 2012-01-26 at 16:22 -0500, Alexandre Bounine wrote:
> > > As we agreed during our discussion about adding DMA Engine support for RapidIO
> > > subsystem, RapidIO and similar clients may benefit from adding an extra context
> > > parameter to device_prep_slave_sg() callback.
> > > See https://lkml.org/lkml/2011/10/24/275 for more details.
> > >
> > > Adding the context parameter will allow to pass client/target specific
> > > information associated with an individual data transfer request.
> > >
> > > In the case of RapidIO support this additional information consists of target
> > > destination ID and its buffer address (which is not mapped into the local CPU
> > > memory space). Because a single RapidIO-capable DMA channel may queue data
> > > transfer requests to different target devices, the per-request configuration
> > > is required.
> > >
> > > The proposed change eliminates need for new subsystem-specific API.
> > > Existing DMA_SLAVE clients will ignore the new parameter.
> > >
> > > This RFC only demonstrates the API change and does not include corresponding
> > > changes to existing DMA_SLAVE clients. Complete set of patches will be provided
> > > after (if) this API change is accepted.
> >
> > This looks good to me. But was thinking if we need to add this new
> > parameter for other slave calls (circular, interleaved, memcpy...)
> >
>
> I agree that cyclic and interleaved calls may benefit from adding that parameter as well.
> Benefits to the cyclic call are straightforward - same as dma_slave.
> Adding a context parameter to the interleaved transfers may be more future proofing option
> than an immediate need. Memcopy and other calls that deal with local memory transfers
> probably should be left untouched.
>
> What if we limit modifications to:
> 1) three calls (slave, cyclic and interleaved) OR
> 2) two (slave and cyclic) at this moment?
>
> I am just more focused on dma_slave just because it fits well to provide RDMA
> over RapidIO fabric.
>
> If everybody agrees, I can go ahead and make changes to all three at once.
For now we need at least slave and cyclic, so pls go ahead and make these changes.
For interleaved, we might need it sooner [1], but I would think it would
need few more changes to the API, so it can be rolled as part of those
changes.
--
~Vinod
[1]: https://lkml.org/lkml/2012/1/30/48
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC] dmaengine/dma_slave: add context parameter to prep_slave_sg callback
2012-01-30 9:30 ` Vinod Koul
2012-01-30 16:55 ` Bounine, Alexandre
@ 2012-02-01 0:09 ` Guennadi Liakhovetski
2012-02-01 5:43 ` Vinod Koul
1 sibling, 1 reply; 8+ messages in thread
From: Guennadi Liakhovetski @ 2012-02-01 0:09 UTC (permalink / raw)
To: Vinod Koul
Cc: Jassi Brar, linux-kernel, Alexandre Bounine, akpm, linuxppc-dev,
dan.j.williams, Russell King
On Mon, 30 Jan 2012, Vinod Koul wrote:
> On Thu, 2012-01-26 at 16:22 -0500, Alexandre Bounine wrote:
> > As we agreed during our discussion about adding DMA Engine support for RapidIO
> > subsystem, RapidIO and similar clients may benefit from adding an extra context
> > parameter to device_prep_slave_sg() callback.
> > See https://lkml.org/lkml/2011/10/24/275 for more details.
> >
> > Adding the context parameter will allow to pass client/target specific
> > information associated with an individual data transfer request.
> >
> > In the case of RapidIO support this additional information consists of target
> > destination ID and its buffer address (which is not mapped into the local CPU
> > memory space). Because a single RapidIO-capable DMA channel may queue data
> > transfer requests to different target devices, the per-request configuration
> > is required.
> >
> > The proposed change eliminates need for new subsystem-specific API.
> > Existing DMA_SLAVE clients will ignore the new parameter.
> >
> > This RFC only demonstrates the API change and does not include corresponding
> > changes to existing DMA_SLAVE clients. Complete set of patches will be provided
> > after (if) this API change is accepted.
> This looks good to me. But was thinking if we need to add this new
> parameter for other slave calls (circular, interleaved, memcpy...)
Yes, we (shdma.c) also need to pass additional slave configuration
information to the dmaengine driver and I also was thinking about
extending the existing API, but my ideas were going more in the direction
of adding a parameter to __dma_request_channel() along the lines of
-struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask, dma_filter_fn fn, void *fn_param);
+struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask, dma_filter_fn fn, void *fn_param,
+ struct dma_slave_desc *slave);
where struct dma_slave_desc would basically just contain an opaque slave
ID and would be embedded by dmaengine drivers in their specific
slave-configuration types. The main difference to the API change being
proposed here is, that I was going to configure DMA channels only once for
each slave upon channel allocation, whereas the proposed change does this
on a per-transfer basis. Therefore my question: do I understand the
explanation of the RapidIO DMA architecture from the above quoted thread
right, that DMA channels can be freely used by client drivers, without
binding them to specific DSP cards? In which case you, probably, don't use
DMA_PRIVATE? But that the decision - to which DSP card this specific
transfer is directed - is made by configuring the DMA channels rather than
embedded in the actual data?
Thanks
Guennadi
>
> >
> > Signed-off-by: Alexandre Bounine <alexandre.bounine@idt.com>
> > Cc: Jassi Brar <jaswinder.singh@linaro.org>
> > Cc: Russell King <rmk@arm.linux.org.uk>
> > Cc: Kumar Gala <galak@kernel.crashing.org>
> > Cc: Matt Porter <mporter@kernel.crashing.org>
> > Cc: Li Yang <leoli@freescale.com>
> > ---
> > include/linux/dmaengine.h | 7 ++++---
> > 1 files changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> > index 679b349..79d71bb 100644
> > --- a/include/linux/dmaengine.h
> > +++ b/include/linux/dmaengine.h
> > @@ -575,7 +575,7 @@ struct dma_device {
> > struct dma_async_tx_descriptor *(*device_prep_slave_sg)(
> > struct dma_chan *chan, struct scatterlist *sgl,
> > unsigned int sg_len, enum dma_transfer_direction direction,
> > - unsigned long flags);
> > + unsigned long flags, void *context);
> > struct dma_async_tx_descriptor *(*device_prep_dma_cyclic)(
> > struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
> > size_t period_len, enum dma_transfer_direction direction);
> > @@ -607,12 +607,13 @@ static inline int dmaengine_slave_config(struct dma_chan *chan,
> >
> > static inline struct dma_async_tx_descriptor *dmaengine_prep_slave_single(
> > struct dma_chan *chan, void *buf, size_t len,
> > - enum dma_transfer_direction dir, unsigned long flags)
> > + enum dma_transfer_direction dir, unsigned long flags, void *context)
> > {
> > struct scatterlist sg;
> > sg_init_one(&sg, buf, len);
> >
> > - return chan->device->device_prep_slave_sg(chan, &sg, 1, dir, flags);
> > + return chan->device->device_prep_slave_sg(chan, &sg, 1, dir, flags,
> > + context);
> > }
> >
> > static inline int dmaengine_terminate_all(struct dma_chan *chan)
>
>
> --
> ~Vinod
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC] dmaengine/dma_slave: add context parameter to prep_slave_sg callback
2012-02-01 0:09 ` Guennadi Liakhovetski
@ 2012-02-01 5:43 ` Vinod Koul
2012-02-01 11:58 ` Guennadi Liakhovetski
0 siblings, 1 reply; 8+ messages in thread
From: Vinod Koul @ 2012-02-01 5:43 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: Jassi Brar, linux-kernel, Alexandre Bounine, akpm, linuxppc-dev,
dan.j.williams, Russell King
On Wed, 2012-02-01 at 01:09 +0100, Guennadi Liakhovetski wrote:
> On Mon, 30 Jan 2012, Vinod Koul wrote:
>
> > On Thu, 2012-01-26 at 16:22 -0500, Alexandre Bounine wrote:
> > > As we agreed during our discussion about adding DMA Engine support for RapidIO
> > > subsystem, RapidIO and similar clients may benefit from adding an extra context
> > > parameter to device_prep_slave_sg() callback.
> > > See https://lkml.org/lkml/2011/10/24/275 for more details.
> > >
> > > Adding the context parameter will allow to pass client/target specific
> > > information associated with an individual data transfer request.
> > >
> > > In the case of RapidIO support this additional information consists of target
> > > destination ID and its buffer address (which is not mapped into the local CPU
> > > memory space). Because a single RapidIO-capable DMA channel may queue data
> > > transfer requests to different target devices, the per-request configuration
> > > is required.
> > >
> > > The proposed change eliminates need for new subsystem-specific API.
> > > Existing DMA_SLAVE clients will ignore the new parameter.
> > >
> > > This RFC only demonstrates the API change and does not include corresponding
> > > changes to existing DMA_SLAVE clients. Complete set of patches will be provided
> > > after (if) this API change is accepted.
> > This looks good to me. But was thinking if we need to add this new
> > parameter for other slave calls (circular, interleaved, memcpy...)
>
> Yes, we (shdma.c) also need to pass additional slave configuration
> information to the dmaengine driver and I also was thinking about
> extending the existing API, but my ideas were going more in the direction
> of adding a parameter to __dma_request_channel() along the lines of
So your question is more on the lines of channel mapping/allocation?
The approach here is to pass controller specific parameters which are
required to setup the respective transfer. Since this is dependent on
each transfer, this needs to be passed in respective prepare.
The two things are completely orthogonal and shouldn't be clubbed.
For your issue we need a separate debate on how to solve this... I am
open to ideas...
--
~Vinod
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC] dmaengine/dma_slave: add context parameter to prep_slave_sg callback
2012-02-01 5:43 ` Vinod Koul
@ 2012-02-01 11:58 ` Guennadi Liakhovetski
2012-02-01 14:39 ` Vinod Koul
0 siblings, 1 reply; 8+ messages in thread
From: Guennadi Liakhovetski @ 2012-02-01 11:58 UTC (permalink / raw)
To: Vinod Koul
Cc: Jassi Brar, linux-kernel, Alexandre Bounine, akpm, linuxppc-dev,
dan.j.williams, Russell King
On Wed, 1 Feb 2012, Vinod Koul wrote:
> On Wed, 2012-02-01 at 01:09 +0100, Guennadi Liakhovetski wrote:
> > On Mon, 30 Jan 2012, Vinod Koul wrote:
> >
> > > On Thu, 2012-01-26 at 16:22 -0500, Alexandre Bounine wrote:
> > > > As we agreed during our discussion about adding DMA Engine support for RapidIO
> > > > subsystem, RapidIO and similar clients may benefit from adding an extra context
> > > > parameter to device_prep_slave_sg() callback.
> > > > See https://lkml.org/lkml/2011/10/24/275 for more details.
> > > >
> > > > Adding the context parameter will allow to pass client/target specific
> > > > information associated with an individual data transfer request.
> > > >
> > > > In the case of RapidIO support this additional information consists of target
> > > > destination ID and its buffer address (which is not mapped into the local CPU
> > > > memory space). Because a single RapidIO-capable DMA channel may queue data
> > > > transfer requests to different target devices, the per-request configuration
> > > > is required.
> > > >
> > > > The proposed change eliminates need for new subsystem-specific API.
> > > > Existing DMA_SLAVE clients will ignore the new parameter.
> > > >
> > > > This RFC only demonstrates the API change and does not include corresponding
> > > > changes to existing DMA_SLAVE clients. Complete set of patches will be provided
> > > > after (if) this API change is accepted.
> > > This looks good to me. But was thinking if we need to add this new
> > > parameter for other slave calls (circular, interleaved, memcpy...)
> >
> > Yes, we (shdma.c) also need to pass additional slave configuration
> > information to the dmaengine driver and I also was thinking about
> > extending the existing API, but my ideas were going more in the direction
> > of adding a parameter to __dma_request_channel() along the lines of
> So your question is more on the lines of channel mapping/allocation?
> The approach here is to pass controller specific parameters which are
> required to setup the respective transfer. Since this is dependent on
> each transfer, this needs to be passed in respective prepare.
>
> The two things are completely orthogonal and shouldn't be clubbed.
> For your issue we need a separate debate on how to solve this... I am
> open to ideas...
Well, I'm not sure whether they are necessarily always orthogonal, they
don't seem so in my case at least. We definitely can use our approach -
configure the channel during allocation. I _think_ we could also perform
the configuration on a per-transfer basis, during the prepare stage, as
this RFC is suggesting, but that definitely would require reworking the
driver somewhat and changing the concept. The current concept is a fixed
DMA channel allocation to slaves for as long as the slave is using DMA.
This is simpler, avoids some overhead during operation and fits well with
the dmaengine PRIVATE channel concept. So, given the choice, we would
prefer to perform the configuration during channel allocation.
Maybe there are cases, where the driver absolutely needs this additional
information during allocation, in which case my proposal would be the only
way to go for them.
I'll post an RFC soon - stay tuned:-)
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC] dmaengine/dma_slave: add context parameter to prep_slave_sg callback
2012-02-01 11:58 ` Guennadi Liakhovetski
@ 2012-02-01 14:39 ` Vinod Koul
0 siblings, 0 replies; 8+ messages in thread
From: Vinod Koul @ 2012-02-01 14:39 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: Jassi Brar, linux-kernel, Alexandre Bounine, akpm, linuxppc-dev,
dan.j.williams, Russell King
On Wed, 2012-02-01 at 12:58 +0100, Guennadi Liakhovetski wrote:
> > The two things are completely orthogonal and shouldn't be clubbed.
> > For your issue we need a separate debate on how to solve this... I am
> > open to ideas...
>
> Well, I'm not sure whether they are necessarily always orthogonal, they
> don't seem so in my case at least. We definitely can use our approach -
> configure the channel during allocation. I _think_ we could also perform
> the configuration on a per-transfer basis, during the prepare stage, as
> this RFC is suggesting, but that definitely would require reworking the
> driver somewhat and changing the concept. The current concept is a fixed
> DMA channel allocation to slaves for as long as the slave is using DMA.
> This is simpler, avoids some overhead during operation and fits well with
> the dmaengine PRIVATE channel concept. So, given the choice, we would
> prefer to perform the configuration during channel allocation.
>
> Maybe there are cases, where the driver absolutely needs this additional
> information during allocation, in which case my proposal would be the only
> way to go for them.
what are you trying to address, sending controller specific information
at allocation or the channel allocation itself. I kind of sense both.
But apprach here is discussed is to pass paramters which are required
for each transfer, not static for a channel, hence the additional
controller specific parameter in respective prepare.
>
> I'll post an RFC soon - stay tuned:-)
Patch is always the best idea :-)
--
~Vinod
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-02-01 14:48 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-26 21:22 [RFC] dmaengine/dma_slave: add context parameter to prep_slave_sg callback Alexandre Bounine
2012-01-30 9:30 ` Vinod Koul
2012-01-30 16:55 ` Bounine, Alexandre
2012-01-31 3:14 ` Vinod Koul
2012-02-01 0:09 ` Guennadi Liakhovetski
2012-02-01 5:43 ` Vinod Koul
2012-02-01 11:58 ` Guennadi Liakhovetski
2012-02-01 14:39 ` Vinod Koul
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox