2010-07-23 17:28:53

by Linus Walleij

[permalink] [raw]
Subject: [PATCH] DMAENGINE: generic slave channel control v3

This adds an interface to the DMAengine to make it possible to
reconfigure a slave channel at runtime. We add a few foreseen
config parameters to the passed struct, with a void * pointer
for custom per-device or per-platform runtime slave data.

Signed-off-by: Linus Walleij <[email protected]>
---
Changes v2->v3:
- Drop the .private member since there is no users. We can always
add it later if need be.
- Fix a few speling mistakes.

Remaining patches adding channel control to ste_dma40 and
coh901318 as well as the PL08x driver remain unaffected since
none of them make use of the private element.
---
include/linux/dmaengine.h | 71 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 71 insertions(+), 0 deletions(-)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 5204f01..c61d4ca 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -114,11 +114,17 @@ enum dma_ctrl_flags {
* @DMA_TERMINATE_ALL: terminate all ongoing transfers
* @DMA_PAUSE: pause ongoing transfers
* @DMA_RESUME: resume paused transfer
+ * @DMA_SLAVE_CONFIG: this command is only implemented by DMA controllers
+ * that need to runtime reconfigure the slave channels (as opposed to passing
+ * configuration data in statically from the platform). An additional
+ * argument of struct dma_slave_config must be passed in with this
+ * command.
*/
enum dma_ctrl_cmd {
DMA_TERMINATE_ALL,
DMA_PAUSE,
DMA_RESUME,
+ DMA_SLAVE_CONFIG,
};

/**
@@ -199,6 +205,71 @@ struct dma_chan_dev {
atomic_t *idr_ref;
};

+/**
+ * enum dma_slave_buswidth - defines bus with of the DMA slave
+ * device, source or target buses
+ */
+enum dma_slave_buswidth {
+ DMA_SLAVE_BUSWIDTH_UNDEFINED = 0,
+ DMA_SLAVE_BUSWIDTH_1_BYTE = 1,
+ DMA_SLAVE_BUSWIDTH_2_BYTES = 2,
+ DMA_SLAVE_BUSWIDTH_4_BYTES = 4,
+ DMA_SLAVE_BUSWIDTH_8_BYTES = 8,
+};
+
+/**
+ * struct dma_slave_config - dma slave channel runtime config
+ * @direction: whether the data shall go in or out on this slave
+ * channel, right now. DMA_TO_DEVICE and DMA_FROM_DEVICE are
+ * legal values, DMA_BIDIRECTIONAL is not acceptable since we
+ * need to differentiate source and target addresses.
+ * @src_addr: this is the physical address where DMA slave data
+ * should be read (RX), if the source is memory this argument is
+ * ignored.
+ * @dst_addr: this is the physical address where DMA slave data
+ * should be written (TX), if the source is memory this argument
+ * is ignored.
+ * @src_addr_width: this is the width in bytes of the source (RX)
+ * register where DMA data shall be read. If the source
+ * is memory this may be ignored depending on architecture.
+ * Legal values: 1, 2, 4, 8.
+ * @dst_addr_width: same as src_addr_width but for destination
+ * target (TX) mutatis mutandis.
+ * @src_maxburst: the maximum number of words (note: words, as in
+ * units of the src_addr_width member, not bytes) that can be sent
+ * in one burst to the device. Typically something like half the
+ * FIFO depth on I/O peripherals so you don't overflow it. This
+ * may or may not be applicable on memory sources.
+ * @dst_maxburst: same as src_maxburst but for destination target
+ * mutatis mutandis.
+ *
+ * This struct is passed in as configuration data to a DMA engine
+ * in order to set up a certain channel for DMA transport at runtime.
+ * The DMA device/engine has to provide support for an additional
+ * command in the channel config interface, DMA_SLAVE_CONFIG
+ * and this struct will then be passed in as an argument to the
+ * DMA engine device_control() function.
+ *
+ * The rationale for adding configuration information to this struct
+ * is as follows: if it is likely that most DMA slave controllers in
+ * the world will support the configuration option, then make it
+ * generic. If not: if it is fixed so that it be sent in static from
+ * the platform data, then prefer to do that. Else, if it is neither
+ * fixed at runtime, nor generic enough (such as bus mastership on
+ * some CPU family and whatnot) then create a custom slave config
+ * struct and pass that, then make this config a member of that
+ * struct, if applicable.
+ */
+struct dma_slave_config {
+ enum dma_data_direction direction;
+ dma_addr_t src_addr;
+ dma_addr_t dst_addr;
+ enum dma_slave_buswidth src_addr_width;
+ enum dma_slave_buswidth dst_addr_width;
+ u32 src_maxburst;
+ u32 dst_maxburst;
+};
+
static inline const char *dma_chan_name(struct dma_chan *chan)
{
return dev_name(&chan->dev->device);
--
1.7.1.1


2010-07-28 07:28:08

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH] DMAENGINE: generic slave channel control v3

On Fri, Jul 23, 2010 at 10:27 AM, Linus Walleij
<[email protected]> wrote:
> This adds an interface to the DMAengine to make it possible to
> reconfigure a slave channel at runtime. We add a few foreseen
> config parameters to the passed struct, with a void * pointer
> for custom per-device or per-platform runtime slave data.
>
> Signed-off-by: Linus Walleij <[email protected]>

Ok, let's go with this version, but one question before I apply it:

What are the rules for when a new dma_slave_config can be set?
Looking at the driver implementations it seems there is nothing
preventing a concurrent call to device_control to occur at "wrong
time"? Is there a synchronization context I'm missing?

--
Dan

2010-07-28 23:04:27

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH] DMAENGINE: generic slave channel control v3

2010/7/28 Dan Williams <[email protected]>:

> What are the rules for when a new dma_slave_config can be set?

My idea is that these are done immediately before any work
to be carried out.

> Looking at the driver implementations it seems there is nothing
> preventing a concurrent call to device_control to occur at "wrong
> time"? ?Is there a synchronization context I'm missing?

I do imagine that the code using this will be naturally serialized,
since you need to configure the channel before using it for
something. i.e. the channel is typically STOPPED. If you like
I can add checking for this to the coh901318 and ste_dma40
patches. (It wouldn't affect this one though.)

Drivers that want to support reconfiguring while a channel is
running a transfer may have to look harder.

Yours,
Linus Walleij