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

[Stephen Warren <swarren@wwwdotorg.org>]

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?