From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Wed, 18 Feb 2015 09:34:53 -0700 Subject: [U-Boot] [PATCH v2 09/12] tegra124: Add PSCI support for Tegra124 In-Reply-To: <54E42D93.2090809@siemens.com> References: <48840a4f6b07a748ff10c20ffa69f6732c967b35.1424091289.git.jan.kiszka@siemens.com> <54E3AC95.7070200@wwwdotorg.org> <54E42D93.2090809@siemens.com> Message-ID: <54E4BF2D.3050103@wwwdotorg.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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?