| Message ID | 1463064854-6719-2-git-send-email-ludovic.desroches@atmel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@atmel.com> > Fixes: e1f7c9eee707 ("dmaengine: at_xdmac: creation of the atmel > eXtended DMA Controller driver") > Cc: stable@vger.kernel.org #v4.1 and later Reviewed-by: Nicolas Ferre <nicolas.ferre@atmel.com> > --- > 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)) { >
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)) {
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@atmel.com> Fixes: e1f7c9eee707 ("dmaengine: at_xdmac: creation of the atmel eXtended DMA Controller driver") Cc: stable@vger.kernel.org #v4.1 and later --- drivers/dma/at_xdmac.c | 54 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 32 insertions(+), 22 deletions(-)