linux-kernel - Re: [PATCH] dmaengine: omap-dma: Fix cyclic suspend/resume

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <54183D49.2070804@ti.com>
Date:	Tue, 16 Sep 2014 16:38:17 +0300
From:	Peter Ujfalusi <peter.ujfalusi@...com>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
CC:	<vinod.koul@...el.com>, <dan.j.williams@...el.com>,
	<dmaengine@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-omap@...r.kernel.org>
Subject: Re: [PATCH] dmaengine: omap-dma: Fix cyclic suspend/resume

On 09/16/2014 04:22 PM, Russell King - ARM Linux wrote:
> On Tue, Sep 16, 2014 at 04:06:25PM +0300, Peter Ujfalusi wrote:
>> When the audio stream is paused or suspended we stop the sDMA and when it
>> is unpsues/resumed we start the channel without reconfiguring it.
>> The omap_dma_stop() clears the link configuration when we pause the dma, but
>> it is not setting it back on start. This will result only one audio buffer
>> to be played back and the DMA will stop, since the linking is disabled.
>> The link need to be enabled in omap_dma_start() to make sure that cyclic
>> transfer can continue.
>>
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@...com>
>> ---
>>  drivers/dma/omap-dma.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
>> index 4cf7d9a950d7..13a02ff87f28 100644
>> --- a/drivers/dma/omap-dma.c
>> +++ b/drivers/dma/omap-dma.c
>> @@ -280,6 +280,7 @@ static void omap_dma_assign(struct omap_dmadev *od, struct omap_chan *c,
>>  static void omap_dma_start(struct omap_chan *c, struct omap_desc *d)
>>  {
>>  	struct omap_dmadev *od = to_omap_dma_dev(c->vc.chan.device);
>> +	uint32_t val;
>>  
>>  	if (__dma_omap15xx(od->plat->dma_attr))
>>  		omap_dma_chan_write(c, CPC, 0);
>> @@ -288,6 +289,17 @@ static void omap_dma_start(struct omap_chan *c, struct omap_desc *d)
>>  
>>  	omap_dma_clear_csr(c);
>>  
>> +	if (!__dma_omap15xx(od->plat->dma_attr) && c->cyclic) {
>> +		val = omap_dma_chan_read(c, CLNK_CTRL);
>> +
>> +		if (dma_omap1())
>> +			val &= ~(1 << 14); /* clear the STOP_LNK bit */
>> +		else
>> +			val |= CLNK_CTRL_ENABLE_LNK;
>> +
>> +		omap_dma_chan_write(c, CLNK_CTRL, val);
>> +	}
>> +
> 
> Why is this soo complicated?  What's wrong with simply writing the
> stored value back from the omap_desc:
> 
> 	omap_dma_chan_write(c, CLNK_CTRL, d->clnk_ctrl);

Oh, I have overlooked this. Thanks.

> In fact, rather than loading up the above fast path with stuff which
> it really doesn't need, why not do this in the resume path?  The
> other thing which should be placed in the resume path is a mb() call,
> since calling omap_dma_start() won't have a barrier in that path.

Do you want it as a separate patch (adding the mb() and to restore the
CLNK_CTRL register) or is it OK if I send it as one patch?

-- 
Péter
--
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/