| Message ID | 20190711074144.25824-3-oohall@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/3] core/platform: Add finalise_dt() callback | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (4db38a36b31045f0a116d388ddeac850b38c8680) |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | success | Test snowpatch/job/snowpatch-skiboot on branch master |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco | success | Signed-off-by present |
Hello Oliver, On Thu, Jul 11, 2019 at 05:41:44PM +1000, Oliver O'Halloran wrote: > >From Gautham's patch to fix pstates_init() that this was based on: > > > 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. > > The same patch also broke fast-reboot on ZZ. Without this patch applied > we get the following assert() fail when fast rebooting: > > [ 1153.398889405,5] CUPD: Waiting read marker LID and in flight parsm completion... > [ 1153.398892228,3] Duplicate property "mi-version" in node /ibm,opal/firmware > [ 1153.398894036,0] Aborting! > CPU 0054 Backtrace: > S: 0000000031ea39b0 R: 0000000030013828 .backtrace+0x34 > S: 0000000031ea3a70 R: 000000003001b268 ._abort+0x4c > S: 0000000031ea3af0 R: 0000000030027788 .new_property+0x80 > S: 0000000031ea3b80 R: 00000000300278d0 .dt_add_property+0xa8 > S: 0000000031ea3c10 R: 0000000030081a98 .fsp_code_update_wait_vpd+0x19c > S: 0000000031ea3d10 R: 000000003008ff98 .ibm_fsp_exit+0x1c > S: 0000000031ea3d80 R: 0000000030014864 .load_and_boot_kernel+0xb14 > S: 0000000031ea3e60 R: 000000003002685c .fast_reboot_entry+0x38c > S: 0000000031ea3f00 R: 0000000030002988 reset_fast_reboot_wakeup+0x40 > > This happens because fsp_code_update_wait_vpd(true) adds some properties > to the DT so moving it to finalise_dt fixes that crash too. > > Cc: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> > Cc: Gautham R. Shenoy <ego@linux.vnet.ibm.com> > Cc: Vaidyanathan Srinivasan <svaidy@linux.ibm.com> > Fixes: 9fc0c1287ada ("Move FSP specific op-panel calls to platform.exit()") > Signed-off-by: Oliver O'Halloran <oohall@gmail.com> This fixes the cpufreq problem on Firenze and ZZ. Tested-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com> > --- > platforms/ibm-fsp/common.c | 17 +++++++++++------ > platforms/ibm-fsp/firenze.c | 1 + > platforms/ibm-fsp/ibm-fsp.h | 1 + > platforms/ibm-fsp/zz.c | 1 + > 4 files changed, 14 insertions(+), 6 deletions(-) > > diff --git a/platforms/ibm-fsp/common.c b/platforms/ibm-fsp/common.c > index 7f7a1f246206..6f9d57a8fb6b 100644 > --- a/platforms/ibm-fsp/common.c > +++ b/platforms/ibm-fsp/common.c > @@ -174,8 +174,11 @@ void ibm_fsp_init(void) > preload_io_vpd(); > } > > -void ibm_fsp_exit(void) > +void ibm_fsp_finalise_dt(bool is_reboot) > { > + if (is_reboot) > + return; > + > /* > * LED related SPCN commands might take a while to > * complete. Call this as late as possible to > @@ -183,18 +186,20 @@ void ibm_fsp_exit(void) > */ > create_led_device_nodes(); > > - /* 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(); > + occ_pstates_init(); > + > + /* Wait for FW VPD data read to complete */ > + fsp_code_update_wait_vpd(true); > > fsp_console_select_stdout(); > +} > > +void ibm_fsp_exit(void) > +{ > op_panel_disable_src_echo(); > > /* Clear SRCs on the op-panel when Linux starts */ > diff --git a/platforms/ibm-fsp/firenze.c b/platforms/ibm-fsp/firenze.c > index 232833480f75..db18e2ff4eba 100644 > --- a/platforms/ibm-fsp/firenze.c > +++ b/platforms/ibm-fsp/firenze.c > @@ -214,6 +214,7 @@ DECLARE_PLATFORM(firenze) = { > .probe = firenze_probe, > .init = firenze_init, > .fast_reboot_init = fsp_console_reset, > + .finalise_dt = ibm_fsp_finalise_dt, > .exit = ibm_fsp_exit, > .cec_power_down = ibm_fsp_cec_power_down, > .cec_reboot = ibm_fsp_cec_reboot, > diff --git a/platforms/ibm-fsp/ibm-fsp.h b/platforms/ibm-fsp/ibm-fsp.h > index dc3969ec688d..c055d4a3e6f9 100644 > --- a/platforms/ibm-fsp/ibm-fsp.h > +++ b/platforms/ibm-fsp/ibm-fsp.h > @@ -20,6 +20,7 @@ > > extern void ibm_fsp_init(void); > extern void ibm_fsp_exit(void); > +void ibm_fsp_finalise_dt(bool is_reboot); > > extern int64_t ibm_fsp_cec_power_down(uint64_t request); > extern int64_t ibm_fsp_cec_reboot(void); > diff --git a/platforms/ibm-fsp/zz.c b/platforms/ibm-fsp/zz.c > index f44c618c0ec1..e369f8d1fc5b 100644 > --- a/platforms/ibm-fsp/zz.c > +++ b/platforms/ibm-fsp/zz.c > @@ -79,6 +79,7 @@ DECLARE_PLATFORM(zz) = { > .probe = zz_probe, > .init = zz_init, > .fast_reboot_init = fsp_console_reset, > + .finalise_dt = ibm_fsp_finalise_dt, > .exit = ibm_fsp_exit, > .cec_power_down = ibm_fsp_cec_power_down, > .cec_reboot = ibm_fsp_cec_reboot, > -- > 2.21.0 >
On 07/11/2019 01:11 PM, Oliver O'Halloran wrote: > From Gautham's patch to fix pstates_init() that this was based on: > >> 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. > > The same patch also broke fast-reboot on ZZ. Without this patch applied > we get the following assert() fail when fast rebooting: > > [ 1153.398889405,5] CUPD: Waiting read marker LID and in flight parsm completion... > [ 1153.398892228,3] Duplicate property "mi-version" in node /ibm,opal/firmware > [ 1153.398894036,0] Aborting! > CPU 0054 Backtrace: > S: 0000000031ea39b0 R: 0000000030013828 .backtrace+0x34 > S: 0000000031ea3a70 R: 000000003001b268 ._abort+0x4c > S: 0000000031ea3af0 R: 0000000030027788 .new_property+0x80 > S: 0000000031ea3b80 R: 00000000300278d0 .dt_add_property+0xa8 > S: 0000000031ea3c10 R: 0000000030081a98 .fsp_code_update_wait_vpd+0x19c > S: 0000000031ea3d10 R: 000000003008ff98 .ibm_fsp_exit+0x1c > S: 0000000031ea3d80 R: 0000000030014864 .load_and_boot_kernel+0xb14 > S: 0000000031ea3e60 R: 000000003002685c .fast_reboot_entry+0x38c > S: 0000000031ea3f00 R: 0000000030002988 reset_fast_reboot_wakeup+0x40 > > This happens because fsp_code_update_wait_vpd(true) adds some properties > to the DT so moving it to finalise_dt fixes that crash too. > > Cc: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> > Cc: Gautham R. Shenoy <ego@linux.vnet.ibm.com> > Cc: Vaidyanathan Srinivasan <svaidy@linux.ibm.com> > Fixes: 9fc0c1287ada ("Move FSP specific op-panel calls to platform.exit()") > Signed-off-by: Oliver O'Halloran <oohall@gmail.com> Looks good. Reviewed-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> -Vasant
"Oliver O'Halloran" <oohall@gmail.com> writes: > From Gautham's patch to fix pstates_init() that this was based on: > >> 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. > > The same patch also broke fast-reboot on ZZ. Without this patch applied > we get the following assert() fail when fast rebooting: > > [ 1153.398889405,5] CUPD: Waiting read marker LID and in flight parsm completion... > [ 1153.398892228,3] Duplicate property "mi-version" in node /ibm,opal/firmware > [ 1153.398894036,0] Aborting! > CPU 0054 Backtrace: > S: 0000000031ea39b0 R: 0000000030013828 .backtrace+0x34 > S: 0000000031ea3a70 R: 000000003001b268 ._abort+0x4c > S: 0000000031ea3af0 R: 0000000030027788 .new_property+0x80 > S: 0000000031ea3b80 R: 00000000300278d0 .dt_add_property+0xa8 > S: 0000000031ea3c10 R: 0000000030081a98 .fsp_code_update_wait_vpd+0x19c > S: 0000000031ea3d10 R: 000000003008ff98 .ibm_fsp_exit+0x1c > S: 0000000031ea3d80 R: 0000000030014864 .load_and_boot_kernel+0xb14 > S: 0000000031ea3e60 R: 000000003002685c .fast_reboot_entry+0x38c > S: 0000000031ea3f00 R: 0000000030002988 reset_fast_reboot_wakeup+0x40 > > This happens because fsp_code_update_wait_vpd(true) adds some properties > to the DT so moving it to finalise_dt fixes that crash too. > > Cc: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> > Cc: Gautham R. Shenoy <ego@linux.vnet.ibm.com> > Cc: Vaidyanathan Srinivasan <svaidy@linux.ibm.com> > Fixes: 9fc0c1287ada ("Move FSP specific op-panel calls to platform.exit()") > Signed-off-by: Oliver O'Halloran <oohall@gmail.com> Thanks (and sorry for breaking things). Series merged to master as of 4c929bb54629fc60d8178383b4e5f854a8d1909c
diff --git a/platforms/ibm-fsp/common.c b/platforms/ibm-fsp/common.c index 7f7a1f246206..6f9d57a8fb6b 100644 --- a/platforms/ibm-fsp/common.c +++ b/platforms/ibm-fsp/common.c @@ -174,8 +174,11 @@ void ibm_fsp_init(void) preload_io_vpd(); } -void ibm_fsp_exit(void) +void ibm_fsp_finalise_dt(bool is_reboot) { + if (is_reboot) + return; + /* * LED related SPCN commands might take a while to * complete. Call this as late as possible to @@ -183,18 +186,20 @@ void ibm_fsp_exit(void) */ create_led_device_nodes(); - /* 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(); + occ_pstates_init(); + + /* Wait for FW VPD data read to complete */ + fsp_code_update_wait_vpd(true); fsp_console_select_stdout(); +} +void ibm_fsp_exit(void) +{ op_panel_disable_src_echo(); /* Clear SRCs on the op-panel when Linux starts */ diff --git a/platforms/ibm-fsp/firenze.c b/platforms/ibm-fsp/firenze.c index 232833480f75..db18e2ff4eba 100644 --- a/platforms/ibm-fsp/firenze.c +++ b/platforms/ibm-fsp/firenze.c @@ -214,6 +214,7 @@ DECLARE_PLATFORM(firenze) = { .probe = firenze_probe, .init = firenze_init, .fast_reboot_init = fsp_console_reset, + .finalise_dt = ibm_fsp_finalise_dt, .exit = ibm_fsp_exit, .cec_power_down = ibm_fsp_cec_power_down, .cec_reboot = ibm_fsp_cec_reboot, diff --git a/platforms/ibm-fsp/ibm-fsp.h b/platforms/ibm-fsp/ibm-fsp.h index dc3969ec688d..c055d4a3e6f9 100644 --- a/platforms/ibm-fsp/ibm-fsp.h +++ b/platforms/ibm-fsp/ibm-fsp.h @@ -20,6 +20,7 @@ extern void ibm_fsp_init(void); extern void ibm_fsp_exit(void); +void ibm_fsp_finalise_dt(bool is_reboot); extern int64_t ibm_fsp_cec_power_down(uint64_t request); extern int64_t ibm_fsp_cec_reboot(void); diff --git a/platforms/ibm-fsp/zz.c b/platforms/ibm-fsp/zz.c index f44c618c0ec1..e369f8d1fc5b 100644 --- a/platforms/ibm-fsp/zz.c +++ b/platforms/ibm-fsp/zz.c @@ -79,6 +79,7 @@ DECLARE_PLATFORM(zz) = { .probe = zz_probe, .init = zz_init, .fast_reboot_init = fsp_console_reset, + .finalise_dt = ibm_fsp_finalise_dt, .exit = ibm_fsp_exit, .cec_power_down = ibm_fsp_cec_power_down, .cec_reboot = ibm_fsp_cec_reboot,