[PATCH] arm, da8xx, mmc: set second MMC controllers default queue

[PATCH] arm, da8xx, mmc: set second MMC controllers default queue

[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 <hs@denx.de>
Signed-off-by: juha.kuikka@gmail.com
Reported-by: juha.kuikka@gmail.com
Cc: linux-mmc@vger.kernel.org
Cc: davinci-linux-open-source@linux.davincidsp.com
Cc: Rajashekhara, Sudhakar <sudhakar.raj@ti.com>
Cc: Ido Yariv <ido@wizery.com>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Wolfgang Denk <wd@denx.de>
---
 arch/arm/mach-davinci/dma.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-davinci/dma.c b/arch/arm/mach-davinci/dma.c
index da90103..e10a251 100644
--- a/arch/arm/mach-davinci/dma.c
+++ b/arch/arm/mach-davinci/dma.c
@@ -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;
-- 
1.7.6.4

Re: [PATCH] arm, da8xx, mmc: set second MMC controllers default queue

[Sergei Shtylyov]

Hello.

On 04-12-2011 13:33, Heiko Schocher wrote:

> The MMC driver allocates channels with EVENTQ_DEFAULT they

    Missed comma before "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

    Please also specify that commit's summary in parens.

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

> Signed-off-by: Heiko Schocher<hs@denx.de>
> Signed-off-by: juha.kuikka@gmail.com
> Reported-by: juha.kuikka@gmail.com
> Cc: linux-mmc@vger.kernel.org
> Cc: davinci-linux-open-source@linux.davincidsp.com
> Cc: Rajashekhara, Sudhakar<sudhakar.raj@ti.com>
> Cc: Ido Yariv<ido@wizery.com>
> Cc: Sekhar Nori<nsekhar@ti.com>
> Cc: Wolfgang Denk<wd@denx.de>

WBR, Sergei

RE: [PATCH] arm, da8xx, mmc: set second MMC controllers default queue

[Rajashekhara, Sudhakar]

Hi,

On Sun, Dec 04, 2011 at 15:03:30, Heiko Schocher wrote:
> 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 <hs@denx.de>
> Signed-off-by: juha.kuikka@gmail.com
> Reported-by: juha.kuikka@gmail.com
> Cc: linux-mmc@vger.kernel.org
> Cc: davinci-linux-open-source@linux.davincidsp.com
> Cc: Rajashekhara, Sudhakar <sudhakar.raj@ti.com>
> Cc: Ido Yariv <ido@wizery.com>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Wolfgang Denk <wd@denx.de>
> ---
>  arch/arm/mach-davinci/dma.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/dma.c b/arch/arm/mach-davinci/dma.c
> index da90103..e10a251 100644
> --- a/arch/arm/mach-davinci/dma.c
> +++ b/arch/arm/mach-davinci/dma.c
> @@ -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 <sudhakar.raj@ti.com>

Regards,
Sudhakar

Re: [PATCH] arm, da8xx, mmc: set second MMC controllers default queue

[Heiko Schocher]

Hello Rajashekhara,

Rajashekhara, Sudhakar wrote:
> Hi,
> 
> On Sun, Dec 04, 2011 at 15:03:30, Heiko Schocher wrote:
>> 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 <hs@denx.de>
>> Signed-off-by: juha.kuikka@gmail.com
>> Reported-by: juha.kuikka@gmail.com
>> Cc: linux-mmc@vger.kernel.org
>> Cc: davinci-linux-open-source@linux.davincidsp.com
>> Cc: Rajashekhara, Sudhakar <sudhakar.raj@ti.com>
>> Cc: Ido Yariv <ido@wizery.com>
>> Cc: Sekhar Nori <nsekhar@ti.com>
>> Cc: Wolfgang Denk <wd@denx.de>
>> ---
>>  arch/arm/mach-davinci/dma.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/dma.c b/arch/arm/mach-davinci/dma.c
>> index da90103..e10a251 100644
>> --- a/arch/arm/mach-davinci/dma.c
>> +++ b/arch/arm/mach-davinci/dma.c
>> @@ -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 <sudhakar.raj@ti.com>

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

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

RE: [PATCH] arm, da8xx, mmc: set second MMC controllers default queue

[Nori, Sekhar]

On Mon, Dec 05, 2011 at 16:16:33, Heiko Schocher wrote:
> 
> 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

RE: [PATCH] arm, da8xx, mmc: set second MMC controllers default queue

[Hernandez, Alejandro]

Sekhar,

Do you still want me to try the patch?

Thanks,
Alejandro

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

On Mon, Dec 05, 2011 at 16:16:33, Heiko Schocher wrote:
> 
> 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@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

[PATCH v2] arm, da8xx, mmc: set second MMC controllers default queue

[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 <hs@denx.de>
Signed-off-by: juha.kuikka@gmail.com
Reported-by: juha.kuikka@gmail.com
Acked-by: Rajashekhara, Sudhakar <sudhakar.raj@ti.com>
Cc: linux-mmc@vger.kernel.org
Cc: davinci-linux-open-source@linux.davincidsp.com
Cc: Rajashekhara, Sudhakar <sudhakar.raj@ti.com>
Cc: Ido Yariv <ido@wizery.com>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Sergei Shtylyov <sshtylyov@mvista.com>

---
- 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(-)

diff --git a/arch/arm/mach-davinci/dma.c b/arch/arm/mach-davinci/dma.c
index da90103..a0cabc7 100644
--- a/arch/arm/mach-davinci/dma.c
+++ b/arch/arm/mach-davinci/dma.c
@@ -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;
-- 
1.7.6.4

Re: [PATCH v2] arm, da8xx, mmc: set second MMC controllers default queue

[Heiko Schocher]

Hello,

Heiko Schocher wrote:
> 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 <hs@denx.de>
> Signed-off-by: juha.kuikka@gmail.com
> Reported-by: juha.kuikka@gmail.com
> Acked-by: Rajashekhara, Sudhakar <sudhakar.raj@ti.com>
> Cc: linux-mmc@vger.kernel.org
> Cc: davinci-linux-open-source@linux.davincidsp.com
> Cc: Rajashekhara, Sudhakar <sudhakar.raj@ti.com>
> Cc: Ido Yariv <ido@wizery.com>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Sergei Shtylyov <sshtylyov@mvista.com>
> 
> ---
> - 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(-)

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

Thanks
bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

RE: [PATCH v2] arm, da8xx, mmc: set second MMC controllers default queue

[Nori, Sekhar]

Hi Heiko,

On Wed, Jan 11, 2012 at 13:58:56, Heiko Schocher wrote:
> Hello,
> 
> Heiko Schocher wrote:
> > 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 <hs@denx.de>
> > Signed-off-by: juha.kuikka@gmail.com
> > Reported-by: juha.kuikka@gmail.com
> > Acked-by: Rajashekhara, Sudhakar <sudhakar.raj@ti.com>
> > Cc: linux-mmc@vger.kernel.org
> > Cc: davinci-linux-open-source@linux.davincidsp.com
> > Cc: Rajashekhara, Sudhakar <sudhakar.raj@ti.com>
> > Cc: Ido Yariv <ido@wizery.com>
> > Cc: Sekhar Nori <nsekhar@ti.com>
> > Cc: Wolfgang Denk <wd@denx.de>
> > Cc: Sergei Shtylyov <sshtylyov@mvista.com>
> > 
> > ---
> > - 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(-)
> 
> 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 linux-arm-kernel@lists.infradead.org on ARM patch submissions.

Thanks,
Sekhar