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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2013-12-04 at 13:37 +0100, Andre Przywara wrote:
> 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.

I mean that it should literally be within that function, not in the
caller.

> 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@xxxxxxxxxx>
> >
> > (I leave the first one up to you in case I've simply misparsed it)
> >
> > Ian.
> >
> >
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel