Re: kern/58372: Temperature reading broken on Intel Core 2 Duo E6300 (patch provided)

["David H. Gutteridge" <david%gutteridge.ca@localhost>]

The following reply was made to PR kern/58372; it has been noted by GNATS.

From: "David H. Gutteridge" <david%gutteridge.ca@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: 
Subject: Re: kern/58372: Temperature reading broken on Intel Core 2 Duo E6300
 (patch provided)
Date: Sun, 14 Jul 2024 23:01:37 -0400

 The history of the relevant changes to this driver is a bit hard to
 follow, as things have sometimes been committed over time without much
 in the way of explanation, e.g., what exact hardware was involved,
 whether there was a PR, etc. Stuff like this is very hard to support
 without entirely adequate documentation. (This is all rather old by
 now, as well.)
 
 I don't think we could apply your second proposed patch as-is. It would
 undo one of the intentions of a previous change. I did just remove the
 redundant model check you pointed out.
 
 The original expectation for any hardware that matches the first model
 check is that:
 
 - All mobile processors are 85
 - All desktop processors are 100
 (This is still how it is with the FreeBSD version of this driver.)
 
 Later, further checks were added for more means of distinguishing
 mobile vs. desktop. The intent was to jump to an entirely different
 TjMax sourcing method if not able to definitively identify mobile vs.
 desktop. We shouldn't be dropping either goto. (When I added more
 detailed reporting of errors, I didn't add such where you're proposing
 on purpose, as it seems perhaps too verbose to report a failure there,
 given there's another try still to occur.)
 
 Yes, the check for < 017 is redundant inside that block. It's unclear
 why it was put there (i.e. was it meant to be "stepping" not "model",
 or it was intended to be placed elsewhere, or just a copy and paste
 mistake -- I don't think there could be such a value possible for
 "stepping" here anyway).
 
 Also, we can't universally change the default to 94 inside that block
 before further tries, as the original expectation is 100. We'd need to
 have very specific checks that narrow to your case. In other words,
 something like your first proposed patch. But, the idea across more
 than one OS is that what falls into your processor's generation would
 either be 85 or 100. I believe Linux also does this (I would have to
 look). So I'm a bit hesitant to touch that.
 
 We certainly do appreciate your work here, and I'm not suggesting we
 can't do more. I'd like to get feedback from others before making
 further adjustments.
 
 Thanks,
 
 Dave