Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37324 )
Change subject: [WIP] soc/amd/common: Access ACPI MMIO via proper symbols ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37324/5/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/gpio_banks.h:
https://review.coreboot.org/c/coreboot/+/37324/5/src/soc/amd/common/block/in... PS5, Line 22: #define GPIO_BANK0_CONTROL(gpio) ((uintptr_t)acpimmio_gpio0 + ((gpio) * 4))
nit: add a '- 0' for the offset for consistency with the others?
also maybe align the numerical constants to the right:
#define GPIO_BANK0_CONTROL(gpio) ((uintptr_t)acpimmio_gpio0 + (((gpio) - 0) * 4)) #define GPIO_BANK1_CONTROL(gpio) ((uintptr_t)acpimmio_gpio1 + (((gpio) - 64) * 4)) #define GPIO_BANK2_CONTROL(gpio) ((uintptr_t)acpimmio_gpio2 + (((gpio) - 128) * 4))
It makes sense to align numbers like that, because ones, tens, hundreds, etc. are all on the same column.