Skip to contentSkip to conversation

arm, da8xx, mmc: set second MMC controllers default queue

PATCH New Heiko Schocher 5 participants 9 messages Dec 4, 2011 – Jan 12, 2012

Subsystems (4)
Arm (32-bit) Arm sub-architectures MMC, SD and SDIO TI DaVinci machine support
lore
Share & tools
lore

Heiko Schocher

author
#0 lore raw

PATCH 1/1 arm, da8xx, mmc: set second MMC controllers default queue

The MMC driver allocates channels with EVENTQ_DEFAULT they get put into EVENTQ_1 which the second EDMA controller does not have and hence transfers stall. This is tried to fix in commit f23fe857bbea393b4b94fe2218c98d934bd3d4cf from Ido Yariv, but missed a fix for the second MMC controller on da850.

Signed-off-by Heiko Schocher <[email protected]>
Signed-off-by juha.kuikka at gmail.com
Reported-by juha.kuikka at gmail.com
Cc linux-mmc at vger.kernel.org
Cc davinci-linux-open-source at linux.davincidsp.com
Cc Rajashekhara, Sudhakar <[email protected]>
Cc Ido Yariv <[email protected]>
Cc Sekhar Nori <[email protected]>
Cc Wolfgang Denk <[email protected]>
---
 arch/arm/mach-davinci/dma.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach-davinci/dma.c +1 -1
index da90103..e10a251 100644
@@ -1513,7 +1513,7 @@ static int __init edma_probe(struct platform_device *pdev)
 		 * started by the codec engine will not cause audio defects.
 		 */
 		for (i = 0; i < edma_cc[j]->num_channels; i++)
-			map_dmach_queue(j, i, EVENTQ_1);
+			map_dmach_queue(j, i, info[j]->default_queue);
 
 		queue_tc_mapping = info[j]->queue_tc_mapping;
 		queue_priority_mapping = info[j]->queue_priority_mapping;
Signature · 1 line
1.7.6.4
#1 lore raw

PATCH 1/1 arm, da8xx, mmc: set second MMC controllers default queue

Hello.

Quoted 1 line · Heiko Schocher

The MMC driver allocates channels with EVENTQ_DEFAULT they

Missed comma before "they"?

Quoted 3 lines

get put into EVENTQ_1 which the second EDMA controller does not have and hence transfers stall. This is tried to fix in commit f23fe857bbea393b4b94fe2218c98d934bd3d4cf

Please also specify that commit's summary in parens.

Quoted 12 lines

from Ido Yariv, but missed a fix for the second MMC controller on da850.

Signed-off-by Heiko Schocher<[email protected]>
Signed-off-by juha.kuikka at gmail.com
Reported-by juha.kuikka at gmail.com
Cc linux-mmc at vger.kernel.org
Cc davinci-linux-open-source at linux.davincidsp.com
Cc Rajashekhara, Sudhakar<[email protected]>
Cc Ido Yariv<[email protected]>
Cc Sekhar Nori<[email protected]>
Cc Wolfgang Denk<[email protected]>

WBR, Sergei

#2 lore raw

PATCH 1/1 arm, da8xx, mmc: set second MMC controllers default queue

In reply to Heiko Schocher

Hi,

Quoted 25 lines · Heiko Schocher

The MMC driver allocates channels with EVENTQ_DEFAULT they get put into EVENTQ_1 which the second EDMA controller does not have and hence transfers stall. This is tried to fix in commit f23fe857bbea393b4b94fe2218c98d934bd3d4cf from Ido Yariv, but missed a fix for the second MMC controller on da850.

Signed-off-by Heiko Schocher <[email protected]>
Signed-off-by juha.kuikka at gmail.com
Reported-by juha.kuikka at gmail.com
Cc linux-mmc at vger.kernel.org
Cc davinci-linux-open-source at linux.davincidsp.com
Cc Rajashekhara, Sudhakar <[email protected]>
Cc Ido Yariv <[email protected]>
Cc Sekhar Nori <[email protected]>
Cc Wolfgang Denk <[email protected]>
---
 arch/arm/mach-davinci/dma.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach-davinci/dma.c +0 -0
index da90103..e10a251 100644

arch/arm/mach-davinci/dma.c static int __init edma_probe(struct platform_device *pdev) · line 1519

Quoted 8 lines
arch/arm/mach-davinci/dma.c +1 -1
@@ -1513,7 +1513,7 @@ static int __init edma_probe(struct platform_device *pdev)
 		 * started by the codec engine will not cause audio defects.
 		 */
 		for (i = 0; i < edma_cc[j]->num_channels; i++)
-			map_dmach_queue(j, i, EVENTQ_1);
+			map_dmach_queue(j, i, info[j]->default_queue);
 
 		queue_tc_mapping = info[j]->queue_tc_mapping;
 		queue_priority_mapping = info[j]->queue_priority_mapping;
Acked-by Rajashekhara, Sudhakar <[email protected]>

Regards,
Sudhakar

author
#3 lore raw

PATCH 1/1 arm, da8xx, mmc: set second MMC controllers default queue

Hello Rajashekhara,

Quoted 34 lines · Sudhakar

Hi,

Quoted 31 lines · Heiko Schocher

The MMC driver allocates channels with EVENTQ_DEFAULT they get put into EVENTQ_1 which the second EDMA controller does not have and hence transfers stall. This is tried to fix in commit f23fe857bbea393b4b94fe2218c98d934bd3d4cf from Ido Yariv, but missed a fix for the second MMC controller on da850.

Signed-off-by Heiko Schocher <[email protected]>
Signed-off-by juha.kuikka at gmail.com
Reported-by juha.kuikka at gmail.com
Cc linux-mmc at vger.kernel.org
Cc davinci-linux-open-source at linux.davincidsp.com
Cc Rajashekhara, Sudhakar <[email protected]>
Cc Ido Yariv <[email protected]>
Cc Sekhar Nori <[email protected]>
Cc Wolfgang Denk <[email protected]>
---
 arch/arm/mach-davinci/dma.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach-davinci/dma.c +1 -1
index da90103..e10a251 100644
@@ -1513,7 +1513,7 @@ static int __init edma_probe(struct platform_device *pdev)
 		 * started by the codec engine will not cause audio defects.
 		 */
 		for (i = 0; i < edma_cc[j]->num_channels; i++)
-			map_dmach_queue(j, i, EVENTQ_1);
+			map_dmach_queue(j, i, info[j]->default_queue);
Quoted 4 lines

arch/arm/mach-davinci/dma.c static int __init edma_probe(struct platform_device *pdev) · line 1519

Quoted 2 lines
arch/arm/mach-davinci/dma.c +0 -0
 		queue_tc_mapping = info[j]->queue_tc_mapping;
 		queue_priority_mapping = info[j]->queue_priority_mapping;
Acked-by Rajashekhara, Sudhakar <[email protected]>

Thanks, but as Ido commented, this patch is no longer necessary, sorry for the noise.

bye,
Heiko

Signature · 2 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
#4 lore raw

PATCH 1/1 arm, da8xx, mmc: set second MMC controllers default queue

Quoted 4 lines · Heiko Schocher

Thanks, but as Ido commented, this patch is no longer necessary, sorry for the noise.

It doesn't look like Ido's comment reached me or the archives. Can you please forward it?

Thanks,
Sekhar

#5 lore raw

PATCH 1/1 arm, da8xx, mmc: set second MMC controllers default queue

Sekhar,

Do you still want me to try the patch?

Thanks,
Alejandro

-----Original Message-----
From: [email protected] [mailto:davinci-linux-open-source-bounces at linux.davincidsp.com] On Behalf Of Nori, Sekhar
Sent: Tuesday, December 06, 2011 2:10 PM
To: hs at denx.de; Rajashekhara, Sudhakar
Cc: davinci-linux-open-source at linux.davincidsp.com; Wolfgang Denk; linux-mmc at vger.kernel.org; Ido Yariv; linux-arm-kernel at lists.infradead.org
Subject: RE: [PATCH] arm, da8xx, mmc: set second MMC controllers default queue

Quoted 4 lines · Heiko Schocher

Thanks, but as Ido commented, this patch is no longer necessary, sorry for the noise.

It doesn't look like Ido's comment reached me or the archives. Can you please forward it?

Thanks,
Sekhar
_______________________________________________
Davinci-linux-open-source mailing list
Davinci-linux-open-source at linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

author
#6 lore raw

PATCH v2 1/1 arm, da8xx, mmc: set second MMC controllers default queue

In reply to Heiko Schocher

The MMC driver allocates channels with EVENTQ_DEFAULT, they get put into EVENTQ_1, which the second EDMA controller does not have and hence transfers stall. This is fixed
in commit f23fe857bbea393b4b94fe2218c98d934bd3d4cf "ARM: davinci: Explicitly set channel controllers' default queues" from Ido Yariv. This patch sets immediately in edma_probe() the proper default_queue, so this patch does not really fix something, it is more a cosmetic change.

Signed-off-by Heiko Schocher <[email protected]>
Signed-off-by [email protected]
Reported-by [email protected]
Acked-by Rajashekhara, Sudhakar <[email protected]>
Cc [email protected]
Cc [email protected]
Cc Rajashekhara, Sudhakar <[email protected]>
Cc Ido Yariv <[email protected]>
Cc Sekhar Nori <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Sergei Shtylyov <[email protected]>

---
- changes for v2:
- add comment from Sergei Shtylyov
add in commit message the commit's summary in parens.
- add comment from Ido Yariv:
changed comment and commit message
- added Acked-by from Rajashekhara, Sudhakar

 arch/arm/mach-davinci/dma.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)
arch/arm/mach-davinci/dma.c +6 -4
index da90103..a0cabc7 100644
@@ -1508,12 +1508,14 @@ static int __init edma_probe(struct platform_device *pdev)
 			goto fail;
 		}
 
-		/* Everything lives on transfer controller 1 until otherwise
-		 * specified. This way, long transfers on the low priority queue
-		 * started by the codec engine will not cause audio defects.
+		/*
+		 * Everything lives on transfer controller 1, except on the
+		 * da850 MMC2 controller, so pass info[j]->default_queue.
+		 * This way, long transfers on the low priority queue started
+		 * by the codec engine will not cause audio defects.
 		 */
 		for (i = 0; i < edma_cc[j]->num_channels; i++)
-			map_dmach_queue(j, i, EVENTQ_1);
+			map_dmach_queue(j, i, info[j]->default_queue);
 
 		queue_tc_mapping = info[j]->queue_tc_mapping;
 		queue_priority_mapping = info[j]->queue_priority_mapping;
Signature · 1 line
1.7.6.4
author
#7 lore raw

Hello,

Quoted 29 lines · Heiko Schocher

The MMC driver allocates channels with EVENTQ_DEFAULT, they get put into EVENTQ_1, which the second EDMA controller does not have and hence transfers stall. This is fixed
in commit f23fe857bbea393b4b94fe2218c98d934bd3d4cf "ARM: davinci: Explicitly set channel controllers' default queues" from Ido Yariv. This patch sets immediately in edma_probe() the proper default_queue, so this patch does not really fix something, it is more a cosmetic change.

Signed-off-by Heiko Schocher <[email protected]>
Signed-off-by [email protected]
Reported-by [email protected]
Acked-by Rajashekhara, Sudhakar <[email protected]>
Cc [email protected]
Cc [email protected]
Cc Rajashekhara, Sudhakar <[email protected]>
Cc Ido Yariv <[email protected]>
Cc Sekhar Nori <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Sergei Shtylyov <[email protected]>

---
- changes for v2:
- add comment from Sergei Shtylyov
add in commit message the commit's summary in parens.
- add comment from Ido Yariv:
changed comment and commit message
- added Acked-by from Rajashekhara, Sudhakar

Quoted 2 lines
 arch/arm/mach-davinci/dma.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

Ping. Can this patch go into mainline, or are there some more objections?

Thanks
bye,
Heiko

Signature · 2 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
#8 lore raw

Hi Heiko,

Quoted 32 lines · Heiko Schocher

Hello,

Quoted 29 lines · Heiko Schocher

The MMC driver allocates channels with EVENTQ_DEFAULT, they get put into EVENTQ_1, which the second EDMA controller does not have and hence transfers stall. This is fixed
in commit f23fe857bbea393b4b94fe2218c98d934bd3d4cf "ARM: davinci: Explicitly set channel controllers' default queues" from Ido Yariv. This patch sets immediately in edma_probe() the proper default_queue, so this patch does not really fix something, it is more a cosmetic change.

Signed-off-by Heiko Schocher <[email protected]>
Signed-off-by [email protected]
Reported-by [email protected]
Acked-by Rajashekhara, Sudhakar <[email protected]>
Cc [email protected]
Cc [email protected]
Cc Rajashekhara, Sudhakar <[email protected]>
Cc Ido Yariv <[email protected]>
Cc Sekhar Nori <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc Sergei Shtylyov <[email protected]>

---
- changes for v2:
- add comment from Sergei Shtylyov
add in commit message the commit's summary in parens.
- add comment from Ido Yariv:
changed comment and commit message
- added Acked-by from Rajashekhara, Sudhakar

Quoted 4 lines
Quoted 2 lines
 arch/arm/mach-davinci/dma.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

Ping. Can this patch go into mainline, or are there some more objections?

Hmm, looks like you asking "no need for applying my patch" in an earlier e-mail on this topic made me discard this.

I had a look and the patch is OK. Except, I think the comment can do better without reference to TC1 since its not relevant once the code has been fixed. Also, the comment is better placed in edma.h where near the default_queue member definition since that's where folks will look up when setting up the default queue.

So, the comment can simply be:

+ /*
+ * Default queue is expected to be a low-priority queue.
+ * This way, long transfers on the default queue started
+ * by the codec engine will not cause audio defects.

Similarly you can drop references to the MMC/SD issue and the fix in the commit text since its not directly relevant to your patch.

Can you fix this up so I can apply it? Please note that patches for mach-davinci should be pre-fixed with "ARM: davinci: ".

Also Cc [email protected] on ARM patch submissions.

Thanks,
Sekhar