Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42136 )
Change subject: superio/nuvoton/nct6776: Add HWM API ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/42136/1/src/mainboard/asrock/b85m_p... File src/mainboard/asrock/b85m_pro4/bootblock.c:
https://review.coreboot.org/c/coreboot/+/42136/1/src/mainboard/asrock/b85m_p... PS1, Line 61: nct6776_hwm_program_speed_cruise(HWM_BASE, CPU_FAN, &scp); Note to self: Since GPIOs aren't configured yet here, tach readings are zero, so the fan will ramp up until ramstage.
https://review.coreboot.org/c/coreboot/+/42136/1/src/superio/nuvoton/nct6776... File src/superio/nuvoton/nct6776/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/42136/1/src/superio/nuvoton/nct6776... PS1, Line 5: all-
would you please explain why it is needed for "all"?
It's not strictly needed for all stages, but I'd rather give some flexibility in case someone wants to configure fan control inside superio.c in ramstage.
I currently do fan setup in bootblock, before raminit happens. Before I wrote the HWM API, I was forcing the fan speed to full on bootblock, half power on romstage and minimum power on ramstage, so I needed the code on three different stages.
https://review.coreboot.org/c/coreboot/+/42136/1/src/superio/nuvoton/nct6776... File src/superio/nuvoton/nct6776/nct6776_hwm.c:
https://review.coreboot.org/c/coreboot/+/42136/1/src/superio/nuvoton/nct6776... PS1, Line 29: nct6776_hwm_force_fan_full_on
isn't there a way to use only the older "fan values"? […]
The point of this function is to "disengage" fan control. That is, make a fan spin up at constant full speed while reconfiguring automatic control. That way, if something bad happens when reconfiguring the SuperIO, there's no risk of overheating.
About using older fan values, if the fan speeds are automatically controlled, it doesn't matter 😉