On Mon, Nov 23, 2015 at 02:22:04PM +0000, Damien Horsley wrote:
> From: "Damien.Horsley" <Damien.Horsley@xxxxxxxxxx>
>
> Use of the CANCEL bit in mdc_terminate_all creates an
> additional 'command done' to appear in the registers (in
> addition to an interrupt).
>
> In addition, there is a potential race between
> mdc_terminate_all and the irq handler if a transfer
> completes at the same time as the terminate all (presently
> this results in an inappropriate warning).
>
> To handle these issues, any outstanding 'command done'
> events are cleared during mdc_terminate_all and the irq
> handler takes no action when there are no new 'command done'
> events.
SO what does 'command done' event represent, and what would be the behaviour
of ignoring those
>
> Signed-off-by: Damien.Horsley <Damien.Horsley@xxxxxxxxxx>
> ---
> drivers/dma/img-mdc-dma.c | 71 ++++++++++++++++++++++++++++++-----------------
> 1 file changed, 45 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/dma/img-mdc-dma.c b/drivers/dma/img-mdc-dma.c
> index 9ca5683..aa09590 100644
> --- a/drivers/dma/img-mdc-dma.c
> +++ b/drivers/dma/img-mdc-dma.c
> @@ -651,6 +651,42 @@ static enum dma_status mdc_tx_status(struct dma_chan *chan,
> return ret;
> }
>
> +static unsigned int mdc_get_new_events(struct mdc_chan *mchan)
> +{
> + u32 val, processed, done1, done2;
> + unsigned int ret;
> +
> + val = mdc_chan_readl(mchan, MDC_CMDS_PROCESSED);
> + processed = (val >> MDC_CMDS_PROCESSED_CMDS_PROCESSED_SHIFT) &
> + MDC_CMDS_PROCESSED_CMDS_PROCESSED_MASK;
This should be right justfied
> + /*
> + * CMDS_DONE may have incremented between reading CMDS_PROCESSED
> + * and clearing INT_ACTIVE. Re-read CMDS_PROCESSED to ensure we
> + * didn't miss a command completion.
> + */
> + do {
> + val = mdc_chan_readl(mchan, MDC_CMDS_PROCESSED);
> + done1 = (val >> MDC_CMDS_PROCESSED_CMDS_DONE_SHIFT) &
> + MDC_CMDS_PROCESSED_CMDS_DONE_MASK;
> + val &= ~((MDC_CMDS_PROCESSED_CMDS_PROCESSED_MASK <<
> + MDC_CMDS_PROCESSED_CMDS_PROCESSED_SHIFT) |
> + MDC_CMDS_PROCESSED_INT_ACTIVE);
> + val |= done1 << MDC_CMDS_PROCESSED_CMDS_PROCESSED_SHIFT;
> + mdc_chan_writel(mchan, val, MDC_CMDS_PROCESSED);
> + val = mdc_chan_readl(mchan, MDC_CMDS_PROCESSED);
> + done2 = (val >> MDC_CMDS_PROCESSED_CMDS_DONE_SHIFT) &
> + MDC_CMDS_PROCESSED_CMDS_DONE_MASK;
this is very hard to read, can you give some empty lines on logical blocks
> + } while (done1 != done2);
> +
> + if (done1 >= processed)
> + ret = done1 - processed;
> + else
> + ret = ((MDC_CMDS_PROCESSED_CMDS_PROCESSED_MASK + 1) -
> + processed) + done1;
> +
> + return ret;
> +}
> +
> static int mdc_terminate_all(struct dma_chan *chan)
> {
> struct mdc_chan *mchan = to_mdc_chan(chan);
> @@ -667,6 +703,8 @@ static int mdc_terminate_all(struct dma_chan *chan)
> mchan->desc = NULL;
> vchan_get_all_descriptors(&mchan->vc, &head);
>
> + mdc_get_new_events(mchan);
> +
Only getting?
--
~Vinod
--
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
