Michael has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44165 )
Change subject: superio/ite: configure EC for fans to full at thermal limit ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/44165/1/src/mainboard/asus/f2a85-m/... File src/mainboard/asus/f2a85-m/devicetree_f2a85-m.cb:
https://review.coreboot.org/c/coreboot/+/44165/1/src/mainboard/asus/f2a85-m/... PS1, Line 52: register "FAN1.smart.full_lmt" = "0"
Default for devicetree options is 0, so you can avoid having to add it everywhere 😊
Thanks! I was unsure about that. Should we keep it anyway?
https://review.coreboot.org/c/coreboot/+/44165/1/src/superio/ite/common/env_... File src/superio/ite/common/env_ctrl.c:
https://review.coreboot.org/c/coreboot/+/44165/1/src/superio/ite/common/env_... PS1, Line 151: ITE_EC_FAN_CTL_FULL_AT_THRML_LMT(conf->full_lmt));
this overwrites what the function call in the lines above set. […]
Ouch. The OR way sounds good. Thanks!
https://review.coreboot.org/c/coreboot/+/44165/1/src/superio/ite/common/env_... File src/superio/ite/common/env_ctrl_chip.h:
https://review.coreboot.org/c/coreboot/+/44165/1/src/superio/ite/common/env_... PS1, Line 60: limit */
that should fit all in one line; the characters per line limit was raised to 96 some time ago
Done. The tmp_delta line could also be squeezed into 96 characters but then for aligning the */ on other lines they would be preceded by mixed tabs/spaces.