Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40278 )
Change subject: mb/purism/librem_whl: Add new board Librem Mini (WHL-U) ......................................................................
Patch Set 5:
(11 comments)
https://review.coreboot.org/c/coreboot/+/40278/4/src/mainboard/purism/librem... File src/mainboard/purism/librem_whl/acpi/ec.asl:
https://review.coreboot.org/c/coreboot/+/40278/4/src/mainboard/purism/librem... PS4, Line 2: /* This file is part of the coreboot project. */
We dropped these lines (apply to all files)
Done
https://review.coreboot.org/c/coreboot/+/40278/4/src/mainboard/purism/librem... PS4, Line 16: Zero
0, please
Done
https://review.coreboot.org/c/coreboot/+/40278/4/src/mainboard/purism/librem... PS4, Line 21: One
1, please
Done
https://review.coreboot.org/c/coreboot/+/40278/4/src/mainboard/purism/librem... File src/mainboard/purism/librem_whl/acpi/mainboard.asl:
https://review.coreboot.org/c/coreboot/+/40278/4/src/mainboard/purism/librem... PS4, Line 17: Method (_STA, 0, NotSerialized) : { : Return (0x0F) : }
Not needed: https://review.coreboot. […]
Done
https://review.coreboot.org/c/coreboot/+/40278/4/src/mainboard/purism/librem... PS4, Line 24: #include "thermal.asl"
I'd include this from "dsdt.asl" and rename the file to "ac. […]
Done
https://review.coreboot.org/c/coreboot/+/40278/4/src/mainboard/purism/librem... File src/mainboard/purism/librem_whl/acpi/thermal.asl:
https://review.coreboot.org/c/coreboot/+/40278/4/src/mainboard/purism/librem... PS4, Line 42:
nit: Enable ACPI debugging in Linux, and add these two lines to every fan's _STA method right before […]
Ack
https://review.coreboot.org/c/coreboot/+/40278/4/src/mainboard/purism/librem... PS4, Line 115: Device (FAN1)
Two fans?
Ack
https://review.coreboot.org/c/coreboot/+/40278/4/src/mainboard/purism/librem... PS4, Line 159: Device (FAN2)
THREE fans?
Ack
https://review.coreboot.org/c/coreboot/+/40278/4/src/mainboard/purism/librem... PS4, Line 203: Device (FAN3)
FOUR???
Ack
https://review.coreboot.org/c/coreboot/+/40278/4/src/mainboard/purism/librem... PS4, Line 247: Device (FAN4)
Are you sure this isn't a drone?
Ack
https://review.coreboot.org/c/coreboot/+/40278/4/src/mainboard/purism/librem... File src/mainboard/purism/librem_whl/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/40278/4/src/mainboard/purism/librem... PS4, Line 26: Scope (_SB.PCI0.LPCB)
Move this scope inside "acpi/ec. […]
Done
-
Angel Pons (Code Review)