Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42373 )
Change subject: mainboard/hp: Add ProBook 6360b ......................................................................
Patch Set 6: Code-Review+1
(9 comments)
Note that comments can be marked as resolved. All of them need to be resolved to enable submit.
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:
Right. thanks. […]
Done
https://review.coreboot.org/c/coreboot/+/42373/2//COMMIT_MSG@27 PS2, Line 27: TPM
Yes
Ok, if it's alive then it's good enough.
https://review.coreboot.org/c/coreboot/+/42373/5/src/mainboard/hp/snb_ivb_la... File src/mainboard/hp/snb_ivb_laptops/Kconfig:
https://review.coreboot.org/c/coreboot/+/42373/5/src/mainboard/hp/snb_ivb_la... PS5, Line 30: default "probook_6360b" if BOARD_HP_PROBOOK_6360B
nit: use alphabetical order?
Done
https://review.coreboot.org/c/coreboot/+/42373/2/src/mainboard/hp/snb_ivb_la... File src/mainboard/hp/snb_ivb_laptops/variants/6360b/board_info.txt:
https://review.coreboot.org/c/coreboot/+/42373/2/src/mainboard/hp/snb_ivb_la... PS2, Line 6: n
Then, I'd change this to "y"
Done
https://review.coreboot.org/c/coreboot/+/42373/2/src/mainboard/hp/snb_ivb_la... File src/mainboard/hp/snb_ivb_laptops/variants/6360b/gpio.c:
PS2:
Autoport
Ack
https://review.coreboot.org/c/coreboot/+/42373/2/src/mainboard/hp/snb_ivb_la... File src/mainboard/hp/snb_ivb_laptops/variants/6360b/hda_verb.c:
PS2:
Autoport
Ack
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
Ok, please update the comment on the previous line
Done
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
I have checked again when running coreboot and the values are right.
Ack
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
Initially I used the same value for `ec_fan_ctrl_value` as 8460p and 2560p but the fan was always on […]
Weird. Maybe the EC firmware isn't the same.