Re: [PATCH] I2C: automatic VRM detection part1

[Dave Jones]

On Thu, Aug 05, 2004 at 11:38:57PM +0000, Linux Kernel wrote:
 
 > diff -Nru a/drivers/i2c/i2c-sensor-vid.c b/drivers/i2c/i2c-sensor-vid.c
 > --- /dev/null	Wed Dec 31 16:00:00 196900
 > +++ b/drivers/i2c/i2c-sensor-vid.c	2004-08-23 16:28:37 -07:00
 > @@ -0,0 +1,99 @@

....
 
 > +#ifdef CONFIG_X86
 > +
 > +static struct vrm_model vrm_models[] = {
 > +	{X86_VENDOR_AMD, 0x6, ANY, 90},		/* Athlon Duron etc */

This is insufficient afaics. Mobile Athlon's/Durons will have
a different VRM to the desktop parts.

 > +	{X86_VENDOR_AMD, 0xF, 0x4, 90},		/* Athlon 64 */

possible the same here.

 > +	{X86_VENDOR_INTEL, 0x6, 0xB, 85},	/* 0xB Tualatin */

again, mobile parts may match this.

 > +	{X86_VENDOR_INTEL, 0x6, ANY, 82},	/* any P6 */

definitly here.

 > +	{X86_VENDOR_INTEL, 0x7, ANY, 0},	/* Itanium */

huh? Under ifdef CONFIG_X86 ?

 > +	{X86_VENDOR_INTEL, 0x10,ANY, 0},	/* Itanium 2 */

ditto.


 > +
 > +int i2c_which_vrm(void)
 > +{
 > +	struct cpuinfo_x86 *c = cpu_data;
 > +	u32 eax;
 > +	u8 eff_family, eff_model;
 > +	int vrm_ret;
 > +
 > +	if (c->x86 < 6) return 0;	/* any CPU with familly lower than 6
 > +				 	dont have VID and/or CPUID */

1. CodingStyle dictates
		if (c->x86 < 6)
			return 0;
2. Typo in 'familly'
 
 > +	eax = cpuid_eax(1);
 > +	eff_family = ((eax & 0x00000F00)>>8);
 > +	eff_model  = ((eax & 0x000000F0)>>4);
 > +	if (eff_family == 0xF) {	/* use extended model & family */
 > +		eff_family += ((eax & 0x00F00000)>>20);
 > +		eff_model += ((eax & 0x000F0000)>>16)<<4;
 > +	}

Is this guaranteed safe on all vendors ?

 > +/* and now something completely different for Non-x86 world*/
 > +#else
 > +int i2c_which_vrm(void)
 > +{
 > +	printk(KERN_INFO "i2c-sensor.o: Unknown VRM version of your CPU\n");
 > +	return 0;
 > +}
 > +#endif

why build this at all on non-x86 if its useless ?


		Dave

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