Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30360 )
Change subject: src/mainboard/libretrend/lt1000: Initial commit ......................................................................
Patch Set 13:
(8 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. […]
Hmm, my intention was to show as much as possible. This mainboard has so many option to connect peripherals that it is hard to show them all. Secondly, I did not wan't the image to be too big, so maybe I went overboard with JPG quality reduction. Suggestions welcome
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 47: Fastboot
What is that?
MRC cache to speed up ram training
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.
This is a general purpose mainboard put in generic case. This is rather an information that LVDS header is and was not tested at all.
https://review.coreboot.org/c/coreboot/+/30360/13/Documentation/mainboard/li... PS13, Line 62: - speakers and mic header
Judging from the missing verbs, this does not work?
Actually, Linux is able to initialize the codec and the audio works somehow. Verbs are missing because I haven't yest found time to analyze how to create them. Any pointers welcome.
https://review.coreboot.org/c/coreboot/+/30360/13/Documentation/mainboard/li... PS13, Line 102: Super I/O, EC
Is it both?
Kind of. It is Super I/O with integrated Environmental Controller (EC). Not the same as Embedded Controller thou. Should I leave it as Super I/O only?
https://review.coreboot.org/c/coreboot/+/30360/13/src/mainboard/libretrend/l... File src/mainboard/libretrend/lt1000/acpi/ec.asl:
PS13:
I think this file must be here.
Indeed. Otherwise it is not building.
https://review.coreboot.org/c/coreboot/+/30360/13/src/mainboard/libretrend/l... File src/mainboard/libretrend/lt1000/acpi/mainboard.asl:
https://review.coreboot.org/c/coreboot/+/30360/13/src/mainboard/libretrend/l... PS13, Line 1: /*
Looks like a lot of boards use that, so it should be put in some common location.
IIRC there was a discussion some time ago about correctness of adding power button in ACPI. I will look for it and see what was it about.
https://review.coreboot.org/c/coreboot/+/30360/13/src/mainboard/libretrend/l... PS13, Line 21: Name (PBST, One)
What is this good for?
Probably nothing. Copy-paste error.