On Fri, 2015-07-31 at 17:20 +0800, b29...@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.
> #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.
> {
> - 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?
Could you explain irq_mask()? Why would there still be IRQs destined for
this CPU at this point?
@@ -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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
