Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40322 )
Change subject: [WIP] [UNTESTED] soc/amd/picasso: Clean up legacy UART config ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/40322/4/src/soc/amd/picasso/include... File src/soc/amd/picasso/include/soc/southbridge.h:
https://review.coreboot.org/c/coreboot/+/40322/4/src/soc/amd/picasso/include... PS4, Line 225: #define FCH_LEGACY_UART_MAP_SHIFT 8 nit: If these are bitfield definitions, I'd indent them with 2 spaces as done in the rest of the file
https://review.coreboot.org/c/coreboot/+/40322/4/src/soc/amd/picasso/uart.c File src/soc/amd/picasso/uart.c:
https://review.coreboot.org/c/coreboot/+/40322/4/src/soc/amd/picasso/uart.c@... PS4, Line 63: leg it's a warlking UART! 😄
https://review.coreboot.org/c/coreboot/+/40322/4/src/soc/amd/picasso/uart.c@... PS4, Line 82: const uint8_t range_idx[ARRAY_SIZE(uart_info)] = { : FCH_LEGACY_UART_RANGE_3F8, : FCH_LEGACY_UART_RANGE_2F8, : FCH_LEGACY_UART_RANGE_3E8, : FCH_LEGACY_UART_RANGE_2E8, : }; How about adding this information to uart_info instead?