Subject: Re: [PATCH] powerpc: move epapr paravirt init of power_save to an initcall

From: Alexander Graf <[email-masked]>
Date: Wed, 30 Apr 2014 22:06:04 +0200

On 30.04.14 22:03, Stuart Yoder wrote:
>
>> -----Original Message-----
>> From: Alexander Graf [mailto:[email-masked]]
>> Sent: Wednesday, April 30, 2014 2:56 PM
>> To: Yoder Stuart-B08248; [email-masked]; Wood Scott-B07421
>> Cc: [email-masked]
>> Subject: Re: [PATCH] powerpc: move epapr paravirt init of power_save to
>> an initcall
>>
>>
>> On 30.04.14 21:54, Stuart Yoder wrote:
>>> From: Stuart Yoder <[email-masked]>
>>>
>>> some restructuring of epapr paravirt init resulted in
>>> ppc_md.power_save being set, and then overwritten to
>>> NULL during machine_init.  This patch splits the
>>> initialization of ppc_md.power_save out into a postcore
>>> init call.
>>>
>>> Signed-off-by: Stuart Yoder <[email-masked]>
>>> ---
>>>    arch/powerpc/kernel/epapr_paravirt.c |   25 ++++++++++++++++++++-----
>>>    1 file changed, 20 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kernel/epapr_paravirt.c
>> b/arch/powerpc/kernel/epapr_paravirt.c
>>> index 6300c13..c49b69c 100644
>>> --- a/arch/powerpc/kernel/epapr_paravirt.c
>>> +++ b/arch/powerpc/kernel/epapr_paravirt.c
>>> @@ -52,11 +52,6 @@ static int __init early_init_dt_scan_epapr(unsigned
>> long node,
>>>    #endif
>>>    	}
>>>
>>> -#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
>>> -	if (of_get_flat_dt_prop(node, "has-idle", NULL))
>>> -		ppc_md.power_save = epapr_ev_idle;
>>> -#endif
>>> -
>>>    	epapr_paravirt_enabled = true;
>>>
>>>    	return 1;
>>> @@ -69,3 +64,23 @@ int __init epapr_paravirt_early_init(void)
>>>    	return 0;
>>>    }
>>>
>>> +static int __init epapr_idle_init_dt_scan(unsigned long node,
>>> +					   const char *uname,
>>> +					   int depth, void *data)
>>> +{
>>> +#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
>>> +	if (of_get_flat_dt_prop(node, "has-idle", NULL))
>>> +		ppc_md.power_save = epapr_ev_idle;
>>> +#endif
>>> +	return 0;
>>> +}
>>> +
>>> +static int __init epapr_idle_init(void)
>>> +{
>>> +	if (epapr_paravirt_enabled)
>>> +		of_scan_flat_dt(epapr_idle_init_dt_scan, NULL);
>> Doesn't this scan all nodes? We only want to match on
>> /hypervisor/has-idle, no?
> I cut/pasted from  the approach the existing code in that file
> took, but yes you're right we just need the one property.
> Let me respin that to look at the hypervisor node only.

The other function aborts early if it doesn't find a 
"hcall-instructions" property. I'm still surprised we don't limit the 
scope to /hypervisor at all though. Is this on purpose maybe?

Alex