lkml.org 
[lkml]   [2010]   [Mar]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 2/2] DMAENGINE: generic channel status
On Thu, 25 Mar 2010, Linus Walleij wrote:

> Convert the device_is_tx_complete() operation on the
> DMA engine to a generic device_tx_status()operation which
> can return three states, DMA_TX_RUNNING, DMA_TX_COMPLETE,
> DMA_TX_PAUSED.
>
> Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
> Cc: Maciej Sosnowski <maciej.sosnowski@intel.com>
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Li Yang <leoli@freescale.com>
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Paul Mundt <lethal@linux-sh.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
> Cc: Magnus Damm <damm@opensource.se>
> Cc: Liam Girdwood <lrg@slimlogic.co.uk>
> Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Cc: Joe Perches <joe@perches.com>
> Cc: Roland Dreier <rdreier@cisco.com>
> ---

[snip]

General: you converted all drivers to the new .device_tx_status() API, but
since they don't implement "residue," you left it uninitialised
everywhere. Wouldn't it be better to set it to 0 or total length,
depending on the complete / not complete status?

> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 0731802..c9f2c67 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -40,11 +40,13 @@ typedef s32 dma_cookie_t;
> * enum dma_status - DMA transaction status
> * @DMA_SUCCESS: transaction completed successfully
> * @DMA_IN_PROGRESS: transaction not yet processed
> + * @DMA_PAUSED: transaction is paused
> * @DMA_ERROR: transaction failed
> */
> enum dma_status {
> DMA_SUCCESS,
> DMA_IN_PROGRESS,
> + DMA_PAUSED,
> DMA_ERROR,
> };
>
> @@ -249,6 +251,21 @@ struct dma_async_tx_descriptor {
> };
>
> /**
> + * struct dma_tx_state - filled in to report the status of
> + * a transfer.
> + * @last: last completed DMA cookie
> + * @used: last issued DMA cookie (i.e. the one in progress)
> + * @residue: the remaining number of bytes left to transmit
> + * on the selected transfer for states DMA_IN_PROGRESS and
> + * DMA_PAUSED if this is implemented in the driver, else 0
> + */
> +struct dma_tx_state {
> + dma_cookie_t last;
> + dma_cookie_t used;
> + u32 residue;

In the original proposal by Dan Williams the last member was "unsigned
long pos." I don't think, even on 64-bit systems anyone would kick off a >
4GB transfer, but who knows... And - I don't particularly like the name
"pos," but I do like the idea of returning bytes transfered better, than
bytes left. Can we change this?

> +};
> +
> +/**
> * struct dma_device - info on the entity supplying DMA services
> * @chancnt: how many DMA channels are supported
> * @privatecnt: how many DMA channels are requested by dma_request_channel
> @@ -276,7 +293,10 @@ struct dma_async_tx_descriptor {
> * @device_prep_slave_sg: prepares a slave dma operation
> * @device_control: manipulate all pending operations on a channel, returns
> * zero or error code
> - * @device_is_tx_complete: poll for transaction completion
> + * @device_tx_status: poll for transaction completion, the optional
> + * txstate parameter can be supplied with a pointer to get a
> + * struct with some transfer information, else the call will just

Maybe "with auxiliary transfer status information, otherwise..."

> + * return a simple status code
> * @device_issue_pending: push pending transactions to hardware
> */
> struct dma_device {
> @@ -329,9 +349,9 @@ struct dma_device {
> unsigned long flags);
> int (*device_control)(struct dma_chan *chan, enum dma_ctrl_cmd cmd);
>
> - enum dma_status (*device_is_tx_complete)(struct dma_chan *chan,
> - dma_cookie_t cookie, dma_cookie_t *last,
> - dma_cookie_t *used);
> + enum dma_status (*device_tx_status)(struct dma_chan *chan,
> + dma_cookie_t cookie,
> + struct dma_tx_state *txstate);
> void (*device_issue_pending)(struct dma_chan *chan);
> };
>
> @@ -572,7 +592,13 @@ static inline void dma_async_issue_pending(struct dma_chan *chan)
> static inline enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
> dma_cookie_t cookie, dma_cookie_t *last, dma_cookie_t *used)
> {
> - return chan->device->device_is_tx_complete(chan, cookie, last, used);
> + struct dma_tx_state state;
> + enum dma_status status;
> +
> + status = chan->device->device_tx_status(chan, cookie, &state);
> + *last = state.last;
> + *used = state.used;

Both last and used can be NULL.

> + return status;
> }
>
> #define dma_async_memcpy_complete(chan, cookie, last, used)\
> --
> 1.6.3.3

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


\
 
 \ /
  Last update: 2010-03-25 22:01    [W:1.433 / U:0.448 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site