Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44135 )
Change subject: mb/google/zork: keep the c-state IO base address alignment ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/44135/4/src/soc/amd/picasso/Kconfig File src/soc/amd/picasso/Kconfig:
https://review.coreboot.org/c/coreboot/+/44135/4/src/soc/amd/picasso/Kconfig... PS4, Line 260: PICASSO_CSTATE_IO_BASE_ADDR Does this need to be in the I/O range specified by CONFIG_PICASSO_ACPI_IO_BASE? If so, this should probably be an offset to that base address instead. Does this need to be changed by some board? If not (which is the case I'd assume without having looked closely into this), I'd prefer to have the offset defined in a header file and use that in all places instead of adding another option, that will need to have a constant value
https://review.coreboot.org/c/coreboot/+/44135/4/src/soc/amd/picasso/include... File src/soc/amd/picasso/include/soc/iomap.h:
https://review.coreboot.org/c/coreboot/+/44135/4/src/soc/amd/picasso/include... PS4, Line 72: 0x0c to my other comment: change this to 0x13 to align things with the AGESA side and don't introduce CONFIG_PICASSO_CSTATE_IO_BASE_ADDR?