Re: [PATCH] dmaengine: at_xdmac: Fix wrongfull report of a channel as in use

On 21-01-19, 14:38, Codrin.Ciubotariu@microchip.com wrote:

On 20.01.2019 13:04, Vinod Koul wrote:

quoted

Hi Codrin,

On 17-01-19, 16:10, Codrin.Ciubotariu@microchip.com wrote:

quoted

From: Codrin Ciubotariu <codrin.ciubotariu@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.

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..

Hi Vinod,

This patch is the fix, since it moves the operations on atchan->status, 
in which the interrupt status register is saved, to a different member 
(irq_status). The AT_XDMAC_CHAN_IS_CYCLIC and AT_XDMAC_CHAN_IS_PAUSED 
bits have nothing in common with the interrupt status register.

The bug reproduces when a 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;
}

I can send v2 if you consider the commit message misleading.

Yes please, that would be better and pls add fixes as suggested by
Ludovic along with his ack

-- 
~Vinod

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel