Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40405 )
Change subject: soc/intel/common: Add support for LPSS UART in ACPI mode ......................................................................
Patch Set 21:
(9 comments)
https://review.coreboot.org/c/coreboot/+/40405/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40405/8//COMMIT_MSG@9 PS8, Line 9: all
There's no difference between pch-h and pch-lp for uart0 and uart1. […]
Done
https://review.coreboot.org/c/coreboot/+/40405/8//COMMIT_MSG@11 PS8, Line 11: PCH
Not accurate any more.
Done
https://review.coreboot.org/c/coreboot/+/40405/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40405/9//COMMIT_MSG@9 PS9, Line 9: On PCH-H all UARTs can't be used under Windows.
You probably mean "not every UART can be used"?
Done
https://review.coreboot.org/c/coreboot/+/40405/9//COMMIT_MSG@12 PS9, Line 12: ACPI code for the Intel LPSS ACPI driver.
This is a separate topic and should be in its own change. For the above, […]
Done
https://review.coreboot.org/c/coreboot/+/40405/8/src/soc/intel/cannonlake/ac... File src/soc/intel/cannonlake/acpi/serialio.asl:
https://review.coreboot.org/c/coreboot/+/40405/8/src/soc/intel/cannonlake/ac... PS8, Line 61: UAR0
Those entries conflict with the ssdt. You advertise the same device twice.
Done
https://review.coreboot.org/c/coreboot/+/40405/21/src/soc/intel/common/block... File src/soc/intel/common/block/uart/chip.h:
https://review.coreboot.org/c/coreboot/+/40405/21/src/soc/intel/common/block... PS21, Line 1: SPDX-License-Identifier: GPL-2.0-only */ : #include <stdint.h>
It's just for aesthetics
Done
https://review.coreboot.org/c/coreboot/+/40405/21/src/soc/intel/common/block... PS21, Line 3: device/pci_ids.h
Maybe add a comment to let people know this is so that mainboards can use these definitions in the d […]
Done
https://review.coreboot.org/c/coreboot/+/40405/15/src/soc/intel/common/block... File src/soc/intel/common/block/uart/uart.c:
https://review.coreboot.org/c/coreboot/+/40405/15/src/soc/intel/common/block... PS15, Line 230:
Double newline
Done
https://review.coreboot.org/c/coreboot/+/40405/21/src/soc/intel/common/block... File src/soc/intel/common/block/uart/uart.c:
https://review.coreboot.org/c/coreboot/+/40405/21/src/soc/intel/common/block... PS21, Line 284: strncmp
strcmp, maybe? […]
Done