Le 12/05/2016 16:54, Ludovic Desroches a écrit :
> An unexpected value of CUBC can lead to a corrupted residue. A more
> complex sequence is needed to detect an inaccurate value for NCA or CUBC.
>
> Signed-off-by: Ludovic Desroches <ludovic.desroches@xxxxxxxxx>
> Fixes: e1f7c9eee707 ("dmaengine: at_xdmac: creation of the atmel
> eXtended DMA Controller driver")
> Cc: stable@xxxxxxxxxxxxxxx #v4.1 and later
Reviewed-by: Nicolas Ferre <nicolas.ferre@xxxxxxxxx>
> ---
> drivers/dma/at_xdmac.c | 54 ++++++++++++++++++++++++++++++--------------------
> 1 file changed, 32 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
> index ba9b0b7..b02494e 100644
> --- a/drivers/dma/at_xdmac.c
> +++ b/drivers/dma/at_xdmac.c
> @@ -1400,6 +1400,7 @@ at_xdmac_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
> u32 cur_nda, check_nda, cur_ubc, mask, value;
> u8 dwidth = 0;
> unsigned long flags;
> + bool initd;
>
> ret = dma_cookie_status(chan, cookie, txstate);
> if (ret == DMA_COMPLETE)
> @@ -1435,34 +1436,43 @@ at_xdmac_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
> }
>
> /*
> - * When processing the residue, we need to read two registers but we
> - * can't do it in an atomic way. AT_XDMAC_CNDA is used to find where
> - * we stand in the descriptor list and AT_XDMAC_CUBC is used
> - * to know how many data are remaining for the current descriptor.
> - * Since the dma channel is not paused to not loose data, between the
> - * AT_XDMAC_CNDA and AT_XDMAC_CUBC read, we may have change of
> - * descriptor.
> - * For that reason, after reading AT_XDMAC_CUBC, we check if we are
> - * still using the same descriptor by reading a second time
> - * AT_XDMAC_CNDA. If AT_XDMAC_CNDA has changed, it means we have to
> - * read again AT_XDMAC_CUBC.
> + * The easiest way to compute the residue should be to pause the DMA
> + * but doing this can lead to miss some data as some devices don't
> + * have FIFO.
> + * We need to read several registers because:
> + * - DMA is running therefore a descriptor change is possible while
> + * reading these registers
> + * - When the block transfer is done, the value of the CUBC register
> + * is set to its initial value until the fetch of the next descriptor.
> + * This value will corrupt the residue calculation so we have to skip
> + * it.
> + *
> + * INITD -------- ------------
> + * |____________________|
> + * _______________________ _______________
> + * NDA @desc2 \/ @desc3
> + * _______________________/\_______________
> + * __________ ___________ _______________
> + * CUBC 0 \/ MAX desc1 \/ MAX desc2
> + * __________/\___________/\_______________
> + *
> + * Since descriptors are aligned on 64 bits, we can assume that
> + * the update of NDA and CUBC is atomic.
> * Memory barriers are used to ensure the read order of the registers.
> - * A max number of retries is set because unlikely it can never ends if
> - * we are transferring a lot of data with small buffers.
> + * A max number of retries is set because unlikely it could never ends.
> */
> - cur_nda = at_xdmac_chan_read(atchan, AT_XDMAC_CNDA) & 0xfffffffc;
> - rmb();
> - cur_ubc = at_xdmac_chan_read(atchan, AT_XDMAC_CUBC);
> for (retry = 0; retry < AT_XDMAC_RESIDUE_MAX_RETRIES; retry++) {
> - rmb();
> check_nda = at_xdmac_chan_read(atchan, AT_XDMAC_CNDA) & 0xfffffffc;
> -
> - if (likely(cur_nda == check_nda))
> - break;
> -
> - cur_nda = check_nda;
> + rmb();
> + initd = !!(at_xdmac_chan_read(atchan, AT_XDMAC_CC) & AT_XDMAC_CC_INITD);
> rmb();
> cur_ubc = at_xdmac_chan_read(atchan, AT_XDMAC_CUBC);
> + rmb();
> + cur_nda = at_xdmac_chan_read(atchan, AT_XDMAC_CNDA) & 0xfffffffc;
> + rmb();
> +
> + if ((check_nda == cur_nda) && initd)
> + break;
> }
>
> if (unlikely(retry >= AT_XDMAC_RESIDUE_MAX_RETRIES)) {
>
--
Nicolas Ferre
--
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
