linux-kernel - Re: [PATCH] x86/hwmon: fix configuration and initialization of coretemp and pkgtemp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 03 Sep 2010 08:08:37 +0100
From:	"Jan Beulich" <JBeulich@...ell.com>
To:	"Fenghua Yu" <fenghua.yu@...el.com>
Cc:	"r.marek@...embler.cz" <r.marek@...embler.cz>,
	"mingo@...e.hu" <mingo@...e.hu>,
	"tglx@...utronix.de" <tglx@...utronix.de>,
	"lm-sensors@...sensors.org" <lm-sensors@...sensors.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"hpa@...or.com" <hpa@...or.com>
Subject: Re: [PATCH] x86/hwmon: fix configuration and initialization of
	 coretemp and pkgtemp

>>> On 02.09.10 at 23:25, Fenghua Yu <fenghua.yu@...el.com> wrote:
>> --- linux-2.6.36-rc3/arch/x86/include/asm/cpufeature.h
>> +++ 2.6.36-rc3-x86-hwmon/arch/x86/include/asm/cpufeature.h
>> @@ -168,6 +168,7 @@
>>  #define X86_FEATURE_XSAVEOPT	(7*32+ 4) /* Optimized Xsave */
>>  #define X86_FEATURE_PLN		(7*32+ 5) /* Intel Power Limit Notification */
>>  #define X86_FEATURE_PTS		(7*32+ 6) /* Intel Package Thermal Status */
>> +#define X86_FEATURE_DTS		(7*32+31) /* Digital Thermal Sensor */
> 
> Is there any paticular reason to chose 7*32+31 instead of 7*32+7?

Oh, right, I actually wanted to changes this before final submission
(I coded it this way locally to avoid eventual collisions until I would
get to submit this).

>> --- linux-2.6.36-rc3/drivers/hwmon/coretemp.c
>> +++ 2.6.36-rc3-x86-hwmon/drivers/hwmon/coretemp.c
>> @@ -423,9 +423,18 @@ static int __cpuinit coretemp_device_add
>>  	int err;
>>  	struct platform_device *pdev;
>>  	struct pdev_entry *pdev_entry;
>> -#ifdef CONFIG_SMP
>>  	struct cpuinfo_x86 *c = &cpu_data(cpu);
>> -#endif
>> +
>> +	/*
>> +	 * CPUID.06H.EAX[0] indicates whether the CPU has thermal
>> +	 * sensors. We check this bit only, all the early CPUs
>> +	 * without thermal sensors will be filtered out.
>> +	 */
>> +	if (!cpu_has(c, X86_FEATURE_DTS)) {
>> +		printk(KERN_INFO DRVNAME ": CPU (model=0x%x)"
>> +		       " has no thermal sensor.\n", c->x86_model);
>> +		return 0;
> 
> Return an error (e.g. -ENODEV) could be better because there is no device for
> this driver. Then caller may handle this error accordingly (no caller 
> handles
> the error currently, though).

No, the intention of the change is for the code to remain logically
the same as it was - since the failed check previously only resulted
in a message getting printed, returning an error indication
(irrespective of there not being a check at the call sites) didn't
seem right to me.

Jan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/