Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Felix Held. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57051 )
Change subject: soc/amd/common/block/lpc,mb/google/guybrush: Use #defines for eSPI setup ......................................................................
Patch Set 1:
(1 comment)
File src/mainboard/google/guybrush/bootblock.c:
https://review.coreboot.org/c/coreboot/+/57051/comment/a6d72922_21130edb PS1, Line 51: dword = pci_read_config32(SOC_LPC_DEV, LPC_MISC_CONTROL_BITS); : dword &= ~(LDRQ0_PD_EN | LDRQ0_EN | BIT(3)); : dword |= LDRQ0_PU_EN; : pci_write_config32(SOC_LPC_DEV, LPC_MISC_CONTROL_BITS, dword); : : pci_write_config32(SOC_LPC_DEV, LPC_IO_PORT_DECODE_ENABLE, 0); : pci_write_config32(SOC_LPC_DEV, LPC_IO_OR_MEM_DECODE_ENABLE, 0); : : dword = pm_read32(0x90); : dword |= 1 << 16; : pm_write32(0x90, dword); : : dword = pm_read32(PM_ACPI_CONF); : dword |= 3 << 10; : pm_write32(PM_ACPI_CONF, dword); Since you are cleaning this up, I think this should be moved to SoC code. I don't think any of this is guybrush mainboard specific. It would be really easy to read if there were appropriately named helper functions doing this work. Also, helps with avoiding duplication: https://review.coreboot.org/plugins/gitiles/coreboot/+/ad5b8b8ef94be2ed28a62... in guybrush and other mainboards.