On Thu, Jan 17, 2019 at 05:10:38PM +0100, Codrin Ciubotariu - M19940 wrote: > From: Codrin Ciubotariu <codrin.ciubota...@microchip.com> > > atchan->status is used for two things: > - pass channel interrupts status from interrupt handler to tasklet; > - channel information like whether it is cyclic or paused; > > Since these operations have nothing in common, this patch adds a > different struct member to keep the interrupts status. > > Fixes a bug in which a channel is wrongfully reported as in use when > trying to obtain a new descriptior for a previously used cyclic channel. > > Signed-off-by: Codrin Ciubotariu <codrin.ciubota...@microchip.com> Acked-by: Ludovic Desroches <ludovic.desroc...@microchip.com>
As Vinod suggested, you may change the commit message to emphasize that the bug comes from the fact that a single variable is used to store different information. The Fixes: tag should be added too. Regards Ludovic > --- > 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 4e557684f792..fe69dccfa0c0 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(&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); > -- > 2.17.1 >