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/
[PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
From: Tang Yuantian Freescale CoreNet-based and Non-CoreNet-based platforms require different PM operations. This patch extracted existing PM operations on Non-CoreNet-based platforms to a new file which can accommodate both platforms. In this way, PM operation codes are clearer structurally. Signed-off-by: Chenhui Zhao Signed-off-by: Tang Yuantian --- arch/powerpc/platforms/85xx/Makefile | 1 + arch/powerpc/platforms/85xx/common.c | 3 + arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c | 95 arch/powerpc/platforms/85xx/smp.c| 86 + arch/powerpc/sysdev/fsl_rcpm.c | 2 - 5 files changed, 128 insertions(+), 59 deletions(-) create mode 100644 arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile index 1fe7fb9..7bc86da 100644 --- a/arch/powerpc/platforms/85xx/Makefile +++ b/arch/powerpc/platforms/85xx/Makefile @@ -2,6 +2,7 @@ # Makefile for the PowerPC 85xx linux kernel. # obj-$(CONFIG_SMP) += smp.o +obj-$(CONFIG_FSL_PMC)+= mpc85xx_pm_ops.o obj-y += common.o diff --git a/arch/powerpc/platforms/85xx/common.c b/arch/powerpc/platforms/85xx/common.c index 7bfb9b1..91475f5 100644 --- a/arch/powerpc/platforms/85xx/common.c +++ b/arch/powerpc/platforms/85xx/common.c @@ -10,10 +10,13 @@ #include #include +#include #include #include "mpc85xx.h" +const struct fsl_pm_ops *qoriq_pm_ops; + static const struct of_device_id mpc85xx_common_ids[] __initconst = { { .type = "soc", }, { .compatible = "soc", }, diff --git a/arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c b/arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c new file mode 100644 index 000..e59714e --- /dev/null +++ b/arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c @@ -0,0 +1,95 @@ +/* + * MPC85xx PM operators + * + * Copyright 2015 Freescale Semiconductor Inc. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + */ + +#include +#include +#include + +#include +#include +#include + +static struct ccsr_guts __iomem *guts; + +static const struct of_device_id mpc85xx_smp_guts_ids[] = { + { .compatible = "fsl,mpc8572-guts", }, + { .compatible = "fsl,p1020-guts", }, + { .compatible = "fsl,p1021-guts", }, + { .compatible = "fsl,p1022-guts", }, + { .compatible = "fsl,p1023-guts", }, + { .compatible = "fsl,p2020-guts", }, + { .compatible = "fsl,bsc9132-guts", }, + {}, +}; + +static void mpc85xx_irq_mask(int cpu) +{ + +} + +static void mpc85xx_irq_unmask(int cpu) +{ + +} + +static void mpc85xx_cpu_die(int cpu) +{ + +} + +static void mpc85xx_cpu_up(int cpu) +{ + +} + +static void mpc85xx_freeze_time_base(bool freeze) +{ + uint32_t mask; + + mask = CCSR_GUTS_DEVDISR_TB0 | CCSR_GUTS_DEVDISR_TB1; + if (freeze) + setbits32(>devdisr, mask); + else + clrbits32(>devdisr, mask); + + in_be32(>devdisr); +} + +static const struct fsl_pm_ops mpc85xx_pm_ops = { + .freeze_time_base = mpc85xx_freeze_time_base, + .irq_mask = mpc85xx_irq_mask, + .irq_unmask = mpc85xx_irq_unmask, + .cpu_die = mpc85xx_cpu_die, + .cpu_up = mpc85xx_cpu_up, +}; + +static int __init mpc85xx_setup_pmc(void) +{ + struct device_node *np; + + 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 -ENOMEM; + } + } + + qoriq_pm_ops = _pm_ops; + + return 0; +} + +/* need to call this before SMP init */ +early_initcall(mpc85xx_setup_pmc) diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c index 1180f78..6811a5b 100644 --- a/arch/powerpc/platforms/85xx/smp.c +++ b/arch/powerpc/platforms/85xx/smp.c @@ -2,7 +2,7 @@ * Author: Andy Fleming *Kumar Gala * - * Copyright 2006-2008, 2011-2012 Freescale Semiconductor Inc. + * Copyright 2006-2008, 2011-2012, 2015 Freescale Semiconductor Inc. * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License as published by the @@ -15,7 +15,6 @@ #include #include #include -#include #include #include #include @@ -26,9 +25,9 @@ #include #include #include -#include #include #include +#include #include #include @@ -43,24 +42,10 @@ struct epapr_spin_table { u32 pir; }; -static struct ccsr_guts __iomem *guts; static u64
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/
[PATCH 1/3] Powerpc: mpc85xx: refactor the PM operations
From: Tang Yuantian yuantian.t...@freescale.com Freescale CoreNet-based and Non-CoreNet-based platforms require different PM operations. This patch extracted existing PM operations on Non-CoreNet-based platforms to a new file which can accommodate both platforms. In this way, PM operation codes are clearer structurally. Signed-off-by: Chenhui Zhao chenhui.z...@freescale.com Signed-off-by: Tang Yuantian yuantian.t...@feescale.com --- arch/powerpc/platforms/85xx/Makefile | 1 + arch/powerpc/platforms/85xx/common.c | 3 + arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c | 95 arch/powerpc/platforms/85xx/smp.c| 86 + arch/powerpc/sysdev/fsl_rcpm.c | 2 - 5 files changed, 128 insertions(+), 59 deletions(-) create mode 100644 arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile index 1fe7fb9..7bc86da 100644 --- a/arch/powerpc/platforms/85xx/Makefile +++ b/arch/powerpc/platforms/85xx/Makefile @@ -2,6 +2,7 @@ # Makefile for the PowerPC 85xx linux kernel. # obj-$(CONFIG_SMP) += smp.o +obj-$(CONFIG_FSL_PMC)+= mpc85xx_pm_ops.o obj-y += common.o diff --git a/arch/powerpc/platforms/85xx/common.c b/arch/powerpc/platforms/85xx/common.c index 7bfb9b1..91475f5 100644 --- a/arch/powerpc/platforms/85xx/common.c +++ b/arch/powerpc/platforms/85xx/common.c @@ -10,10 +10,13 @@ #include linux/of_platform.h #include asm/qe.h +#include asm/fsl_pm.h #include sysdev/cpm2_pic.h #include mpc85xx.h +const struct fsl_pm_ops *qoriq_pm_ops; + static const struct of_device_id mpc85xx_common_ids[] __initconst = { { .type = soc, }, { .compatible = soc, }, diff --git a/arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c b/arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c new file mode 100644 index 000..e59714e --- /dev/null +++ b/arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c @@ -0,0 +1,95 @@ +/* + * MPC85xx PM operators + * + * Copyright 2015 Freescale Semiconductor Inc. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + */ + +#include linux/kernel.h +#include linux/of.h +#include linux/of_address.h + +#include asm/io.h +#include asm/fsl_guts.h +#include asm/fsl_pm.h + +static struct ccsr_guts __iomem *guts; + +static const struct of_device_id mpc85xx_smp_guts_ids[] = { + { .compatible = fsl,mpc8572-guts, }, + { .compatible = fsl,p1020-guts, }, + { .compatible = fsl,p1021-guts, }, + { .compatible = fsl,p1022-guts, }, + { .compatible = fsl,p1023-guts, }, + { .compatible = fsl,p2020-guts, }, + { .compatible = fsl,bsc9132-guts, }, + {}, +}; + +static void mpc85xx_irq_mask(int cpu) +{ + +} + +static void mpc85xx_irq_unmask(int cpu) +{ + +} + +static void mpc85xx_cpu_die(int cpu) +{ + +} + +static void mpc85xx_cpu_up(int cpu) +{ + +} + +static void mpc85xx_freeze_time_base(bool freeze) +{ + uint32_t mask; + + mask = CCSR_GUTS_DEVDISR_TB0 | CCSR_GUTS_DEVDISR_TB1; + if (freeze) + setbits32(guts-devdisr, mask); + else + clrbits32(guts-devdisr, mask); + + in_be32(guts-devdisr); +} + +static const struct fsl_pm_ops mpc85xx_pm_ops = { + .freeze_time_base = mpc85xx_freeze_time_base, + .irq_mask = mpc85xx_irq_mask, + .irq_unmask = mpc85xx_irq_unmask, + .cpu_die = mpc85xx_cpu_die, + .cpu_up = mpc85xx_cpu_up, +}; + +static int __init mpc85xx_setup_pmc(void) +{ + struct device_node *np; + + 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 -ENOMEM; + } + } + + qoriq_pm_ops = mpc85xx_pm_ops; + + return 0; +} + +/* need to call this before SMP init */ +early_initcall(mpc85xx_setup_pmc) diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c index 1180f78..6811a5b 100644 --- a/arch/powerpc/platforms/85xx/smp.c +++ b/arch/powerpc/platforms/85xx/smp.c @@ -2,7 +2,7 @@ * Author: Andy Fleming aflem...@freescale.com *Kumar Gala ga...@kernel.crashing.org * - * Copyright 2006-2008, 2011-2012 Freescale Semiconductor Inc. + * Copyright 2006-2008, 2011-2012, 2015 Freescale Semiconductor Inc. * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License as published by the @@ -15,7 +15,6 @@ #include linux/init.h #include linux/delay.h #include