i2c driver changes for 2.5.66; adding w83781d support.

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

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

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

Hi

Ok, this is the w83781d driver updated for 2.5.66bk2. It works over here.

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.

Regards,

Martin Schlemmer

@@ -36,5 +36,19 @@ config SENSORS_LM75
 	  You will also need the latest user-space utilties: you can find them
 	  in the lm_sensors package, which you can download at 
 	  http://www.lm-sensors.nu
+	  
+config SENSORS_W83781D
+	tristate "  Winbond W83781D, W83782D, W83783S, W83627HF, Asus AS99127F"
+	depends on I2C && I2C_PROC
+	help
+	  If you say yes here you get support for the Winbond W8378x series
+	  of sensor chips: the W83781D, W83782D, W83783S and W83682HF,
+	  and the similar Asus AS99127F. This
+	  can also be built as a module which can be inserted and removed
+	  while the kernel is running.
+	  
+	  You will also need the latest user-space utilties: you can find them
+	  in the lm_sensors package, which you can download at
+	  http://www.lm-sensors.nu
 
 endmenu

@@ -4,3 +4,4 @@
 
 obj-$(CONFIG_SENSORS_ADM1021)	+= adm1021.o
 obj-$(CONFIG_SENSORS_LM75)	+= lm75.o
+obj-$(CONFIG_SENSORS_W83781D)	+= w83781d.o

I've just done this with the via686a driver. Saves about 100 lines of code.

Comments?

Jan

@@ -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, }
 };

Nice example, thanks =) The w83781d does things in similar fashion, except that it support 6 different 'models', that have more or less the same readouts except for here and there a few more or less.

Regards,

Martin Schlemmer

<snip>

diff -Nru a/drivers/i2c/chips/lm75.c b/drivers/i2c/chips/lm75.c
@@ -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
@@ -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 <[email protected]>");
 MODULE_DESCRIPTION("i2c-proc driver");

diff -Nru a/include/linux/i2c-proc.h b/include/linux/i2c-proc.h
@@ -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 */
 

i2c-proc.c is the wrong place. Please add a i2c-sensor.c file with helper code for hardware sensors driver (i2c_detect should move over to there from i2c-proc.c aswell)

Oh, I agree. I just threw it there as I was matching up with other functions already in that file. Eventually i2c-proc.c should be deleted, as all of the proc stuff will be gone. I like the idea of i2c-sensor.c for the remaining i2c_detect() function.

thanks,

greg k-h

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

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

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.

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.

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

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.

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

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

Regards,

Martin Schlemmer

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

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

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

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/

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

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

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

I agree.

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

Hi!

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

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

Hi!

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

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

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

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

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

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

That looks very good to me, nice job.

Sensors developers, does this look sane?

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

[ stripped some cc's ]

No it stays 0. Happens also with other files in sysfs.

Jan

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

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

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

thanks,

greg k-h

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

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.

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

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

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

Regards,

Mark M. Hoffman
[email protected]

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

Hi!

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

Looks nice, thanks.

Some of the nasty casts should be fixed up though. Stuff like:

Quick question .... With sysfs, is it not needed to call
i2c_detach_client ? I am asking this as it seems from patches
that you remove all these calls ...

Regards,

Martin Schlemmer

No, that's still required. I didn't delete that in my lm75.c patch, did I?

You just don't have to call i2c_deregister_entry, as we don't call i2c_register_entry anymore.

thanks,

greg k-h

Hi

This should have a working w83781d driver updated for 2.5.66-bk4. Currently sysfs support is working, and are according to the spec (sensors-sysfs) in the 'lm sensors sysfs file structure' thread. Thus I used 'temp_input[1-3]', as there was not final decision on having them temp_input[0-2] as well, for example.

This patch is against a 'vanilla' 2.5.66-bk4 (do not have w83781d.c from lm_sensors2 cvs present). Retry, as previous did not go through due to size.

Regards,

Martin Schlemmer

diff -urPp ../linux-2.5.66bk4/drivers/i2c/chips/Kconfig linux.w83781d/drivers/i2c/chips/Kconfig
@@ -36,5 +36,19 @@ config SENSORS_LM75
 	  You will also need the latest user-space utilties: you can find them
 	  in the lm_sensors package, which you can download at 
 	  http://www.lm-sensors.nu
+	  
+config SENSORS_W83781D
+	tristate "  Winbond W83781D, W83782D, W83783S, W83627HF, Asus AS99127F"
+	depends on I2C && I2C_PROC
+	help
+	  If you say yes here you get support for the Winbond W8378x series
+	  of sensor chips: the W83781D, W83782D, W83783S and W83682HF,
+	  and the similar Asus AS99127F. This
+	  can also be built as a module which can be inserted and removed
+	  while the kernel is running.
+	  
+	  You will also need the latest user-space utilties: you can find them
+	  in the lm_sensors package, which you can download at
+	  http://www.lm-sensors.nu
 
 endmenu

diff -urPp ../linux-2.5.66bk4/drivers/i2c/chips/Makefile linux.w83781d/drivers/i2c/chips/Makefile
@@ -4,3 +4,4 @@
 
 obj-$(CONFIG_SENSORS_ADM1021)	+= adm1021.o
 obj-$(CONFIG_SENSORS_LM75)	+= lm75.o
+obj-$(CONFIG_SENSORS_W83781D)	+= w83781d.o

Thanks, I've applied this to my trees and will send it off to Linus in a short bit.

greg k-h