Angel Pons 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 13:
(12 comments)
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... File src/soc/intel/cannonlake/pch_h.c:
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 2: /* This file is part of the coreboot project. */
drop this comment, but leave a blank line between the SPDX identifier and the #includes
Done, and sorted the includes
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 15:
double blank line
Done
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 18: switch (dev->path.pci.devfn)
agreed.
Done
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 20: // UART0
Such useful comments! I don't think we need them, do we? 😄
Done
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 22: struct acpi_irq irq
maybe declare this outside of the switch? that way, no braces are needed.
Done
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 43:
Another double blank line
Done
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 52:
spurious blank line
Done
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 92: switch (dev->path.pci.devfn)
Would be good to fix
Done
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 94: // UART0
We can live without these comments
Done
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 112:
double empty line
Done
https://review.coreboot.org/c/coreboot/+/40405/12/src/soc/intel/cannonlake/p... PS12, Line 119: .acpi_fill_ssdt = pch_h_uart_fill_ssdt,
We can do away with one tab less
Done
https://review.coreboot.org/c/coreboot/+/40405/13/src/soc/intel/cannonlake/p... File src/soc/intel/cannonlake/pch_h.c:
https://review.coreboot.org/c/coreboot/+/40405/13/src/soc/intel/cannonlake/p... PS13, Line 56: AAAAA