Re: [PATCH 0/4] ARM: add PSCI host support

[Andre Przywara <andre.przywara@linaro.org>]

On 11/25/2013 03:50 PM, Ian Campbell wrote:
> On Mon, 2013-11-25 at 15:21 +0100, Andre Przywara wrote:
>> On 11/25/2013 03:03 PM, Ian Campbell wrote:
>>> On Mon, 2013-11-25 at 13:00 +0000, George Dunlap wrote:
>>>> On Mon, Nov 25, 2013 at 12:02 PM, Andre Przywara
>>>> <andre.przywara@linaro.org> wrote:
>>>>> Xen did not make use of the host provided ARM PSCI (Power State
>>>>> Coordination Interface) functionality so far, but relied on platform
>>>>> specific SMP bringup functions.
>>>>> This series adds support for PSCI on the host by reading the required
>>>>> information from the DTB and invoking the appropriate handler when
>>>>> bringing up each single CPU.
>>>>> Since PSCI is defined for both ARM32 and ARM64, I put the code in a
>>>>> file shared by both.
>>>>> The ARM32 code was tested on Midway, but the ARM64 code was compile
>>>>> tested only.
>>>>>
>>>>> This approach seems to be the least intrusive, but one could also use
>>>>> more of the current ARM64 code by copying the PSCI/spin-table
>>>>> distinction code to a shared file and use that from both
>>>>> architectures. However that seems more complicated.
>>>>>
>>>>> Please take a look and complain ;-)
>>>>>
>>>>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>>>>
>>>> Ian, do you agree that this is too late for 4.4?
>>>
>>> I'm in two minds. On the one hand none of the existing platforms
>>> currently require this functionality, so it has clearly not been
>>> necessary up to now.
>>>
>>> On the other hand it plays into the strategy of allowing people to
>>> trivially support their platform, and since it is a standard way to do
>>> power control on ARM (albeit quite new and so far uptake is not huge) I
>>> think it is expected that many new platforms will use it.
>>>
>>> Of our current platforms Midway can optionally use PSCI (we have
>>> "native" code at the minute)
>>
>> but which is not upstream yet, right?
>
> Oh right, I forgot it was still waiting for an Ack from you and thought
> I'd committed it when I had not.

I deliberately held back my ACK: on this one to give PSCI a chance, 
since it turned out to be easier than I thought.
Technically I am OK with Julien's patch, so I can ACK it as well if you 
like.

>> So if you are considering dropping PSCI for 4.4, I'd like to know so
>> that I can ack Julien's "native" SMP patch.
>> I hope at least this patch can make it for 4.4?
>
> Yes, one or the other should definitely go in for 4.4. It changes the
> argument for the PSCI stuff a bit too, since we can now enable midway
> and make it easier for other platforms at the same time.

That was my thinking. But I see both George's and your point with a 
release manager's hat on, so I am OK with whatever you decide.

Thanks for caring!
Andre

> [...]
>>> An alternative could be requiring for 4.4 that the platform code
>>> explicitly call into/request PSCI for 4.4 and only move to automatically
>>> using it in the absence of the platform code saying otherwise for 4.5.
>>
>> So you are thinking about a change in the priorities?
>
> I was only suggesting as a way to mitigate risk for 4.4 -- long term we
> should certainly do as Linux does and prefer PSCI. (I confess I wasn't
> sure how this manifests in Linux, if its at odds with what I wrote
> then ...oops)
>>   The Linux kernel
>> prefers PSCI over a native method, which is how I modeled the Xen patch
>> also. This has the advantage of having control in the DTB, so if PSCI
>> fails in Xen, one could do "fdt rm /psci" in u-boot to get the old
>> behavior back.
>>
>>> This has the advantage of being zero risk, but the downside of not being
>>> very well tested (we could enable it for Midway, with the attendant
>>> increase in risk).
>>
>> So are you concerned about one of the existing platforms breaking SMP as
>> soon as it gets PSCI support? One could change the patch to only use
>> PSCI if platform_cpu_up() does _not_ return an explicit "ignore PSCI"
>> value, if that helps.
>
> I'm addressing George's concerns as release manager about the risk of
> taking any sort of PSCI patches at this stage.
>
> Ian.
>