Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29475 )
Change subject: drivers/i2c/nct7802y: Add new hardware-monitoring IC ......................................................................
Uploaded patch set 6.
(6 comments)
https://review.coreboot.org/#/c/29475/1/src/drivers/i2c/nct7802y/nct7802y.h File src/drivers/i2c/nct7802y/nct7802y.h:
https://review.coreboot.org/#/c/29475/1/src/drivers/i2c/nct7802y/nct7802y.h@... PS1, Line 39: << 0
Just to align things and make it more visible that it's a bit mask.
Done
https://review.coreboot.org/#/c/29475/1/src/drivers/i2c/nct7802y/nct7802y.h@... PS1, Line 55: tab
maybe rename tab to fan, so it's clearer that that's the fan id. […]
Done
https://review.coreboot.org/#/c/29475/1/src/drivers/i2c/nct7802y/nct7802y.h@... PS1, Line 93: unset_mask
clear_mask?
Done
https://review.coreboot.org/#/c/29475/1/src/drivers/i2c/nct7802y/nct7802y_fa... File src/drivers/i2c/nct7802y/nct7802y_fan.c:
https://review.coreboot.org/#/c/29475/1/src/drivers/i2c/nct7802y/nct7802y_fa... PS1, Line 32: 3
use NCT7802Y_FAN_CNT here
Done
https://review.coreboot.org/#/c/29475/1/src/drivers/i2c/nct7802y/nct7802y_fa... PS1, Line 68: div
for the case smart.mode == SMART_FAN_DUTY, div = 1 is likely wrong (comment in chip. […]
Ack, Smart Fan documentation mentions a 255 maximum.
https://review.coreboot.org/#/c/29475/1/src/drivers/i2c/nct7802y/nct7802y_fa... PS1, Line 82: return;
print a warning, when config is 0?
Done
Check moved into nct7802y_init().