Skip to contentSkip to conversation

automatic VRM detection part2

PATCH New Rudolf Marek 3 participants 4 messages May 19, 2005

lore
Share & tools
lore

Rudolf Marek

author
#0 lore raw

PATCH 1/1 automatic VRM detection part2

Hello,

This is second part, which just adds the functionality to existing code base, also including support of vid inputs for it8712 chip.

This patch was also briefly reviewed by Jean Delvare.

Signed-off-by Rudolf Marek <[email protected]>

Oh I forgot, sorry. Both patches are against 2.6.8rc2

Regards

Rudolf

drivers/i2c/chips/adm1025.c +1 -1
diff -Naur d/drivers/i2c/chips/adm1025.c c/drivers/i2c/chips/adm1025.c
@@ -455,7 +455,7 @@
 	struct adm1025_data *data = i2c_get_clientdata(client);
 	int i;
 
-	data->vrm = 82;
+	data->vrm = i2c_which_vrm();
 
 	/*
 	 * Set high limits
drivers/i2c/chips/asb100.c +1 -4
diff -Naur d/drivers/i2c/chips/asb100.c c/drivers/i2c/chips/asb100.c
@@ -63,9 +63,6 @@
 static unsigned int normal_isa[] = { I2C_CLIENT_ISA_END };
 static unsigned int normal_isa_range[] = { I2C_CLIENT_ISA_END };
 
-/* default VRM to 9.0 instead of 8.2 */
-#define ASB100_DEFAULT_VRM 90
-
 /* Insmod parameters */
 SENSORS_INSMOD_1(asb100);
 I2C_CLIENT_MODULE_PARM(force_subclients, "List of subclient addresses: "

@@ -959,7 +956,7 @@
 
 	vid = asb100_read_value(client, ASB100_REG_VID_FANDIV) & 0x0f;
 	vid |= (asb100_read_value(client, ASB100_REG_CHIPID) & 0x01) << 4;
-	data->vrm = ASB100_DEFAULT_VRM;
+	data->vrm = i2c_which_vrm();
 	vid = vid_from_reg(vid, data->vrm);
 
 	/* Start monitoring */
drivers/i2c/chips/it87.c +52 -4

111 lines not on this page · raw message

drivers/i2c/chips/lm85.c +1 -4
diff -Naur d/drivers/i2c/chips/lm85.c c/drivers/i2c/chips/lm85.c
@@ -308,9 +308,6 @@
  * version of the driver.
  */
 
-/* Typically used with Pentium 4 systems v9.1 VRM spec */
-#define LM85_INIT_VRM  91
-
 /* Chip sampling rates
  *
  * Some sensors are not updated more frequently than once per second

@@ -832,7 +829,7 @@
 		goto ERROR1;
 
 	/* Set the VRM version */
-	data->vrm = LM85_INIT_VRM ;
+	data->vrm = i2c_which_vrm();
 
 	/* Initialize the LM85 chip */
 	lm85_init_client(new_client);
drivers/i2c/chips/w83627hf.c +1 -1
diff -Naur d/drivers/i2c/chips/w83627hf.c c/drivers/i2c/chips/w83627hf.c
@@ -1281,7 +1281,7 @@
 		data->vrm = (data->vrm_ovt & 0x01) ? 90 : 82;
 	} else {
 		/* Convert VID to voltage based on default VRM */
-		data->vrm = DEFAULT_VRM;
+		data->vrm = i2c_which_vrm();
 	}
 
 	tmp = w83627hf_read_value(client, W83781D_REG_SCFG1);
drivers/i2c/chips/w83781d.c +1 -1
diff -Naur d/drivers/i2c/chips/w83781d.c c/drivers/i2c/chips/w83781d.c
@@ -1513,7 +1513,7 @@
 		w83781d_write_value(client, W83781D_REG_BEEP_INTS2, 0);
 	}
 
-	data->vrm = 82;
+	data->vrm = i2c_which_vrm();
 
 	if ((type != w83781d) && (type != as99127f)) {
 		tmp = w83781d_read_value(client, W83781D_REG_SCFG1);
#1 lore raw

PATCH 1/1 automatic VRM detection part2

Hello Rudolf:

Quoted 5 lines · Rudolf Marek

Hello,

This is second part, which just adds the functionality to existing code base, also including support of vid inputs for it8712 chip.

<snip>

One small comment:

Quoted 10 lines
drivers/i2c/chips/asb100.c +1 -1
@@ -959,7 +956,7 @@
 
 	vid = asb100_read_value(client, ASB100_REG_VID_FANDIV) & 0x0f;
 	vid |= (asb100_read_value(client, ASB100_REG_CHIPID) & 0x01) << 4;
-	data->vrm = ASB100_DEFAULT_VRM;
+	data->vrm = i2c_which_vrm();
 	vid = vid_from_reg(vid, data->vrm);
 
 	/* Start monitoring */

diff -Naur d/drivers/i2c/chips/it87.c c/drivers/i2c/chips/it87.c

Why not this instead? E.g.

	if (!(data->vrm = i2c_which_vrm()))
		data->vrm = ASB100_DEFAULT_VRM;

That way, non x86 or some x86 that isn't recognized might fall back and still get it right, or at least work the same as it used to.

Regards,

Signature · 2 lines
Mark M. Hoffman
[email protected]
#2 lore raw

PATCH 1/1 automatic VRM detection part2

In reply to Rudolf Marek
Quoted 14 lines

One small comment:

drivers/i2c/chips/asb100.c line 962

Quoted 9 lines
drivers/i2c/chips/asb100.c +1 -1
@@ -959,7 +956,7 @@
 
 	vid = asb100_read_value(client, ASB100_REG_VID_FANDIV) & 0x0f;
 	vid |= (asb100_read_value(client, ASB100_REG_CHIPID) & 0x01) << 4;
-	data->vrm = ASB100_DEFAULT_VRM;
+	data->vrm = i2c_which_vrm();
 	vid = vid_from_reg(vid, data->vrm);
 
 	/* Start monitoring */

Why not this instead? E.g.

Quoted 5 lines
	if (!(data->vrm = i2c_which_vrm()))
		data->vrm = ASB100_DEFAULT_VRM;

That way, non x86 or some x86 that isn't recognized might fall back and still get it right, or at least work the same as it used to.

Unsupported CPUs are rather unlikely to use VRM 9.0 (ASB100_DEFAULT_VRM), so in most cases it won't help. If we want to default to something, we would certainly default to the same VRM version for all drivers, and set this default version to the latest known version (10.0 as of now). But even then the chances that it'll work out of the box are slim.

So I think it makes more sense to clearly show that we do not support a given CPU yet, so that:
1* People can report to us.
2* Userspace knows it shouldn't trust the (zero) VID value.

At any rate, if we decide we want to default to something in this case, then let's default to the same VRM version for all drivers, being the VRM we consider is the more likely to be correct for unknown (i.e. recent) CPUs, and have the mechanism placed in i2c_which_vrm() itself rather than ranging all over the various drivers.

But I like it the way Ruik proposed.

Signature · 2 lines
Jean "Khali" Delvare
http://khali.linux-fr.org/
#3 lore raw

PATCH 1/1 automatic VRM detection part2

In reply to Rudolf Marek
Quoted 14 lines · Jean Delvare
Quoted 14 lines

One small comment:

drivers/i2c/chips/asb100.c line 962

Quoted 9 lines
drivers/i2c/chips/asb100.c +1 -1
@@ -959,7 +956,7 @@
 
 	vid = asb100_read_value(client, ASB100_REG_VID_FANDIV) & 0x0f;
 	vid |= (asb100_read_value(client, ASB100_REG_CHIPID) & 0x01) << 4;
-	data->vrm = ASB100_DEFAULT_VRM;
+	data->vrm = i2c_which_vrm();
 	vid = vid_from_reg(vid, data->vrm);
 
 	/* Start monitoring */

Why not this instead? E.g.

Quoted 7 lines
Quoted 5 lines
	if (!(data->vrm = i2c_which_vrm()))
		data->vrm = ASB100_DEFAULT_VRM;

That way, non x86 or some x86 that isn't recognized might fall back and still get it right, or at least work the same as it used to.

Unsupported CPUs are rather unlikely to use VRM 9.0

.. assuming we have a complete list, yes.

Quoted 5 lines

(ASB100_DEFAULT_VRM), so in most cases it won't help. If we want to default to something, we would certainly default to the same VRM version for all drivers, and set this default version to the latest known version (10.0 as of now). But even then the chances that it'll work out of the box are slim.

True and true.

Quoted 5 lines

So I think it makes more sense to clearly show that we do not support a given CPU yet, so that:
1* People can report to us.
2* Userspace knows it shouldn't trust the (zero) VID value.

Quoted 7 lines

At any rate, if we decide we want to default to something in this case, then let's default to the same VRM version for all drivers, being the VRM we consider is the more likely to be correct for unknown (i.e. recent) CPUs, and have the mechanism placed in i2c_which_vrm() itself rather than ranging all over the various drivers.

But I like it the way Ruik proposed.

OK, I have no strong objection to the patch as-is... just thought I'd ask though.

Regards,

Signature · 2 lines
Mark M. Hoffman
[email protected]