| Message ID | 1425983107-20612-1-git-send-email-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Tue, Mar 10, 2015 at 12:25:07PM +0200, Andy Shevchenko wrote: > In some cases we might have DMA powered off and therefore get 0xffffffff from > the register. This patch introduces a counter to prevent a hang. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/dma/dw/core.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c > index a8ad052..3f514d6 100644 > --- a/drivers/dma/dw/core.c > +++ b/drivers/dma/dw/core.c > @@ -191,8 +191,10 @@ static inline void dwc_dump_chan_regs(struct dw_dma_chan *dwc) > > static inline void dwc_chan_disable(struct dw_dma *dw, struct dw_dma_chan *dwc) > { > + unsigned int count = 20; > + > channel_clear_bit(dw, CH_EN, dwc->mask); > - while (dma_readl(dw, CH_EN) & dwc->mask) > + while (dma_readl(dw, CH_EN) & dwc->mask && count--) you have a write instructions before this, so how is that write is not impacted and read is. Why would DMA be powered off?
On Mon, 2015-03-16 at 22:18 +0530, Vinod Koul wrote: > On Tue, Mar 10, 2015 at 12:25:07PM +0200, Andy Shevchenko wrote: > > In some cases we might have DMA powered off and therefore get 0xffffffff from > > the register. This patch introduces a counter to prevent a hang. > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > --- > > drivers/dma/dw/core.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c > > index a8ad052..3f514d6 100644 > > --- a/drivers/dma/dw/core.c > > +++ b/drivers/dma/dw/core.c > > @@ -191,8 +191,10 @@ static inline void dwc_dump_chan_regs(struct dw_dma_chan *dwc) > > > > static inline void dwc_chan_disable(struct dw_dma *dw, struct dw_dma_chan *dwc) > > { > > + unsigned int count = 20; > > + > > channel_clear_bit(dw, CH_EN, dwc->mask); > > - while (dma_readl(dw, CH_EN) & dwc->mask) > > + while (dma_readl(dw, CH_EN) & dwc->mask && count--) > you have a write instructions before this, so how is that write is not > impacted and read is. The write rather can be impacted, but we can't know this for sure (needs some probe on the interconnect / IOSF to see that). > Why would DMA be powered off? In our case DMA is a private IP to the host controllers (SPI, for example), So, the power rail is shared for SPI and DMA. SPI uses runtime PM and can be switched off. The actual issue we have is happened at dw_dma_off(). I decided to add an extra to dwc_chan_disable(). Give another thought I'm okay to fix just dw_dma_off() case.
Hi Andy, On Tue, Mar 10, 2015 at 12:25:07PM +0200, Andy Shevchenko wrote: > In some cases we might have DMA powered off and therefore get 0xffffffff from > the register. This patch introduces a counter to prevent a hang. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/dma/dw/core.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c > index a8ad052..3f514d6 100644 > --- a/drivers/dma/dw/core.c > +++ b/drivers/dma/dw/core.c > @@ -191,8 +191,10 @@ static inline void dwc_dump_chan_regs(struct dw_dma_chan *dwc) > > static inline void dwc_chan_disable(struct dw_dma *dw, struct dw_dma_chan *dwc) > { > + unsigned int count = 20; > + > channel_clear_bit(dw, CH_EN, dwc->mask); > - while (dma_readl(dw, CH_EN) & dwc->mask) > + while (dma_readl(dw, CH_EN) & dwc->mask && count--) It looks like a good case for the new readl_poll_timeout_* functions. Maxime
On Tue, 2015-03-17 at 15:13 +0100, Maxime Ripard wrote: > Hi Andy, > > On Tue, Mar 10, 2015 at 12:25:07PM +0200, Andy Shevchenko wrote: > > In some cases we might have DMA powered off and therefore get 0xffffffff from > > the register. This patch introduces a counter to prevent a hang. > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > --- > > drivers/dma/dw/core.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c > > index a8ad052..3f514d6 100644 > > --- a/drivers/dma/dw/core.c > > +++ b/drivers/dma/dw/core.c > > @@ -191,8 +191,10 @@ static inline void dwc_dump_chan_regs(struct dw_dma_chan *dwc) > > > > static inline void dwc_chan_disable(struct dw_dma *dw, struct dw_dma_chan *dwc) > > { > > + unsigned int count = 20; > > + > > channel_clear_bit(dw, CH_EN, dwc->mask); > > - while (dma_readl(dw, CH_EN) & dwc->mask) > > + while (dma_readl(dw, CH_EN) & dwc->mask && count--) > > It looks like a good case for the new readl_poll_timeout_* functions. I like the idea, but it is not a good case unfortunately. There are not endianess-aware. And I don't have time to implement the generic support of that.
On Tue, Mar 17, 2015 at 04:57:46PM +0200, Andy Shevchenko wrote: > On Tue, 2015-03-17 at 15:13 +0100, Maxime Ripard wrote: > > Hi Andy, > > > > On Tue, Mar 10, 2015 at 12:25:07PM +0200, Andy Shevchenko wrote: > > > In some cases we might have DMA powered off and therefore get 0xffffffff from > > > the register. This patch introduces a counter to prevent a hang. > > > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > --- > > > drivers/dma/dw/core.c | 7 +++++-- > > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c > > > index a8ad052..3f514d6 100644 > > > --- a/drivers/dma/dw/core.c > > > +++ b/drivers/dma/dw/core.c > > > @@ -191,8 +191,10 @@ static inline void dwc_dump_chan_regs(struct dw_dma_chan *dwc) > > > > > > static inline void dwc_chan_disable(struct dw_dma *dw, struct dw_dma_chan *dwc) > > > { > > > + unsigned int count = 20; > > > + > > > channel_clear_bit(dw, CH_EN, dwc->mask); > > > - while (dma_readl(dw, CH_EN) & dwc->mask) > > > + while (dma_readl(dw, CH_EN) & dwc->mask && count--) > > > > It looks like a good case for the new readl_poll_timeout_* functions. > > I like the idea, but it is not a good case unfortunately. There are not > endianess-aware. And I don't have time to implement the generic support > of that. Ok, nevermind then :) Maxime
diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c index a8ad052..3f514d6 100644 --- a/drivers/dma/dw/core.c +++ b/drivers/dma/dw/core.c @@ -191,8 +191,10 @@ static inline void dwc_dump_chan_regs(struct dw_dma_chan *dwc) static inline void dwc_chan_disable(struct dw_dma *dw, struct dw_dma_chan *dwc) { + unsigned int count = 20; + channel_clear_bit(dw, CH_EN, dwc->mask); - while (dma_readl(dw, CH_EN) & dwc->mask) + while (dma_readl(dw, CH_EN) & dwc->mask && count--) cpu_relax(); } @@ -1109,6 +1111,7 @@ static void dwc_issue_pending(struct dma_chan *chan) static void dw_dma_off(struct dw_dma *dw) { + unsigned int count = 20; int i; dma_writel(dw, CFG, 0); @@ -1118,7 +1121,7 @@ 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) + while (dma_readl(dw, CFG) & DW_CFG_DMA_EN && count--) cpu_relax(); for (i = 0; i < dw->dma.chancnt; i++)
In some cases we might have DMA powered off and therefore get 0xffffffff from the register. This patch introduces a counter to prevent a hang. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/dma/dw/core.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)