Bill XIE has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41390 )
Change subject: mainboard/lenovo/x230: Add ThinkPad x230s as a variant ......................................................................
Patch Set 16:
(20 comments)
https://review.coreboot.org/c/coreboot/+/41390/16//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41390/16//COMMIT_MSG@12 PS16, Line 12: pci-e
PCIe
Done
https://review.coreboot.org/c/coreboot/+/41390/16//COMMIT_MSG@13 PS16, Line 13: n
Capitalize all elements of the list?
Done
https://review.coreboot.org/c/coreboot/+/41390/16//COMMIT_MSG@14 PS16, Line 14: tpm
TPM
Done
https://review.coreboot.org/c/coreboot/+/41390/16//COMMIT_MSG@18 PS16, Line 18: DIMM 8GiB
8 GiB SO-DIMM
Done
https://review.coreboot.org/c/coreboot/+/41390/16//COMMIT_MSG@20 PS16, Line 20: usb2
USB2
Done
https://review.coreboot.org/c/coreboot/+/41390/16//COMMIT_MSG@20 PS16, Line 20: pci-e
PCIe
Done
https://review.coreboot.org/c/coreboot/+/41390/16//COMMIT_MSG@21 PS16, Line 21: wwan
WWAN
Done
https://review.coreboot.org/c/coreboot/+/41390/16//COMMIT_MSG@21 PS16, Line 21:
two spaces
Done
https://review.coreboot.org/c/coreboot/+/41390/16//COMMIT_MSG@21 PS16, Line 21: usb2
USB2
Done
https://review.coreboot.org/c/coreboot/+/41390/16//COMMIT_MSG@21 PS16, Line 21: sata
SATA
Done
https://review.coreboot.org/c/coreboot/+/41390/16//COMMIT_MSG@22 PS16, Line 22: pci-e
PCIe
Done
https://review.coreboot.org/c/coreboot/+/41390/16//COMMIT_MSG@24 PS16, Line 24: graphic
I'd use plural here, "graphics init"
Done
https://review.coreboot.org/c/coreboot/+/41390/16//COMMIT_MSG@27 PS16, Line 27: Thinkpad
ThinkPad
Done
https://review.coreboot.org/c/coreboot/+/41390/16//COMMIT_MSG@30 PS16, Line 30: Seabios
SeaBIOS […]
Done
https://review.coreboot.org/c/coreboot/+/41390/16/Documentation/mainboard/le... File Documentation/mainboard/lenovo/x230s.md:
https://review.coreboot.org/c/coreboot/+/41390/16/Documentation/mainboard/le... PS16, Line 1:
missing "ThinkPad"
Done
https://review.coreboot.org/c/coreboot/+/41390/16/Documentation/mainboard/le... PS16, Line 11: Its
I assume the "Its" possessive pronoun refers to "the ThinkPad X230s", but it is not clear at all. […]
Done
https://review.coreboot.org/c/coreboot/+/41390/16/Documentation/mainboard/le... PS16, Line 13: mine
not sure if documentation should refer to the author. It's not directly visible in doc.cb. […]
Done
https://review.coreboot.org/c/coreboot/+/41390/16/src/mainboard/lenovo/x230/... File src/mainboard/lenovo/x230/variants/x230s/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/41390/16/src/mainboard/lenovo/x230/... PS16, Line 17: 0x0065
0x0065 = 101, this is redundant
Done
https://review.coreboot.org/c/coreboot/+/41390/16/src/mainboard/lenovo/x230/... PS16, Line 21:
This should be: […]
They may be added in CB:41505, but is 0x3 really necessary? since xhci port3 of X230s is not exposed anywhere.
https://review.coreboot.org/c/coreboot/+/41390/16/src/mainboard/lenovo/x230/... PS16, Line 23: #
Huh? An empty comment?
Done