[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4E2DDAEA.6050809@atmel.com>
Date: Mon, 25 Jul 2011 23:06:50 +0200
From: Nicolas Ferre <nicolas.ferre@...el.com>
To: Vinod Koul <vkoul@...radead.org>
CC: dan.j.williams@...el.com, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org,
Uwe Kleine-König
<u.kleine-koenig@...gutronix.de>
Subject: Re: [PATCH 2/3] dmaengine: at_hdmac: improve power management routines
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@...el.com>
>> Signed-off-by: Uwe Kleine-König<u.kleine-koenig@...gutronix.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.
>> + }
>> return 0;
>> }
>>
>> static const struct dev_pm_ops at_dma_dev_pm_ops = {
>> + .prepare = at_dma_prepare,
>> .suspend_noirq = at_dma_suspend_noirq,
>> .resume_noirq = at_dma_resume_noirq,
>> };
>> diff --git a/drivers/dma/at_hdmac_regs.h b/drivers/dma/at_hdmac_regs.h
>> index 087dbf1..6f0c4a3 100644
>> --- a/drivers/dma/at_hdmac_regs.h
>> +++ b/drivers/dma/at_hdmac_regs.h
>> @@ -204,6 +204,9 @@ enum atc_status {
>> * @status: transmit status information from irq/prep* functions
>> * to tasklet (use atomic operations)
>> * @tasklet: bottom half to finish transaction work
>> + * @save_cfg: configuration register that is saved on suspend/resume cycle
>> + * @save_dscr: for cyclic operations, preserve next descriptor address in
>> + * the cyclic list on suspend/resume cycle
>> * @lock: serializes enqueue/dequeue operations to descriptors lists
>> * @completed_cookie: identifier for the most recently completed operation
>> * @active_list: list of descriptors dmaengine is being running on
>> @@ -218,6 +221,8 @@ struct at_dma_chan {
>> u8 mask;
>> unsigned long status;
>> struct tasklet_struct tasklet;
>> + u32 save_cfg;
>> + u32 save_dscr;
>>
>> spinlock_t lock;
>>
>> @@ -248,6 +253,7 @@ static inline struct at_dma_chan *to_at_dma_chan(struct dma_chan *dchan)
>> * @chan_common: common dmaengine dma_device object members
>> * @ch_regs: memory mapped register base
>> * @clk: dma controller clock
>> + * @save_imr: interrupt mask register that is saved on suspend/resume cycle
>> * @all_chan_mask: all channels availlable in a mask
>> * @dma_desc_pool: base of DMA descriptor region (DMA address)
>> * @chan: channels table to store at_dma_chan structures
>> @@ -256,6 +262,7 @@ struct at_dma {
>> struct dma_device dma_common;
>> void __iomem *regs;
>> struct clk *clk;
>> + u32 save_imr;
>>
>> u8 all_chan_mask;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists