Re: [lm-sensors] hwmon: (coretemp) Check the sensor existence

All of lore.kernel.org
 help / color / mirror / Atom feed

* Re: [lm-sensors] hwmon: (coretemp) Check the sensor existence
@ 2010-01-08 16:01 Huaxu Wan
  2010-01-08 18:14 ` Jean Delvare
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Huaxu Wan @ 2010-01-08 16:01 UTC (permalink / raw)
  To: lm-sensors

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

> * 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.

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

-- 
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
                   ` (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

end of thread, other threads:[~2010-01-11  8:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.