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

[Vasant Hegde]

On 07/11/2019 10:40 AM, 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?
> 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.

Does this work? And we should put fat comment explaining why we should have 
platform.exit before create_dtb().


diff --git a/core/init.c b/core/init.c
index fb55d4a9d..b8c2c05b9 100644
--- a/core/init.c
+++ b/core/init.c
@@ -580,6 +580,9 @@ void __noreturn load_and_boot_kernel(bool is_reboot)

         add_fast_reboot_dt_entries();

+       if (platform.exit)
+               platform.exit();
+
         /* Create the device tree blob to boot OS. */
         fdt = create_dtb(dt_root, false);
         if (!fdt) {
@@ -603,9 +606,6 @@ void __noreturn load_and_boot_kernel(bool is_reboot)
                 assert(0);
         }

-       if (platform.exit)
-               platform.exit();
-
         /* Take processors out of nap */
         cpu_set_sreset_enable(false);
         cpu_set_ipi_enable(false);

-Vasant