Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30360 )
Change subject: src/mainboard/libretrend/lt1000: Initial commit ......................................................................
Patch Set 13:
(12 comments)
https://review.coreboot.org/c/coreboot/+/30360/13/Documentation/mainboard/li... File Documentation/mainboard/libretrend/lt1000.jpg:
PS13: This image is a bit too grainy for its size. Any better options?
https://review.coreboot.org/c/coreboot/+/30360/13/Documentation/mainboard/li... File Documentation/mainboard/libretrend/lt1000.md:
https://review.coreboot.org/c/coreboot/+/30360/13/Documentation/mainboard/li... PS13, Line 60: LVDS header It's not going to work with libgfxinit, though. It's disabled.
https://review.coreboot.org/c/coreboot/+/30360/13/Documentation/mainboard/li... PS13, Line 61: onboard Capitalize this: "Onboard"
https://review.coreboot.org/c/coreboot/+/30360/13/Documentation/mainboard/li... PS13, Line 102: Super I/O, EC Is it both?
https://review.coreboot.org/c/coreboot/+/30360/13/src/mainboard/libretrend/l... File src/mainboard/libretrend/lt1000/acpi/ec.asl:
PS13:
can be dropped?
I think this file must be here.
https://review.coreboot.org/c/coreboot/+/30360/13/src/mainboard/libretrend/l... File src/mainboard/libretrend/lt1000/acpi_tables.c:
PS13:
pretty sure this can be dropped
Definitely.
https://review.coreboot.org/c/coreboot/+/30360/13/src/mainboard/libretrend/l... File src/mainboard/libretrend/lt1000/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/30360/13/src/mainboard/libretrend/l... PS13, Line 65: 3 There's an enum for this
https://review.coreboot.org/c/coreboot/+/30360/13/src/mainboard/libretrend/l... File src/mainboard/libretrend/lt1000/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/30360/13/src/mainboard/libretrend/l... PS13, Line 28: // Some generic macros This comment got dropped
https://review.coreboot.org/c/coreboot/+/30360/13/src/mainboard/libretrend/l... PS13, Line 34: // CPU This comment is not useful
https://review.coreboot.org/c/coreboot/+/30360/13/src/mainboard/libretrend/l... PS13, Line 37: Scope (_SB) { : Device (PCI0) Device (_SB.PCI0)
https://review.coreboot.org/c/coreboot/+/30360/13/src/mainboard/libretrend/l... PS13, Line 45: // Chipset specific sleep states This got dropped as well.
https://review.coreboot.org/c/coreboot/+/30360/3/src/mainboard/libretrend/lt... File src/mainboard/libretrend/lt1000/romstage.c:
https://review.coreboot.org/c/coreboot/+/30360/3/src/mainboard/libretrend/lt... PS3, Line 26: #include <console/console.h>
not used? […]
Done
-
Angel Pons (Code Review)