From: Vinod Koul <vkoul@infradead.org>
To: Nicolas Ferre <nicolas.ferre@atmel.com>
Cc: vinod.koul@intel.com, dan.j.williams@intel.com,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org,
"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Subject: Re: [PATCH 2/3] dmaengine: at_hdmac: improve power management routines
Date: Tue, 26 Jul 2011 16:21:54 +0530 [thread overview]
Message-ID: <1311677514.24316.6.camel@vkoul-mobl4> (raw)
In-Reply-To: <4E2DDAEA.6050809@atmel.com>
On Mon, 2011-07-25 at 23:06 +0200, Nicolas Ferre wrote:
> On 07/07/2011 04:20 AM, Vinod Koul wrote:
> > On Tue, 2011-06-28 at 13:17 +0200, Nicolas Ferre wrote:
> >> Save/restore dma controller state across a suspend-resume sequence.
> >> The prepare() function will wait for the non-cyclic channels to become idle.
> >> It also deals with cyclic operations with the start at next period while
> >> resuming.
> >>
> >> Signed-off-by: Nicolas Ferre<nicolas.ferre@atmel.com>
> >> Signed-off-by: Uwe Kleine-König<u.kleine-koenig@pengutronix.de>
> >> ---
> >> drivers/dma/at_hdmac.c | 88 ++++++++++++++++++++++++++++++++++++++++++-
> >> drivers/dma/at_hdmac_regs.h | 7 +++
> >> 2 files changed, 94 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
> >> index fd87b96..7096adb 100644
> >> --- a/drivers/dma/at_hdmac.c
> >> +++ b/drivers/dma/at_hdmac.c
> >> @@ -1385,27 +1385,113 @@ static void at_dma_shutdown(struct platform_device *pdev)
> >> clk_disable(atdma->clk);
> >> }
> >>
> >> +static int at_dma_prepare(struct device *dev)
> >> +{
> >> + struct platform_device *pdev = to_platform_device(dev);
> >> + struct at_dma *atdma = platform_get_drvdata(pdev);
> >> + struct dma_chan *chan, *_chan;
> >> +
> >> + list_for_each_entry_safe(chan, _chan,&atdma->dma_common.channels,
> >> + device_node) {
> >> + struct at_dma_chan *atchan = to_at_dma_chan(chan);
> >> + /* wait for transaction completion (except in cyclic case) */
> >> + if (atc_chan_is_enabled(atchan)&&
> >> + !test_bit(ATC_IS_CYCLIC,&atchan->status))
> >> + return -EAGAIN;
> > pls fix indent here
>
> Fixed by replacement of test_bit() with a wrapper in a patch to come (as
> you suggest hereafter).
>
> >> + }
> >> + return 0;
> >> +}
> >> +
> >> +static void atc_suspend_cyclic(struct at_dma_chan *atchan)
> >> +{
> >> + struct dma_chan *chan =&atchan->chan_common;
> >> +
> >> + /* Channel should be paused by user
> >> + * do it anyway even if it is not done already */
> >> + if (!test_bit(ATC_IS_PAUSED,&atchan->status)) {
> >> + dev_warn(chan2dev(chan),
> >> + "cyclic channel not paused, should be done by channel user\n");
> >> + atc_control(chan, DMA_PAUSE, 0);
> >> + }
> >> +
> >> + /* now preserve additional data for cyclic operations */
> >> + /* next descriptor address in the cyclic list */
> >> + atchan->save_dscr = channel_readl(atchan, DSCR);
> >> +
> >> + vdbg_dump_regs(atchan);
> >> +}
> >> +
> >> static int at_dma_suspend_noirq(struct device *dev)
> >> {
> >> struct platform_device *pdev = to_platform_device(dev);
> >> struct at_dma *atdma = platform_get_drvdata(pdev);
> >> + struct dma_chan *chan, *_chan;
> >>
> >> - at_dma_off(platform_get_drvdata(pdev));
> >> + /* preserve data */
> >> + list_for_each_entry_safe(chan, _chan,&atdma->dma_common.channels,
> >> + device_node) {
> >> + struct at_dma_chan *atchan = to_at_dma_chan(chan);
> >> +
> >> + if (test_bit(ATC_IS_CYCLIC,&atchan->status))
> >> + atc_suspend_cyclic(atchan);
> >> + atchan->save_cfg = channel_readl(atchan, CFG);
> >> + }
> >> + atdma->save_imr = dma_readl(atdma, EBCIMR);
> >> +
> >> + /* disable DMA controller */
> >> + at_dma_off(atdma);
> >> clk_disable(atdma->clk);
> >> return 0;
> >> }
> >>
> >> +static void atc_resume_cyclic(struct at_dma_chan *atchan)
> >> +{
> >> + struct at_dma *atdma = to_at_dma(atchan->chan_common.device);
> >> +
> >> + /* restore channel status for cyclic descriptors list:
> >> + * next descriptor in the cyclic list at the time of suspend */
> >> + channel_writel(atchan, SADDR, 0);
> >> + channel_writel(atchan, DADDR, 0);
> >> + channel_writel(atchan, CTRLA, 0);
> >> + channel_writel(atchan, CTRLB, 0);
> >> + channel_writel(atchan, DSCR, atchan->save_dscr);
> >> + dma_writel(atdma, CHER, atchan->mask);
> >> +
> >> + /* channel pause status should be removed by channel user
> >> + * We cannot take the initiative to do it here */
> >> +
> >> + vdbg_dump_regs(atchan);
> >> +}
> >> +
> >> static int at_dma_resume_noirq(struct device *dev)
> >> {
> >> struct platform_device *pdev = to_platform_device(dev);
> >> struct at_dma *atdma = platform_get_drvdata(pdev);
> >> + struct dma_chan *chan, *_chan;
> >>
> >> + /* bring back DMA controller */
> >> clk_enable(atdma->clk);
> >> dma_writel(atdma, EN, AT_DMA_ENABLE);
> >> +
> >> + /* clear any pending interrupt */
> >> + while (dma_readl(atdma, EBCISR))
> >> + cpu_relax();
> >> +
> >> + /* restore saved data */
> >> + dma_writel(atdma, EBCIER, atdma->save_imr);
> >> + list_for_each_entry_safe(chan, _chan,&atdma->dma_common.channels,
> >> + device_node) {
> >> + struct at_dma_chan *atchan = to_at_dma_chan(chan);
> >> +
> >> + channel_writel(atchan, CFG, atchan->save_cfg);
> >> + if (test_bit(ATC_IS_CYCLIC,&atchan->status))
> >> + atc_resume_cyclic(atchan);
> > This testing on bits seems to be reused few times how about wrapping it
> > up in a routine?
>
> True: I write a little patch for this and the "PAUSE" state. I send a
> patch for this now:
> dmaengine: at_hdmac: add wrappers for testing channel state
> So can you:
> 1/ queue 1/3 and 2/3 of this patch series
> 2/ queue the following patch named
> "dmaengine: at_hdmac: add wrappers for testing channel state"
> on top of that
>
> 3/ drop the patch 3/3 of this series: it certainly have to be reworked
> with all slave config infrastructure implementation in the at_hdmac driver.
The indent needs to be fixed before I can apply this one, sorry but
error introduced in one patch cannot be fixed in next, it is not
supposed to work that way
--
~Vinod Koul
Intel Corp.
next prev parent reply other threads:[~2011-07-26 13:49 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-28 11:17 [PATCH 1/3] dmaengine: at_hdmac: replace spin_lock* with irqsave variants Nicolas Ferre
2011-06-28 10:15 ` Nicolas Ferre
2011-06-28 11:17 ` [PATCH 2/3] dmaengine: at_hdmac: improve power management routines Nicolas Ferre
2011-07-07 2:20 ` Vinod Koul
2011-07-25 21:06 ` Nicolas Ferre
2011-07-26 10:51 ` Vinod Koul [this message]
2011-07-27 12:21 ` [PATCH V2 1/3] dmaengine: at_hdmac: replace spin_lock* with irqsave variants Nicolas Ferre
2011-07-25 21:09 ` [PATCH] dmaengine: at_hdmac: add wrappers for testing channel state Nicolas Ferre
2011-08-01 14:18 ` [PATCH V2 3/3] " Nicolas Ferre
2011-07-27 12:21 ` [PATCH V2 2/3] dmaengine: at_hdmac: improve power management routines Nicolas Ferre
2011-06-28 11:17 ` [PATCH 3/3] dmaengine: at_hdmac: add slave config operation Nicolas Ferre
2011-07-07 2:33 ` Vinod Koul
2011-07-25 21:12 ` Nicolas Ferre
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1311677514.24316.6.camel@vkoul-mobl4 \
--to=vkoul@infradead.org \
--cc=dan.j.williams@intel.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nicolas.ferre@atmel.com \
--cc=u.kleine-koenig@pengutronix.de \
--cc=vinod.koul@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).