Nico Huber 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:
(1 comment)
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"
To answer your last question, yes we've seen cases where the 2nd UART couldn't be configured to the 2f8 default. Reasons have been custom hardware occupying the same address or legacy software requiring another address.
I take that as a `no`, as I asked specifically for a case that uses this code.
It's a long story but I hope this clarifies my earlier remarks.
Yes, I guess. It seems you want to add generic, but unused (at the moment) functionality. IMO, a nice intent, but nothing will prevent it from breaking before anybody makes use of it.
IMHO, we should only support what in-tree boards need. If we wanted to support every possible case of the silicon, we'd be looking forward to a ten times bigger code base.
If we want to fully configure the lpc forwarding from the device tree. I would suggest to add a 16 lpc_io_enables item that overrides the chipset default and a 16 bit lpc_io_decode item that allows control over the io decode ranges.
All necessary information is already (supposed to be) in the device tree. All we need is to fill the registers with it. Have a look at soc/intel/common/block/ lpc/lpc_lib.c, lpc_open_pmio_window(). This code already handles the case for non-standard i/o port ranges. All what's missing is a list of the port ranges supported by the lp_io_decode/_enables registers, to handle these specifically.