[v2,2/6] arm: davinci: Explicitly set channel controllers' default queues
diff mbox

Message ID 1310303679-17936-3-git-send-email-ido@wizery.com
State New, archived
Headers show

Commit Message

Ido Yariv July 10, 2011, 1:14 p.m. UTC
Davinci platforms may define a default queue for each channel
controller. If one is not defined, the default queue is set to EVENTQ_1.
However, there's no way to distinguish between an unset default queue to
one that is set to EVENTQ_0, as EVENTQ_0 = 0.

Explicitly specify the default queue for all channel controllers on all
Davinci platforms to EVENTQ_1, and don't overwrite it in the EDMA probe
function.

One exception is the DA850 board, for which EVENTQ_1 is not a valid
option for its second channel controller. Use EVENTQ_0 instead for that
channel controller.

Signed-off-by: Ido Yariv <ido@wizery.com>
---
 arch/arm/mach-davinci/devices-da8xx.c     |    3 +++
 arch/arm/mach-davinci/devices-tnetv107x.c |    1 +
 arch/arm/mach-davinci/dm355.c             |    1 +
 arch/arm/mach-davinci/dm644x.c            |    1 +
 arch/arm/mach-davinci/dm646x.c            |    1 +
 arch/arm/mach-davinci/dma.c               |    2 --
 6 files changed, 7 insertions(+), 2 deletions(-)

Comments

Sekhar Nori July 25, 2011, 1:15 p.m. UTC | #1
On Sun, Jul 10, 2011 at 18:44:35, Ido Yariv wrote:
> Davinci platforms may define a default queue for each channel
> controller. If one is not defined, the default queue is set to EVENTQ_1.
> However, there's no way to distinguish between an unset default queue to
> one that is set to EVENTQ_0, as EVENTQ_0 = 0.
> 
> Explicitly specify the default queue for all channel controllers on all
> Davinci platforms to EVENTQ_1, and don't overwrite it in the EDMA probe
> function.
> 
> One exception is the DA850 board, for which EVENTQ_1 is not a valid
> option for its second channel controller. Use EVENTQ_0 instead for that
> channel controller.
> 
> Signed-off-by: Ido Yariv <ido@wizery.com>

Looks good to me. Will queue for v3.2/fixes

BTW, Arnd has indicated a preference for "ARM: davinci: "
prefix so I will make that change while applying.

Thanks,
Sekhar
Sergei Shtylyov July 25, 2011, 1:33 p.m. UTC | #2
Hello.

Nori, Sekhar wrote:

>> Davinci platforms may define a default queue for each channel
>> controller. If one is not defined, the default queue is set to EVENTQ_1.
>> However, there's no way to distinguish between an unset default queue to
>> one that is set to EVENTQ_0, as EVENTQ_0 = 0.

>> Explicitly specify the default queue for all channel controllers on all
>> Davinci platforms to EVENTQ_1, and don't overwrite it in the EDMA probe
>> function.

>> One exception is the DA850 board, for which EVENTQ_1 is not a valid
>> option for its second channel controller. Use EVENTQ_0 instead for that
>> channel controller.

>> Signed-off-by: Ido Yariv <ido@wizery.com>

> Looks good to me. Will queue for v3.2/fixes

    Why wait for 3.2? If this is considered a fix, it should be applied to 3.1, no?

WBR, Sergei
Sekhar Nori July 25, 2011, 4:31 p.m. UTC | #3
Hi Sergei,

On Mon, Jul 25, 2011 at 19:03:45, Sergei Shtylyov wrote:
> Hello.
> 
> Nori, Sekhar wrote:
> 
> >> Davinci platforms may define a default queue for each channel
> >> controller. If one is not defined, the default queue is set to EVENTQ_1.
> >> However, there's no way to distinguish between an unset default queue to
> >> one that is set to EVENTQ_0, as EVENTQ_0 = 0.
> 
> >> Explicitly specify the default queue for all channel controllers on all
> >> Davinci platforms to EVENTQ_1, and don't overwrite it in the EDMA probe
> >> function.
> 
> >> One exception is the DA850 board, for which EVENTQ_1 is not a valid
> >> option for its second channel controller. Use EVENTQ_0 instead for that
> >> channel controller.
> 
> >> Signed-off-by: Ido Yariv <ido@wizery.com>
> 
> > Looks good to me. Will queue for v3.2/fixes
> 
>     Why wait for 3.2? If this is considered a fix, it should be applied to 3.1, no?

3.2/fixes just indicates it will be queued as a fix/cleanup
for 3.2 so it will have higher priority for merge when compared
to a new feature.

This patch doesn't really fix any existing broken functionality.
It corrects event queue configuration for EDMA CC1 on DA850 for
which there are no current users in mainline.

So, not sending for v3.1.

Thanks,
Sekhar
Ben Gardiner July 29, 2011, 2:42 p.m. UTC | #4
On Mon, Jul 25, 2011 at 12:31 PM, Nori, Sekhar <nsekhar@ti.com> wrote:
> Hi Sergei,
>
> On Mon, Jul 25, 2011 at 19:03:45, Sergei Shtylyov wrote:
>> Hello.
>>
>> Nori, Sekhar wrote:
>>
>> >> Davinci platforms may define a default queue for each channel
>> >> controller. If one is not defined, the default queue is set to EVENTQ_1.
>> >> However, there's no way to distinguish between an unset default queue to
>> >> one that is set to EVENTQ_0, as EVENTQ_0 = 0.
>>
>> >> Explicitly specify the default queue for all channel controllers on all
>> >> Davinci platforms to EVENTQ_1, and don't overwrite it in the EDMA probe
>> >> function.
>>
>> >> One exception is the DA850 board, for which EVENTQ_1 is not a valid
>> >> option for its second channel controller. Use EVENTQ_0 instead for that
>> >> channel controller.
>>
>> >> Signed-off-by: Ido Yariv <ido@wizery.com>
>>
>> > Looks good to me. Will queue for v3.2/fixes
>>
>>     Why wait for 3.2? If this is considered a fix, it should be applied to 3.1, no?
>
> 3.2/fixes just indicates it will be queued as a fix/cleanup
> for 3.2 so it will have higher priority for merge when compared
> to a new feature.
>
> This patch doesn't really fix any existing broken functionality.
> It corrects event queue configuration for EDMA CC1 on DA850 for
> which there are no current users in mainline.
>
> So, not sending for v3.1.

If it's not too late:

Tested this patch and it fixes SD/MMC1 support on da850.

Tested-by: Ben Gardiner <bengardiner@nanometrics.ca>

Thanks, Ido!

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

Patch
diff mbox

diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index fc4e98e..502222a 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -133,6 +133,7 @@  static struct edma_soc_info da830_edma_cc0_info = {
 	.n_cc			= 1,
 	.queue_tc_mapping	= da8xx_queue_tc_mapping,
 	.queue_priority_mapping	= da8xx_queue_priority_mapping,
+	.default_queue		= EVENTQ_1,
 };
 
 static struct edma_soc_info *da830_edma_info[EDMA_MAX_CC] = {
@@ -148,6 +149,7 @@  static struct edma_soc_info da850_edma_cc_info[] = {
 		.n_cc			= 1,
 		.queue_tc_mapping	= da8xx_queue_tc_mapping,
 		.queue_priority_mapping	= da8xx_queue_priority_mapping,
+		.default_queue		= EVENTQ_1,
 	},
 	{
 		.n_channel		= 32,
@@ -157,6 +159,7 @@  static struct edma_soc_info da850_edma_cc_info[] = {
 		.n_cc			= 1,
 		.queue_tc_mapping	= da850_queue_tc_mapping,
 		.queue_priority_mapping	= da850_queue_priority_mapping,
+		.default_queue		= EVENTQ_0,
 	},
 };
 
diff --git a/arch/arm/mach-davinci/devices-tnetv107x.c b/arch/arm/mach-davinci/devices-tnetv107x.c
index 6162cae..6298d46 100644
--- a/arch/arm/mach-davinci/devices-tnetv107x.c
+++ b/arch/arm/mach-davinci/devices-tnetv107x.c
@@ -80,6 +80,7 @@  static struct edma_soc_info edma_cc0_info = {
 	.n_cc			= 1,
 	.queue_tc_mapping	= edma_tc_mapping,
 	.queue_priority_mapping	= edma_priority_mapping,
+	.default_queue		= EVENTQ_1;
 };
 
 static struct edma_soc_info *tnetv107x_edma_info[EDMA_MAX_CC] = {
diff --git a/arch/arm/mach-davinci/dm355.c b/arch/arm/mach-davinci/dm355.c
index a3a94e9..f1d9c75 100644
--- a/arch/arm/mach-davinci/dm355.c
+++ b/arch/arm/mach-davinci/dm355.c
@@ -591,6 +591,7 @@  static struct edma_soc_info edma_cc0_info = {
 	.n_cc			= 1,
 	.queue_tc_mapping	= queue_tc_mapping,
 	.queue_priority_mapping	= queue_priority_mapping,
+	.default_queue		= EVENTQ_1;
 };
 
 static struct edma_soc_info *dm355_edma_info[EDMA_MAX_CC] = {
diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach-davinci/dm644x.c
index 4c82c27..9971990 100644
--- a/arch/arm/mach-davinci/dm644x.c
+++ b/arch/arm/mach-davinci/dm644x.c
@@ -514,6 +514,7 @@  static struct edma_soc_info edma_cc0_info = {
 	.n_cc			= 1,
 	.queue_tc_mapping	= queue_tc_mapping,
 	.queue_priority_mapping	= queue_priority_mapping,
+	.default_queue		= EVENTQ_1;
 };
 
 static struct edma_soc_info *dm644x_edma_info[EDMA_MAX_CC] = {
diff --git a/arch/arm/mach-davinci/dm646x.c b/arch/arm/mach-davinci/dm646x.c
index 1e0f809..a40372d 100644
--- a/arch/arm/mach-davinci/dm646x.c
+++ b/arch/arm/mach-davinci/dm646x.c
@@ -551,6 +551,7 @@  static struct edma_soc_info edma_cc0_info = {
 	.n_cc			= 1,
 	.queue_tc_mapping	= dm646x_queue_tc_mapping,
 	.queue_priority_mapping	= dm646x_queue_priority_mapping,
+	.default_queue		= EVENTQ_1;
 };
 
 static struct edma_soc_info *dm646x_edma_info[EDMA_MAX_CC] = {
diff --git a/arch/arm/mach-davinci/dma.c b/arch/arm/mach-davinci/dma.c
index 6b96698..6ba4191 100644
--- a/arch/arm/mach-davinci/dma.c
+++ b/arch/arm/mach-davinci/dma.c
@@ -1450,8 +1450,6 @@  static int __init edma_probe(struct platform_device *pdev)
 							EDMA_MAX_CC);
 
 		edma_cc[j]->default_queue = info[j]->default_queue;
-		if (!edma_cc[j]->default_queue)
-			edma_cc[j]->default_queue = EVENTQ_1;
 
 		dev_dbg(&pdev->dev, "DMA REG BASE ADDR=%p\n",
 			edmacc_regs_base[j]);