Re: [PATCH v3.5] dw_dmac: return proper residue value

[Viresh Kumar]

On Thu, Jan 24, 2013 at 6:37 PM, Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> Currently the driver returns full length of the active descriptor which is
> wrong. We have to go throught the active descriptor and substract the length of
> each sent children in the chain from the total length along with the actual
> data in the DMA channel registers.
>
> The cyclic case is not handled by this patch due to len field in the descriptor
> structure is left untouched by the original code.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

Firstly the issue i raised earlier about not updating residue from
tasklet or interrupt
handler was wrong. As i had an older version of code in my mind. This got solved
with following patch:

commit 77bcc497c60ec62dbb84abc809a6e218d53409e9
Author: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Date:   Fri Jan 18 14:14:15 2013 +0200

    dw_dmac: move soft LLP code from tasklet to dwc_scan_descriptors


> ---
> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> +static inline u32 dwc_get_residue(struct dw_dma_chan *dwc)
> +{
> +       unsigned long flags;
> +       u32 residue;
> +
> +       spin_lock_irqsave(&dwc->lock, flags);
> +
> +       residue = dwc->residue;

you need to release the lock just here.

> +       if (test_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags) && residue)
> +               residue -= dwc_get_sent(dwc);

why do you need lock while reading the control registers? It looks you didn't
get what i wanted to say earlier. We are using locks to protect some part for
shared data or critical sections. these are playing with dwc transfer
lists, etc..

Probably we don't need a lock to read the control register as nobody can
guarantee that another access is not happening currently. As hardware is
changing this register continuously for the transfer.

Let me know if i am missing something.

> +       spin_unlock_irqrestore(&dwc->lock, flags);
> +       return residue;
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/