[PATCH 1/2] ARM: OMAP: DMA: Fix DMA channel end definition for OMAP36xx-PLUS devices

[PATCH 1/2] ARM: OMAP: DMA: Fix DMA channel end definition for OMAP36xx-PLUS devices

[Jon Hunter]

From: Jon Hunter <jon-hunter@ti.com>

I recently noticed there are a couple bugs in the OMAP2-PLUS DMA driver code.

1. The CCDN DMA register is valid for all OMAP devices from OMAP3630 onwards.
   For OMAP3630+ devices the CCDN register is the upper register in the DMA
   register map and so is used to define the channel end address for a given
   DMA channel. Today the driver code is written to only use the CCDN as the
   upper address for OMAP3630 and OMAP4430 devices where as it should be used
   for all OMAP3630+ devices. Therefore use the CCDN register as the upper
   address in the DMA register map for devices OMAP3630 and beyond.

2. The OMAP2 DMA driver is using the macro cpu_is_omap4430() and from reviewing
   the plat/cpu.h file I see that cpu_is_omap4430() is defined as 0 even when
   CONFIG_ARCH_OMAP4 is defined. This is another bug in of itself (addressed in
   the 2nd patch of this series), but the DMA driver should not be using this.
   Hence, there is a dependency between this patch and the next.

3. Finally, update the comment for the registers CDP, CNDP and CCDP as registers
   for OMAP3630-PLUS devices and not just OMAP4 devices.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/dma.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/dma.c b/arch/arm/mach-omap2/dma.c
index a59a45a..4fd26b4 100644
--- a/arch/arm/mach-omap2/dma.c
+++ b/arch/arm/mach-omap2/dma.c
@@ -81,7 +81,7 @@ static u16 reg_map[] = {
 	[CCFN]			= 0xc0,
 	[COLOR]			= 0xc4,
 
-	/* OMAP4 specific registers */
+	/* OMAP36XX-PLUS specific registers */
 	[CDP]			= 0xd0,
 	[CNDP]			= 0xd4,
 	[CCDN]			= 0xd8,
@@ -227,7 +227,7 @@ static int __init omap2_system_dma_init_dev(struct omap_hwmod *oh, void *unused)
 
 	dma_stride		= OMAP2_DMA_STRIDE;
 	dma_common_ch_start	= CSDP;
-	if (cpu_is_omap3630() || cpu_is_omap4430())
+	if (!cpu_is_omap24xx() && !cpu_is_omap3430())
 		dma_common_ch_end = CCDN;
 	else
 		dma_common_ch_end = CCFN;
-- 
1.7.1

Re: [PATCH 1/2] ARM: OMAP: DMA: Fix DMA channel end definition for OMAP36xx-PLUS devices

[Jon Hunter]

Hi Tony,

On 3/1/2012 19:10, Jon Hunter wrote:
> From: Jon Hunter<jon-hunter@ti.com>
>
> I recently noticed there are a couple bugs in the OMAP2-PLUS DMA driver code.
>
> 1. The CCDN DMA register is valid for all OMAP devices from OMAP3630 onwards.
>     For OMAP3630+ devices the CCDN register is the upper register in the DMA
>     register map and so is used to define the channel end address for a given
>     DMA channel. Today the driver code is written to only use the CCDN as the
>     upper address for OMAP3630 and OMAP4430 devices where as it should be used
>     for all OMAP3630+ devices. Therefore use the CCDN register as the upper
>     address in the DMA register map for devices OMAP3630 and beyond.
>
> 2. The OMAP2 DMA driver is using the macro cpu_is_omap4430() and from reviewing
>     the plat/cpu.h file I see that cpu_is_omap4430() is defined as 0 even when
>     CONFIG_ARCH_OMAP4 is defined. This is another bug in of itself (addressed in
>     the 2nd patch of this series), but the DMA driver should not be using this.
>     Hence, there is a dependency between this patch and the next.
>
> 3. Finally, update the comment for the registers CDP, CNDP and CCDP as registers
>     for OMAP3630-PLUS devices and not just OMAP4 devices.
>
> Signed-off-by: Jon Hunter<jon-hunter@ti.com>
> ---
>   arch/arm/mach-omap2/dma.c |    4 ++--
>   1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/dma.c b/arch/arm/mach-omap2/dma.c
> index a59a45a..4fd26b4 100644
> --- a/arch/arm/mach-omap2/dma.c
> +++ b/arch/arm/mach-omap2/dma.c
> @@ -81,7 +81,7 @@ static u16 reg_map[] = {
>   	[CCFN]			= 0xc0,
>   	[COLOR]			= 0xc4,
>
> -	/* OMAP4 specific registers */
> +	/* OMAP36XX-PLUS specific registers */
>   	[CDP]			= 0xd0,
>   	[CNDP]			= 0xd4,
>   	[CCDN]			= 0xd8,
> @@ -227,7 +227,7 @@ static int __init omap2_system_dma_init_dev(struct omap_hwmod *oh, void *unused)
>
>   	dma_stride		= OMAP2_DMA_STRIDE;
>   	dma_common_ch_start	= CSDP;
> -	if (cpu_is_omap3630() || cpu_is_omap4430())
> +	if (!cpu_is_omap24xx()&&  !cpu_is_omap3430())
>   		dma_common_ch_end = CCDN;
>   	else
>   		dma_common_ch_end = CCFN;

Oops! I see that Peter has already fixed this one [1].

Peter, looking at your fix it is good for OMAP4 but should we also fix 
for OMAP5 while we are at it? I checked and the register map is the same 
for OMAP5.

Tony, I still think we should get rid of cpu_is_omap4430() altogether [2].

Cheers
Jon

[1] http://www.spinics.net/lists/linux-omap/msg65459.html
[2] http://www.spinics.net/lists/linux-omap/msg65702.html

Re: [PATCH 1/2] ARM: OMAP: DMA: Fix DMA channel end definition for OMAP36xx-PLUS devices

[Tony Lindgren]

* Jon Hunter <jon-hunter@ti.com> [120302 14:47]:
> 
> Oops! I see that Peter has already fixed this one [1].
> 
> Peter, looking at your fix it is good for OMAP4 but should we also
> fix for OMAP5 while we are at it? I checked and the register map is
> the same for OMAP5.
> 
> Tony, I still think we should get rid of cpu_is_omap4430() altogether [2].

OK, makes sense. Applying [2] into fixes-non-critical-part2.

Thanks,

Tony 


> [1] http://www.spinics.net/lists/linux-omap/msg65459.html
> [2] http://www.spinics.net/lists/linux-omap/msg65702.html