Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46259 )
Change subject: intel/common/pmc: Add functions for IPC mailbox in ACPI ......................................................................
Patch Set 10:
(5 comments)
https://review.coreboot.org/c/coreboot/+/46259/10/src/soc/intel/common/block... File src/soc/intel/common/block/pmc/pmc_ipc.c:
https://review.coreboot.org/c/coreboot/+/46259/10/src/soc/intel/common/block... PS10, Line 112: FIELDLIST_NAMESTR("IWB3", 32), /* Write Buffer 3 */ I wonder if the results contained in the read buffer are ever worth keeping around? I see they weren't used in the .asl file, but does it provide additional status beyond the IERR bit?
https://review.coreboot.org/c/coreboot/+/46259/10/src/soc/intel/common/block... PS10, Line 164: Local0 Local1
https://review.coreboot.org/c/coreboot/+/46259/10/src/soc/intel/common/block... PS10, Line 172: 0 Should this be a #define in the .h file? So if the results of the IPCW call are needed elsehwere they can be referenced from here, same with error & timeout?
https://review.coreboot.org/c/coreboot/+/46259/10/src/soc/intel/common/block... PS10, Line 181: Local0 Local1
https://review.coreboot.org/c/coreboot/+/46259/10/src/soc/intel/common/block... PS10, Line 215: acpigen_write_integer acpigen_write_dword? probably doesn't matter, it does know the write buffer fields are 32 bits wide.