| Message ID | 1505732881-7484-2-git-send-email-lesne@alse-fr.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On 18.09.2017 13:08, Sylvain Lesne wrote: > Commit 6084fc2ec478 ("dmaengine: altera: Use macros instead of structs > to describe the registers") introduced a minus sign before a register > offset. > > This leads to soft-locks of the DMA controller, since reading the last > status byte is required to pop the response from the FIFO. Failing to > do so will lead to a full FIFO, which means that the DMA controller > will stop processing descriptors. > > Signed-off-by: Sylvain Lesne <lesne@alse-fr.com> > --- > According to the datasheet, I think we also could drop the "bytes > transferred" read, since only the last byte of the status register has > the side effect of emptying the FIFO. > --- > drivers/dma/altera-msgdma.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma/altera-msgdma.c b/drivers/dma/altera-msgdma.c > index 32905d5606ac..35cbf2365f68 100644 > --- a/drivers/dma/altera-msgdma.c > +++ b/drivers/dma/altera-msgdma.c > @@ -698,7 +698,7 @@ static void msgdma_tasklet(unsigned long data) > * bits. So we need to just drop these values. > */ > size = ioread32(mdev->resp + MSGDMA_RESP_BYTES_TRANSFERRED); > - status = ioread32(mdev->resp - MSGDMA_RESP_STATUS); > + status = ioread32(mdev->resp + MSGDMA_RESP_STATUS); > > msgdma_complete_descriptor(mdev); > msgdma_chan_desc_cleanup(mdev); > Thanks for spotting this: Reviewed-by: Stefan Roese <sr@denx.de> Thanks, Stefan -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/dma/altera-msgdma.c b/drivers/dma/altera-msgdma.c index 32905d5606ac..35cbf2365f68 100644 --- a/drivers/dma/altera-msgdma.c +++ b/drivers/dma/altera-msgdma.c @@ -698,7 +698,7 @@ static void msgdma_tasklet(unsigned long data) * bits. So we need to just drop these values. */ size = ioread32(mdev->resp + MSGDMA_RESP_BYTES_TRANSFERRED); - status = ioread32(mdev->resp - MSGDMA_RESP_STATUS); + status = ioread32(mdev->resp + MSGDMA_RESP_STATUS); msgdma_complete_descriptor(mdev); msgdma_chan_desc_cleanup(mdev);
Commit 6084fc2ec478 ("dmaengine: altera: Use macros instead of structs to describe the registers") introduced a minus sign before a register offset. This leads to soft-locks of the DMA controller, since reading the last status byte is required to pop the response from the FIFO. Failing to do so will lead to a full FIFO, which means that the DMA controller will stop processing descriptors. Signed-off-by: Sylvain Lesne <lesne@alse-fr.com> --- According to the datasheet, I think we also could drop the "bytes transferred" read, since only the last byte of the status register has the side effect of emptying the FIFO. --- drivers/dma/altera-msgdma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)