[U-Boot] [PATCH v2 09/12] tegra124: Add PSCI support for Tegra124

[Thierry Reding <treding@nvidia.com>]

On Wed, Feb 18, 2015 at 09:34:53AM -0700, Stephen Warren wrote:
> On 02/17/2015 11:13 PM, Jan Kiszka wrote:
> >On 2015-02-17 22:03, Stephen Warren wrote:
> >>On 02/16/2015 05:54 AM, Jan Kiszka wrote:
> >>>This is based on Thierry Reding's work and uses Ian Campell's
> >>>preparatory patches. It comes with full support for CPU_ON/OFF PSCI
> >>>services. The algorithm used in this version for turning CPUs on and
> >>>off was proposed by Thierry Reding in
> >>>http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/210881. It
> >>>consists of first enabling CPU1..3 via the PMC, just to powergate them
> >>>again with the help of the Flow Controller. Once the Flow Controller is
> >>>in place, we can leave the PMC alone while processing CPU_ON and CPU_OFF
> >>>PSCI requests.
> >>
> >>>diff --git a/arch/arm/cpu/armv7/tegra124/ap.c
> >>>b/arch/arm/cpu/armv7/tegra124/ap.c
> >>
> >>>+void ap_pm_init(void)
> >>>+{
> >>>+    struct flow_ctlr *flow = (struct flow_ctlr *)NV_PA_FLOW_BASE;
> >>>+    struct pmc_ctlr *pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
> >>>+
> >>>+    writel((u32)park_cpu, EXCEP_VECTOR_CPU_RESET_VECTOR);
> >>>+
> >>>+    tegra_powergate_power_on(TEGRA_POWERGATE_CPU1);
> >>>+    tegra_powergate_power_on(TEGRA_POWERGATE_CPU2);
> >>>+    tegra_powergate_power_on(TEGRA_POWERGATE_CPU3);
> >>>+
> >>>+    writel((2 << CSR_WAIT_WFI_SHIFT) | CSR_ENABLE, &flow->cpu1_csr);
> >>>+    writel((4 << CSR_WAIT_WFI_SHIFT) | CSR_ENABLE, &flow->cpu2_csr);
> >>>+    writel((8 << CSR_WAIT_WFI_SHIFT) | CSR_ENABLE, &flow->cpu3_csr);
> >>>+
> >>>+    writel(EVENT_MODE_STOP, &flow->halt_cpu1_events);
> >>>+    writel(EVENT_MODE_STOP, &flow->halt_cpu2_events);
> >>>+    writel(EVENT_MODE_STOP, &flow->halt_cpu3_events);
> >>
> >>I would expect to set up halt_cpu*_events before powering on the CPUs,
> >>to make sure that they do the expected action on the very first WFI. So,
> >>shouldn't the order above be:
> >>
> >>Write to halt_cpu*_events
> >>Write to cpu*_csr
> >>power_on
> >
> >Yeah, that was my original expectation as well. But
> >
> >diff --git a/arch/arm/cpu/armv7/tegra124/ap.c b/arch/arm/cpu/armv7/tegra124/ap.c
> >index eebc0ea..240c71d 100644
> >--- a/arch/arm/cpu/armv7/tegra124/ap.c
> >+++ b/arch/arm/cpu/armv7/tegra124/ap.c
> >@@ -25,10 +25,6 @@ void ap_pm_init(void)
> >
> >  	writel((u32)park_cpu, EXCEP_VECTOR_CPU_RESET_VECTOR);
> >
> >-	tegra_powergate_power_on(TEGRA_POWERGATE_CPU1);
> >-	tegra_powergate_power_on(TEGRA_POWERGATE_CPU2);
> >-	tegra_powergate_power_on(TEGRA_POWERGATE_CPU3);
> >-
> >  	writel((2 << CSR_WAIT_WFI_SHIFT) | CSR_ENABLE, &flow->cpu1_csr);
> >  	writel((4 << CSR_WAIT_WFI_SHIFT) | CSR_ENABLE, &flow->cpu2_csr);
> >  	writel((8 << CSR_WAIT_WFI_SHIFT) | CSR_ENABLE, &flow->cpu3_csr);
> >@@ -37,6 +33,10 @@ void ap_pm_init(void)
> >  	writel(EVENT_MODE_STOP, &flow->halt_cpu2_events);
> >  	writel(EVENT_MODE_STOP, &flow->halt_cpu3_events);
> >
> >+	tegra_powergate_power_on(TEGRA_POWERGATE_CPU1);
> >+	tegra_powergate_power_on(TEGRA_POWERGATE_CPU2);
> >+	tegra_powergate_power_on(TEGRA_POWERGATE_CPU3);
> >+
> >  	while (readl(&pmc->pmc_pwrgate_status) & ((1 << TEGRA_POWERGATE_CPU1) |
> >  						  (1 << TEGRA_POWERGATE_CPU2) |
> >  						  (1 << TEGRA_POWERGATE_CPU3)))
> >
> >doesn't work in practice. I suspect the power-on overwrites what the
> >flow controller configures in the PMC beforehand. But maybe someone can
> >explain this better than me.
> 
> Thierry, Peter, can you comment on why that is, and whether the original
> code sequence is safe; does it matter that the target CPU executes WFI
> before the flow controller is configured what to do on WFI?

As I mentioned before, I don't think it's safe to change the powergate
status of more than one partition at once. I'm not sure that this will
change anything regarding the relative positioning of powergate on vs.
writing CPU halt events, but I agree with Stephen that running the CPU
without the halt events being programmed could cause them to simply go
into a WFI without them actually being turned off.

Perhaps if unpowergating after writing the halt events registers doesn't
work a safer way would be to go and forcibly wake up all CPUs again
after they are powered up (using the IMMEDIATE_WAKE bit in the CSR)?

I haven't seen anything in the documentation regarding why unpowergating
after writing halt event registers wouldn't work. I'm sure I haven't
looked at all the documentation, but this is about as knowledgeable as I
am regarding the CPUs and the flow controller. Perhaps Peter will indeed
know more than that.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150219/eb00ec00/attachment.sig>