Hi Codrin,
On 17-01-19, 16:10, Codrin.Ciubotariu@xxxxxxxxxxxxx wrote:
> From: Codrin Ciubotariu <codrin.ciubotariu@xxxxxxxxxxxxx>
>
> 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.
This looks reasonable but am unable to see how the bug is fixed. Perhaps
would be great to split the bug part (which can go to fixes) and remove
the reuse of variable as a subsequent patch..
>
> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@xxxxxxxxxxxxx>
> ---
> 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
--
~Vinod
