Re: [PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
On Fri, 2015-08-07 at 11:19 +0800, Chenhui Zhao wrote: > On Fri, Aug 7, 2015 at 2:02 AM, Scott Wood > wrote: > > On Thu, 2015-08-06 at 13:54 +0800, Chenhui Zhao wrote: > > > On Thu, Aug 6, 2015 at 1:46 PM, Scott Wood > > > wrote: > > > > On Thu, 2015-08-06 at 12:20 +0800, Chenhui Zhao wrote: > > > > > On Thu, Aug 6, 2015 at 10:57 AM, Scott Wood > > > > > > > > > > wrote: > > > > > > On Wed, 2015-08-05 at 18:11 +0800, Chenhui Zhao wrote: > > > > > > > On Tue, Aug 4, 2015 at 4:26 AM, Scott Wood > > > > > > > > > > > > wrote: > > > > > > > > On Mon, 2015-08-03 at 19:32 +0800, Chenhui Zhao wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > On Sat, Aug 1, 2015 at 7:59 AM, Scott Wood > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > 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. > > > > > > > > > > > > > > > > So why do we need to set those bits in RCPM? Is it just > > > > > caution? > > > > > > > > > > > > > > Setting these bits can mask interrupts signalled to RCPM > > > from > > > > > MPIC > > > > > > > as a > > > > > > > means of > > > > > > > waking up from a lower power state. So, cores will not be > > > > > waked up > > > > > > > unexpectedly. > > > > > > > > > > > > Why would the MPIC be signalling those interrupts if they've > > > been > > > > > > masked at > > > > > > the MPIC? > > > > > > > > > > > > -Scott > > > > > > > > > > > > > > > > The interrupts to RCPM from MPIC are IRQ, Machine Check, NMI > > > and > > > > > Critical interrupts. Some of them didn't be masked in MPIC. > > > > > > > > What interrupt could actually happen to a sleeping cpu that this > > > > protects > > > > against? > > > > > > > > -Scott > > > > > > Not sure. Maybe spurious interrupts or hardware exceptions. > > > > Spurious interrupts happen due to race conditions. They don't happen > > because > > the MPIC is bored and decides to ring a CPU's doorbell and hide in > > the bushes. > > > > If by "hardware exceptions" you mean machine checks, how would such a > > machine > > check be generated by a core that is off? > > > > > However, setting them make sure dead cpus can not be waked up > > > unexpectedly. > > > > I'm not seeing enough value here to warrant resurrecting the old > > sleep node > > stuff. > > > > -Scott > > My guess maybe not accurate. My point is that electronic parts don't > always work as expected. Taking preventative measures can make the > system more robust. In addition, this step is required in deep sleep > procedure. The deep sleep part is more convincing -- so MPIC masking is not effective during deep sleep? -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/
Re: [PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
On Fri, 2015-08-07 at 11:19 +0800, Chenhui Zhao wrote: On Fri, Aug 7, 2015 at 2:02 AM, Scott Wood scottw...@freescale.com wrote: On Thu, 2015-08-06 at 13:54 +0800, Chenhui Zhao wrote: On Thu, Aug 6, 2015 at 1:46 PM, Scott Wood scottw...@freescale.com wrote: On Thu, 2015-08-06 at 12:20 +0800, Chenhui Zhao wrote: On Thu, Aug 6, 2015 at 10:57 AM, Scott Wood scottw...@freescale.com wrote: On Wed, 2015-08-05 at 18:11 +0800, Chenhui Zhao wrote: On Tue, Aug 4, 2015 at 4:26 AM, Scott Wood scottw...@freescale.com wrote: On Mon, 2015-08-03 at 19:32 +0800, Chenhui Zhao wrote: On Sat, Aug 1, 2015 at 7:59 AM, Scott Wood scottw...@freescale.com wrote: 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. So why do we need to set those bits in RCPM? Is it just caution? Setting these bits can mask interrupts signalled to RCPM from MPIC as a means of waking up from a lower power state. So, cores will not be waked up unexpectedly. Why would the MPIC be signalling those interrupts if they've been masked at the MPIC? -Scott The interrupts to RCPM from MPIC are IRQ, Machine Check, NMI and Critical interrupts. Some of them didn't be masked in MPIC. What interrupt could actually happen to a sleeping cpu that this protects against? -Scott Not sure. Maybe spurious interrupts or hardware exceptions. Spurious interrupts happen due to race conditions. They don't happen because the MPIC is bored and decides to ring a CPU's doorbell and hide in the bushes. If by hardware exceptions you mean machine checks, how would such a machine check be generated by a core that is off? However, setting them make sure dead cpus can not be waked up unexpectedly. I'm not seeing enough value here to warrant resurrecting the old sleep node stuff. -Scott My guess maybe not accurate. My point is that electronic parts don't always work as expected. Taking preventative measures can make the system more robust. In addition, this step is required in deep sleep procedure. The deep sleep part is more convincing -- so MPIC masking is not effective during deep sleep? -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/
Re: [PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
On Fri, Aug 7, 2015 at 2:02 AM, Scott Wood wrote: On Thu, 2015-08-06 at 13:54 +0800, Chenhui Zhao wrote: On Thu, Aug 6, 2015 at 1:46 PM, Scott Wood wrote: > On Thu, 2015-08-06 at 12:20 +0800, Chenhui Zhao wrote: > > On Thu, Aug 6, 2015 at 10:57 AM, Scott Wood > > > > wrote: > > > On Wed, 2015-08-05 at 18:11 +0800, Chenhui Zhao wrote: > > > > On Tue, Aug 4, 2015 at 4:26 AM, Scott Wood > > > > > > wrote: > > > > > On Mon, 2015-08-03 at 19:32 +0800, Chenhui Zhao wrote: > > > > > > > > > > > > > > > > > > On Sat, Aug 1, 2015 at 7:59 AM, Scott Wood > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > 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. > > > > > > > > > > So why do we need to set those bits in RCPM? Is it just > > caution? > > > > > > > > Setting these bits can mask interrupts signalled to RCPM from > > MPIC > > > > as a > > > > means of > > > > waking up from a lower power state. So, cores will not be > > waked up > > > > unexpectedly. > > > > > > Why would the MPIC be signalling those interrupts if they've been > > > masked at > > > the MPIC? > > > > > > -Scott > > > > > > > The interrupts to RCPM from MPIC are IRQ, Machine Check, NMI and > > Critical interrupts. Some of them didn't be masked in MPIC. > > What interrupt could actually happen to a sleeping cpu that this > protects > against? > > -Scott Not sure. Maybe spurious interrupts or hardware exceptions. Spurious interrupts happen due to race conditions. They don't happen because the MPIC is bored and decides to ring a CPU's doorbell and hide in the bushes. If by "hardware exceptions" you mean machine checks, how would such a machine check be generated by a core that is off? However, setting them make sure dead cpus can not be waked up unexpectedly. I'm not seeing enough value here to warrant resurrecting the old sleep node stuff. -Scott My guess maybe not accurate. My point is that electronic parts don't always work as expected. Taking preventative measures can make the system more robust. In addition, this step is required in deep sleep procedure. -Chenhui -- 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/
Re: [PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
On Thu, 2015-08-06 at 13:54 +0800, Chenhui Zhao wrote: > On Thu, Aug 6, 2015 at 1:46 PM, Scott Wood > wrote: > > On Thu, 2015-08-06 at 12:20 +0800, Chenhui Zhao wrote: > > > On Thu, Aug 6, 2015 at 10:57 AM, Scott Wood > > > > > > wrote: > > > > On Wed, 2015-08-05 at 18:11 +0800, Chenhui Zhao wrote: > > > > > On Tue, Aug 4, 2015 at 4:26 AM, Scott Wood > > > > > > > > wrote: > > > > > > On Mon, 2015-08-03 at 19:32 +0800, Chenhui Zhao wrote: > > > > > > > > > > > > > > > > > > > > > On Sat, Aug 1, 2015 at 7:59 AM, Scott Wood > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > 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. > > > > > > > > > > > > So why do we need to set those bits in RCPM? Is it just > > > caution? > > > > > > > > > > Setting these bits can mask interrupts signalled to RCPM from > > > MPIC > > > > > as a > > > > > means of > > > > > waking up from a lower power state. So, cores will not be > > > waked up > > > > > unexpectedly. > > > > > > > > Why would the MPIC be signalling those interrupts if they've been > > > > masked at > > > > the MPIC? > > > > > > > > -Scott > > > > > > > > > > The interrupts to RCPM from MPIC are IRQ, Machine Check, NMI and > > > Critical interrupts. Some of them didn't be masked in MPIC. > > > > What interrupt could actually happen to a sleeping cpu that this > > protects > > against? > > > > -Scott > > Not sure. Maybe spurious interrupts or hardware exceptions. Spurious interrupts happen due to race conditions. They don't happen because the MPIC is bored and decides to ring a CPU's doorbell and hide in the bushes. If by "hardware exceptions" you mean machine checks, how would such a machine check be generated by a core that is off? > However, setting them make sure dead cpus can not be waked up unexpectedly. I'm not seeing enough value here to warrant resurrecting the old sleep node stuff. -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/
Re: [PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
On Fri, Aug 7, 2015 at 2:02 AM, Scott Wood scottw...@freescale.com wrote: On Thu, 2015-08-06 at 13:54 +0800, Chenhui Zhao wrote: On Thu, Aug 6, 2015 at 1:46 PM, Scott Wood scottw...@freescale.com wrote: On Thu, 2015-08-06 at 12:20 +0800, Chenhui Zhao wrote: On Thu, Aug 6, 2015 at 10:57 AM, Scott Wood scottw...@freescale.com wrote: On Wed, 2015-08-05 at 18:11 +0800, Chenhui Zhao wrote: On Tue, Aug 4, 2015 at 4:26 AM, Scott Wood scottw...@freescale.com wrote: On Mon, 2015-08-03 at 19:32 +0800, Chenhui Zhao wrote: On Sat, Aug 1, 2015 at 7:59 AM, Scott Wood scottw...@freescale.com wrote: 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. So why do we need to set those bits in RCPM? Is it just caution? Setting these bits can mask interrupts signalled to RCPM from MPIC as a means of waking up from a lower power state. So, cores will not be waked up unexpectedly. Why would the MPIC be signalling those interrupts if they've been masked at the MPIC? -Scott The interrupts to RCPM from MPIC are IRQ, Machine Check, NMI and Critical interrupts. Some of them didn't be masked in MPIC. What interrupt could actually happen to a sleeping cpu that this protects against? -Scott Not sure. Maybe spurious interrupts or hardware exceptions. Spurious interrupts happen due to race conditions. They don't happen because the MPIC is bored and decides to ring a CPU's doorbell and hide in the bushes. If by hardware exceptions you mean machine checks, how would such a machine check be generated by a core that is off? However, setting them make sure dead cpus can not be waked up unexpectedly. I'm not seeing enough value here to warrant resurrecting the old sleep node stuff. -Scott My guess maybe not accurate. My point is that electronic parts don't always work as expected. Taking preventative measures can make the system more robust. In addition, this step is required in deep sleep procedure. -Chenhui -- 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/
Re: [PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
On Thu, 2015-08-06 at 13:54 +0800, Chenhui Zhao wrote: On Thu, Aug 6, 2015 at 1:46 PM, Scott Wood scottw...@freescale.com wrote: On Thu, 2015-08-06 at 12:20 +0800, Chenhui Zhao wrote: On Thu, Aug 6, 2015 at 10:57 AM, Scott Wood scottw...@freescale.com wrote: On Wed, 2015-08-05 at 18:11 +0800, Chenhui Zhao wrote: On Tue, Aug 4, 2015 at 4:26 AM, Scott Wood scottw...@freescale.com wrote: On Mon, 2015-08-03 at 19:32 +0800, Chenhui Zhao wrote: On Sat, Aug 1, 2015 at 7:59 AM, Scott Wood scottw...@freescale.com wrote: 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. So why do we need to set those bits in RCPM? Is it just caution? Setting these bits can mask interrupts signalled to RCPM from MPIC as a means of waking up from a lower power state. So, cores will not be waked up unexpectedly. Why would the MPIC be signalling those interrupts if they've been masked at the MPIC? -Scott The interrupts to RCPM from MPIC are IRQ, Machine Check, NMI and Critical interrupts. Some of them didn't be masked in MPIC. What interrupt could actually happen to a sleeping cpu that this protects against? -Scott Not sure. Maybe spurious interrupts or hardware exceptions. Spurious interrupts happen due to race conditions. They don't happen because the MPIC is bored and decides to ring a CPU's doorbell and hide in the bushes. If by hardware exceptions you mean machine checks, how would such a machine check be generated by a core that is off? However, setting them make sure dead cpus can not be waked up unexpectedly. I'm not seeing enough value here to warrant resurrecting the old sleep node stuff. -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/
Re: [PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
On Thu, Aug 6, 2015 at 1:46 PM, Scott Wood wrote: On Thu, 2015-08-06 at 12:20 +0800, Chenhui Zhao wrote: On Thu, Aug 6, 2015 at 10:57 AM, Scott Wood wrote: > On Wed, 2015-08-05 at 18:11 +0800, Chenhui Zhao wrote: > > On Tue, Aug 4, 2015 at 4:26 AM, Scott Wood > > wrote: > > > On Mon, 2015-08-03 at 19:32 +0800, Chenhui Zhao wrote: > > > > > > > > > > > > On Sat, Aug 1, 2015 at 7:59 AM, Scott Wood > > > > > > wrote: > > > > > > > > > > > > > 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. > > > > > > So why do we need to set those bits in RCPM? Is it just caution? > > > > Setting these bits can mask interrupts signalled to RCPM from MPIC > > as a > > means of > > waking up from a lower power state. So, cores will not be waked up > > unexpectedly. > > Why would the MPIC be signalling those interrupts if they've been > masked at > the MPIC? > > -Scott > The interrupts to RCPM from MPIC are IRQ, Machine Check, NMI and Critical interrupts. Some of them didn't be masked in MPIC. What interrupt could actually happen to a sleeping cpu that this protects against? -Scott Not sure. Maybe spurious interrupts or hardware exceptions. However, setting them make sure dead cpus can not be waked up unexpectedly. -Chenhui -- 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/
Re: [PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
On Thu, 2015-08-06 at 12:20 +0800, Chenhui Zhao wrote: > On Thu, Aug 6, 2015 at 10:57 AM, Scott Wood > wrote: > > On Wed, 2015-08-05 at 18:11 +0800, Chenhui Zhao wrote: > > > On Tue, Aug 4, 2015 at 4:26 AM, Scott Wood > > > wrote: > > > > On Mon, 2015-08-03 at 19:32 +0800, Chenhui Zhao wrote: > > > > > > > > > > > > > > > On Sat, Aug 1, 2015 at 7:59 AM, Scott Wood > > > > > > > > wrote: > > > > > > > > > > > > > > > > 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. > > > > > > > > So why do we need to set those bits in RCPM? Is it just caution? > > > > > > Setting these bits can mask interrupts signalled to RCPM from MPIC > > > as a > > > means of > > > waking up from a lower power state. So, cores will not be waked up > > > unexpectedly. > > > > Why would the MPIC be signalling those interrupts if they've been > > masked at > > the MPIC? > > > > -Scott > > > > The interrupts to RCPM from MPIC are IRQ, Machine Check, NMI and > Critical interrupts. Some of them didn't be masked in MPIC. What interrupt could actually happen to a sleeping cpu that this protects against? -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/
Re: [PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
On Thu, Aug 6, 2015 at 10:57 AM, Scott Wood wrote: On Wed, 2015-08-05 at 18:11 +0800, Chenhui Zhao wrote: On Tue, Aug 4, 2015 at 4:26 AM, Scott Wood wrote: > On Mon, 2015-08-03 at 19:32 +0800, Chenhui Zhao wrote: > > > > > > On Sat, Aug 1, 2015 at 7:59 AM, Scott Wood > > wrote: > > > > > > > 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. > > So why do we need to set those bits in RCPM? Is it just caution? Setting these bits can mask interrupts signalled to RCPM from MPIC as a means of waking up from a lower power state. So, cores will not be waked up unexpectedly. Why would the MPIC be signalling those interrupts if they've been masked at the MPIC? -Scott The interrupts to RCPM from MPIC are IRQ, Machine Check, NMI and Critical interrupts. Some of them didn't be masked in MPIC. -Chenhui -- 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/
Re: [PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
On Wed, 2015-08-05 at 18:11 +0800, Chenhui Zhao wrote: > On Tue, Aug 4, 2015 at 4:26 AM, Scott Wood > wrote: > > On Mon, 2015-08-03 at 19:32 +0800, Chenhui Zhao wrote: > > > > > > > > > On Sat, Aug 1, 2015 at 7:59 AM, Scott Wood > > > wrote: > > > > > > > > > > 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. > > > > So why do we need to set those bits in RCPM? Is it just caution? > > Setting these bits can mask interrupts signalled to RCPM from MPIC as a > means of > waking up from a lower power state. So, cores will not be waked up > unexpectedly. Why would the MPIC be signalling those interrupts if they've been masked at the MPIC? -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/
Re: [PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
On Tue, Aug 4, 2015 at 4:26 AM, Scott Wood
wrote:
On Mon, 2015-08-03 at 19:32 +0800, Chenhui Zhao wrote:
>
On Sat, Aug 1, 2015 at 7:59 AM, Scott Wood
wrote:
>
> 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.
So why do we need to set those bits in RCPM? Is it just caution?
Setting these bits can mask interrupts signalled to RCPM from MPIC as a
means of
waking up from a lower power state. So, cores will not be waked up
unexpectedly.
> @@ -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.
What stops a user from trying to use cpu hotplug on unsupported cpus,
or in a
virtualized environment, and crashing here?
-Scott
Will set these callback functions only if qoriq_pm_ops is valid.
-Chenhui
--
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/
Re: [PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
On Tue, Aug 4, 2015 at 4:26 AM, Scott Wood scottw...@freescale.com
wrote:
On Mon, 2015-08-03 at 19:32 +0800, Chenhui Zhao wrote:
On Sat, Aug 1, 2015 at 7:59 AM, Scott Wood scottw...@freescale.com
wrote:
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.
So why do we need to set those bits in RCPM? Is it just caution?
Setting these bits can mask interrupts signalled to RCPM from MPIC as a
means of
waking up from a lower power state. So, cores will not be waked up
unexpectedly.
@@ -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.
What stops a user from trying to use cpu hotplug on unsupported cpus,
or in a
virtualized environment, and crashing here?
-Scott
Will set these callback functions only if qoriq_pm_ops is valid.
-Chenhui
--
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/
Re: [PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
On Thu, Aug 6, 2015 at 1:46 PM, Scott Wood scottw...@freescale.com wrote: On Thu, 2015-08-06 at 12:20 +0800, Chenhui Zhao wrote: On Thu, Aug 6, 2015 at 10:57 AM, Scott Wood scottw...@freescale.com wrote: On Wed, 2015-08-05 at 18:11 +0800, Chenhui Zhao wrote: On Tue, Aug 4, 2015 at 4:26 AM, Scott Wood scottw...@freescale.com wrote: On Mon, 2015-08-03 at 19:32 +0800, Chenhui Zhao wrote: On Sat, Aug 1, 2015 at 7:59 AM, Scott Wood scottw...@freescale.com wrote: 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. So why do we need to set those bits in RCPM? Is it just caution? Setting these bits can mask interrupts signalled to RCPM from MPIC as a means of waking up from a lower power state. So, cores will not be waked up unexpectedly. Why would the MPIC be signalling those interrupts if they've been masked at the MPIC? -Scott The interrupts to RCPM from MPIC are IRQ, Machine Check, NMI and Critical interrupts. Some of them didn't be masked in MPIC. What interrupt could actually happen to a sleeping cpu that this protects against? -Scott Not sure. Maybe spurious interrupts or hardware exceptions. However, setting them make sure dead cpus can not be waked up unexpectedly. -Chenhui -- 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/
Re: [PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
On Thu, Aug 6, 2015 at 10:57 AM, Scott Wood scottw...@freescale.com wrote: On Wed, 2015-08-05 at 18:11 +0800, Chenhui Zhao wrote: On Tue, Aug 4, 2015 at 4:26 AM, Scott Wood scottw...@freescale.com wrote: On Mon, 2015-08-03 at 19:32 +0800, Chenhui Zhao wrote: On Sat, Aug 1, 2015 at 7:59 AM, Scott Wood scottw...@freescale.com wrote: 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. So why do we need to set those bits in RCPM? Is it just caution? Setting these bits can mask interrupts signalled to RCPM from MPIC as a means of waking up from a lower power state. So, cores will not be waked up unexpectedly. Why would the MPIC be signalling those interrupts if they've been masked at the MPIC? -Scott The interrupts to RCPM from MPIC are IRQ, Machine Check, NMI and Critical interrupts. Some of them didn't be masked in MPIC. -Chenhui -- 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/
Re: [PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
On Thu, 2015-08-06 at 12:20 +0800, Chenhui Zhao wrote: On Thu, Aug 6, 2015 at 10:57 AM, Scott Wood scottw...@freescale.com wrote: On Wed, 2015-08-05 at 18:11 +0800, Chenhui Zhao wrote: On Tue, Aug 4, 2015 at 4:26 AM, Scott Wood scottw...@freescale.com wrote: On Mon, 2015-08-03 at 19:32 +0800, Chenhui Zhao wrote: On Sat, Aug 1, 2015 at 7:59 AM, Scott Wood scottw...@freescale.com wrote: 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. So why do we need to set those bits in RCPM? Is it just caution? Setting these bits can mask interrupts signalled to RCPM from MPIC as a means of waking up from a lower power state. So, cores will not be waked up unexpectedly. Why would the MPIC be signalling those interrupts if they've been masked at the MPIC? -Scott The interrupts to RCPM from MPIC are IRQ, Machine Check, NMI and Critical interrupts. Some of them didn't be masked in MPIC. What interrupt could actually happen to a sleeping cpu that this protects against? -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/
Re: [PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
On Wed, 2015-08-05 at 18:11 +0800, Chenhui Zhao wrote: On Tue, Aug 4, 2015 at 4:26 AM, Scott Wood scottw...@freescale.com wrote: On Mon, 2015-08-03 at 19:32 +0800, Chenhui Zhao wrote: On Sat, Aug 1, 2015 at 7:59 AM, Scott Wood scottw...@freescale.com wrote: 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. So why do we need to set those bits in RCPM? Is it just caution? Setting these bits can mask interrupts signalled to RCPM from MPIC as a means of waking up from a lower power state. So, cores will not be waked up unexpectedly. Why would the MPIC be signalling those interrupts if they've been masked at the MPIC? -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/
Re: [PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
On Mon, 2015-08-03 at 19:32 +0800, Chenhui Zhao wrote:
> >
> On Sat, Aug 1, 2015 at 7:59 AM, Scott Wood
> wrote:
> >
> > 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.
So why do we need to set those bits in RCPM? Is it just caution?
> > @@ -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.
What stops a user from trying to use cpu hotplug on unsupported cpus, or in a
virtualized environment, and crashing here?
-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/
Re: [PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
On Sat, Aug 1, 2015 at 7:59 AM, Scott Wood
wrote:
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.
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
--
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/
Re: [PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
On Sat, Aug 1, 2015 at 7:59 AM, Scott Wood scottw...@freescale.com
wrote:
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.
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
--
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/
Re: [PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
On Mon, 2015-08-03 at 19:32 +0800, Chenhui Zhao wrote:
On Sat, Aug 1, 2015 at 7:59 AM, Scott Wood scottw...@freescale.com
wrote:
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.
So why do we need to set those bits in RCPM? Is it just caution?
@@ -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.
What stops a user from trying to use cpu hotplug on unsupported cpus, or in a
virtualized environment, and crashing here?
-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/
Re: [PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
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/
Re: [PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
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/
