lkml.org 
[lkml]   [2015]   [Aug]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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



\
 
 \ /
  Last update: 2015-08-03 13:41    [W:0.164 / U:0.384 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site