Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48246 )
Change subject: soc/intel/common/block/acpi: Add soc MADT IOAPIC callback ......................................................................
Patch Set 8:
(4 comments)
I'm still not convinced that a weak implementation does anything better than a Kconfig symbol, but I don't mind too much.
https://review.coreboot.org/c/coreboot/+/48246/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48246/8//COMMIT_MSG@7 PS8, Line 7: callback Why call it `callback`, isn't it just a hook? The soc code didn't call, did it?
https://review.coreboot.org/c/coreboot/+/48246/8/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/acpi.h:
https://review.coreboot.org/c/coreboot/+/48246/8/src/soc/intel/common/block/... PS8, Line 66: calculate_power Please find a more specific name, e.g. soc_calculate_package_power() (I'm guessing it's about the package), it's a global symbol after all.
https://review.coreboot.org/c/coreboot/+/48246/8/src/soc/intel/common/block/... PS8, Line 72: __packed Why is this packed? Isn't it more efficient to let the compiler decide?
https://review.coreboot.org/c/coreboot/+/48246/8/src/soc/intel/common/block/... PS8, Line 72: acpi_ioapic_t We usually don't do typedefs and the _t suffix is reserved for standard libraries, AFAIK.