Hi Dave,
Thank you for the patch.
On Thursday 14 Jul 2016 15:00:09 Dave Jiang wrote:
> Adding a new callback that will provide the error result for a transaction.
> The result is allocated on the stack and the callback should create a copy
> if it wishes to retain the information after exiting.
>
> Signed-off-by: Dave Jiang <dave.jiang@xxxxxxxxx>
> ---
> include/linux/dmaengine.h | 35 ++++++++++++++++++++++++++++++++---
Missing change for Documentation/dmaengine/*
> 1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index b601f23..ab8c498 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -441,6 +441,21 @@ typedef bool (*dma_filter_fn)(struct dma_chan *chan,
> void *filter_param);
>
> typedef void (*dma_async_tx_callback)(void *dma_async_param);
>
> +enum dma_trans_result {
> + DMA_TRANS_NOERROR = 0,
> + DMA_TRANS_READ_FAILED,
> + DMA_TRANS_WRITE_FAILED,
> + DMA_TRANS_ABORTED,
> +};
> +
> +struct dmaengine_result {
> + enum dma_trans_result result;
> + u32 residue;
> +};
> +
> +typedef void (*dma_async_tx_callback_result)(void *dma_async_param,
> + struct dmaengine_result *result);
How about making the result const ? Is there any reason to allow the callback
to modify the result ?
> +
> struct dmaengine_unmap_data {
> u8 map_cnt;
> u8 to_cnt;
> @@ -478,6 +493,7 @@ struct dma_async_tx_descriptor {
> dma_cookie_t (*tx_submit)(struct dma_async_tx_descriptor *tx);
> int (*desc_free)(struct dma_async_tx_descriptor *tx);
> dma_async_tx_callback callback;
> + dma_async_tx_callback_result callback_result;
So we now have two APIs. On the DMA engine drivers side, the transition path
is quite clear. Drivers should be ported to the new API to report results. The
results will be ignored by older client drivers, and newer client drivers that
interact with older DMA engine drivers will get dummy results that always
report success. There's no regression compared to the current API as client
drivers have no way to detect failures today.
On the client drivers side however, what's your plan to transition to the new
API ? We don't want to keep two APIs forever, so we should get all client
drivers converted in the not too distant future. There needs to be an
incentive for that to happen, unless you plan to do the work yourself. The
latter option would certainly be appreciated :-)
> void *callback_param;
> struct dmaengine_unmap_data *unmap;
> #ifdef CONFIG_ASYNC_TX_ENABLE_CHANNEL_SWITCH
> @@ -1374,6 +1390,7 @@ static inline int dmaengine_desc_free(struct
> dma_async_tx_descriptor *desc)
>
> struct dma_desc_callback {
> dma_async_tx_callback callback;
> + dma_async_tx_callback_result callback_result;
> void *callback_param;
> };
>
> @@ -1382,14 +1399,26 @@ dmaengine_desc_get_callback(struct
> dma_async_tx_descriptor *tx, struct dma_desc_callback *cb)
> {
> cb->callback = tx->callback;
> + cb->callback_result = tx->callback_result;
> cb->callback_param = tx->callback_param;
> }
>
> static inline void
> -dmaengine_desc_callback_invoke(struct dma_desc_callback *cb, void *result)
> -{
> - if (cb->callback)
> +dmaengine_desc_callback_invoke(struct dma_desc_callback *cb,
> + struct dmaengine_result *result)
> +{
> + struct dmaengine_result dummy_result = {
> + .result = DMA_TRANS_NOERROR,
> + .residue = 0
> + };
> +
> + if (cb->callback_result) {
> + if (!result)
> + result = &dummy_result;
> + cb->callback_result(cb->callback_param, result);
> + } else if (cb->callback) {
> cb->callback(cb->callback_param);
> + }
> }
>
> /* --- DMA device --- */
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
