Pablo Stebler has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42373 )
Change subject: mainboard/hp: Add ProBook 6360b ......................................................................
Patch Set 9:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42373/9/src/ec/hp/kbc1126/ec.c File src/ec/hp/kbc1126/ec.c:
https://review.coreboot.org/c/coreboot/+/42373/9/src/ec/hp/kbc1126/ec.c@121 PS9, Line 121: int timeout = 0x17ff;
Use decimal notation, and add the unit into the name? Also use `size_t` or `unsigned int`.
To be clear, these three lines are copy-pasted from the send_kbc_* functions above, that are almost the same as the ones in src/ec/acpi/ec.c .
https://review.coreboot.org/c/coreboot/+/42373/9/src/ec/hp/kbc1126/ec.c@117 PS9, Line 117: kbc1126_kbdled(conf->ec_ctrl_reg, 0); : : /* The EC needs additional time to process the first command on a cold : boot. */ : int timeout = 0x17ff; : while ((inb(conf->ec_cmd_port) & KBD_IBF) && --timeout) :
It's better to make a separate commit for this. […]
No, the EC only needs a lot more time after receiving the second data value for the LEDs.