Re: [PATCH 1/1] dma: imx-sdma: add 1ms delay to ensure SDMA channel is stopped

[Jiada Wang]

Hello Vinod

On 02/13/2017 07:22 PM, Vinod Koul wrote:
> On Mon, Feb 13, 2017 at 03:30:19PM +0900, Jiada Wang wrote:
> > > > +static int sdma_disable_channel_with_delay(struct dma_chan *chan)
> > > > +{
> > > > +	sdma_disable_channel(chan);
> > > > +	mdelay(1);
> > >
> > > what is the gaurantee that 1ms is fine? Shouldn't you poll the bit to see
> > > channel is disabled properly..
> > I got the information from NXP (freescale) R&D team,
> > according to them, by write '1' to SDMA_H_STATSTOP, only disables
> > the related sdma channel (so poll HE bit will indicates the channel
> > has been disabled),
> > but it cannot ensure SDMA core stop to access modules' FIFO,
> > SDMA core may still is running, this is a bug in HW.
>
> Okay b ut you are not doing the HE bit here..??
by calling sdma_disable_channel(chan) here, sdma driver clears
corresponding HE bit, thus disables the channel. But it can't ensure
SDMA core is stopped by this operation.

> > regarding if the '1ms' is enough to ensure SDMA core has stopped,
> > NXP R&D team mentioned:
> > "we should add some delay of one BD SDMA cost time after disable the
> > channel bit, the maximum is 1ms"
> > so I assume 1ms should work for all cases
>
> At least please document this in changelog and comments in code.
I will update my patch to add comments once all concerns are addressed.

Thanks,
Jiada