| Message ID | 1381220587-29697-4-git-send-email-josephl@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10/08/2013 02:23 AM, Joseph Lo wrote: > Because the CPU0 was the first up and the last down core when cluster > power up/down or platform suspend. So only CPU0 needs the rest of the > functions to reset flow controller and re-enable SCU and L2. We also > move the L2 init function for Cortex-A15 to there. The secondery CPU > can just call cpu_resume. Is that really true? I thought that starting with Tegra114, all the CPUs were independent, so that any CPU could be the last CPU to be power-gated. Isn't that exactly why we don't need coupled cpuidle or anything similar on Tegra114 > diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S > not_ca9: > + mov32 r9, 0xc0f > + cmp r8, r9 > + bleq tegra_init_l2_for_a15 That's checking whether the CPU type is a Cortex-A15, isn't it? The only CPUs that exist NVIDIA SoCs are Cortex-A9 and Cortex-A15, so I don't see why we need to check whether the CPU is a Cortex-A15, given this label is jumped to only when the CPU isn't a Cortex-A9.
On Wed, 2013-10-09 at 01:00 +0800, Stephen Warren wrote: > On 10/08/2013 02:23 AM, Joseph Lo wrote: > > Because the CPU0 was the first up and the last down core when cluster > > power up/down or platform suspend. So only CPU0 needs the rest of the > > functions to reset flow controller and re-enable SCU and L2. We also > > move the L2 init function for Cortex-A15 to there. The secondery CPU > > can just call cpu_resume. > > Is that really true? I thought that starting with Tegra114, all the CPUs > were independent, so that any CPU could be the last CPU to be > power-gated. Isn't that exactly why we don't need coupled cpuidle or > anything similar on Tegra114 > Yes, it's true. I realize the role of CPU0 is the same across all current Tegra chips. Although we can support independent power gate for CPU0 in Tegra114/124, for cluster power control it still needs to be set up by CPU0 (last down and first up). So I send this patch for tunning the code that only need for CPU0. > > diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S > > > not_ca9: > > + mov32 r9, 0xc0f > > + cmp r8, r9 > > + bleq tegra_init_l2_for_a15 > > That's checking whether the CPU type is a Cortex-A15, isn't it? The only > CPUs that exist NVIDIA SoCs are Cortex-A9 and Cortex-A15, so I don't see > why we need to check whether the CPU is a Cortex-A15, given this label > is jumped to only when the CPU isn't a Cortex-A9. Good catch. Will fix.
On Wed, 2013-10-09 at 11:11 +0800, Joseph Lo wrote: > On Wed, 2013-10-09 at 01:00 +0800, Stephen Warren wrote: > > On 10/08/2013 02:23 AM, Joseph Lo wrote: > > > diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S > > > > > not_ca9: > > > + mov32 r9, 0xc0f > > > + cmp r8, r9 > > > + bleq tegra_init_l2_for_a15 > > > > That's checking whether the CPU type is a Cortex-A15, isn't it? The only > > CPUs that exist NVIDIA SoCs are Cortex-A9 and Cortex-A15, so I don't see > > why we need to check whether the CPU is a Cortex-A15, given this label > > is jumped to only when the CPU isn't a Cortex-A9. > > Good catch. Will fix. > Ah! I just realize the Cortex-A9 will go through here as well. So I still need to code to make sure only Cortex-A15 execute the function.
On 10/09/2013 02:23 AM, Joseph Lo wrote: > On Wed, 2013-10-09 at 11:11 +0800, Joseph Lo wrote: >> On Wed, 2013-10-09 at 01:00 +0800, Stephen Warren wrote: >>> On 10/08/2013 02:23 AM, Joseph Lo wrote: >>>> diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S >>> >>>> not_ca9: >>>> + mov32 r9, 0xc0f >>>> + cmp r8, r9 >>>> + bleq tegra_init_l2_for_a15 >>> >>> That's checking whether the CPU type is a Cortex-A15, isn't it? The only >>> CPUs that exist NVIDIA SoCs are Cortex-A9 and Cortex-A15, so I don't see >>> why we need to check whether the CPU is a Cortex-A15, given this label >>> is jumped to only when the CPU isn't a Cortex-A9. >> >> Good catch. Will fix. >> > Ah! I just realize the Cortex-A9 will go through here as well. So I > still need to code to make sure only Cortex-A15 execute the function. In that case, there's a bug in the label name; A9 CPUs shouldn't execute code that's "not_ca9"...
diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S index f527b2c..b63e69c 100644 --- a/arch/arm/mach-tegra/reset-handler.S +++ b/arch/arm/mach-tegra/reset-handler.S @@ -45,17 +45,11 @@ ENTRY(tegra_resume) check_cpu_part_num 0xc09, r8, r9 bleq v7_invalidate_l1 - blne tegra_init_l2_for_a15 cpu_id r0 - tegra_get_soc_id TEGRA_APB_MISC_BASE, r6 - cmp r6, #TEGRA114 - beq no_cpu0_chk - cmp r0, #0 @ CPU0? THUMB( it ne ) bne cpu_resume @ no -no_cpu0_chk: /* Are we on Tegra20? */ cmp r6, #TEGRA20 @@ -87,6 +81,9 @@ no_cpu0_chk: /* L2 cache resume & re-enable */ l2_cache_resume r0, r1, r2, l2x0_saved_regs_addr not_ca9: + mov32 r9, 0xc0f + cmp r8, r9 + bleq tegra_init_l2_for_a15 b cpu_resume ENDPROC(tegra_resume)
Because the CPU0 was the first up and the last down core when cluster power up/down or platform suspend. So only CPU0 needs the rest of the functions to reset flow controller and re-enable SCU and L2. We also move the L2 init function for Cortex-A15 to there. The secondery CPU can just call cpu_resume. Signed-off-by: Joseph Lo <josephl@nvidia.com> --- arch/arm/mach-tegra/reset-handler.S | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)