Attention is currently required from: Damien Zammit, Michał Żygowski, Paul Menzel, Angel Pons, Michael Niewöhner, Piotr Król, Felix Held. Michał Kopeć has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63672 )
Change subject: superio/ite/common: Add support for SuperIOs with 6 temperature reading registers ......................................................................
Patch Set 2:
(8 comments)
Commit Message:
PS1:
Please wrap lines at 72 characters. For the commit summary, try to abbreviate it a bit.
Fixed line wrapping and split the changes into 2 sommits, so this commit message is not as long anymore
https://review.coreboot.org/c/coreboot/+/63672/comment/806cf913_9776d803 PS1, Line 14: Additionally, decouple temperature offset/min/max configuration from TMPIN : mode configuration, since in these SIOs, TMPINs do not have to map directly : to temperature reading registers (e.g. TMPIN1 can read to temperature reading : register 3).
+1, this can and should be done in two commits: […]
Moved TMPIN/temp config decoupling to CB:63700
https://review.coreboot.org/c/coreboot/+/63672/comment/d1a8c018_fd9fd126 PS1, Line 19: Add a Kconfig option SUPERIO_ITE_ENV_CTRL_6_TEMPS for these ECs.
Is there no way to determin the number of registers at run-time?
I don't know of a way to determine this, but as Angel says, it's known at build time anyway
Patchset:
PS2: Moved the TMPIN / temperature reg config split to CB:63700
File src/superio/ite/common/env_ctrl.c:
https://review.coreboot.org/c/coreboot/+/63672/comment/f286766e_e68c1e21 PS1, Line 125: int
bool?
Done
https://review.coreboot.org/c/coreboot/+/63672/comment/74f3d0c4_601ec8ff PS1, Line 127: if (source >= THERMAL_SOURCE_PECI1 && source <= THERMAL_SOURCE_PECI5) : return 1; : : return 0;
Something like: […]
Yes... done.
https://review.coreboot.org/c/coreboot/+/63672/comment/ab26471c_a07f12ab PS1, Line 138:
nit: drop blank line
Done
https://review.coreboot.org/c/coreboot/+/63672/comment/3d635d53_12e597c8 PS1, Line 145: temp - 1
Looks like you could refactor this to avoid the `- 1` everywhere.
Done