Re: lm sensors sysfs file structure

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

[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

[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

[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

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

[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

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

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

i2c driver changes for 2.5.66; adding w83781d support.

[Martin Schlemmer]

Hi

I am interested in this, but I only have LKML at work.  Mind
sending me all the changes to 2.5.66 as one bulk patch?

Also, I did some initial patch to get w83781d support in, but
it very rough.  I will try to get to doing it according to your
'How to convert i2c adapter drivers into good kernel code' ...
Who do I sent it to if completed ?


Thanks,

-- 
Martin Schlemmer

Re: i2c driver changes for 2.5.66; adding w83781d support.

[Greg KH]

On Tue, Mar 25, 2003 at 10:53:14AM +0200, Martin Schlemmer wrote:
> Hi
> 
> I am interested in this, but I only have LKML at work.  Mind
> sending me all the changes to 2.5.66 as one bulk patch?

They should be all in the patch-2.5.66-bk1 file on kernel.org right now.

> Also, I did some initial patch to get w83781d support in, but
> it very rough.  I will try to get to doing it according to your
> 'How to convert i2c adapter drivers into good kernel code' ...
> Who do I sent it to if completed ?

I have a port of this driver in my kernel tree, if you really want to
use it right now.  But I'd recommend waiting on converting the chip
drivers until we get the sysfs interface conversion done, otherwise we
will have to go back and modify all files again :)

See my other post about the eeprom.c driver for more info on the sysfs
changes needed for i2c chip drivers, it isn't as simple.

But if you really want to do it, great.  Send it on to lkml, and
probably the sensors list.  You can cc me if you want to.

thanks,

greg k-h

w83781d i2c driver updated for 2.5.66 (without sysfs support)

[Martin Schlemmer]

Skipped content of type multipart/mixed-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20030326/6d1e64b3/attachment.bin

w83781d i2c driver updated for 2.5.66 (without sysfs support)

[Jan Dittmer]

Martin Schlemmer wrote:
> 
> I did look at the changes needed for sysfs, but this beast have
> about 6 ctl_tables, and is hairy in general.  I am not sure what
> is the best way to do it for the different chips, so here is what
> I have until I or somebody else can do the sysfs stuff.
> 
I've just done this with the via686a driver. Saves about 100 lines of code.

Comments?

Jan

--- c/drivers/i2c/chips/via686a.c	2003-03-26 10:35:04.000000000 +0100
+++ b/drivers/i2c/chips/via686a.c	2003-03-26 19:57:19.000000000 +0100
@@ -394,25 +394,185 @@
  			  unsigned short flags, int kind);
  static int via686a_detach_client(struct i2c_client *client);

-static int via686a_read_value(struct i2c_client *client, u8 register);
-static void via686a_write_value(struct i2c_client *client, u8 register,
-				u8 value);
+static inline int via686a_read_value(struct i2c_client *client, u8 reg)
+{
+	return (inb_p(client->addr + reg));
+}
+
+static inline void via686a_write_value(struct i2c_client *client, u8 reg,
+				       u8 value)
+{
+	outb_p(value, client->addr + reg);
+}
+
  static void via686a_update_client(struct i2c_client *client);
  static void via686a_init_client(struct i2c_client *client);

+/* following are the sysfs callback functions */
+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 %ld %ld\n",
+		IN_FROM_REG(data->in_min[nr], nr),
+		IN_FROM_REG(data->in_max[nr], nr),
+		IN_FROM_REG(data->in[nr], nr) );
+}
+
+static ssize_t store_in(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);
+	int in_min, in_max, ret;
+	ret = sscanf(buf, "%d %d", &in_min, &in_max);
+	if (ret = -1) return -EINVAL;
+	if (ret >= 1) {
+		data->in_min[nr] = IN_TO_REG(in_min, nr);
+		via686a_write_value(client, VIA686A_REG_IN_MIN(nr), data->in_min[nr]);
+	}
+	if (ret >= 2) {
+		data->in_max[nr] = IN_TO_REG(in_max, 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 store_in_##offset (struct device *dev, const char *buf, 
size_t count) \
+{								\
+	return store_in(dev, buf, count, 0x##offset);		\
+}								\
+static DEVICE_ATTR(in##offset, S_IRUGO| S_IWUSR, show_in_##offset, 
store_in_##offset)
+
+show_in_offset(0);
+show_in_offset(1);
+show_in_offset(2);
+show_in_offset(3);
+show_in_offset(4);
+
+static ssize_t show_temp(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 %ld %ld\n",
+		TEMP_FROM_REG(data->temp_over[nr]),
+		TEMP_FROM_REG(data->temp_hyst[nr]),
+		TEMP_FROM_REG10(data->temp[nr]) );
+}
+static ssize_t store_temp(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);
+	int temp_over, temp_hyst, ret;
+	printk(buf);
+	ret = sscanf(buf, "%d %d", &temp_over, &temp_hyst);
+	if (ret = -1) return -EINVAL;
+	if (ret >= 1) {
+		data->temp_over[nr] = TEMP_TO_REG(temp_over);
+		via686a_write_value(client, VIA686A_REG_TEMP_OVER(nr + 1), 
data->temp_over[nr]);
+	}
+	if (ret >= 2) {
+		data->temp_hyst[nr] = TEMP_TO_REG(temp_hyst);
+		via686a_write_value(client, VIA686A_REG_TEMP_HYST(nr + 1), 
data->temp_hyst[nr]);
+	}
+	return count;
+}
+#define show_temp_offset(offset)			\
+static ssize_t						\
+show_temp_##offset (struct device *dev, char *buf)	\
+{							\
+	return show_temp(dev, buf, 0x##offset);		\
+}							\
+static ssize_t store_temp_##offset (struct device *dev, const char 
*buf, size_t count) \
+{								\
+	return store_temp(dev, buf, count, 0x##offset);		\
+}								\
+static DEVICE_ATTR(temp##offset, S_IRUGO | S_IWUSR, show_temp_##offset, 
store_temp_##offset)
+
+show_temp_offset(0);
+show_temp_offset(1);
+show_temp_offset(2);
+
+static ssize_t show_fan(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,"%d %d\n",
+		FAN_FROM_REG(data->fan_min[nr - 1], DIV_FROM_REG(data->fan_div[nr - 1])),
+		FAN_FROM_REG(data->fan[nr - 1], DIV_FROM_REG(data->fan_div[nr - 1])) );
+}
+static ssize_t store_fan(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);
+	int fan_min, ret;
+	ret = sscanf(buf, "%d", &fan_min);
+	if (ret = -1) return -EINVAL;
+	if (ret >= 1) {
+		data->fan_min[nr] = FAN_TO_REG(fan_min, DIV_FROM_REG(data->fan_div[nr]));
+		via686a_write_value(client, VIA686A_REG_FAN_MIN(nr+1), 
data->fan_min[nr]);
+	}
+	return count;
+}
+#define show_fan_offset(offset)				\
+static ssize_t						\
+show_fan_##offset (struct device *dev, char *buf)	\
+{							\
+	return show_fan(dev, buf, 0x##offset);		\
+}							\
+static ssize_t store_fan_##offset (struct device *dev, const char *buf, 
size_t count) \
+{								\
+	return store_fan(dev, buf, count, 0x##offset);		\
+}								\
+static DEVICE_ATTR(fan##offset, S_IRUGO | S_IWUSR, show_fan_##offset, 
store_fan_##offset)
+
+show_fan_offset(1);
+show_fan_offset(2);
+
+static ssize_t show_alarm(struct device *dev, char *buf) {
+	struct i2c_client *client = to_i2c_client(dev);
+	struct via686a_data *data = i2c_get_clientdata(client);
+	via686a_update_client(client);
+
+	return sprintf(buf,"%d\n", ALARMS_FROM_REG(data->alarms));
+}
+static DEVICE_ATTR(alarm, S_IRUGO | S_IWUSR, show_alarm, NULL);
+
+static ssize_t show_fan_div(struct device *dev, char *buf) {
+	struct i2c_client *client = to_i2c_client(dev);
+	struct via686a_data *data = i2c_get_clientdata(client);
+	via686a_update_client(client);
+
+	return sprintf(buf,"%d %d\n",
+		DIV_FROM_REG(data->fan_div[0]),
+		DIV_FROM_REG(data->fan_div[1]) );
+}
+static ssize_t store_fan_div(struct device *dev, const char *buf, 
size_t count) {
+	struct i2c_client *client = to_i2c_client(dev);
+	struct via686a_data *data = i2c_get_clientdata(client);
+	int fan_div[2], ret, old;

-static void via686a_in(struct i2c_client *client, int operation,
-		       int ctl_name, int *nrels_mag, long *results);
-static void via686a_fan(struct i2c_client *client, int operation,
-			int ctl_name, int *nrels_mag, long *results);
-static void via686a_temp(struct i2c_client *client, int operation,
-			 int ctl_name, int *nrels_mag, long *results);
-static void via686a_alarms(struct i2c_client *client, int operation,
-			   int ctl_name, int *nrels_mag, long *results);
-static void via686a_fan_div(struct i2c_client *client, int operation,
-			    int ctl_name, int *nrels_mag, long *results);
+	ret = sscanf(buf, "%d %d", &fan_div[0], &fan_div[1]);
+	if (ret = -1) return -EINVAL;
+	old = via686a_read_value(client, VIA686A_REG_FANDIV);
+	if (ret >= 2) {
+		data->fan_min[1] = DIV_TO_REG(fan_div[1]);
+		old = (old & 0x3f) | (data->fan_div[1] << 6);
+	}
+	if (ret >= 1) {
+		data->fan_min[0] = DIV_TO_REG(fan_div[0]);
+		old = (old & 0xcf) | (data->fan_div[0] << 4);
+		via686a_write_value(client, VIA686A_REG_FANDIV, old);
+	}
+	return count;
+}
+static DEVICE_ATTR(fan_div, S_IRUGO | S_IWUSR, show_fan_div, 
store_fan_div);

-static int via686a_id = 0;

  /* The driver. I choose to use type i2c_driver, as at is identical to both
     smbus_driver and isa_driver, and clients could be of either kind */
@@ -426,95 +586,18 @@
  };


-
-/* The /proc/sys entries */
-
-/* -- SENSORS SYSCTL START -- */
-#define VIA686A_SYSCTL_IN0 1000
-#define VIA686A_SYSCTL_IN1 1001
-#define VIA686A_SYSCTL_IN2 1002
-#define VIA686A_SYSCTL_IN3 1003
-#define VIA686A_SYSCTL_IN4 1004
-#define VIA686A_SYSCTL_FAN1 1101
-#define VIA686A_SYSCTL_FAN2 1102
-#define VIA686A_SYSCTL_TEMP 1200
-#define VIA686A_SYSCTL_TEMP2 1201
-#define VIA686A_SYSCTL_TEMP3 1202
-#define VIA686A_SYSCTL_FAN_DIV 2000
-#define VIA686A_SYSCTL_ALARMS 2001
-
-#define VIA686A_ALARM_IN0 0x01
-#define VIA686A_ALARM_IN1 0x02
-#define VIA686A_ALARM_IN2 0x04
-#define VIA686A_ALARM_IN3 0x08
-#define VIA686A_ALARM_TEMP 0x10
-#define VIA686A_ALARM_FAN1 0x40
-#define VIA686A_ALARM_FAN2 0x80
-#define VIA686A_ALARM_IN4 0x100
-#define VIA686A_ALARM_TEMP2 0x800
-#define VIA686A_ALARM_CHAS 0x1000
-#define VIA686A_ALARM_TEMP3 0x8000
-
-/* -- SENSORS SYSCTL END -- */
-
-/* These files are created for each detected VIA686A. This is just a 
template;
-   though at first sight, you might think we could use a statically
-   allocated list, we need some way to get back to the parent - which
-   is done through one of the 'extra' fields which are initialized
-   when a new copy is allocated. */
-static ctl_table via686a_dir_table_template[] = {
-	{VIA686A_SYSCTL_IN0, "in0", NULL, 0, 0644, NULL, &i2c_proc_real,
-	 &i2c_sysctl_real, NULL, &via686a_in},
-	{VIA686A_SYSCTL_IN1, "in1", NULL, 0, 0644, NULL, &i2c_proc_real,
-	 &i2c_sysctl_real, NULL, &via686a_in},
-	{VIA686A_SYSCTL_IN2, "in2", NULL, 0, 0644, NULL, &i2c_proc_real,
-	 &i2c_sysctl_real, NULL, &via686a_in},
-	{VIA686A_SYSCTL_IN3, "in3", NULL, 0, 0644, NULL, &i2c_proc_real,
-	 &i2c_sysctl_real, NULL, &via686a_in},
-	{VIA686A_SYSCTL_IN4, "in4", NULL, 0, 0644, NULL, &i2c_proc_real,
-	 &i2c_sysctl_real, NULL, &via686a_in},
-	{VIA686A_SYSCTL_FAN1, "fan1", NULL, 0, 0644, NULL, &i2c_proc_real,
-	 &i2c_sysctl_real, NULL, &via686a_fan},
-	{VIA686A_SYSCTL_FAN2, "fan2", NULL, 0, 0644, NULL, &i2c_proc_real,
-	 &i2c_sysctl_real, NULL, &via686a_fan},
-	{VIA686A_SYSCTL_TEMP, "temp1", NULL, 0, 0644, NULL, &i2c_proc_real,
-	 &i2c_sysctl_real, NULL, &via686a_temp},
-	{VIA686A_SYSCTL_TEMP2, "temp2", NULL, 0, 0644, NULL,
-	 &i2c_proc_real, &i2c_sysctl_real, NULL, &via686a_temp},
-	{VIA686A_SYSCTL_TEMP3, "temp3", NULL, 0, 0644, NULL,
-	 &i2c_proc_real, &i2c_sysctl_real, NULL, &via686a_temp},
-	{VIA686A_SYSCTL_FAN_DIV, "fan_div", NULL, 0, 0644, NULL,
-	 &i2c_proc_real, &i2c_sysctl_real, NULL, &via686a_fan_div},
-	{VIA686A_SYSCTL_ALARMS, "alarms", NULL, 0, 0444, NULL,
-	 &i2c_proc_real, &i2c_sysctl_real, NULL, &via686a_alarms},
-	{0}
-};
-
-static inline int via686a_read_value(struct i2c_client *client, u8 reg)
-{
-	return (inb_p(client->addr + reg));
-}
-
-static inline void via686a_write_value(struct i2c_client *client, u8 reg,
-				       u8 value)
-{
-	outb_p(value, client->addr + reg);
-}
-
  /* This is called when the module is loaded */
  static int via686a_attach_adapter(struct i2c_adapter *adapter)
  {
  	return i2c_detect(adapter, &addr_data, via686a_detect);
  }

-int via686a_detect(struct i2c_adapter *adapter, int address,
+static int via686a_detect(struct i2c_adapter *adapter, int address,
  		   unsigned short flags, int kind)
  {
-	int i;
  	struct i2c_client *new_client;
  	struct via686a_data *data;
  	int err = 0;
-	const char *type_name = "via686a";
  	const char client_name[] = "via686a chip";
  	u16 val;

@@ -573,28 +656,31 @@
  	/* Fill in the remaining client fields and put into the global list */
  	snprintf(new_client->dev.name, DEVICE_NAME_SIZE, client_name);

-	new_client->id = via686a_id++;
  	data->valid = 0;
  	init_MUTEX(&data->update_lock);
  	/* Tell the I2C layer a new client has arrived */
  	if ((err = i2c_attach_client(new_client)))
  		goto ERROR3;
-
-	/* Register a new directory entry with module sensors */
-	if ((i = i2c_register_entry((struct i2c_client *) new_client,
-					type_name,
-					via686a_dir_table_template)) < 0) {
-		err = i;
-		goto ERROR4;
-	}
-	data->sysctl_id = i;
+	
+	device_create_file(&new_client->dev, &dev_attr_in0);
+	device_create_file(&new_client->dev, &dev_attr_in1);
+	device_create_file(&new_client->dev, &dev_attr_in2);
+	device_create_file(&new_client->dev, &dev_attr_in3);
+	device_create_file(&new_client->dev, &dev_attr_in4);
+	device_create_file(&new_client->dev, &dev_attr_temp0);
+	device_create_file(&new_client->dev, &dev_attr_temp1);
+	device_create_file(&new_client->dev, &dev_attr_temp2);
+	device_create_file(&new_client->dev, &dev_attr_fan1);
+	device_create_file(&new_client->dev, &dev_attr_fan2);
+	device_create_file(&new_client->dev, &dev_attr_alarm);
+	device_create_file(&new_client->dev, &dev_attr_fan_div);

  	/* Initialize the VIA686A chip */
  	via686a_init_client(new_client);
  	return 0;

-      ERROR4:
-	i2c_detach_client(new_client);
+//      ERROR4:
+//	i2c_detach_client(new_client);
        ERROR3:
  	release_region(address, VIA686A_EXTENT);
  	kfree(new_client);
@@ -605,8 +691,8 @@
  static int via686a_detach_client(struct i2c_client *client)
  {
  	int err;
-	struct via686a_data *data = i2c_get_clientdata(client);
-	i2c_deregister_entry(data->sysctl_id);
+//	struct via686a_data *data = i2c_get_clientdata(client);
+//	i2c_deregister_entry(data->sysctl_id);

  	if ((err = i2c_detach_client(client))) {
  		dev_err(&client->dev,
@@ -739,157 +825,12 @@
  	up(&data->update_lock);
  }

-
-/* The next few functions are the call-back functions of the /proc/sys and
-   sysctl files. Which function is used is defined in the ctl_table in
-   the extra1 field.
-   Each function must return the magnitude (power of 10 to divide the date
-   with) if it is called with operation=SENSORS_PROC_REAL_INFO. It must
-   put a maximum of *nrels elements in results reflecting the data of this
-   file, and set *nrels to the number it actually put in it, if operation=
-   SENSORS_PROC_REAL_READ. Finally, it must get upto *nrels elements from
-   results and write them to the chip, if 
operations=SENSORS_PROC_REAL_WRITE.
-   Note that on SENSORS_PROC_REAL_READ, I do not check whether results is
-   large enough (by checking the incoming value of *nrels). This is not 
very
-   good practice, but as long as you put less than about 5 values in 
results,
-   you can assume it is large enough. */
-static void via686a_in(struct i2c_client *client, int operation, int 
ctl_name,
-               int *nrels_mag, long *results)
-{
-	struct via686a_data *data = i2c_get_clientdata(client);
-	int nr = ctl_name - VIA686A_SYSCTL_IN0;
-
-	if (operation = SENSORS_PROC_REAL_INFO)
-		*nrels_mag = 2;
-	else if (operation = SENSORS_PROC_REAL_READ) {
-		via686a_update_client(client);
-		results[0] = IN_FROM_REG(data->in_min[nr], nr);
-		results[1] = IN_FROM_REG(data->in_max[nr], nr);
-		results[2] = IN_FROM_REG(data->in[nr], nr);
-		*nrels_mag = 3;
-	} else if (operation = SENSORS_PROC_REAL_WRITE) {
-		if (*nrels_mag >= 1) {
-			data->in_min[nr] = IN_TO_REG(results[0], nr);
-			via686a_write_value(client, VIA686A_REG_IN_MIN(nr),
-					    data->in_min[nr]);
-		}
-		if (*nrels_mag >= 2) {
-			data->in_max[nr] = IN_TO_REG(results[1], nr);
-			via686a_write_value(client, VIA686A_REG_IN_MAX(nr),
-					    data->in_max[nr]);
-		}
-	}
-}
-
-void via686a_fan(struct i2c_client *client, int operation, int ctl_name,
-		 int *nrels_mag, long *results)
-{
-	struct via686a_data *data = i2c_get_clientdata(client);
-	int nr = ctl_name - VIA686A_SYSCTL_FAN1 + 1;
-
-	if (operation = SENSORS_PROC_REAL_INFO)
-		*nrels_mag = 0;
-	else if (operation = SENSORS_PROC_REAL_READ) {
-		via686a_update_client(client);
-		results[0] = FAN_FROM_REG(data->fan_min[nr - 1],
-					  DIV_FROM_REG(data->fan_div
-						       [nr - 1]));
-		results[1] = FAN_FROM_REG(data->fan[nr - 1],
-				 DIV_FROM_REG(data->fan_div[nr - 1]));
-		*nrels_mag = 2;
-	} else if (operation = SENSORS_PROC_REAL_WRITE) {
-		if (*nrels_mag >= 1) {
-			data->fan_min[nr - 1] = FAN_TO_REG(results[0],
-							   DIV_FROM_REG(data->
-							      fan_div[nr -1]));
-			via686a_write_value(client,
-					    VIA686A_REG_FAN_MIN(nr),
-					    data->fan_min[nr - 1]);
-		}
-	}
-}
-
-void via686a_temp(struct i2c_client *client, int operation, int ctl_name,
-		  int *nrels_mag, long *results)
-{
-	struct via686a_data *data = i2c_get_clientdata(client);
-	int nr = ctl_name - VIA686A_SYSCTL_TEMP;
-
-	if (operation = SENSORS_PROC_REAL_INFO)
-		*nrels_mag = 1;
-	else if (operation = SENSORS_PROC_REAL_READ) {
-		via686a_update_client(client);
-		results[0] = TEMP_FROM_REG(data->temp_over[nr]);
-		results[1] = TEMP_FROM_REG(data->temp_hyst[nr]);
-		results[2] = TEMP_FROM_REG10(data->temp[nr]);
-		*nrels_mag = 3;
-	} else if (operation = SENSORS_PROC_REAL_WRITE) {
-		if (*nrels_mag >= 1) {
-			data->temp_over[nr] = TEMP_TO_REG(results[0]);
-			via686a_write_value(client,
-					    VIA686A_REG_TEMP_OVER(nr + 1),
-					    data->temp_over[nr]);
-		}
-		if (*nrels_mag >= 2) {
-			data->temp_hyst[nr] = TEMP_TO_REG(results[1]);
-			via686a_write_value(client,
-					    VIA686A_REG_TEMP_HYST(nr + 1),
-					    data->temp_hyst[nr]);
-		}
-	}
-}
-
-void via686a_alarms(struct i2c_client *client, int operation, int ctl_name,
-		    int *nrels_mag, long *results)
-{
-	struct via686a_data *data = i2c_get_clientdata(client);
-	if (operation = SENSORS_PROC_REAL_INFO)
-		*nrels_mag = 0;
-	else if (operation = SENSORS_PROC_REAL_READ) {
-		via686a_update_client(client);
-		results[0] = ALARMS_FROM_REG(data->alarms);
-		*nrels_mag = 1;
-	}
-}
-
-void via686a_fan_div(struct i2c_client *client, int operation,
-		     int ctl_name, int *nrels_mag, long *results)
-{
-	struct via686a_data *data = i2c_get_clientdata(client);
-	int old;
-
-	if (operation = SENSORS_PROC_REAL_INFO)
-		*nrels_mag = 0;
-	else if (operation = SENSORS_PROC_REAL_READ) {
-		via686a_update_client(client);
-		results[0] = DIV_FROM_REG(data->fan_div[0]);
-		results[1] = DIV_FROM_REG(data->fan_div[1]);
-		*nrels_mag = 2;
-	} else if (operation = SENSORS_PROC_REAL_WRITE) {
-		old = via686a_read_value(client, VIA686A_REG_FANDIV);
-		if (*nrels_mag >= 2) {
-			data->fan_div[1] = DIV_TO_REG(results[1]);
-			old = (old & 0x3f) | (data->fan_div[1] << 6);
-		}
-		if (*nrels_mag >= 1) {
-			data->fan_div[0] = DIV_TO_REG(results[0]);
-			old = (old & 0xcf) | (data->fan_div[0] << 4);
-			via686a_write_value(client, VIA686A_REG_FANDIV,
-					    old);
-		}
-	}
-}
-
-
  static struct pci_device_id via686a_pci_ids[] __devinitdata = {
         {
  	       .vendor 		= PCI_VENDOR_ID_VIA,
  	       .device 		= PCI_DEVICE_ID_VIA_82C686_4,
  	       .subvendor	= PCI_ANY_ID,
  	       .subdevice	= PCI_ANY_ID,
-	       .class		= 0,
-	       .class_mask	= 0,
-	       .driver_data	= 0,
         },
         { 0, }
  };

w83781d i2c driver updated for 2.5.66 (without sysfs support)

[Greg KH]

On Wed, Mar 26, 2003 at 08:40:58PM +0100, Jan Dittmer wrote:
> Martin Schlemmer wrote:
> >
> >I did look at the changes needed for sysfs, but this beast have
> >about 6 ctl_tables, and is hairy in general.  I am not sure what
> >is the best way to do it for the different chips, so here is what
> >I have until I or somebody else can do the sysfs stuff.
> >
> I've just done this with the via686a driver. Saves about 100 lines of code.
> 
> Comments?

<snip>

> +/* following are the sysfs callback functions */
> +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 %ld %ld\n",
> +		IN_FROM_REG(data->in_min[nr], nr),
> +		IN_FROM_REG(data->in_max[nr], nr),
> +		IN_FROM_REG(data->in[nr], nr) );
> +}

We should really split these multivalue files up into individual files,
as sysfs is for single value files.  Makes parsing easier too.

Here's a patch for the lm75.c driver that does this.  As we are going to
need a "generic" read and write for the "real" values that the i2c
drivers use, I added these files to the i2c-proc.c file.

Yes, i2c_read_real() doesn't really work just yet :)

Anyway, I think this is the direction we should be moving to.  And what
is "temp_os" and "temp_hyst"?  Should these files be named something a
bit more descriptive?

thanks,

greg k-h

diff -Nru a/drivers/i2c/chips/lm75.c b/drivers/i2c/chips/lm75.c
--- a/drivers/i2c/chips/lm75.c	Wed Mar 26 12:28:45 2003
+++ b/drivers/i2c/chips/lm75.c	Wed Mar 26 12:28:45 2003
@@ -102,6 +102,49 @@
 
 static int lm75_id = 0;
 
+#define show(value)	\
+static ssize_t show_##value(struct device *dev, char *buf)	\
+{								\
+	struct i2c_client *client = to_i2c_client(dev);		\
+	struct lm75_data *data = i2c_get_clientdata(client);	\
+	int temp;						\
+								\
+	lm75_update_client(client);				\
+	temp = TEMP_FROM_REG(data->value);			\
+	return i2c_write_real(buf, temp, 1);			\
+}
+show(temp);
+show(temp_os);
+show(temp_hyst);
+
+static ssize_t set_temp_os(struct device *dev, const char *buf, size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct lm75_data *data = i2c_get_clientdata(client);
+	int temp;
+
+	i2c_read_real(buf, &temp, 1);
+	data->temp_os = TEMP_TO_REG(temp);
+	lm75_write_value(client, LM75_REG_TEMP_OS, data->temp_os);
+	return count;
+}
+
+static ssize_t set_temp_hyst(struct device *dev, const char *buf, size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct lm75_data *data = i2c_get_clientdata(client);
+	int temp;
+
+	i2c_read_real(buf, &temp, 1);
+	data->temp_hyst = TEMP_TO_REG(temp);
+	lm75_write_value(client, LM75_REG_TEMP_HYST, data->temp_hyst);
+	return count;
+}
+
+static DEVICE_ATTR(temp, S_IRUGO, show_temp, NULL);
+static DEVICE_ATTR(temp_os, 0644, show_temp_os, set_temp_os);
+static DEVICE_ATTR(temp_hyst, 0644, show_temp_hyst, set_temp_hyst);
+
 static int lm75_attach_adapter(struct i2c_adapter *adapter)
 {
 	return i2c_detect(adapter, &addr_data, lm75_detect);
@@ -192,6 +235,10 @@
 	if ((err = i2c_attach_client(new_client)))
 		goto error3;
 
+	device_create_file(&new_client->dev, &dev_attr_temp);
+	device_create_file(&new_client->dev, &dev_attr_temp_os);
+	device_create_file(&new_client->dev, &dev_attr_temp_hyst);
+
 	/* Register a new directory entry with module sensors */
 	i = i2c_register_entry(new_client, type_name, lm75_dir_table_template);
 	if (i < 0) {
diff -Nru a/drivers/i2c/i2c-proc.c b/drivers/i2c/i2c-proc.c
--- a/drivers/i2c/i2c-proc.c	Wed Mar 26 12:28:45 2003
+++ b/drivers/i2c/i2c-proc.c	Wed Mar 26 12:28:45 2003
@@ -381,6 +381,14 @@
 	return ret;
 }
 
+int i2c_read_real(const char *buf, int *real, int magnitude)
+{
+	char *temp;
+
+	*real = simple_strtoul(buf, &temp, 10);
+
+	return 0;
+}
 
 /* nrels contains initially the maximum number of elements which can be
    put in results, and finally the number of elements actually put there.
@@ -499,6 +507,29 @@
 	return 0;
 }
 
+int i2c_write_real(char *buf, int real, int magnitude)
+{
+	char printfstr[12];
+	int mag;
+	int times;
+	int field_location;
+
+	mag = magnitude;
+	for (times = 1; mag-- > 0; times *= 10)
+		;
+
+	if (real < 0) {
+		strcpy(printfstr, "-%ld.%0Xld");
+		field_location = 7;
+	} else {
+		strcpy(printfstr, "%ld.%0Xld");
+		field_location = 6;
+	}
+	printfstr[field_location] = magnitude + '0';
+	real = abs(real);
+	return sprintf(buf, printfstr, real / times, real % times);
+}
+
 static int i2c_write_reals(int nrels, void *buffer, size_t *bufsize,
 			 long *results, int magnitude)
 {
@@ -724,6 +755,8 @@
 EXPORT_SYMBOL(i2c_proc_real);
 EXPORT_SYMBOL(i2c_sysctl_real);
 EXPORT_SYMBOL(i2c_detect);
+EXPORT_SYMBOL(i2c_write_real);
+EXPORT_SYMBOL(i2c_read_real);
 
 MODULE_AUTHOR("Frodo Looijaard <frodol@dds.nl>");
 MODULE_DESCRIPTION("i2c-proc driver");
diff -Nru a/include/linux/i2c-proc.h b/include/linux/i2c-proc.h
--- a/include/linux/i2c-proc.h	Wed Mar 26 12:28:45 2003
+++ b/include/linux/i2c-proc.h	Wed Mar 26 12:28:45 2003
@@ -410,5 +410,8 @@
 	char name[SENSORS_PREFIX_MAX + 13];
 };
 
+extern int i2c_write_real(char *buf, int real, int magnitude);
+extern int i2c_read_real(const char *buf, int *real, int magnitude);
+
 #endif				/* def _LINUX_I2C_PROC_H */
 

w83781d i2c driver updated for 2.5.66 (without sysfs support)

[Mark Studebaker]

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.

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.

While not all drivers conform perfectly to the our standard (link below)
(lm75 temp_os and temp_hyst don't), this is the naming and
data format standard we attempt to follow. This has evolved
over the years. If the chip drivers in the kernel
diverge from this, or even worse diverge from each other haphazardly,
we're going to end up with a mess and no usable userspace tools
for a long long time. 

Please consider keeping the file names and contents unchanged.

thanks
mds

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


lm_sensors /proc naming standars for sensors chip drivers:

http://www2.lm-sensors.nu/~lm78/cvs/lm_sensors2/doc/developers/proc



Greg KH wrote:
> 
> On Wed, Mar 26, 2003 at 08:40:58PM +0100, Jan Dittmer wrote:
> > Martin Schlemmer wrote:
> > >
> > >I did look at the changes needed for sysfs, but this beast have
> > >about 6 ctl_tables, and is hairy in general.  I am not sure what
> > >is the best way to do it for the different chips, so here is what
> > >I have until I or somebody else can do the sysfs stuff.
> > >
> > I've just done this with the via686a driver. Saves about 100 lines of code.
> >
> > Comments?
> 
> <snip>
> 
> > +/* following are the sysfs callback functions */
> > +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 %ld %ld\n",
> > +             IN_FROM_REG(data->in_min[nr], nr),
> > +             IN_FROM_REG(data->in_max[nr], nr),
> > +             IN_FROM_REG(data->in[nr], nr) );
> > +}
> 
> We should really split these multivalue files up into individual files,
> as sysfs is for single value files.  Makes parsing easier too.
> 
> Here's a patch for the lm75.c driver that does this.

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

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

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

[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

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

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]

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

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 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

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:

> 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

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

[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/

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

[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

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

[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

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

[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

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 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

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

[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...

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

[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

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. 
> 
> 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?]

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

[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

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

[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

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 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

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

[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

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

[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);

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

[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

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

[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

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

[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

[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 --]

lm sensors sysfs file structure

[Martin Schlemmer]

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



-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20030327/b7e1284f/attachment.bin

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

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

[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.

lm sensors sysfs file structure

[Greg KH]

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

-------------- next part --------------
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

[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

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]

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

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

[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

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

[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

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

[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...

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...