* [PATCH 1/2] dma: shdma: fix runtime PM: clear channel buffers on reset
2012-01-04 14:34 [PATCH 0/2] dma: shdma: clear buffers on reset Guennadi Liakhovetski
@ 2012-01-04 14:34 ` Guennadi Liakhovetski
2012-03-28 5:25 ` Paul Mundt
2012-01-04 14:34 ` [PATCH 2/2] ARM: mach-shmobile: specify CHCLR registers on SH7372 Guennadi Liakhovetski
2012-01-06 6:11 ` [PATCH 0/2] dma: shdma: clear buffers on reset Vinod Koul
2 siblings, 1 reply; 6+ messages in thread
From: Guennadi Liakhovetski @ 2012-01-04 14:34 UTC (permalink / raw)
To: linux-kernel
Cc: Vinod Koul, linux-sh, linux-pm, Rafael J. Wysocki, Magnus Damm
On platforms, supporting power domains, if the domain, containing a DMAC
instance is powered down, the driver fails to resume correctly. On those
platforms DMAC channels have an additional CHCLR register for clearing
channel buffers. Using this register during runtime resume fixes the
problem.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/dma/shdma.c | 47 +++++++++++++++++++++++++++++++----------------
include/linux/sh_dma.h | 2 ++
2 files changed, 33 insertions(+), 16 deletions(-)
diff --git a/drivers/dma/shdma.c b/drivers/dma/shdma.c
index 81809c2..e1a2fa6 100644
--- a/drivers/dma/shdma.c
+++ b/drivers/dma/shdma.c
@@ -57,6 +57,15 @@ static LIST_HEAD(sh_dmae_devices);
static unsigned long sh_dmae_slave_used[BITS_TO_LONGS(SH_DMA_SLAVE_NUMBER)];
static void sh_dmae_chan_ld_cleanup(struct sh_dmae_chan *sh_chan, bool all);
+static void sh_chan_xfer_ld_queue(struct sh_dmae_chan *sh_chan);
+
+static void chclr_write(struct sh_dmae_chan *sh_dc, u32 data)
+{
+ struct sh_dmae_device *shdev = to_sh_dev(sh_dc);
+
+ __raw_writel(data, shdev->chan_reg +
+ shdev->pdata->channel[sh_dc->id].chclr_offset);
+}
static void sh_dmae_writel(struct sh_dmae_chan *sh_dc, u32 data, u32 reg)
{
@@ -129,6 +138,15 @@ static int sh_dmae_rst(struct sh_dmae_device *shdev)
dmaor = dmaor_read(shdev) & ~(DMAOR_NMIF | DMAOR_AE | DMAOR_DME);
+ if (shdev->pdata->chclr_present) {
+ int i;
+ for (i = 0; i < shdev->pdata->channel_num; i++) {
+ struct sh_dmae_chan *sh_chan = shdev->chan[i];
+ if (sh_chan)
+ chclr_write(sh_chan, 0);
+ }
+ }
+
dmaor_write(shdev, dmaor | shdev->pdata->dmaor_init);
dmaor = dmaor_read(shdev);
@@ -139,6 +157,10 @@ static int sh_dmae_rst(struct sh_dmae_device *shdev)
dev_warn(shdev->common.dev, "Can't initialize DMAOR.\n");
return -EIO;
}
+ if (shdev->pdata->dmaor_init & ~dmaor)
+ dev_warn(shdev->common.dev,
+ "DMAOR=0x%x hasn't latched the initial value 0x%x.\n",
+ dmaor, shdev->pdata->dmaor_init);
return 0;
}
@@ -259,8 +281,6 @@ static int dmae_set_dmars(struct sh_dmae_chan *sh_chan, u16 val)
return 0;
}
-static void sh_chan_xfer_ld_queue(struct sh_dmae_chan *sh_chan);
-
static dma_cookie_t sh_dmae_tx_submit(struct dma_async_tx_descriptor *tx)
{
struct sh_desc *desc = tx_to_sh_desc(tx), *chunk, *last = desc, *c;
@@ -340,6 +360,8 @@ static dma_cookie_t sh_dmae_tx_submit(struct dma_async_tx_descriptor *tx)
sh_chan_xfer_ld_queue(sh_chan);
sh_chan->pm_state = DMAE_PM_ESTABLISHED;
}
+ } else {
+ sh_chan->pm_state = DMAE_PM_PENDING;
}
spin_unlock_irq(&sh_chan->desc_lock);
@@ -1225,6 +1247,8 @@ static int __init sh_dmae_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, shdev);
+ shdev->common.dev = &pdev->dev;
+
pm_runtime_enable(&pdev->dev);
pm_runtime_get_sync(&pdev->dev);
@@ -1254,7 +1278,6 @@ static int __init sh_dmae_probe(struct platform_device *pdev)
shdev->common.device_prep_slave_sg = sh_dmae_prep_slave_sg;
shdev->common.device_control = sh_dmae_control;
- shdev->common.dev = &pdev->dev;
/* Default transfer size of 32 bytes requires 32-byte alignment */
shdev->common.copy_align = LOG2_DEFAULT_XFER_SIZE;
@@ -1435,22 +1458,17 @@ static int sh_dmae_runtime_resume(struct device *dev)
#ifdef CONFIG_PM
static int sh_dmae_suspend(struct device *dev)
{
- struct sh_dmae_device *shdev = dev_get_drvdata(dev);
- int i;
-
- for (i = 0; i < shdev->pdata->channel_num; i++) {
- struct sh_dmae_chan *sh_chan = shdev->chan[i];
- if (sh_chan->descs_allocated)
- sh_chan->pm_error = pm_runtime_put_sync(dev);
- }
-
return 0;
}
static int sh_dmae_resume(struct device *dev)
{
struct sh_dmae_device *shdev = dev_get_drvdata(dev);
- int i;
+ int i, ret;
+
+ ret = sh_dmae_rst(shdev);
+ if (ret < 0)
+ dev_err(dev, "Failed to reset!\n");
for (i = 0; i < shdev->pdata->channel_num; i++) {
struct sh_dmae_chan *sh_chan = shdev->chan[i];
@@ -1459,9 +1477,6 @@ static int sh_dmae_resume(struct device *dev)
if (!sh_chan->descs_allocated)
continue;
- if (!sh_chan->pm_error)
- pm_runtime_get_sync(dev);
-
if (param) {
const struct sh_dmae_slave_config *cfg = param->config;
dmae_set_dmars(sh_chan, cfg->mid_rid);
diff --git a/include/linux/sh_dma.h b/include/linux/sh_dma.h
index cb2dd11..e11e171 100644
--- a/include/linux/sh_dma.h
+++ b/include/linux/sh_dma.h
@@ -48,6 +48,7 @@ struct sh_dmae_channel {
unsigned int offset;
unsigned int dmars;
unsigned int dmars_bit;
+ unsigned int chclr_offset;
};
struct sh_dmae_pdata {
@@ -68,6 +69,7 @@ struct sh_dmae_pdata {
unsigned int dmaor_is_32bit:1;
unsigned int needs_tend_set:1;
unsigned int no_dmars:1;
+ unsigned int chclr_present:1;
};
/* DMA register */
--
1.7.2.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] dma: shdma: fix runtime PM: clear channel buffers on reset
2012-01-04 14:34 ` [PATCH 1/2] dma: shdma: fix runtime PM: clear channel " Guennadi Liakhovetski
@ 2012-03-28 5:25 ` Paul Mundt
2012-03-28 7:02 ` Guennadi Liakhovetski
0 siblings, 1 reply; 6+ messages in thread
From: Paul Mundt @ 2012-03-28 5:25 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: linux-kernel, Vinod Koul, linux-sh, linux-pm, Rafael J. Wysocki,
Magnus Damm
On Wed, Jan 04, 2012 at 03:34:17PM +0100, Guennadi Liakhovetski wrote:
> On platforms, supporting power domains, if the domain, containing a DMAC
> instance is powered down, the driver fails to resume correctly. On those
> platforms DMAC channels have an additional CHCLR register for clearing
> channel buffers. Using this register during runtime resume fixes the
> problem.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
..
> @@ -139,6 +157,10 @@ static int sh_dmae_rst(struct sh_dmae_device *shdev)
> dev_warn(shdev->common.dev, "Can't initialize DMAOR.\n");
> return -EIO;
> }
> + if (shdev->pdata->dmaor_init & ~dmaor)
> + dev_warn(shdev->common.dev,
> + "DMAOR=0x%x hasn't latched the initial value 0x%x.\n",
> + dmaor, shdev->pdata->dmaor_init);
> return 0;
> }
>
I've just hit this on SH7786, which I suspect means that I've got it
wired up incorrectly somehow. This hunk also appears completely unrelated
to the changelog, so I'm wondering what exactly it's for. If this is a
probe attempt then presumably we should be handing down an error and
-ENODEV'ing in the init path? Is it expected that this is non-fatal in
the other reset cases?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] dma: shdma: fix runtime PM: clear channel buffers on reset
2012-03-28 5:25 ` Paul Mundt
@ 2012-03-28 7:02 ` Guennadi Liakhovetski
0 siblings, 0 replies; 6+ messages in thread
From: Guennadi Liakhovetski @ 2012-03-28 7:02 UTC (permalink / raw)
To: Paul Mundt
Cc: linux-kernel, Vinod Koul, linux-sh, linux-pm, Rafael J. Wysocki,
Magnus Damm
On Wed, 28 Mar 2012, Paul Mundt wrote:
> On Wed, Jan 04, 2012 at 03:34:17PM +0100, Guennadi Liakhovetski wrote:
> > On platforms, supporting power domains, if the domain, containing a DMAC
> > instance is powered down, the driver fails to resume correctly. On those
> > platforms DMAC channels have an additional CHCLR register for clearing
> > channel buffers. Using this register during runtime resume fixes the
> > problem.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
>
> ..
>
> > @@ -139,6 +157,10 @@ static int sh_dmae_rst(struct sh_dmae_device *shdev)
> > dev_warn(shdev->common.dev, "Can't initialize DMAOR.\n");
> > return -EIO;
> > }
> > + if (shdev->pdata->dmaor_init & ~dmaor)
> > + dev_warn(shdev->common.dev,
> > + "DMAOR=0x%x hasn't latched the initial value 0x%x.\n",
> > + dmaor, shdev->pdata->dmaor_init);
> > return 0;
> > }
> >
> I've just hit this on SH7786, which I suspect means that I've got it
> wired up incorrectly somehow. This hunk also appears completely unrelated
> to the changelog, so I'm wondering what exactly it's for. If this is a
> probe attempt then presumably we should be handing down an error and
> -ENODEV'ing in the init path? Is it expected that this is non-fatal in
> the other reset cases?
The answer is "I don't know." I'm also seeing this in several cases, e.g.,
on sh7372 one of the two USB DMA controllers always triggers this, IIRC.
This failure does indeed seem to be non-fatal, and it has been simply
ignored in the past. I just hit it in my tests and decided it was worth
drawing attention to it to maybe actually find out what's going on there.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] ARM: mach-shmobile: specify CHCLR registers on SH7372
2012-01-04 14:34 [PATCH 0/2] dma: shdma: clear buffers on reset Guennadi Liakhovetski
2012-01-04 14:34 ` [PATCH 1/2] dma: shdma: fix runtime PM: clear channel " Guennadi Liakhovetski
@ 2012-01-04 14:34 ` Guennadi Liakhovetski
2012-01-06 6:11 ` [PATCH 0/2] dma: shdma: clear buffers on reset Vinod Koul
2 siblings, 0 replies; 6+ messages in thread
From: Guennadi Liakhovetski @ 2012-01-04 14:34 UTC (permalink / raw)
To: linux-kernel
Cc: Vinod Koul, linux-sh, linux-pm, Rafael J. Wysocki, Magnus Damm
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
arch/arm/mach-shmobile/setup-sh7372.c | 15 ++++++++++++---
1 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-shmobile/setup-sh7372.c b/arch/arm/mach-shmobile/setup-sh7372.c
index 2380389..83dc940 100644
--- a/arch/arm/mach-shmobile/setup-sh7372.c
+++ b/arch/arm/mach-shmobile/setup-sh7372.c
@@ -445,31 +445,39 @@ static const struct sh_dmae_slave_config sh7372_dmae_slaves[] = {
},
};
+#define SH7372_CHCLR 0x220
+
static const struct sh_dmae_channel sh7372_dmae_channels[] = {
{
.offset = 0,
.dmars = 0,
.dmars_bit = 0,
+ .chclr_offset = SH7372_CHCLR + 0,
}, {
.offset = 0x10,
.dmars = 0,
.dmars_bit = 8,
+ .chclr_offset = SH7372_CHCLR + 0x10,
}, {
.offset = 0x20,
.dmars = 4,
.dmars_bit = 0,
+ .chclr_offset = SH7372_CHCLR + 0x20,
}, {
.offset = 0x30,
.dmars = 4,
.dmars_bit = 8,
+ .chclr_offset = SH7372_CHCLR + 0x30,
}, {
.offset = 0x50,
.dmars = 8,
.dmars_bit = 0,
+ .chclr_offset = SH7372_CHCLR + 0x50,
}, {
.offset = 0x60,
.dmars = 8,
.dmars_bit = 8,
+ .chclr_offset = SH7372_CHCLR + 0x60,
}
};
@@ -487,6 +495,7 @@ static struct sh_dmae_pdata dma_platform_data = {
.ts_shift = ts_shift,
.ts_shift_num = ARRAY_SIZE(ts_shift),
.dmaor_init = DMAOR_DME,
+ .chclr_present = 1,
};
/* Resource order important! */
@@ -494,7 +503,7 @@ static struct resource sh7372_dmae0_resources[] = {
{
/* Channel registers and DMAOR */
.start = 0xfe008020,
- .end = 0xfe00808f,
+ .end = 0xfe00828f,
.flags = IORESOURCE_MEM,
},
{
@@ -522,7 +531,7 @@ static struct resource sh7372_dmae1_resources[] = {
{
/* Channel registers and DMAOR */
.start = 0xfe018020,
- .end = 0xfe01808f,
+ .end = 0xfe01828f,
.flags = IORESOURCE_MEM,
},
{
@@ -550,7 +559,7 @@ static struct resource sh7372_dmae2_resources[] = {
{
/* Channel registers and DMAOR */
.start = 0xfe028020,
- .end = 0xfe02808f,
+ .end = 0xfe02828f,
.flags = IORESOURCE_MEM,
},
{
--
1.7.2.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/2] dma: shdma: clear buffers on reset
2012-01-04 14:34 [PATCH 0/2] dma: shdma: clear buffers on reset Guennadi Liakhovetski
2012-01-04 14:34 ` [PATCH 1/2] dma: shdma: fix runtime PM: clear channel " Guennadi Liakhovetski
2012-01-04 14:34 ` [PATCH 2/2] ARM: mach-shmobile: specify CHCLR registers on SH7372 Guennadi Liakhovetski
@ 2012-01-06 6:11 ` Vinod Koul
2 siblings, 0 replies; 6+ messages in thread
From: Vinod Koul @ 2012-01-06 6:11 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: linux-kernel, linux-sh, linux-pm, Rafael J. Wysocki, Magnus Damm
On Wed, 2012-01-04 at 15:34 +0100, Guennadi Liakhovetski wrote:
> This patch set fixes a runtime PM problem on shdma, whereby it fails to
> recover after the power domain has been switched off.
>
Both applied, thanks
--
~Vinod
^ permalink raw reply [flat|nested] 6+ messages in thread