Change in ...coreboot[master]: src/drivers/intel/fsp1_1: Configure UART after memory init

[Michał Żygowski (Code Review)]

Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/28464 )

Change subject: src/drivers/intel/fsp1_1: Configure UART after memory init
......................................................................

Patch Set 2: Code-Review+2
> Patch Set 2: Code-Review-1
>
> Hi Frans,
> This patch doesn't appear to do anything to the UART. Perhaps you forgot to add a file when you committed the patch?
>
> Also, since this seems specific to the board you're working on (since you're using an external UART) can you enable/disable the internal UART in mainboard-specific code? I'm thinking mainboard_romstage_entry() or similar function. That will avoid the need to add the weak symbol to the generic codepath that will be taken on other boards.

Yes, it is mainboard specific, thus the weak hook function. Braswell processors have internal legacy UART which uses IO base 0x3f8 and IRQ4 and this is the UART which is enabled by FSP memory init phase. In case the mainbaord has a SuperIO with UART for example, the serial port LDN on SuperIO must be reinitialized and internal UART disabled. Otherwise no serial output on headless systems. The hook will provide such possibility on mainboard level, while empty weak implementation won't impact any other mainboards. On the other hand, leaving the internal serial port enabled, while the port is not used, may have serious boot time impact on higher debug levels.

Another issue here is that Your offer to disable the port in mainboard_romstage_entry or any other function executed later will lead to a loss of very important debug messages, when debugging with serial port. I mean we will loose the returned status code of memory init, which is very important. Thus the weak function has to be executed right after memory init.

In all aspects the change will bring only benefits.