Re: [PATCH v2 3/6] arm: parse PSCI node from the host device-tree

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

On 12/02/2013 04:05 PM, Ian Campbell wrote:
> On Mon, 2013-12-02 at 12:08 +0100, Andre Przywara wrote:
>> +int __init psci_init(void)
>> +{
> [...]
>> +    /* Since Xen runs in HYP all of the time, it does not make sense to
>> +     * let it call into HYP for PSCI handling, since the handler won't
>> +     * just be there. So bail out with an error if "smc" is not used.
>
> s/won't just/just won't/.
>
>> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
>> index 52cef30..3a9be90 100644
>> --- a/xen/arch/arm/smpboot.c
>> +++ b/xen/arch/arm/smpboot.c
>> [...]
>> @@ -105,6 +106,12 @@ void __init smp_init_cpus(void)
>>       bool_t bootcpu_valid = 0;
>>       int rc;
>>
>> +    if ( psci_init() == 0 )
>> +    {
>> +        printk(XENLOG_INFO "Using PSCI for SMP bringup\n");
>> +        psci_available = 1;
>
> IMHO this log + flag twiddling belong as the last act of psci init.

Well, it is already, at least kind of.
psci_available means that the DTB contains a valid and sane PSCI node, 
so Xen should use PSCI as the SMP bringup method (that's why _available 
and not _enabled).
And the message was to inform the user that PSCI is _going to be used_ 
for SMP bringup. I should change the wording to be more clear here.
On Linux this kind of information gave me valuable hints on debugging 
SMP issues in the past.

Thanks

>
> Other than those two things this patch looks good. If you change at
> least the second one then:
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> (I leave the first one up to you in case I've simply misparsed it)
>
> Ian.
>
>