On Mon, Nov 23, 2015 at 02:22:04PM +0000, Damien Horsley wrote:
> From: "Damien.Horsley" <[email protected]>
> 
> 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 <[email protected]>
> ---
>  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 linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to