Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47531 )
Change subject: soc/intel/common/block/p2sb: Add hpet BDF functions ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47531/6/src/soc/intel/common/block/... File src/soc/intel/common/block/p2sb/p2sb.c:
https://review.coreboot.org/c/coreboot/+/47531/6/src/soc/intel/common/block/... PS6, Line 33: pci_write_config32(PCH_DEV_P2SB, PCI_BASE_ADDRESS_0, P2SB_BAR); nit: put `raw` after the struct?
https://review.coreboot.org/c/coreboot/+/47531/6/src/soc/intel/common/block/... PS6, Line 59: union p2sb_bdf bdf; I'd make these variables constant:
const bool was_hidden = p2sb_is_hidden(); p2sb_unhide(); const union p2sb_bdf bdf = { .raw = pci_read_config16(PCH_DEV_P2SB, PCH_P2SB_HBDF) }; if (was_hidden) p2sb_hide(); return bdf;
Also, please rename `p2sb_state`. Thinking of the P2SB state as "true" or "false" makes my brain hurt. I've suggested `was_hidden`.