Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38748 )
Change subject: soc/intel/common/block/lpc: Add support to allow all UART options ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38748/1/src/soc/intel/common/block/... File src/soc/intel/common/block/lpc/Kconfig:
https://review.coreboot.org/c/coreboot/+/38748/1/src/soc/intel/common/block/... PS1, Line 17: prompt "Index for COMA UART"
Why the prompt? Doesn't this depend on devicetree or static hardware settings? […]
Right now this information is not in the device tree so we would have to add it. Right now only the generic ranges can be defined there. The code as it is always opens the 3f8 window and opens the 2f8 window if SOC_INTEL_COMMON_BLOCK_LPC_COMB_UART is defined. Which obviously isn't a very good idea.
What I did here was a quick solution for this issue that doesn't require changes for existing boards. If we need to add it to the device tree I am wondering what we should add, just the com ports? All possibilities in the register? Or would it be better to create an interface that can be used by code actually initializing a port. This would then request a range to be opened and the lpc handler can then decide how to deal with it. I guess this would be nice but a lot of work.
What is your suggestion here?
https://review.coreboot.org/c/coreboot/+/38748/1/src/soc/intel/common/block/... PS1, Line 18: DRIVERS_UART_8250IO
I know it's probably derived from line 8, but I don't understand it... […]
TYou only need to open up the com port windows if an IO based UART is used. The MMIO based uarts don't require this. This is also the reason why it is used in line 8.