[PATCH 00/12] dma: various minor clean ups for slave drivers

[PATCH 00/12] dma: various minor clean ups for slave drivers

[Andy Shevchenko]

Here is a set of small independent patches that clean up or fix minor things
across DMA slave drivers.

Andy Shevchenko (12):
  imx-sdma: remove useless variable
  mxs-dma: remove useless variable
  edma: no need to assign residue to 0 explicitly
  ep93xx_dma: remove useless use of lock
  fsldma: remove useless use of lock
  mmp_pdma: remove useless use of lock
  mpc512x_dma: remove useless use of lock
  pch_dma: remove useless use of lock
  tegra20-apb-dma: remove useless use of lock
  ipu_idmac: re-use dma_cookie_status()
  mmp_tdma: set cookies as well when asked for tx status
  txx9dmac: return DMA_SUCCESS immediately from device_tx_status()

 drivers/dma/edma.c            |  2 --
 drivers/dma/ep93xx_dma.c      | 10 +---------
 drivers/dma/fsldma.c          | 10 +---------
 drivers/dma/imx-sdma.c        |  9 +++------
 drivers/dma/ipu/ipu_idmac.c   |  5 +----
 drivers/dma/mmp_pdma.c        | 10 +---------
 drivers/dma/mmp_tdma.c        |  3 ++-
 drivers/dma/mpc512x_dma.c     | 10 +---------
 drivers/dma/mxs-dma.c         |  4 +---
 drivers/dma/pch_dma.c         |  9 +--------
 drivers/dma/tegra20-apb-dma.c |  8 +++-----
 drivers/dma/txx9dmac.c        | 13 ++++++-------
 12 files changed, 21 insertions(+), 72 deletions(-)

-- 
1.8.2.rc0.22.gb3600c3

[PATCH 09/12] tegra20-apb-dma: remove useless use of lock

[Andy Shevchenko]

Accordingly to dma_cookie_status() description locking is not required.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: linux-tegra@vger.kernel.org
---
 drivers/dma/tegra20-apb-dma.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
index 33f59ec..019ccfa 100644
--- a/drivers/dma/tegra20-apb-dma.c
+++ b/drivers/dma/tegra20-apb-dma.c
@@ -767,13 +767,11 @@ static enum dma_status tegra_dma_tx_status(struct dma_chan *dc,
 	unsigned long flags;
 	unsigned int residual;
 
-	spin_lock_irqsave(&tdc->lock, flags);
-
 	ret = dma_cookie_status(dc, cookie, txstate);
-	if (ret == DMA_SUCCESS) {
-		spin_unlock_irqrestore(&tdc->lock, flags);
+	if (ret == DMA_SUCCESS)
 		return ret;
-	}
+
+	spin_lock_irqsave(&tdc->lock, flags);
 
 	/* Check on wait_ack desc status */
 	list_for_each_entry(dma_desc, &tdc->free_dma_desc, node) {
-- 
1.8.2.rc0.22.gb3600c3

Re: [PATCH 09/12] tegra20-apb-dma: remove useless use of lock

[Laxman Dewangan]

On Monday 27 May 2013 05:44 PM, Andy Shevchenko wrote:
> Accordingly to dma_cookie_status() description locking is not required.
>
I think we need lock here:
 From isr handler, we call dma_cookie_complete() which is in 
spin-locked. This function updates  tx->chan->completed_cookie = tx->cookie;
In tegra_dma_tx_status(), we check for dma_cookie_status() which access 
the chan->completed_cookie; and it decides status based on this

As the access of  chan->completed_cookie  are from different context, we 
need this locking.

But did not get why it is documented as locking is not require if shared 
variable is getting changed/access from different context simultaneously.

Re: [PATCH 09/12] tegra20-apb-dma: remove useless use of lock

[Andy Shevchenko]

On Wed, May 29, 2013 at 1:56 PM, Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> On Monday 27 May 2013 05:44 PM, Andy Shevchenko wrote:
>>
>> Accordingly to dma_cookie_status() description locking is not required.
>>
> I think we need lock here:
> From isr handler, we call dma_cookie_complete() which is in spin-locked.
> This function updates  tx->chan->completed_cookie = tx->cookie;
> In tegra_dma_tx_status(), we check for dma_cookie_status() which access the
> chan->completed_cookie; and it decides status based on this
>
> As the access of  chan->completed_cookie  are from different context, we
> need this locking.

You need to have a consistent data in the cookies. This is guaranteed
by memory barrier if I got it correctly.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 00/12] dma: various minor clean ups for slave drivers

[Vinod Koul]

On Mon, May 27, 2013 at 03:14:30PM +0300, Andy Shevchenko wrote:
> Here is a set of small independent patches that clean up or fix minor things
> across DMA slave drivers.
The series looks fine. I am going to wait a day more and apply, pls speak up if
you disagree and ack if you agree

--
~Vinod
> 
> Andy Shevchenko (12):
>   imx-sdma: remove useless variable
>   mxs-dma: remove useless variable
>   edma: no need to assign residue to 0 explicitly
>   ep93xx_dma: remove useless use of lock
>   fsldma: remove useless use of lock
>   mmp_pdma: remove useless use of lock
>   mpc512x_dma: remove useless use of lock
>   pch_dma: remove useless use of lock
>   tegra20-apb-dma: remove useless use of lock
>   ipu_idmac: re-use dma_cookie_status()
>   mmp_tdma: set cookies as well when asked for tx status
>   txx9dmac: return DMA_SUCCESS immediately from device_tx_status()
> 
>  drivers/dma/edma.c            |  2 --
>  drivers/dma/ep93xx_dma.c      | 10 +---------
>  drivers/dma/fsldma.c          | 10 +---------
>  drivers/dma/imx-sdma.c        |  9 +++------
>  drivers/dma/ipu/ipu_idmac.c   |  5 +----
>  drivers/dma/mmp_pdma.c        | 10 +---------
>  drivers/dma/mmp_tdma.c        |  3 ++-
>  drivers/dma/mpc512x_dma.c     | 10 +---------
>  drivers/dma/mxs-dma.c         |  4 +---
>  drivers/dma/pch_dma.c         |  9 +--------
>  drivers/dma/tegra20-apb-dma.c |  8 +++-----
>  drivers/dma/txx9dmac.c        | 13 ++++++-------
>  12 files changed, 21 insertions(+), 72 deletions(-)
> 
> -- 
> 1.8.2.rc0.22.gb3600c3
> 

-- 

Re: [PATCH 00/12] dma: various minor clean ups for slave drivers

[Andy Shevchenko]

On Thu, May 30, 2013 at 8:47 PM, Vinod Koul <vinod.koul-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> On Mon, May 27, 2013 at 03:14:30PM +0300, Andy Shevchenko wrote:
>> Here is a set of small independent patches that clean up or fix minor things
>> across DMA slave drivers.
> The series looks fine. I am going to wait a day more and apply, pls speak up if
> you disagree and ack if you agree

I'm not in hurry with it. Please, take your time and do whatever it requires.
Thank you!

--
With Best Regards,
Andy Shevchenko

Re: [PATCH 00/12] dma: various minor clean ups for slave drivers

[Dan Williams]

On Thu, May 30, 2013 at 10:47 AM, Vinod Koul <vinod.koul-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> On Mon, May 27, 2013 at 03:14:30PM +0300, Andy Shevchenko wrote:
>> Here is a set of small independent patches that clean up or fix minor things
>> across DMA slave drivers.
> The series looks fine. I am going to wait a day more and apply, pls speak up if
> you disagree and ack if you agree

Looks ok to me.  Reminds we can probably take this one step further
and provide a generic implementation for the common case.  It's just a
bit inconsistent though that some engines will poll the completion
handler (try to advance the state of the last completed cookie)
whereas others just assume things will be completed asynchronously.

--
Dan

Re: [PATCH 00/12] dma: various minor clean ups for slave drivers

[Andy Shevchenko]

On Thu, 2013-05-30 at 23:17 +0530, Vinod Koul wrote: 
> On Mon, May 27, 2013 at 03:14:30PM +0300, Andy Shevchenko wrote:
> > Here is a set of small independent patches that clean up or fix minor things
> > across DMA slave drivers.
> The series looks fine. I am going to wait a day more and apply, pls speak up if
> you disagree and ack if you agree

Kindly remind about this series.

> 
> --
> ~Vinod
> > 
> > Andy Shevchenko (12):
> >   imx-sdma: remove useless variable
> >   mxs-dma: remove useless variable
> >   edma: no need to assign residue to 0 explicitly
> >   ep93xx_dma: remove useless use of lock
> >   fsldma: remove useless use of lock
> >   mmp_pdma: remove useless use of lock
> >   mpc512x_dma: remove useless use of lock
> >   pch_dma: remove useless use of lock
> >   tegra20-apb-dma: remove useless use of lock
> >   ipu_idmac: re-use dma_cookie_status()
> >   mmp_tdma: set cookies as well when asked for tx status
> >   txx9dmac: return DMA_SUCCESS immediately from device_tx_status()
> > 
> >  drivers/dma/edma.c            |  2 --
> >  drivers/dma/ep93xx_dma.c      | 10 +---------
> >  drivers/dma/fsldma.c          | 10 +---------
> >  drivers/dma/imx-sdma.c        |  9 +++------
> >  drivers/dma/ipu/ipu_idmac.c   |  5 +----
> >  drivers/dma/mmp_pdma.c        | 10 +---------
> >  drivers/dma/mmp_tdma.c        |  3 ++-
> >  drivers/dma/mpc512x_dma.c     | 10 +---------
> >  drivers/dma/mxs-dma.c         |  4 +---
> >  drivers/dma/pch_dma.c         |  9 +--------
> >  drivers/dma/tegra20-apb-dma.c |  8 +++-----
> >  drivers/dma/txx9dmac.c        | 13 ++++++-------
> >  12 files changed, 21 insertions(+), 72 deletions(-)
> > 
> > -- 
> > 1.8.2.rc0.22.gb3600c3
> > 
> 

-- 
Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Intel Finland Oy

Re: [PATCH 00/12] dma: various minor clean ups for slave drivers

[Vinod Koul]

On Fri, May 31, 2013 at 05:09:51PM -0700, Dan Williams wrote:
> On Thu, May 30, 2013 at 10:47 AM, Vinod Koul <vinod.koul-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> > On Mon, May 27, 2013 at 03:14:30PM +0300, Andy Shevchenko wrote:
> >> Here is a set of small independent patches that clean up or fix minor things
> >> across DMA slave drivers.
> > The series looks fine. I am going to wait a day more and apply, pls speak up if
> > you disagree and ack if you agree
> 
> Looks ok to me.  Reminds we can probably take this one step further
> and provide a generic implementation for the common case.  It's just a
> bit inconsistent though that some engines will poll the completion
> handler (try to advance the state of the last completed cookie)
> whereas others just assume things will be completed asynchronously.
agree with that. These are the inconsistencies some of which are based on
hardware and some are user iterpretations...

--
~Vinod

Re: [PATCH 00/12] dma: various minor clean ups for slave drivers

[Vinod Koul]

On Thu, May 30, 2013 at 09:32:19PM +0300, Andy Shevchenko wrote:
> >> Here is a set of small independent patches that clean up or fix minor things
> >> across DMA slave drivers.

Applied thanks

--
~Vinod

Re: [PATCH 00/12] dma: various minor clean ups for slave drivers

[Vinod Koul]

On Mon, Jul 15, 2013 at 01:21:17PM +0300, Andy Shevchenko wrote:
> On Mon, 2013-07-15 at 15:07 +0530, Vinod Koul wrote: 
> > On Thu, May 30, 2013 at 09:32:19PM +0300, Andy Shevchenko wrote:
> > > >> Here is a set of small independent patches that clean up or fix minor things
> > > >> across DMA slave drivers.
> > 
> > Applied thanks
> 
> Thank you. You were faster than me, I was just about to send rebased
> version.
:)

I suspected changes are trivial and it should apply or with slight modfications.
it did apply cleanly so no reason to delay applying on shiny new -rc1

For folks in this part of world, monday morning Linus drops the rc1 usually, so
get staright to it!

--
~Vinod

Re: [PATCH 00/12] dma: various minor clean ups for slave drivers

[Andy Shevchenko]

On Mon, 2013-07-15 at 15:07 +0530, Vinod Koul wrote: 
> On Thu, May 30, 2013 at 09:32:19PM +0300, Andy Shevchenko wrote:
> > >> Here is a set of small independent patches that clean up or fix minor things
> > >> across DMA slave drivers.
> 
> Applied thanks

Thank you. You were faster than me, I was just about to send rebased
version.

-- 
Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Intel Finland Oy