2012-05-23 11:27:59

by Tushar Behera

Subject: [PATCH] dmaengine: pl330: dont complete descriptor for cyclic dma

Commit eab215855803 ("dmaengine: pl330: dont complete descriptor for
cyclic dma") wrongly completes descriptor for cyclic dma, hence following
BUG_ON is still hit with cyclic DMA operations.

kernel BUG at drivers/dma/dmaengine.h:53!

Signed-off-by: Tushar Behera <[email protected]>
---
drivers/dma/pl330.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index fa3fb21..8c44f17 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -2322,7 +2322,7 @@ static void pl330_tasklet(unsigned long data)
/* Pick up ripe tomatoes */
list_for_each_entry_safe(desc, _dt, &pch->work_list, node)
if (desc->status == DONE) {
- if (pch->cyclic)
+ if (!pch->cyclic)
dma_cookie_complete(&desc->txd);
list_move_tail(&desc->node, &list);
}
--
1.7.4.1

2012-05-24 04:24:57

by Tushar Behera

Subject: Re: [PATCH] dmaengine: pl330: dont complete descriptor for cyclic dma

Forgot to add driver author in the original mail, adding now.

On 05/23/2012 04:47 PM, Tushar Behera wrote:
> Commit eab215855803 ("dmaengine: pl330: dont complete descriptor for
> cyclic dma") wrongly completes descriptor for cyclic dma, hence following
> BUG_ON is still hit with cyclic DMA operations.
>
> kernel BUG at drivers/dma/dmaengine.h:53!
>
> Signed-off-by: Tushar Behera <[email protected]>
> ---
> drivers/dma/pl330.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index fa3fb21..8c44f17 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -2322,7 +2322,7 @@ static void pl330_tasklet(unsigned long data)
> /* Pick up ripe tomatoes */
> list_for_each_entry_safe(desc, _dt, &pch->work_list, node)
> if (desc->status == DONE) {
> - if (pch->cyclic)
> + if (!pch->cyclic)
> dma_cookie_complete(&desc->txd);
> list_move_tail(&desc->node, &list);
> }


--
Tushar Behera

2012-05-24 05:04:13

by Jassi Brar

Subject: Re: [PATCH] dmaengine: pl330: dont complete descriptor for cyclic dma

On 24 May 2012 09:54, Tushar Behera <[email protected]> wrote:
> Forgot to add driver author in the original mail, adding now.
>
> On 05/23/2012 04:47 PM, Tushar Behera wrote:
>> Commit eab215855803 ("dmaengine: pl330: dont complete descriptor for
>> cyclic dma") wrongly completes descriptor for cyclic dma, hence following
>> BUG_ON is still hit with cyclic DMA operations.
>>
>> kernel BUG at drivers/dma/dmaengine.h:53!
>>
>> Signed-off-by: Tushar Behera <[email protected]>
>> ---
>> ?drivers/dma/pl330.c | ? ?2 +-
>> ?1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
>> index fa3fb21..8c44f17 100644
>> --- a/drivers/dma/pl330.c
>> +++ b/drivers/dma/pl330.c
>> @@ -2322,7 +2322,7 @@ static void pl330_tasklet(unsigned long data)
>> ? ? ? /* Pick up ripe tomatoes */
>> ? ? ? list_for_each_entry_safe(desc, _dt, &pch->work_list, node)
>> ? ? ? ? ? ? ? if (desc->status == DONE) {
>> - ? ? ? ? ? ? ? ? ? ? if (pch->cyclic)
>> + ? ? ? ? ? ? ? ? ? ? if (!pch->cyclic)
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? dma_cookie_complete(&desc->txd);
>> ? ? ? ? ? ? ? ? ? ? ? list_move_tail(&desc->node, &list);
>> ? ? ? ? ? ? ? }
>
Acked-by: Jassi Brar <[email protected]>

Obviously a typo, but the commit eab2158 had a tested-by, so I hope
this doesn't break something else.

2012-05-25 08:02:52

by Vinod Koul

Subject: Re: [PATCH] dmaengine: pl330: dont complete descriptor for cyclic dma

On Thu, 2012-05-24 at 10:34 +0530, Jassi Brar wrote:
> On 24 May 2012 09:54, Tushar Behera <[email protected]> wrote:
> > Forgot to add driver author in the original mail, adding now.
> >
> > On 05/23/2012 04:47 PM, Tushar Behera wrote:
> >> Commit eab215855803 ("dmaengine: pl330: dont complete descriptor for
> >> cyclic dma") wrongly completes descriptor for cyclic dma, hence following
> >> BUG_ON is still hit with cyclic DMA operations.
> >>
> >> kernel BUG at drivers/dma/dmaengine.h:53!
> >>
> >> Signed-off-by: Tushar Behera <[email protected]>
> >> ---
> >> drivers/dma/pl330.c | 2 +-
> >> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> >> index fa3fb21..8c44f17 100644
> >> --- a/drivers/dma/pl330.c
> >> +++ b/drivers/dma/pl330.c
> >> @@ -2322,7 +2322,7 @@ static void pl330_tasklet(unsigned long data)
> >> /* Pick up ripe tomatoes */
> >> list_for_each_entry_safe(desc, _dt, &pch->work_list, node)
> >> if (desc->status == DONE) {
> >> - if (pch->cyclic)
> >> + if (!pch->cyclic)
> >> dma_cookie_complete(&desc->txd);
> >> list_move_tail(&desc->node, &list);
> >> }
> >
> Acked-by: Jassi Brar <[email protected]>
>
> Obviously a typo, but the commit eab2158 had a tested-by, so I hope
> this doesn't break something else.
Precisely my worry as well. Thomas?

>From the code this looks as the right one.

--
~Vinod

2012-05-25 08:31:35

by Thomas Abraham

Subject: Re: [PATCH] dmaengine: pl330: dont complete descriptor for cyclic dma

On 25 May 2012 13:26, Vinod Koul <[email protected]> wrote:
> On Thu, 2012-05-24 at 10:34 +0530, Jassi Brar wrote:
>> On 24 May 2012 09:54, Tushar Behera <[email protected]> wrote:
>> > Forgot to add driver author in the original mail, adding now.
>> >
>> > On 05/23/2012 04:47 PM, Tushar Behera wrote:
>> >> Commit eab215855803 ("dmaengine: pl330: dont complete descriptor for
>> >> cyclic dma") wrongly completes descriptor for cyclic dma, hence following
>> >> BUG_ON is still hit with cyclic DMA operations.
>> >>
>> >> kernel BUG at drivers/dma/dmaengine.h:53!
>> >>
>> >> Signed-off-by: Tushar Behera <[email protected]>
>> >> ---
>> >> ?drivers/dma/pl330.c | ? ?2 +-
>> >> ?1 files changed, 1 insertions(+), 1 deletions(-)
>> >>
>> >> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
>> >> index fa3fb21..8c44f17 100644
>> >> --- a/drivers/dma/pl330.c
>> >> +++ b/drivers/dma/pl330.c
>> >> @@ -2322,7 +2322,7 @@ static void pl330_tasklet(unsigned long data)
>> >> ? ? ? /* Pick up ripe tomatoes */
>> >> ? ? ? list_for_each_entry_safe(desc, _dt, &pch->work_list, node)
>> >> ? ? ? ? ? ? ? if (desc->status == DONE) {
>> >> - ? ? ? ? ? ? ? ? ? ? if (pch->cyclic)
>> >> + ? ? ? ? ? ? ? ? ? ? if (!pch->cyclic)
>> >> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? dma_cookie_complete(&desc->txd);
>> >> ? ? ? ? ? ? ? ? ? ? ? list_move_tail(&desc->node, &list);
>> >> ? ? ? ? ? ? ? }
>> >
>> Acked-by: Jassi Brar <[email protected]>
>>
>> Obviously a typo, but the commit eab2158 had a tested-by, so I hope
>> this doesn't break something else.
> Precisely my worry as well. Thomas?

I am not sure what I have missed here. Since it was a simple change, I
think I manually edited the code for the test. I apologize for the
mistake in the test.

Regards,
Thomas.

>
> >From the code this looks as the right one.
>
> --
> ~Vinod
>

2012-06-07 08:20:53

by Vinod Koul

Subject: Re: [PATCH] dmaengine: pl330: dont complete descriptor for cyclic dma

On Wed, 2012-05-23 at 16:47 +0530, Tushar Behera wrote:
> Commit eab215855803 ("dmaengine: pl330: dont complete descriptor for
> cyclic dma") wrongly completes descriptor for cyclic dma, hence following
> BUG_ON is still hit with cyclic DMA operations.
>
> kernel BUG at drivers/dma/dmaengine.h:53!
>
> Signed-off-by: Tushar Behera <[email protected]>
> ---
> drivers/dma/pl330.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index fa3fb21..8c44f17 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -2322,7 +2322,7 @@ static void pl330_tasklet(unsigned long data)
> /* Pick up ripe tomatoes */
> list_for_each_entry_safe(desc, _dt, &pch->work_list, node)
> if (desc->status == DONE) {
> - if (pch->cyclic)
> + if (!pch->cyclic)
> dma_cookie_complete(&desc->txd);
> list_move_tail(&desc->node, &list);
> }
Applied thanks. I have cc'ed stable.

--
~Vinod