Attention is currently required from: Jason Glenesk, Raul Rangel. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50270 )
Change subject: soc/amd/cezanne/fch: add ACPI I/O port setup ......................................................................
Patch Set 1:
(2 comments)
File src/soc/amd/cezanne/include/soc/iomap.h:
https://review.coreboot.org/c/coreboot/+/50270/comment/9019ad61_62d6d95c PS1, Line 19: 0x0400
Curious why not use CONFIG_CEZANNE_ACPI_IO_BASE. 0x400 should be fine though.
I don't really expect that to need to be changed and just to avoid clutter in the kconfig. this is also the only base address in the io space that is in kconfig in picasso and no board overrides that; the rest of the io base addresses are in there as defines, so this makes it more consistent. it also removes the chance to have overlapping io regions due to misconfiguration in the kconfig. i'm planning to port this change back to picasso as well
https://review.coreboot.org/c/coreboot/+/50270/comment/d9c48cfd_67c0496b PS1, Line 25: 0x10
This differs from Picasso. […]
i commented on the patch you linked that that doesn't line up with what the reference code does and try to keep this closely to the reference code, since we did run into some issue on picasso due to using a value different to what the reference code did. i also opened a ticket for the ppr to have it aligned with the reference code. have a look at the values of PcdAmdFchCfgAcpiPm1EvtBlkAddr and friends in the corresponding .dec file. i probably should write a patch for picasso as well, but that one definitely needs to be tested on different hardware
-
Felix Held (Code Review)