diff mbox

ARM: tegra: remove the ifdef of ARCH SoC in the tegra_resume

Message ID 1370247004-31846-1-git-send-email-josephl@nvidia.com (mailing list archive)
State New, archived
Headers show

Commit Message

Joseph Lo June 3, 2013, 8:10 a.m. UTC
Removing the ifdef of ARCH_TEGRA_SoC in the tegra_resume function. Because
we always build with all Tegra SoCs support and had a runtime chip
detection code there. And we expect most of the chips would need the code
in the future.

We also fix a typo of a macro name that cause a build error.

Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
 arch/arm/mach-tegra/reset-handler.S | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Stephen Warren June 3, 2013, 3:26 p.m. UTC | #1
On 06/03/2013 02:10 AM, Joseph Lo wrote:
> Removing the ifdef of ARCH_TEGRA_SoC in the tegra_resume function. Because
> we always build with all Tegra SoCs support and had a runtime chip
> detection code there. And we expect most of the chips would need the code
> in the future.
> 
> We also fix a typo of a macro name that cause a build error.

OK, this fixes the build issue, but this patch raises some questions
about the code...

> diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S

> -#ifndef CONFIG_ARCH_TEGRA_2x_SOC
>  	/* Are we on Tegra20? */
>  	cmp	r6, #TEGRA20
>  	beq	1f				@ Yes

Was that ifdef completely incorrect before? I can see why the cmp/beq
might be ifdef'd (although it's not worth it), but I assume the code
after that beq was intended to run on all chips after Tegra20. The ifdef
as it was written does something rather different; it prevents any of
that code from running unless the kernel doesn't have Tegra20 support.
So, I think that the removal of the ifdef is more of a bug-fix that
"because we always build with all Tegra SoCs support. Let me know, and
I'll re-write the commit description to something more accurate...

>  	/* Clear the flow controller flags for this CPU. */
> -	cpu_to_csr_req r1, r0
> +	cpu_to_csr_reg r1, r0
>  	mov32	r2, TEGRA_FLOW_CTRL_BASE
>  	ldr	r1, [r2, r1]
>  	/* Clear event & intr flag */
> @@ -70,7 +69,6 @@ no_cpu0_chk:
>  	bic	r1, r1, r0
>  	str	r1, [r2]
>  1:
> -#endif
>  
>  	check_cpu_part_num 0xc09, r8, r9
>  	bne	not_ca9
Arnd Bergmann June 3, 2013, 10:48 p.m. UTC | #2
On Monday 03 June 2013, Stephen Warren wrote:
> Was that ifdef completely incorrect before? I can see why the cmp/beq
> might be ifdef'd (although it's not worth it), but I assume the code
> after that beq was intended to run on all chips after Tegra20. The ifdef
> as it was written does something rather different; it prevents any of
> that code from running unless the kernel doesn't have Tegra20 support.
> So, I think that the removal of the ifdef is more of a bug-fix that
> "because we always build with all Tegra SoCs support. Let me know, and
> I'll re-write the commit description to something more accurate...

The #ifdef was recently changed from #if TEGRA3 to #if !TEGRA2, which support
for TEGRA4 was added. I think this is where it broke.

	Arnd
Joseph Lo June 4, 2013, 1:25 a.m. UTC | #3
On Tue, 2013-06-04 at 06:48 +0800, Arnd Bergmann wrote:
> On Monday 03 June 2013, Stephen Warren wrote:
> > Was that ifdef completely incorrect before? I can see why the cmp/beq
> > might be ifdef'd (although it's not worth it), but I assume the code
> > after that beq was intended to run on all chips after Tegra20. The ifdef
> > as it was written does something rather different; it prevents any of
> > that code from running unless the kernel doesn't have Tegra20 support.
> > So, I think that the removal of the ifdef is more of a bug-fix that
> > "because we always build with all Tegra SoCs support. Let me know, and
> > I'll re-write the commit description to something more accurate...
> 
> The #ifdef was recently changed from #if TEGRA3 to #if !TEGRA2, which support
> for TEGRA4 was added. I think this is where it broke.
> 
Yes, exactly. Sorry again for the error code.

Joseph
Stephen Warren June 5, 2013, 5:44 p.m. UTC | #4
On 06/03/2013 02:10 AM, Joseph Lo wrote:
> Removing the ifdef of ARCH_TEGRA_SoC in the tegra_resume function. Because
> we always build with all Tegra SoCs support and had a runtime chip
> detection code there. And we expect most of the chips would need the code
> in the future.
> 
> We also fix a typo of a macro name that cause a build error.

Applied to Tegra's for-3.11/soc, with re-written commit description.
diff mbox

Patch

diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
index d2042ac..39dc9e7 100644
--- a/arch/arm/mach-tegra/reset-handler.S
+++ b/arch/arm/mach-tegra/reset-handler.S
@@ -54,12 +54,11 @@  ENTRY(tegra_resume)
 	bne	cpu_resume			@ no
 no_cpu0_chk:
 
-#ifndef CONFIG_ARCH_TEGRA_2x_SOC
 	/* Are we on Tegra20? */
 	cmp	r6, #TEGRA20
 	beq	1f				@ Yes
 	/* Clear the flow controller flags for this CPU. */
-	cpu_to_csr_req r1, r0
+	cpu_to_csr_reg r1, r0
 	mov32	r2, TEGRA_FLOW_CTRL_BASE
 	ldr	r1, [r2, r1]
 	/* Clear event & intr flag */
@@ -70,7 +69,6 @@  no_cpu0_chk:
 	bic	r1, r1, r0
 	str	r1, [r2]
 1:
-#endif
 
 	check_cpu_part_num 0xc09, r8, r9
 	bne	not_ca9