RE: [PATCH V2 1/1] Fix a dead loop on dw shutdown

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> This is also related to
> https://bugzilla.kernel.org/show_bug.cgi?id=101271
> 
> LPSS DMA (looks like you are using Intel hardware) has power auto gating
> which can't be superseded in software. You are lucky that system doesn't
> hang completely.

It seems intel SOC had integrated the "power auto gating", but are you sure any 
Other SOCs had intergraded such feature? Like other ARM SOCs.

> 
> By the way, I think that ->shutdown() hook doesn't add a much value at all.

I don’t agree with it, it maybe ok on Intel SOCs, but this driver is for synopsys dmac ip,
So it should need the shutdown for other SOC vendors like ARM base vendors. We should 
Sync the dma before shutdown.

> On one hand the users have to care about being quiescent during shutdown,
> on the other there is nothing to prevent an abruption of execution.

In normal work flow, it maybe ok. But if the state machine in wrong state for some
corner case for this synopsys dmac controller, so current code will go into dead loop.

> 
> Vinod, we have really few DMA drivers that implement ->shutdown() hook.
> Do we really need it there and thus does is make sense to add to all DMA
> drivers?
> 
> P.S. The patch itself was once published here
> http://www.spinics.net/lists/dmaengine/msg03964.html. So, I also wonder
> how LPSS DMA power gating is working in the hardware and can we
> supersede the work flow in software.
> 
> > >
> > > Signed-off-by: Figo <tianfei.zhang@xxxxxxxxx>
> > > ---
> > >  drivers/dma/dw/core.c |    8 ++++++--
> > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c index
> > > bedce03..9c5f3f5 100644
> > > --- a/drivers/dma/dw/core.c
> > > +++ b/drivers/dma/dw/core.c
> > > @@ -1107,6 +1107,7 @@ static void dwc_issue_pending(struct
> dma_chan
> > > *chan)
> > >  static void dw_dma_off(struct dw_dma *dw)  {
> > >  	int i;
> > > +	int retry = 100;
> > >
> > >  	dma_writel(dw, CFG, 0);
> > >
> > > @@ -1115,8 +1116,11 @@ static void dw_dma_off(struct dw_dma *dw)
> > >  	channel_clear_bit(dw, MASK.DST_TRAN, dw->all_chan_mask);
> > >  	channel_clear_bit(dw, MASK.ERROR, dw->all_chan_mask);
> > >
> > > -	while (dma_readl(dw, CFG) & DW_CFG_DMA_EN)
> > > -		cpu_relax();
> > > +	while ((dma_readl(dw, CFG) & DW_CFG_DMA_EN) && --retry)
> > > +		udelay(100);
> > > +
> > > +	if (retry == 0)
> > > +		dev_err(&dw->dma.dev, "%s error :timeout\n",
> > > __func__);
> > >
> > >  	for (i = 0; i < dw->dma.chancnt; i++)
> > >  		dw->chan[i].initialized = false;
> >
> > Acked-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
> >
> 
> --
> Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Intel Finland Oy
��.n��������+%������w��{.n��������)�)��jg��������ݢj����G�������j:+v���w�m������w�������h�����٥




[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux PCI]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux