Marc Jones 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)
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?
I'd been thinking of the soc calling the common apci code, but hook works, too.
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 […]
soc_* has been used for functions that hook or call back to the soc specific code. This is a common function. I'll try changing the name to something more specific.
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?
I was being consistent with acpi_cstate_t and acpi_tstate_t definitions.
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.
I was being consistent with acpi_cstate_t and acpi_tstate_t, but they are defined by the acpi library. I thought that the _t was for typedef. I'll change this.