From 06fefe5a023583f235f642b3f667bb40de3eddaa Mon Sep 17 00:00:00 2001 From: Codrin Ciubotariu Date: Wed, 23 Jan 2019 16:33:47 +0000 Subject: [PATCH 1/4] dmaengine: at_xdmac: Fix wrongfull report of a channel as in use atchan->status variable is used to store two different information: - pass channel interrupts status from interrupt handler to tasklet; - channel information like whether it is cyclic or paused; This causes a bug when device_terminate_all() is called, (AT_XDMAC_CHAN_IS_CYCLIC cleared on atchan->status) and then a late End of Block interrupt arrives (AT_XDMAC_CIS_BIS), which sets bit 0 of atchan->status. Bit 0 is also used for AT_XDMAC_CHAN_IS_CYCLIC, so when a new descriptor for a cyclic transfer is created, the driver reports the channel as in use: if (test_and_set_bit(AT_XDMAC_CHAN_IS_CYCLIC, &atchan->status)) { dev_err(chan2dev(chan), "channel currently used\n"); return NULL; } This patch fixes the bug by adding a different struct member to keep the interrupts status separated from the channel status bits. Fixes: e1f7c9eee707 ("dmaengine: at_xdmac: creation of the atmel eXtended DMA Controller driver") Signed-off-by: Codrin Ciubotariu Acked-by: Ludovic Desroches --- drivers/dma/at_xdmac.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c index c00e3923d7d811..5884a9b19e71bd 100644 --- a/drivers/dma/at_xdmac.c +++ b/drivers/dma/at_xdmac.c @@ -203,6 +203,7 @@ struct at_xdmac_chan { u32 save_cim; u32 save_cnda; u32 save_cndc; + u32 irq_status; unsigned long status; struct tasklet_struct tasklet; struct dma_slave_config sconfig; @@ -1580,8 +1581,8 @@ static void at_xdmac_tasklet(unsigned long data) struct at_xdmac_desc *desc; u32 error_mask; - dev_dbg(chan2dev(&atchan->chan), "%s: status=0x%08lx\n", - __func__, atchan->status); + dev_dbg(chan2dev(&atchan->chan), "%s: status=0x%08x\n", + __func__, atchan->irq_status); error_mask = AT_XDMAC_CIS_RBEIS | AT_XDMAC_CIS_WBEIS @@ -1589,15 +1590,15 @@ static void at_xdmac_tasklet(unsigned long data) if (at_xdmac_chan_is_cyclic(atchan)) { at_xdmac_handle_cyclic(atchan); - } else if ((atchan->status & AT_XDMAC_CIS_LIS) - || (atchan->status & error_mask)) { + } else if ((atchan->irq_status & AT_XDMAC_CIS_LIS) + || (atchan->irq_status & error_mask)) { struct dma_async_tx_descriptor *txd; - if (atchan->status & AT_XDMAC_CIS_RBEIS) + if (atchan->irq_status & AT_XDMAC_CIS_RBEIS) dev_err(chan2dev(&atchan->chan), "read bus error!!!"); - if (atchan->status & AT_XDMAC_CIS_WBEIS) + if (atchan->irq_status & AT_XDMAC_CIS_WBEIS) dev_err(chan2dev(&atchan->chan), "write bus error!!!"); - if (atchan->status & AT_XDMAC_CIS_ROIS) + if (atchan->irq_status & AT_XDMAC_CIS_ROIS) dev_err(chan2dev(&atchan->chan), "request overflow error!!!"); spin_lock_bh(&atchan->lock); @@ -1652,7 +1653,7 @@ static irqreturn_t at_xdmac_interrupt(int irq, void *dev_id) atchan = &atxdmac->chan[i]; chan_imr = at_xdmac_chan_read(atchan, AT_XDMAC_CIM); chan_status = at_xdmac_chan_read(atchan, AT_XDMAC_CIS); - atchan->status = chan_status & chan_imr; + atchan->irq_status = chan_status & chan_imr; dev_vdbg(atxdmac->dma.dev, "%s: chan%d: imr=0x%x, status=0x%x\n", __func__, i, chan_imr, chan_status); @@ -1666,7 +1667,7 @@ static irqreturn_t at_xdmac_interrupt(int irq, void *dev_id) at_xdmac_chan_read(atchan, AT_XDMAC_CDA), at_xdmac_chan_read(atchan, AT_XDMAC_CUBC)); - if (atchan->status & (AT_XDMAC_CIS_RBEIS | AT_XDMAC_CIS_WBEIS)) + if (atchan->irq_status & (AT_XDMAC_CIS_RBEIS | AT_XDMAC_CIS_WBEIS)) at_xdmac_write(atxdmac, AT_XDMAC_GD, atchan->mask); tasklet_schedule(&atchan->tasklet); From 5b4fa3bb43de063109ee40b2383e9cbda1be2e1a Mon Sep 17 00:00:00 2001 From: Nicolas Ferre Date: Tue, 5 Feb 2019 12:03:41 +0100 Subject: [PATCH 2/4] dmaengine: at_xdmac: remove BUG_ON macro in tasklet Even if this case shouldn't happen when controller is properly programmed, it's still better to avoid dumping a kernel Oops for this. As the sequence may happen only for debugging purposes, log the error and just finish the tasklet call. Signed-off-by: Nicolas Ferre --- drivers/dma/at_xdmac.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c index 5884a9b19e71bd..1a8ef1c4b59976 100644 --- a/drivers/dma/at_xdmac.c +++ b/drivers/dma/at_xdmac.c @@ -1606,7 +1606,11 @@ static void at_xdmac_tasklet(unsigned long data) struct at_xdmac_desc, xfer_node); dev_vdbg(chan2dev(&atchan->chan), "%s: desc 0x%p\n", __func__, desc); - BUG_ON(!desc->active_xfer); + if (!desc->active_xfer) { + dev_err(chan2dev(&atchan->chan), "Xfer not active: exiting"); + spin_unlock_bh(&atchan->lock); + return; + } txd = &desc->tx_dma_desc; From 52562782d6c0c170eb88b30b5fb0344d9e7b2bb2 Mon Sep 17 00:00:00 2001 From: Nicolas Ferre Date: Tue, 5 Feb 2019 12:03:42 +0100 Subject: [PATCH 3/4] dmaengine: at_xdmac: enhance channel errors handling in tasklet Complement the identification of errors with stoping the channel and dumping the descriptor that led to the error case. Signed-off-by: Nicolas Ferre --- drivers/dma/at_xdmac.c | 43 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 6 deletions(-) diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c index 1a8ef1c4b59976..dc6bde8eab69b5 100644 --- a/drivers/dma/at_xdmac.c +++ b/drivers/dma/at_xdmac.c @@ -1575,6 +1575,41 @@ static void at_xdmac_handle_cyclic(struct at_xdmac_chan *atchan) dmaengine_desc_get_callback_invoke(txd, NULL); } +static void at_xdmac_handle_error(struct at_xdmac_chan *atchan) +{ + struct at_xdmac *atxdmac = to_at_xdmac(atchan->chan.device); + struct at_xdmac_desc *bad_desc; + + /* + * The descriptor currently at the head of the active list is + * broked. Since we don't have any way to report errors, we'll + * just have to scream loudly and try to carry on. + */ + if (atchan->irq_status & AT_XDMAC_CIS_RBEIS) + dev_err(chan2dev(&atchan->chan), "read bus error!!!"); + if (atchan->irq_status & AT_XDMAC_CIS_WBEIS) + dev_err(chan2dev(&atchan->chan), "write bus error!!!"); + if (atchan->irq_status & AT_XDMAC_CIS_ROIS) + dev_err(chan2dev(&atchan->chan), "request overflow error!!!"); + + spin_lock_bh(&atchan->lock); + /* Channel must be disabled first as it's not done automatically */ + at_xdmac_write(atxdmac, AT_XDMAC_GD, atchan->mask); + while (at_xdmac_read(atxdmac, AT_XDMAC_GS) & atchan->mask) + cpu_relax(); + bad_desc = list_first_entry(&atchan->xfers_list, + struct at_xdmac_desc, + xfer_node); + spin_unlock_bh(&atchan->lock); + /* Print bad descriptor's details if needed */ + dev_dbg(chan2dev(&atchan->chan), + "%s: lld: mbr_sa=%pad, mbr_da=%pad, mbr_ubc=0x%08x\n", + __func__, &bad_desc->lld.mbr_sa, &bad_desc->lld.mbr_da, + bad_desc->lld.mbr_ubc); + + /* Then continue with usual descriptor management */ +} + static void at_xdmac_tasklet(unsigned long data) { struct at_xdmac_chan *atchan = (struct at_xdmac_chan *)data; @@ -1594,12 +1629,8 @@ static void at_xdmac_tasklet(unsigned long data) || (atchan->irq_status & error_mask)) { struct dma_async_tx_descriptor *txd; - if (atchan->irq_status & AT_XDMAC_CIS_RBEIS) - dev_err(chan2dev(&atchan->chan), "read bus error!!!"); - if (atchan->irq_status & AT_XDMAC_CIS_WBEIS) - dev_err(chan2dev(&atchan->chan), "write bus error!!!"); - if (atchan->irq_status & AT_XDMAC_CIS_ROIS) - dev_err(chan2dev(&atchan->chan), "request overflow error!!!"); + if (atchan->irq_status & error_mask) + at_xdmac_handle_error(atchan); spin_lock_bh(&atchan->lock); desc = list_first_entry(&atchan->xfers_list, From a902f6fc50ee4c1de74eb3028182aafd00f45503 Mon Sep 17 00:00:00 2001 From: Nicolas Ferre Date: Tue, 5 Feb 2019 12:03:43 +0100 Subject: [PATCH 4/4] dmaengine: at_xdmac: only monitor overflow errors for peripheral xfer The overflow error flag (ROI: Request Overflow Error) is only relevant for the case when the channel handles a peripheral synchronized transfer. Not in the case of memory to memory transfer where there is no hardware request signal. Remove the use of this interrupt source in such a case. It's based on the first descriptor which holds the configuration for the whole linked list transfer. Signed-off-by: Nicolas Ferre --- drivers/dma/at_xdmac.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c index dc6bde8eab69b5..672792492b68ae 100644 --- a/drivers/dma/at_xdmac.c +++ b/drivers/dma/at_xdmac.c @@ -308,6 +308,11 @@ static inline int at_xdmac_csize(u32 maxburst) return csize; }; +static inline bool at_xdmac_chan_is_peripheral_xfer(u32 cfg) +{ + return cfg & AT_XDMAC_CC_TYPE_PER_TRAN; +} + static inline u8 at_xdmac_get_dwidth(u32 cfg) { return (cfg & AT_XDMAC_CC_DWIDTH_MASK) >> AT_XDMAC_CC_DWIDTH_OFFSET; @@ -389,7 +394,13 @@ static void at_xdmac_start_xfer(struct at_xdmac_chan *atchan, at_xdmac_chan_read(atchan, AT_XDMAC_CUBC)); at_xdmac_chan_write(atchan, AT_XDMAC_CID, 0xffffffff); - reg = AT_XDMAC_CIE_RBEIE | AT_XDMAC_CIE_WBEIE | AT_XDMAC_CIE_ROIE; + reg = AT_XDMAC_CIE_RBEIE | AT_XDMAC_CIE_WBEIE; + /* + * Request Overflow Error is only for peripheral synchronized transfers + */ + if (at_xdmac_chan_is_peripheral_xfer(first->lld.mbr_cfg)) + reg |= AT_XDMAC_CIE_ROIE; + /* * There is no end of list when doing cyclic dma, we need to get * an interrupt after each periods.