Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39766 )
Change subject: drivers/i2c/nct7802y: Configure remote diodes and local sensor ......................................................................
Patch Set 31: Code-Review+1
(1 comment)
Thanks for the update. I only now realized that you put the code amongst the fan init. Any particular reason? I don't think it needs its own file, but would prefer it in `nct7802y.c`, maybe just a small function nct7802y_init_sensors()? But I don't feel strong about it.
https://review.coreboot.org/c/coreboot/+/39766/31/src/drivers/i2c/nct7802y/n... File src/drivers/i2c/nct7802y/nct7802y_fan.c:
https://review.coreboot.org/c/coreboot/+/39766/31/src/drivers/i2c/nct7802y/n... PS31, Line 82: i << 1 Please just write `i * 2`. This is not very readable. Also wouldn't need parentheses.
Could also be a macro, e.g.
#define MODE_SELECTION_RTDx(x, val) ((val) << (x) * 2)