* Re: [lm-sensors] hwmon: (coretemp) Check the sensor existence
2010-01-08 16:01 [lm-sensors] hwmon: (coretemp) Check the sensor existence Huaxu Wan
@ 2010-01-08 18:14 ` Jean Delvare
2010-01-11 5:34 ` Huaxu Wan
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Jean Delvare @ 2010-01-08 18:14 UTC (permalink / raw)
To: lm-sensors
On Sat, 9 Jan 2010 00:01:44 +0800, Huaxu Wan wrote:
> On 11:28 Fri 08 Jan, Jean Delvare wrote:
> > Hi Huaxu,
> >
> > On Fri, 8 Jan 2010 17:23:27 +0800, Huaxu Wan wrote:
> > >
> > > hwmon: (coretemp) Check the sensor existence instead of enumeration.
> > >
> > > The processor supports a digital thermal sensor if CPUID.06H.EAX[0] = 1.
> > >
> > > Signed-off-by: Huaxu Wan <huaxu.wan@linux.intel.com>
> > >
> > > ---
> > > Documentation/hwmon/coretemp | 3 ---
> > > drivers/hwmon/coretemp.c | 24 +++++++-----------------
> > > 2 files changed, 7 insertions(+), 20 deletions(-)
> > >
> >
> > Hmm. I don't like it for two reasons:
> >
> > * Which guarantee do we have that "the processor supports a digital
> > thermal sensor" implies that this digital thermal sensor is
> > compatible with what the Core/Core2/Atom have? Couldn't the format
> > change in the future?
>
> It's quote from page 612 of Software Developer’s Manual V3A[1](14.5.5 On
> Die Digital Thermal Sensors). Look from the history, I think it's
> compatible with Core/Core2/Atom and even the feature CPU. Actually, this
> patch has tested on old P4, Core2, i3(launched today?) system, all works
> as expected.
>
> [1] http://www.intel.com/Assets/PDF/manual/253668.pdf
$ wget http://www.intel.com/Assets/PDF/manual/253668.pdf
--2010-01-08 19:05:33-- http://www.intel.com/Assets/PDF/manual/253668.pdf
Resolving www.intel.com... 88.221.93.91, 88.221.93.112
Connecting to www.intel.com|88.221.93.91|:80... connected.
HTTP request sent, awaiting response... 403 Forbidden
2010-01-08 19:05:33 ERROR 403: Forbidden.
$
I am not worried about past models. I am worried about future models
which could feature a different implementation of the thermal sensor.
> > * Each CPU model has its own high temperature limit, which we use to
> > compute the current temperature. So we can't pretend that we support
> > all future CPU models, we really do not. Just look at how complex
> > function adjust_tjmax is.
> >
>
> TjMax varies with specific CPU series, sometimes even different within
> same modle for industrial temperature. At this point, we can't support
> all feature CPU models, but we can support all the released CPUs. I had
> a plan to make an investigation about those.
>
> > It would only make sense to have "universal" support if the driver
> > could report a relative value. Unfortunately our sysfs interface
> > doesn't support this, although there have been discussions on that
> > topic lately:
> >
> > http://thread.gmane.org/gmane.linux.drivers.sensors/20721/focus‘7317
> >
>
> This patch avoid to patch everytime for new release CPU. We foucs on
> the adjust_tjmax function calibration. I think the effort is much less
> than calibration for each CPU model.
Effort is one thing. Telling the user that we support his/her CPU when
we don't really (and thus potentially report wrong temperature values)
is another. I think this is simply not acceptable.
> BTW, I saw openBSD[2] use the CPUID.06H.EAX[0] to identify the sensor.
>
> [2] http://archives.neohapsis.com/archives/openbsd/2007-05/2506.html
I am not opposed to using this bit. What I am opposed to is using this
bit as a replacement for the list of known supported CPU models. This
bit could be used to exit quickly on unsupported models and let the
user know when they have a CPU that isn't yet supported but could be.
That would be better than our current code for that.
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [lm-sensors] hwmon: (coretemp) Check the sensor existence
2010-01-08 16:01 [lm-sensors] hwmon: (coretemp) Check the sensor existence Huaxu Wan
2010-01-08 18:14 ` Jean Delvare
@ 2010-01-11 5:34 ` Huaxu Wan
2010-01-11 6:17 ` Huaxu Wan
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Huaxu Wan @ 2010-01-11 5:34 UTC (permalink / raw)
To: lm-sensors
On 19:14 Fri 08 Jan, Jean Delvare wrote:
> On Sat, 9 Jan 2010 00:01:44 +0800, Huaxu Wan wrote:
> > On 11:28 Fri 08 Jan, Jean Delvare wrote:
> > > Hi Huaxu,
> > >
> > > On Fri, 8 Jan 2010 17:23:27 +0800, Huaxu Wan wrote:
> > > >
> > > > hwmon: (coretemp) Check the sensor existence instead of enumeration.
> > > >
> > > > The processor supports a digital thermal sensor if CPUID.06H.EAX[0] = 1.
> > > >
> > > > Signed-off-by: Huaxu Wan <huaxu.wan@linux.intel.com>
> > > >
> > > > ---
> > > > Documentation/hwmon/coretemp | 3 ---
> > > > drivers/hwmon/coretemp.c | 24 +++++++-----------------
> > > > 2 files changed, 7 insertions(+), 20 deletions(-)
> > > >
> > >
> > > Hmm. I don't like it for two reasons:
> > >
> > > * Which guarantee do we have that "the processor supports a digital
> > > thermal sensor" implies that this digital thermal sensor is
> > > compatible with what the Core/Core2/Atom have? Couldn't the format
> > > change in the future?
> >
> > It's quote from page 612 of Software Developer’s Manual V3A[1](14.5.5 On
> > Die Digital Thermal Sensors). Look from the history, I think it's
> > compatible with Core/Core2/Atom and even the feature CPU. Actually, this
> > patch has tested on old P4, Core2, i3(launched today?) system, all works
> > as expected.
> >
> > [1] http://www.intel.com/Assets/PDF/manual/253668.pdf
>
> $ wget http://www.intel.com/Assets/PDF/manual/253668.pdf
> --2010-01-08 19:05:33-- http://www.intel.com/Assets/PDF/manual/253668.pdf
> Resolving www.intel.com... 88.221.93.91, 88.221.93.112
> Connecting to www.intel.com|88.221.93.91|:80... connected.
> HTTP request sent, awaiting response... 403 Forbidden
> 2010-01-08 19:05:33 ERROR 403: Forbidden.
> $
>
> I am not worried about past models. I am worried about future models
> which could feature a different implementation of the thermal sensor.
>
> > > * Each CPU model has its own high temperature limit, which we use to
> > > compute the current temperature. So we can't pretend that we support
> > > all future CPU models, we really do not. Just look at how complex
> > > function adjust_tjmax is.
> > >
> >
> > TjMax varies with specific CPU series, sometimes even different within
> > same modle for industrial temperature. At this point, we can't support
> > all feature CPU models, but we can support all the released CPUs. I had
> > a plan to make an investigation about those.
> >
> > > It would only make sense to have "universal" support if the driver
> > > could report a relative value. Unfortunately our sysfs interface
> > > doesn't support this, although there have been discussions on that
> > > topic lately:
> > >
> > > http://thread.gmane.org/gmane.linux.drivers.sensors/20721/focus‘7317
> > >
> >
> > This patch avoid to patch everytime for new release CPU. We foucs on
> > the adjust_tjmax function calibration. I think the effort is much less
> > than calibration for each CPU model.
>
> Effort is one thing. Telling the user that we support his/her CPU when
> we don't really (and thus potentially report wrong temperature values)
> is another. I think this is simply not acceptable.
>
> > BTW, I saw openBSD[2] use the CPUID.06H.EAX[0] to identify the sensor.
> >
> > [2] http://archives.neohapsis.com/archives/openbsd/2007-05/2506.html
>
> I am not opposed to using this bit. What I am opposed to is using this
> bit as a replacement for the list of known supported CPU models. This
> bit could be used to exit quickly on unsupported models and let the
> user know when they have a CPU that isn't yet supported but could be.
> That would be better than our current code for that.
>
I agree with you that this patch introduce some potential issues for
the old CPU models. Thank you. I'll confirm with CPU team about
CPUID.06H.EAX[0] bit for feature models first before any action.
--
Thanks
Huaxu
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [lm-sensors] hwmon: (coretemp) Check the sensor existence
2010-01-08 16:01 [lm-sensors] hwmon: (coretemp) Check the sensor existence Huaxu Wan
2010-01-08 18:14 ` Jean Delvare
2010-01-11 5:34 ` Huaxu Wan
@ 2010-01-11 6:17 ` Huaxu Wan
2010-01-11 6:29 ` Adam Nielsen
2010-01-11 8:35 ` Jean Delvare
4 siblings, 0 replies; 6+ messages in thread
From: Huaxu Wan @ 2010-01-11 6:17 UTC (permalink / raw)
To: lm-sensors
On 19:14 Fri 08 Jan, Jean Delvare wrote:
> On Sat, 9 Jan 2010 00:01:44 +0800, Huaxu Wan wrote:
> > It's quote from page 612 of Software Developer’s Manual V3A[1](14.5.5 On
> > Die Digital Thermal Sensors). Look from the history, I think it's
> > compatible with Core/Core2/Atom and even the feature CPU. Actually, this
> > patch has tested on old P4, Core2, i3(launched today?) system, all works
> > as expected.
> >
> > [1] http://www.intel.com/Assets/PDF/manual/253668.pdf
>
> $ wget http://www.intel.com/Assets/PDF/manual/253668.pdf
> --2010-01-08 19:05:33-- http://www.intel.com/Assets/PDF/manual/253668.pdf
> Resolving www.intel.com... 88.221.93.91, 88.221.93.112
> Connecting to www.intel.com|88.221.93.91|:80... connected.
> HTTP request sent, awaiting response... 403 Forbidden
> 2010-01-08 19:05:33 ERROR 403: Forbidden.
> $
I can access the above link at external, I don't know why. Try the link:
http://www.intel.com/products/processor/manuals/ or google "253668.pdf".
--
Thanks
Huaxu
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [lm-sensors] hwmon: (coretemp) Check the sensor existence
2010-01-08 16:01 [lm-sensors] hwmon: (coretemp) Check the sensor existence Huaxu Wan
` (2 preceding siblings ...)
2010-01-11 6:17 ` Huaxu Wan
@ 2010-01-11 6:29 ` Adam Nielsen
2010-01-11 8:35 ` Jean Delvare
4 siblings, 0 replies; 6+ messages in thread
From: Adam Nielsen @ 2010-01-11 6:29 UTC (permalink / raw)
To: lm-sensors
>>> [1] http://www.intel.com/Assets/PDF/manual/253668.pdf
>> $ wget http://www.intel.com/Assets/PDF/manual/253668.pdf
>> --2010-01-08 19:05:33-- http://www.intel.com/Assets/PDF/manual/253668.pdf
>> Resolving www.intel.com... 88.221.93.91, 88.221.93.112
>> Connecting to www.intel.com|88.221.93.91|:80... connected.
>> HTTP request sent, awaiting response... 403 Forbidden
>> 2010-01-08 19:05:33 ERROR 403: Forbidden.
>> $
>
> I can access the above link at external, I don't know why. Try the link:
> http://www.intel.com/products/processor/manuals/ or google "253668.pdf".
Try this instead:
$ wget http://www.intel.com/Assets/PDF/manual/253668.pdf --user-agent=""
Fixes the 403 for me :-)
Cheers,
Adam.
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [lm-sensors] hwmon: (coretemp) Check the sensor existence
2010-01-08 16:01 [lm-sensors] hwmon: (coretemp) Check the sensor existence Huaxu Wan
` (3 preceding siblings ...)
2010-01-11 6:29 ` Adam Nielsen
@ 2010-01-11 8:35 ` Jean Delvare
4 siblings, 0 replies; 6+ messages in thread
From: Jean Delvare @ 2010-01-11 8:35 UTC (permalink / raw)
To: lm-sensors
Hi Adam,
On Mon, 11 Jan 2010 16:29:44 +1000, Adam Nielsen wrote:
> >>> [1] http://www.intel.com/Assets/PDF/manual/253668.pdf
> >> $ wget http://www.intel.com/Assets/PDF/manual/253668.pdf
> >> --2010-01-08 19:05:33-- http://www.intel.com/Assets/PDF/manual/253668.pdf
> >> Resolving www.intel.com... 88.221.93.91, 88.221.93.112
> >> Connecting to www.intel.com|88.221.93.91|:80... connected.
> >> HTTP request sent, awaiting response... 403 Forbidden
> >> 2010-01-08 19:05:33 ERROR 403: Forbidden.
> >> $
> >
> > I can access the above link at external, I don't know why. Try the link:
> > http://www.intel.com/products/processor/manuals/ or google "253668.pdf".
>
> Try this instead:
>
> $ wget http://www.intel.com/Assets/PDF/manual/253668.pdf --user-agent=""
>
> Fixes the 403 for me :-)
Probably Intel protecting themselves from web suckers. Thanks for the
tip :)
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 6+ messages in thread