Pablo Stebler has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42373 )
Change subject: mainboard/hp: Add ProBook 6360b ......................................................................
Patch Set 3:
(8 comments)
https://review.coreboot.org/c/coreboot/+/42373/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42373/2//COMMIT_MSG@8 PS2, Line 8:
Please mention how you did this. […]
I used autoport and moved the generated code into a variant. I added a link to the full autoport output in the commit message.
https://review.coreboot.org/c/coreboot/+/42373/2/src/mainboard/hp/snb_ivb_la... File src/mainboard/hp/snb_ivb_laptops/variants/6360b/early_init.c:
https://review.coreboot.org/c/coreboot/+/42373/2/src/mainboard/hp/snb_ivb_la... PS2, Line 13: { 1, 1, 0 }, /* left front */
Are these comments correct?
Yes, they are based on the output of lsusb -t.
https://review.coreboot.org/c/coreboot/+/42373/2/src/mainboard/hp/snb_ivb_la... PS2, Line 31: lpc47n217_enable_serial(SERIAL_DEV, CONFIG_TTYS0_BASE);
The board does not seem to have any serial port
The serial port is on the docking station, which I don't have.
https://review.coreboot.org/c/coreboot/+/42373/2/src/mainboard/hp/snb_ivb_la... File src/mainboard/hp/snb_ivb_laptops/variants/6360b/gma-mainboard.ads:
https://review.coreboot.org/c/coreboot/+/42373/2/src/mainboard/hp/snb_ivb_la... PS2, Line 11: ports : constant Port_List :=
Are all of these video ports accessible?
The laptop has one DP port and one VGA, the docking station has two DP (and some analog ports). The internal screen is single-channel LVDS. I assume that all the DP ports are DP++.
https://review.coreboot.org/c/coreboot/+/42373/2/src/mainboard/hp/snb_ivb_la... File src/mainboard/hp/snb_ivb_laptops/variants/6360b/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/42373/2/src/mainboard/hp/snb_ivb_la... PS2, Line 20: 0x3b
This mask enables ports 0, 1, 3, 4, 5
Autoport detected 1, 3, 4, 5. 0 and 1 have been checked manually. Out of 2, 3, 4, 5, one is unused, one is eSATA and the other two are for the docking station.
https://review.coreboot.org/c/coreboot/+/42373/2/src/mainboard/hp/snb_ivb_la... PS2, Line 22: device pci 1c.0 on end # PCIe Port #1
Are PCIe ports described correctly?
Yes, using lspci.
https://review.coreboot.org/c/coreboot/+/42373/2/src/mainboard/hp/snb_ivb_la... PS2, Line 38: register "ec_data_port" = "0x60" : register "ec_cmd_port" = "0x64" : register "ec_ctrl_reg" = "0xca" : register "ec_fan_ctrl_value" = "0x6e" : device pnp ff.1 off end
Did you check the EC settings?
Yes, I have checked them using radare2 on the EcThermalInit EFI module.
https://review.coreboot.org/c/coreboot/+/42373/2/src/mainboard/hp/snb_ivb_la... PS2, Line 44: chip superio/smsc/lpc47n217 : device pnp 4e.3 on # Parallel : io 0x60 = 0x378 : irq 0x70 = 7 : end : device pnp 4e.4 on # COM1 : io 0x60 = 0x3f8 : irq 0x70 = 4 : end : device pnp 4e.5 off end # COM2 : end
I can't see any of these on the board
They are on the docking station.