[PATCH] dma: edma: fix incorrect SG list handling

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH] dma: edma: fix incorrect SG list handling

Sekhar Nori
The code to handle any length SG lists calls edma_resume()
even before edma_start() is called. This is incorrect
because edma_resume() enables edma events on the channel
after which CPU (in edma_start) cannot clear posted
events by writing to ECR (per the EDMA user's guide).

Because of this EDMA transfers fail to start if due
to some reason there is a pending EDMA event registered
even before EDMA transfers are started. This can happen if
an EDMA event is a byproduct of device initialization.

Fix this by calling edma_resume() only if it is not the
first batch of MAX_NR_SG elements.

Without this patch, MMC/SD fails to function on DA850 EVM
with DMA. The behaviour is triggered by specific IP and
this can explain why the issue was not reported before
(example with MMC/SD on AM335x).

Tested on DA850 EVM and AM335x EVM-SK using MMC/SD card.

Cc: <[hidden email]> # v3.12.x+
Cc: Joel Fernandes <[hidden email]>
Reported-by: Jon Ringle <[hidden email]>
Signed-off-by: Sekhar Nori <[hidden email]>
---
Jon, can you please confirm this fixes the issue you
reported?
Joel, can you ack this please? In particular, I was
not able to test with SG list greater than MAX_NR_SG.

 drivers/dma/edma.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index cd8da45..bf5ad0f 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -182,11 +182,13 @@ static void edma_execute(struct edma_chan *echan)
   echan->ecc->dummy_slot);
  }
 
- edma_resume(echan->ch_num);
-
  if (edesc->processed <= MAX_NR_SG) {
  dev_dbg(dev, "first transfer starting %d\n", echan->ch_num);
  edma_start(echan->ch_num);
+ } else {
+ dev_dbg(dev, "chan: %d: completed %d elements, resuming\n",
+ echan->ch_num, edesc->processed);
+ edma_resume(echan->ch_num);
  }
 
  /*
--
1.7.10.1

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] dma: edma: fix incorrect SG list handling

Jon Ringle

On Mon, 17 Mar 2014, Sekhar Nori wrote:

> The code to handle any length SG lists calls edma_resume()
> even before edma_start() is called. This is incorrect
> because edma_resume() enables edma events on the channel
> after which CPU (in edma_start) cannot clear posted
> events by writing to ECR (per the EDMA user's guide).
>
> Because of this EDMA transfers fail to start if due
> to some reason there is a pending EDMA event registered
> even before EDMA transfers are started. This can happen if
> an EDMA event is a byproduct of device initialization.
>
> Fix this by calling edma_resume() only if it is not the
> first batch of MAX_NR_SG elements.
>
> Without this patch, MMC/SD fails to function on DA850 EVM
> with DMA. The behaviour is triggered by specific IP and
> this can explain why the issue was not reported before
> (example with MMC/SD on AM335x).
>
> Tested on DA850 EVM and AM335x EVM-SK using MMC/SD card.
>
> Cc: <[hidden email]> # v3.12.x+
> Cc: Joel Fernandes <[hidden email]>
> Reported-by: Jon Ringle <[hidden email]>
> Signed-off-by: Sekhar Nori <[hidden email]>
> ---
> Jon, can you please confirm this fixes the issue you
> reported?

The patch does not apply on linux-3.12 due to changes to the 3 context
lines at the start of the hunk.

But, I manually fixed up the code and it does fix the issue  on our AM1808
board.

> Joel, can you ack this please? In particular, I was
> not able to test with SG list greater than MAX_NR_SG.
>
>  drivers/dma/edma.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
> index cd8da45..bf5ad0f 100644
> --- a/drivers/dma/edma.c
> +++ b/drivers/dma/edma.c
> @@ -182,11 +182,13 @@ static void edma_execute(struct edma_chan *echan)
>    echan->ecc->dummy_slot);
>   }
>  

The above 3 lines are different on linux-3.12. I have the following in my
linux-3.12 tree:
        if (edesc->processed == edesc->pset_nr)
                edma_link(echan->slot[nslots-1], echan->ecc->dummy_slot);

> - edma_resume(echan->ch_num);
> -
>   if (edesc->processed <= MAX_NR_SG) {
>   dev_dbg(dev, "first transfer starting %d\n", echan->ch_num);
>   edma_start(echan->ch_num);
> + } else {
> + dev_dbg(dev, "chan: %d: completed %d elements, resuming\n",
> + echan->ch_num, edesc->processed);
> + edma_resume(echan->ch_num);
>   }
>  
>   /*
> --
> 1.7.10.1
>
>
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] dma: edma: fix incorrect SG list handling

Sekhar Nori
Hi Jon,

On Monday 17 March 2014 06:28 PM, Jon Ringle wrote:

>
> On Mon, 17 Mar 2014, Sekhar Nori wrote:
>
>> The code to handle any length SG lists calls edma_resume()
>> even before edma_start() is called. This is incorrect
>> because edma_resume() enables edma events on the channel
>> after which CPU (in edma_start) cannot clear posted
>> events by writing to ECR (per the EDMA user's guide).
>>
>> Because of this EDMA transfers fail to start if due
>> to some reason there is a pending EDMA event registered
>> even before EDMA transfers are started. This can happen if
>> an EDMA event is a byproduct of device initialization.
>>
>> Fix this by calling edma_resume() only if it is not the
>> first batch of MAX_NR_SG elements.
>>
>> Without this patch, MMC/SD fails to function on DA850 EVM
>> with DMA. The behaviour is triggered by specific IP and
>> this can explain why the issue was not reported before
>> (example with MMC/SD on AM335x).
>>
>> Tested on DA850 EVM and AM335x EVM-SK using MMC/SD card.
>>
>> Cc: <[hidden email]> # v3.12.x+
>> Cc: Joel Fernandes <[hidden email]>
>> Reported-by: Jon Ringle <[hidden email]>
>> Signed-off-by: Sekhar Nori <[hidden email]>
>> ---
>> Jon, can you please confirm this fixes the issue you
>> reported?
>
> The patch does not apply on linux-3.12 due to changes to the 3 context
> lines at the start of the hunk.
>
> But, I manually fixed up the code and it does fix the issue  on our AM1808
> board.

Thanks for the testing. The patch is meant for latest mainline but based
on what you said, a manual backport to v3.12-stable will be required.

Can you please reply with a formal Tested-by: ?

Regards,
Sekhar
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] dma: edma: fix incorrect SG list handling

Jon Ringle
On Mon, 17 Mar 2014, Sekhar Nori wrote:

> Hi Jon,
>
> On Monday 17 March 2014 06:28 PM, Jon Ringle wrote:
> >
> > On Mon, 17 Mar 2014, Sekhar Nori wrote:
> >
> >> The code to handle any length SG lists calls edma_resume()
> >> even before edma_start() is called. This is incorrect
> >> because edma_resume() enables edma events on the channel
> >> after which CPU (in edma_start) cannot clear posted
> >> events by writing to ECR (per the EDMA user's guide).
> >>
> >> Because of this EDMA transfers fail to start if due
> >> to some reason there is a pending EDMA event registered
> >> even before EDMA transfers are started. This can happen if
> >> an EDMA event is a byproduct of device initialization.
> >>
> >> Fix this by calling edma_resume() only if it is not the
> >> first batch of MAX_NR_SG elements.
> >>
> >> Without this patch, MMC/SD fails to function on DA850 EVM
> >> with DMA. The behaviour is triggered by specific IP and
> >> this can explain why the issue was not reported before
> >> (example with MMC/SD on AM335x).
> >>
> >> Tested on DA850 EVM and AM335x EVM-SK using MMC/SD card.
> >>
> >> Cc: <[hidden email]> # v3.12.x+
> >> Cc: Joel Fernandes <[hidden email]>
> >> Reported-by: Jon Ringle <[hidden email]>
> >> Signed-off-by: Sekhar Nori <[hidden email]>
> >> ---
> >> Jon, can you please confirm this fixes the issue you
> >> reported?
> >
> > The patch does not apply on linux-3.12 due to changes to the 3 context
> > lines at the start of the hunk.
> >
> > But, I manually fixed up the code and it does fix the issue  on our AM1808
> > board.
>
> Thanks for the testing. The patch is meant for latest mainline but based
> on what you said, a manual backport to v3.12-stable will be required.
>
> Can you please reply with a formal Tested-by: ?

Tested-by: Jon Ringle <[hidden email]>
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] dma: edma: fix incorrect SG list handling

Vinod Koul-3
On Mon, Mar 17, 2014 at 09:14:14AM -0400, Jon Ringle wrote:

> On Mon, 17 Mar 2014, Sekhar Nori wrote:
>
> > Hi Jon,
> >
> > On Monday 17 March 2014 06:28 PM, Jon Ringle wrote:
> > >
> > > On Mon, 17 Mar 2014, Sekhar Nori wrote:
> > >
> > >> The code to handle any length SG lists calls edma_resume()
> > >> even before edma_start() is called. This is incorrect
> > >> because edma_resume() enables edma events on the channel
> > >> after which CPU (in edma_start) cannot clear posted
> > >> events by writing to ECR (per the EDMA user's guide).
> > >>
> > >> Because of this EDMA transfers fail to start if due
> > >> to some reason there is a pending EDMA event registered
> > >> even before EDMA transfers are started. This can happen if
> > >> an EDMA event is a byproduct of device initialization.
> > >>
> > >> Fix this by calling edma_resume() only if it is not the
> > >> first batch of MAX_NR_SG elements.
> > >>
> > >> Without this patch, MMC/SD fails to function on DA850 EVM
> > >> with DMA. The behaviour is triggered by specific IP and
> > >> this can explain why the issue was not reported before
> > >> (example with MMC/SD on AM335x).
> > >>
> > >> Tested on DA850 EVM and AM335x EVM-SK using MMC/SD card.
> > >>
> > >> Cc: <[hidden email]> # v3.12.x+
> > >> Cc: Joel Fernandes <[hidden email]>
> > >> Reported-by: Jon Ringle <[hidden email]>
> > >> Signed-off-by: Sekhar Nori <[hidden email]>
> > >> ---
> > >> Jon, can you please confirm this fixes the issue you
> > >> reported?
> > >
> > > The patch does not apply on linux-3.12 due to changes to the 3 context
> > > lines at the start of the hunk.
> > >
> > > But, I manually fixed up the code and it does fix the issue  on our AM1808
> > > board.
> >
> > Thanks for the testing. The patch is meant for latest mainline but based
> > on what you said, a manual backport to v3.12-stable will be required.
> >
> > Can you please reply with a formal Tested-by: ?
>
> Tested-by: Jon Ringle <[hidden email]>
where is this patch, somehow am not able to find in my inbox or archives...

--
~Vinod
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] dma: edma: fix incorrect SG list handling

Joel Fernandes
On 03/18/2014 10:28 AM, Vinod Koul wrote:

> On Mon, Mar 17, 2014 at 09:14:14AM -0400, Jon Ringle wrote:
>> On Mon, 17 Mar 2014, Sekhar Nori wrote:
>>
>>> Hi Jon,
>>>
>>> On Monday 17 March 2014 06:28 PM, Jon Ringle wrote:
>>>>
>>>> On Mon, 17 Mar 2014, Sekhar Nori wrote:
>>>>
>>>>> The code to handle any length SG lists calls edma_resume()
>>>>> even before edma_start() is called. This is incorrect
>>>>> because edma_resume() enables edma events on the channel
>>>>> after which CPU (in edma_start) cannot clear posted
>>>>> events by writing to ECR (per the EDMA user's guide).
>>>>>
>>>>> Because of this EDMA transfers fail to start if due
>>>>> to some reason there is a pending EDMA event registered
>>>>> even before EDMA transfers are started. This can happen if
>>>>> an EDMA event is a byproduct of device initialization.
>>>>>
>>>>> Fix this by calling edma_resume() only if it is not the
>>>>> first batch of MAX_NR_SG elements.
>>>>>
>>>>> Without this patch, MMC/SD fails to function on DA850 EVM
>>>>> with DMA. The behaviour is triggered by specific IP and
>>>>> this can explain why the issue was not reported before
>>>>> (example with MMC/SD on AM335x).
>>>>>
>>>>> Tested on DA850 EVM and AM335x EVM-SK using MMC/SD card.
>>>>>
>>>>> Cc: <[hidden email]> # v3.12.x+
>>>>> Cc: Joel Fernandes <[hidden email]>
>>>>> Reported-by: Jon Ringle <[hidden email]>
>>>>> Signed-off-by: Sekhar Nori <[hidden email]>
>>>>> ---
>>>>> Jon, can you please confirm this fixes the issue you
>>>>> reported?
>>>>
>>>> The patch does not apply on linux-3.12 due to changes to the 3 context
>>>> lines at the start of the hunk.
>>>>
>>>> But, I manually fixed up the code and it does fix the issue  on our AM1808
>>>> board.
>>>
>>> Thanks for the testing. The patch is meant for latest mainline but based
>>> on what you said, a manual backport to v3.12-stable will be required.
>>>
>>> Can you please reply with a formal Tested-by: ?
>>
>> Tested-by: Jon Ringle <[hidden email]>
> where is this patch, somehow am not able to find in my inbox or archives...
>

I found it archived here:
http://comments.gmane.org/gmane.linux.davinci/28569

Patch doesn't breaking anything for > MAX_NR_SG list size on AM335x, so
it looks good.

Acked-by: Joel Fernandes <[hidden email]>

Regards,
-Joel
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] dma: edma: fix incorrect SG list handling

Alexander Holler
In reply to this post by Jon Ringle
Am 17.03.2014 14:14, schrieb Jon Ringle:
> On Mon, 17 Mar 2014, Sekhar Nori wrote:
>> Can you please reply with a formal Tested-by: ?
>
> Tested-by: Jon Ringle <[hidden email]>

Finally working mmc on davinci (didn't work without DMA here too).

Tested-by: Alexander Holler <[hidden email]>

Regards,

Alexander Holler
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] dma: edma: fix incorrect SG list handling

Sekhar Nori
In reply to this post by Joel Fernandes
On Wednesday 19 March 2014 12:16 AM, Joel Fernandes wrote:

> On 03/18/2014 10:28 AM, Vinod Koul wrote:
>> where is this patch, somehow am not able to find in my inbox or archives...
>>
>
> I found it archived here:
> http://comments.gmane.org/gmane.linux.davinci/28569
>
> Patch doesn't breaking anything for > MAX_NR_SG list size on AM335x, so
> it looks good.
>
> Acked-by: Joel Fernandes <[hidden email]>

Joel, thanks for the ack.

Vinod, Looks like the vger and Intel spam filters dropped the patch. I
will try again.

Thanks,
Sekhar
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] dma: edma: fix incorrect SG list handling

Sekhar Nori
On Wednesday 19 March 2014 10:09 AM, Sekhar Nori wrote:

> On Wednesday 19 March 2014 12:16 AM, Joel Fernandes wrote:
>> On 03/18/2014 10:28 AM, Vinod Koul wrote:
>>> where is this patch, somehow am not able to find in my inbox or archives...
>>>
>>
>> I found it archived here:
>> http://comments.gmane.org/gmane.linux.davinci/28569
>>
>> Patch doesn't breaking anything for > MAX_NR_SG list size on AM335x, so
>> it looks good.
>>
>> Acked-by: Joel Fernandes <[hidden email]>
>
> Joel, thanks for the ack.
>
> Vinod, Looks like the vger and Intel spam filters dropped the patch. I
> will try again.

Okay, I see see the patch in dmaengine list archives now and hopefully
it has reached Vinod too.

I think the issue last time around was that From: and Return-Path:
headers ended up having different addresses.

Thanks,
Sekhar

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Loading...