* [PATCH] powerpc: move epapr paravirt init of power_save to an initcall @ 2014-04-30 19:54 Stuart Yoder 2014-04-30 19:56 ` Alexander Graf 0 siblings, 1 reply; 8+ messages in thread From: Stuart Yoder @ 2014-04-30 19:54 UTC (permalink / raw) To: benh, scottwood; +Cc: linuxppc-dev, agraf, Stuart Yoder From: Stuart Yoder <stuart.yoder@freescale.com> 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 <stuart.yoder@freescale.com> --- 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); + + return 0; +} + +postcore_initcall(epapr_idle_init); -- 1.7.9.7 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: move epapr paravirt init of power_save to an initcall 2014-04-30 19:54 [PATCH] powerpc: move epapr paravirt init of power_save to an initcall Stuart Yoder @ 2014-04-30 19:56 ` Alexander Graf 2014-04-30 20:03 ` Stuart Yoder 0 siblings, 1 reply; 8+ messages in thread From: Alexander Graf @ 2014-04-30 19:56 UTC (permalink / raw) To: Stuart Yoder, benh, scottwood; +Cc: linuxppc-dev On 30.04.14 21:54, Stuart Yoder wrote: > From: Stuart Yoder <stuart.yoder@freescale.com> > > 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 <stuart.yoder@freescale.com> > --- > 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? Alex ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] powerpc: move epapr paravirt init of power_save to an initcall 2014-04-30 19:56 ` Alexander Graf @ 2014-04-30 20:03 ` Stuart Yoder 2014-04-30 20:06 ` Alexander Graf 2014-04-30 20:09 ` Alexander Graf 0 siblings, 2 replies; 8+ messages in thread From: Stuart Yoder @ 2014-04-30 20:03 UTC (permalink / raw) To: Alexander Graf, benh@kernel.crashing.org, Scott Wood Cc: linuxppc-dev@lists.ozlabs.org > -----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 >=20 >=20 > On 30.04.14 21:54, Stuart Yoder wrote: > > From: Stuart Yoder <stuart.yoder@freescale.com> > > > > 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 <stuart.yoder@freescale.com> > > --- > > 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 =3D epapr_ev_idle; > > -#endif > > - > > epapr_paravirt_enabled =3D 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 =3D 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); >=20 > 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. Stuart ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: move epapr paravirt init of power_save to an initcall 2014-04-30 20:03 ` Stuart Yoder @ 2014-04-30 20:06 ` Alexander Graf 2014-04-30 20:09 ` Alexander Graf 1 sibling, 0 replies; 8+ messages in thread From: Alexander Graf @ 2014-04-30 20:06 UTC (permalink / raw) To: Stuart Yoder, benh@kernel.crashing.org, Scott Wood Cc: linuxppc-dev@lists.ozlabs.org On 30.04.14 22:03, Stuart Yoder wrote: > >> -----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: >>> From: Stuart Yoder <stuart.yoder@freescale.com> >>> >>> 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 <stuart.yoder@freescale.com> >>> --- >>> 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: move epapr paravirt init of power_save to an initcall 2014-04-30 20:03 ` Stuart Yoder 2014-04-30 20:06 ` Alexander Graf @ 2014-04-30 20:09 ` Alexander Graf 2014-05-05 12:17 ` Tudor Laurentiu 1 sibling, 1 reply; 8+ messages in thread From: Alexander Graf @ 2014-04-30 20:09 UTC (permalink / raw) To: Stuart Yoder, benh@kernel.crashing.org, Scott Wood Cc: Laurentiu.Tudor, linuxppc-dev@lists.ozlabs.org On 30.04.14 22:03, Stuart Yoder wrote: > >> -----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: >>> From: Stuart Yoder <stuart.yoder@freescale.com> >>> >>> 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 <stuart.yoder@freescale.com> >>> --- >>> 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. Yeah, the same commit that introduced the breakage on has-idle also removed the explicit check for /hypervisor. Laurentiu, was this change on purpose? commit 4e21b94c9c644c43223878f4c848e852743e789c Author: Laurentiu TUDOR <Laurentiu.Tudor@freescale.com> Date: Wed Jul 3 17:13:15 2013 +0300 powerpc/85xx: Move ePAPR paravirt initialization earlier At console init, when the kernel tries to flush the log buffer the ePAPR byte-channel based console write fails silently, losing the buffered messages. This happens because The ePAPR para-virtualization init isn't done early enough so that the hcall instruction to be set, causing the byte-channel write hcall to be a nop. To fix, change the ePAPR para-virt init to use early device tree functions and move it in early init. Signed-off-by: Laurentiu Tudor <Laurentiu.Tudor@freescale.com> Signed-off-by: Scott Wood <scottwood@freescale.com> [...] diff --git a/arch/powerpc/kernel/epapr_paravirt.c b/arch/powerpc/kernel/epapr_paravirt.c index d44a571..6300c13 100644 --- a/arch/powerpc/kernel/epapr_paravirt.c +++ b/arch/powerpc/kernel/epapr_paravirt.c @@ -30,22 +30,20 @@ extern u32 epapr_ev_idle_start[]; bool epapr_paravirt_enabled; -static int __init epapr_paravirt_init(void) +static int __init early_init_dt_scan_epapr(unsigned long node, + const char *uname, + int depth, void *data) { - struct device_node *hyper_node; const u32 *insts; - int len, i; + unsigned long len; + int i; - hyper_node = of_find_node_by_path("/hypervisor"); - if (!hyper_node) - return -ENODEV; - - insts = of_get_property(hyper_node, "hcall-instructions", &len); + insts = of_get_flat_dt_prop(node, "hcall-instructions", &len); if (!insts) - return -ENODEV; + return 0; if (len % 4 || len > (4 * 4)) - return -ENODEV; + return -1; [...] Alex ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: move epapr paravirt init of power_save to an initcall 2014-04-30 20:09 ` Alexander Graf @ 2014-05-05 12:17 ` Tudor Laurentiu 2014-05-05 12:21 ` Alexander Graf 0 siblings, 1 reply; 8+ messages in thread From: Tudor Laurentiu @ 2014-05-05 12:17 UTC (permalink / raw) To: Alexander Graf Cc: Scott Wood, Laurentiu.Tudor, linuxppc-dev@lists.ozlabs.org, Stuart Yoder On 04/30/2014 11:09 PM, Alexander Graf wrote: > > On 30.04.14 22:03, Stuart Yoder wrote: >> >>> -----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: >>>> From: Stuart Yoder <stuart.yoder@freescale.com> >>>> >>>> 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 <stuart.yoder@freescale.com> >>>> --- >>>> 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. > > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: move epapr paravirt init of power_save to an initcall 2014-05-05 12:17 ` Tudor Laurentiu @ 2014-05-05 12:21 ` Alexander Graf 2014-05-05 12:35 ` Tudor Laurentiu 0 siblings, 1 reply; 8+ messages in thread From: Alexander Graf @ 2014-05-05 12:21 UTC (permalink / raw) To: Tudor Laurentiu Cc: Scott Wood, Laurentiu.Tudor, linuxppc-dev@lists.ozlabs.org, Stuart Yoder On 05/05/2014 02:17 PM, Tudor Laurentiu wrote: > On 04/30/2014 11:09 PM, Alexander Graf wrote: >> >> On 30.04.14 22:03, Stuart Yoder wrote: >>> >>>> -----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: >>>>> From: Stuart Yoder <stuart.yoder@freescale.com> >>>>> >>>>> 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 <stuart.yoder@freescale.com> >>>>> --- >>>>> 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. >> >> 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. Ok, so it is an accident. Could you please post a patch that checks that the node we're looking at is called "hypervisor"? The simple API should give you enough information for that at least. Maybe you could even check that the parent node is the root node. Alex ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: move epapr paravirt init of power_save to an initcall 2014-05-05 12:21 ` Alexander Graf @ 2014-05-05 12:35 ` Tudor Laurentiu 0 siblings, 0 replies; 8+ messages in thread From: Tudor Laurentiu @ 2014-05-05 12:35 UTC (permalink / raw) To: Alexander Graf Cc: Scott Wood, Laurentiu.Tudor, linuxppc-dev@lists.ozlabs.org, Stuart Yoder On 05/05/2014 03:21 PM, Alexander Graf wrote: > On 05/05/2014 02:17 PM, Tudor Laurentiu wrote: >> On 04/30/2014 11:09 PM, Alexander Graf wrote: >>> >>> On 30.04.14 22:03, Stuart Yoder wrote: >>>> >>>>> -----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: >>>>>> From: Stuart Yoder <stuart.yoder@freescale.com> >>>>>> >>>>>> 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 <stuart.yoder@freescale.com> >>>>>> --- >>>>>> 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. >>> >>> 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. > > Ok, so it is an accident. Could you please post a patch that checks that > the node we're looking at is called "hypervisor"? The simple API should > give you enough information for that at least. Maybe you could even > check that the parent node is the root node. > Just had a quick look and it looks that that early fdt api was improved with a function that allows specifying a starting path for the scan (of_scan_flat_dt_by_path() added in commit 57d74bcf3072b65bde5aa540cedc976a75c48e5c). So i think we can simply use that instead. --- Best Regards, Laurentiu ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-05-05 12:51 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-04-30 19:54 [PATCH] powerpc: move epapr paravirt init of power_save to an initcall Stuart Yoder 2014-04-30 19:56 ` Alexander Graf 2014-04-30 20:03 ` Stuart Yoder 2014-04-30 20:06 ` Alexander Graf 2014-04-30 20:09 ` Alexander Graf 2014-05-05 12:17 ` Tudor Laurentiu 2014-05-05 12:21 ` Alexander Graf 2014-05-05 12:35 ` Tudor Laurentiu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).