RE: lm sensors sysfs file structure

RE: lm sensors sysfs file structure

[Grover, Andrew]

> From: Dominik Brodowski [mailto:linux@brodo.de] 
> > Had occured to me too. There was talk of a libpower or the likes 
> > mentioned on acpi-devel a year or so back, but afaik nothing really 
> > came of it.
> 
> Actually, the "ospmd" tool (available at 
> http://acpi.sourceforge.net ) already seems to > manage APM and 
> ACPI input. Well, and speaking of ACPI and 
> sysfs in the same message: IMHO the /proc/acpi/ interface 
> should be replaced by something in /sysfs/ as well....

Agree w.r.t. ACPI /proc switching to sysfs. Just a matter of someone
sitting down and making the changes.

It is still not clear to me if libpower should subsume libsensors, or
vice versa, or keep them separate. There are areas of overlap, as well
as significant areas of non-overlap.

Regards -- Andy

RE: lm sensors sysfs file structure

[Patrick Mochel]

> It is still not clear to me if libpower should subsume libsensors, or
> vice versa, or keep them separate. There are areas of overlap, as well
> as significant areas of non-overlap.

IMHO, neither should subsume the other; there should be a 'libdevice', or
similar that has both a sensor and a power component to it.


	-pat

Re: lm sensors sysfs file structure

[Dominik Brodowski]

On Thu, Apr 03, 2003 at 11:49:44AM +0100, Dave Jones wrote:
> On Wed, Apr 02, 2003 at 04:28:22PM -0800, Greg KH wrote:
> 
>  > > FWIW, I'm taking the same fixed-point millivolt approach with the
>  > > sysfs overrides for cpufreq.  Having similar things in sysfs
>  > > exporting the same units seems to be a good idea.
>  > 
>  > Hm, in looking around the kernel some more, it seems that there are a
>  > number of other places that export voltage and temperature values (ACPI
>  > being one of the most obvious.)  It might be time to start thinking of a
>  > single userspace library to access all of these kinds of values in a
>  > system, instead of having to probe around different parts of the sysfs
>  > tree by hand...
> 
> Had occured to me too. There was talk of a libpower or the likes
> mentioned on acpi-devel a year or so back, but afaik nothing really
> came of it.

Actually, the "ospmd" tool (available at http://acpi.sourceforge.net )
already seems to manage APM and ACPI input. Well, and speaking of ACPI and 
sysfs in the same message: IMHO the /proc/acpi/ interface should be replaced
by something in /sysfs/ as well....

	Dominik

Re: lm sensors sysfs file structure

[Dave Jones]

On Wed, Apr 02, 2003 at 04:28:22PM -0800, Greg KH wrote:

 > > FWIW, I'm taking the same fixed-point millivolt approach with the
 > > sysfs overrides for cpufreq.  Having similar things in sysfs
 > > exporting the same units seems to be a good idea.
 > 
 > Hm, in looking around the kernel some more, it seems that there are a
 > number of other places that export voltage and temperature values (ACPI
 > being one of the most obvious.)  It might be time to start thinking of a
 > single userspace library to access all of these kinds of values in a
 > system, instead of having to probe around different parts of the sysfs
 > tree by hand...

Had occured to me too. There was talk of a libpower or the likes
mentioned on acpi-devel a year or so back, but afaik nothing really
came of it.

		Dave

Re: lm sensors sysfs file structure

[Greg KH]

On Wed, Apr 02, 2003 at 12:27:47AM +0100, Dave Jones wrote:
> On Mon, Mar 31, 2003 at 10:44:39PM -0800, Greg KH wrote:
>  > > Well, you had cV for PSU voltages and mV for cpu core voltage. I guess mV
>  > > and mili-deg-C everywhere would be nicer. 
>  > As for why no floating point, it's a pain in the but to both output a
>  > fixed point number from the kernel into floating point, and to parse a
>  > floating point number from userspace within the kernel, turning it into
>  > a fixed point number.  With the proposal I wrote up, none of that is
>  > needed, and all userspace has to do is divide by a factor of 10 to get
>  > the proper value.
> 
> FWIW, I'm taking the same fixed-point millivolt approach with the
> sysfs overrides for cpufreq.  Having similar things in sysfs
> exporting the same units seems to be a good idea.

Hm, in looking around the kernel some more, it seems that there are a
number of other places that export voltage and temperature values (ACPI
being one of the most obvious.)  It might be time to start thinking of a
single userspace library to access all of these kinds of values in a
system, instead of having to probe around different parts of the sysfs
tree by hand...

thanks,

greg k-h

Re: lm sensors sysfs file structure

[Dave Jones]

On Mon, Mar 31, 2003 at 10:44:39PM -0800, Greg KH wrote:
 > > Well, you had cV for PSU voltages and mV for cpu core voltage. I guess mV
 > > and mili-deg-C everywhere would be nicer. 
 > As for why no floating point, it's a pain in the but to both output a
 > fixed point number from the kernel into floating point, and to parse a
 > floating point number from userspace within the kernel, turning it into
 > a fixed point number.  With the proposal I wrote up, none of that is
 > needed, and all userspace has to do is divide by a factor of 10 to get
 > the proper value.

FWIW, I'm taking the same fixed-point millivolt approach with the
sysfs overrides for cpufreq.  Having similar things in sysfs
exporting the same units seems to be a good idea.

		Dave

Re: lm sensors sysfs file structure

[Pavel Machek]

Hi!

> > > > >	       	Floating point values XXX.X or XXX.XX in degrees Celcius.
> > > > 
> > > > If we're restructuring it, I think we should also agree on _one_ common 
> > > > denominator for all values ie. mVolt and milli-Degree Celsius, so that 
> > > > no userspace program ever again has know how to convert them to 
> > > > user-readable values and every user can just cat the values and doesn't 
> > > > have to wonder if it's centi-Volt, milli-Volt, centi-Degree, dezi-Degree 
> > > > or whatever.
> > > 
> > > Um, that's what my proposal stated.  Do you not agree with it?  (You're
> > > quoting the existing document above, not my proposed changes.)
> > 
> > Well, you had cV for PSU voltages and
> > mV for cpu core voltage. I guess mV
> > and mili-deg-C everywhere would be
> > nicer. 
> 
> I was trying to keep consistant with what the old /proc values were
> reported as.  I'll go fix that up.
> 
> As for why no floating point, it's a pain in the but to both output a
> fixed point number from the kernel into floating point, and to parse a
> floating point number from userspace within the kernel, turning it into
> a fixed point number.  With the proposal I wrote up, none of that is
> needed, and all userspace has to do is divide by a factor of 10 to get
> the proper value.

Okay, I did not realize it is input, too. Outputing reals is really
easy; but parsing them. Okay, I guess I'm convinced. You should still
use mili-something, not centi-something...
								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: lm sensors sysfs file structure

[Greg KH]

On Sun, Mar 30, 2003 at 09:23:12PM +0200, Pavel Machek wrote:
> Hi!
> 
> > > >	       	Floating point values XXX.X or XXX.XX in degrees Celcius.
> > > 
> > > If we're restructuring it, I think we should also agree on _one_ common 
> > > denominator for all values ie. mVolt and milli-Degree Celsius, so that 
> > > no userspace program ever again has know how to convert them to 
> > > user-readable values and every user can just cat the values and doesn't 
> > > have to wonder if it's centi-Volt, milli-Volt, centi-Degree, dezi-Degree 
> > > or whatever.
> > 
> > Um, that's what my proposal stated.  Do you not agree with it?  (You're
> > quoting the existing document above, not my proposed changes.)
> 
> Well, you had cV for PSU voltages and
> mV for cpu core voltage. I guess mV
> and mili-deg-C everywhere would be
> nicer. 

I was trying to keep consistant with what the old /proc values were
reported as.  I'll go fix that up.

As for why no floating point, it's a pain in the but to both output a
fixed point number from the kernel into floating point, and to parse a
floating point number from userspace within the kernel, turning it into
a fixed point number.  With the proposal I wrote up, none of that is
needed, and all userspace has to do is divide by a factor of 10 to get
the proper value.

It's much simpler and easier to validate we got it right code.  If
you're still not convinced take a look at the code in
drivers/i2c/i2c-proc.c::i2c_parse_reals() for an example of some hairy
code...

thanks,

greg k-h

Re: lm sensors sysfs file structure

[Pavel Machek]

Hi!

> > >	       	Floating point values XXX.X or XXX.XX in degrees Celcius.
> > 
> > If we're restructuring it, I think we should also agree on _one_ common 
> > denominator for all values ie. mVolt and milli-Degree Celsius, so that 
> > no userspace program ever again has know how to convert them to 
> > user-readable values and every user can just cat the values and doesn't 
> > have to wonder if it's centi-Volt, milli-Volt, centi-Degree, dezi-Degree 
> > or whatever.
> 
> Um, that's what my proposal stated.  Do you not agree with it?  (You're
> quoting the existing document above, not my proposed changes.)

Well, you had cV for PSU voltages and
mV for cpu core voltage. I guess mV
and mili-deg-C everywhere would be
nicer. 

				Pavel
-- 
				Pavel
Written on sharp zaurus, because my Velo1 broke. If you have Velo you don't need...

Re: lm sensors sysfs file structure

[Pavel Machek]

Hi!

> temp_min[1-3]	Temperature min or hysteresis value.
> 		Fixed point value in form XXXXX and should be divided by
> 		100 to get degrees Celsius.  This is preferably a

I believe floating point is nicer... It is
improbable to have miliKelvins around,
still I don'n see why fixedpoint...
				Pavel
-- 
				Pavel
Written on sharp zaurus, because my Velo1 broke. If you have Velo you don't need...

Re: lm sensors sysfs file structure

[Greg KH]

On Fri, Mar 28, 2003 at 09:21:48AM +0200, Martin Schlemmer wrote:
> 
> Silly w83781d again.  temp1 is a u8, and temp2 and temp3 is u16
> (if they are supported on the specific model.
> 
> Should we do any bounds checking on input via sysfs ?

So that you can't hurt your hardware or crash the os, yes.

I think the write ability is limited to root, so that's a good first
step too.

thanks,

greg k-h

Re: lm sensors sysfs file structure

[Martin Schlemmer]

On Fri, 2003-03-28 at 01:10, Greg KH wrote:
> On Thu, Mar 27, 2003 at 06:00:51PM -0500, Albert Cahalan wrote:
> > Greg KH writes:
> > 
> > > temp_max[1-3]   Temperature max value.
> > >                 Fixed point value in form XXXXX and
> > >                 should be divided by
> > >                 100 to get degrees Celsius.
> > >                 Read/Write value.
> > 
> > Celsius can go negative, which may be yucky
> > and hard to test. Kelvin generally doesn't
> > suffer this problem. (yeah, yeah, quantum stuff...)
> 
> Wow, only 4 hours before someone mentioned Kelvin, I think I lost a bet
> with someone :)
> 
> Seriously, let the value go negative, no problem.  As long as it isn't
> floating point input which has to be parsed by the kernel.  That's all I
> care about.
> 

Silly w83781d again.  temp1 is a u8, and temp2 and temp3 is u16
(if they are supported on the specific model.

Should we do any bounds checking on input via sysfs ?


Regards,
-- 
Martin Schlemmer

Re: lm sensors sysfs file structure

[Martin Schlemmer]

On Thu, 2003-03-27 at 23:53, Greg KH wrote:
> On Thu, Mar 27, 2003 at 09:32:46PM +0100, Jan Dittmer wrote:
> > Greg KH wrote:
> > 
> > >fan_min[1-3]	Fan minimum value
> > >in_min[0-8]	Voltage min value.
> > >pwn[1-3]	Pulse width modulation fan control.
> > >temp_input[1-3] Temperature input value.
> > Why not start all these counts from 0? Is there any reason to start from 
> > 1? Historical reasons or does the datasheet start the counting from 1?
> 
> Hm, good point.  It looks like most of the values started at 1, with the
> one exception being the voltage files "in".
> 
> I don't know why it's this way, history?  Perhaps someone from the
> sensors project can tell us.
> 

Might be because you usually have marked on the boards fan1, fan2, etc.
Thus it should be less confusing for the user, as it is a more 1-1
mapping to the real locations.  As the voltage inputs just have to be
mapped correctly, it should not bother him ?



Regards,
-- 
Martin Schlemmer

Re: lm sensors sysfs file structure

[Greg KH]

On Thu, Mar 27, 2003 at 06:00:51PM -0500, Albert Cahalan wrote:
> Greg KH writes:
> 
> > temp_max[1-3]   Temperature max value.
> >                 Fixed point value in form XXXXX and
> >                 should be divided by
> >                 100 to get degrees Celsius.
> >                 Read/Write value.
> 
> Celsius can go negative, which may be yucky
> and hard to test. Kelvin generally doesn't
> suffer this problem. (yeah, yeah, quantum stuff...)

Wow, only 4 hours before someone mentioned Kelvin, I think I lost a bet
with someone :)

Seriously, let the value go negative, no problem.  As long as it isn't
floating point input which has to be parsed by the kernel.  That's all I
care about.

> Getting temperature display into "top" would sure
> be nice, but not if that means requiring a library
> that almost nobody has installed. It's good to give
> apps a simple way to get CPU temperature, including
> per-CPU data for SMP systems when available.

libsensors is installed on almost all distros these days.

> Info about sensor quality would be good. For example,
> my CPU measures temperature in 4-degree increments
> and is not calibrated.

I doubt the kernel driver knows this information.

thanks,

greg k-h

Re: lm sensors sysfs file structure

[Albert Cahalan]

Greg KH writes:

> temp_max[1-3]   Temperature max value.
>                 Fixed point value in form XXXXX and
>                 should be divided by
>                 100 to get degrees Celsius.
>                 Read/Write value.

Celsius can go negative, which may be yucky
and hard to test. Kelvin generally doesn't
suffer this problem. (yeah, yeah, quantum stuff...)

Getting temperature display into "top" would sure
be nice, but not if that means requiring a library
that almost nobody has installed. It's good to give
apps a simple way to get CPU temperature, including
per-CPU data for SMP systems when available.

Info about sensor quality would be good. For example,
my CPU measures temperature in 4-degree increments
and is not calibrated.

Re: lm sensors sysfs file structure

[Mark M. Hoffman]

* Greg KH <greg@kroah.com> [2003-03-27 13:53:44 -0800]:
> On Thu, Mar 27, 2003 at 09:32:46PM +0100, Jan Dittmer wrote:
> > Greg KH wrote:
> > 
> > >fan_min[1-3]	Fan minimum value
> > >in_min[0-8]	Voltage min value.
> > >pwn[1-3]	Pulse width modulation fan control.
> > >temp_input[1-3] Temperature input value.
> > Why not start all these counts from 0? Is there any reason to start from 
> > 1? Historical reasons or does the datasheet start the counting from 1?
> 
> Hm, good point.  It looks like most of the values started at 1, with the
> one exception being the voltage files "in".
> 
> I don't know why it's this way, history?  Perhaps someone from the
> sensors project can tell us.

Hardware designers tend to count devices starting with "1".  Silly them. :)

Regards,

-- 
Mark M. Hoffman
mhoffman@lightlink.com

Re: lm sensors sysfs file structure

[Greg KH]

On Thu, Mar 27, 2003 at 09:32:46PM +0100, Jan Dittmer wrote:
> Greg KH wrote:
> 
> >fan_min[1-3]	Fan minimum value
> >in_min[0-8]	Voltage min value.
> >pwn[1-3]	Pulse width modulation fan control.
> >temp_input[1-3] Temperature input value.
> Why not start all these counts from 0? Is there any reason to start from 
> 1? Historical reasons or does the datasheet start the counting from 1?

Hm, good point.  It looks like most of the values started at 1, with the
one exception being the voltage files "in".

I don't know why it's this way, history?  Perhaps someone from the
sensors project can tell us.

thanks,

greg k-h

Re: lm sensors sysfs file structure

[Jan Dittmer]

Greg KH wrote:

> fan_min[1-3]	Fan minimum value
> in_min[0-8]	Voltage min value.
> pwn[1-3]	Pulse width modulation fan control.
> temp_input[1-3] Temperature input value.
Why not start all these counts from 0? Is there any reason to start from 
1? Historical reasons or does the datasheet start the counting from 1?

Jan

Re: lm sensors sysfs file structure

[Greg KH]

[-- Attachment #1: Type: text/plain, Size: 160 bytes --]

Ok, I've modified the /proc file document to reflect the proposed sysfs
file changes and included it below.

Any comments?  Any objections?

thanks,

greg k-h


[-- Attachment #2: sensors-sysfs --]
[-- Type: text/plain, Size: 5796 bytes --]

Naming and data format standards for sysfs files
------------------------------------------------

The libsensors library offers an interface to the raw sensors data
through the sysfs interface. See libsensors documentation and source for
more further information.

An alternative method that some programs use is to access the sysfs
files directly. This document briefly describes the standards that the
drivers follow, so that an application program can scan for entries and
access this data in a simple and consistent way.

If you are developing a userspace application please send us feedback on
this standard.

Note that motherboards vary widely in the connections to sensor chips.
There is no standard that ensures, for example, that the second
temperature sensor is connected to the CPU, or that the second fan is on
the CPU. Therefore, programs must provide a facility for the user to
label or bind /proc entries for display.  Sensor chips often have unused
inputs that should be ignored by user programs.

Each chip gets its own directory in the sysfs /sys/devices tree.  To
find all sensor chips, it is easier to follow the symlinks from
/sys/i2c/devices/

All sysfs values are fixed point numbers.  To get the true value of some
of the values, you should divide by the specified value.

There is only one value per file, unlike the older /proc specification.

Alarms are direct indications read from the chips. The drivers do NOT
make comparisons of readings to thresholds. This allows violations
between readings to be caught and alarmed. The exact definition of an
alarm (for example, whether a threshold must be met or must be exceeded
to cause an alarm) is chip-dependent.


-------------------------------------------------------------------------

sysfs entries are as follows:


Entry		Function
-----		--------
alarms		Alarm bitmask.
		Read only.
		Integer representation of one to four bytes.
		A '1' bit means an alarm.
		Chips should be programmed for 'comparator' mode so that
		the alarm will 'come back' after you read the register
		if it is still valid.
		Generally a direct representation of a chip's internal
		alarm registers; there is no standard for the position
		of individual bits.
		Bits are defined in kernel/include/sensors.h.

beep_enable	Beep/interrupt enable
		0 to disable.
		1 to enable.
		Read/Write

beep_mask	Bitmask for beep.
		Same format as 'alarms' with the same bit locations.
		Read only.

curr_max[1-n]	Current max value
		Fixed point XXXX, divide by 100 to get Amps.
		Read/Write.

curr_min[1-n]	Current min or hysteresis value.
		Preferably a hysteresis value, reported as a absolute
		current, NOT a delta from the max value.
		Fixed point XXXX, divide by 100 to get Amps.
		Read/Write.

curr_input[1-n]	Current input value
		Fixed point XXXX, divide by 100 to get Amps.
		Read only.
		
fan_min[1-3]	Fan minimum value
		Integer value indicating RPM
		Read/Write.

fan_input[1-3]	Fan input value.
		Integer value indicating RPM
		Read only.

fan_div[1-3]	Fan divisor.
		Integers in powers of two (1,2,4,8,16,32,64,128).
		Some chips only support values 1,2,4,8.
		See doc/fan-divisors for details.

in_min[0-8]	Voltage min value.
		Fixed point value in form XXX.  Divide by 100 to get
		Volts.
		Read/Write
		
in_max[0-8]	Voltage max value.
		Fixed point value in form XXX.  Divide by 100 to get
		Volts.
		Read/Write
		
in_input[0-8]	Voltage input value.
		Fixed point value in form XXX.  Divide by 100 to get
		Volts.
		Read only
		Actual voltage depends on the scaling resistors on the
		motherboard, as recommended in the chip datasheet.
		This varies by chip and by motherboard.
		Because of this variation, values are generally NOT scaled
		by the chip driver, and must be done by the application.
		However, some drivers (notably lm87 and via686a)
		do scale, with various degrees of success.
		These drivers will output the actual voltage.
		First two values are read/write and third is read only.
		Typical usage:
			in_*0	CPU #1 voltage (not scaled)
			in_*1	CPU #1 voltage (not scaled)
			in_*2	3.3V nominal (not scaled)
			in_*3	5.0V nominal (scaled)
			in_*4	12.0V nominal (scaled)
			in_*5	-12.0V nominal (scaled)
			in_*6	-5.0V nominal (scaled)
			in_*7	varies
			in_*8	varies

pwn[1-3]	Pulse width modulation fan control.
		Integer 0 - 255
		Read/Write
		255 is max or 100%.
		Corresponds to the fans 1-3.

pwn_enable[1-3] pwn enable
		not always present even if pwn* is.
		0 to turn off
		1 to turn on
		Read/Write

sensor[1-3]	Sensor type selection.
		Integers 1,2,3, or thermistor Beta value (3435)
		Read/Write.

temp_max[1-3]	Temperature max value.
		Fixed point value in form XXXXX and should be divided by
		100 to get degrees Celsius.
		Read/Write value.

temp_min[1-3]	Temperature min or hysteresis value.
		Fixed point value in form XXXXX and should be divided by
		100 to get degrees Celsius.  This is preferably a
		hysteresis value, reported as a absolute temperature,
		NOT a delta from the max value.
		Read/Write value.

temp_input[1-3] Temperature input value.
		Read only value.

		If there are multiple temperature sensors, temp_*1 is
		generally the sensor inside the chip itself, generally
		reported as "motherboard temperature".  temp_*2 and
		temp_*3 are generally sensors external to the chip
		itself, for example the thermal diode inside the CPU or
		a thermistor nearby.

vid		CPU core voltage.
		Read only.
		Fixed point value in form XXXX corresponding to CPU core
		voltage as told to the sensor chip.  Divide by 1000 to
		get Volts.  Not always correct.

vrm		Voltage Regulator Module version number. 
		Read only.
		Two digit number (XX), first is major version, second is
		minor version.
		Affects the way the driver calculates the core voltage from
		the vid pins. See doc/vid for details.

Re: lm sensors sysfs file structure

[Greg KH]

On Thu, Mar 27, 2003 at 09:15:23PM +0200, Martin Schlemmer wrote:
> On Thu, 2003-03-27 at 20:52, Greg KH wrote:
> 
> > > Is this the way you want to go? Just an example for the voltages.
> > 
> > That looks very good to me, nice 
> 
> While we are at it, some form question.  The w83781d have a
> magnitude of files in sysfs if you split them like this, so
> I went for the shorter (easier?) way.
> 
> This ok, or should I split it up a bit more.  Note that I
> have not done much for indentation yet.

This is fine with me, whatever works for you.  Either way, we are
abusing macros a bunch :)

thanks,

greg k-h

Re: lm sensors sysfs file structure

[Martin Schlemmer]

[-- Attachment #1: Type: text/plain, Size: 2519 bytes --]

On Thu, 2003-03-27 at 20:52, Greg KH wrote:

> > Is this the way you want to go? Just an example for the voltages.
> 
> That looks very good to me, nice 

While we are at it, some form question.  The w83781d have a
magnitude of files in sysfs if you split them like this, so
I went for the shorter (easier?) way.

This ok, or should I split it up a bit more.  Note that I
have not done much for indentation yet.

------------------------------------------------------
#define show_in_reg(reg) \
static ssize_t show_##reg (struct device *dev, char *buf, int nr) \
{ \
    struct i2c_client *client = to_i2c_client(dev); \
    struct w83781d_data *data = i2c_get_clientdata(client); \
    w83781d_update_client(client); \
     \
    return sprintf(buf,"%ld\n", \
        IN_FROM_REG(data->reg[nr])); \
}
show_in_reg(in);
show_in_reg(in_min);
show_in_reg(in_max);

#define store_in_reg(REG,reg) \
static ssize_t store_##reg (struct device *dev, const char *buf, size_t
count, int nr) \
{ \
    struct i2c_client *client = to_i2c_client(dev); \
    struct w83781d_data *data = i2c_get_clientdata(client); \
    int reg, ret; \
     \
    ret = sscanf(buf, "%d", &reg); \
    if (ret == -1) return -EINVAL; \
    if (ret >= 1) { \
        data->reg[nr] = IN_TO_REG(reg); \
        w83781d_write_value(client, W83781D_REG_IN_##REG(nr),
data->reg[nr]); \
    } \
    return count; \
}
store_in_reg(MIN, in_min);
store_in_reg(MAX, in_max);

#define show_in_offset(offset) \
static ssize_t \
show_in_##offset (struct device *dev, char *buf) \
{ \
        return show_in(dev, buf, 0x##offset); \
} \
static DEVICE_ATTR(in_input##offset, S_IRUGO | S_IWUSR,
show_in_##offset, NULL)

#define show_in_reg_offset(reg,offset) \
static ssize_t show_##reg##offset (struct device *dev, char *buf) \
{ \
    return show_##reg (dev, buf, 0x##offset); \
} \
static ssize_t store_##reg##offset (struct device *dev, const char *buf,
size_t count) \
{ \
    return store_##reg (dev, buf, count, 0x##offset); \
} \
static DEVICE_ATTR(##reg##offset, S_IRUGO| S_IWUSR, show_##reg##offset,
store_##reg##offset)

#define show_in_offsets(offset) \
show_in_offset(offset); \
show_in_reg_offset(in_min, offset); \
show_in_reg_offset(in_max, offset);

show_in_offsets(0);
show_in_offsets(1);
show_in_offsets(2);
show_in_offsets(3);
show_in_offsets(4);
show_in_offsets(5);
show_in_offsets(6);
show_in_offsets(7);
show_in_offsets(8);



-- 

Martin Schlemmer




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: lm sensors sysfs file structure

[Jan Dittmer]

[ stripped some cc's ]

Greg KH wrote:
> On Thu, Mar 27, 2003 at 07:40:26PM +0100, Jan Dittmer wrote:
> 
>>Btw, is it indended behaviour of sysfs, that after writing to a file, 
>>the size is zero?
> 
> 
> Hm, don't know about that, I haven't seen that before.  If you cat the
> file after writing it, does the file size change?
> 
No it stays 0. Happens also with other files in sysfs.

Jan

Re: lm sensors sysfs file structure

[Greg KH]

On Thu, Mar 27, 2003 at 07:40:26PM +0100, Jan Dittmer wrote:
> Greg KH wrote:
> >That would give us one value per file, use no floating point in the
> >kernel (fake or not) and generally make things a whole lot more orderly.
> >Also, if a sensor does not have a max value (for example, I don't really
> >know if this is true), instead of having to fake a value, it can just
> >not create the file.  Then userspace can easily detect this is not
> >supported, and is not a placeholder value.
> >
> 
> Is this the way you want to go? Just an example for the voltages.

That looks very good to me, nice job.

Sensors developers, does this look sane?

> Btw, is it indended behaviour of sysfs, that after writing to a file, 
> the size is zero?

Hm, don't know about that, I haven't seen that before.  If you cat the
file after writing it, does the file size change?

thanks,

greg k-h

Re: lm sensors sysfs file structure

[Jan Dittmer]

Greg KH wrote:
> That would give us one value per file, use no floating point in the
> kernel (fake or not) and generally make things a whole lot more orderly.
> Also, if a sensor does not have a max value (for example, I don't really
> know if this is true), instead of having to fake a value, it can just
> not create the file.  Then userspace can easily detect this is not
> supported, and is not a placeholder value.
> 

Is this the way you want to go? Just an example for the voltages.

Btw, is it indended behaviour of sysfs, that after writing to a file, 
the size is zero?

ds666:/sys/devices/legacy/i2c-0/i2c_dev_0# echo 100 > in4_min
ds666:/sys/devices/legacy/i2c-0/i2c_dev_0# ls -l in4_min
-rw-r--r--    1 root     root            0 Mar 27 19:18 in4_min


-r--r--r--    1 root     root         4096 Mar 27 19:18 in0_input
-rw-r--r--    1 root     root         4096 Mar 27 19:18 in0_max
-rw-r--r--    1 root     root         4096 Mar 27 19:18 in0_min
-r--r--r--    1 root     root         4096 Mar 27 19:18 in1_input
-rw-r--r--    1 root     root         4096 Mar 27 19:18 in1_max
-rw-r--r--    1 root     root         4096 Mar 27 19:18 in1_min
-r--r--r--    1 root     root         4096 Mar 27 19:18 in2_input
-rw-r--r--    1 root     root         4096 Mar 27 19:18 in2_max
-rw-r--r--    1 root     root         4096 Mar 27 19:18 in2_min
-r--r--r--    1 root     root         4096 Mar 27 19:18 in3_input
-rw-r--r--    1 root     root         4096 Mar 27 19:18 in3_max
-rw-r--r--    1 root     root         4096 Mar 27 19:18 in3_min
-r--r--r--    1 root     root         4096 Mar 27 19:18 in4_input
-rw-r--r--    1 root     root         4096 Mar 27 19:18 in4_max
-rw-r--r--    1 root     root         4096 Mar 27 19:18 in4_min

/* 7 voltage sensors */
static ssize_t show_in(struct device *dev, char *buf, int nr) {
         struct i2c_client *client = to_i2c_client(dev);
         struct via686a_data *data = i2c_get_clientdata(client);
         via686a_update_client(client);
         return sprintf(buf,"%ld\n", IN_FROM_REG(data->in[nr], nr) );
}

static ssize_t show_in_min(struct device *dev, char *buf, int nr) {
         struct i2c_client *client = to_i2c_client(dev);
         struct via686a_data *data = i2c_get_clientdata(client);
         via686a_update_client(client);
         return sprintf(buf,"%ld\n", IN_FROM_REG(data->in_min[nr], nr) );
}

static ssize_t show_in_max(struct device *dev, char *buf, int nr) {
         struct i2c_client *client = to_i2c_client(dev);
         struct via686a_data *data = i2c_get_clientdata(client);
         via686a_update_client(client);
         return sprintf(buf,"%ld\n", IN_FROM_REG(data->in_max[nr], nr) );
}

static ssize_t set_in_min(struct device *dev, const char *buf,
                 size_t count, int nr) {
         struct i2c_client *client = to_i2c_client(dev);
         struct via686a_data *data = i2c_get_clientdata(client);
         unsigned long val = simple_strtoul(buf, NULL, 10);
         data->in_min[nr] = IN_TO_REG(val,nr);
         via686a_write_value(client, VIA686A_REG_IN_MIN(nr),
                         data->in_min[nr]);
         return count;
}
static ssize_t set_in_max(struct device *dev, const char *buf,
                 size_t count, int nr) {
         struct i2c_client *client = to_i2c_client(dev);
         struct via686a_data *data = i2c_get_clientdata(client);
         unsigned long val = simple_strtoul(buf, NULL, 10);
         data->in_max[nr] = IN_TO_REG(val,nr);
         via686a_write_value(client, VIA686A_REG_IN_MAX(nr),
                         data->in_max[nr]);
         return count;
}
#define show_in_offset(offset)                                  \
static ssize_t                                                  \
         show_in##offset (struct device *dev, char *buf)         \
{                                                               \
         return show_in(dev, buf, 0x##offset);                   \
}                                                               \
static ssize_t                                                  \
         show_in##offset##_min (struct device *dev, char *buf)   \
{                                                               \
         return show_in_min(dev, buf, 0x##offset);               \
}                                                               \
static ssize_t                                                  \
         show_in##offset##_max (struct device *dev, char *buf)   \
{                                                               \
         return show_in_max(dev, buf, 0x##offset);               \
}                                                               \
static ssize_t set_in##offset##_min (struct device *dev,        \
                 const char *buf, size_t count)                  \
{                                                               \
         return set_in_min(dev, buf, count, 0x##offset);         \
}                                                               \
static ssize_t set_in##offset##_max (struct device *dev,        \
                         const char *buf, size_t count)          \
{                                                               \
         return set_in_max(dev, buf, count, 0x##offset);         \
}                                                               \
static DEVICE_ATTR(in##offset##_input,                          \
                 S_IRUGO, show_in##offset, NULL)                 \
static DEVICE_ATTR(in##offset##_min, S_IRUGO | S_IWUSR,         \
                 show_in##offset##_min, set_in##offset##_min)    \
static DEVICE_ATTR(in##offset##_max, S_IRUGO | S_IWUSR,         \
                 show_in##offset##_max, set_in##offset##_max)

show_in_offset(0);
show_in_offset(1);
show_in_offset(2);
show_in_offset(3);
show_in_offset(4);

Re: lm sensors sysfs file structure

[Patrick Mochel]

> > Btw, is it indended behaviour of sysfs, that after writing to a file, 
> > the size is zero?
> 
> Hm, don't know about that, I haven't seen that before.  If you cat the
> file after writing it, does the file size change?

It's a known problem that I haven't been able to track down. 

The file size is hardwired to PAGE_SIZE when it is created. For some 
reason, it's reset to 0 after it's opened. I haven't had a chance to track 
down why, or how to reset it..


	-pat

Re: lm sensors sysfs file structure

[Greg KH]

On Thu, Mar 27, 2003 at 07:06:08PM +0100, Jan Dittmer wrote:
> Greg KH wrote:
> >>If we're restructuring it, I think we should also agree on _one_ common 
> >>denominator for all values ie. mVolt and milli-Degree Celsius, so that 
> >>no userspace program ever again has know how to convert them to 
> >>user-readable values and every user can just cat the values and doesn't 
> >>have to wonder if it's centi-Volt, milli-Volt, centi-Degree, dezi-Degree 
> >>or whatever.
> >
> >Um, that's what my proposal stated.  Do you not agree with it?  (You're
> >quoting the existing document above, not my proposed changes.)
> 
> I just wanted to emphasis that _all_ units should be milli oder centi. 
> Not mixing centiDegrees and milliVolts or one driver using milliVolt and 
> another centiVolt.

I agree.

> From your description it could well be, that one driver uses centi's 
> and another milli's, both for voltage or one driver uses milliVolt but 
> centi-degree.

Huh?  I said:

temp_max[1-3]   Temperature max value.
                Fixed point value in form XXXXX and should be divided by
                100 to get degrees Celsius.
                Read/Write value.

Where is the ability to use a different scale from different drivers in
that?

Anyway, it sounds like we are agreeing here, so I guess I'll go and
write up the whole document in the new style and post it for comments.

thanks,

greg k-h

Re: lm sensors sysfs file structure

[Jan Dittmer]

Greg KH wrote:
>>If we're restructuring it, I think we should also agree on _one_ common 
>>denominator for all values ie. mVolt and milli-Degree Celsius, so that 
>>no userspace program ever again has know how to convert them to 
>>user-readable values and every user can just cat the values and doesn't 
>>have to wonder if it's centi-Volt, milli-Volt, centi-Degree, dezi-Degree 
>>or whatever.
> 
> 
> Um, that's what my proposal stated.  Do you not agree with it?  (You're
> quoting the existing document above, not my proposed changes.)


I just wanted to emphasis that _all_ units should be milli oder centi. 
Not mixing centiDegrees and milliVolts or one driver using milliVolt and 
another centiVolt.
 From your description it could well be, that one driver uses centi's 
and another milli's, both for voltage or one driver uses milliVolt but 
centi-degree.

Thanks,

Jan

Re: lm sensors sysfs file structure

[Greg KH]

On Thu, Mar 27, 2003 at 11:46:16AM +0100, Jan Dittmer wrote:
> >Entry	Values	Function
> >-----	------	--------
> >temp,
> >temp[1-3]  3	Temperature max, min or hysteresis, and input value.
> >	       	Floating point values XXX.X or XXX.XX in degrees Celcius.
> 
> If we're restructuring it, I think we should also agree on _one_ common 
> denominator for all values ie. mVolt and milli-Degree Celsius, so that 
> no userspace program ever again has know how to convert them to 
> user-readable values and every user can just cat the values and doesn't 
> have to wonder if it's centi-Volt, milli-Volt, centi-Degree, dezi-Degree 
> or whatever.

Um, that's what my proposal stated.  Do you not agree with it?  (You're
quoting the existing document above, not my proposed changes.)

thanks,

greg k-h

Re: lm sensors sysfs file structure

[Mark M. Hoffman]

* Jean Delvare <khali@linux-fr.org> [2003-03-27 14:31:40 +0100]:
> > Ah, and I don't want to do small corrections like 1 or 2 percent up or
> > or down, but things like this for lm80:
> >      compute in0 (24/14.7 + 1) * @ ,       @ / (24/14.7 + 1)
> >      compute in2 (22.1/30 + 1) * @ ,       @ / (22.1/30 + 1)
> >      compute in3 (2.8/1.9) * @,            @ * 1.9/2.8
> >      compute in4 (160/30.1 + 1) * @,       @ / (160/30.1 + 1)
> > 
> > These differ a lot. And as stated in the sensors.conf seem to be from 
> > the datasheet of the lm80 and not mainboard specific.
> 
> They may be mainboard specific. The formulae rely on *suggested*
> resistor values, which the integrators may or may not follow. There
> definitely is a need for userspace conversion. Libsensors needs to be
> tunable by the user.
> 
> I don't know about the kernel conversion Jan is talking about (never
> coded a sensor driver myself), so I can't talk about this half, but I'm
> pretty sure you shouldn't even think of moving all conversions into
> kernel space.
> 
> Mark, Phil, maybe you could explain the reasons better than I would?

I'm not the Mark to which Jean refers, but anyway...  he is correct.
The sensor chip cannot read temperatures directly, only voltage.  The
conversion from degress C to V is dependent on the mainboard for the
reasons Jean mentions.

But also, the chip driver cannot read voltage directly, only bits in a
register.  *That* conversion is *not* mainboard dependent; it is
sensor chip specific.

Even when the subject in question is voltage (e.g. power supply +12V),
there is mainboard dependent nonsense between the "real value" and what
is presented at the pin of the sensor chip.

So there is a legitimate need for two conversions.  IMHO, the 
mainboard dependent one *must* be done in userspace (as Jean says).
But the conversion of raw register values to volts should happen in
the kernel; fixed point maths are sufficient for that.

(Yes I'm simplifying; external sensors can present data to sensors
chips in other electrical formats, e.g. PWM.  Same argument applies.)

Regards,

-- 
Mark M. Hoffman
mhoffman@lightlink.com

Re: lm sensors sysfs file structure

[Jean Delvare]

> Ah, and I don't want to do small corrections like 1 or 2 percent up or
> or down, but things like this for lm80:
>      compute in0 (24/14.7 + 1) * @ ,       @ / (24/14.7 + 1)
>      compute in2 (22.1/30 + 1) * @ ,       @ / (22.1/30 + 1)
>      compute in3 (2.8/1.9) * @,            @ * 1.9/2.8
>      compute in4 (160/30.1 + 1) * @,       @ / (160/30.1 + 1)
> 
> These differ a lot. And as stated in the sensors.conf seem to be from 
> the datasheet of the lm80 and not mainboard specific.

They may be mainboard specific. The formulae rely on *suggested*
resistor values, which the integrators may or may not follow. There
definitely is a need for userspace conversion. Libsensors needs to be
tunable by the user.

I don't know about the kernel conversion Jan is talking about (never
coded a sensor driver myself), so I can't talk about this half, but I'm
pretty sure you shouldn't even think of moving all conversions into
kernel space.

Mark, Phil, maybe you could explain the reasons better than I would?

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

Re: lm sensors sysfs file structure

[Jan Dittmer]

Martin Schlemmer wrote:

> For instance, what my bios say, and what the raw reading from 
> /proc is, is two different things.  I also had to slightly adjust
> things between some models of Asus boards I had.
> 
But why not return always the raw data then and don't do any conversion 
at all? I mean, if we already try to guess the real value in the driver, 
we should try to get it right.
Ah, and I don't want to do small corrections like 1 or 2 percent up or 
down, but things like this for lm80:
     compute in0 (24/14.7 + 1) * @ ,       @ / (24/14.7 + 1)
     compute in2 (22.1/30 + 1) * @ ,       @ / (22.1/30 + 1)
     compute in3 (2.8/1.9) * @,            @ * 1.9/2.8
     compute in4 (160/30.1 + 1) * @,       @ / (160/30.1 + 1)

These differ a lot. And as stated in the sensors.conf seem to be from 
the datasheet of the lm80 and not mainboard specific.
So that the sysfs values at least are near reality.

Thanks,

Jan

Re: lm sensors sysfs file structure

[Martin Schlemmer]

On Thu, 2003-03-27 at 14:27, Jan Dittmer wrote:

> Btw., why is temperature conversion done twice? First time in kernel 
> space and then with the help of sensors.conf again in user space?
> Wouldn't it be much nicer to move this to the drivers? So there would be 
> no need anymore to do this in userspace and one could rely on the values
> found in sysfs?!
> 

I guess for stuff like fan_div, etc at least, is that not all the
chips are on spec, the sensors are located on different places,
and the why the manufacturer of the board did things could all
influence things.  Thus you need to be able to tweak fan_dev,
etc for each board individually to get the most precise reading.

For instance, what my bios say, and what the raw reading from 
/proc is, is two different things.  I also had to slightly adjust
things between some models of Asus boards I had.


Regards, 

-- 
Martin Schlemmer

Re: lm sensors sysfs file structure

[Jan Dittmer]

Martin Schlemmer wrote:
> On Thu, 2003-03-27 at 12:46, Jan Dittmer wrote:
> 
>>Greg KH wrote:
>>
>>>True, but multi-valued files are not allowed in sysfs.  It's especially
>>>obnoxious that you have 3 value files when you read them, but only
>>>expect 2 values for writing.  The way I split up the values in the
>>>lm75.c driver shows a user exactly which values  are writable, and
>>>enforces this on the file system level.
>>
> 
> Right, can we just get this finalised soon ?  Not much fun in redoing
> something 2 times already ;)
> 

Btw., why is temperature conversion done twice? First time in kernel 
space and then with the help of sensors.conf again in user space?
Wouldn't it be much nicer to move this to the drivers? So there would be 
no need anymore to do this in userspace and one could rely on the values
found in sysfs?!

Thanks,

Jan

Re: lm sensors sysfs file structure

[Martin Schlemmer]

On Thu, 2003-03-27 at 12:46, Jan Dittmer wrote:
> Greg KH wrote:
> > True, but multi-valued files are not allowed in sysfs.  It's especially
> > obnoxious that you have 3 value files when you read them, but only
> > expect 2 values for writing.  The way I split up the values in the
> > lm75.c driver shows a user exactly which values  are writable, and
> > enforces this on the file system level.
> 
> Agreed, I never knew which of the three values had which functionality.
> For via686a this would be inX, inX_min, inX_max, tempX, tempX_overshoot 
> (over = overshoot = os ?), tempX_hyst, and so on.
> 
> > Entry	Values	Function
> > -----	------	--------
> > temp,
> > temp[1-3]  3	Temperature max, min or hysteresis, and input value.
> > 	       	Floating point values XXX.X or XXX.XX in degrees Celcius.
> 
> If we're restructuring it, I think we should also agree on _one_ common 
> denominator for all values ie. mVolt and milli-Degree Celsius, so that 
> no userspace program ever again has know how to convert them to 
> user-readable values and every user can just cat the values and doesn't 
> have to wonder if it's centi-Volt, milli-Volt, centi-Degree, dezi-Degree 
> or whatever.
> 

Right, can we just get this finalised soon ?  Not much fun in redoing
something 2 times already ;)


Regards,

-- 
Martin Schlemmer

Re: lm sensors sysfs file structure

[Jan Dittmer]

Greg KH wrote:
> True, but multi-valued files are not allowed in sysfs.  It's especially
> obnoxious that you have 3 value files when you read them, but only
> expect 2 values for writing.  The way I split up the values in the
> lm75.c driver shows a user exactly which values  are writable, and
> enforces this on the file system level.

Agreed, I never knew which of the three values had which functionality.
For via686a this would be inX, inX_min, inX_max, tempX, tempX_overshoot 
(over = overshoot = os ?), tempX_hyst, and so on.

> Entry	Values	Function
> -----	------	--------
> temp,
> temp[1-3]  3	Temperature max, min or hysteresis, and input value.
> 	       	Floating point values XXX.X or XXX.XX in degrees Celcius.

If we're restructuring it, I think we should also agree on _one_ common 
denominator for all values ie. mVolt and milli-Degree Celsius, so that 
no userspace program ever again has know how to convert them to 
user-readable values and every user can just cat the values and doesn't 
have to wonder if it's centi-Volt, milli-Volt, centi-Degree, dezi-Degree 
or whatever.

Thanks,

Jan

lm sensors sysfs file structure

[Greg KH]

Subject changed to reflect change in topic, and to call attention to
others who might be interested.

On Wed, Mar 26, 2003 at 05:26:54PM -0500, Mark Studebaker wrote:
> If you rename the files and/or split multivalue files into separate
> single value files, or change the format of the contents,
> and continue these changes across the 30 or so "chip" drivers of ours,
> you will completely blow up our libsensors library, and userspace programs.

Well ideally libsensors can change and then userspace programs will
never know the difference :)

I need to start digging into the libsensors code to be able to back this
up, but at first glance I think it's feasible.

> If all the patches do is move all the files unchanged from
> /proc/sys/dev/sensors/... to /sysfs/... then that change is much much easier
> to incorporate in our programs.

True, but multi-valued files are not allowed in sysfs.  It's especially
obnoxious that you have 3 value files when you read them, but only
expect 2 values for writing.  The way I split up the values in the
lm75.c driver shows a user exactly which values are writable, and
enforces this on the file system level.

I don't want to detract from all of the work you, and the lm_sensors
group has done in the past, it's quite nice.  I'm just trying to fit the
drivers into current kernel policies in the best way possible.
Remember, I want this to work, and for everyone to come to an
understanding.

> lm_sensors /proc naming standars for sensors chip drivers:
> 
> http://www2.lm-sensors.nu/~lm78/cvs/lm_sensors2/doc/developers/proc

Thanks for the link.  I don't really think this is a problem.  As an
example, the temp files, which are currently defined as a proc file with
this description:

Entry	Values	Function
-----	------	--------
temp,
temp[1-3]  3	Temperature max, min or hysteresis, and input value.
	       	Floating point values XXX.X or XXX.XX in degrees Celcius.
		'temp' is used if there is only one temperature sensor on the
		chip; for multiple temps. start with 'temp1'.
		Temp1 is generally the sensor inside the chip itself,
		generally reported as "motherboard temperature".
		Temp2 and temp3 are generally sensors external to the chip
		itself, for example the thermal diode inside the CPU or a
		termistor nearby. The second value is preferably a hysteresis
		value, reported as a absolute temperature, NOT a delta from
		the max value.
		First two values are read/write and third is read only.

Could be rewritten to say:

Entry		Function
-----		--------
temp_max[1-3]	Temperature max value.
		Fixed point value in form XXXXX and should be divided by
		100 to get degrees Celsius.
		Read/Write value.

temp_min[1-3]	Temperature min or hysteresis value.
		Fixed point value in form XXXXX and should be divided by
		100 to get degrees Celsius.  This is preferably a
		hysteresis value, reported as a absolute temperature, NOT
		a delta from the max value.
		Read/Write value.

temp_input[1-3]	Temperature input value.
		Read only value.

		If there are multiple temperature sensors, temp_*1 is
		generally the sensor inside the chip itself, generally
		reported as "motherboard temperature".  temp_*2 and
		temp_*3 are generally sensors external to the chip
		itself, for example the thermal diode inside the CPU or a
		thermistor nearby.

That would give us one value per file, use no floating point in the
kernel (fake or not) and generally make things a whole lot more orderly.
Also, if a sensor does not have a max value (for example, I don't really
know if this is true), instead of having to fake a value, it can just
not create the file.  Then userspace can easily detect this is not
supported, and is not a placeholder value.

Does that sound reasonable?

thanks,

greg k-h