[coreboot-gerrit] Change in ...coreboot[master]: src/superio/ite/common: Prepare for ITE IT8786E SuperIO

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30553 )

Change subject: src/superio/ite/common: Prepare for ITE IT8786E SuperIO
......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/#/c/30553/2/src/superio/ite/common/env_ctrl.h 
File src/superio/ite/common/env_ctrl.h:

https://review.coreboot.org/#/c/30553/2/src/superio/ite/common/env_ctrl.h@53 
PS2, Line 53: /* ITE IT8786E has following 16Hz smoothing:
> comment not very helpful. […]

hmmm, maybe put it into a definition?

#define IT8786E_FAN_PWM_SMOOTHING_16HZ  (1 << 6)

Wouldn't look nice and is easy to overlook, but comments
aren't read either in my experience.

https://review.coreboot.org/#/c/30553/2/src/superio/ite/common/env_ctrl.h@60 
PS2, Line 60: #define   ITE_EC_FAN_PWM_SMOOTHING_4HZ		(3 << 6)
or maybe give all of them a different prefix? e.g.
`ITE_EC_FAN_ALT_PWM_SMOOTHING`