Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32531 )
Change subject: mb/51nb: Add support for the 51nb X210 ......................................................................
Patch Set 39:
(11 comments)
Patch Set 37:
please split adding the EC from adding the mainboard
done
https://review.coreboot.org/c/coreboot/+/32531/37/Documentation/mainboard/51... File Documentation/mainboard/51nb/x210.jpg:
PS37:
Please put this image into https://tinyjpg. […]
Done
https://review.coreboot.org/c/coreboot/+/32531/37/src/mainboard/51nb/x210/ac... File src/mainboard/51nb/x210/acpi/battery.asl:
PS37:
Is this file using spaces? The other ACPI files seem affected as well
Done
https://review.coreboot.org/c/coreboot/+/32531/37/src/mainboard/51nb/x210/bo... File src/mainboard/51nb/x210/board.fmd:
https://review.coreboot.org/c/coreboot/+/32531/37/src/mainboard/51nb/x210/bo... PS37, Line 6: FLASH@0xff800000 0x800000
FLASH 8M
Done
https://review.coreboot.org/c/coreboot/+/32531/37/src/mainboard/51nb/x210/de... File src/mainboard/51nb/x210/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/32531/37/src/mainboard/51nb/x210/de... PS37, Line 59: 3
SaGv_Enabled
Done
https://review.coreboot.org/c/coreboot/+/32531/37/src/mainboard/51nb/x210/de... PS37, Line 98: 0x0
Just 0 (apply everywhere)
Done
https://review.coreboot.org/c/coreboot/+/32531/37/src/mainboard/51nb/x210/de... PS37, Line 229:
nit: align the "on" keywords?
Done
https://review.coreboot.org/c/coreboot/+/32531/37/src/mainboard/51nb/x210/ds... File src/mainboard/51nb/x210/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/32531/37/src/mainboard/51nb/x210/ds... PS37, Line 17:
Should be making use of arch/acpi. […]
Done
https://review.coreboot.org/c/coreboot/+/32531/37/src/mainboard/51nb/x210/ds... PS37, Line 30: global
Either drop the comment, or capitalize `Global`
Done
https://review.coreboot.org/c/coreboot/+/32531/37/src/mainboard/51nb/x210/ds... PS37, Line 33: // CPU
This comment provides little value
Done
https://review.coreboot.org/c/coreboot/+/32531/37/src/mainboard/51nb/x210/ds... PS37, Line 36: Scope (_SB) { : Device (PCI0)
Device (_SB.PCI0) […]
Done
https://review.coreboot.org/c/coreboot/+/32531/33/src/mainboard/51nb/x210/hd... File src/mainboard/51nb/x210/hda_verb.c:
https://review.coreboot.org/c/coreboot/+/32531/33/src/mainboard/51nb/x210/hd... PS33, Line 39: /* Pin Complex (NID 0x19) */
Ideally, I've yet to see that happen, though... One could also use higher-level macros as well. […]
Done