Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42523 )
Change subject: [WIP] soc/amd/common: Place guards for ACPIMMIO on ENV_ARM ......................................................................
Patch Set 7:
(3 comments)
https://review.coreboot.org/c/coreboot/+/42523/7/src/soc/amd/common/block/ac... File src/soc/amd/common/block/acpimmio/mmio_util.c:
https://review.coreboot.org/c/coreboot/+/42523/7/src/soc/amd/common/block/ac... PS7, Line 32: DECLARE_ACPIMMIO_BANK No reason to use DECLARE_ACPIMMIO_BANK in this block.
https://review.coreboot.org/c/coreboot/+/42523/7/src/soc/amd/common/block/ac... File src/soc/amd/common/block/acpimmio/psp_verstage_stub.c:
PS7: I would just remove these since we can directly set acpimmio_iomux from psp_verstage.
https://review.coreboot.org/c/coreboot/+/42523/7/src/soc/amd/picasso/i2c.c File src/soc/amd/picasso/i2c.c:
https://review.coreboot.org/c/coreboot/+/42523/7/src/soc/amd/picasso/i2c.c@5... PS7, Line 55: switch (dev->path.mmio.addr) {
You could just use the i2c_bus_address[x] values? I don't think those case statements will hold up, […]
This is also why I split it off into a different file. It's only needed when generating ACPI tables, which verstage doesn't do.