Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31616 )
Change subject: superio/ite/common: add option for enabling 5 FANs ......................................................................
Patch Set 3:
(1 comment)
Do you want to keep it from compiling unnecessary code?
Yes. I've recently played a bit with Secure Boot and noticed that every byte in IBB counts, maybe I'm too paranoid about that though.
So is it arguable? I don't think this code will ever be put into a constrained environment (it should only be used in ramstage, I guess). And removing the #if's would make the whole thing easier to read, IMHO.
https://review.coreboot.org/#/c/31616/3/src/superio/ite/common/env_ctrl.h File src/superio/ite/common/env_ctrl.h:
https://review.coreboot.org/#/c/31616/3/src/superio/ite/common/env_ctrl.h@66 PS3, Line 66: ) There is still a forest of parentheses that only distracts they eye, IMHO. If this works, a simple expression should work, too, and we could even align the register offsets somewhat with those of the simpler definitions, e.g. #define ITE_EC_FAN_TAC_LIMIT(x) ((x) ? 0x84 + ((x)-4) * 2 : 0x10 + ((x)-1))