Attention is currently required from: Raul Rangel, Tim Van Patten, Karthik Ramasubramanian, Mark Hasemeyer.
Jon Murphy has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74110 )
Change subject: mb/google/myst: Add eSPI configuration ......................................................................
Patch Set 6:
(4 comments)
File src/mainboard/google/myst/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/74110/comment/1a802722_3fb3b7df PS6, Line 7: .generic_io_range[0] = { : .base = 0x62, : .size = 1, : }, : .generic_io_range[1] = { : .base = 0x66, : .size = 1, : },
Why doesn't this comment apply to myst also? […]
Phoenix has additional windows. It actually looks like MDN does as well, so I'm not sure why we didn't fix this on Skyrim b/221095488
https://review.coreboot.org/c/coreboot/+/74110/comment/307d81e0_22b753d8 PS6, Line 29: .op_freq_mhz = ESPI_OP_FREQ_33_MHZ,
For initial stability, should we start at 16MHz for bring up and then move up to 33?
Done
https://review.coreboot.org/c/coreboot/+/74110/comment/0bec83fa_5e65d5d9 PS6, Line 31: ESPI_ALERT_PIN_IN_BAND
go/myst-gpios lists this as `ESPI_EC_ALERT_SOC_ODL`, implying this should likely be `ESPI_ALERT_PIN_ […]
Done
https://review.coreboot.org/c/coreboot/+/74110/comment/5b826409_f7630194 PS6, Line 37: ESPI_VW_IRQ_LEVEL_HIGH(12)
IRQ12 was removed from skyrim (https://review.coreboot.org/c/coreboot/+/63704). […]
It's not, old copy paste. I need to cross check this CL against the one you referenced and where Skyrim is today.
-
Jon Murphy (Code Review)