Messages in this thread Patch in this message | | | Date | Mon, 3 Aug 2015 19:32:58 +0800 | From | Chenhui Zhao <> | Subject | Re: [PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations |
| |
On Sat, Aug 1, 2015 at 7:59 AM, Scott Wood <scottwood@freescale.com> wrote: > On Fri, 2015-07-31 at 17:20 +0800, b29983@freescale.com wrote: >> @@ -71,7 +56,7 @@ static void mpc85xx_give_timebase(void) >> barrier(); >> tb_req = 0; >> >> - mpc85xx_timebase_freeze(1); >> + qoriq_pm_ops->freeze_time_base(1); > > freeze_time_base() takes a bool. Use true/false.
OK.
> > >> #ifdef CONFIG_PPC64 >> /* >> * e5500/e6500 have a workaround for erratum A-006958 in place >> @@ -104,7 +89,7 @@ static void mpc85xx_give_timebase(void) >> while (tb_valid) >> barrier(); >> >> - mpc85xx_timebase_freeze(0); >> + qoriq_pm_ops->freeze_time_base(0); >> >> local_irq_restore(flags); >> } >> @@ -127,20 +112,10 @@ static void mpc85xx_take_timebase(void) >> } >> >> #ifdef CONFIG_HOTPLUG_CPU >> -static void smp_85xx_mach_cpu_die(void) >> +static void e500_cpu_idle(void) > > This is not the function that gets called during normal cpu idle, and > it > shouldn't be named to look like it is.
Sorry, it's a typo. It shoule be "e500_cpu_die".
> >> { >> - unsigned int cpu = smp_processor_id(); >> u32 tmp; >> >> - local_irq_disable(); >> - idle_task_exit(); >> - generic_set_cpu_dead(cpu); >> - mb(); >> - >> - mtspr(SPRN_TCR, 0); >> - >> - cur_cpu_spec->cpu_down_flush(); >> - >> tmp = (mfspr(SPRN_HID0) & ~(HID0_DOZE|HID0_SLEEP)) | HID0_NAP; >> mtspr(SPRN_HID0, tmp); >> isync(); >> @@ -151,6 +126,25 @@ static void smp_85xx_mach_cpu_die(void) >> mb(); >> mtmsr(tmp); >> isync(); >> +} >> + >> +static void qoriq_cpu_dying(void) >> +{ >> + unsigned int cpu = smp_processor_id(); >> + >> + hard_irq_disable(); >> + /* mask all irqs to prevent cpu wakeup */ >> + qoriq_pm_ops->irq_mask(cpu); >> + idle_task_exit(); >> + >> + mtspr(SPRN_TCR, 0); >> + mtspr(SPRN_TSR, mfspr(SPRN_TSR)); >> + >> + cur_cpu_spec->cpu_down_flush(); >> + >> + generic_set_cpu_dead(cpu); >> + >> + e500_cpu_idle(); > > Why is something that claims to be applicable to all qoriq directly > calling > an e500v2-specific function?
Added "#ifndef CONFIG_PPC_E500MC" in the following patch.
> > Could you explain irq_mask()? Why would there still be IRQs destined > for > this CPU at this point?
This function just masks irq by setting the registers in RCPM (for example, RCPM_CPMIMR, RCPM_CPMCIMR). Actually, all irqs to this CPU have been migrated to other CPUs.
> > @@ -431,21 +415,9 @@ void __init mpc85xx_smp_init(void) >> smp_85xx_ops.probe = NULL; >> } >> >> - np = of_find_matching_node(NULL, mpc85xx_smp_guts_ids); >> - if (np) { >> - guts = of_iomap(np, 0); >> - of_node_put(np); >> - if (!guts) { >> - pr_err("%s: Could not map guts node >> address\n", >> - >> __func__); >> - return; >> - } >> - smp_85xx_ops.give_timebase = mpc85xx_give_timebase; >> - smp_85xx_ops.take_timebase = mpc85xx_take_timebase; >> #ifdef CONFIG_HOTPLUG_CPU >> - ppc_md.cpu_die = smp_85xx_mach_cpu_die; >> + ppc_md.cpu_die = qoriq_cpu_dying; >> #endif > > Shouldn't you make sure there's a valid qoriq_pm_ops before setting > cpu_die()? Or make sure that qoriq_cpu_dying() works regardless. > > -Scott
This patch is just for e500v2. The following patches will handle the case of e500mc, e5500 and e6500.
-Chenhui
| |