On 2015-02-20 10:36, Jan Kiszka wrote: > On 2015-02-19 10:14, Thierry Reding wrote: >> 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 > > tegra_powergate_set() already synchronizes the caller on the completion > of the switch. So the existing code is safe in this regard. > > However, the K1 manual also states that the START bit of the toggle > register should be checked prior to starting a request. This is not done > by tegra_powergate_set() - probably because it is a K1-only requirement, > not applying to older CPUs. Not sure, though, if waiting for START=0 is > practically required when already waiting for the switch to be processed > by the PMC before continuing. > >> 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. > > The CPUs most probably go into WFI first, because we wait for the > partition to be reported as powered up, but it seems they can be turned > off while in WFI as well. I'm not basing this on anything stated in the > manual, just on experiments. > >> >> 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. > > Yes, more insights would indeed be welcome!
Ping regarding this last open point. I'm sitting on v4 of this series - v3 had a nasty bug in the CNTFRQ fixup, I addressed the reviews and added further cleanups - and I would like to close this topic eventually. Jan PS: After KVM, I was also able to get the Jailhouse hypervisor running on the TK1 - thanks to this series (v4, to be precise): http://thread.gmane.org/gmane.linux.jailhouse/2580 -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
