Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/36424 )
Change subject: sb600spi.c: Improve IMC runtime detection in handle_imc() ......................................................................
Patch Set 5:
(6 comments)
https://review.coreboot.org/c/flashrom/+/36424/5/sb600spi.c File sb600spi.c:
https://review.coreboot.org/c/flashrom/+/36424/5/sb600spi.c@526 PS5, Line 526: * sb7xx, sp5100: PM_Reg: B0h-B1h) etc. */ Looks like this comment could use an update.
https://review.coreboot.org/c/flashrom/+/36424/5/sb600spi.c@533 PS5, Line 533: /* On Yangtze and newer chips, check the EcEnableStrap bit */ I'm not familiar with the AMD chipsets... but comment above seems to suggest, that the strap existed before Yangtze, why the constraint?
https://review.coreboot.org/c/flashrom/+/36424/5/sb600spi.c@534 PS5, Line 534: spurious space
https://review.coreboot.org/c/flashrom/+/36424/5/sb600spi.c@535 PS5, Line 535: ACPI_MMIO_BASE & 0xfffff000 It's a constant, and this masking is a no-op?
https://review.coreboot.org/c/flashrom/+/36424/5/sb600spi.c@535 PS5, Line 535: AcpiMMIO lower-case, please. e.g. `acpi_mmio`
https://review.coreboot.org/c/flashrom/+/36424/5/sb600spi.c@548 PS5, Line 548: msg_pinfo msg_perr()?