On 04/30/2014 11:09 PM, Alexander Graf wrote:

On 30.04.14 22:03, Stuart Yoder wrote:

quoted

quoted

-----Original Message-----
From: Alexander Graf [mailto:agraf@suse.de]
Sent: Wednesday, April 30, 2014 2:56 PM
To: Yoder Stuart-B08248; benh@kernel.crashing.org; Wood Scott-B07421
Cc: linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH] powerpc: move epapr paravirt init of power_save to
an initcall


On 30.04.14 21:54, Stuart Yoder wrote:

quoted

From: Stuart Yoder <redacted>

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 <redacted>
---
   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

quoted

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,

quoted

   #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.

Yeah, the same commit that introduced the breakage on has-idle also
removed the explicit check for /hypervisor.

Laurentiu, was this change on purpose?

Alex,

IIRC, at that time i had to switch from the normal "of" functions to a 
completely different api that's available in early init stage. This 
early "of" api is pretty limited (e.g. doesn't have a way to address a 
specific node) and i had to use that function that scans the whole tree.

---
Best Regards, Laurentiu