Prev: [PATCH v2 0/4] timer: Added usleep[_range] timer
Next: [PATCH 2/4] Documentation: Add timers/timers-howto.txt
From: Guenter Roeck on 28 Jul 2010 15:40
On Wed, 2010-07-28 at 15:00 -0400, Fenghua Yu wrote:
> From: Fenghua Yu <fenghua.yu(a)intel.com>
>
> This patch adds a hwmon driver for package level thermal control. The driver
> dumps package level thermal information through sysfs interface so that upper
> level application (e.g. lm_sensor) can retrive the information.
>
> Instead of having the package level hwmon code in coretemp, I write a seperate
> driver pkgtemp because:
>
> First, package level thermal sensors include not only sensors for each core,
> but also sensors for uncore, memory controller or other components in the
> package. Logically it will be clear to have a seperate hwmon driver for package
> level hwmon to monitor wider range of sensors in a package. Merging package
> thermal driver into core thermal driver doesn't make sense and may mislead.
>
> Secondly, merging the two drivers together may cause coding mess. It's easier
> to include various package level sensors info if more sensor information is
> implemented. Coretemp code needs to consider a lot of legacy machine cases.
> Pkgtemp code only considers platform starting from Sandy Bridge.
>
> On a 1Sx4Cx2T Sandy Bridge platform, lm-sensors dumps the pkgtemp and coretemp:
>
> pkgtemp-isa-0000
> Adapter: ISA adapter
> physical id 0: +33.0°C (high = +79.0°C, crit = +99.0°C)
>
> coretemp-isa-0000
> Adapter: ISA adapter
> Core 0: +32.0°C (high = +79.0°C, crit = +99.0°C)
>
> coretemp-isa-0001
> Adapter: ISA adapter
> Core 1: +32.0°C (high = +79.0°C, crit = +99.0°C)
>
> coretemp-isa-0002
> Adapter: ISA adapter
> Core 2: +32.0°C (high = +79.0°C, crit = +99.0°C)
>
> coretemp-isa-0003
> Adapter: ISA adapter
> Core 3: +32.0°C (high = +79.0°C, crit = +99.0°C)
>
> Signed-off-by: Fenghua Yu <fenghua.yu(a)intel.com>
> Reviewed-by: Len Brown <len.brown(a)intel.com>
> Reviewed-by: Guenter Roeck <guenter.roeck(a)ericsson.com>

That I provided feedback does not mean that I endorsed a "Reviewed-by"
signature. It is inappropriate to add such a signature without my
approval.

Guenter


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on 30 Jul 2010 21:00
On 07/29/2010 05:13 PM, Fenghua Yu wrote:
> +
> +enum { SHOW_TEMP, SHOW_TJMAX, SHOW_TTARGET, SHOW_LABEL, SHOW_NAME } SHOW;
> +

This conflicts with an equally poorly named global variable in
drivers/hwmon/via-cputemp.c, and the conflict is causing a build failure.

I think both these drivers have the same bug: a missing "typedef" before
the enum keyword, as present in coretemp.c. Of course, one can question
if it should be given a typename at all since in none of these drivers
they are actually referenced by type, and instead the enumeration is
just used as a source of constants, which can perfectly well be handled
with an unnamed enum:

enum { SHOW_TEMP, SHOW_TJMAX, SHOW_TTARGET, SHOW_LABEL, SHOW_NAME };

-hpa
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Guenter Roeck on 30 Jul 2010 21:20
On Fri, Jul 30, 2010 at 08:57:56PM -0400, H. Peter Anvin wrote:
> On 07/29/2010 05:13 PM, Fenghua Yu wrote:
> > +
> > +enum { SHOW_TEMP, SHOW_TJMAX, SHOW_TTARGET, SHOW_LABEL, SHOW_NAME } SHOW;
> > +
>
> This conflicts with an equally poorly named global variable in
> drivers/hwmon/via-cputemp.c, and the conflict is causing a build failure.
>
> I think both these drivers have the same bug: a missing "typedef" before
> the enum keyword, as present in coretemp.c. Of course, one can question
> if it should be given a typename at all since in none of these drivers

Especially since it isn't really a type name, but a global variable named SHOW.
Type name (also called tag) would be enum SHOW { ... }; .

Guenter

> they are actually referenced by type, and instead the enumeration is
> just used as a source of constants, which can perfectly well be handled
> with an unnamed enum:
>
> enum { SHOW_TEMP, SHOW_TJMAX, SHOW_TTARGET, SHOW_LABEL, SHOW_NAME };
>
> -hpa
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: [PATCH v2 0/4] timer: Added usleep[_range] timer
Next: [PATCH 2/4] Documentation: Add timers/timers-howto.txt