Re: [PATCH 2.6] (10/11) hwmon vs i2c, second round
Hi Alexey, > > I see very little reason why vid_from_reg and vid_to_reg are > > inlined. The former is not exactly short, > > 1) > and their arguments aren't known at > compile time, > > [Compiler can optimise them away _completely_ if both arguments are > known at compile time or significantly of only one is known.] Good point, I'll try to remember that. It doesn't apply here though except for lm78 I think, and maybe lm93 when it gets ported. That's not the majority of users though, so I still believe uninlining is the correct decision. > > and they are never called in speed critical areas. Uninlining them > > should cause little performance loss if any, and saves a signficant > > space and compilation time as well. > > 2) VID_FROM_REG is asking for removal from lm85. True, I wrote a patch doing this already: http://lists.lm-sensors.org/pipermail/lm-sensors/2005-July/013148.html Just wait for Greg to pick it and it'll show in -mm. > 3) vid_to_reg is used only by atxp1. That's right. Do you suggest that it should be kept inlined then? Similar drivers may be written in the future, causing vid_to_reg to gain users and possibly grow larger (to support more VRM/VRD standards), then we'll certainly want to uninline it anyway - but I agree that this ain't the case at the moment. I'll change that patch to only uninline vid_from_reg and not vid_to_reg if a majority prefers me to do so. Thanks for your comments :) -- Jean Delvare - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2.6] (10/11) hwmon vs i2c, second round
One nitpicking comment and two observations: On Sun, Jul 31, 2005 at 10:02:24PM +0200, Jean Delvare wrote: > I see very little reason why vid_from_reg and vid_to_reg are inlined. > The former is not exactly short, 1) and their arguments aren't known at compile time, [Compiler can optimise them away _completely_ if both arguments are known at compile time or significantly of only one is known.] > and they are never called in speed critical areas. Uninlining them should > cause little performance loss if any, and saves a signficant space and > compilation time as well. 2) VID_FROM_REG is asking for removal from lm85. 3) vid_to_reg is used only by atxp1. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2.6] (10/11) hwmon vs i2c, second round
I see very little reason why vid_from_reg and vid_to_reg are inlined.
The former is not exactly short, and they are never called in speed
critical areas. Uninlining them should cause little performance loss
if any, and saves a signficant space and compilation time as well.
Signed-off-by: Jean Delvare <[EMAIL PROTECTED]>
drivers/hwmon/hwmon-vid.c | 99 ++
include/linux/hwmon-vid.h | 90 ++---
2 files changed, 104 insertions(+), 85 deletions(-)
--- linux-2.6.13-rc4.orig/drivers/hwmon/hwmon-vid.c 2005-07-31
17:00:12.0 +0200
+++ linux-2.6.13-rc4/drivers/hwmon/hwmon-vid.c 2005-07-31 20:55:47.0
+0200
@@ -3,6 +3,10 @@
Copyright (c) 2004 Rudolf Marek <[EMAIL PROTECTED]>
+Partly imported from i2c-vid.h of the lm_sensors project
+Copyright (c) 2002 Mark D. Studebaker <[EMAIL PROTECTED]>
+With assistance from Trent Piepho <[EMAIL PROTECTED]>
+
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 2 of the License, or
@@ -23,6 +27,99 @@
#include
#include
+/*
+Common code for decoding VID pins.
+
+References:
+
+For VRM 8.4 to 9.1, "VRM x.y DC-DC Converter Design Guidelines",
+available at http://developer.intel.com/.
+
+For VRD 10.0 and up, "VRD x.y Design Guide",
+available at http://developer.intel.com/.
+
+AMD Opteron processors don't follow the Intel specifications.
+I'm going to "make up" 2.4 as the spec number for the Opterons.
+No good reason just a mnemonic for the 24x Opteron processor
+series.
+
+Opteron VID encoding is:
+ 0 = 1.550 V
+ 1 = 1.525 V
+. . . .
+ 0 = 0.800 V
+ 1 = 0.000 V (off)
+*/
+
+/* vrm is the VRM/VRD document version multiplied by 10.
+ val is the 4-, 5- or 6-bit VID code.
+ Returned value is in mV to avoid floating point in the kernel. */
+int vid_from_reg(int val, int vrm)
+{
+ int vid;
+
+ switch(vrm) {
+
+ case 0:
+ return 0;
+
+ case 100: /* VRD 10.0 */
+ if((val & 0x1f) == 0x1f)
+ return 0;
+ if((val & 0x1f) <= 0x09 || val == 0x0a)
+ vid = 10875 - (val & 0x1f) * 250;
+ else
+ vid = 18625 - (val & 0x1f) * 250;
+ if(val & 0x20)
+ vid -= 125;
+ vid /= 10; /* only return 3 dec. places for now */
+ return vid;
+
+ case 24:/* Opteron processor */
+ return(val == 0x1f ? 0 : 1550 - val * 25);
+
+ case 91:/* VRM 9.1 */
+ case 90:/* VRM 9.0 */
+ return(val == 0x1f ? 0 :
+ 1850 - val * 25);
+
+ case 85:/* VRM 8.5 */
+ return((val & 0x10 ? 25 : 0) +
+ ((val & 0x0f) > 0x04 ? 2050 : 1250) -
+ ((val & 0x0f) * 50));
+
+ case 84:/* VRM 8.4 */
+ val &= 0x0f;
+ /* fall through */
+ default:/* VRM 8.2 */
+ return(val == 0x1f ? 0 :
+ val & 0x10 ? 5100 - (val) * 100 :
+2050 - (val) * 50);
+ }
+}
+
+/* vrm is the VRM/VRD document version multiplied by 10.
+ val is in mV to avoid floating point in the kernel.
+ Returned value is the 4-, 5- or 6-bit VID code.
+ Note that only VRM 9.x is supported for now. */
+int vid_to_reg(int val, int vrm)
+{
+ switch (vrm) {
+ case 91:/* VRM 9.1 */
+ case 90:/* VRM 9.0 */
+ return ((val >= 1100) && (val <= 1850) ?
+ ((18499 - val * 10) / 25 + 5) / 10 : -1);
+ default:
+ return -1;
+ }
+}
+
+
+/*
+After this point is the code to automatically determine which
+VRM/VRD specification should be used depending on the CPU.
+*/
+
struct vrm_model {
u8 vendor;
u8 eff_family;
@@ -96,6 +193,8 @@
}
#endif
+EXPORT_SYMBOL(vid_from_reg);
+EXPORT_SYMBOL(vid_to_reg);
EXPORT_SYMBOL(vid_which_vrm);
MODULE_AUTHOR("Rudolf Marek <[EMAIL PROTECTED]>");
--- linux-2.6.13-rc4.orig/include/linux/hwmon-vid.h 2005-07-31
17:00:35.0 +0200
+++ linux-2.6.13-rc4/include/linux/hwmon-vid.h 2005-07-31 20:55:47.0
+0200
@@ -20,91 +20,11 @@
Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
-/*
-This file contains common code for decoding VID pins.
-This file is #included in various chip drivers in this directory.
-As the user is unlikely to load more than one driver which
-includes this code we don't worry about the wasted space.
-
[PATCH 2.6] (10/11) hwmon vs i2c, second round
I see very little reason why vid_from_reg and vid_to_reg are inlined.
The former is not exactly short, and they are never called in speed
critical areas. Uninlining them should cause little performance loss
if any, and saves a signficant space and compilation time as well.
Signed-off-by: Jean Delvare [EMAIL PROTECTED]
drivers/hwmon/hwmon-vid.c | 99 ++
include/linux/hwmon-vid.h | 90 ++---
2 files changed, 104 insertions(+), 85 deletions(-)
--- linux-2.6.13-rc4.orig/drivers/hwmon/hwmon-vid.c 2005-07-31
17:00:12.0 +0200
+++ linux-2.6.13-rc4/drivers/hwmon/hwmon-vid.c 2005-07-31 20:55:47.0
+0200
@@ -3,6 +3,10 @@
Copyright (c) 2004 Rudolf Marek [EMAIL PROTECTED]
+Partly imported from i2c-vid.h of the lm_sensors project
+Copyright (c) 2002 Mark D. Studebaker [EMAIL PROTECTED]
+With assistance from Trent Piepho [EMAIL PROTECTED]
+
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 2 of the License, or
@@ -23,6 +27,99 @@
#include linux/kernel.h
#include linux/hwmon-vid.h
+/*
+Common code for decoding VID pins.
+
+References:
+
+For VRM 8.4 to 9.1, VRM x.y DC-DC Converter Design Guidelines,
+available at http://developer.intel.com/.
+
+For VRD 10.0 and up, VRD x.y Design Guide,
+available at http://developer.intel.com/.
+
+AMD Opteron processors don't follow the Intel specifications.
+I'm going to make up 2.4 as the spec number for the Opterons.
+No good reason just a mnemonic for the 24x Opteron processor
+series.
+
+Opteron VID encoding is:
+ 0 = 1.550 V
+ 1 = 1.525 V
+. . . .
+ 0 = 0.800 V
+ 1 = 0.000 V (off)
+*/
+
+/* vrm is the VRM/VRD document version multiplied by 10.
+ val is the 4-, 5- or 6-bit VID code.
+ Returned value is in mV to avoid floating point in the kernel. */
+int vid_from_reg(int val, int vrm)
+{
+ int vid;
+
+ switch(vrm) {
+
+ case 0:
+ return 0;
+
+ case 100: /* VRD 10.0 */
+ if((val 0x1f) == 0x1f)
+ return 0;
+ if((val 0x1f) = 0x09 || val == 0x0a)
+ vid = 10875 - (val 0x1f) * 250;
+ else
+ vid = 18625 - (val 0x1f) * 250;
+ if(val 0x20)
+ vid -= 125;
+ vid /= 10; /* only return 3 dec. places for now */
+ return vid;
+
+ case 24:/* Opteron processor */
+ return(val == 0x1f ? 0 : 1550 - val * 25);
+
+ case 91:/* VRM 9.1 */
+ case 90:/* VRM 9.0 */
+ return(val == 0x1f ? 0 :
+ 1850 - val * 25);
+
+ case 85:/* VRM 8.5 */
+ return((val 0x10 ? 25 : 0) +
+ ((val 0x0f) 0x04 ? 2050 : 1250) -
+ ((val 0x0f) * 50));
+
+ case 84:/* VRM 8.4 */
+ val = 0x0f;
+ /* fall through */
+ default:/* VRM 8.2 */
+ return(val == 0x1f ? 0 :
+ val 0x10 ? 5100 - (val) * 100 :
+2050 - (val) * 50);
+ }
+}
+
+/* vrm is the VRM/VRD document version multiplied by 10.
+ val is in mV to avoid floating point in the kernel.
+ Returned value is the 4-, 5- or 6-bit VID code.
+ Note that only VRM 9.x is supported for now. */
+int vid_to_reg(int val, int vrm)
+{
+ switch (vrm) {
+ case 91:/* VRM 9.1 */
+ case 90:/* VRM 9.0 */
+ return ((val = 1100) (val = 1850) ?
+ ((18499 - val * 10) / 25 + 5) / 10 : -1);
+ default:
+ return -1;
+ }
+}
+
+
+/*
+After this point is the code to automatically determine which
+VRM/VRD specification should be used depending on the CPU.
+*/
+
struct vrm_model {
u8 vendor;
u8 eff_family;
@@ -96,6 +193,8 @@
}
#endif
+EXPORT_SYMBOL(vid_from_reg);
+EXPORT_SYMBOL(vid_to_reg);
EXPORT_SYMBOL(vid_which_vrm);
MODULE_AUTHOR(Rudolf Marek [EMAIL PROTECTED]);
--- linux-2.6.13-rc4.orig/include/linux/hwmon-vid.h 2005-07-31
17:00:35.0 +0200
+++ linux-2.6.13-rc4/include/linux/hwmon-vid.h 2005-07-31 20:55:47.0
+0200
@@ -20,91 +20,11 @@
Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
-/*
-This file contains common code for decoding VID pins.
-This file is #included in various chip drivers in this directory.
-As the user is unlikely to load more than one driver which
-includes this code we don't worry about the wasted space.
-
Re: [PATCH 2.6] (10/11) hwmon vs i2c, second round
One nitpicking comment and two observations: On Sun, Jul 31, 2005 at 10:02:24PM +0200, Jean Delvare wrote: I see very little reason why vid_from_reg and vid_to_reg are inlined. The former is not exactly short, 1) and their arguments aren't known at compile time, [Compiler can optimise them away _completely_ if both arguments are known at compile time or significantly of only one is known.] and they are never called in speed critical areas. Uninlining them should cause little performance loss if any, and saves a signficant space and compilation time as well. 2) VID_FROM_REG is asking for removal from lm85. 3) vid_to_reg is used only by atxp1. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2.6] (10/11) hwmon vs i2c, second round
Hi Alexey, I see very little reason why vid_from_reg and vid_to_reg are inlined. The former is not exactly short, 1) and their arguments aren't known at compile time, [Compiler can optimise them away _completely_ if both arguments are known at compile time or significantly of only one is known.] Good point, I'll try to remember that. It doesn't apply here though except for lm78 I think, and maybe lm93 when it gets ported. That's not the majority of users though, so I still believe uninlining is the correct decision. and they are never called in speed critical areas. Uninlining them should cause little performance loss if any, and saves a signficant space and compilation time as well. 2) VID_FROM_REG is asking for removal from lm85. True, I wrote a patch doing this already: http://lists.lm-sensors.org/pipermail/lm-sensors/2005-July/013148.html Just wait for Greg to pick it and it'll show in -mm. 3) vid_to_reg is used only by atxp1. That's right. Do you suggest that it should be kept inlined then? Similar drivers may be written in the future, causing vid_to_reg to gain users and possibly grow larger (to support more VRM/VRD standards), then we'll certainly want to uninline it anyway - but I agree that this ain't the case at the moment. I'll change that patch to only uninline vid_from_reg and not vid_to_reg if a majority prefers me to do so. Thanks for your comments :) -- Jean Delvare - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
