Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/cannonlake: Add support for UARTs on PCH-H ......................................................................
Patch Set 9:
(4 comments)
https://review.coreboot.org/c/coreboot/+/40405/9/src/soc/intel/cannonlake/pc... File src/soc/intel/cannonlake/pch_h.c:
PS9:
I generally prefer per-platform drivers if there is anything platform […]
it evolved into something generic. besides _HID and _CID there's nothing special about it.
https://review.coreboot.org/c/coreboot/+/40405/9/src/soc/intel/cannonlake/pc... PS9, Line 28: struct acpi_irq irq = ACPI_IRQ_LEVEL_LOW(LPSS_UART0_IRQ); LPSS_UART1_IRQ
https://review.coreboot.org/c/coreboot/+/40405/9/src/soc/intel/cannonlake/pc... PS9, Line 34: LPSS_UART0_IRQ LPSS_UART2_IRQ
https://review.coreboot.org/c/coreboot/+/40405/9/src/soc/intel/cannonlake/pc... PS9, Line 66: ", according to the ACPI spec 6.1 Chapter 6.1 "For devices on an enumerable type of bus, such as a PCI bus, ... ACPI system firmware must supply an _ADR ..." and "A device object must contain either an _HID object or an _ADR object, but should not contain both."
Should we limit this driver to only generate code when the LPSS Uart works in "ACPI mode" and be a noop when running in "PCI mode"?