On Mon, 2015-10-12 at 10:04 +0000, Zhang, Tianfei wrote: > > > > 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. My point is to mention the bug report which is related to Intel hardware. Below I asked you why do you have such issue in the first place? > > > > > 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. I'm talking about DMA IP and as you noticed I asked Vinod as well to share his opinion about ->shutdown() hook. > > > 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. I'm not objecting this patch, but it fixes the symptoms (though that is okay since the unlimited busy loops is a bad idea and I already propose d a fix half a year ago), not the original issue. > > > > > 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 -- Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
