Attention is currently required from: Anil Kumar K, Jérémy Compostella, Krishna P Bhat D, Rizwan Qureshi, sridhar siricilla.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77069?usp=email )
Change subject: soc/intel/cse: Implement APIs to access PSR backup status in CMOS ......................................................................
Patch Set 7:
(4 comments)
File src/soc/intel/common/block/cse/cse_lite_cmos.c:
https://review.coreboot.org/c/coreboot/+/77069/comment/c3599319_c74e5550 : PS7, Line 113: int8_t int ?
File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/77069/comment/0001ecaa_03245714 : PS7, Line 176: int8_t int
https://review.coreboot.org/c/coreboot/+/77069/comment/f9fc7f90_e106ef87 : PS7, Line 176: backup_status nit: status or value
the DS name itself is `psr_backup_status` hence, this field can be renamed as value (aka psr_backup_status.value)
File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/77069/comment/b4070b24_c03804e5 : PS5, Line 171: 0x55
Hi Subrata , We kept the values 0x55 and 0x22 to make sure these are not commonly occuring values like 0/1 . So we suggest to have them 0x55/0x22 to ensure it doesn't coincide with default cmos values during bootup
honestly i don't know if 0x55/0x22 has any special meaning. requesting to use some meaningful value as per embedded system programming guidelines.
additionally, you don't need to worry about CMOS default value because you have a signature and CRC field in ur DS which can be used to catch a nude/bare CMOS.