| Message ID | 1395057448-22904-1-git-send-email-nsekhar@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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: <stable@vger.kernel.org> # v3.12.x+ > Cc: Joel Fernandes <joelf@ti.com> > Reported-by: Jon Ringle <jringle@gridpoint.com> > Signed-off-by: Sekhar Nori <nsekhar@ti.com> > --- > 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 > >
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: <stable@vger.kernel.org> # v3.12.x+ >> Cc: Joel Fernandes <joelf@ti.com> >> Reported-by: Jon Ringle <jringle@gridpoint.com> >> Signed-off-by: Sekhar Nori <nsekhar@ti.com> >> --- >> 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
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: <stable@vger.kernel.org> # v3.12.x+ > >> Cc: Joel Fernandes <joelf@ti.com> > >> Reported-by: Jon Ringle <jringle@gridpoint.com> > >> Signed-off-by: Sekhar Nori <nsekhar@ti.com> > >> --- > >> 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 <jringle@gridpoint.com>
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: <stable@vger.kernel.org> # v3.12.x+ > > >> Cc: Joel Fernandes <joelf@ti.com> > > >> Reported-by: Jon Ringle <jringle@gridpoint.com> > > >> Signed-off-by: Sekhar Nori <nsekhar@ti.com> > > >> --- > > >> 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 <jringle@gridpoint.com> where is this patch, somehow am not able to find in my inbox or archives...
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: <stable@vger.kernel.org> # v3.12.x+ >>>>> Cc: Joel Fernandes <joelf@ti.com> >>>>> Reported-by: Jon Ringle <jringle@gridpoint.com> >>>>> Signed-off-by: Sekhar Nori <nsekhar@ti.com> >>>>> --- >>>>> 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 <jringle@gridpoint.com> > 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 <joelf@ti.com> Regards, -Joel
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 <jringle@gridpoint.com> Finally working mmc on davinci (didn't work without DMA here too). Tested-by: Alexander Holler <holler@ahsoftware.de> Regards, Alexander Holler
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 <joelf@ti.com> Joel, thanks for the ack. Vinod, Looks like the vger and Intel spam filters dropped the patch. I will try again. Thanks, Sekhar
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 <joelf@ti.com> > > 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
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); } /*
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: <stable@vger.kernel.org> # v3.12.x+ Cc: Joel Fernandes <joelf@ti.com> Reported-by: Jon Ringle <jringle@gridpoint.com> Signed-off-by: Sekhar Nori <nsekhar@ti.com> --- 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(-)