Attention is currently required from: Anil Kumar K, Jérémy Compostella, Krishna P Bhat D, Paul Menzel, Rizwan Qureshi, sridhar siricilla.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74577?usp=email )
Change subject: soc/intel/cse: Back up PSR data during CSE FW downgrade ......................................................................
Patch Set 52:
(2 comments)
File src/soc/intel/common/block/cse/cse_lite.c:
https://review.coreboot.org/c/coreboot/+/74577/comment/9cbd3f81_b98eb8e0 : PS50, Line 1125: An attempt to send PSR back-up command has been made.
something like "Back-up failed", is what i'm looking for record purposes. I just wanted to ensure there is a tracker in FW (CMOS or elogtool)to justify the PSR data loss. If google cloud missed the device telemetry then there has to some tracker to tell what might have happened which causes the data lose. Like a heart beat implementation where we would like to keep the last status (good or bad).
Yes, instead of adding a new CMOS state,
please do that
we are creating an ELOG event in case of failures. That is in the subsequent patches.
yes, i have reviewed that.
would you mind to move this discussion merits into your original PSR design doc? so we can do a retro later for adding feature for next gen ?
File src/soc/intel/common/block/cse/cse_lite.c:
https://review.coreboot.org/c/coreboot/+/74577/comment/9c1a3ea1_bc288f14 : PS52, Line 1115: if (backup_psr_resp.status != PSR_STATUS_SUCCESS)
if we have this check for PSR bit (https://review.coreboot.org/c/coreboot/+/74874) we will not be invoking this on non VPRO skus
that i understand but I'm not sure if its okay to send PSR_BACKUP_DONE during failure and fix later in some patch train.
may be worth puling these two lines inside the if block and return immediately ? ``` update_psr_backup_status(PSR_BACKUP_FAILED); return; ```
of assume `update_and_exit` block will always PSR_BACKUP_FAILED hence, you need to bail out if PSR is successful and PSR_BACKUP_SUCCESS at earliest.