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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140916132259.GE12379@n2100.arm.linux.org.uk>
Date:	Tue, 16 Sep 2014 14:22:59 +0100
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Peter Ujfalusi <peter.ujfalusi@...com>
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 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);

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.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
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/