Skip to contentSkip to conversation

improve i2c probing

PATCH In review Nathan Lutchansky 3 participants 18 messages Aug 15, 2005 – Aug 20, 2005

Subsystems (7)
lore
Share & tools
lore
#0 lore raw

cover improve i2c probing

Hi everyone,

This patch series makes a couple of improvements to the i2c device probing process.

Currently, when a new i2c bus needs to be probed, the i2c subsystem calls the attach_adapter callback for each loaded i2c client driver, which must call the i2c_probe function with a list of addresses to be probed and another callback for reporting detected devices:

    static int foo_attach_adapter(struct i2c_adapter *adapter)
    {
            if (!(adapter->class & I2C_CLASS_HWMON))
                    return 0;
            return i2c_probe(adapter, &addr_data, foo_detect);
    }

Virtually every i2c client driver uses exactly the same code, so there's little point in requiring them all to implement this callback. The first patch in this series adds two new fields to the i2c_driver structure, address_data' and detect_client', and if they are set by the driver, the i2c core will automatically call i2c_probe using those fields as the second and third argument. If the `class' field of the i2c_driver structure is set, it will be compared with the adapter class first.

Patches 2 and 3 add these fields to the i2c_driver initializer in the i2c hwmon and misc i2c chip drivers and remove the corresponding attach_adapter callbacks.

The second improvement (which is really the point of this patch set) is to add the functions i2c_probe_device and i2c_remove_device for directly creating and destroying i2c clients on a particular adapter:

int i2c_probe_device(struct i2c_adapter *adapter, int driver_id,
int addr, int kind);
int i2c_remove_device(struct i2c_adapter *adapter, int driver_id,
int addr);

These functions make the i2c subsystem usable for special-purpose i2c buses where probing isn't possible, either because probing is known to be dangerous for devices that are present on the bus, or because the i2c adapter lacks quick writes and/or error reporting.

The final patch adds a new i2c adapter flag to indicate that the adapter should never be probed.

This patch set applies cleanly to the end of Greg KH's i2c patch queue, as of 12-Aug-2005. -Nathan

 Documentation/i2c/writing-clients |   58 ++++++++++++++---------
 drivers/hwmon/adm1021.c           |   12 +---
 drivers/hwmon/adm1025.c           |   12 +---
 drivers/hwmon/adm1026.c           |   13 +----
 drivers/hwmon/adm1031.c           |   13 +----
 drivers/hwmon/adm9240.c           |   12 +---
 drivers/hwmon/asb100.c            |   17 +-----
 drivers/hwmon/atxp1.c             |    9 ---
 drivers/hwmon/ds1621.c            |   10 ----
 drivers/hwmon/fscher.c            |   12 +---
 drivers/hwmon/fscpos.c            |   12 +---
 drivers/hwmon/gl518sm.c           |   12 +---
 drivers/hwmon/gl520sm.c           |   12 +---
 drivers/hwmon/it87.c              |   17 +-----
 drivers/hwmon/lm63.c              |   12 +---
 drivers/hwmon/lm75.c              |   13 +----
 drivers/hwmon/lm77.c              |   13 +----
 drivers/hwmon/lm78.c              |   17 +-----
 drivers/hwmon/lm80.c              |   12 +---
 drivers/hwmon/lm83.c              |   12 +---
 drivers/hwmon/lm85.c              |   12 +---
 drivers/hwmon/lm87.c              |   12 +---
 drivers/hwmon/lm90.c              |   12 +---
 drivers/hwmon/lm92.c              |   11 +---
 drivers/hwmon/max1619.c           |   12 +---
 drivers/hwmon/w83781d.c           |   17 +-----
 drivers/hwmon/w83792d.c           |   18 +------
 drivers/hwmon/w83l785ts.c         |   12 +---
 drivers/i2c/chips/ds1337.c        |    9 ---
 drivers/i2c/chips/ds1374.c        |    8 ---
 drivers/i2c/chips/eeprom.c        |   10 ----
 drivers/i2c/chips/m41t00.c        |    9 ---
 drivers/i2c/chips/max6875.c       |   10 ----
 drivers/i2c/chips/pca9539.c       |   10 ----
 drivers/i2c/chips/pcf8574.c       |   10 ----
 drivers/i2c/chips/pcf8591.c       |   10 ----
 drivers/i2c/chips/rtc8564.c       |    8 ---
 drivers/i2c/i2c-core.c            |   95 ++++++++++++++++++++++++++++++++++++--
 include/linux/i2c.h               |   21 ++++++++
 39 files changed, 246 insertions(+), 360 deletions(-)
#1 lore raw

PATCH 1/5 call i2c_probe from i2c core

Add the address_data and detect_client fields to the i2c_driver structure. If these are set, i2c core will call i2c_probe directly when attach_adapter would have been called. If the i2c_driver class field is also set, probing will only be done on adapters with an intersecting class field.

The attach_adapter callback will still be called if it is present, but this makes it unnecessary for almost all in-tree i2c drivers.

 Documentation/i2c/writing-clients |   58 ++++++++++++++++++++++++--------------
 drivers/i2c/i2c-core.c            |   21 +++++++++++--
 include/linux/i2c.h               |   11 +++++++
 3 files changed, 65 insertions(+), 25 deletions(-)
include/linux/i2c.h +11 -0
Index: linux-2.6.13-rc6+gregkh/include/linux/i2c.h
===================================================================
@@ -48,6 +48,7 @@ struct i2c_algorithm;
 struct i2c_adapter;
 struct i2c_client;
 struct i2c_driver;
+struct i2c_client_address_data;
 union i2c_smbus_data;
 
 /*
@@ -113,6 +114,7 @@ struct i2c_driver {
 	int id;
 	unsigned int class;
 	unsigned int flags;		/* div., see below		*/
+	struct i2c_client_address_data *address_data;
 
 	/* Notifies the driver that a new bus has appeared. This routine
 	 * can be used by the driver to test if the bus meets its conditions
@@ -123,6 +125,15 @@ struct i2c_driver {
 	int (*attach_adapter)(struct i2c_adapter *);
 	int (*detach_adapter)(struct i2c_adapter *);
 
+	/* Requests that the driver validate an address on a bus and attach a
+	 * new client. If this routine is supplied, it will be called for
+	 * each device on new buses that appear, provided the bus class
+	 * matches the class field and devices exist at the addresses listed
+	 * in the address_data field. For most drivers, this mechanism can
+	 * be used instead of an attach_adapter routine.
+	 */
+	int (*detect_client)(struct i2c_adapter *, int addr, int kind);
+
 	/* tells the driver that a client is about to be deleted & gives it 
 	 * the chance to remove its private data. Also, if the client struct
 	 * has been dynamically allocated by the driver in the function above,
drivers/i2c/i2c-core.c +17 -4
Index: linux-2.6.13-rc6+gregkh/drivers/i2c/i2c-core.c
===================================================================
@@ -193,9 +193,16 @@ int i2c_add_adapter(struct i2c_adapter *
 	/* inform drivers of new adapters */
 	list_for_each(item,&drivers) {
 		driver = list_entry(item, struct i2c_driver, list);
-		if (driver->flags & I2C_DF_NOTIFY)
-			/* We ignore the return code; if it fails, too bad */
-			driver->attach_adapter(adap);
+		if (driver->flags & I2C_DF_NOTIFY) {
+			/* We ignore the return codes; if it fails, too bad */
+			if (driver->attach_adapter)
+				driver->attach_adapter(adap);
+			if (driver->detect_client && driver->address_data &&
+					((driver->class & adap->class) ||
+						driver->class == 0))
+				i2c_probe(adap, driver->address_data,
+						driver->detect_client);
+		}
 	}
 
 out_unlock:
@@ -307,7 +314,13 @@ int i2c_add_driver(struct i2c_driver *dr
 	if (driver->flags & I2C_DF_NOTIFY) {
 		list_for_each(item,&adapters) {
 			adapter = list_entry(item, struct i2c_adapter, list);
-			driver->attach_adapter(adapter);
+			if (driver->attach_adapter)
+				driver->attach_adapter(adapter);
+			if (driver->detect_client && driver->address_data &&
+					((driver->class & adapter->class) ||
+						driver->class == 0))
+				i2c_probe(adapter, driver->address_data,
+						driver->detect_client);
 		}
 	}
 
Documentation/i2c/writing-clients +37 -21

88 lines not on this page · raw message

#2 lore raw

Re: [PATCH 1/5] call i2c_probe from i2c core

Hi Nathan,

Quoted 2 lines

Index: linux-2.6.13-rc6+gregkh/include/linux/i2c.h
===================================================================

You should probably be using the --no-index option of quilt 0.42 (if you are using quilt as I presumed), as I heard Linus doesn't like these index lines in the patches he receives.

Quoted 26 lines
drivers/i2c/i2c-core.c +10 -3
@@ -193,9 +193,16 @@ int i2c_add_adapter(struct i2c_adapter *
 	/* inform drivers of new adapters */
 	list_for_each(item,&drivers) {
 		driver = list_entry(item, struct i2c_driver, list);
-		if (driver->flags & I2C_DF_NOTIFY)
-			/* We ignore the return code; if it fails, too bad */
-			driver->attach_adapter(adap);
+		if (driver->flags & I2C_DF_NOTIFY) {
+			/* We ignore the return codes; if it fails, too bad */
+			if (driver->attach_adapter)
+				driver->attach_adapter(adap);
+			if (driver->detect_client && driver->address_data &&
+					((driver->class & adap->class) ||
+						driver->class == 0))
+				i2c_probe(adap, driver->address_data,
+						driver->detect_client);
+		}
 	}
 
 out_unlock:
@@ -307,7 +314,13 @@ int i2c_add_driver(struct i2c_driver *dr
 	if (driver->flags & I2C_DF_NOTIFY) {
 		list_for_each(item,&adapters) {
 			adapter = list_entry(item, struct i2c_adapter, list);

drivers/i2c/i2c-core.c int i2c_add_driver(struct i2c_driver *dr · line 325

Quoted 10 lines
drivers/i2c/i2c-core.c +7 -1
-			driver->attach_adapter(adapter);
+			if (driver->attach_adapter)
+				driver->attach_adapter(adapter);
+			if (driver->detect_client && driver->address_data &&
+					((driver->class & adapter->class) ||
+						driver->class == 0))
+				i2c_probe(adapter, driver->address_data,
+						driver->detect_client);
 		}
 	}

Couldn't we check for the return value of driver->attach_adapter()? That way this function could conditionally prevent i2c_probe() from being run. This is just a random proposal, I don't know if some drivers would have an interest in doing that.

Also, maybe we can put this new code in a separate function to be called from both i2c_add_adapter and i2c_add_driver, so as to not duplicate code? Or would this be too much overhead? It could be made inline then. Again, just a random thought.

Documentation/i2c/writing-clients

Quoted 2 lines
Documentation/i2c/writing-clients +0 -0

Thanks for the documentation update. However, you didn't update i2c/porting-clients accordingly. Could you please?

Thanks,

Signature · 1 line
Jean Delvare
#3 lore raw

Re: [PATCH 1/5] call i2c_probe from i2c core

Quoted 3 lines · Jean Delvare

You should probably be using the --no-index option of quilt 0.42 (if you are using quilt as I presumed), as I heard Linus doesn't like these index lines in the patches he receives.

Ooh, thanks!

Quoted 10 lines
Quoted 10 lines
drivers/i2c/i2c-core.c +6 -1
 	if (driver->flags & I2C_DF_NOTIFY) {
 		list_for_each(item,&adapters) {
 			adapter = list_entry(item, struct i2c_adapter, list);
-			driver->attach_adapter(adapter);
+			if (driver->attach_adapter)
+				driver->attach_adapter(adapter);
+			if (driver->detect_client && driver->address_data &&
+					((driver->class & adapter->class) ||
+						driver->class == 0))
+				i2c_probe(adapter, driver->address_data,
Quoted 8 lines

drivers/i2c/i2c-core.c int i2c_add_driver(struct i2c_driver *dr · line 325

Quoted 3 lines
drivers/i2c/i2c-core.c +1 -0
+						driver->detect_client);
 		}
 	}

Couldn't we check for the return value of driver->attach_adapter()? That way this function could conditionally prevent i2c_probe() from being run. This is just a random proposal, I don't know if some drivers would have an interest in doing that.

Yeah, I was thinking about that too, but I can't think of a reasonable return code to use. -1 for "don't probe"? Or <0 for "fatal error, don't touch this bus any more"? Anyway, client drivers will probably only use one of the two detection methods because if they need to implement attach_adapter they can just call i2c_probe from there.

Quoted 4 lines

Also, maybe we can put this new code in a separate function to be called from both i2c_add_adapter and i2c_add_driver, so as to not duplicate code? Or would this be too much overhead? It could be made inline then. Again, just a random thought.

Sure, I can do that.

Quoted 5 lines

Documentation/i2c/writing-clients

Quoted 2 lines
Documentation/i2c/writing-clients +0 -0

Thanks for the documentation update. However, you didn't update i2c/porting-clients accordingly. Could you please?

I didn't think anybody was porting client drivers any more, but if you're still updating that doc, I can too. :-) -Nathan

#4 lore raw

Re: [PATCH 1/5] call i2c_probe from i2c core

Hi Nathan,

Quoted 19 lines
Quoted 10 lines
Quoted 10 lines
drivers/i2c/i2c-core.c +6 -1
 	if (driver->flags & I2C_DF_NOTIFY) {
 		list_for_each(item,&adapters) {
 			adapter = list_entry(item, struct i2c_adapter, list);
-			driver->attach_adapter(adapter);
+			if (driver->attach_adapter)
+				driver->attach_adapter(adapter);
+			if (driver->detect_client && driver->address_data &&
+					((driver->class & adapter->class) ||
+						driver->class == 0))
+				i2c_probe(adapter, driver->address_data,
Quoted 8 lines

drivers/i2c/i2c-core.c int i2c_add_driver(struct i2c_driver *dr · line 325

Quoted 3 lines
drivers/i2c/i2c-core.c +1 -0
+						driver->detect_client);
 		}
 	}

Couldn't we check for the return value of driver->attach_adapter()? That way this function could conditionally prevent i2c_probe() from being run. This is just a random proposal, I don't know if some drivers would have an interest in doing that.

Quoted 5 lines

Yeah, I was thinking about that too, but I can't think of a reasonable return code to use. -1 for "don't probe"? Or <0 for "fatal error, don't touch this bus any more"? Anyway, client drivers will probably only use one of the two detection methods because if they need to implement attach_adapter they can just call i2c_probe from there.

Oh well, on second thought you are probably right, there is little benefit in implementing my proposal, and this will make the code more complex. Let's just forget about it until someone actually needs it.

Quoted 2 lines

I didn't think anybody was porting client drivers any more, but if you're still updating that doc, I can too. :-)

I count 15 drivers [1] of the lm_sensors project that are still missing from Linux 2.6. Maybe some of these will never be ported due to a lack of interest, but I believe a good half will be (five ports are in progress already.) So we need to keep the i2c/porting-client document up-to-date for a few more months.

Thanks.

[1] adm1024, bmcsensors, ds1307, fscscy, lm93, matorb, max6650, maxilife, mic74, mtp008, saa1064, smartbatt, thmc50, vt1211, vt8231.

Signature · 1 line
Jean Delvare
#5 lore raw

PATCH 2/5 remove attach_adapter from i2c hwmon drivers

In reply to Nathan Lutchansky

The attach_adapter callback of every i2c hwmon driver can be removed, now that the i2c core will implicitly call i2c_probe if the address data is present in the i2c_driver structure.

 drivers/hwmon/adm1021.c   |   12 +++---------
 drivers/hwmon/adm1025.c   |   12 +++---------
 drivers/hwmon/adm1026.c   |   13 +++----------
 drivers/hwmon/adm1031.c   |   13 +++----------
 drivers/hwmon/adm9240.c   |   12 +++---------
 drivers/hwmon/asb100.c    |   17 +++--------------
 drivers/hwmon/atxp1.c     |    9 ++-------
 drivers/hwmon/ds1621.c    |   10 ++--------
 drivers/hwmon/fscher.c    |   12 +++---------
 drivers/hwmon/fscpos.c    |   12 +++---------
 drivers/hwmon/gl518sm.c   |   12 +++---------
 drivers/hwmon/gl520sm.c   |   12 +++---------
 drivers/hwmon/it87.c      |   17 +++--------------
 drivers/hwmon/lm63.c      |   12 +++---------
 drivers/hwmon/lm75.c      |   13 +++----------
 drivers/hwmon/lm77.c      |   13 +++----------
 drivers/hwmon/lm78.c      |   17 +++--------------
 drivers/hwmon/lm80.c      |   12 +++---------
 drivers/hwmon/lm83.c      |   12 +++---------
 drivers/hwmon/lm85.c      |   12 +++---------
 drivers/hwmon/lm87.c      |   12 +++---------
 drivers/hwmon/lm90.c      |   12 +++---------
 drivers/hwmon/lm92.c      |   11 +++--------
 drivers/hwmon/max1619.c   |   12 +++---------
 drivers/hwmon/w83781d.c   |   17 +++--------------
 drivers/hwmon/w83792d.c   |   18 +++---------------
 drivers/hwmon/w83l785ts.c |   12 +++---------
 27 files changed, 79 insertions(+), 269 deletions(-)
drivers/hwmon/adm1021.c +3 -9
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/adm1021.c
===================================================================
@@ -111,7 +111,6 @@ struct adm1021_data {
 	u8	remote_temp_offset_prec;
 };
 
-static int adm1021_attach_adapter(struct i2c_adapter *adapter);
 static int adm1021_detect(struct i2c_adapter *adapter, int address, int kind);
 static void adm1021_init_client(struct i2c_client *client);
 static int adm1021_detach_client(struct i2c_client *client);
@@ -129,8 +128,10 @@ static struct i2c_driver adm1021_driver 
 	.owner		= THIS_MODULE,
 	.name		= "adm1021",
 	.id		= I2C_DRIVERID_ADM1021,
+	.class		= I2C_CLASS_HWMON,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= adm1021_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= adm1021_detect,
 	.detach_client	= adm1021_detach_client,
 };
 
@@ -182,13 +183,6 @@ static DEVICE_ATTR(temp2_input, S_IRUGO,
 static DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL);
 
 
-static int adm1021_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, adm1021_detect);
-}
-
 static int adm1021_detect(struct i2c_adapter *adapter, int address, int kind)
 {
 	int i;
drivers/hwmon/adm1025.c +3 -9
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/adm1025.c
===================================================================
@@ -107,7 +107,6 @@ static int in_scale[6] = { 2500, 2250, 3
  * Functions declaration
  */
 
-static int adm1025_attach_adapter(struct i2c_adapter *adapter);
 static int adm1025_detect(struct i2c_adapter *adapter, int address, int kind);
 static void adm1025_init_client(struct i2c_client *client);
 static int adm1025_detach_client(struct i2c_client *client);
@@ -121,8 +120,10 @@ static struct i2c_driver adm1025_driver 
 	.owner		= THIS_MODULE,
 	.name		= "adm1025",
 	.id		= I2C_DRIVERID_ADM1025,
+	.class		= I2C_CLASS_HWMON,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= adm1025_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= adm1025_detect,
 	.detach_client	= adm1025_detach_client,
 };
 
@@ -309,13 +310,6 @@ static DEVICE_ATTR(vrm, S_IRUGO | S_IWUS
  * Real code
  */
 
-static int adm1025_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, adm1025_detect);
-}
-
 /*
  * The following function does more than just detection. If detection
  * succeeds, it also registers the new chip.
drivers/hwmon/adm1026.c +3 -10
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/adm1026.c
===================================================================
@@ -294,7 +294,6 @@ struct adm1026_data {
 	u8 config3;             /* Register value */
 };
 
-static int adm1026_attach_adapter(struct i2c_adapter *adapter);
 static int adm1026_detect(struct i2c_adapter *adapter, int address,
 	int kind);
 static int adm1026_detach_client(struct i2c_client *client);
@@ -310,19 +309,13 @@ static void adm1026_init_client(struct i
 static struct i2c_driver adm1026_driver = {
 	.owner          = THIS_MODULE,
 	.name           = "adm1026",
+	.class		= I2C_CLASS_HWMON,
 	.flags          = I2C_DF_NOTIFY,
-	.attach_adapter = adm1026_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client  = adm1026_detect,
 	.detach_client  = adm1026_detach_client,
 };
 
-int adm1026_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON)) {
-		return 0;
-	}
-	return i2c_probe(adapter, &addr_data, adm1026_detect);
-}
-
 int adm1026_detach_client(struct i2c_client *client)
 {
 	struct adm1026_data *data = i2c_get_clientdata(client);
drivers/hwmon/adm1031.c +3 -10
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/adm1031.c
===================================================================
@@ -97,7 +97,6 @@ struct adm1031_data {
 	s8 temp_crit[3];
 };
 
-static int adm1031_attach_adapter(struct i2c_adapter *adapter);
 static int adm1031_detect(struct i2c_adapter *adapter, int address, int kind);
 static void adm1031_init_client(struct i2c_client *client);
 static int adm1031_detach_client(struct i2c_client *client);
@@ -107,8 +106,10 @@ static struct adm1031_data *adm1031_upda
 static struct i2c_driver adm1031_driver = {
 	.owner = THIS_MODULE,
 	.name = "adm1031",
+	.class = I2C_CLASS_HWMON,
 	.flags = I2C_DF_NOTIFY,
-	.attach_adapter = adm1031_attach_adapter,
+	.address_data = &addr_data,
+	.detect_client = adm1031_detect,
 	.detach_client = adm1031_detach_client,
 };
 
@@ -722,14 +723,6 @@ static ssize_t show_alarms(struct device
 static DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL);
 
 
-static int adm1031_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, adm1031_detect);
-}
-
-/* This function is called by i2c_probe */
 static int adm1031_detect(struct i2c_adapter *adapter, int address, int kind)
 {
 	struct i2c_client *new_client;
drivers/hwmon/adm9240.c +3 -9
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/adm9240.c
===================================================================
@@ -129,7 +129,6 @@ static inline unsigned int AOUT_FROM_REG
 	return SCALE(reg, 1250, 255);
 }
 
-static int adm9240_attach_adapter(struct i2c_adapter *adapter);
 static int adm9240_detect(struct i2c_adapter *adapter, int address, int kind);
 static void adm9240_init_client(struct i2c_client *client);
 static int adm9240_detach_client(struct i2c_client *client);
@@ -140,8 +139,10 @@ static struct i2c_driver adm9240_driver 
 	.owner		= THIS_MODULE,
 	.name		= "adm9240",
 	.id		= I2C_DRIVERID_ADM9240,
+	.class		= I2C_CLASS_HWMON,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= adm9240_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= adm9240_detect,
 	.detach_client	= adm9240_detach_client,
 };
 
@@ -630,13 +631,6 @@ exit:
 	return err;
 }
 
-static int adm9240_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, adm9240_detect);
-}
-
 static int adm9240_detach_client(struct i2c_client *client)
 {
 	struct adm9240_data *data = i2c_get_clientdata(client);
drivers/hwmon/asb100.c +3 -14
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/asb100.c
===================================================================
@@ -210,7 +210,6 @@ struct asb100_data {
 static int asb100_read_value(struct i2c_client *client, u16 reg);
 static void asb100_write_value(struct i2c_client *client, u16 reg, u16 val);
 
-static int asb100_attach_adapter(struct i2c_adapter *adapter);
 static int asb100_detect(struct i2c_adapter *adapter, int address, int kind);
 static int asb100_detach_client(struct i2c_client *client);
 static struct asb100_data *asb100_update_device(struct device *dev);
@@ -220,8 +219,10 @@ static struct i2c_driver asb100_driver =
 	.owner		= THIS_MODULE,
 	.name		= "asb100",
 	.id		= I2C_DRIVERID_ASB100,
+	.class		= I2C_CLASS_HWMON,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= asb100_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= asb100_detect,
 	.detach_client	= asb100_detach_client,
 };
 
@@ -611,18 +612,6 @@ static DEVICE_ATTR(pwm1_enable, S_IRUGO 
 	device_create_file(&new_client->dev, &dev_attr_pwm1_enable); \
 } while (0)
 
-/* This function is called when:
-	asb100_driver is inserted (when this module is loaded), for each
-		available adapter
-	when a new adapter is inserted (and asb100_driver is still present)
- */
-static int asb100_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, asb100_detect);
-}
-
 static int asb100_detect_subclients(struct i2c_adapter *adapter, int address,
 		int kind, struct i2c_client *new_client)
 {
drivers/hwmon/atxp1.c +2 -7
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/atxp1.c
===================================================================
@@ -44,7 +44,6 @@ static unsigned short normal_i2c[] = { 0
 
 I2C_CLIENT_INSMOD_1(atxp1);
 
-static int atxp1_attach_adapter(struct i2c_adapter * adapter);
 static int atxp1_detach_client(struct i2c_client * client);
 static struct atxp1_data * atxp1_update_device(struct device *dev);
 static int atxp1_detect(struct i2c_adapter *adapter, int address, int kind);
@@ -53,7 +52,8 @@ static struct i2c_driver atxp1_driver = 
 	.owner		= THIS_MODULE,
 	.name		= "atxp1",
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter = atxp1_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= atxp1_detect,
 	.detach_client	= atxp1_detach_client,
 };
 
@@ -251,11 +251,6 @@ static ssize_t atxp1_storegpio2(struct d
 static DEVICE_ATTR(gpio2, S_IRUGO | S_IWUSR, atxp1_showgpio2, atxp1_storegpio2);
 
 
-static int atxp1_attach_adapter(struct i2c_adapter *adapter)
-{
-	return i2c_probe(adapter, &addr_data, &atxp1_detect);
-};
-
 static int atxp1_detect(struct i2c_adapter *adapter, int address, int kind)
 {
 	struct i2c_client * new_client;
drivers/hwmon/ds1621.c +2 -8
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/ds1621.c
===================================================================
@@ -80,7 +80,6 @@ struct ds1621_data {
 	u8 conf;			/* Register encoding, combined */
 };
 
-static int ds1621_attach_adapter(struct i2c_adapter *adapter);
 static int ds1621_detect(struct i2c_adapter *adapter, int address,
 			 int kind);
 static void ds1621_init_client(struct i2c_client *client);
@@ -93,7 +92,8 @@ static struct i2c_driver ds1621_driver =
 	.name		= "ds1621",
 	.id		= I2C_DRIVERID_DS1621,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= ds1621_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= ds1621_detect,
 	.detach_client	= ds1621_detach_client,
 };
 
@@ -178,12 +178,6 @@ static DEVICE_ATTR(temp1_min, S_IWUSR | 
 static DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp_max, set_temp_max);
 
 
-static int ds1621_attach_adapter(struct i2c_adapter *adapter)
-{
-	return i2c_probe(adapter, &addr_data, ds1621_detect);
-}
-
-/* This function is called by i2c_probe */
 int ds1621_detect(struct i2c_adapter *adapter, int address,
                   int kind)
 {
drivers/hwmon/fscher.c +3 -9
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/fscher.c
===================================================================
@@ -104,7 +104,6 @@ I2C_CLIENT_INSMOD_1(fscher);
  * Functions declaration
  */
 
-static int fscher_attach_adapter(struct i2c_adapter *adapter);
 static int fscher_detect(struct i2c_adapter *adapter, int address, int kind);
 static int fscher_detach_client(struct i2c_client *client);
 static struct fscher_data *fscher_update_device(struct device *dev);
@@ -121,8 +120,10 @@ static struct i2c_driver fscher_driver =
 	.owner		= THIS_MODULE,
 	.name		= "fscher",
 	.id		= I2C_DRIVERID_FSCHER,
+	.class		= I2C_CLASS_HWMON,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= fscher_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= fscher_detect,
 	.detach_client	= fscher_detach_client,
 };
 
@@ -284,13 +285,6 @@ do { \
  * Real code
  */
 
-static int fscher_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, fscher_detect);
-}
-
 static int fscher_detect(struct i2c_adapter *adapter, int address, int kind)
 {
 	struct i2c_client *new_client;
drivers/hwmon/fscpos.c +3 -9
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/fscpos.c
===================================================================
@@ -85,7 +85,6 @@ static u8 FSCPOS_REG_TEMP_STATE[] = { 0x
 /*
  * Functions declaration
  */
-static int fscpos_attach_adapter(struct i2c_adapter *adapter);
 static int fscpos_detect(struct i2c_adapter *adapter, int address, int kind);
 static int fscpos_detach_client(struct i2c_client *client);
 
@@ -103,8 +102,10 @@ static struct i2c_driver fscpos_driver =
 	.owner		= THIS_MODULE,
 	.name		= "fscpos",
 	.id		= I2C_DRIVERID_FSCPOS,
+	.class		= I2C_CLASS_HWMON,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= fscpos_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= fscpos_detect,
 	.detach_client	= fscpos_detach_client,
 };
 
@@ -431,13 +432,6 @@ static DEVICE_ATTR(in0_input, S_IRUGO, s
 static DEVICE_ATTR(in1_input, S_IRUGO, show_volt_5, NULL);
 static DEVICE_ATTR(in2_input, S_IRUGO, show_volt_batt, NULL);
 
-static int fscpos_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, fscpos_detect);
-}
-
 int fscpos_detect(struct i2c_adapter *adapter, int address, int kind)
 {
 	struct i2c_client *new_client;
drivers/hwmon/gl518sm.c +3 -9
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/gl518sm.c
===================================================================
@@ -141,7 +141,6 @@ struct gl518_data {
 	u8 beep_enable;		/* Boolean */
 };
 
-static int gl518_attach_adapter(struct i2c_adapter *adapter);
 static int gl518_detect(struct i2c_adapter *adapter, int address, int kind);
 static void gl518_init_client(struct i2c_client *client);
 static int gl518_detach_client(struct i2c_client *client);
@@ -154,8 +153,10 @@ static struct i2c_driver gl518_driver = 
 	.owner		= THIS_MODULE,
 	.name		= "gl518sm",
 	.id		= I2C_DRIVERID_GL518,
+	.class		= I2C_CLASS_HWMON,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= gl518_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= gl518_detect,
 	.detach_client	= gl518_detach_client,
 };
 
@@ -343,13 +344,6 @@ static DEVICE_ATTR(beep_mask, S_IWUSR|S_
  * Real code
  */
 
-static int gl518_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, gl518_detect);
-}
-
 static int gl518_detect(struct i2c_adapter *adapter, int address, int kind)
 {
 	int i;
drivers/hwmon/gl520sm.c +3 -9
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/gl520sm.c
===================================================================
@@ -99,7 +99,6 @@ That's why _TEMP2 and _IN4 access the sa
  * Function declarations
  */
 
-static int gl520_attach_adapter(struct i2c_adapter *adapter);
 static int gl520_detect(struct i2c_adapter *adapter, int address, int kind);
 static void gl520_init_client(struct i2c_client *client);
 static int gl520_detach_client(struct i2c_client *client);
@@ -112,8 +111,10 @@ static struct i2c_driver gl520_driver = 
 	.owner		= THIS_MODULE,
 	.name		= "gl520sm",
 	.id		= I2C_DRIVERID_GL520,
+	.class		= I2C_CLASS_HWMON,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= gl520_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= gl520_detect,
 	.detach_client	= gl520_detach_client,
 };
 
@@ -515,13 +516,6 @@ static ssize_t set_beep_mask(struct i2c_
  * Real code
  */
 
-static int gl520_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, gl520_detect);
-}
-
 static int gl520_detect(struct i2c_adapter *adapter, int address, int kind)
 {
 	struct i2c_client *new_client;
drivers/hwmon/lm92.c +3 -8
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/lm92.c
===================================================================
@@ -384,13 +384,6 @@ exit:
 	return err;
 }
 
-static int lm92_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, lm92_detect);
-}
-
 static int lm92_detach_client(struct i2c_client *client)
 {
 	struct lm92_data *data = i2c_get_clientdata(client);
@@ -414,8 +407,10 @@ static struct i2c_driver lm92_driver = {
 	.owner		= THIS_MODULE,
 	.name		= "lm92",
 	.id		= I2C_DRIVERID_LM92,
+	.class		= I2C_CLASS_HWMON,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= lm92_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= lm92_detect,
 	.detach_client	= lm92_detach_client,
 };
 
drivers/hwmon/it87.c +3 -14
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/it87.c
===================================================================
@@ -220,7 +220,6 @@ struct it87_data {
 };
 
 
-static int it87_attach_adapter(struct i2c_adapter *adapter);
 static int it87_isa_attach_adapter(struct i2c_adapter *adapter);
 static int it87_detect(struct i2c_adapter *adapter, int address, int kind);
 static int it87_detach_client(struct i2c_client *client);
@@ -237,8 +236,10 @@ static struct i2c_driver it87_driver = {
 	.owner		= THIS_MODULE,
 	.name		= "it87",
 	.id		= I2C_DRIVERID_IT87,
+	.class		= I2C_CLASS_HWMON,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= it87_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= it87_detect,
 	.detach_client	= it87_detach_client,
 };
 
@@ -689,17 +690,6 @@ static DEVICE_ATTR(cpu0_vid, S_IRUGO, sh
 #define device_create_file_vid(client) \
 device_create_file(&client->dev, &dev_attr_cpu0_vid)
 
-/* This function is called when:
-     * it87_driver is inserted (when this module is loaded), for each
-       available adapter
-     * when a new adapter is inserted (and it87_driver is still present) */
-static int it87_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, it87_detect);
-}
-
 static int it87_isa_attach_adapter(struct i2c_adapter *adapter)
 {
 	return it87_detect(adapter, isa_address, -1);
@@ -737,7 +727,6 @@ exit:
 	return err;
 }
 
-/* This function is called by i2c_probe */
 int it87_detect(struct i2c_adapter *adapter, int address, int kind)
 {
 	int i;
drivers/hwmon/lm63.c +3 -9
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/lm63.c
===================================================================
@@ -126,7 +126,6 @@ I2C_CLIENT_INSMOD_1(lm63);
  * Functions declaration
  */
 
-static int lm63_attach_adapter(struct i2c_adapter *adapter);
 static int lm63_detach_client(struct i2c_client *client);
 
 static struct lm63_data *lm63_update_device(struct device *dev);
@@ -141,8 +140,10 @@ static void lm63_init_client(struct i2c_
 static struct i2c_driver lm63_driver = {
 	.owner		= THIS_MODULE,
 	.name		= "lm63",
+	.class		= I2C_CLASS_HWMON,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= lm63_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= lm63_detect,
 	.detach_client	= lm63_detach_client,
 };
 
@@ -355,13 +356,6 @@ static DEVICE_ATTR(alarms, S_IRUGO, show
  * Real code
  */
 
-static int lm63_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, lm63_detect);
-}
-
 /*
  * The following function does more than just detection. If detection
  * succeeds, it also registers the new chip.
drivers/hwmon/lm75.c +3 -10
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/lm75.c
===================================================================
@@ -55,7 +55,6 @@ struct lm75_data {
 	u16			temp_hyst;
 };
 
-static int lm75_attach_adapter(struct i2c_adapter *adapter);
 static int lm75_detect(struct i2c_adapter *adapter, int address, int kind);
 static void lm75_init_client(struct i2c_client *client);
 static int lm75_detach_client(struct i2c_client *client);
@@ -69,8 +68,10 @@ static struct i2c_driver lm75_driver = {
 	.owner		= THIS_MODULE,
 	.name		= "lm75",
 	.id		= I2C_DRIVERID_LM75,
+	.class		= I2C_CLASS_HWMON,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= lm75_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= lm75_detect,
 	.detach_client	= lm75_detach_client,
 };
 
@@ -104,14 +105,6 @@ static DEVICE_ATTR(temp1_max, S_IWUSR | 
 static DEVICE_ATTR(temp1_max_hyst, S_IWUSR | S_IRUGO, show_temp_hyst, set_temp_hyst);
 static DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_input, NULL);
 
-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);
-}
-
-/* This function is called by i2c_probe */
 static int lm75_detect(struct i2c_adapter *adapter, int address, int kind)
 {
 	int i;
drivers/hwmon/lm77.c +3 -10
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/lm77.c
===================================================================
@@ -62,7 +62,6 @@ struct lm77_data {
 	u8			alarms;
 };
 
-static int lm77_attach_adapter(struct i2c_adapter *adapter);
 static int lm77_detect(struct i2c_adapter *adapter, int address, int kind);
 static void lm77_init_client(struct i2c_client *client);
 static int lm77_detach_client(struct i2c_client *client);
@@ -76,8 +75,10 @@ static struct lm77_data *lm77_update_dev
 static struct i2c_driver lm77_driver = {
 	.owner		= THIS_MODULE,
 	.name		= "lm77",
+	.class		= I2C_CLASS_HWMON,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter = lm77_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= lm77_detect,
 	.detach_client	= lm77_detach_client,
 };
 
@@ -204,14 +205,6 @@ static DEVICE_ATTR(temp1_max_hyst, S_IRU
 static DEVICE_ATTR(alarms, S_IRUGO,
 		   show_alarms, NULL);
 
-static int lm77_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, lm77_detect);
-}
-
-/* This function is called by i2c_probe */
 static int lm77_detect(struct i2c_adapter *adapter, int address, int kind)
 {
 	struct i2c_client *new_client;
drivers/hwmon/lm78.c +3 -14
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/lm78.c
===================================================================
@@ -152,7 +152,6 @@ struct lm78_data {
 };
 
 
-static int lm78_attach_adapter(struct i2c_adapter *adapter);
 static int lm78_isa_attach_adapter(struct i2c_adapter *adapter);
 static int lm78_detect(struct i2c_adapter *adapter, int address, int kind);
 static int lm78_detach_client(struct i2c_client *client);
@@ -167,8 +166,10 @@ static struct i2c_driver lm78_driver = {
 	.owner		= THIS_MODULE,
 	.name		= "lm78",
 	.id		= I2C_DRIVERID_LM78,
+	.class		= I2C_CLASS_HWMON,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= lm78_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= lm78_detect,
 	.detach_client	= lm78_detach_client,
 };
 
@@ -463,23 +464,11 @@ static ssize_t show_alarms(struct device
 }
 static DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL);
 
-/* This function is called when:
-     * lm78_driver is inserted (when this module is loaded), for each
-       available adapter
-     * when a new adapter is inserted (and lm78_driver is still present) */
-static int lm78_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, lm78_detect);
-}
-
 static int lm78_isa_attach_adapter(struct i2c_adapter *adapter)
 {
 	return lm78_detect(adapter, isa_address, -1);
 }
 
-/* This function is called by i2c_probe */
 int lm78_detect(struct i2c_adapter *adapter, int address, int kind)
 {
 	int i, err;
drivers/hwmon/lm80.c +3 -9
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/lm80.c
===================================================================
@@ -130,7 +130,6 @@ struct lm80_data {
  * Functions declaration
  */
 
-static int lm80_attach_adapter(struct i2c_adapter *adapter);
 static int lm80_detect(struct i2c_adapter *adapter, int address, int kind);
 static void lm80_init_client(struct i2c_client *client);
 static int lm80_detach_client(struct i2c_client *client);
@@ -146,8 +145,10 @@ static struct i2c_driver lm80_driver = {
 	.owner		= THIS_MODULE,
 	.name		= "lm80",
 	.id		= I2C_DRIVERID_LM80,
+	.class		= I2C_CLASS_HWMON,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= lm80_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= lm80_detect,
 	.detach_client	= lm80_detach_client,
 };
 
@@ -386,13 +387,6 @@ static DEVICE_ATTR(alarms, S_IRUGO, show
  * Real code
  */
 
-static int lm80_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, lm80_detect);
-}
-
 int lm80_detect(struct i2c_adapter *adapter, int address, int kind)
 {
 	int i, cur;
drivers/hwmon/lm83.c +3 -9
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/lm83.c
===================================================================
@@ -114,7 +114,6 @@ static const u8 LM83_REG_W_HIGH[] = {
  * Functions declaration
  */
 
-static int lm83_attach_adapter(struct i2c_adapter *adapter);
 static int lm83_detect(struct i2c_adapter *adapter, int address, int kind);
 static int lm83_detach_client(struct i2c_client *client);
 static struct lm83_data *lm83_update_device(struct device *dev);
@@ -127,8 +126,10 @@ static struct i2c_driver lm83_driver = {
 	.owner		= THIS_MODULE,
 	.name		= "lm83",
 	.id		= I2C_DRIVERID_LM83,
+	.class		= I2C_CLASS_HWMON,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= lm83_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= lm83_detect,
 	.detach_client	= lm83_detach_client,
 };
 
@@ -209,13 +210,6 @@ static DEVICE_ATTR(alarms, S_IRUGO, show
  * Real code
  */
 
-static int lm83_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, lm83_detect);
-}
-
 /*
  * The following function does more than just detection. If detection
  * succeeds, it also registers the new chip.
drivers/hwmon/lm85.c +3 -9
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/lm85.c
===================================================================
@@ -368,7 +368,6 @@ struct lm85_data {
 	struct lm85_zone zone[3];
 };
 
-static int lm85_attach_adapter(struct i2c_adapter *adapter);
 static int lm85_detect(struct i2c_adapter *adapter, int address,
 			int kind);
 static int lm85_detach_client(struct i2c_client *client);
@@ -383,8 +382,10 @@ static struct i2c_driver lm85_driver = {
 	.owner          = THIS_MODULE,
 	.name           = "lm85",
 	.id             = I2C_DRIVERID_LM85,
+	.class		= I2C_CLASS_HWMON,
 	.flags          = I2C_DF_NOTIFY,
-	.attach_adapter = lm85_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= lm85_detect,
 	.detach_client  = lm85_detach_client,
 };
 
@@ -1007,13 +1008,6 @@ temp_auto(1);
 temp_auto(2);
 temp_auto(3);
 
-int lm85_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, lm85_detect);
-}
-
 int lm85_detect(struct i2c_adapter *adapter, int address,
 		int kind)
 {
drivers/hwmon/lm87.c +3 -9
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/lm87.c
===================================================================
@@ -150,7 +150,6 @@ static u8 LM87_REG_TEMP_LOW[3] = { 0x3A,
  * Functions declaration
  */
 
-static int lm87_attach_adapter(struct i2c_adapter *adapter);
 static int lm87_detect(struct i2c_adapter *adapter, int address, int kind);
 static void lm87_init_client(struct i2c_client *client);
 static int lm87_detach_client(struct i2c_client *client);
@@ -164,8 +163,10 @@ static struct i2c_driver lm87_driver = {
 	.owner		= THIS_MODULE,
 	.name		= "lm87",
 	.id		= I2C_DRIVERID_LM87,
+	.class		= I2C_CLASS_HWMON,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= lm87_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= lm87_detect,
 	.detach_client	= lm87_detach_client,
 };
 
@@ -534,13 +535,6 @@ static DEVICE_ATTR(aout_output, S_IRUGO 
  * Real code
  */
 
-static int lm87_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, lm87_detect);
-}
-
 /*
  * The following function does more than just detection. If detection
  * succeeds, it also registers the new chip.
drivers/hwmon/lm90.c +3 -9
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/lm90.c
===================================================================
@@ -174,7 +174,6 @@ I2C_CLIENT_INSMOD_6(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);
 static void lm90_init_client(struct i2c_client *client);
@@ -189,8 +188,10 @@ static struct i2c_driver lm90_driver = {
 	.owner		= THIS_MODULE,
 	.name		= "lm90",
 	.id		= I2C_DRIVERID_LM90,
+	.class		= I2C_CLASS_HWMON,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= lm90_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= lm90_detect,
 	.detach_client	= lm90_detach_client,
 };
 
@@ -349,13 +350,6 @@ static DEVICE_ATTR(alarms, S_IRUGO, show
  * Real code
  */
 
-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.
drivers/hwmon/max1619.c +3 -9
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/max1619.c
===================================================================
@@ -78,7 +78,6 @@ I2C_CLIENT_INSMOD_1(max1619);
  * Functions declaration
  */
 
-static int max1619_attach_adapter(struct i2c_adapter *adapter);
 static int max1619_detect(struct i2c_adapter *adapter, int address,
 	int kind);
 static void max1619_init_client(struct i2c_client *client);
@@ -92,8 +91,10 @@ static struct max1619_data *max1619_upda
 static struct i2c_driver max1619_driver = {
 	.owner		= THIS_MODULE,
 	.name		= "max1619",
+	.class		= I2C_CLASS_HWMON,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= max1619_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= max1619_detect,
 	.detach_client	= max1619_detach_client,
 };
 
@@ -175,13 +176,6 @@ static DEVICE_ATTR(alarms, S_IRUGO, show
  * Real code
  */
 
-static int max1619_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, max1619_detect);
-}
-
 /*
  * The following function does more than just detection. If detection
  * succeeds, it also registers the new chip.
drivers/hwmon/w83781d.c +3 -14
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/w83781d.c
===================================================================
@@ -257,7 +257,6 @@ struct w83781d_data {
 	u8 vrm;
 };
 
-static int w83781d_attach_adapter(struct i2c_adapter *adapter);
 static int w83781d_isa_attach_adapter(struct i2c_adapter *adapter);
 static int w83781d_detect(struct i2c_adapter *adapter, int address, int kind);
 static int w83781d_detach_client(struct i2c_client *client);
@@ -272,8 +271,10 @@ static struct i2c_driver w83781d_driver 
 	.owner = THIS_MODULE,
 	.name = "w83781d",
 	.id = I2C_DRIVERID_W83781D,
+	.class = I2C_CLASS_HWMON,
 	.flags = I2C_DF_NOTIFY,
-	.attach_adapter = w83781d_attach_adapter,
+	.address_data = &addr_data,
+	.detect_client = w83781d_detect,
 	.detach_client = w83781d_detach_client,
 };
 
@@ -859,18 +860,6 @@ do { \
 device_create_file(&client->dev, &dev_attr_temp##offset##_type); \
 } while (0)
 
-/* This function is called when:
-     * w83781d_driver is inserted (when this module is loaded), for each
-       available adapter
-     * when a new adapter is inserted (and w83781d_driver is still present) */
-static int
-w83781d_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, w83781d_detect);
-}
-
 static int
 w83781d_isa_attach_adapter(struct i2c_adapter *adapter)
 {
drivers/hwmon/w83792d.c +3 -15
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/w83792d.c
===================================================================
@@ -300,7 +300,6 @@ struct w83792d_data {
 	u8 sf2_levels[3][4];	/* Smart FanII: Fan1,2,3 duty cycle levels */
 };
 
-static int w83792d_attach_adapter(struct i2c_adapter *adapter);
 static int w83792d_detect(struct i2c_adapter *adapter, int address, int kind);
 static int w83792d_detach_client(struct i2c_client *client);
 
@@ -318,8 +317,10 @@ static void w83792d_init_client(struct i
 static struct i2c_driver w83792d_driver = {
 	.owner = THIS_MODULE,
 	.name = "w83792d",
+	.class = I2C_CLASS_HWMON,
 	.flags = I2C_DF_NOTIFY,
-	.attach_adapter = w83792d_attach_adapter,
+	.address_data = &addr_data,
+	.detect_client = w83792d_detect,
 	.detach_client = w83792d_detach_client,
 };
 
@@ -1065,19 +1066,6 @@ device_create_file(&client->dev, \
 } while (0)
 
 
-/* This function is called when:
-     * w83792d_driver is inserted (when this module is loaded), for each
-       available adapter
-     * when a new adapter is inserted (and w83792d_driver is still present) */
-static int
-w83792d_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, w83792d_detect);
-}
-
-
 static int
 w83792d_create_subclient(struct i2c_adapter *adapter,
 				struct i2c_client *new_client, int addr,
drivers/hwmon/w83l785ts.c +3 -9
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/w83l785ts.c
===================================================================
@@ -79,7 +79,6 @@ I2C_CLIENT_INSMOD_1(w83l785ts);
  * Functions declaration
  */
 
-static int w83l785ts_attach_adapter(struct i2c_adapter *adapter);
 static int w83l785ts_detect(struct i2c_adapter *adapter, int address,
 	int kind);
 static int w83l785ts_detach_client(struct i2c_client *client);
@@ -94,8 +93,10 @@ static struct i2c_driver w83l785ts_drive
 	.owner		= THIS_MODULE,
 	.name		= "w83l785ts",
 	.id		= I2C_DRIVERID_W83L785TS,
+	.class		= I2C_CLASS_HWMON,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= w83l785ts_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= w83l785ts_detect,
 	.detach_client	= w83l785ts_detach_client,
 };
 
@@ -137,13 +138,6 @@ static DEVICE_ATTR(temp1_max, S_IRUGO, s
  * Real code
  */
 
-static int w83l785ts_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, w83l785ts_detect);
-}
-
 /*
  * The following function does more than just detection. If detection
  * succeeds, it also registers the new chip.
#6 lore raw

Re: [PATCH 2/5] remove attach_adapter from i2c hwmon drivers

Hi Nathan,

Quoted 71 lines

Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/atxp1.c
===================================================================

drivers/hwmon/atxp1.c +0 -1
@@ -44,7 +44,6 @@ static unsigned short normal_i2c[] = { 0
 
 I2C_CLIENT_INSMOD_1(atxp1);
 
-static int atxp1_attach_adapter(struct i2c_adapter * adapter);
 static int atxp1_detach_client(struct i2c_client * client);
 static struct atxp1_data * atxp1_update_device(struct device *dev);
 static int atxp1_detect(struct i2c_adapter *adapter, int address, int

kind);

drivers/hwmon/atxp1.c +2 -6
@@ -53,7 +52,8 @@ static struct i2c_driver atxp1_driver = 
 	.owner		= THIS_MODULE,
 	.name		= "atxp1",
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter = atxp1_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= atxp1_detect,
 	.detach_client	= atxp1_detach_client,
 };
 
@@ -251,11 +251,6 @@ static ssize_t atxp1_storegpio2(struct d
 static DEVICE_ATTR(gpio2, S_IRUGO | S_IWUSR, atxp1_showgpio2,
 atxp1_storegpio2);
 
 
-static int atxp1_attach_adapter(struct i2c_adapter *adapter)
-{
-	return i2c_probe(adapter, &addr_data, &atxp1_detect);
-};
-
 static int atxp1_detect(struct i2c_adapter *adapter, int address, int
 kind) {

struct i2c_client * new_client;
Index: linux-2.6.13-rc6+gregkh/drivers/hwmon/ds1621.c
===================================================================

drivers/hwmon/ds1621.c +2 -8
@@ -80,7 +80,6 @@ struct ds1621_data {
 	u8 conf;			/* Register encoding, combined */
 };
 
-static int ds1621_attach_adapter(struct i2c_adapter *adapter);
 static int ds1621_detect(struct i2c_adapter *adapter, int address,
 			 int kind);
 static void ds1621_init_client(struct i2c_client *client);
@@ -93,7 +92,8 @@ static struct i2c_driver ds1621_driver =
 	.name		= "ds1621",
 	.id		= I2C_DRIVERID_DS1621,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= ds1621_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= ds1621_detect,
 	.detach_client	= ds1621_detach_client,
 };
 
@@ -178,12 +178,6 @@ static DEVICE_ATTR(temp1_min, S_IWUSR | 
 static DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp_max,
 set_temp_max);
 
 
-static int ds1621_attach_adapter(struct i2c_adapter *adapter)
-{
-	return i2c_probe(adapter, &addr_data, ds1621_detect);
-}
-
-/* This function is called by i2c_probe */
 int ds1621_detect(struct i2c_adapter *adapter, int address,
                   int kind)
Quoted 1 line

{

I noticed that these two drivers do not properly define their .class to I2C_CLASS_HWMON. I understand that doing so doesn't belong to this patch, but having an additional patch to apply on top it fixing the problem would be great. Can you do that? Or I will, it doesn't really matter (the patch should be really straightforward now.)

Thanks,

Signature · 1 line
Jean Delvare
#7 lore raw

PATCH 3/5 remove attach_adapter from misc i2c chip drivers

In reply to Nathan Lutchansky

The attach_adapter callback of most misc i2c chip drivers can be removed, now that the i2c core will implicitly call i2c_probe if the address data is present in the i2c_driver structure.

 drivers/i2c/chips/ds1337.c  |    9 ++-------
 drivers/i2c/chips/ds1374.c  |    8 ++------
 drivers/i2c/chips/eeprom.c  |   10 ++--------
 drivers/i2c/chips/m41t00.c  |    9 ++-------
 drivers/i2c/chips/max6875.c |   10 ++--------
 drivers/i2c/chips/pca9539.c |   10 ++--------
 drivers/i2c/chips/pcf8574.c |   10 ++--------
 drivers/i2c/chips/pcf8591.c |   10 ++--------
 drivers/i2c/chips/rtc8564.c |    8 ++------
 9 files changed, 18 insertions(+), 66 deletions(-)
drivers/i2c/chips/ds1337.c +2 -7
Index: linux-2.6.13-rc6+gregkh/drivers/i2c/chips/ds1337.c
===================================================================
@@ -41,7 +41,6 @@ static unsigned short normal_i2c[] = { 0
 
 I2C_CLIENT_INSMOD_1(ds1337);
 
-static int ds1337_attach_adapter(struct i2c_adapter *adapter);
 static int ds1337_detect(struct i2c_adapter *adapter, int address, int kind);
 static void ds1337_init_client(struct i2c_client *client);
 static int ds1337_detach_client(struct i2c_client *client);
@@ -55,7 +54,8 @@ static struct i2c_driver ds1337_driver =
 	.owner		= THIS_MODULE,
 	.name		= "ds1337",
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= ds1337_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= ds1337_detect,
 	.detach_client	= ds1337_detach_client,
 	.command	= ds1337_command,
 };
@@ -223,11 +223,6 @@ int ds1337_do_command(int bus, int cmd, 
 	return -ENODEV;
 }
 
-static int ds1337_attach_adapter(struct i2c_adapter *adapter)
-{
-	return i2c_probe(adapter, &addr_data, ds1337_detect);
-}
-
 /*
  * The following function does more than just detection. If detection
  * succeeds, it also registers the new chip.
drivers/i2c/chips/ds1374.c +2 -6
Index: linux-2.6.13-rc6+gregkh/drivers/i2c/chips/ds1374.c
===================================================================
@@ -216,11 +216,6 @@ static int ds1374_probe(struct i2c_adapt
 	return 0;
 }
 
-static int ds1374_attach(struct i2c_adapter *adap)
-{
-	return i2c_probe(adap, &addr_data, ds1374_probe);
-}
-
 static int ds1374_detach(struct i2c_client *client)
 {
 	int rc;
@@ -237,7 +232,8 @@ static struct i2c_driver ds1374_driver =
 	.name = DS1374_DRV_NAME,
 	.id = I2C_DRIVERID_DS1374,
 	.flags = I2C_DF_NOTIFY,
-	.attach_adapter = ds1374_attach,
+	.address_data = &addr_data,
+	.detect_client = ds1374_probe,
 	.detach_client = ds1374_detach,
 };
 
drivers/i2c/chips/eeprom.c +2 -8
Index: linux-2.6.13-rc6+gregkh/drivers/i2c/chips/eeprom.c
===================================================================
@@ -62,7 +62,6 @@ struct eeprom_data {
 };
 
 
-static int eeprom_attach_adapter(struct i2c_adapter *adapter);
 static int eeprom_detect(struct i2c_adapter *adapter, int address, int kind);
 static int eeprom_detach_client(struct i2c_client *client);
 
@@ -72,7 +71,8 @@ static struct i2c_driver eeprom_driver =
 	.name		= "eeprom",
 	.id		= I2C_DRIVERID_EEPROM,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= eeprom_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= eeprom_detect,
 	.detach_client	= eeprom_detach_client,
 };
 
@@ -149,12 +149,6 @@ static struct bin_attribute eeprom_attr 
 	.read = eeprom_read,
 };
 
-static int eeprom_attach_adapter(struct i2c_adapter *adapter)
-{
-	return i2c_probe(adapter, &addr_data, eeprom_detect);
-}
-
-/* This function is called by i2c_probe */
 int eeprom_detect(struct i2c_adapter *adapter, int address, int kind)
 {
 	struct i2c_client *new_client;
drivers/i2c/chips/m41t00.c +2 -7
Index: linux-2.6.13-rc6+gregkh/drivers/i2c/chips/m41t00.c
===================================================================
@@ -195,12 +195,6 @@ m41t00_probe(struct i2c_adapter *adap, i
 }
 
 static int
-m41t00_attach(struct i2c_adapter *adap)
-{
-	return i2c_probe(adap, &addr_data, m41t00_probe);
-}
-
-static int
 m41t00_detach(struct i2c_client *client)
 {
 	int	rc;
@@ -217,7 +211,8 @@ static struct i2c_driver m41t00_driver =
 	.name		= M41T00_DRV_NAME,
 	.id		= I2C_DRIVERID_STM41T00,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= m41t00_attach,
+	.address_data	= &addr_data,
+	.detect_client	= m41t00_probe,
 	.detach_client	= m41t00_detach,
 };
 
drivers/i2c/chips/max6875.c +2 -8
Index: linux-2.6.13-rc6+gregkh/drivers/i2c/chips/max6875.c
===================================================================
@@ -61,7 +61,6 @@ struct max6875_data {
 	unsigned long		last_updated[USER_EEPROM_SLICES];
 };
 
-static int max6875_attach_adapter(struct i2c_adapter *adapter);
 static int max6875_detect(struct i2c_adapter *adapter, int address, int kind);
 static int max6875_detach_client(struct i2c_client *client);
 
@@ -70,7 +69,8 @@ static struct i2c_driver max6875_driver 
 	.owner		= THIS_MODULE,
 	.name		= "max6875",
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= max6875_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= max6875_detect,
 	.detach_client	= max6875_detach_client,
 };
 
@@ -158,12 +158,6 @@ static struct bin_attribute user_eeprom_
 	.read = max6875_read,
 };
 
-static int max6875_attach_adapter(struct i2c_adapter *adapter)
-{
-	return i2c_probe(adapter, &addr_data, max6875_detect);
-}
-
-/* This function is called by i2c_probe */
 static int max6875_detect(struct i2c_adapter *adapter, int address, int kind)
 {
 	struct i2c_client *real_client;
drivers/i2c/chips/pca9539.c +2 -8
Index: linux-2.6.13-rc6+gregkh/drivers/i2c/chips/pca9539.c
===================================================================
@@ -32,7 +32,6 @@ enum pca9539_cmd
 	PCA9539_DIRECTION_1	= 7,
 };
 
-static int pca9539_attach_adapter(struct i2c_adapter *adapter);
 static int pca9539_detect(struct i2c_adapter *adapter, int address, int kind);
 static int pca9539_detach_client(struct i2c_client *client);
 
@@ -41,7 +40,8 @@ static struct i2c_driver pca9539_driver 
 	.owner		= THIS_MODULE,
 	.name		= "pca9539",
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= pca9539_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= pca9539_detect,
 	.detach_client	= pca9539_detach_client,
 };
 
@@ -105,12 +105,6 @@ static struct attribute_group pca9539_de
 	.attrs = pca9539_attributes,
 };
 
-static int pca9539_attach_adapter(struct i2c_adapter *adapter)
-{
-	return i2c_probe(adapter, &addr_data, pca9539_detect);
-}
-
-/* This function is called by i2c_probe */
 static int pca9539_detect(struct i2c_adapter *adapter, int address, int kind)
 {
 	struct i2c_client *new_client;
drivers/i2c/chips/pcf8574.c +2 -8
Index: linux-2.6.13-rc6+gregkh/drivers/i2c/chips/pcf8574.c
===================================================================
@@ -58,7 +58,6 @@ struct pcf8574_data {
 	u8 write;			/* Remember last written value */
 };
 
-static int pcf8574_attach_adapter(struct i2c_adapter *adapter);
 static int pcf8574_detect(struct i2c_adapter *adapter, int address, int kind);
 static int pcf8574_detach_client(struct i2c_client *client);
 static void pcf8574_init_client(struct i2c_client *client);
@@ -69,7 +68,8 @@ static struct i2c_driver pcf8574_driver 
 	.name		= "pcf8574",
 	.id		= I2C_DRIVERID_PCF8574,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= pcf8574_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= pcf8574_detect,
 	.detach_client	= pcf8574_detach_client,
 };
 
@@ -109,12 +109,6 @@ static DEVICE_ATTR(write, S_IWUSR | S_IR
  * Real code
  */
 
-static int pcf8574_attach_adapter(struct i2c_adapter *adapter)
-{
-	return i2c_probe(adapter, &addr_data, pcf8574_detect);
-}
-
-/* This function is called by i2c_probe */
 int pcf8574_detect(struct i2c_adapter *adapter, int address, int kind)
 {
 	struct i2c_client *new_client;
drivers/i2c/chips/pcf8591.c +2 -8
Index: linux-2.6.13-rc6+gregkh/drivers/i2c/chips/pcf8591.c
===================================================================
@@ -80,7 +80,6 @@ struct pcf8591_data {
 	u8 aout;
 };
 
-static int pcf8591_attach_adapter(struct i2c_adapter *adapter);
 static int pcf8591_detect(struct i2c_adapter *adapter, int address, int kind);
 static int pcf8591_detach_client(struct i2c_client *client);
 static void pcf8591_init_client(struct i2c_client *client);
@@ -92,7 +91,8 @@ static struct i2c_driver pcf8591_driver 
 	.name		= "pcf8591",
 	.id		= I2C_DRIVERID_PCF8591,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter	= pcf8591_attach_adapter,
+	.address_data	= &addr_data,
+	.detect_client	= pcf8591_detect,
 	.detach_client	= pcf8591_detach_client,
 };
 
@@ -160,12 +160,6 @@ static DEVICE_ATTR(out0_enable, S_IWUSR 
 /*
  * Real code
  */
-static int pcf8591_attach_adapter(struct i2c_adapter *adapter)
-{
-	return i2c_probe(adapter, &addr_data, pcf8591_detect);
-}
-
-/* This function is called by i2c_probe */
 int pcf8591_detect(struct i2c_adapter *adapter, int address, int kind)
 {
 	struct i2c_client *new_client;
drivers/i2c/chips/rtc8564.c +2 -6
Index: linux-2.6.13-rc6+gregkh/drivers/i2c/chips/rtc8564.c
===================================================================
@@ -196,11 +196,6 @@ done:
 	return ret;
 }
 
-static int rtc8564_probe(struct i2c_adapter *adap)
-{
-	return i2c_probe(adap, &addr_data, rtc8564_attach);
-}
-
 static int rtc8564_detach(struct i2c_client *client)
 {
 	i2c_detach_client(client);
@@ -366,7 +361,8 @@ static struct i2c_driver rtc8564_driver 
 	.name		= "RTC8564",
 	.id		= I2C_DRIVERID_RTC8564,
 	.flags		= I2C_DF_NOTIFY,
-	.attach_adapter = rtc8564_probe,
+	.address_data	= &addr_data,
+	.detect_client	= rtc8564_attach,
 	.detach_client	= rtc8564_detach,
 	.command	= rtc8564_command
 };
#8 lore raw

PATCH 4/5 add i2c_probe_device and i2c_remove_device

In reply to Nathan Lutchansky

Add new i2c_probe_device and i2c_remove_device functions to explicitly instantiate new i2c clients by adapter, driver id and bus address.

These functions can be used for special-purpose adapters, such as those on TV tuner cards, where we generally know in advance what devices are attached. This is important in cases where the adapter does not support probing or when probing is potentially dangerous to the connected devices.

 drivers/i2c/i2c-core.c |   71 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c.h    |    6 ++++
 2 files changed, 77 insertions(+)
drivers/i2c/i2c-core.c +71 -0
Index: linux-2.6.13-rc6+gregkh/drivers/i2c/i2c-core.c
===================================================================
@@ -671,6 +671,75 @@ int i2c_control(struct i2c_client *clien
 }
 
 /* ----------------------------------------------------
+ * direct add/remove functions to avoid probing
+ * ----------------------------------------------------
+ */
+
+int i2c_probe_device(struct i2c_adapter *adapter, int driver_id,
+		     int addr, int kind)
+{
+	struct list_head   *item;
+	struct i2c_driver  *driver = NULL;
+
+	/* There's no way to probe addresses on this adapter... */
+	if (kind < 0 && !i2c_check_functionality(adapter,I2C_FUNC_SMBUS_QUICK))
+		return -EINVAL;
+
+	down(&core_lists);
+	list_for_each(item,&drivers) {
+		driver = list_entry(item, struct i2c_driver, list);
+		if (driver->id == driver_id)
+			break;
+	}
+	up(&core_lists);
+	if (!item)
+		return -ENOENT;
+
+	/* Already in use? */
+	if (i2c_check_addr(adapter, addr))
+		return -EBUSY;
+
+	/* Make sure there is something at this address, unless forced */
+	if (kind < 0) {
+		if (i2c_smbus_xfer(adapter, addr, 0, 0, 0,
+				   I2C_SMBUS_QUICK, NULL) < 0)
+			return -ENODEV;
+
+		/* prevent 24RF08 corruption */
+		if ((addr & ~0x0f) == 0x50)
+			i2c_smbus_xfer(adapter, addr, 0, 0, 0,
+				       I2C_SMBUS_QUICK, NULL);
+	}
+
+	return driver->detect_client(adapter, addr, kind);
+}
+
+int i2c_remove_device(struct i2c_adapter *adapter, int driver_id, int addr)
+{
+	struct list_head  *item, *_n;
+	struct i2c_client *client;
+	int res = 0;
+
+	down(&core_lists);
+
+	list_for_each_safe(item, _n, &adapter->clients) {
+		client = list_entry(item, struct i2c_client, list);
+		if (client->addr != addr || client->driver->id != driver_id)
+			continue;
+		if ((res=client->driver->detach_client(client)))
+			dev_err(&adapter->dev, "detach_client failed for "
+				"client [%s] at address 0x%02x\n",
+				client->name, client->addr);
+		goto out_unlock;
+	}
+	res = -ENOENT;
+
+ out_unlock:
+	up(&core_lists);
+	return res;
+}
+
+/* ----------------------------------------------------
  * the i2c address scanning function
  * Will not work for 10-bit addresses!
  * ----------------------------------------------------
@@ -1230,6 +1299,8 @@ EXPORT_SYMBOL(i2c_control);
 EXPORT_SYMBOL(i2c_transfer);
 EXPORT_SYMBOL(i2c_get_adapter);
 EXPORT_SYMBOL(i2c_put_adapter);
+EXPORT_SYMBOL(i2c_probe_device);
+EXPORT_SYMBOL(i2c_remove_device);
 EXPORT_SYMBOL(i2c_probe);
 
 EXPORT_SYMBOL(i2c_smbus_xfer);
include/linux/i2c.h +6 -0
Index: linux-2.6.13-rc6+gregkh/include/linux/i2c.h
===================================================================
@@ -359,6 +359,12 @@ extern void i2c_clients_command(struct i
    you can cheat by simply not registering. Not recommended, of course! */
 extern int i2c_check_addr (struct i2c_adapter *adapter, int addr);
 
+/* Direct add/remove functions to create or remove clients by address. */
+extern int i2c_probe_device(struct i2c_adapter *adapter, int driver_id,
+			    int addr, int kind);
+extern int i2c_remove_device(struct i2c_adapter *adapter, int driver_id,
+			     int addr);
+
 /* Detect function. It iterates over all possible addresses itself.
  * It will only call found_proc if some client is connected at the
  * specific address (unless a 'force' matched);
#9 lore raw

Re: [PATCH 4/5] add i2c_probe_device and i2c_remove_device

Hi Nathan,

Quoted 5 lines

These functions can be used for special-purpose adapters, such as those on TV tuner cards, where we generally know in advance what devices are attached. This is important in cases where the adapter does not support probing or when probing is potentially dangerous to the connected devices.

Do you know of any adapter actually not supporting the SMBus Quick command (which we use for probing)?

Quoted 9 lines
drivers/i2c/i2c-core.c +3 -0
@@ -671,6 +671,75 @@ int i2c_control(struct i2c_client *clien
 }
 
 /* ----------------------------------------------------
+ * direct add/remove functions to avoid probing
+ * ----------------------------------------------------
+ */

drivers/i2c/i2c-core.c int i2c_control(struct i2c_client *clien · line 686

Quoted 10 lines
drivers/i2c/i2c-core.c +10 -0
+
+int i2c_probe_device(struct i2c_adapter *adapter, int driver_id,
+		     int addr, int kind)
+{
+	struct list_head   *item;
+	struct i2c_driver  *driver = NULL;
+
+	/* There's no way to probe addresses on this adapter... */
+	if (kind < 0 && !i2c_check_functionality(adapter,I2C_FUNC_SMBUS_QUICK))
+		return -EINVAL;

Coding style please: one space after the comma.

drivers/i2c/i2c-core.c int i2c_control(struct i2c_client *clien · line 689

Quoted 3 lines
drivers/i2c/i2c-core.c +3 -0
+
+	down(&core_lists);
+	list_for_each(item,&drivers) {

Ditto.

Quoted 16 lines
drivers/i2c/i2c-core.c +16 -0
+		driver = list_entry(item, struct i2c_driver, list);
+		if (driver->id == driver_id)
+			break;
+	}
+	up(&core_lists);
+	if (!item)
+		return -ENOENT;
+
+	/* Already in use? */
+	if (i2c_check_addr(adapter, addr))
+		return -EBUSY;
+
+	/* Make sure there is something at this address, unless forced */
+	if (kind < 0) {
+		if (i2c_smbus_xfer(adapter, addr, 0, 0, 0,
+				   I2C_SMBUS_QUICK, NULL) < 0)

drivers/i2c/i2c-core.c int i2c_control(struct i2c_client *clien · line 715

Quoted 10 lines
drivers/i2c/i2c-core.c +10 -0
+			return -ENODEV;
+
+		/* prevent 24RF08 corruption */
+		if ((addr & ~0x0f) == 0x50)
+			i2c_smbus_xfer(adapter, addr, 0, 0, 0,
+				       I2C_SMBUS_QUICK, NULL);
+	}
+
+	return driver->detect_client(adapter, addr, kind);
+}

You are duplicating a part of i2c_probe_address() here. Why don't you simply call it?

This part of the code is very sensible because of the 24RF08 corruption issue. I have plans to change the probing method, e.g. by using SMBus Receive Byte instead of SMBus Quick for the 0x50-0x5F address range. Thus I would really appreciate if this code would not be duplicated.

Thanks,

Signature · 1 line
Jean Delvare
#10 lore raw

Re: [PATCH 4/5] add i2c_probe_device and i2c_remove_device

Quoted 8 lines · Jean Delvare
Quoted 5 lines

These functions can be used for special-purpose adapters, such as those on TV tuner cards, where we generally know in advance what devices are attached. This is important in cases where the adapter does not support probing or when probing is potentially dangerous to the connected devices.

Do you know of any adapter actually not supporting the SMBus Quick command (which we use for probing)?

I do, in fact, which is the reason I submitted these patches in the first place. :-)

The WIS GO7007, which is the MPEG1/2/4 video encoder used in the Plextor ConvertX, has an on-board i2c interface that supports nothing but 8-bit and 16-bit register reads and writes. Worse, it does not correctly report i2c errors. Even if it did support probing, though, I wouldn't want to use it because the i2c adapter generally lives on the other end of a USB and requires a minimum of 15 USB round trips per i2c transaction, so probing 124 i2c addresses would take a while.

Quoted 10 lines

drivers/i2c/i2c-core.c int i2c_control(struct i2c_client *clien · line 686

Quoted 2 lines
drivers/i2c/i2c-core.c +2 -0
+	if (kind < 0 && !i2c_check_functionality(adapter,I2C_FUNC_SMBUS_QUICK))
+		return -EINVAL;

Coding style please: one space after the comma.

drivers/i2c/i2c-core.c int i2c_control(struct i2c_client *clien · line 689

Quoted 3 lines
drivers/i2c/i2c-core.c +3 -0
+
+	down(&core_lists);
+	list_for_each(item,&drivers) {

Ditto.

Yeah, I copied those lines from other parts of i2c-core.c. But I'll fix them.

Quoted 21 lines
Quoted 21 lines
drivers/i2c/i2c-core.c +21 -0
+		driver = list_entry(item, struct i2c_driver, list);
+		if (driver->id == driver_id)
+			break;
+	}
+	up(&core_lists);
+	if (!item)
+		return -ENOENT;
+
+	/* Already in use? */
+	if (i2c_check_addr(adapter, addr))
+		return -EBUSY;
+
+	/* Make sure there is something at this address, unless forced */
+	if (kind < 0) {
+		if (i2c_smbus_xfer(adapter, addr, 0, 0, 0,
+				   I2C_SMBUS_QUICK, NULL) < 0)
+			return -ENODEV;
+
+		/* prevent 24RF08 corruption */
+		if ((addr & ~0x0f) == 0x50)
+			i2c_smbus_xfer(adapter, addr, 0, 0, 0,
Quoted 8 lines

drivers/i2c/i2c-core.c int i2c_control(struct i2c_client *clien · line 715

Quoted 5 lines
drivers/i2c/i2c-core.c +5 -0
+				       I2C_SMBUS_QUICK, NULL);
+	}
+
+	return driver->detect_client(adapter, addr, kind);
+}

You are duplicating a part of i2c_probe_address() here. Why don't you simply call it?

I could, I guess. Is there a reason i2c_probe_address is limited to 7-bit addresses? Clients with 10-bit addresses (I've never seen one, but I guess they exist?) should be able to be instantiated though the known-device list.

I think the way to go about this would be to rework and resubmit the first 3 patches once we decide how to handle the no-probe adapter flag, and later I will send a separate patch set for review with the known-device list implementation.

Thank you for all your help! -Nathan

#11 lore raw

Re: [PATCH 4/5] add i2c_probe_device and i2c_remove_device

Hi Nathan,

Quoted 9 lines
Quoted 2 lines

Do you know of any adapter actually not supporting the SMBus Quick command (which we use for probing)?

I do, in fact, which is the reason I submitted these patches in the first place. :-)

The WIS GO7007, which is the MPEG1/2/4 video encoder used in the Plextor ConvertX, has an on-board i2c interface that supports nothing but 8-bit and 16-bit register reads and writes.

Do you mean that it supports SMBus Read Byte but not SMBus Receive Byte?

I'm asking because, as said in a previous post, I am considering using SMBus Receive Byte as an alternative to SMBus Quick for bus probing where SMBus Quick is either unavailable or unsafe.

Quoted 1 line

Worse, it does not correctly report i2c errors.

Can you elaborate on this?

Quoted 4 lines

Even if it did support probing, though, I wouldn't want to use it
because the i2c adapter generally lives on the other end of a USB and requires a minimum of 15 USB round trips per i2c transaction, so probing 124 i2c addresses would take a while.

Why would it probe 124 addresses? Even in the current model this wouldn't happen. For every given driver, only listed addresses are probed. This is rarely more than 8 addresses. But still, the point is valid, if we do know what chips are on a given bus and at what addresses, probing could be skipped.

BTW, I'me just noticing that my reimplementation of i2c_probe() could be improved. It currently exists if SMBus Quick cannot be used. However, now that forced addresses are done first, this test could be moved until forced addresses have been processed:

 drivers/i2c/i2c-core.c |   15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)
drivers/i2c/i2c-core.c +11 -4
@@ -706,10 +706,6 @@
 	int i, err;
 	int adap_id = i2c_adapter_id(adapter);
 
-	/* Forget it if we can't probe using SMBUS_QUICK */
-	if (! i2c_check_functionality(adapter,I2C_FUNC_SMBUS_QUICK))
-		return -1;
-
 	/* Force entries are done first, and are not affected by ignore
 	   entries */
 	if (address_data->forces) {
@@ -736,6 +732,17 @@
 		}
 	}
 
+	/* Stop here if we can't use SMBUS_QUICK */
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_QUICK)) {
+		if (address_data->probe[0] == I2C_CLIENT_END
+		 && address_data->normal_i2c[0] == I2C_CLIENT_END)
+		 	return 0;
+
+		dev_warn(&adapter->dev, "SMBus Quick command not supported, "
+			 "can't probe for chips\n");
+		return -1;
+	}
+
 	/* Probe entries are done second, and are not affected by ignore
 	   entries either */
 	for (i = 0; address_data->probe[i] != I2C_CLIENT_END; i += 2) {

Would this help you? It looks to me like you could then call this function for your specific busses, with a properly crafted i2c_client_address_data structure (only force entries, no probe/normal_i2c entries). Or do you need more than that?

Quoted 4 lines
Quoted 1 line

Coding style please: one space after the comma.

(...)
Yeah, I copied those lines from other parts of i2c-core.c. But I'll fix them.

True, I know that coding style in the i2c-core is less than optimal at places :(

Quoted 4 lines

(...) Is there a reason i2c_probe_address is limited to 7-bit addresses? Clients with 10-bit addresses (I've never seen one, but I guess they exist?) should be able to be instantiated though the known-device list.

I've never seen 10-bit chips either. There is preliminary support in i2c-core as far as I can see, but I wouldn't expect too much from it, as it was probably never tested and might even not be complete enough to work at all.

10-bit addressing is an extension of the I2C protocol. It requires pushing one additional byte on the bus for each command, and is incompatible with SMBus 2.0 [1], which might explain why it ain't popular. Quite frankly, I can't see why any manufacturer would start using 10-bit addresses when I've never seen a crowded I2C bus. I think I remember that there is no theoretical limit to the number of chips on an I2C bus (other than the total number of addresses), but there are electrical conditions that must be fulfilled, making it clear that we aren't going to see I2C busses with more than 20 chips or so. It also looks like manufacturers are more likely to use multiplexing than 10-bit addresses if they really need to put many chips on a bus.

All in all, I don't see any reason why we would work on 10-bit address support until there is actually a need for this. There are much higher priorities at the moment.

[1] SMBus 2.0, specifications, page 25:
"All 10-bit slave addresses are reserved for future use and are outside the scope of this specification."

Quoted 4 lines

I think the way to go about this would be to rework and resubmit the first 3 patches once we decide how to handle the no-probe adapter flag, and later I will send a separate patch set for review with the known-device list implementation.

Sounds like a good plan to me.

Thanks,

Signature · 1 line
Jean Delvare
#12 lore raw

PATCH 5/5 new flag to disable i2c probing for an adapter

In reply to Nathan Lutchansky

Re-add the flags field to the i2c_adapter structure so we can add a new flag I2C_ADAPTER_SKIP_PROBE. When an adapter sets this flag, the bus will never be probed by any i2c client driver, even if forced addresses are listed. Adapters that set this flag will need to use the i2c_probe_device function to attach new clients.

 drivers/i2c/i2c-core.c |    3 +++
 include/linux/i2c.h    |    4 ++++
 2 files changed, 7 insertions(+)
drivers/i2c/i2c-core.c +3 -0
Index: linux-2.6.13-rc6+gregkh/drivers/i2c/i2c-core.c
===================================================================
@@ -788,6 +788,9 @@ int i2c_probe(struct i2c_adapter *adapte
 	int i, err;
 	int adap_id = i2c_adapter_id(adapter);
 
+	if (adapter->flags & I2C_ADAPTER_SKIP_PROBE)
+		return 0;
+
 	/* Forget it if we can't probe using SMBUS_QUICK */
 	if (! i2c_check_functionality(adapter,I2C_FUNC_SMBUS_QUICK))
 		return -1;
include/linux/i2c.h +4 -0
Index: linux-2.6.13-rc6+gregkh/include/linux/i2c.h
===================================================================
@@ -233,6 +233,7 @@ struct i2c_adapter {
 	unsigned int id;/* == is algo->id | hwdep.struct->id, 		*/
 			/* for registered values see below		*/
 	unsigned int class;
+	unsigned int flags;		/* div., see below		*/
 	struct i2c_algorithm *algo;/* the algorithm to access the bus	*/
 	void *algo_data;
 
@@ -290,6 +291,9 @@ static inline void i2c_set_adapdata (str
 #define I2C_CLIENT_TEN	0x10			/* we have a ten bit chip address	*/
 						/* Must equal I2C_M_TEN below */
 
+/*flags for the adapter struct: */
+#define I2C_ADAPTER_SKIP_PROBE		0x01	/* Don't probe for devices */
+
 /* i2c adapter classes (bitmask) */
 #define I2C_CLASS_HWMON		(1<<0)	/* lm_sensors, ... */
 #define I2C_CLASS_TV_ANALOG	(1<<1)	/* bttv + friends */
#13 lore raw
In reply to Nathan Lutchansky

Hi Nathan,

Quoted 6 lines

Virtually every i2c client driver uses exactly the same code, so there's little point in requiring them all to implement this callback. The first patch in this series adds two new fields to the i2c_driver structure, address_data' and detect_client', and if they are set by the driver, the i2c core will automatically call i2c_probe using those fields as the second and third argument.

This sounds very good to me. I have been wanting to do that some times ago, but for some reason didn't. Most probably I was waiting to be done with the various other i2c-core cleanups that were needed before (and this just happened.)

Quoted 2 lines

If the `class' field of the i2c_driver structure is set, it will be compared with the adapter class first.

I'm not sure I agree with the "if" clause, more about that below.

Quoted 3 lines

Patches 2 and 3 add these fields to the i2c_driver initializer in the i2c hwmon and misc i2c chip drivers and remove the corresponding attach_adapter callbacks.

Looks good to me too.

Quoted 9 lines

The second improvement (which is really the point of this patch set) is to add the functions i2c_probe_device and i2c_remove_device for directly creating and destroying i2c clients on a particular adapter:

int i2c_probe_device(struct i2c_adapter *adapter, int driver_id,
int addr, int kind);
int i2c_remove_device(struct i2c_adapter *adapter, int driver_id,
int addr);

Quoted 7 lines

These functions make the i2c subsystem usable for special-purpose i2c buses where probing isn't possible, either because probing is known to be dangerous for devices that are present on the bus, or because the i2c adapter lacks quick writes and/or error reporting.

The final patch adds a new i2c adapter flag to indicate that the adapter should never be probed.

I think I understand the point of i2c_probe_device(). However, it would help if you could additionally show how this is going to help the media/video drivers. Currently, all these drivers use the traditional probing mecanism, and have to jam "foreign" probes, right? I would hope that these two patches will make it possible to improve this. Can you provide a few examples of use? We need to figure out how good this new interface/mechanism is, and this can only be done with concrete examples.

I do not understand the point of i2c_remove_device() though. You shouldn't have to search for the clients you have been registering explicitely before. Instead, you should keep a reference to them (which suggests that maybe i2c_remove_device should return an i2c_client* instead of an int.) With that reference, you should be able to unregister the client directly, which is much more efficient that walking all the list of clients for that adapter.

I am although a bit surprised that you are passing the adapter as an i2c_adapter* and the driver as an id. Why not passing the driver as a i2c_driver*? Or both the adapter and the driver as ids? This is a truly naïve question, I just don't know. This is where concrete examples really help.

I am not totally convinced by the reintroduction of the i2c_adapter flags. I hope we can do without it.

One possibility would be to have an additional class of client, say I2C_CLASS_MISC. This would cover all the chip drivers which do not have a well-defined class, so that every client would *have to* define a class (we could enforce that at core level - I think this was the planned ultimate goal of .class when it was first introduced.) Adapters that do not want to be probed would then simply *not* define a class, so the class test would always fail for them, and no client could attach through the "automatic" probe mechanism.

If I2C_CLASS_MISC doesn't please you, then we can simply keep the idea that i2c_adapters that do not want to be probed do not define a class. This alone could be a condition to skip any probing to that adapters, we just have to make an explicit test on i2c_adapter.id not being 0 before going on with generic probing.

Another possibility, in case you really don't like neither the first one nor its variant, would be to keep two separate lists of i2c adapters, one with those which want probing, and one with those which don't. As it seems that these are two very different types of adapters, on which we will use completely different probing functions, this seems to make sense, and should speed up the processing on both sides. I admit I don't know if doing so would have implications at the i2c-core level. Maybe Greg can comment on this idea?

I have additional comments on the code itself, which I will post as replies to each patch.

Thanks,

Signature · 1 line
Jean Delvare
#14 lore raw
Quoted 10 lines · Jean Delvare
Quoted 9 lines

The second improvement (which is really the point of this patch set) is to add the functions i2c_probe_device and i2c_remove_device for directly creating and destroying i2c clients on a particular adapter:

int i2c_probe_device(struct i2c_adapter *adapter, int driver_id,
int addr, int kind);
int i2c_remove_device(struct i2c_adapter *adapter, int driver_id,
int addr);

Quoted 8 lines

I think I understand the point of i2c_probe_device(). However, it would help if you could additionally show how this is going to help the media/video drivers. Currently, all these drivers use the traditional probing mecanism, and have to jam "foreign" probes, right? I would hope that these two patches will make it possible to improve this. Can you provide a few examples of use? We need to figure out how good this new interface/mechanism is, and this can only be done with concrete examples.

OK, so I realized a few hours ago that the i2c_probe_device and i2c_remove_device interface is probably the wrong way to go about things, and it's broken anyway because the instantiated devices don't survive if the client driver module is unloaded and reloaded.

Here's what I'm after. Devices like video capture cards often have on-board i2c buses for controlling chips like the video decoder and TV tuner. The devices connected to these buses and their addresses can almost always be determined by the PCI/USB ID of the card or by reading an on-board EEPROM. With these special-purpose i2c buses, there's really no need to do any i2c probing, but we've always been forced to use probing anyway because that's the only way to instantiate new i2c clients. With the i2c_probe_device function I was attempting to provide a means for video capture card drivers to directly instantiate the i2c clients it already knows exist without having to probe for them. (The i2c_remove_device function was only present for symmetry...)

My new (well, old) idea for explicitly instantiating i2c clients, instead of i2c_probe_device, is to put a new field into the i2c_adapter with a list of (driver id, address, kind) tuples that should be force-detected by the i2c core.

1. Video capture card driver discovers new capture card
2. Driver creates new (unprobed) i2c adapter with this device list:

     {
         { I2C_DRIVERID_EEPROM, 0x50, 0 }
     }
 3. i2c core does no probing but force-detects an EEPROM at 0x50
 4. Driver reads EEPROM and updates the device list:
     {
         { I2C_DRIVERID_EEPROM, 0x50, 0 }
         { I2C_DRIVERID_SAA7115, 0x20, 0 }
         { I2C_DRIVERID_TUNER, 0x61, TUNER_PHILIPS_FM1236_MK3 }
     }

5. Driver somehow notifies the i2c core that the device list changed
6. i2c core force-detects the remaining two clients

I'd really like to model the known-device list after the PCI subsystem and other buses that track what devices "should" be connected based on configuration information from the bus's host adapter, but I haven't had time to research it yet.

Quoted 8 lines

I am not totally convinced by the reintroduction of the i2c_adapter flags. I hope we can do without it.

One possibility would be to have an additional class of client, say I2C_CLASS_MISC. This would cover all the chip drivers which do not have a well-defined class, so that every client would *have to* define a class (we could enforce that at core level - I think this was the planned ultimate goal of .class when it was first introduced.)

I'm not sure I like lumping all the "unclassified" clients together. The class mechanism is used to limit probing of an adapter to client drivers with similar functions, so putting a bunch of client drivers that would never be on the same bus into the same class is kind of silly. (You'd never expect to find a keyboard controller on the same bus as an IR motion sensor, for example.)

Quoted 2 lines

If I2C_CLASS_MISC doesn't please you, then we can simply keep the idea that i2c_adapters that do not want to be probed do not define a class.

I like this better. :-) This kind of implies, though, that i2c adapters that define *any* class would be probed by drivers that had no class defined. This might be the correct way to go about it. -Nathan

#15 lore raw

Hi Nathan,

Quoted 4 lines

OK, so I realized a few hours ago that the i2c_probe_device and i2c_remove_device interface is probably the wrong way to go about things, and it's broken anyway because the instantiated devices don't survive if the client driver module is unloaded and reloaded.

There is no way a client would survive a driver module cycling. This is the whole point of module cycling :) Clients have to be destroyed at the time the driver module is unloaded, as the driver holds all the client code. This will happen whatever the implementation, so I don't see what you think is wrong here.

Quoted 4 lines

(...) With the i2c_probe_device function I was
attempting to provide a means for video capture card drivers to directly instantiate the i2c clients it already knows exist without having to probe for them.

You don't need to convince me that i2c_probe_device() or something similar is a good idea :) Media/video and RTC driver authors have been complaining for quite some time about the lack of such a mechanism.

Quoted 1 line

(The i2c_remove_device function was only present for symmetry...)

Symmetry is not needed. If you look at the current code, you'll see that loading a client driver and unloading it are not similar operations.

Quoted 17 lines

My new (well, old) idea for explicitly instantiating i2c clients, instead of i2c_probe_device, is to put a new field into the i2c_adapter with a list of (driver id, address, kind) tuples that should be force-detected by the i2c core.

1. Video capture card driver discovers new capture card
2. Driver creates new (unprobed) i2c adapter with this device list:

     {
         { I2C_DRIVERID_EEPROM, 0x50, 0 }
     }
 3. i2c core does no probing but force-detects an EEPROM at 0x50
 4. Driver reads EEPROM and updates the device list:
     {
         { I2C_DRIVERID_EEPROM, 0x50, 0 }
         { I2C_DRIVERID_SAA7115, 0x20, 0 }
         { I2C_DRIVERID_TUNER, 0x61, TUNER_PHILIPS_FM1236_MK3 }
     }
Quoted 2 lines

5. Driver somehow notifies the i2c core that the device list changed
6. i2c core force-detects the remaining two clients

I don't much like it. You are trying to add to i2c_adapter something that doesn't belong to it IMHO. Why not simply pass the client force information to the probe function explicitely, like you originally planned? Sounds much more modular to me. I really don't get what problem you are trying to solve with this new proposal.

We most certainly won't get much further without real patches to illustrate the possibilities anyway.

Quoted 7 lines
Quoted 6 lines

One possibility would be to have an additional class of client, say I2C_CLASS_MISC. This would cover all the chip drivers which do not have a well-defined class, so that every client would *have to* define a class (we could enforce that at core level - I think this was the planned ultimate goal of .class when it was first introduced.)

Quoted 6 lines

I'm not sure I like lumping all the "unclassified" clients together. The class mechanism is used to limit probing of an adapter to client drivers with similar functions, so putting a bunch of client drivers that would never be on the same bus into the same class is kind of silly. (You'd never expect to find a keyboard controller on the same bus as an IR motion sensor, for example.)

This definitely wouldn't be worse than the current state where these drivers will probe for clients on all adapters ;)

However, you are right that I2C_CLASS_MISC is a bad idea. We can use I2C_CLASS_ALL for the exact same purpose, as described below.

Can we establish a list of these unclassified clients, and determine their needs? I do believe that for example RTC drivers do know exactly where their clients are, and would be fine with not probing for them, just like is the case of the media/video drivers. So far, we have been discussing about adapters that didn't want to be probed, but I now tend to think that this applies to client drivers as well. In other words, a client driver not defining a class would mean "never probe" rather than "probe all busses". This brings back some symmetry into the problem.

The benefit here would be that the class matching code would be reduced to its most simple expression, with no exceptions. Adapters and client drivers which are fine with the automatic probing mechanism would *have to* define their classes. Adapters and client drivers which do not want the i2c-core to handle client registrations for them simply do not define their class, and handle everything on their own (through attach_adapter().)

It is probably worth reminding the semantics for the i2c_adapter class. It doesn't define the kind of devices that can be found on this bus, but instead the kind of client devices the i2c-core is *allowed to probe for* on this bus. I hope that we agree on this. Likewise, the i2c_driver class is not about the kind of chip the driver is for, but about which busses this client driver wants to attach to in a probed, automated way.

Quoted 7 lines
Quoted 3 lines

If I2C_CLASS_MISC doesn't please you, then we can simply keep the idea that i2c_adapters that do not want to be probed do not define a class.

I like this better. :-) This kind of implies, though, that i2c adapters that define *any* class would be probed by drivers that had no class defined. This might be the correct way to go about it.

On second thought, I don't think so. The fact that adding a given class to an adapter would allow client drivers that do *not* define this class to probe the bus is a weird side effect, and hard to justify. But fortunately my new proposal doesn't suffer from this problem.

I propose that we go for the most simple and symmetrical model. i2c_add_driver and i2c_add_adapter call driver->attach_adapter() if it is defined, then call i2c_probe() if everything needed is defined and if (and only if) driver->class & adap->class is non-zero. Adapter or client drivers that really want to accept all clients or to connect to all adapters, resepectively, can use I2C_CLASS_ALL, although I wouldn't expect many to do. Note that this is only the expression of an acceptance, a client driver with I2C_CLASS_ALL will still not connect to an adapter with no class defined (and that's exactly what we want, isn't it?)

I don't think we need anything more complex than that, unless someone can point out a need that my proposal doesn't fulfill.

Of course, we will need your i2c_probe_device() function (or anything equivalent, but it looked just fine to me) for the non-probed client registrations.

Thanks,

Signature · 1 line
Jean Delvare
#16 lore raw
In reply to Nathan Lutchansky
Quoted 4 lines · Nathan Lutchansky

Hi everyone,

This patch series makes a couple of improvements to the i2c device probing process.

<snip>

These all generally look quite good, thanks. But it looks like you and Jean went back and forth on a few issues, care to repost an updated series of patches based on that exchange so I can have them get some testing in the -mm tree?

thanks,

greg k-h

#17 lore raw
Quoted 10 lines · Greg KH
Quoted 2 lines · Nathan Lutchansky

This patch series makes a couple of improvements to the i2c device probing process.

<snip>

These all generally look quite good, thanks. But it looks like you and Jean went back and forth on a few issues, care to repost an updated series of patches based on that exchange so I can have them get some testing in the -mm tree?

Yup, I'm going to break the series into two parts and post the first one probably tomorrow. I haven't been in a hurry since Jean's been away this week. -Nathan