i2c: Introduce i2c listeners

Hi all,

This patch set demonstrates a new concept which I would like to add to the i2c subsystem. It is named "i2c listeners" and is based on the following structure:

/**
* struct i2c_listener - Be informed of adapter addition and removal
* @class: What kind of i2c device the listener may instantiate
* @attach_adapter: Called after adapter addition (unimplemented)
* @detach_adapter: Called before adapter removal (unimplemented)
* @address_data: The I2C addresses to probe, ignore or force
* @detect: Callback for device detection
* @clients: List of detected clients we created
*
* The @detect callback returns an i2c_board_info structure for device
* creation if the detection was successful, NULL otherwise. *
* The @clients list is handled by i2c-core.
*/

struct i2c_listener {
	unsigned int class;

	int (*attach_adapter)(struct i2c_adapter *);
	int (*detach_adapter)(struct i2c_adapter *);

	const struct i2c_client_address_data *address_data;
	int (*detect)(struct i2c_adapter *, int address, int kind,
		      struct i2c_board_info *);
	struct list_head clients;

	struct list_head list;
};

The goal is to replace legacy i2c drivers. The functional differences between legacy drivers and new-style ones are that legacy drivers get notified when i2c adapters are created or deleted, they create their own devices, and they can probe the hardware before they do so. There are cases where we want to be able to do this (e.g. for hardware monitoring devices on PC motherboards) so we can't just switch to new-style everywhere. Still, legacy drivers are a pain to handle, they cause headaches to developers because their locking model is weird, and they duplicate a lot of code.

By implementing the needed functionality, almost unchanged, outside of the drivers, my hope is to be able to convert all legacy drivers to new-style drivers in a matter of months. Without it, it would take years... or even never happen at all.

There are two distinct parts in i2c_listener: the notification callbacks, and the device detection callback. The notification callbacks are exactly the same as what legacy i2c drivers already have. I did not implement them yet, as I have no immediate need for them, but I suspect I will have to for the V4L/DVB drivers (if not I'll just remove them.) The device detection part is implemented in patch 1/4.

The mechanism behind the device detection callback is as follows:
* When a new i2c_notifier is added, for each existing i2c_adapter, address_data (those format is the same as what i2c_probe() is already using for legacy drivers) is processed by i2c-core. For each relevant address, the detect callback will be called, with a pointer to an empty struct i2c_board_info address as the last parameter. The detect callback will attempt to detect a supported device at the given address, and if successful, it will fill the struct i2c_board_info with the parameters required to instantiate a new-style i2c device.
* When a new i2c_adapter is added, for each existing i2c_notifier, the same happens.
* When it gets a filled struct i2c_board_info from a detect callback, i2c-core will instantiate it. If a new-style driver exists for the device, it will be able to bind with the device.
* We keep track of the devices created that way, in a per-listener list.
* When an i2c_listener is removed, all devices that originate from it are destroyed.
* When an i2c_adapter is removed, all devices on it that were created by an i2c_listener are destroyed.

So, drivers which currently implement both a new-style i2c_driver and a legacy i2c_driver (or drivers which would be converted that way soon) can instead implement a new-style i2c_driver and an i2c_listener. There are two major advantages in doing so:
* All i2c drivers become new-style drivers. We get rid of legacy drivers altogether, allowing for long awaited cleanups in i2c-core.
* The code needed in each driver to implement an i2c_listener is way smaller than the code needed to implement a legacy driver, even when we were sharing as much code as possible between the new-style driver and the legacy driver. This is very visible in patches 3/4 and 4/4, which remove ~35 lines of code per driver.

I would appreciate feedback on both the concept and the implementation. David, I am particularly interested in your feedback, of course.

Patch 1/4 implements the mechanism, then patches 2/4, 3/4 and 4/4 demonstrate its use in 3 hardware monitoring drivers (lm90, f75375s and lm75, respectively.) The patches go on top of 2.6.26-rc4 with some additional dependencies listed in each patch. Testers welcome!

Jean Delvare

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

Add an i2c listener mechanism to the i2c-subsystem. This mechanism lets device drivers be notified of addition and removal of adapters, and gives them a chance to detect devices they would support and to ask i2c-core to instantiate them.

---
This patch depends on:
http://khali.linux-fr.org/devel/linux-2.6/jdelvare-i2c/i2c-use-class_for_each_device.patch

Implementation note: i2c_detect and i2c_detect_address are basically clones of i2c_probe and i2c_probe_address but operating on a callback with a slightly different prototype. This duplicated code accounts for a large part of the patch, but once legacy drivers are gone, i2c_probe and i2c_probe_address will be removed, so thus is no long-term duplication.

 drivers/i2c/i2c-core.c |  235 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c.h    |   34 ++++++
 2 files changed, 269 insertions(+)

@@ -43,6 +43,7 @@ struct i2c_adapter;
 struct i2c_client;
 struct i2c_driver;
 union i2c_smbus_data;
+struct i2c_board_info;
 
 /*
  * The master routines are the ones normally used to transmit data to devices

@@ -92,6 +93,34 @@ extern s32 i2c_smbus_write_i2c_block_dat
 					  u8 command, u8 length,
 					  const u8 *values);
 
+/**
+ * struct i2c_listener - Be informed of adapter addition and removal
+ * @class: What kind of i2c device the listener may instantiate
+ * @attach_adapter: Called after adapter addition (unimplemented)
+ * @detach_adapter: Called before adapter removal (unimplemented)
+ * @address_data: The I2C addresses to probe, ignore or force
+ * @detect: Callback for device detection
+ * @clients: List of detected clients we created
+ *
+ * The @detect callback returns an i2c_board_info structure for device
+ * creation if the detection was successful, NULL otherwise.
+ *
+ * The @clients list is handled by i2c-core.
+ */
+struct i2c_listener {
+	unsigned int class;
+
+	int (*attach_adapter)(struct i2c_adapter *);
+	int (*detach_adapter)(struct i2c_adapter *);
+
+	const struct i2c_client_address_data *address_data;
+	int (*detect)(struct i2c_adapter *, int address, int kind,
+		      struct i2c_board_info *);
+	struct list_head clients;
+
+	struct list_head list;
+};
+
 /*
  * A driver is capable of handling one or more physical devices present on
  * I2C adapters. This information is used to inform the driver of adapter

@@ -155,6 +184,7 @@ struct i2c_driver {
  * @dev: Driver model device node for the slave.
  * @irq: indicates the IRQ generated by this device (if any)
  * @list: list of active/busy clients (DEPRECATED)
+ * @detected: member of i2c_listener.clients
  * @released: used to synchronize client releases & detaches and references
  *
  * An i2c_client identifies a single device (i.e. chip) connected to an

@@ -172,6 +202,7 @@ struct i2c_client {
 	struct device dev;		/* the device structure		*/
 	int irq;			/* irq issued by device (or -1) */
 	struct list_head list;		/* DEPRECATED */
+	struct list_head detected;
 	struct completion released;
 };
 #define to_i2c_client(d) container_of(d, struct i2c_client, dev)

@@ -382,6 +413,9 @@ extern int i2c_add_adapter(struct i2c_ad
 extern int i2c_del_adapter(struct i2c_adapter *);
 extern int i2c_add_numbered_adapter(struct i2c_adapter *);
 
+extern void i2c_add_listener(struct i2c_listener *);
+extern void i2c_del_listener(struct i2c_listener *);
+
 extern int i2c_register_driver(struct module *, struct i2c_driver *);
 extern void i2c_del_driver(struct i2c_driver *);
 

Jean Delvare

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

Convert the lm90 hardware monitoring driver to a new-style i2c driver, including an i2c_listener for automatic device detection and creation.

---
 lm90.c |  108 +++++++++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 65 insertions(+), 43 deletions(-)

@@ -187,23 +187,43 @@ I2C_CLIENT_INSMOD_7(lm90, adm1032, lm99,
  * Functions declaration
  */
 
-static int lm90_attach_adapter(struct i2c_adapter *adapter);
 static int lm90_detect(struct i2c_adapter *adapter, int address,
-	int kind);
+		       int kind, struct i2c_board_info *info);
+static int lm90_probe(struct i2c_client *client,
+		      const struct i2c_device_id *id);
 static void lm90_init_client(struct i2c_client *client);
-static int lm90_detach_client(struct i2c_client *client);
+static int lm90_remove(struct i2c_client *client);
 static struct lm90_data *lm90_update_device(struct device *dev);
 
 /*
  * Driver data (common to all clients)
  */
 
+static struct i2c_device_id lm90_id[] = {
+	{ "lm90", lm90 },
+	{ "adm1032", adm1032 },
+	{ "lm99", lm99 },
+	{ "lm86", lm86 },
+	{ "max6657", max6657 },
+	{ "adt7461", adt7461 },
+	{ "max6680", max6680 },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, lm90_id);
+
 static struct i2c_driver lm90_driver = {
 	.driver = {
 		.name	= "lm90",
 	},
-	.attach_adapter	= lm90_attach_adapter,
-	.detach_client	= lm90_detach_client,
+	.probe		= lm90_probe,
+	.remove		= lm90_remove,
+	.id_table	= lm90_id,
+};
+
+static struct i2c_listener lm90_listener = {
+	.class		= I2C_CLASS_HWMON,
+	.address_data	= &addr_data,
+	.detect		= lm90_detect,
 };
 
 /*

@@ -211,7 +231,6 @@ static struct i2c_driver lm90_driver = {
  */
 
 struct lm90_data {
-	struct i2c_client client;
 	struct device *hwmon_dev;
 	struct mutex update_lock;
 	char valid; /* zero until following fields are valid */

@@ -477,40 +496,26 @@ static int lm90_read_reg(struct i2c_clie
 	return 0;
 }
 
-static int lm90_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, lm90_detect);
-}
-
-/*
- * The following function does more than just detection. If detection
- * succeeds, it also registers the new chip.
- */
-static int lm90_detect(struct i2c_adapter *adapter, int address, int kind)
+/* Returns -ENODEV if no supported device is detected */
+static int lm90_detect(struct i2c_adapter *adapter, int address, int kind,
+		       struct i2c_board_info *info)
 {
 	struct i2c_client *new_client;
-	struct lm90_data *data;
-	int err = 0;
+	int err = -ENODEV;
 	const char *name = "";
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
 		goto exit;
 
-	if (!(data = kzalloc(sizeof(struct lm90_data), GFP_KERNEL))) {
+	new_client = kzalloc(sizeof(struct i2c_client), GFP_KERNEL);
+	if (!new_client) {
 		err = -ENOMEM;
 		goto exit;
 	}
 
-	/* The common I2C client data is placed right before the
-	   LM90-specific data. */
-	new_client = &data->client;
-	i2c_set_clientdata(new_client, data);
+	/* Enough to use smbus calls */
 	new_client->addr = address;
 	new_client->adapter = adapter;
-	new_client->driver = &lm90_driver;
-	new_client->flags = 0;
 
 	/*
 	 * Now we do the remaining detection. A negative kind means that

@@ -613,7 +618,10 @@ static int lm90_detect(struct i2c_adapte
 			goto exit_free;
 		}
 	}
+	err = 0;	/* detection OK */
 
+	/* Fill the i2c board info */
+	info->addr = address;
 	if (kind == lm90) {
 		name = "lm90";
 	} else if (kind == adm1032) {

@@ -621,7 +629,7 @@ static int lm90_detect(struct i2c_adapte
 		/* The ADM1032 supports PEC, but only if combined
 		   transactions are not used. */
 		if (i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE))
-			new_client->flags |= I2C_CLIENT_PEC;
+			info->flags |= I2C_CLIENT_PEC;
 	} else if (kind == lm99) {
 		name = "lm99";
 	} else if (kind == lm86) {

@@ -633,23 +641,41 @@ static int lm90_detect(struct i2c_adapte
 	} else if (kind == adt7461) {
 		name = "adt7461";
 	}
+	strlcpy(info->type, name, I2C_NAME_SIZE);
 
-	/* We can fill in the remaining client fields */
-	strlcpy(new_client->name, name, I2C_NAME_SIZE);
-	data->valid = 0;
-	data->kind = kind;
+exit_free:
+	kfree(new_client);
+exit:
+	return err;
+}
+
+static int lm90_probe(struct i2c_client *new_client,
+		      const struct i2c_device_id *id)
+{
+	struct i2c_adapter *adapter = to_i2c_adapter(new_client->dev.parent);
+	struct lm90_data *data;
+	int err;
+
+	if (!(data = kzalloc(sizeof(struct lm90_data), GFP_KERNEL))) {
+		err = -ENOMEM;
+		goto exit;
+	}
+	i2c_set_clientdata(new_client, data);
 	mutex_init(&data->update_lock);
 
-	/* Tell the I2C layer a new client has arrived */
-	if ((err = i2c_attach_client(new_client)))
-		goto exit_free;
+	/* Set the device type */
+	data->kind = id->driver_data;
+	if (data->kind == adm1032) {
+		if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE))
+			new_client->flags &= ~I2C_CLIENT_PEC;
+	}
 
 	/* Initialize the LM90 chip */
 	lm90_init_client(new_client);
 
 	/* Register sysfs hooks */
 	if ((err = sysfs_create_group(&new_client->dev.kobj, &lm90_group)))
-		goto exit_detach;
+		goto exit_free;
 	if (new_client->flags & I2C_CLIENT_PEC) {
 		if ((err = device_create_file(&new_client->dev,
 					      &dev_attr_pec)))

@@ -672,8 +698,6 @@ static int lm90_detect(struct i2c_adapte
 exit_remove_files:
 	sysfs_remove_group(&new_client->dev.kobj, &lm90_group);
 	device_remove_file(&new_client->dev, &dev_attr_pec);
-exit_detach:
-	i2c_detach_client(new_client);
 exit_free:
 	kfree(data);
 exit:

@@ -710,10 +734,9 @@ static void lm90_init_client(struct i2c_
 		i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1, config);
 }
 
-static int lm90_detach_client(struct i2c_client *client)
+static int lm90_remove(struct i2c_client *client)
 {
 	struct lm90_data *data = i2c_get_clientdata(client);
-	int err;
 
 	hwmon_device_unregister(data->hwmon_dev);
 	sysfs_remove_group(&client->dev.kobj, &lm90_group);

@@ -722,9 +745,6 @@ static int lm90_detach_client(struct i2c
 		device_remove_file(&client->dev,
 				   &sensor_dev_attr_temp2_offset.dev_attr);
 
-	if ((err = i2c_detach_client(client)))
-		return err;
-
 	kfree(data);
 	return 0;
 }

@@ -794,12 +814,14 @@ static struct lm90_data *lm90_update_dev
 
 static int __init sensors_lm90_init(void)
 {
+	i2c_add_listener(&lm90_listener);
 	return i2c_add_driver(&lm90_driver);
 }
 
 static void __exit sensors_lm90_exit(void)
 {
 	i2c_del_driver(&lm90_driver);
+	i2c_del_listener(&lm90_listener);
 }
 
 MODULE_AUTHOR("Jean Delvare <[email protected]>");

Jean Delvare

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

Replace the legacy f75375s i2c_driver by an i2c_listener.

---
Riku, any chance you could test this one? Thanks.

 drivers/hwmon/f75375s.c |   67 +++++++++--------------------------------------
 1 file changed, 14 insertions(+), 53 deletions(-)

@@ -114,19 +114,16 @@ struct f75375_data {
 	s8 temp_max_hyst[2];
 };
 
-static int f75375_attach_adapter(struct i2c_adapter *adapter);
-static int f75375_detect(struct i2c_adapter *adapter, int address, int kind);
-static int f75375_detach_client(struct i2c_client *client);
+static int f75375_detect(struct i2c_adapter *adapter, int address, int kind,
+			 struct i2c_board_info *info);
 static int f75375_probe(struct i2c_client *client,
 			const struct i2c_device_id *id);
 static int f75375_remove(struct i2c_client *client);
 
-static struct i2c_driver f75375_legacy_driver = {
-	.driver = {
-		.name = "f75375_legacy",
-	},
-	.attach_adapter = f75375_attach_adapter,
-	.detach_client = f75375_detach_client,
+static struct i2c_listener f75375_listener = {
+	.class		= I2C_CLASS_HWMON,
+	.address_data	= &addr_data,
+	.detect		= f75375_detect,
 };
 
 static const struct i2c_device_id f75375_id[] = {

@@ -607,22 +604,6 @@ static const struct attribute_group f753
 	.attrs = f75375_attributes,
 };
 
-static int f75375_detach_client(struct i2c_client *client)
-{
-	int err;
-
-	f75375_remove(client);
-	err = i2c_detach_client(client);
-	if (err) {
-		dev_err(&client->dev,
-			"Client deregistration failed, "
-			"client not detached.\n");
-		return err;
-	}
-	kfree(client);
-	return 0;
-}
-
 static void f75375_init(struct i2c_client *client, struct f75375_data *data,
 		struct f75375s_platform_data *f75375s_pdata)
 {

@@ -700,21 +681,14 @@ static int f75375_remove(struct i2c_clie
 	return 0;
 }
 
-static int f75375_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, f75375_detect);
-}
-
 /* This function is called by i2c_probe */
-static int f75375_detect(struct i2c_adapter *adapter, int address, int kind)
+static int f75375_detect(struct i2c_adapter *adapter, int address, int kind,
+			 struct i2c_board_info *info)
 {
 	struct i2c_client *client;
 	u8 version = 0;
-	int err = 0;
+	int err = -ENODEV;
 	const char *name = "";
-	struct i2c_device_id id;
 
 	if (!(client = kzalloc(sizeof(*client), GFP_KERNEL))) {
 		err = -ENOMEM;

@@ -722,7 +696,6 @@ static int f75375_detect(struct i2c_adap
 	}
 	client->addr = address;
 	client->adapter = adapter;
-	client->driver = &f75375_legacy_driver;
 
 	if (kind < 0) {
 		u16 vendid = f75375_read16(client, F75375_REG_VENDOR);

@@ -739,6 +712,7 @@ static int f75375_detect(struct i2c_adap
 			goto exit_free;
 		}
 	}
+	err = 0;	/* detection OK */
 
 	if (kind == f75375) {
 		name = "f75375";

@@ -746,20 +720,9 @@ static int f75375_detect(struct i2c_adap
 		name = "f75373";
 	}
 	dev_info(&adapter->dev, "found %s version: %02X\n", name, version);
-	strlcpy(client->name, name, I2C_NAME_SIZE);
-
-	if ((err = i2c_attach_client(client)))
-		goto exit_free;
-
-	strlcpy(id.name, name, I2C_NAME_SIZE);
-	id.driver_data = kind;
-	if ((err = f75375_probe(client, &id)) < 0)
-		goto exit_detach;
+	strlcpy(info->type, name, I2C_NAME_SIZE);
+	info->addr = address;
 
-	return 0;
-
-exit_detach:
-	i2c_detach_client(client);
 exit_free:
 	kfree(client);
 exit:

@@ -773,16 +736,14 @@ static int __init sensors_f75375_init(vo
 	if (status)
 		return status;
 
-	status = i2c_add_driver(&f75375_legacy_driver);
-	if (status)
-		i2c_del_driver(&f75375_driver);
+	i2c_add_listener(&f75375_listener);
 
 	return status;
 }
 
 static void __exit sensors_f75375_exit(void)
 {
-	i2c_del_driver(&f75375_legacy_driver);
+	i2c_del_listener(&f75375_listener);
 	i2c_del_driver(&f75375_driver);
 }
 

Jean Delvare

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

I can test that the new style driver still works after these tests, but I don't have testing if the driver still works in cases were f75375_legacy driver used to work (which I guess is what you are interested in) is tricker. I do however like the concept.

I have already tested the latter, using i2c-stub + a dump of a F75375S chip. So if you can test the former (new-style) we cover the whole set of possibilities and that's alright :)

Thanks,

Jean Delvare

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

I failed to get the patchset against Linux 2.6.26-rc5, and didn't have time to fight:

../p2/\[PATCH_1_4\]_i2c__Introduce_i2c_listeners.txt
patching file include/linux/i2c.h
Hunk #2 succeeded at 94 (offset 1 line).
Hunk #3 succeeded at 185 (offset 1 line).
Hunk #4 succeeded at 203 (offset 1 line).
patching file drivers/i2c/i2c-core.c
Hunk #1 succeeded at 43 (offset 1 line).
Hunk #2 succeeded at 432 (offset 6 lines).
Hunk #3 succeeded at 463 (offset 6 lines).
Hunk #4 FAILED at 598.
Hunk #5 succeeded at 612 (offset 7 lines).
Hunk #6 succeeded at 796 (offset 12 lines).
Hunk #7 succeeded at 1240 (offset -35 lines).
1 out of 7 hunks FAILED -- saving rejects to file drivers/i2c/i2c-core.c.rej

However, none of the functions you touched are used in F75375S driver with new-style loading path, so it would seem fine for me.

_______________________________________________
i2c mailing list [email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

Hi Riku,

This patch depends on:
http://khali.linux-fr.org/devel/linux-2.6/jdelvare-i2c/i2c-use-class_for_each_device.patch

Did you apply that patch first?

The part that needs testing is in i2c-core, not in the f75375s driver itself.

That being said, I am about to post a second version of the patch set, you probably want to wait for it before you do your tests.

Jean Delvare

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

Replace the legacy lm75 i2c_driver by an i2c_listener.

---
This goes on top of David Brownell's patches:
http://lists.lm-sensors.org/pipermail/lm-sensors/2008-April/022931.html
http://lists.lm-sensors.org/pipermail/lm-sensors/2008-May/023040.html
http://lists.lm-sensors.org/pipermail/lm-sensors/2008-May/023041.html

Also available from:
http://jdelvare.pck.nerim.net/sensors/lm75/

David, any chance you could test this one? Thanks.

 drivers/hwmon/lm75.c |   64 ++++++++++----------------------------------------
 1 file changed, 14 insertions(+), 50 deletions(-)

@@ -54,11 +54,11 @@ enum lm75_type {		/* keep sorted in alph
 	tmp75,
 };
 
-/* Addresses scanned by legacy style driver binding */
+/* Addresses scanned by listener */
 static const unsigned short normal_i2c[] = { 0x48, 0x49, 0x4a, 0x4b, 0x4c,
 					0x4d, 0x4e, 0x4f, I2C_CLIENT_END };
 
-/* Insmod parameters (only for legacy style driver binding) */
+/* Insmod parameters (for listener) */
 I2C_CLIENT_INSMOD_1(lm75);
 
 

@@ -247,16 +247,12 @@ static struct i2c_driver lm75_driver = {
 
 /*-----------------------------------------------------------------------*/
 
-/* "Legacy" I2C driver binding */
-
-static struct i2c_driver lm75_legacy_driver;
-
-/* This function is called by i2c_probe */
-static int lm75_detect(struct i2c_adapter *adapter, int address, int kind)
+static int lm75_detect(struct i2c_adapter *adapter, int address, int kind,
+		       struct i2c_board_info *info)
 {
 	int i;
 	struct i2c_client *new_client;
-	int err = 0;
+	int err = -ENODEV;
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
 				     I2C_FUNC_SMBUS_WORD_DATA))

@@ -273,8 +269,6 @@ static int lm75_detect(struct i2c_adapte
 
 	new_client->addr = address;
 	new_client->adapter = adapter;
-	new_client->driver = &lm75_legacy_driver;
-	new_client->flags = 0;
 
 	/* Now, we do the remaining detection. There is no identification-
 	   dedicated register so we have to rely on several tricks:

@@ -313,52 +307,24 @@ static int lm75_detect(struct i2c_adapte
 			 || i2c_smbus_read_word_data(new_client, i + 3) != os)
 				goto exit_free;
 	}
+	err = 0;	/* detection OK */
 
 	/* NOTE: we treat "force=..." and "force_lm75=..." the same.
 	 * Only new-style driver binding distinguishes chip types.
 	 */
-	strlcpy(new_client->name, "lm75", I2C_NAME_SIZE);
-
-	/* Tell the I2C layer a new client has arrived */
-	err = i2c_attach_client(new_client);
-	if (err)
-		goto exit_free;
-
-	err = lm75_probe(new_client, NULL);
-	if (err < 0)
-		goto exit_detach;
+	strlcpy(info->type, "lm75", I2C_NAME_SIZE);
+	info->addr = address;
 
-	return 0;
-
-exit_detach:
-	i2c_detach_client(new_client);
 exit_free:
 	kfree(new_client);
 exit:
 	return err;
 }
 
-static int lm75_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, lm75_detect);
-}
-
-static int lm75_detach_client(struct i2c_client *client)
-{
-	lm75_remove(client);
-	i2c_detach_client(client);
-	kfree(client);
-	return 0;
-}
-
-static struct i2c_driver lm75_legacy_driver = {
-	.driver = {
-		.name	= "lm75_legacy",
-	},
-	.attach_adapter	= lm75_attach_adapter,
-	.detach_client	= lm75_detach_client,
+static struct i2c_listener lm75_listener = {
+	.class		= I2C_CLASS_HWMON,
+	.address_data	= &addr_data,
+	.detect		= lm75_detect,
 };
 
 /*-----------------------------------------------------------------------*/

@@ -430,16 +396,14 @@ static int __init sensors_lm75_init(void
 	if (status < 0)
 		return status;
 
-	status = i2c_add_driver(&lm75_legacy_driver);
-	if (status < 0)
-		i2c_del_driver(&lm75_driver);
+	i2c_add_listener(&lm75_listener);
 
 	return status;
 }
 
 static void __exit sensors_lm75_exit(void)
 {
-	i2c_del_driver(&lm75_legacy_driver);
+	i2c_del_listener(&lm75_listener);
 	i2c_del_driver(&lm75_driver);
 }
 

Jean Delvare

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

How does this fit in with the existing driver bus structure in kernel? This sounds a lot like hot plug and buses already implement hot plug events. This isn't exactly hot plug but it may be close enough that the existing bus APIs will work. Note that both devices and entire buses can be hot plugged with existing code.

Jon Smirl
[email protected]

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

Could the existing API be used something like this?

Drivers register with i2c core using standard driver registration. Class and address ranges are exposed in a structure like PCI IDs. I2C can make up it's own ID structure equivalent to the PCI IDs one.

When a new adapter/bus is created i2c core gets the event. Core then scans the registered driver structures looking for class matches. When a match is encountered it repeatedly calls the driver's probe function with the addresses from the range. Driver returns true/false on each probe which triggers creation of the device.

Why do drivers need the bus creation (attach/detach adapter) events?

Jon Smirl
[email protected]

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

No, no, no. I've been repeating it many times already: I2C device addresses are NOT IDs. If you're even thinking of going in this direction, you're plain wrong, sorry. Many devices share the same address, and many devices can use different addresses. The address is really only one attribute of an I2C device, amongst many others. It's not an identifier.

Please also realize that we _do_ have "IDs" for I2C devices by now. As there are no standard numbers as PCI has, we use arbitrary strings (typically the device name). That's what new-style drivers use for device binding since kernel 2.6.26. And you can't have more than one ID format for a given subsystem, and we don't want to break new-style drivers, so thinking about a radically different form of IDs for the i2c subsystem is a waste of time: it's simply not going to happen.

Side question: why would i2c-core want events when adapters are created, given that it does create them itself?

The driver's probe function is, by definition of the device driver model, called on an existing device. So the probe function cannot decide whether the device should be created or not - it's too late at this point.

One thing that we could consider though is merging i2c_listener into i2c_driver. For some reason I had not considered it - but I somehow expect funny locking issues if we do that.

Originally, to probe the adapters in search of devices to support. I seem to remember that some V4L/DVB drivers do more than that in their attach_adapter callback, but I need to take a closer look. And i2c-dev deserves particular attention, as it needs to do bookkeeping of all the adapters that are present. In fact we might have to implement attach/detach adapter just for i2c-dev. More on that after I have read the code again.

Jean Delvare

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

I know these aren't identifiers, I'm proposing that they be ranges of address to scan. The driver model doesn't force these to be IDs, but in most cases they are IDs. To the driver model it's just a pointer to a struct, no interpretation is done of the structure. It's the PCI bus driver that interprets these structs as identifiers. In our case the bus driver is inside i2c core and it interprets the structs as address ranges and then calls the probe function sequentially with the different value.

I shouldn't have compared this to PCI IDs, I was just thinking of it as data that gets processed by i2c core. PCI IDs are an example of data that is processed by the PCI core. Their meaning as ids is supplied by PCI core, not the kernel driver framework.

I used 'event' as a generic term for meaning the driver core was aware that the adapter was created.

I don't believe this is has to be true although usually it is. A driver is passed in information from the bus core on the probe function. It then looks at that information and decides if it wants to bind. Drivers return true/false if they want to bind. If they return false the device structure is destroyed (or reused for the next probe call).

There is nothing in the model that requires an actual device to be present for a probe call to happen. A perfectly valid reason for refusing to bind would be not having hardware present at the address. This is how older ISA drivers are loaded that predate PNP. They search for the hardware in the probe function. That's probably why it is called "probe" but that predates my involvement in Linux.

We are not far apart on this proposal if you combine the class/address_data fields into i2c_driver and then use the existing probe function instead of detect(). The i2c_client struct already has an address field. A driver would then check to see if there was hardware at the i2c_client.address and return false if it is not there. i2c core would just reuse the i2c_client struct with the next address and do another probe.

Operations on an adapter need to be serialized but I thought the adapters already supported that. If they don't they need to be fixed anyway because of the BKL removal.

I don't think believe devices should need attach/detach adapter functions. If they do, there is probably some other way to achieve the same effect without needing the functions. i2c-dev may need access to some i2c core structures to avoid the need for these calls.

Jon Smirl
[email protected]

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

For driver model conformant subsystems, it's always true.

That is, a bus-specific struct wrapping a "struct device" instance.

Wrong. The probe returns zero if it bound, else negative errno. Those functions don't return booleans.

No. The struct device is not destroyed at that time for ANY driver model code I've seen in Linux. Destroying it would prevent driver modules from binding to that device when they get loaded later.

Well, a "struct device" must be present, but there might not be hardware backing it. Each particular bus infrastructure instantiates those structs in bus-specific ways.

True.

Depends somewhat on what flavor of PNP you mean too.

And that legacy model is something that's getting removed everywhere practical. Such "legacy drivers" don't support hotplugging or most of the other mechanisms defined by the driver model, since they have the driver creating the device structs (instead of bus infrastructure).

You're right that this is why the name probe() remains. I prefer the bind()/unbind() names too.

- Dave

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

You guys are so literal. I should have said return 0 or -ENODEV or some other error.

Don't they get destroyed when hotplug devices are removed from the system? Could this be considered a hotplug, create struct device, probe to see if it is claimed, unhotplug if not claimed, destroy struct device.

But I agree with what you meant, that they never get destroyed when there is known hardware backing them in the system.

Those drivers are getting removed because the hardware is disappearing and we have ACPI/PNP now to tell if if is present, not because they misused the driver code. We just don't need the old style probing code anymore and it was unreliable.

There is is a parallel here. With PowerPC device trees I can put the exact address of the i2c device into the board specific device tree. Now there's no need to probe addresses. That's equivalent to what's happening with ACPI and serial ports. Maybe we'll implement device trees on all archs and then we can get rid of i2c device probing.

Jon Smirl
[email protected]

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

You seemed intent on describing some new and never-seen-before model for how driver model probing works, though ...

That's not "at that time". That's "when the hardware is removed" ... for busses which support such detection. USB supports it. PCI itself doesn't (like I2C, SPI, etc).

CardBus, Hotplug PCI, and hotplug PCIE do report removals, so they can remove such device structs later. But they would never do it as part of enumeration.

No. Think more about what goes into hotplugging. It's not just "some device is at that address". It's also "device with such-and-such-identifiers is there".

I've seen folk re-inventing such probing for custom hardware, FWIW. (Such stuff doesn't get merged upstream very often.)

There are surely worse futures to envision!

ACPI would need to expose I2C/SMBus though. I suspect that's something many current x86 platform vendors would oppose.

- Dave

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

I don't think it is out of the scope of what already exists. The alternative is to add a new pre-probe() API which does the function of the original probe and then calls the current probe(). Why don't we just stick with probe() and use it like it was before we had things like ACPI?

Now that we've had this discussion it looks to me that the scan capability should be added to struct i2c_driver instead of being standalone.

I guess what is a little strange to me is that the pre-probe() function is like a static driver function and probe() is an instance function. Are there other examples of static driver functions? Which way you do it is a style question, they will both work.

Doesn't something like this work? (obviously this isn't complete)

for (address = start; address <= stop;) {

	make a new client
	for (; address <=stop; address++) {
		client->address = address;
		res = device_register(&client->dev);
		if (res != -ENODEV)
			break;
	}
}

delete the extra client

Can these driver functions be eliminated?

	int (*attach_adapter)(struct i2c_adapter *);
	int (*detach_adapter)(struct i2c_adapter *);
	int (*detach_client)(struct i2c_client *);

Jon Smirl
[email protected]

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

The problem there is: how do you know which pre-probe() function(s) to try? If you could answer that reliably you'd be able to probe() directly. If you can't, you'd need to load all I2C driver modules until one works... not the desired system model.

Module load and unload code; platform_driver_probe() type stuff.

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

I see now that the adapter class should be added to the driver name alias string. The we could load all modules of a given class.

But I don't think we need to go searching for the right module to load since i2c is not generally a dynamic bus. If you install a video card with an i2c bus, the driver for the video card can just load_module the right i2c driver modules.

I do wonder if we could get hardware monitors to autoload. When the adapter driver for the motherboard loads, it could load module all drivers of class hardware_monitor. Then each one could do its detection thing. Unload the ones that weren't found. That would really help people who can figure out the right driver module to load. You could even get fancy and generate a uevent after the right module was found. A script in the uevent could save the info so you don't have to repeat on each boot. Of course this assume that the hardware monitor modules can reliably autodetect.

Jon Smirl
[email protected]

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

Ah ah. Oh wait, you were serious? Come on, why would anyone on earth want to load all modules of a given class?

Sometimes yes, sometimes no. For most V4L/DVB, the main driver should know which i2c chip drivers it needs, but unfortunately for old drivers such as bttv this knowledge has never been recorded, so there is no alternative to (at least partial) probing in practice. For video adapters, they could load the eeprom driver as they all give access to EDID EEPROMs, but if the card also has a hardware monitoring chip, we simply don't know what it is. We just can't record in the kernel drivers what hardware monitoring chip exists on thousands of cards out there (same problem as for motherboards.)

Only if they don't do the device detection/identification themselves. I've considered it for a moment but now I'm fairly certain that it would be a dangerous and unmaintainable mess. Better let each driver include its detection/identification routine, and delegate the responsibility of loading the right driver to user-space (as is already the case.)

No, really, forget about this, that's simply never going to happen. That's way too costly and dangerous.

And do it all over again when the graphics adapter driver is loaded? And when an parport-to-I2C or USB-to-I2C adapter is plugged?

If people can't run "sensors-detect" there's not much we can do for them ;)

You mean, instead of just using the sensors-detect script which we already have and has worked fine for (almost) everyone for 8 years now? And the few cases where sensors-detect didn't work are a very good reason to _not_ attempt to do the same in the kernel. SMBus probing is _very_ tricky and we really want to control everything that happens.

Jean Delvare

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

What about live CDs and things like that? I do agree that there is no immediate need to do something like this. But in the long run a scheme like this could eliminate the need for sensors-detect.

It doesn't take as long to load/unload the modules as you might think, they would all be on an initrd. The probing time should be the same as the sensors-detect script. Maintenance would be better since the detection code would only exist in one place instead of two.

File this away to think about for the future. After the drivers are converted to the new model and have the ability to detect this can be experimented with.

Jon Smirl
[email protected]

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

Why do you want to eliminate sensors-detect in the first place? If you really want to load the sensor drivers automatically, just run sensors-detect at boot time.

I just tested, that's 2.3 s on my system, and then 0.85 s to unload the drivers. So, total 3 s. I certainly don't want my system to take 3 more seconds to load. And that's the best case, at HZ=1000. At HZ=100 it will be worse.

Anyway, if you really want to load all the drivers at boot time, this should be a user-space decision and not a kernel-space decision. It can already be done in user-space today (but note that no distribution does this, thankfully), so you're trying to solve a problem that doesn't really exist.

2.1 MB of hwmon drivers in initrd? To be added to all the i2c bus drivers... I doubt anybody wants this. These drivers aren't needed to boot the system so why put them in initrd? Furthermore, I don't see how we care if the drivers are in initrd or not.

Admittedly it would be nice to have only one set of detection code to maintain. But there are many drawbacks. For example, how do you detect devices we don't yet support? Or devices that are supported in a later kernel? Devices for which the user didn't build the driver yet? It is very easy to point users to the latest version of sensors-detect and ask them to run it. Compare this to "you have to upgrade to the latest kernel and build all the drivers first and only then we can tell you the one you need".

You are free to experiment with whatever you want, of course, but my feeling is that you are going in the wrong direction. If anything, I'd rather make sensors-detect safer, faster and more controllable (command line interface...) than trying to move it inside the kernel.

Jean Delvare

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

You're thinking of probing the wrong way around, methinks. We want to add something _before_ the current probe function, not after it. Adding the actual hardware detection/identification after the driver model's probe() implies that a device has already been created (otherwise .probe() can't be called). Chicken and egg problem.

After a good night of sleep I've reached the same conclusion. Given that many drivers would have a new-style i2c driver and an i2c listener, it's probably easier to merge both. I'll give it a try when I get time, but as stated elsewhere in this thread, I wouldn't be too surprised to hit a deadlock or two on the road.

I guess it depends on what you mean with a "static function" and "instance function" for a non-object language ;) Put it simple terms, the hardware detection function has to operate on addresses (i.e. potential devices) while the device driver model's probe function operates on actual devices. I fully agree with David that this function should really be named .bind(), not .probe().

You don't seriously propose to scan all addresses on every I2C bus for every i2c chip driver that we load, do you? This would be awfully slow and dangerous. No way.

This is exactly my goal with i2c_notifier (or any similar approach - again I'm not claiming my proposal is the only way to go). Sorry if it wasn't clear, maybe I should have mentioned it explicitly in patch 0/4.

Jean Delvare

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

This is just a small part of the code, you need to add in all the other parts from the patch.
Exactly the same addresses would be scanned under each scheme. .My question is, does the driver need two entry points (detect and probe) or could only one (probe) be used? I think detect and probe can be the same function.

With the detect() function it would look somthing like this:

for (address = start; address <= stop;) {

      for (; address <=stop; address++) {
             do a generic detect
              if (driver->detect(address) {
                     make a new client
                     client->address = address;
                     res = device_register(&client->dev);
                     if (!res)
                            delete the extra client
             }
       }
}

Jon Smirl
[email protected]

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

Hi Jon,

Oh, David is several persons now? ;)

It's the manufacturer who decide, not us. And at the moment, the PC architecture isn't using device trees for I2C/SMBus. That's an unfortunate fact we have to live with.

Jean Delvare

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

Hi Jon,

Yes.

No. Again, detect() creates the very device that probe() will be called on, so merging detect() into probe() is, say, like dividing by 0.

(OK, not my most direct metaphor of the day - oh well ;))

Out of curiosity, what would be the name of the device backing that i2c_client? The device's name is what the driver core uses to know which driver's probe() method needs to be called.

Maybe I'm just not seeing what you want to do. I'll be happy to see a (tested) patch implementing your idea though. If you can come up with something that works and has advantages over what I proposed, I'll certainly consider it.

Yes, the operations on each adapter are already serialized, no problem with this.

Care to enlighten me on what this other way would be? It's easy to claim this of pretty much everything, but that's not valuable without details on the "other way", sorry.

On top of that, you'd also need to explain why adapter_attach and adapter_detach are bad, before claiming that they should be replaced by something else. I am worried about the use that is done of these callbacks at the moment, not by the callbacks themselves. If there's a legitimate use of them (I still need to check the V4L/DVB drivers) I see no reason to remove them.

That's certainly a possibility, yes, and I'll look into this if it turns out that i2c-dev is the last user of any part of i2c_driver. But then I wonder why i2c-dev was implemented the way it was, if all it needed was access to some i2c-core internals.

Jean Delvare

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

I'm just proposing optimizations to the scheme, we are in agreement on the core of the proposal. Let's go ahead with the way you proposed (with it moved to i2c_driver). Then after you get everything working with the new scheme I'll see if I can add optimizations to remove some of the entry points. It is more important to get the old to new driver conversion going.

Jon Smirl
[email protected]

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

Couldn't you say the probe function is called on a potential device? The probe function can return -ENODEV, in which can other driver's probes get called, and it's perfectly ok if no driver binds to it.

The way PCI works, is that when a new pci bus is created, each address is probed and a device is created if anything responds. The generic bus code tries to match each device to a driver or drivers and calls those drivers' probe functions. The drivers don't have to claim the device in the probe function. The bus code handles all the cases of a driver or bus getting added or removed in various orders.

So why can't I2C do this too?

When a new bus is created, the i2c core scans it and creates a device for each chip that responds. Each driver's probe function is called in turn on the device and it gets to device if it wants to drive it or not, probably based on i2c address range since there is usually little else to go on.

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

... by config space accessors which all PCI devices support.

... using a formally managed set of product identifiers.

No such product identifiers, and in general no way to tell what's sitting at a given address. And in fact, there's no sure way to tell if a device is present there, since when an I2C device is busy, it's not required to ack its address.

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

PCI is not immune to broken hardware. It's much better than I2C of course, which doesn't even have a scanning method devices are supposed to follow.

There are plenty of PCI devices with broken ids.

So scanning an I2C bus doesn't work as well as with PCI, we all know that. How does creating a new "i2c listeners" system solve any of these problems? You still want to probe for a chip and there's no foolproof way to do that, and there's still no way to know for sure what chip you have found.

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

As I said, not just "scanning" but also "identifying".

PCI was designed explicitly for automatic configuration. I2C was not.

That's an entirely different (and unrelated) question.

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

No need to be so pedantic, by scanning I meant probing and identifying.

For a thread on the subject "Introduce i2c listeners", I don't think so. When I first saw the patch, I wondered why exsting driver model couldn't handle it. It looks like Jon thought the same thing. No one ever said scanning/probing/identifying/whatever an I2C bus worked as well as it does on a bus like PCI. But I don't see why that prevents the driver model from being used.

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

On Thursday 05 June 2008, Trent Piepho wrote:.

For the subthread on why I2C can't scan like PCI ... it sure seemed to me like it was! But ok, you maybe weren't focussed on that subthread.

Well, without commenting on these new listeners/notifiers ...

All I can observe is that what the driver model provides is just the framework through which the device enumeration is exposed.

That device enumeration is bus-specific. It can build on hardware for device identification (as with USB and PCI) and maybe hotplug (USB, and PCI flavors like Cardbus and hotplug PCI). Or it can build on software tables, as in PNPACPI and OpenFirmware. You can view cases like USB and PCMCIA cards (8/16 bits) as being hybrid:
hotplug hardware, combined with software tables in the device.

The I2C problem has been that hardware conventions can't work in general, so Linux has had to come up with its own conventions for software tables.

If nothing else, we have an existence proof (in "new style" I2C drivers) that we *can* use the driver model with I2C.

- Dave

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

Hi Trent,

There are two key properties of the i2c listeners:

1* They live besides the driver model and complement it, rather than
replacing it. They have to, because the driver model cares about device enumeration and device/driver binding, NOT device detection and identification in the hardware sense of the terms.

2* Listeners allow the i2c-core to delegate the device detection /
identification to the drivers themselves (avoiding a fat, slow and unmaintainable centralized detection routine) but the drivers are still not creating the devices themselves (so the device driver model's spirit is respected.)

That's correct, but the fact is that for example all I2C-based hardware monitoring drivers are already doing their best to identify the chips they support, with good success. The question is not whether we'll do that or not - we _have_ to do it in many cases. The question is how we implement it.

Currently we have a separate binding model for this case, I'm proposing something that would instead fit in the standard binding model, only adding the part that the binding model doesn't cover. This is certainly an improvement, I don't think anyone can deny this. Not to say that my proposal cannot be improved further - it certainly can.

Jean Delvare

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

I made a mistake comparing this to PCI IDs. I should not have used PCI IDs as an example and gotten us off on the tangent of not having a global i2c device namespace.

Adding the address range to search is an unrelated problem to the name space issue.

Jon Smirl
[email protected]

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

Hi Jon,

It's outside of the bus structure. The device driver model defines how drivers bind to the devices they support. It doesn't imply much on how devices are instantiated. For bus types where the present devices are enumerated (like PCI) the device instantiation is pretty straightforward, but in the case of I2C/SMBus where you have to either know in advance or actively probe and guess which devices are there, you clearly need additional code and strategy to instantiate the devices.

That's the very reason why the i2c subsystem didn't originally follow the device driver model (until David converted it to add support for the standard model in addition to our custom one.)

I don't think it is related to hotplug at all. The issue here is not whether devices are hotplugged or not. It's also not about how the drivers are notified about devices and how they bind to them. The issue is the step before that: how do you find and instantiate the devices if you have no preliminary knowledge on what devices are present.

Now, if I have really been duplicating a mechanism that exists in the driver core, I'll be happy to simplify my code to take benefit of it. But then please point me to the exact piece of code you're thinking about.

Jean Delvare

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

Which, for the record, will be a bit delayed with respect to the core i2c_listener abstraction. This deserves a bit more thought than I can put into it in the immediate future. (Ditto firing up the rig needed to test the lm75 updates.)

Except for this small bit of instafeedback: the approach of splitting the detect() logic out, as an adjunct to a core "new style" driver, is an interesting way to address some core problems with legacy drivers (vs the driver model).

It gives a graceful migration model towards new-style code, and lets drivers get updated without needing to start by abandoning the "poke at the hardware" style enumeration in those cases where it seems to act mostly OK today. While I'd like to see that poking be largely removed from the kernel, factoring it out (in i2c_listener or whatever) is clearly useful.

- Dave

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c