linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* 2.6 - sysfs sensor nameing inconsistency
@ 2003-07-15 18:14 Andrey Borzenkov
  2003-07-15 20:18 ` Greg KH
  0 siblings, 1 reply; 15+ messages in thread
From: Andrey Borzenkov @ 2003-07-15 18:14 UTC (permalink / raw)
  To: linux-kernel

In 2.4 all sensor chip got a subdirectory with name derived from type_name - a 
single word describing sensor, like

adm1021.c:              type_name = "max1617";
adm1021.c:              type_name = "max1617a";
adm1021.c:              type_name = "adm1021";
adm1021.c:              type_name = "adm1023";
adm1021.c:              type_name = "thmc10";
adm1021.c:              type_name = "lm84";
adm1021.c:              type_name = "gl523sm";
adm1021.c:              type_name = "mc1066";
...

etc. All user-level configuration (sensors, gkrellm) have been using these 
names to match available sensors and configuration data.

In 2.6 sensors appear under /sysfs, type_name no more used and the only 
identification available is .../name, but it seems to be arbitrary chosen 
like

- single word ("it87") - lm87.c
- "name chip" or "name subclient" - most others (lm78.c, wd83781d.c etc)
- completely arbitrary shiny description - "Generic LM85", "National LM85-B" 
etc in lm85.c

This means, any user program accessing sensors need incompatible changes and 
comfiuration cannot be shared between 2.4 and 2.6 without serious redesign 
and/or some translation layer.

If there are serious reasons to keep current names in "name" - what about 
adding extra type_name property that will hold type_name compatible with 2.4, 
at least for those drivers that are also available there. This would allow 
easily reuse existing sensors configuration.

TIA

-andrey

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: 2.6 - sysfs sensor nameing inconsistency
  2003-07-15 18:14 2.6 - sysfs sensor nameing inconsistency Andrey Borzenkov
@ 2003-07-15 20:18 ` Greg KH
  2003-07-26 18:00   ` Andrey Borzenkov
  0 siblings, 1 reply; 15+ messages in thread
From: Greg KH @ 2003-07-15 20:18 UTC (permalink / raw)
  To: Andrey Borzenkov; +Cc: linux-kernel

On Tue, Jul 15, 2003 at 10:14:38PM +0400, Andrey Borzenkov wrote:
> In 2.4 all sensor chip got a subdirectory with name derived from type_name - a 
> single word describing sensor, like

And they used sysctl and proc, ugh, ugly :)

> adm1021.c:              type_name = "max1617";
> adm1021.c:              type_name = "max1617a";
> adm1021.c:              type_name = "adm1021";
> adm1021.c:              type_name = "adm1023";
> adm1021.c:              type_name = "thmc10";
> adm1021.c:              type_name = "lm84";
> adm1021.c:              type_name = "gl523sm";
> adm1021.c:              type_name = "mc1066";
> ...
> 
> etc. All user-level configuration (sensors, gkrellm) have been using these 
> names to match available sensors and configuration data.
> 
> In 2.6 sensors appear under /sysfs, type_name no more used and the only 
> identification available is .../name, but it seems to be arbitrary chosen 
> like
> 
> - single word ("it87") - lm87.c
> - "name chip" or "name subclient" - most others (lm78.c, wd83781d.c etc)
> - completely arbitrary shiny description - "Generic LM85", "National LM85-B" 
> etc in lm85.c
> 
> This means, any user program accessing sensors need incompatible changes and 
> comfiuration cannot be shared between 2.4 and 2.6 without serious redesign 
> and/or some translation layer.

The "translation layer" is libsensors.  libsensors needs to be rewritten
for 2.6.  The sensors people know this, and it's even detailed on their
web page.  Any help with this is greatly appreciated.

> If there are serious reasons to keep current names in "name" - what about 
> adding extra type_name property that will hold type_name compatible with 2.4, 
> at least for those drivers that are also available there. This would allow 
> easily reuse existing sensors configuration.

Patches to help do this are always welcome :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: 2.6 - sysfs sensor nameing inconsistency
  2003-07-15 20:18 ` Greg KH
@ 2003-07-26 18:00   ` Andrey Borzenkov
  2003-08-15 20:51     ` Greg KH
  0 siblings, 1 reply; 15+ messages in thread
From: Andrey Borzenkov @ 2003-07-26 18:00 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel

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

On Wednesday 16 July 2003 00:18, Greg KH wrote:
[...]
> The "translation layer" is libsensors.  libsensors needs to be rewritten
> for 2.6.  The sensors people know this, and it's even detailed on their
> web page.  Any help with this is greatly appreciated.
>

I do mean libsensors. Also gkrellm does not use libsensors (it interfaces with 
proc/sys directly) and I already have user reports about naming problems.

> > If there are serious reasons to keep current names in "name" - what about
> > adding extra type_name property that will hold type_name compatible with
> > 2.4, at least for those drivers that are also available there. This would
> > allow easily reuse existing sensors configuration.
>
> Patches to help do this are always welcome :)
>

Attached is patch against 2.6.0-test1 that adds type_name to all in-tree 
sensors; it sets it to the same values as corr. 2.4 senors and (in one case) 
changes client name to match that of 2.4.

Assuming this patch (or variant thereof) is accepted I can then produce 
libsensors patch that will easily reuse current sensors.conf. I have already 
done it for gkrellm and as Mandrake is going to include 2.6 in next release 
sensors support becomes more of an issue.

It compiles and w83781d works. Comments appreciated.

regards

-andrey

[-- Attachment #2: 2.6.0-test1-sensors_type_name.patch --]
[-- Type: text/x-diff, Size: 11252 bytes --]

--- linux-2.6.0-test1-smp/drivers/i2c/chips/adm1021.c.type_name	2003-06-26 21:41:22.000000000 +0400
+++ linux-2.6.0-test1-smp/drivers/i2c/chips/adm1021.c	2003-07-26 20:47:19.000000000 +0400
@@ -139,7 +139,6 @@ static void adm1021_update_client(struct
 /* (amalysh) read only mode, otherwise any limit's writing confuse BIOS */
 static int read_only = 0;
 
-
 /* This is the driver that will be inserted */
 static struct i2c_driver adm1021_driver = {
 	.owner		= THIS_MODULE,
@@ -150,6 +149,13 @@ static struct i2c_driver adm1021_driver 
 	.detach_client	= adm1021_detach_client,
 };
 
+static const char *type_name = "";
+static ssize_t show_type_name(struct device *dev, char *buf)
+{
+	return sprintf(buf, "%s\n", type_name);			
+}
+static DEVICE_ATTR(type_name, S_IRUGO, show_type_name, NULL);
+
 /* I choose here for semi-static allocation. Complete dynamic
    allocation could also be used; the code needed for this would probably
    take more memory than the datastructure takes now. */
@@ -224,7 +230,6 @@ static int adm1021_detect(struct i2c_ada
 	struct i2c_client *new_client;
 	struct adm1021_data *data;
 	int err = 0;
-	const char *type_name = "";
 	const char *client_name = "";
 
 	/* Make sure we aren't probing the ISA bus!! This is just a safety check
@@ -331,6 +336,7 @@ static int adm1021_detect(struct i2c_ada
 	if ((err = i2c_attach_client(new_client)))
 		goto error3;
 
+	device_create_file(&new_client->dev, &dev_attr_type_name);
 	device_create_file(&new_client->dev, &dev_attr_temp_max1);
 	device_create_file(&new_client->dev, &dev_attr_temp_min1);
 	device_create_file(&new_client->dev, &dev_attr_temp_input1);
--- linux-2.6.0-test1-smp/drivers/i2c/chips/it87.c.type_name	2003-06-26 21:39:34.000000000 +0400
+++ linux-2.6.0-test1-smp/drivers/i2c/chips/it87.c	2003-07-26 21:13:52.000000000 +0400
@@ -236,6 +236,13 @@ static struct i2c_driver it87_driver = {
 
 static int it87_id = 0;
 
+static const char *type_name = "";
+static ssize_t show_type_name(struct device *dev, char *buf)
+{
+	return sprintf(buf, "%s\n", type_name);			
+}
+static DEVICE_ATTR(type_name, S_IRUGO, show_type_name, NULL);
+
 static ssize_t show_in(struct device *dev, char *buf, int nr)
 {
 	struct i2c_client *client = to_i2c_client(dev);
@@ -591,7 +598,6 @@ int it87_detect(struct i2c_adapter *adap
 	struct i2c_client *new_client = NULL;
 	struct it87_data *data;
 	int err = 0;
-	const char *name = "";
 	const char *client_name = "";
 	int is_isa = i2c_is_isa_adapter(adapter);
 
@@ -601,7 +607,7 @@ int it87_detect(struct i2c_adapter *adap
 
 	/* Reserve the ISA region */
 	if (is_isa)
-		if (!request_region(address, IT87_EXTENT, name))
+		if (!request_region(address, IT87_EXTENT, type_name))
 			goto ERROR0;
 
 	/* Probe whether there is anything available on this address. Already
@@ -680,10 +686,10 @@ int it87_detect(struct i2c_adapter *adap
 	}
 
 	if (kind == it87) {
-		name = "it87";
+		type_name = "it87";
 		client_name = "IT87 chip";
 	} /* else if (kind == it8712) {
-		name = "it8712";
+		type_name = "it8712";
 		client_name = "IT87-J chip";
 	} */ else {
 		dev_dbg(&adapter->dev, "Internal error: unknown kind (%d)?!?",
@@ -692,7 +698,7 @@ int it87_detect(struct i2c_adapter *adap
 	}
 
 	/* Fill in the remaining client fields and put it into the global list */
-	strlcpy(new_client->dev.name, name, DEVICE_NAME_SIZE);
+	strlcpy(new_client->dev.name, client_name, DEVICE_NAME_SIZE);
 
 	data->type = kind;
 
@@ -705,6 +711,7 @@ int it87_detect(struct i2c_adapter *adap
 		goto ERROR1;
 
 	/* register sysfs hooks */
+	device_create_file(&new_client->dev, &dev_attr_type_name);
 	device_create_file(&new_client->dev, &dev_attr_in_input0);
 	device_create_file(&new_client->dev, &dev_attr_in_input1);
 	device_create_file(&new_client->dev, &dev_attr_in_input2);
--- linux-2.6.0-test1-smp/drivers/i2c/chips/lm75.c.type_name	2003-06-26 21:39:34.000000000 +0400
+++ linux-2.6.0-test1-smp/drivers/i2c/chips/lm75.c	2003-07-26 20:52:45.000000000 +0400
@@ -86,6 +86,13 @@ static struct i2c_driver lm75_driver = {
 
 static int lm75_id = 0;
 
+static const char *type_name = "";
+static ssize_t show_type_name(struct device *dev, char *buf)
+{
+	return sprintf(buf, "%s\n", type_name);			
+}
+static DEVICE_ATTR(type_name, S_IRUGO, show_type_name, NULL);
+
 #define show(value)	\
 static ssize_t show_##value(struct device *dev, char *buf)	\
 {								\
@@ -133,7 +140,7 @@ static int lm75_detect(struct i2c_adapte
 	struct i2c_client *new_client;
 	struct lm75_data *data;
 	int err = 0;
-	const char *name;
+	const char *client_name;
 
 	/* Make sure we aren't probing the ISA bus!! This is just a safety check
 	   at this moment; i2c_detect really won't call us. */
@@ -186,7 +193,8 @@ static int lm75_detect(struct i2c_adapte
 		kind = lm75;
 
 	if (kind == lm75) {
-		name = "lm75";
+		type_name = "lm75";
+		client_name = "LM75 chip";
 	} else {
 		dev_dbg(&adapter->dev, "Internal error: unknown kind (%d)?!?",
 			kind);
@@ -194,7 +202,7 @@ static int lm75_detect(struct i2c_adapte
 	}
 
 	/* Fill in the remaining client fields and put it into the global list */
-	strlcpy(new_client->dev.name, name, DEVICE_NAME_SIZE);
+	strlcpy(new_client->dev.name, client_name, DEVICE_NAME_SIZE);
 
 	new_client->id = lm75_id++;
 	data->valid = 0;
@@ -204,6 +212,7 @@ static int lm75_detect(struct i2c_adapte
 	if ((err = i2c_attach_client(new_client)))
 		goto exit_free;
 
+	device_create_file(&new_client->dev, &dev_attr_type_name);
 	device_create_file(&new_client->dev, &dev_attr_temp_max);
 	device_create_file(&new_client->dev, &dev_attr_temp_min);
 	device_create_file(&new_client->dev, &dev_attr_temp_input);
--- linux-2.6.0-test1-smp/drivers/i2c/chips/lm78.c.type_name	2003-06-26 21:41:22.000000000 +0400
+++ linux-2.6.0-test1-smp/drivers/i2c/chips/lm78.c	2003-07-26 20:54:14.000000000 +0400
@@ -231,6 +231,13 @@ static struct i2c_driver lm78_driver = {
 	.detach_client	= lm78_detach_client,
 };
 
+static const char *type_name = "";
+static ssize_t show_type_name(struct device *dev, char *buf)
+{
+	return sprintf(buf, "%s\n", type_name);			
+}
+static DEVICE_ATTR(type_name, S_IRUGO, show_type_name, NULL);
+
 /* 7 Voltages */
 static ssize_t show_in(struct device *dev, char *buf, int nr)
 {
@@ -625,10 +632,13 @@ int lm78_detect(struct i2c_adapter *adap
 	}
 
 	if (kind == lm78) {
+		type_name = "lm78";
 		client_name = "LM78 chip";
 	} else if (kind == lm78j) {
+		type_name = "lm78-j";
 		client_name = "LM78-J chip";
 	} else if (kind == lm79) {
+		type_name = "lm79";
 		client_name = "LM79 chip";
 	} else {
 		dev_dbg(&adapter->dev, "Internal error: unknown kind (%d)?!?",
@@ -649,6 +659,7 @@ int lm78_detect(struct i2c_adapter *adap
 		goto ERROR2;
 
 	/* register sysfs hooks */
+	device_create_file(&new_client->dev, &dev_attr_type_name);
 	device_create_file(&new_client->dev, &dev_attr_in_input0);
 	device_create_file(&new_client->dev, &dev_attr_in_min0);
 	device_create_file(&new_client->dev, &dev_attr_in_max0);
--- linux-2.6.0-test1-smp/drivers/i2c/chips/lm85.c.type_name	2003-06-26 21:41:22.000000000 +0400
+++ linux-2.6.0-test1-smp/drivers/i2c/chips/lm85.c	2003-07-26 20:54:42.000000000 +0400
@@ -411,6 +411,13 @@ static struct i2c_driver lm85_driver = {
 /* Unique ID assigned to each LM85 detected */
 static int lm85_id = 0;
 
+static const char *type_name = "";
+static ssize_t show_type_name(struct device *dev, char *buf)
+{
+	return sprintf(buf, "%s\n", type_name);			
+}
+static DEVICE_ATTR(type_name, S_IRUGO, show_type_name, NULL);
+
 
 /* 4 Fans */
 static ssize_t show_fan(struct device *dev, char *buf, int nr)
@@ -759,7 +766,6 @@ int lm85_detect(struct i2c_adapter *adap
 	struct i2c_client *new_client = NULL;
 	struct lm85_data *data;
 	int err = 0;
-	const char *type_name = "";
 
 	if (i2c_is_isa_adapter(adapter)) {
 		/* This chip has no ISA interface */
@@ -892,6 +898,7 @@ int lm85_detect(struct i2c_adapter *adap
 	/* Set the VRM version */
 	data->vrm = LM85_INIT_VRM ;
 
+	device_create_file(&new_client->dev, &dev_attr_type_name);
 	device_create_file(&new_client->dev, &dev_attr_fan_input1);
 	device_create_file(&new_client->dev, &dev_attr_fan_input2);
 	device_create_file(&new_client->dev, &dev_attr_fan_input3);
--- linux-2.6.0-test1-smp/drivers/i2c/chips/via686a.c.type_name	2003-06-26 21:39:34.000000000 +0400
+++ linux-2.6.0-test1-smp/drivers/i2c/chips/via686a.c	2003-07-26 20:57:01.000000000 +0400
@@ -409,6 +409,13 @@ static void via686a_init_client(struct i
 
 /* following are the sysfs callback functions */
 
+static const char *type_name = "via686a";
+static ssize_t show_type_name(struct device *dev, char *buf)
+{
+	return sprintf(buf, "%s\n", type_name);			
+}
+static DEVICE_ATTR(type_name, S_IRUGO, show_type_name, NULL);
+
 /* 7 voltage sensors */
 static ssize_t show_in(struct device *dev, char *buf, int nr) {
 	struct i2c_client *client = to_i2c_client(dev);
@@ -671,7 +678,7 @@ static int via686a_detect(struct i2c_ada
 	struct i2c_client *new_client;
 	struct via686a_data *data;
 	int err = 0;
-	const char client_name[] = "via686a chip";
+	const char client_name[] = "Via 686A Integrated Sensors";
 	u16 val;
 
 	/* Make sure we are probing the ISA bus!!  */
@@ -736,6 +743,7 @@ static int via686a_detect(struct i2c_ada
 		goto ERROR3;
 	
 	/* register sysfs hooks */
+	device_create_file(&new_client->dev, &dev_attr_type_name);
 	device_create_file(&new_client->dev, &dev_attr_in_input0);
 	device_create_file(&new_client->dev, &dev_attr_in_input1);
 	device_create_file(&new_client->dev, &dev_attr_in_input2);
--- linux-2.6.0-test1-smp/drivers/i2c/chips/w83781d.c.type_name	2003-06-26 21:41:22.000000000 +0400
+++ linux-2.6.0-test1-smp/drivers/i2c/chips/w83781d.c	2003-07-26 21:05:38.000000000 +0400
@@ -357,6 +357,13 @@ static struct i2c_driver w83781d_driver 
 };
 
 /* following are the sysfs callback functions */
+static const char *type_name = "";
+static ssize_t show_type_name(struct device *dev, char *buf)
+{
+	return sprintf(buf, "%s\n", type_name);			
+}
+static DEVICE_ATTR(type_name, S_IRUGO, show_type_name, NULL);
+
 #define show_in_reg(reg) \
 static ssize_t show_##reg (struct device *dev, char *buf, int nr) \
 { \
@@ -1304,19 +1311,27 @@ w83781d_detect(struct i2c_adapter *adapt
 	}
 
 	if (kind == w83781d) {
+		type_name = "w83781d";
 		client_name = "W83781D chip";
 	} else if (kind == w83782d) {
+		type_name = "w83782d";
 		client_name = "W83782D chip";
 	} else if (kind == w83783s) {
+		type_name = "w83783s";
 		client_name = "W83783S chip";
 	} else if (kind == w83627hf) {
-		if (val1 == 0x90)
+		if (val1 == 0x90) {
+			type_name = "w83627thf";
 			client_name = "W83627THF chip";
-		else
+		} else {
+			type_name = "w83627hf";
 			client_name = "W83627HF chip";
+		}
 	} else if (kind == as99127f) {
+		type_name = "as99127f";
 		client_name = "AS99127F chip";
 	} else if (kind == w83697hf) {
+		type_name = "w83697hf";
 		client_name = "W83697HF chip";
 	} else {
 		dev_err(&new_client->dev, "Internal error: unknown "
@@ -1346,6 +1361,8 @@ w83781d_detect(struct i2c_adapter *adapt
 		data->lm75[1] = NULL;
 	}
 
+	device_create_file(&new_client->dev, &dev_attr_type_name);
+
 	device_create_file_in(new_client, 0);
 	if (kind != w83783s && kind != w83697hf)
 		device_create_file_in(new_client, 1);

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: 2.6 - sysfs sensor nameing inconsistency
  2003-07-26 18:00   ` Andrey Borzenkov
@ 2003-08-15 20:51     ` Greg KH
  2003-08-16 15:38       ` Andrey Borzenkov
  0 siblings, 1 reply; 15+ messages in thread
From: Greg KH @ 2003-08-15 20:51 UTC (permalink / raw)
  To: Andrey Borzenkov; +Cc: linux-kernel

On Sat, Jul 26, 2003 at 10:00:51PM +0400, Andrey Borzenkov wrote:
> 
> Attached is patch against 2.6.0-test1 that adds type_name to all in-tree 
> sensors; it sets it to the same values as corr. 2.4 senors and (in one case) 
> changes client name to match that of 2.4.
> 
> Assuming this patch (or variant thereof) is accepted I can then produce 
> libsensors patch that will easily reuse current sensors.conf. I have already 
> done it for gkrellm and as Mandrake is going to include 2.6 in next release 
> sensors support becomes more of an issue.

I like this idea, but now that the name logic has changed in the i2c
code, care to re-do this patch?  Just set the name field instead of
creating a new file in sysfs.

Sound ok?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: 2.6 - sysfs sensor nameing inconsistency
  2003-08-15 20:51     ` Greg KH
@ 2003-08-16 15:38       ` Andrey Borzenkov
  2003-08-16 16:50         ` Greg KH
  0 siblings, 1 reply; 15+ messages in thread
From: Andrey Borzenkov @ 2003-08-16 15:38 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel

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

On Saturday 16 August 2003 00:51, Greg KH wrote:
> On Sat, Jul 26, 2003 at 10:00:51PM +0400, Andrey Borzenkov wrote:
> > Attached is patch against 2.6.0-test1 that adds type_name to all in-tree
> > sensors; it sets it to the same values as corr. 2.4 senors and (in one
> > case) changes client name to match that of 2.4.
> >
> > Assuming this patch (or variant thereof) is accepted I can then produce
> > libsensors patch that will easily reuse current sensors.conf. I have
> > already done it for gkrellm and as Mandrake is going to include 2.6 in
> > next release sensors support becomes more of an issue.
>
> I like this idea, but now that the name logic has changed in the i2c
> code, care to re-do this patch?  Just set the name field instead of
> creating a new file in sysfs.
>

something like attached patch? I like it as well :)

note that in 2.6.0-test3 name in sysfs is empty. I had to add a chunk to 
i2c-core to at least test my patch. or may be I misunderstood how 
client->name is used.

regards

-andrey


[-- Attachment #2: 2.6.0-test3-sensors_type_name-2.patch --]
[-- Type: text/x-diff, Size: 10937 bytes --]

--- ../tmp/linux-2.6.0-test3/drivers/i2c/i2c-core.c	2003-08-09 13:10:05.000000000 +0400
+++ linux-2.6.0-test3-smp/drivers/i2c/i2c-core.c	2003-08-16 19:19:17.000000000 +0400
@@ -347,7 +347,7 @@ int i2c_attach_client(struct i2c_client 
 	}
 
 	DEB(dev_dbg(&adapter->dev, "client [%s] registered to adapter\n",
-			client->dev.name));
+			client->name));
 
 	if (client->flags & I2C_CLIENT_ALLOW_USE)
 		client->usage_count = 0;
@@ -356,6 +356,7 @@ int i2c_attach_client(struct i2c_client 
 	client->dev.driver = &client->driver->driver;
 	client->dev.bus = &i2c_bus_type;
 	client->dev.release = &i2c_client_release;
+	strcpy(client->dev.name, client->name);
 	
 	snprintf(&client->dev.bus_id[0], sizeof(client->dev.bus_id),
 		"%d-%04x", i2c_adapter_id(adapter), client->addr);
--- ../tmp/linux-2.6.0-test3/drivers/i2c/chips/adm1021.c	2003-08-09 13:10:04.000000000 +0400
+++ linux-2.6.0-test3-smp/drivers/i2c/chips/adm1021.c	2003-08-16 18:45:16.000000000 +0400
@@ -225,7 +225,6 @@ static int adm1021_detect(struct i2c_ada
 	struct adm1021_data *data;
 	int err = 0;
 	const char *type_name = "";
-	const char *client_name = "";
 
 	/* Make sure we aren't probing the ISA bus!! This is just a safety check
 	   at this moment; i2c_detect really won't call us. */
@@ -291,28 +290,20 @@ static int adm1021_detect(struct i2c_ada
 
 	if (kind == max1617) {
 		type_name = "max1617";
-		client_name = "MAX1617 chip";
 	} else if (kind == max1617a) {
 		type_name = "max1617a";
-		client_name = "MAX1617A chip";
 	} else if (kind == adm1021) {
 		type_name = "adm1021";
-		client_name = "ADM1021 chip";
 	} else if (kind == adm1023) {
 		type_name = "adm1023";
-		client_name = "ADM1023 chip";
 	} else if (kind == thmc10) {
 		type_name = "thmc10";
-		client_name = "THMC10 chip";
 	} else if (kind == lm84) {
 		type_name = "lm84";
-		client_name = "LM84 chip";
 	} else if (kind == gl523sm) {
 		type_name = "gl523sm";
-		client_name = "GL523SM chip";
 	} else if (kind == mc1066) {
 		type_name = "mc1066";
-		client_name = "MC1066 chip";
 	} else {
 		dev_err(&adapter->dev, "Internal error: unknown kind (%d)?!?",
 			kind);
@@ -320,7 +311,7 @@ static int adm1021_detect(struct i2c_ada
 	}
 
 	/* Fill in the remaining client fields and put it into the global list */
-	strlcpy(new_client->name, client_name, DEVICE_NAME_SIZE);
+	strlcpy(new_client->name, type_name, DEVICE_NAME_SIZE);
 	data->type = kind;
 
 	new_client->id = adm1021_id++;
--- ../tmp/linux-2.6.0-test3/drivers/i2c/chips/it87.c	2003-08-09 13:10:04.000000000 +0400
+++ linux-2.6.0-test3-smp/drivers/i2c/chips/it87.c	2003-08-16 18:59:14.000000000 +0400
@@ -591,8 +591,7 @@ int it87_detect(struct i2c_adapter *adap
 	struct i2c_client *new_client = NULL;
 	struct it87_data *data;
 	int err = 0;
-	const char *name = "";
-	const char *client_name = "";
+	const char *type_name = "";
 	int is_isa = i2c_is_isa_adapter(adapter);
 
 	if (!is_isa && 
@@ -601,7 +600,7 @@ int it87_detect(struct i2c_adapter *adap
 
 	/* Reserve the ISA region */
 	if (is_isa)
-		if (!request_region(address, IT87_EXTENT, name))
+		if (!request_region(address, IT87_EXTENT, type_name))
 			goto ERROR0;
 
 	/* Probe whether there is anything available on this address. Already
@@ -680,11 +679,9 @@ int it87_detect(struct i2c_adapter *adap
 	}
 
 	if (kind == it87) {
-		name = "it87";
-		client_name = "IT87 chip";
+		type_name = "it87";
 	} /* else if (kind == it8712) {
-		name = "it8712";
-		client_name = "IT87-J chip";
+		type_name = "it8712";
 	} */ else {
 		dev_dbg(&adapter->dev, "Internal error: unknown kind (%d)?!?",
 			kind);
@@ -692,7 +689,7 @@ int it87_detect(struct i2c_adapter *adap
 	}
 
 	/* Fill in the remaining client fields and put it into the global list */
-	strlcpy(new_client->name, name, DEVICE_NAME_SIZE);
+	strlcpy(new_client->name, type_name, DEVICE_NAME_SIZE);
 
 	data->type = kind;
 
--- ../tmp/linux-2.6.0-test3/drivers/i2c/chips/lm75.c	2003-08-09 13:10:04.000000000 +0400
+++ linux-2.6.0-test3-smp/drivers/i2c/chips/lm75.c	2003-08-16 18:47:13.000000000 +0400
@@ -133,7 +133,7 @@ static int lm75_detect(struct i2c_adapte
 	struct i2c_client *new_client;
 	struct lm75_data *data;
 	int err = 0;
-	const char *name;
+	const char *type_name;
 
 	/* Make sure we aren't probing the ISA bus!! This is just a safety check
 	   at this moment; i2c_detect really won't call us. */
@@ -186,7 +186,7 @@ static int lm75_detect(struct i2c_adapte
 		kind = lm75;
 
 	if (kind == lm75) {
-		name = "lm75";
+		type_name = "lm75";
 	} else {
 		dev_dbg(&adapter->dev, "Internal error: unknown kind (%d)?!?",
 			kind);
@@ -194,7 +194,7 @@ static int lm75_detect(struct i2c_adapte
 	}
 
 	/* Fill in the remaining client fields and put it into the global list */
-	strlcpy(new_client->name, name, DEVICE_NAME_SIZE);
+	strlcpy(new_client->name, type_name, DEVICE_NAME_SIZE);
 
 	new_client->id = lm75_id++;
 	data->valid = 0;
--- ../tmp/linux-2.6.0-test3/drivers/i2c/chips/lm78.c	2003-08-09 13:10:04.000000000 +0400
+++ linux-2.6.0-test3-smp/drivers/i2c/chips/lm78.c	2003-08-16 18:48:09.000000000 +0400
@@ -519,7 +519,7 @@ int lm78_detect(struct i2c_adapter *adap
 	int i, err;
 	struct i2c_client *new_client;
 	struct lm78_data *data;
-	const char *client_name = "";
+	const char *type_name = "";
 	int is_isa = i2c_is_isa_adapter(adapter);
 
 	if (!is_isa &&
@@ -625,11 +625,11 @@ int lm78_detect(struct i2c_adapter *adap
 	}
 
 	if (kind == lm78) {
-		client_name = "LM78 chip";
+		type_name = "lm78";
 	} else if (kind == lm78j) {
-		client_name = "LM78-J chip";
+		type_name = "lm78-j";
 	} else if (kind == lm79) {
-		client_name = "LM79 chip";
+		type_name = "lm79";
 	} else {
 		dev_dbg(&adapter->dev, "Internal error: unknown kind (%d)?!?",
 			kind);
@@ -638,7 +638,7 @@ int lm78_detect(struct i2c_adapter *adap
 	}
 
 	/* Fill in the remaining client fields and put into the global list */
-	strlcpy(new_client->name, client_name, DEVICE_NAME_SIZE);
+	strlcpy(new_client->name, type_name, DEVICE_NAME_SIZE);
 	data->type = kind;
 
 	data->valid = 0;
--- ../tmp/linux-2.6.0-test3/drivers/i2c/chips/lm85.c	2003-08-09 13:10:04.000000000 +0400
+++ linux-2.6.0-test3-smp/drivers/i2c/chips/lm85.c	2003-08-16 18:49:17.000000000 +0400
@@ -853,24 +853,20 @@ int lm85_detect(struct i2c_adapter *adap
 	/* Fill in the chip specific driver values */
 	if ( kind == any_chip ) {
 		type_name = "lm85";
-		strlcpy(new_client->name, "Generic LM85", DEVICE_NAME_SIZE);
 	} else if ( kind == lm85b ) {
 		type_name = "lm85b";
-		strlcpy(new_client->name, "National LM85-B", DEVICE_NAME_SIZE);
 	} else if ( kind == lm85c ) {
 		type_name = "lm85c";
-		strlcpy(new_client->name, "National LM85-C", DEVICE_NAME_SIZE);
 	} else if ( kind == adm1027 ) {
 		type_name = "adm1027";
-		strlcpy(new_client->name, "Analog Devices ADM1027", DEVICE_NAME_SIZE);
 	} else if ( kind == adt7463 ) {
 		type_name = "adt7463";
-		strlcpy(new_client->name, "Analog Devices ADT7463", DEVICE_NAME_SIZE);
 	} else {
 		dev_dbg(&adapter->dev, "Internal error, invalid kind (%d)!", kind);
 		err = -EFAULT ;
 		goto ERROR1;
 	}
+	strlcpy(new_client->name, type_name, DEVICE_NAME_SIZE);
 
 	/* Fill in the remaining client fields */
 	new_client->id = lm85_id++;
--- ../tmp/linux-2.6.0-test3/drivers/i2c/chips/via686a.c	2003-08-09 13:10:05.000000000 +0400
+++ linux-2.6.0-test3-smp/drivers/i2c/chips/via686a.c	2003-08-16 18:51:18.000000000 +0400
@@ -671,7 +671,7 @@ static int via686a_detect(struct i2c_ada
 	struct i2c_client *new_client;
 	struct via686a_data *data;
 	int err = 0;
-	const char client_name[] = "via686a chip";
+	const char type_name[] = "via686a";
 	u16 val;
 
 	/* Make sure we are probing the ISA bus!!  */
@@ -727,7 +727,7 @@ static int via686a_detect(struct i2c_ada
 	new_client->dev.parent = &adapter->dev;
 
 	/* Fill in the remaining client fields and put into the global list */
-	snprintf(new_client->name, DEVICE_NAME_SIZE, client_name);
+	strlcpy(new_client->name, type_name, DEVICE_NAME_SIZE);
 
 	data->valid = 0;
 	init_MUTEX(&data->update_lock);
--- ../tmp/linux-2.6.0-test3/drivers/i2c/chips/w83781d.c	2003-08-09 13:10:05.000000000 +0400
+++ linux-2.6.0-test3-smp/drivers/i2c/chips/w83781d.c	2003-08-16 19:21:27.000000000 +0400
@@ -1041,7 +1041,7 @@ w83781d_detect_subclients(struct i2c_ada
 {
 	int i, val1 = 0, id;
 	int err;
-	const char *client_name;
+	const char *type_name;
 	struct w83781d_data *data = i2c_get_clientdata(new_client);
 
 	data->lm75[0] = kmalloc(sizeof(struct i2c_client), GFP_KERNEL);
@@ -1098,17 +1098,17 @@ w83781d_detect_subclients(struct i2c_ada
 	}
 
 	if (kind == w83781d)
-		client_name = "W83781D subclient";
+		type_name = "w83781d";
 	else if (kind == w83782d)
-		client_name = "W83782D subclient";
+		type_name = "w83782d";
 	else if (kind == w83783s)
-		client_name = "W83783S subclient";
+		type_name = "w83783s";
 	else if (kind == w83627hf)
-		client_name = "W83627HF subclient";
+		type_name = "w83627hf";
 	else if (kind == as99127f)
-		client_name = "AS99127F subclient";
+		type_name = "as99127f";
 	else
-		client_name = "unknown subclient?";
+		type_name = "unknown subclient?";
 
 	for (i = 0; i <= 1; i++) {
 		/* store all data in w83781d */
@@ -1116,7 +1116,7 @@ w83781d_detect_subclients(struct i2c_ada
 		data->lm75[i]->adapter = adapter;
 		data->lm75[i]->driver = &w83781d_driver;
 		data->lm75[i]->flags = 0;
-		strlcpy(data->lm75[i]->name, client_name,
+		strlcpy(data->lm75[i]->name, type_name,
 			DEVICE_NAME_SIZE);
 		if ((err = i2c_attach_client(data->lm75[i]))) {
 			dev_err(&new_client->dev, "Subclient %d "
@@ -1152,7 +1152,7 @@ w83781d_detect(struct i2c_adapter *adapt
 	struct i2c_client *new_client;
 	struct w83781d_data *data;
 	int err;
-	const char *client_name = "";
+	const char *type_name = "";
 	int is_isa = i2c_is_isa_adapter(adapter);
 	enum vendor { winbond, asus } vendid;
 
@@ -1304,20 +1304,20 @@ w83781d_detect(struct i2c_adapter *adapt
 	}
 
 	if (kind == w83781d) {
-		client_name = "W83781D chip";
+		type_name = "w83781d";
 	} else if (kind == w83782d) {
-		client_name = "W83782D chip";
+		type_name = "w83782d";
 	} else if (kind == w83783s) {
-		client_name = "W83783S chip";
+		type_name = "w83783s";
 	} else if (kind == w83627hf) {
 		if (val1 == 0x90)
-			client_name = "W83627THF chip";
+			type_name = "w83627thf";
 		else
-			client_name = "W83627HF chip";
+			type_name = "w83627hf";
 	} else if (kind == as99127f) {
-		client_name = "AS99127F chip";
+		type_name = "as99127f";
 	} else if (kind == w83697hf) {
-		client_name = "W83697HF chip";
+		type_name = "w83697hf";
 	} else {
 		dev_err(&new_client->dev, "Internal error: unknown "
 						"kind (%d)?!?", kind);
@@ -1326,7 +1326,7 @@ w83781d_detect(struct i2c_adapter *adapt
 	}
 
 	/* Fill in the remaining client fields and put into the global list */
-	strlcpy(new_client->name, client_name, DEVICE_NAME_SIZE);
+	strlcpy(new_client->name, type_name, DEVICE_NAME_SIZE);
 	data->type = kind;
 
 	data->valid = 0;

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: 2.6 - sysfs sensor nameing inconsistency
  2003-08-16 15:38       ` Andrey Borzenkov
@ 2003-08-16 16:50         ` Greg KH
  2003-08-18 16:49           ` Andrey Borzenkov
  2003-08-19 19:19           ` Andrey Borzenkov
  0 siblings, 2 replies; 15+ messages in thread
From: Greg KH @ 2003-08-16 16:50 UTC (permalink / raw)
  To: Andrey Borzenkov; +Cc: linux-kernel

On Sat, Aug 16, 2003 at 07:38:47PM +0400, Andrey Borzenkov wrote:
> On Saturday 16 August 2003 00:51, Greg KH wrote:
> > On Sat, Jul 26, 2003 at 10:00:51PM +0400, Andrey Borzenkov wrote:
> > > Attached is patch against 2.6.0-test1 that adds type_name to all in-tree
> > > sensors; it sets it to the same values as corr. 2.4 senors and (in one
> > > case) changes client name to match that of 2.4.
> > >
> > > Assuming this patch (or variant thereof) is accepted I can then produce
> > > libsensors patch that will easily reuse current sensors.conf. I have
> > > already done it for gkrellm and as Mandrake is going to include 2.6 in
> > > next release sensors support becomes more of an issue.
> >
> > I like this idea, but now that the name logic has changed in the i2c
> > code, care to re-do this patch?  Just set the name field instead of
> > creating a new file in sysfs.
> >
> 
> something like attached patch? I like it as well :)

Why rename local variables?  Your patch would be a lot smaller if you
just keep the same local name variable, and fix up the name strings.

> note that in 2.6.0-test3 name in sysfs is empty. I had to add a chunk to 
> i2c-core to at least test my patch. or may be I misunderstood how 
> client->name is used.

No, you are correct.  I've added the name info back in the -bk tree
right now.  Try 2.6.0-test3-bk4 for the proper usage.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: 2.6 - sysfs sensor nameing inconsistency
  2003-08-16 16:50         ` Greg KH
@ 2003-08-18 16:49           ` Andrey Borzenkov
  2003-08-18 21:31             ` Greg KH
  2003-08-19 19:19           ` Andrey Borzenkov
  1 sibling, 1 reply; 15+ messages in thread
From: Andrey Borzenkov @ 2003-08-18 16:49 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel

On Saturday 16 August 2003 20:50, Greg KH wrote:
> > > I like this idea, but now that the name logic has changed in the i2c
> > > code, care to re-do this patch?  Just set the name field instead of
> > > creating a new file in sysfs.
> >
> > something like attached patch? I like it as well :)
>
> Why rename local variables?  Your patch would be a lot smaller if you
> just keep the same local name variable, and fix up the name strings.
>

To make it clear for anyone porting other drivers that we are using type_name 
and not client_name or whatever. In 2.4 every driver have both; mixing name, 
type_name and client_name will just add to confusion.

I will redo patch if you insist but I really prefer having things consistent 
if possible.

thank you

-andrey

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: 2.6 - sysfs sensor nameing inconsistency
  2003-08-18 16:49           ` Andrey Borzenkov
@ 2003-08-18 21:31             ` Greg KH
  0 siblings, 0 replies; 15+ messages in thread
From: Greg KH @ 2003-08-18 21:31 UTC (permalink / raw)
  To: Andrey Borzenkov; +Cc: linux-kernel, sensors

On Mon, Aug 18, 2003 at 08:49:57PM +0400, Andrey Borzenkov wrote:
> On Saturday 16 August 2003 20:50, Greg KH wrote:
> > > > I like this idea, but now that the name logic has changed in the i2c
> > > > code, care to re-do this patch?  Just set the name field instead of
> > > > creating a new file in sysfs.
> > >
> > > something like attached patch? I like it as well :)
> >
> > Why rename local variables?  Your patch would be a lot smaller if you
> > just keep the same local name variable, and fix up the name strings.
> >
> 
> To make it clear for anyone porting other drivers that we are using type_name 
> and not client_name or whatever. In 2.4 every driver have both; mixing name, 
> type_name and client_name will just add to confusion.

No, we don't need both a "type_name" and a "client_name" anymore, right?
So it's just a simple "name" for the i2c client device.  "type_name" is
handled by the name of the client driver now.

> I will redo patch if you insist but I really prefer having things consistent 
> if possible.

I prefer having things make sense :)
So yes, I'd prefer if you change your patch.

I've cced the sensors mailing list to get any feedback from them.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: 2.6 - sysfs sensor nameing inconsistency
  2003-08-16 16:50         ` Greg KH
  2003-08-18 16:49           ` Andrey Borzenkov
@ 2003-08-19 19:19           ` Andrey Borzenkov
  2003-08-19 19:45             ` Greg KH
  1 sibling, 1 reply; 15+ messages in thread
From: Andrey Borzenkov @ 2003-08-19 19:19 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel

On Sat, Jul 26, 2003 at 10:00:51PM +0400, Andrey Borzenkov wrote:
> Assuming this patch (or variant thereof) is accepted I can then
> produce libsensors patch that will easily reuse current sensors.conf.
> I have already done it for gkrellm and as Mandrake is going to
> include 2.6 in next release sensors support becomes more of an issue.

There are more issues than just type_name.

In libsensors only one file need to be touched - proc.c (file that deals with 
kernel interface). Unfortunately, not all information expected by libsensors 
is currently available is sysfs. Here is output of first stab:

{pts/1}% LD_LIBRARY_PATH=$PWD/lib ./prog/sensors/sensors
as99127f-i2c-0-2d
Adapter: DUMMY
Algorithm: DUMMY
VCore 1:   +1.70 V  (min =  +1.49 V, max =  +1.81 V)
+3.3V:     +3.47 V  (min =  +2.98 V, max =  +3.63 V)
+5V:       +5.00 V  (min =  +4.52 V, max =  +5.48 V)
+12V:     +11.37 V  (min = +10.82 V, max = +13.13 V)
-12V:     -11.51 V  (min = -12.33 V, max = -15.07 V)       ALARM
-5V:       -5.03 V  (min =  -4.50 V, max =  -5.49 V)
CPU:      4753 RPM  (min = 3000 RPM, div = 2)
Front:    2909 RPM  (min = 3000 RPM, div = 2)              ALARM
ERROR: Can't get TEMP1 data!
ERROR: Can't get TEMP2 data!
ERROR: Can't get TEMP3 data!
vid:      +1.650 V
alarms:
beep_enable:
          Sound alarm enabled

So we have following problems:

1. I do not know where to get information on adapters (called busses actually 
in libsensors) and algorithms. I.e. the information corresponding to 2.4:

{pts/2}% cat ~/tmp/i2c-bus
i2c-0   smbus           SMBus I801 adapter at e800              Non-I2C SMBus 
adapter

2. I do not know - and sysfs does not provide any information - how to 
identify chips-of-interest as opposed to generic i2c devices. I.e. w83781d 
has both clients and subclients (2.4 again):

{pts/2}% cat ~/tmp/i2c-0-bus
2d      AS99127F chip                           W83781D sensor driver
48      AS99127F subclient                      W83781D sensor driver
49      AS99127F subclient                      W83781D sensor driver

and we are interested in clients only. In 2.4 we get this information from 
kernel as result of sysctl :))

{pts/2}% cat ~/tmp/i2c-sysctl-chips
256     as99127f-i2c-0-2d

while in 2.6 we see in sysfs all three devices:

pts/2}% l /sys/bus/i2c/devices
0-002d@  0-0048@  0-0049@

without any obvious way to know which one(s) to use.

3. libsensors asks for hysteresis value. This one does not exist in sysfs (so 
all temp readings fail). Is it emulated by kernel or read off chip?

4. I do not have the slightest idea how ISA adapters look like in sysfs and 
where they are located. Anyone can give me example?

So the patch to ibsensors is actually trivial enough. Main issue is contents 
of sysfs

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: 2.6 - sysfs sensor nameing inconsistency
  2003-08-19 19:19           ` Andrey Borzenkov
@ 2003-08-19 19:45             ` Greg KH
  2003-08-31 16:25               ` Andrey Borzenkov
  0 siblings, 1 reply; 15+ messages in thread
From: Greg KH @ 2003-08-19 19:45 UTC (permalink / raw)
  To: Andrey Borzenkov; +Cc: linux-kernel, sensors

On Tue, Aug 19, 2003 at 11:19:01PM +0400, Andrey Borzenkov wrote:
> On Sat, Jul 26, 2003 at 10:00:51PM +0400, Andrey Borzenkov wrote:
> > Assuming this patch (or variant thereof) is accepted I can then
> > produce libsensors patch that will easily reuse current sensors.conf.
> > I have already done it for gkrellm and as Mandrake is going to
> > include 2.6 in next release sensors support becomes more of an issue.
> 
> There are more issues than just type_name.

Hey, one thing at a time :)

> In libsensors only one file need to be touched - proc.c (file that deals with 
> kernel interface). Unfortunately, not all information expected by libsensors 
> is currently available is sysfs. Here is output of first stab:
> 
> {pts/1}% LD_LIBRARY_PATH=$PWD/lib ./prog/sensors/sensors
> as99127f-i2c-0-2d
> Adapter: DUMMY
> Algorithm: DUMMY
> VCore 1:   +1.70 V  (min =  +1.49 V, max =  +1.81 V)
> +3.3V:     +3.47 V  (min =  +2.98 V, max =  +3.63 V)
> +5V:       +5.00 V  (min =  +4.52 V, max =  +5.48 V)
> +12V:     +11.37 V  (min = +10.82 V, max = +13.13 V)
> -12V:     -11.51 V  (min = -12.33 V, max = -15.07 V)       ALARM
> -5V:       -5.03 V  (min =  -4.50 V, max =  -5.49 V)
> CPU:      4753 RPM  (min = 3000 RPM, div = 2)
> Front:    2909 RPM  (min = 3000 RPM, div = 2)              ALARM
> ERROR: Can't get TEMP1 data!
> ERROR: Can't get TEMP2 data!
> ERROR: Can't get TEMP3 data!
> vid:      +1.650 V
> alarms:
> beep_enable:
>           Sound alarm enabled
> 
> So we have following problems:
> 
> 1. I do not know where to get information on adapters (called busses actually 
> in libsensors) and algorithms. I.e. the information corresponding to 2.4:
> 
> {pts/2}% cat ~/tmp/i2c-bus
> i2c-0   smbus           SMBus I801 adapter at e800              Non-I2C SMBus adapter

Look in /sys/class/i2c-adapter/  It shows all of the i2c adapters in the
system, and points to where they are in the device tree.  For example,
on my desktop:

$ tree ../class/i2c-adapter/
../class/i2c-adapter/
|-- i2c-0
|   |-- device -> ../../../devices/pci0000:00/0000:00:1f.3/i2c-0
|   `-- driver -> ../../../bus/i2c/drivers/i2c_adapter
`-- i2c-1
    |-- device -> ../../../devices/legacy/i2c-1
    `-- driver -> ../../../bus/i2c/drivers/i2c_adapter

Then look at the device directory for the i2c-0 adapter:

$ ls /sys/class/i2c-adapter/i2c-0/device/
0-0048/  0-0049/  0-0053/  detach_state  name  power/

Hm, a name file:
$ cat /sys/class/i2c-adapter/i2c-0/device/name 
SMBus I801 adapter at 8000

Ah, the same info as you showed for 2.4 :)

> 2. I do not know - and sysfs does not provide any information - how to 
> identify chips-of-interest as opposed to generic i2c devices. I.e. w83781d 
> has both clients and subclients (2.4 again):
> 
> {pts/2}% cat ~/tmp/i2c-0-bus
> 2d      AS99127F chip                           W83781D sensor driver
> 48      AS99127F subclient                      W83781D sensor driver
> 49      AS99127F subclient                      W83781D sensor driver
> 
> and we are interested in clients only. In 2.4 we get this information from 
> kernel as result of sysctl :))
> 
> {pts/2}% cat ~/tmp/i2c-sysctl-chips
> 256     as99127f-i2c-0-2d
> 
> while in 2.6 we see in sysfs all three devices:
> 
> pts/2}% l /sys/bus/i2c/devices
> 0-002d@  0-0048@  0-0049@
> 
> without any obvious way to know which one(s) to use.

That's what you are going to have to set the name file to in the
i2c_client structure, much like your patch did.  Then look at the
different name files in each device directory to see what kind of device
it is (chip, subclient, etc.)

> 3. libsensors asks for hysteresis value. This one does not exist in sysfs (so 
> all temp readings fail). Is it emulated by kernel or read off chip?

The kernel is exporting all of the info that it knows about through
sysfs.  If we missed a value somewhere, please let us know.  I'm pretty
sure they are all present:

$ ls /sys/class/i2c-adapter/i2c-0/device/0-0048/
detach_state  name  power/  temp_input  temp_max  temp_min

> 4. I do not have the slightest idea how ISA adapters look like in sysfs and 
> where they are located. Anyone can give me example?

They show up on the legacy bus:

$ tree /sys/class/i2c-adapter/i2c-1/
/sys/class/i2c-adapter/i2c-1/
|-- device -> ../../../devices/legacy/i2c-1
`-- driver -> ../../../bus/i2c/drivers/i2c_adapter

$ ls /sys/class/i2c-adapter/i2c-1/device/
detach_state  name  power

I don't have any drivers loaded that are attached to this adapter right
now, but I think you get the idea (it's the same as for pci devices.)

> So the patch to ibsensors is actually trivial enough. Main issue is contents 
> of sysfs

You might want to take a look at libsysfs, it makes finding all of the
devices and attributes in the sysfs tree a whole lot easer.

Hope this helps,

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: 2.6 - sysfs sensor nameing inconsistency
  2003-08-19 19:45             ` Greg KH
@ 2003-08-31 16:25               ` Andrey Borzenkov
  2003-09-22 22:29                 ` Greg KH
  0 siblings, 1 reply; 15+ messages in thread
From: Andrey Borzenkov @ 2003-08-31 16:25 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, sensors

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

On Tuesday 19 August 2003 23:45, Greg KH wrote:
> On Tue, Aug 19, 2003 at 11:19:01PM +0400, Andrey Borzenkov wrote:
[...]
> > There are more issues than just type_name.
[...]
> > 1. I do not know where to get information on adapters (called busses
> > actually in libsensors) and algorithms. I.e. the information
> > corresponding to 2.4:
> >
> > {pts/2}% cat ~/tmp/i2c-bus
> > i2c-0   smbus           SMBus I801 adapter at e800              Non-I2C
> > SMBus adapter
>
> Look in /sys/class/i2c-adapter/
[...]
> Hm, a name file:
> $ cat /sys/class/i2c-adapter/i2c-0/device/name
> SMBus I801 adapter at 8000
>
> Ah, the same info as you showed for 2.4 :)
>

Sure it not the same info it is only part of it. I do not know if libsensors 
just using this for display or needs it internally. Thank you for pointer 
though.

> > 2. I do not know - and sysfs does not provide any information - how to
> > identify chips-of-interest as opposed to generic i2c devices. I.e.
> > w83781d has both clients and subclients (2.4 again):
> >
> > {pts/2}% cat ~/tmp/i2c-0-bus
> > 2d      AS99127F chip                           W83781D sensor driver
> > 48      AS99127F subclient                      W83781D sensor driver
> > 49      AS99127F subclient                      W83781D sensor driver
> >
[...]
>
> That's what you are going to have to set the name file to in the
> i2c_client structure, much like your patch did.  Then look at the
> different name files in each device directory to see what kind of device
> it is (chip, subclient, etc.)
>

OK attached patch sets all names to just chip name for chips themselves and 
"chipname subclient" when subclient ios registered.

> > 3. libsensors asks for hysteresis value. This one does not exist in sysfs
> > (so all temp readings fail). Is it emulated by kernel or read off chip?
>
> The kernel is exporting all of the info that it knows about through
> sysfs.

it just arbitrarily (re-)names them. "min" is not hysteresis; name is badly 
chosen.
[...]
> > 4. I do not have the slightest idea how ISA adapters look like in sysfs
> > and where they are located. Anyone can give me example?
>
> They show up on the legacy bus:
>
> $ tree /sys/class/i2c-adapter/i2c-1/
> /sys/class/i2c-adapter/i2c-1/
>
> |-- device -> ../../../devices/legacy/i2c-1
>

This does not help much. Libsensors expects as adapter identification either 
"i2c-N" or "isa". If I set it to "isa" I do not have any way to determine 
sysfs path except by rescanning /sys/class/i2c-adapter every time. Having 
/sys/class/i2-adapter/isa/... would be better, apparently it is assumed that 
only one such adapter can exist.
[...]
> > So the patch to ibsensors is actually trivial enough. Main issue is
> > contents of sysfs
>
> You might want to take a look at libsysfs, it makes finding all of the
> devices and attributes in the sysfs tree a whole lot easer.
>

No I did not want to make code dependent on libsysfs, besides arsing itself is 
trivial.

Attached is patch against 2.6.0-test3-bk8 that sets all chip names to simply 
chip names :) and patch for libsensors that makes use of it. libsensors still 
needs to be built under 2.4, no attempt is made to allow building under 2.6. 
I built it using make LINUX_HADERS=/home/bor/src/linux-2.4.21/linux 
COMPILE_KERNEL=0. It is possible to get rid of this dependency but I'd like 
to settle sysfs issue first. As can be seen the ugliest part is name 
translation between sysfs attributes and libsensors feature names ... if 
attributes were named consistently the whole would be much more simple.

This works perfectly for reading, writing does not work here but it is not my 
patch. Reading:
{pts/2}% LD_LIBRARY_PATH=./lib ./prog/sensors/sensors
as99127f-i2c-0-2d
Adapter: SMBus I801 adapter at e800
Algorithm: Not available via sysfs
VCore 1:   +1.70 V  (min =  +1.49 V, max =  +1.81 V)
+3.3V:     +3.47 V  (min =  +2.98 V, max =  +3.63 V)
+5V:       +5.00 V  (min =  +4.52 V, max =  +5.48 V)
+12V:     +11.37 V  (min = +10.82 V, max = +13.13 V)
-12V:     -11.57 V  (min = -12.33 V, max = -15.07 V)       ALARM
-5V:       -5.03 V  (min =  -4.50 V, max =  -5.49 V)
CPU:      4787 RPM  (min = 3000 RPM, div = 2)
Front:    2922 RPM  (min = 3000 RPM, div = 2)              ALARM
MB:          +28╟C  (limit = +127╟C, hysteresis =  +60╟C)
CPU:       +36.0╟C  (limit = +111╟C, hysteresis =  +97╟C)
HDD:       +37.0╟C  (limit = +120╟C, hysteresis = +100╟C)
vid:      +1.650 V
alarms:
beep_enable:
          Sound alarm enabled

writing:

{pts/2}% cat /sys/class/i2c-adapter/i2c-0/device/0-002d/in_max2
3632
{pts/2}% sudo zsh -c 'echo 3500 > 
/sys/class/i2c-adapter/i2c-0/device/0-002d/in_max2'
{pts/2}% cat /sys/class/i2c-adapter/i2c-0/device/0-002d/in_max2
400

so writing is broken at least for w83781d driver

-andrey

[-- Attachment #2: 2.6.0-test3-bk8-sensors_type_name-3.patch --]
[-- Type: text/x-diff, Size: 6288 bytes --]

--- linux-2.6.0-test3-bk8/drivers/i2c/chips/adm1021.c.chip_name	2003-08-20 20:58:39.000000000 +0400
+++ linux-2.6.0-test3-bk8/drivers/i2c/chips/adm1021.c	2003-08-23 21:30:57.000000000 +0400
@@ -225,7 +225,6 @@ static int adm1021_detect(struct i2c_ada
 	struct adm1021_data *data;
 	int err = 0;
 	const char *type_name = "";
-	const char *client_name = "";
 
 	/* Make sure we aren't probing the ISA bus!! This is just a safety check
 	   at this moment; i2c_detect really won't call us. */
@@ -291,28 +290,20 @@ static int adm1021_detect(struct i2c_ada
 
 	if (kind == max1617) {
 		type_name = "max1617";
-		client_name = "MAX1617 chip";
 	} else if (kind == max1617a) {
 		type_name = "max1617a";
-		client_name = "MAX1617A chip";
 	} else if (kind == adm1021) {
 		type_name = "adm1021";
-		client_name = "ADM1021 chip";
 	} else if (kind == adm1023) {
 		type_name = "adm1023";
-		client_name = "ADM1023 chip";
 	} else if (kind == thmc10) {
 		type_name = "thmc10";
-		client_name = "THMC10 chip";
 	} else if (kind == lm84) {
 		type_name = "lm84";
-		client_name = "LM84 chip";
 	} else if (kind == gl523sm) {
 		type_name = "gl523sm";
-		client_name = "GL523SM chip";
 	} else if (kind == mc1066) {
 		type_name = "mc1066";
-		client_name = "MC1066 chip";
 	} else {
 		dev_err(&adapter->dev, "Internal error: unknown kind (%d)?!?",
 			kind);
@@ -320,7 +311,7 @@ static int adm1021_detect(struct i2c_ada
 	}
 
 	/* Fill in the remaining client fields and put it into the global list */
-	strlcpy(new_client->name, client_name, DEVICE_NAME_SIZE);
+	strlcpy(new_client->name, type_name, DEVICE_NAME_SIZE);
 	data->type = kind;
 
 	new_client->id = adm1021_id++;
--- linux-2.6.0-test3-bk8/drivers/i2c/chips/it87.c.chip_name	2003-08-09 13:10:04.000000000 +0400
+++ linux-2.6.0-test3-bk8/drivers/i2c/chips/it87.c	2003-08-23 21:31:30.000000000 +0400
@@ -592,7 +592,6 @@ int it87_detect(struct i2c_adapter *adap
 	struct it87_data *data;
 	int err = 0;
 	const char *name = "";
-	const char *client_name = "";
 	int is_isa = i2c_is_isa_adapter(adapter);
 
 	if (!is_isa && 
@@ -681,10 +680,8 @@ int it87_detect(struct i2c_adapter *adap
 
 	if (kind == it87) {
 		name = "it87";
-		client_name = "IT87 chip";
 	} /* else if (kind == it8712) {
 		name = "it8712";
-		client_name = "IT87-J chip";
 	} */ else {
 		dev_dbg(&adapter->dev, "Internal error: unknown kind (%d)?!?",
 			kind);
--- linux-2.6.0-test3-bk8/drivers/i2c/chips/lm78.c.chip_name	2003-08-09 13:10:04.000000000 +0400
+++ linux-2.6.0-test3-bk8/drivers/i2c/chips/lm78.c	2003-08-23 21:32:17.000000000 +0400
@@ -625,11 +625,11 @@ int lm78_detect(struct i2c_adapter *adap
 	}
 
 	if (kind == lm78) {
-		client_name = "LM78 chip";
+		client_name = "lm78";
 	} else if (kind == lm78j) {
-		client_name = "LM78-J chip";
+		client_name = "lm78-j";
 	} else if (kind == lm79) {
-		client_name = "LM79 chip";
+		client_name = "lm79";
 	} else {
 		dev_dbg(&adapter->dev, "Internal error: unknown kind (%d)?!?",
 			kind);
--- linux-2.6.0-test3-bk8/drivers/i2c/chips/lm85.c.chip_name	2003-08-09 13:10:04.000000000 +0400
+++ linux-2.6.0-test3-bk8/drivers/i2c/chips/lm85.c	2003-08-23 21:32:59.000000000 +0400
@@ -853,24 +853,20 @@ int lm85_detect(struct i2c_adapter *adap
 	/* Fill in the chip specific driver values */
 	if ( kind == any_chip ) {
 		type_name = "lm85";
-		strlcpy(new_client->name, "Generic LM85", DEVICE_NAME_SIZE);
 	} else if ( kind == lm85b ) {
 		type_name = "lm85b";
-		strlcpy(new_client->name, "National LM85-B", DEVICE_NAME_SIZE);
 	} else if ( kind == lm85c ) {
 		type_name = "lm85c";
-		strlcpy(new_client->name, "National LM85-C", DEVICE_NAME_SIZE);
 	} else if ( kind == adm1027 ) {
 		type_name = "adm1027";
-		strlcpy(new_client->name, "Analog Devices ADM1027", DEVICE_NAME_SIZE);
 	} else if ( kind == adt7463 ) {
 		type_name = "adt7463";
-		strlcpy(new_client->name, "Analog Devices ADT7463", DEVICE_NAME_SIZE);
 	} else {
 		dev_dbg(&adapter->dev, "Internal error, invalid kind (%d)!", kind);
 		err = -EFAULT ;
 		goto ERROR1;
 	}
+	strlcpy(new_client->name, type_name, DEVICE_NAME_SIZE);
 
 	/* Fill in the remaining client fields */
 	new_client->id = lm85_id++;
--- linux-2.6.0-test3-bk8/drivers/i2c/chips/via686a.c.chip_name	2003-08-09 13:10:05.000000000 +0400
+++ linux-2.6.0-test3-bk8/drivers/i2c/chips/via686a.c	2003-08-23 21:33:22.000000000 +0400
@@ -671,7 +671,7 @@ static int via686a_detect(struct i2c_ada
 	struct i2c_client *new_client;
 	struct via686a_data *data;
 	int err = 0;
-	const char client_name[] = "via686a chip";
+	const char client_name[] = "via686a";
 	u16 val;
 
 	/* Make sure we are probing the ISA bus!!  */
--- linux-2.6.0-test3-bk8/drivers/i2c/chips/w83781d.c.chip_name	2003-08-09 13:10:05.000000000 +0400
+++ linux-2.6.0-test3-bk8/drivers/i2c/chips/w83781d.c	2003-08-23 21:34:51.000000000 +0400
@@ -1098,15 +1098,15 @@ w83781d_detect_subclients(struct i2c_ada
 	}
 
 	if (kind == w83781d)
-		client_name = "W83781D subclient";
+		client_name = "w83781d subclient";
 	else if (kind == w83782d)
-		client_name = "W83782D subclient";
+		client_name = "w83782d subclient";
 	else if (kind == w83783s)
-		client_name = "W83783S subclient";
+		client_name = "w83783s subclient";
 	else if (kind == w83627hf)
-		client_name = "W83627HF subclient";
+		client_name = "w83627hf subclient";
 	else if (kind == as99127f)
-		client_name = "AS99127F subclient";
+		client_name = "as99127f subclient";
 	else
 		client_name = "unknown subclient?";
 
@@ -1304,20 +1304,20 @@ w83781d_detect(struct i2c_adapter *adapt
 	}
 
 	if (kind == w83781d) {
-		client_name = "W83781D chip";
+		client_name = "w83781d";
 	} else if (kind == w83782d) {
-		client_name = "W83782D chip";
+		client_name = "w83782d";
 	} else if (kind == w83783s) {
-		client_name = "W83783S chip";
+		client_name = "w83783s";
 	} else if (kind == w83627hf) {
 		if (val1 == 0x90)
-			client_name = "W83627THF chip";
+			client_name = "w83627thf";
 		else
-			client_name = "W83627HF chip";
+			client_name = "w83627hf";
 	} else if (kind == as99127f) {
-		client_name = "AS99127F chip";
+		client_name = "as99127f";
 	} else if (kind == w83697hf) {
-		client_name = "W83697HF chip";
+		client_name = "w83697hf";
 	} else {
 		dev_err(&new_client->dev, "Internal error: unknown "
 						"kind (%d)?!?", kind);

[-- Attachment #3: lm_sensors-2.8.0-sysfs.patch --]
[-- Type: text/x-diff, Size: 7303 bytes --]

--- lm_sensors-2.8.0/lib/proc.c.sysfs	2003-06-09 01:33:38.000000000 +0400
+++ lm_sensors-2.8.0/lib/proc.c	2003-08-23 22:23:33.000000000 +0400
@@ -20,6 +20,8 @@
 #include <stddef.h>
 #include <stdio.h>
 #include <string.h>
+#include <dirent.h>
+#include <unistd.h>
 #include <sys/sysctl.h>
 #include "kernel/include/sensors.h"
 #include "data.h"
@@ -59,6 +61,158 @@ static int sensors_get_chip_id(sensors_c
                                        &sensors_proc_bus_max,\
                                        sizeof(struct sensors_bus))
 
+static int using_sysfs;
+static char sysfs_i2c_bus[] = "/sys/bus/i2c/devices";
+static char sysfs_i2c_class[] = "/sys/class/i2c-adapter";
+
+/* This parses sysfs tree for available i2c chips */
+static int sensors_parse_sysfs_chips(void)
+{
+  DIR *dir;
+  struct dirent *dentry;
+  char path[256], buf[256];
+  FILE *f;
+  int len;
+  sensors_proc_chips_entry entry;
+
+  if ((dir = opendir(sysfs_i2c_bus)) == NULL)
+    return -SENSORS_ERR_PROC;
+
+  using_sysfs = 1;
+
+  while ((dentry = readdir(dir)) != NULL) {
+    if (dentry->d_name[0] == '.' || dentry->d_ino <= 0)
+      continue;
+    snprintf(path, sizeof(path), "%s/%s/name", sysfs_i2c_bus, dentry->d_name);
+    if ((f = fopen(path, "r")) == NULL)
+      continue;
+    buf[0] = '\0';
+    fgets(buf, sizeof(buf), f);
+    fclose(f);
+    len = strlen(buf) - 1;
+    if (buf[len] == '\n')
+      buf[len] = '\0';
+    if (strchr(buf, ' '))
+      continue; /* e.g. "as99127f subclient" */
+    sscanf(dentry->d_name, "%d-%x", &entry.name.bus, &entry.name.addr);
+    entry.name.prefix = strdup(buf);
+
+    add_proc_chips(&entry);
+  }
+  closedir(dir);
+  return 0;
+}
+
+static int sensors_parse_sysfs_class(void)
+{
+  DIR *dir;
+  struct dirent *dentry;
+  char path[256], buf[256];
+  FILE *f;
+  int len;
+  sensors_bus entry;
+
+  if ((dir = opendir(sysfs_i2c_class)) == NULL)
+    return -SENSORS_ERR_PROC;
+
+  while ((dentry = readdir(dir)) != NULL) {
+    if (dentry->d_name[0] == '.' || dentry->d_ino <= 0)
+      continue;
+
+    sscanf(dentry->d_name, "i2c-%d", &entry.number);
+    entry.algorithm = strdup("Not available via sysfs");
+    sprintf(path, "%s/%s/device/name", sysfs_i2c_class, dentry->d_name);
+    if ((f = fopen(path, "r")) == NULL) {
+      entry.adapter = strdup("UNKNOWN");
+      continue;
+    }
+    fgets(buf, sizeof(buf), f);
+    fclose(f);
+    len = strlen(buf) - 1;
+    if (buf[len] == '\n')
+      buf[len] = '\0';
+    entry.adapter = strdup(buf);
+    add_bus(&entry);
+  }
+
+  return 0;
+}
+  
+static int ends_with(const char *str, const char *suffix)
+{
+  int len = strlen(str);
+  int slen = strlen(suffix);
+
+  if (len <= slen)
+    return 0;
+
+  return strcmp(str + len - slen, suffix) == 0;
+}
+
+static void
+sensors_sysfs_convert_feature_name(const char *name, char *buf)
+{
+  char part1[256], part2[256];
+  int n;
+
+  if (sscanf(name, "temp%d", &n) == 1) {
+    if (ends_with(name, "_hyst"))
+      sprintf(buf, "temp_min%d", n);
+    else if (ends_with(name, "_over"))
+      sprintf(buf, "temp_max%d", n);
+    else
+      sprintf(buf, "temp_input%d", n);
+  } else if (sscanf(name, "%[^0-9]%d_%s", part1, &n, part2) == 3)
+    sprintf(buf, "%s_%s%d", part1, part2, n);
+  else if (sscanf(name, "%[^0-9]%d", part1, &n) == 2)
+    sprintf(buf, "%s_input%d", part1, n);
+  else if (strcmp(name, "beeps") == 0)
+    strcpy(buf, "beep_mask");
+  else
+    strcpy(buf, name);
+}
+
+/* This reads a feature /proc file */
+static int sensors_read_sysfs(sensors_chip_name name,
+  const sensors_chip_feature *the_feature, double *value)
+{
+  char path[256], buf[256];
+  FILE *f;
+  long l;
+  
+  sensors_sysfs_convert_feature_name(the_feature->name, buf);
+  snprintf(path, sizeof(path), "%s/%d-%04x/%s", sysfs_i2c_bus, name.bus, name.addr, buf);
+  if ((f = fopen(path, "r")) == NULL)
+    return -SENSORS_ERR_PROC;
+  fscanf(f, "%ld", &l);
+  fclose(f);
+  *value = l;
+  if (strncmp(buf, "in_", strlen("in_")) == 0
+      || strncmp(buf, "temp_", strlen("temp_")) == 0)
+    *value /= 10.0;
+  return 0;
+}
+  
+static int sensors_write_sysfs(sensors_chip_name name,
+  const sensors_chip_feature *the_feature, double value)
+{
+  char path[256], buf[256];
+  FILE *f;
+  long l;
+  
+  sensors_sysfs_convert_feature_name(the_feature->name, buf);
+  snprintf(path, sizeof(path), "%s/%d-%04x/%s", sysfs_i2c_bus, name.bus, name.addr, buf);
+  if (strncmp(buf, "in_", strlen("in_")) == 0
+      || strncmp(buf, "temp_", strlen("temp_")) == 0)
+    value *= 10.0;
+  l = (long) value;
+  if ((f = fopen(path, "w")) == NULL)
+    return -SENSORS_ERR_PROC;
+  fprintf(f, "%ld", l);
+  fclose(f);
+
+  return 0;
+}
 /* This reads /proc/sys/dev/sensors/chips into memory */
 int sensors_read_proc_chips(void)
 {
@@ -69,7 +223,7 @@ int sensors_read_proc_chips(void)
   int res,lineno;
 
   if (sysctl(name, 3, bufptr, &buflen, NULL, 0))
-    return -SENSORS_ERR_PROC;
+    return sensors_parse_sysfs_chips();
 
   lineno = 1;
   while (buflen >= sizeof(struct i2c_chips_data)) {
@@ -96,6 +250,9 @@ int sensors_read_proc_bus(void)
   sensors_bus entry;
   int lineno;
 
+  if (using_sysfs)
+    return sensors_parse_sysfs_class();
+
   f = fopen("/proc/bus/i2c","r");
   if (!f)
     return -SENSORS_ERR_PROC;
@@ -158,10 +315,17 @@ int sensors_read_proc(sensors_chip_name 
     return sysctl_name[2];
   if (! (the_feature = sensors_lookup_feature_nr(name.prefix,feature)))
     return -SENSORS_ERR_NO_ENTRY;
-  sysctl_name[3] = the_feature->sysctl;
-  if (sysctl(sysctl_name, 4, buf, &buflen, NULL, 0))
-    return -SENSORS_ERR_PROC;
-  *value = *((long *) (buf + the_feature->offset));
+
+  if (using_sysfs) {
+    int ret = sensors_read_sysfs(name, the_feature, value);
+    if (ret)
+      return ret;
+  } else {
+    sysctl_name[3] = the_feature->sysctl;
+    if (sysctl(sysctl_name, 4, buf, &buflen, NULL, 0))
+      return -SENSORS_ERR_PROC;
+    *value = *((long *) (buf + the_feature->offset));
+  }
   for (mag = the_feature->scaling; mag > 0; mag --)
     *value /= 10.0;
   for (; mag < 0; mag ++)
@@ -180,17 +344,22 @@ int sensors_write_proc(sensors_chip_name
     return sysctl_name[2];
   if (! (the_feature = sensors_lookup_feature_nr(name.prefix,feature)))
     return -SENSORS_ERR_NO_ENTRY;
-  sysctl_name[3] = the_feature->sysctl;
-  if (sysctl(sysctl_name, 4, buf, &buflen, NULL, 0))
-    return -SENSORS_ERR_PROC;
-  if (sysctl_name[0] != CTL_DEV) { sysctl_name[0] = CTL_DEV ; }
   for (mag = the_feature->scaling; mag > 0; mag --)
     value *= 10.0;
   for (; mag < 0; mag ++)
     value /= 10.0;
-  * ((long *) (buf + the_feature->offset)) = (long) value;
-  buflen = the_feature->offset + sizeof(long);
-  if (sysctl(sysctl_name, 4, NULL, 0, buf, buflen))
-    return -SENSORS_ERR_PROC;
-  return 0;
+
+  if (using_sysfs) {
+    return sensors_write_sysfs(name, the_feature, value);
+  } else {
+    sysctl_name[3] = the_feature->sysctl;
+    if (sysctl(sysctl_name, 4, buf, &buflen, NULL, 0))
+      return -SENSORS_ERR_PROC;
+    if (sysctl_name[0] != CTL_DEV) { sysctl_name[0] = CTL_DEV ; }
+    * ((long *) (buf + the_feature->offset)) = (long) value;
+    buflen = the_feature->offset + sizeof(long);
+    if (sysctl(sysctl_name, 4, NULL, 0, buf, buflen))
+      return -SENSORS_ERR_PROC;
+    return 0;
+  }
 }

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: 2.6 - sysfs sensor nameing inconsistency
  2003-08-31 16:25               ` Andrey Borzenkov
@ 2003-09-22 22:29                 ` Greg KH
  2003-11-02 18:50                   ` Andrey Borzenkov
  0 siblings, 1 reply; 15+ messages in thread
From: Greg KH @ 2003-09-22 22:29 UTC (permalink / raw)
  To: Andrey Borzenkov; +Cc: linux-kernel, sensors

On Sun, Aug 31, 2003 at 08:25:41PM +0400, Andrey Borzenkov wrote:
> On Tuesday 19 August 2003 23:45, Greg KH wrote:
> > That's what you are going to have to set the name file to in the
> > i2c_client structure, much like your patch did.  Then look at the
> > different name files in each device directory to see what kind of device
> > it is (chip, subclient, etc.)
> >
> 
> OK attached patch sets all names to just chip name for chips themselves and 
> "chipname subclient" when subclient ios registered.

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


> > > 3. libsensors asks for hysteresis value. This one does not exist in sysfs
> > > (so all temp readings fail). Is it emulated by kernel or read off chip?
> >
> > The kernel is exporting all of the info that it knows about through
> > sysfs.
> 
> it just arbitrarily (re-)names them. "min" is not hysteresis; name is badly 
> chosen.

Do you have a proposed change to the current
Documentation/i2c/sysfs-interface document?  If you can think of better
names that make more sense, I'd be glad to change things to make it
easier.

> [...]
> > > 4. I do not have the slightest idea how ISA adapters look like in sysfs
> > > and where they are located. Anyone can give me example?
> >
> > They show up on the legacy bus:
> >
> > $ tree /sys/class/i2c-adapter/i2c-1/
> > /sys/class/i2c-adapter/i2c-1/
> >
> > |-- device -> ../../../devices/legacy/i2c-1
> >
> 
> This does not help much. Libsensors expects as adapter identification either 
> "i2c-N" or "isa". If I set it to "isa" I do not have any way to determine 
> sysfs path except by rescanning /sys/class/i2c-adapter every time. Having 
> /sys/class/i2-adapter/isa/... would be better, apparently it is assumed that 
> only one such adapter can exist.

No, we internally do not differentiate between isa and non-isa adapters,
so why should we force that on the user?  They work the same as far as
users notice, and now we are consistant in our naming.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: 2.6 - sysfs sensor nameing inconsistency
  2003-09-22 22:29                 ` Greg KH
@ 2003-11-02 18:50                   ` Andrey Borzenkov
  0 siblings, 0 replies; 15+ messages in thread
From: Andrey Borzenkov @ 2003-11-02 18:50 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, sensors

On Tuesday 23 September 2003 02:29, Greg KH wrote:
>
> Thanks, I've applied this and will send it off to Linus in a bit.
>

thank you.

> > it just arbitrarily (re-)names them. "min" is not hysteresis; name is
> > badly chosen.
>
> Do you have a proposed change to the current
> Documentation/i2c/sysfs-interface document?  If you can think of better
> names that make more sense, I'd be glad to change things to make it
> easier.
>

it is probably too late now to change sysfs names when some programs already 
use it. I'll check sysfs-interface, thank you for pointer.

> > > > 4. I do not have the slightest idea how ISA adapters look like in
> > > > sysfs and where they are located. Anyone can give me example?
> > >
> > > They show up on the legacy bus:
> > >
> > > $ tree /sys/class/i2c-adapter/i2c-1/
> > > /sys/class/i2c-adapter/i2c-1/
> > >
> > > |-- device -> ../../../devices/legacy/i2c-1
> >
> > This does not help much. Libsensors expects as adapter identification
> > either "i2c-N" or "isa". If I set it to "isa" I do not have any way to
> > determine sysfs path except by rescanning /sys/class/i2c-adapter every
> > time. Having /sys/class/i2-adapter/isa/... would be better, apparently it
> > is assumed that only one such adapter can exist.
>
> No, we internally do not differentiate between isa and non-isa adapters,
> so why should we force that on the user?  They work the same as far as
> users notice, and now we are consistant in our naming.
>

Well, I just tried to match what users get out of libsensors on 2.4. The 
reason was compatibility with /etc/sensors.conf where "isa" can possibly be 
used as part of chip name. But I'd like that someone from sensors developers 
comment on this.

thank you for your comments.

-andrey


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: 2.6 - sysfs sensor nameing inconsistency
@ 2003-07-27  6:23 Andrey Borzenkov
  0 siblings, 0 replies; 15+ messages in thread
From: Andrey Borzenkov @ 2003-07-27  6:23 UTC (permalink / raw)
  To: linux-kernel; +Cc: Margit Schubert-While

>> Attached is patch against 2.6.0-test1 that adds type_name to all in-tree
>> sensors; it sets it to the same values as corr. 2.4 senors and (in one 
case)
>> changes client name to match that of 2.4.
>
> Well, it certainly doesn't with the lm85.c  :-)
> Hint - names are in lib/chips.h in sensors package :-)

It was my fault I should not start changing names, sorry. Sometimes this 
happens.

If this patch is accepted it is OK though, we may change other names to be 
more user-friendly as well.

-andrey

Please Cc me I am not on lkml



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: 2.6 - sysfs sensor nameing inconsistency
@ 2003-07-27  4:42 Margit Schubert-While
  0 siblings, 0 replies; 15+ messages in thread
From: Margit Schubert-While @ 2003-07-27  4:42 UTC (permalink / raw)
  To: linux-kernel

 > Attached is patch against 2.6.0-test1 that adds type_name to all in-tree
 > sensors; it sets it to the same values as corr. 2.4 senors and (in one case)
 > changes client name to match that of 2.4.

Well, it certainly doesn't with the lm85.c  :-)
Hint - names are in lib/chips.h in sensors package :-)

Although, I will be working over lm85.c in the next week or so.

 > +static const char *type_name = "";
 > +static ssize_t show_type_name(struct device *dev, char *buf)
 > +{
 > + return sprintf(buf, "%s\n", type_name);
 > +}
 > +static DEVICE_ATTR(type_name, S_IRUGO, show_type_name, NULL);

 > - const char *type_name = "";

 > + device_create_file(&new_client->dev, &dev_attr_type_name);



Margit 


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2003-11-02 18:55 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-07-15 18:14 2.6 - sysfs sensor nameing inconsistency Andrey Borzenkov
2003-07-15 20:18 ` Greg KH
2003-07-26 18:00   ` Andrey Borzenkov
2003-08-15 20:51     ` Greg KH
2003-08-16 15:38       ` Andrey Borzenkov
2003-08-16 16:50         ` Greg KH
2003-08-18 16:49           ` Andrey Borzenkov
2003-08-18 21:31             ` Greg KH
2003-08-19 19:19           ` Andrey Borzenkov
2003-08-19 19:45             ` Greg KH
2003-08-31 16:25               ` Andrey Borzenkov
2003-09-22 22:29                 ` Greg KH
2003-11-02 18:50                   ` Andrey Borzenkov
2003-07-27  4:42 Margit Schubert-While
2003-07-27  6:23 Andrey Borzenkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).