On Wed, Jan 12, 2022 at 08:47:29AM PST, Guenter Roeck wrote:
On Tue, Jan 11, 2022 at 07:48:24PM -0800, Zev Weiss wrote:
These registers report CPU temperatures (and, depending on the system,
sometimes chipset temperatures) via the TSI interface on AMD systems.
They're distinct from most of the other Super-IO temperature readings
(CPUTIN, SYSTIN, etc.) in that they're not a selectable source for
monitoring and are in a different (higher resolution) format, but can
still provide useful temperature data.
Signed-off-by: Zev Weiss <zev@xxxxxxxxxxxxxxxxx>
Tested-by: Renze Nicolai <renze@xxxxxxxxx>
---
[ ... ]
+ switch (data->kind) {
+ case nct6791:
+ case nct6792:
+ case nct6793:
+ data->REG_TSI_TEMP = NCT6776_REG_TSI_TEMP;
+ num_reg_tsi_temp = ARRAY_SIZE(NCT6776_REG_TSI_TEMP);
+ break;
+ case nct6795:
+ case nct6796:
+ case nct6797:
+ case nct6798:
+ data->REG_TSI_TEMP = NCT6796_REG_TSI_TEMP;
+ num_reg_tsi_temp = ARRAY_SIZE(NCT6796_REG_TSI_TEMP);
+ break;
+ default:
+ dev_warn(dev, "unknown number of TSI temp registers for %s\n", data->name);
+ num_reg_tsi_temp = 0;
+ break;
Please drop the message. This is new functionality which should not
suddenly produce a warning for chips which possibly do not support those
registers in the first place. For users with those chips the warning
is just confusing.
Just to confirm, this switch currently covers all the enum cases handled
by the surrounding switch case, so as it stands the default case should
be unreachable and hence it should be guaranteed not to warn for any
existing chip, only for a hypothetical newly-added one (the intent being
that someone adding another chip in the future would be less likely to
accidentally omit TSI support). Unfortunately gcc doesn't seem to be
quite smart enough to realize that and avoid warning about missing
switch cases in the inner switch if I remove the default case entirely,
so we can't just rely on the compiler to warn about it.
Shall I still remove it anyway? (I could certainly also add a comment
clarifying the above.)
Zev
