Attention is currently required from: Andy Pont, Paul Menzel, Angel Pons. Sean Rhodes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58428 )
Change subject: mb/starlabs/labtop: Add LabTop Mk IV ......................................................................
Patch Set 97:
(7 comments)
File src/mainboard/starlabs/labtop/variants/cml/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/58428/comment/dc3b9091_239df700 PS95, Line 2: # CPU
nit: missing a tab
Done
https://review.coreboot.org/c/coreboot/+/58428/comment/b0a938c0_701bdf3f PS95, Line 79: # Internal Webcam : register "usb2_ports[6]" = "USB2_PORT_MID(OC0)" : # Internal Bluetooth : register "usb2_ports[9]" = "USB2_PORT_MID(OC0)"
Internal USB ports shouldn't be assigned any overcurrent pins
Done
File src/mainboard/starlabs/labtop/variants/cml/gpio.c:
https://review.coreboot.org/c/coreboot/+/58428/comment/a5613e15_73cb48f1 PS95, Line 19: PAD_CFG_GPI(GPP_H7, NONE, PLTRST),
You probably want to configure the pads for UART #2 here as well
Done
File src/mainboard/starlabs/labtop/variants/cml/hda_verb.c:
https://review.coreboot.org/c/coreboot/+/58428/comment/0a0747f5_f8090dad PS95, Line 14: 0x10EC1200
nit: drop value from comment, it's in the code already
Done
File src/mainboard/starlabs/labtop/variants/cml/romstage.c:
https://review.coreboot.org/c/coreboot/+/58428/comment/6e5dea9f_c271d587 PS95, Line 11: u8
Would `unsigned int` work? I'd like to drop the cast in the returned value.
Done
https://review.coreboot.org/c/coreboot/+/58428/comment/7fec130f_39e6fb10 PS95, Line 18: ID3
nit: missing opening parenthesis
Done
https://review.coreboot.org/c/coreboot/+/58428/comment/2567afe8_8ee33422 PS95, Line 23: * 1 1 1 Samsung 4G single channel : * 1 1 0 Samsung 8G dual channel : * 1 0 1 Micron 4G single channel : * 1 0 0 Micron 8G dual channel : * 0 1 1 Hynix 4G single channel : * 0 1 0 Hynix 8G dual channel : * 0 0 1 Micron 16G dual channel : * 0 0 0 Hynix 16G dual channel
nit: Could you please invert the order of the rows? It makes more sense to start with `0 0 0`
Done
-
Sean Rhodes (Code Review)