[Skiboot] [PATCH] init: Perform pstates-init before dt blob creation

[Gautham R Shenoy]

Hi Vasant,


On Thu, Jul 11, 2019 at 10:40:58AM +0530, Vasant Hegde wrote:
> On 07/10/2019 05:59 PM, Gautham R. Shenoy wrote:
> >From: "Gautham R. Shenoy" <ego at linux.vnet.ibm.com>
> >
> >On FSP based systems (particularly POWER8), we perform
> >occ_pstates_init() late in the boot to allow OCC to be loaded. Hence
> >this was being performed in platform.exit(). occ_pstates_init() would
> >add pstate information into the device-tree.
> >
> >A recent commit 9fc0c1287ada ("Move FSP specific op-panel calls to
> >platform.exit()") moved the invocation of platform.exit() after the
> >creation of device-tree blob. As a result, on FSP based systems, we
> >don't have the pstate information in the device-tree, and thus the
> >Kernel is unable to perform frequency scaling.
> >
> >Fix this by moving occ_pstates_init() out of ibm_fsp_exit() and call
> >it before the creation of the device-tree blob.
> >
> >Fixes: commit 9fc0c1287ada ("Move FSP specific op-panel calls to
> >platform.exit()")
> >
> >Signed-off-by: Gautham R. Shenoy <ego at linux.vnet.ibm.com>
> 
> Thanks for the fix.
> 
> >---
> >  core/init.c                | 6 ++++++
> >  platforms/ibm-fsp/common.c | 7 -------
> >  2 files changed, 6 insertions(+), 7 deletions(-)
> >
> >diff --git a/core/init.c b/core/init.c
> >index d0f28f2..8eb0729 100644
> >--- a/core/init.c
> >+++ b/core/init.c
> >@@ -557,6 +557,12 @@ void __noreturn load_and_boot_kernel(bool is_reboot)
> >  		if (!occ_sensors_init())
> >  			dts_sensor_create_nodes(sensor_node);
> >
> >+		/*
> >+		 * OCC takes few secs to boot on FSP systems. Call
> >+		 * this as late as as possible to avoid delay.
> >+		 */
> >+		if (!platform.bmc)
> >+			occ_pstates_init();
> 
> So you will end up calling occ_pstates_init() on simulators as well. Do you
> really want that?

Perhaps not. You are right, I hadn't factored in the simulators. We
should do this only for FSP based systems.

> Today its occ_pstates, tomorrow we may have something else.
> IMO we should continue to have all those things inside platform.exit()
> function and move
> platform.exit function just before creating dtb.

Fine by me. However, in commit 9fc0c1287ada the platform.exit() was
moved from the point right before creation of the dtb to a later
stage to incorporate the op-panel calls into the platform.exit(). Will
it cause any problems if we move the platform.exit() up in the code ?

> 
> 
> >  	} else {
> >  		/* fdt will be rebuilt */
> >  		free(fdt);
> >diff --git a/platforms/ibm-fsp/common.c b/platforms/ibm-fsp/common.c
> >index 7f7a1f2..6da25d1 100644
> >--- a/platforms/ibm-fsp/common.c
> >+++ b/platforms/ibm-fsp/common.c
> >@@ -186,13 +186,6 @@ void ibm_fsp_exit(void)
> >  	/* Wait for FW VPD data read to complete */
> >  	fsp_code_update_wait_vpd(true);
> >
> >-	/*
> >-	 * OCC takes few secs to boot.  Call this as late as
> >-	 * as possible to avoid delay.
> >-	 */
> >-	if (fsp_present())
> >-		occ_pstates_init();
> 
> Given that its inside fsp exit() function, fsp_present() check is redundant.
> We should cleanup that.

Yeah, will clean that up.

> 
> -Vasant