[PATCH 0/3] Drop generic i2c chip driver module parameters

[PATCH 0/3] Drop generic i2c chip driver module parameters

[Jean Delvare]

Hi all,

Here comes the first part of a large patch series which aims at
dropping the generic i2c chip driver module parameters that were
inherited from the lm-sensors project: probe, ignore, force and
force_<type>. The same functionality is much better handled in a
centralized way than replicated across a hundred drivers, and the
recent changes to the binding model make it possible. Actually, the
replacement interface is already implemented: these are the sysfs
new_device and delete_device entries which are now created in each i2c
adapter directory. It might need some tweaking in the future, but the
core functionality is there already.

The second part will require changes to individual drivers. For
historical reasons, affected are mostly hwmon drivers: 44 out of
the 49 i2c-based hwmon drivers are affected, and only a few other
drivers are affected outside of the hwmon subsystem. Some statistics on
these drivers:
* Removes 15 lines of source code per driver on average.
* Removes 1.4 kB of binary module per driver on average (-7.7%).

The third part will happen later, when all individual drivers are
updated. These are interface cleanups which must be synchronized
across all affected drivers, so it's quite a pain to handle. I'll keep
them in my local tree for now, and push them upstream once all the rest
is there already.

In the end, this is 140 lines removed from <linux/i2c.h> (-19%).

Full combined diffstat:

 Documentation/i2c/old-module-parameters |   44 +++++
 Documentation/i2c/writing-clients       |    2 
 drivers/hwmon/adm1021.c                 |   87 +++--------
 drivers/hwmon/adm1025.c                 |   85 +++--------
 drivers/hwmon/adm1026.c                 |   58 +++----
 drivers/hwmon/adm1029.c                 |   71 ++-------
 drivers/hwmon/adm1031.c                 |   35 +---
 drivers/hwmon/adm9240.c                 |   70 +++------
 drivers/hwmon/ads7828.c                 |   35 +---
 drivers/hwmon/adt7462.c                 |   38 +----
 drivers/hwmon/adt7470.c                 |   38 +----
 drivers/hwmon/adt7473.c                 |   38 +----
 drivers/hwmon/adt7475.c                 |   22 +-
 drivers/hwmon/asb100.c                  |   66 ++------
 drivers/hwmon/atxp1.c                   |   11 -
 drivers/hwmon/dme1737.c                 |   42 +----
 drivers/hwmon/ds1621.c                  |   31 +---
 drivers/hwmon/f75375s.c                 |   40 +----
 drivers/hwmon/fschmd.c                  |   60 +++----
 drivers/hwmon/gl518sm.c                 |   40 +----
 drivers/hwmon/gl520sm.c                 |   25 +--
 drivers/hwmon/lm63.c                    |   69 +++------
 drivers/hwmon/lm75.c                    |   65 +++-----
 drivers/hwmon/lm77.c                    |   90 +++++------
 drivers/hwmon/lm78.c                    |   69 +++------
 drivers/hwmon/lm80.c                    |   13 -
 drivers/hwmon/lm83.c                    |   93 +++---------
 drivers/hwmon/lm85.c                    |  171 +++++++++-------------
 drivers/hwmon/lm87.c                    |   53 ++-----
 drivers/hwmon/lm90.c                    |  242 +++++++++++++-------------------
 drivers/hwmon/lm92.c                    |   40 +----
 drivers/hwmon/lm93.c                    |   40 +----
 drivers/hwmon/lm95241.c                 |   57 +------
 drivers/hwmon/max1619.c                 |   76 +++-------
 drivers/hwmon/max6650.c                 |   30 ---
 drivers/hwmon/pcf8591.c                 |    5 
 drivers/hwmon/smsc47m192.c              |   39 ++---
 drivers/hwmon/thmc50.c                  |   40 +----
 drivers/hwmon/tmp401.c                  |   64 +++-----
 drivers/hwmon/tmp421.c                  |   48 ++----
 drivers/hwmon/w83781d.c                 |  141 +++++++-----------
 drivers/hwmon/w83791d.c                 |   69 ++-------
 drivers/hwmon/w83792d.c                 |   70 ++-------
 drivers/hwmon/w83793.c                  |   62 ++------
 drivers/hwmon/w83l785ts.c               |   82 +++-------
 drivers/hwmon/w83l786ng.c               |   68 ++------
 drivers/i2c/i2c-core.c                  |  102 ++-----------
 drivers/misc/eeprom/eeprom.c            |    8 -
 drivers/misc/ics932s401.c               |   46 ++----
 include/linux/i2c.h                     |  146 -------------------
 50 files changed, 1082 insertions(+), 1954 deletions(-)

-- 
Jean Delvare

[PATCH 1/3] i2c: Drop probe, ignore and force module parameters

[Jean Delvare]

The legacy probe and force module parameters are obsolete now, the
same can be achieved using the new_device sysfs interface, which is
both more flexible and cheaper (it is implemented by i2c-core rather
than replicated in every driver module.)

The legacy ignore module parameters can be dropped as well. Ignoring
can be done by instantiating a "dummy" device at the problematic
address.

This is the first step of a huge cleanup to i2c-core's i2c_detect
function, i2c.h's I2C_CLIENT_INSMOD* macros, and all drivers that made
use of them.

Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
---
 Documentation/i2c/old-module-parameters |   44 ++++++++++++++
 drivers/i2c/i2c-core.c                  |   65 ----------------------
 include/linux/i2c.h                     |   91 -------------------------------
 3 files changed, 46 insertions(+), 154 deletions(-)

--- linux-2.6.32-rc3.orig/include/linux/i2c.h	2009-10-05 12:57:41.000000000 +0200
+++ linux-2.6.32-rc3/include/linux/i2c.h	2009-10-05 21:17:14.000000000 +0200
@@ -110,7 +110,7 @@ extern s32 i2c_smbus_write_i2c_block_dat
  * @driver: Device driver model driver
  * @id_table: List of I2C devices supported by this driver
  * @detect: Callback for device detection
- * @address_data: The I2C addresses to probe, ignore or force (for detect)
+ * @address_data: The I2C addresses to probe (for detect)
  * @clients: List of detected clients we created (for i2c-core use only)
  *
  * The driver.owner field should be set to the module owner of this driver.
@@ -379,9 +379,6 @@ static inline void i2c_set_adapdata(stru
  */
 struct i2c_client_address_data {
 	const unsigned short *normal_i2c;
-	const unsigned short *probe;
-	const unsigned short *ignore;
-	const unsigned short * const *forces;
 };
 
 /* Internal numbers to terminate lists */
@@ -595,134 +592,48 @@ union i2c_smbus_data {
   module_param_array(var, short, &var##_num, 0); \
   MODULE_PARM_DESC(var, desc)
 
-#define I2C_CLIENT_MODULE_PARM_FORCE(name)				\
-I2C_CLIENT_MODULE_PARM(force_##name,					\
-		       "List of adapter,address pairs which are "	\
-		       "unquestionably assumed to contain a `"		\
-		       # name "' chip")
-
-
 #define I2C_CLIENT_INSMOD_COMMON					\
-I2C_CLIENT_MODULE_PARM(probe, "List of adapter,address pairs to scan "	\
-		       "additionally");					\
-I2C_CLIENT_MODULE_PARM(ignore, "List of adapter,address pairs not to "	\
-		       "scan");						\
 static const struct i2c_client_address_data addr_data = {		\
 	.normal_i2c	= normal_i2c,					\
-	.probe		= probe,					\
-	.ignore		= ignore,					\
-	.forces		= forces,					\
 }
 
-#define I2C_CLIENT_FORCE_TEXT \
-	"List of adapter,address pairs to boldly assume to be present"
-
 /* These are the ones you want to use in your own drivers. Pick the one
    which matches the number of devices the driver differenciates between. */
 #define I2C_CLIENT_INSMOD						\
-I2C_CLIENT_MODULE_PARM(force, I2C_CLIENT_FORCE_TEXT);			\
-static const unsigned short * const forces[] = { force, NULL };		\
 I2C_CLIENT_INSMOD_COMMON
 
 #define I2C_CLIENT_INSMOD_1(chip1)					\
 enum chips { any_chip, chip1 };						\
-I2C_CLIENT_MODULE_PARM(force, I2C_CLIENT_FORCE_TEXT);			\
-I2C_CLIENT_MODULE_PARM_FORCE(chip1);					\
-static const unsigned short * const forces[] =	{ force,		\
-	force_##chip1, NULL };						\
 I2C_CLIENT_INSMOD_COMMON
 
 #define I2C_CLIENT_INSMOD_2(chip1, chip2)				\
 enum chips { any_chip, chip1, chip2 };					\
-I2C_CLIENT_MODULE_PARM(force, I2C_CLIENT_FORCE_TEXT);			\
-I2C_CLIENT_MODULE_PARM_FORCE(chip1);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip2);					\
-static const unsigned short * const forces[] =	{ force,		\
-	force_##chip1, force_##chip2, NULL };				\
 I2C_CLIENT_INSMOD_COMMON
 
 #define I2C_CLIENT_INSMOD_3(chip1, chip2, chip3)			\
 enum chips { any_chip, chip1, chip2, chip3 };				\
-I2C_CLIENT_MODULE_PARM(force, I2C_CLIENT_FORCE_TEXT);			\
-I2C_CLIENT_MODULE_PARM_FORCE(chip1);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip2);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip3);					\
-static const unsigned short * const forces[] =	{ force,		\
-	force_##chip1, force_##chip2, force_##chip3, NULL };		\
 I2C_CLIENT_INSMOD_COMMON
 
 #define I2C_CLIENT_INSMOD_4(chip1, chip2, chip3, chip4)			\
 enum chips { any_chip, chip1, chip2, chip3, chip4 };			\
-I2C_CLIENT_MODULE_PARM(force, I2C_CLIENT_FORCE_TEXT);			\
-I2C_CLIENT_MODULE_PARM_FORCE(chip1);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip2);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip3);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip4);					\
-static const unsigned short * const forces[] =	{ force,		\
-	force_##chip1, force_##chip2, force_##chip3,			\
-	force_##chip4, NULL};						\
 I2C_CLIENT_INSMOD_COMMON
 
 #define I2C_CLIENT_INSMOD_5(chip1, chip2, chip3, chip4, chip5)		\
 enum chips { any_chip, chip1, chip2, chip3, chip4, chip5 };		\
-I2C_CLIENT_MODULE_PARM(force, I2C_CLIENT_FORCE_TEXT);			\
-I2C_CLIENT_MODULE_PARM_FORCE(chip1);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip2);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip3);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip4);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip5);					\
-static const unsigned short * const forces[] = { force,			\
-	force_##chip1, force_##chip2, force_##chip3,			\
-	force_##chip4, force_##chip5, NULL };				\
 I2C_CLIENT_INSMOD_COMMON
 
 #define I2C_CLIENT_INSMOD_6(chip1, chip2, chip3, chip4, chip5, chip6)	\
 enum chips { any_chip, chip1, chip2, chip3, chip4, chip5, chip6 };	\
-I2C_CLIENT_MODULE_PARM(force, I2C_CLIENT_FORCE_TEXT);			\
-I2C_CLIENT_MODULE_PARM_FORCE(chip1);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip2);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip3);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip4);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip5);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip6);					\
-static const unsigned short * const forces[] = { force,			\
-	force_##chip1, force_##chip2, force_##chip3,			\
-	force_##chip4, force_##chip5, force_##chip6, NULL };		\
 I2C_CLIENT_INSMOD_COMMON
 
 #define I2C_CLIENT_INSMOD_7(chip1, chip2, chip3, chip4, chip5, chip6, chip7) \
 enum chips { any_chip, chip1, chip2, chip3, chip4, chip5, chip6,	\
 	     chip7 };							\
-I2C_CLIENT_MODULE_PARM(force, I2C_CLIENT_FORCE_TEXT);			\
-I2C_CLIENT_MODULE_PARM_FORCE(chip1);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip2);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip3);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip4);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip5);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip6);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip7);					\
-static const unsigned short * const forces[] = { force,			\
-	force_##chip1, force_##chip2, force_##chip3,			\
-	force_##chip4, force_##chip5, force_##chip6,			\
-	force_##chip7, NULL };						\
 I2C_CLIENT_INSMOD_COMMON
 
 #define I2C_CLIENT_INSMOD_8(chip1, chip2, chip3, chip4, chip5, chip6, chip7, chip8) \
 enum chips { any_chip, chip1, chip2, chip3, chip4, chip5, chip6,	\
 	     chip7, chip8 };						\
-I2C_CLIENT_MODULE_PARM(force, I2C_CLIENT_FORCE_TEXT);			\
-I2C_CLIENT_MODULE_PARM_FORCE(chip1);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip2);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip3);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip4);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip5);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip6);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip7);					\
-I2C_CLIENT_MODULE_PARM_FORCE(chip8);					\
-static const unsigned short * const forces[] = { force,			\
-	force_##chip1, force_##chip2, force_##chip3,			\
-	force_##chip4, force_##chip5, force_##chip6,			\
-	force_##chip7, force_##chip8, NULL };				\
 I2C_CLIENT_INSMOD_COMMON
 #endif /* __KERNEL__ */
 #endif /* _LINUX_I2C_H */
--- linux-2.6.32-rc3.orig/drivers/i2c/i2c-core.c	2009-10-05 12:57:41.000000000 +0200
+++ linux-2.6.32-rc3/drivers/i2c/i2c-core.c	2009-10-05 21:17:14.000000000 +0200
@@ -1248,40 +1248,13 @@ static int i2c_detect(struct i2c_adapter
 		return -ENOMEM;
 	temp_client->adapter = adapter;
 
-	/* Force entries are done first, and are not affected by ignore
-	   entries */
-	if (address_data->forces) {
-		const unsigned short * const *forces = address_data->forces;
-		int kind;
-
-		for (kind = 0; forces[kind]; kind++) {
-			for (i = 0; forces[kind][i] != I2C_CLIENT_END;
-			     i += 2) {
-				if (forces[kind][i] == adap_id
-				 || forces[kind][i] == ANY_I2C_BUS) {
-					dev_dbg(&adapter->dev, "found force "
-						"parameter for adapter %d, "
-						"addr 0x%02x, kind %d\n",
-						adap_id, forces[kind][i + 1],
-						kind);
-					temp_client->addr = forces[kind][i + 1];
-					err = i2c_detect_address(temp_client,
-						kind, driver);
-					if (err)
-						goto exit_free;
-				}
-			}
-		}
-	}
-
 	/* Stop here if the classes do not match */
 	if (!(adapter->class & driver->class))
 		goto exit_free;
 
 	/* 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)
+		if (address_data->normal_i2c[0] == I2C_CLIENT_END)
 			goto exit_free;
 
 		dev_warn(&adapter->dev, "SMBus Quick command not supported, "
@@ -1290,43 +1263,7 @@ static int i2c_detect(struct i2c_adapter
 		goto exit_free;
 	}
 
-	/* 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) {
-		if (address_data->probe[i] == adap_id
-		 || address_data->probe[i] == ANY_I2C_BUS) {
-			dev_dbg(&adapter->dev, "found probe parameter for "
-				"adapter %d, addr 0x%02x\n", adap_id,
-				address_data->probe[i + 1]);
-			temp_client->addr = address_data->probe[i + 1];
-			err = i2c_detect_address(temp_client, -1, driver);
-			if (err)
-				goto exit_free;
-		}
-	}
-
-	/* Normal entries are done last, unless shadowed by an ignore entry */
 	for (i = 0; address_data->normal_i2c[i] != I2C_CLIENT_END; i += 1) {
-		int j, ignore;
-
-		ignore = 0;
-		for (j = 0; address_data->ignore[j] != I2C_CLIENT_END;
-		     j += 2) {
-			if ((address_data->ignore[j] == adap_id ||
-			     address_data->ignore[j] == ANY_I2C_BUS)
-			 && address_data->ignore[j + 1]
-			    == address_data->normal_i2c[i]) {
-				dev_dbg(&adapter->dev, "found ignore "
-					"parameter for adapter %d, "
-					"addr 0x%02x\n", adap_id,
-					address_data->ignore[j + 1]);
-				ignore = 1;
-				break;
-			}
-		}
-		if (ignore)
-			continue;
-
 		dev_dbg(&adapter->dev, "found normal entry for adapter %d, "
 			"addr 0x%02x\n", adap_id,
 			address_data->normal_i2c[i]);
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.32-rc3/Documentation/i2c/old-module-parameters	2009-10-05 21:17:14.000000000 +0200
@@ -0,0 +1,44 @@
+I2C device driver binding control from user-space
+=================================================
+
+Up to kernel 2.6.32, many i2c drivers used helper macros provided by
+<linux/i2c.h> which created standard module parameters to let the user
+control how the driver would probe i2c buses and attach to devices. These
+parameters were known as "probe" (to let the driver probe for an extra
+address), "force" (to forcibly attach the driver to a given device) and
+"ignore" (to prevent a driver from probing a given address).
+
+With the conversion of the i2c subsystem to the standard device driver
+binding model, it became clear that these per-module parameters were no
+longer needed, and that a centralized implementation was possible. The new,
+sysfs-based interface is described in the documentation file
+"instantiating-devices", section "Method 4: Instantiate from user-space".
+
+Below is a mapping from the old module parameters to the new interface.
+
+Attaching a driver to an I2C device
+-----------------------------------
+
+Old method (module parameters):
+# modprobe <driver> probe=1,0x2d
+# modprobe <driver> force=1,0x2d
+# modprobe <driver> force_<device>=1,0x2d
+
+New method (sysfs interface):
+# echo <device> 0x2d > /sys/bus/i2c/devices/i2c-1/new_device
+
+Preventing a driver from attaching to an I2C device
+---------------------------------------------------
+
+Old method (module parameters):
+# modprobe <driver> ignore=1,0x2f
+
+New method (sysfs interface):
+# echo dummy 0x2f > /sys/bus/i2c/devices/i2c-1/new_device
+# modprobe <driver>
+
+Of course, it is important to instantiate the "dummy" device before loading
+the driver. The dummy device will be handled by i2c-core itself, preventing
+other drivers from binding to it later on. If there is a real device at the
+problematic address, and you want another driver to bind to it, then simply
+pass the name of the device in question instead of "dummy".

-- 
Jean Delvare

[PATCH 2/3] i2c: Simplify i2c_detect_address

[Jean Delvare]

The kind parameter of i2c_detect_address() always has value -1, so we
can get rid of it.

Next step is to update all i2c detect callback functions to get rid of
this now useless parameter.

Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
---
 drivers/i2c/i2c-core.c |   24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

--- linux-2.6.32-rc3.orig/drivers/i2c/i2c-core.c	2009-10-05 21:17:14.000000000 +0200
+++ linux-2.6.32-rc3/drivers/i2c/i2c-core.c	2009-10-06 09:55:31.000000000 +0200
@@ -1169,7 +1169,7 @@ EXPORT_SYMBOL(i2c_master_recv);
  * ----------------------------------------------------
  */
 
-static int i2c_detect_address(struct i2c_client *temp_client, int kind,
+static int i2c_detect_address(struct i2c_client *temp_client,
 			      struct i2c_driver *driver)
 {
 	struct i2c_board_info info;
@@ -1188,22 +1188,18 @@ static int i2c_detect_address(struct i2c
 	if (i2c_check_addr(adapter, addr))
 		return 0;
 
-	/* 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 0;
-
-		/* prevent 24RF08 corruption */
-		if ((addr & ~0x0f) == 0x50)
-			i2c_smbus_xfer(adapter, addr, 0, 0, 0,
-				       I2C_SMBUS_QUICK, NULL);
-	}
+	/* Make sure there is something at this address */
+	if (i2c_smbus_xfer(adapter, addr, 0, 0, 0, I2C_SMBUS_QUICK, NULL) < 0)
+		return 0;
+
+	/* Prevent 24RF08 corruption */
+	if ((addr & ~0x0f) == 0x50)
+		i2c_smbus_xfer(adapter, addr, 0, 0, 0, I2C_SMBUS_QUICK, NULL);
 
 	/* Finally call the custom detection function */
 	memset(&info, 0, sizeof(struct i2c_board_info));
 	info.addr = addr;
-	err = driver->detect(temp_client, kind, &info);
+	err = driver->detect(temp_client, -1, &info);
 	if (err) {
 		/* -ENODEV is returned if the detection fails. We catch it
 		   here as this isn't an error. */
@@ -1268,7 +1264,7 @@ static int i2c_detect(struct i2c_adapter
 			"addr 0x%02x\n", adap_id,
 			address_data->normal_i2c[i]);
 		temp_client->addr = address_data->normal_i2c[i];
-		err = i2c_detect_address(temp_client, -1, driver);
+		err = i2c_detect_address(temp_client, driver);
 		if (err)
 			goto exit_free;
 	}

-- 
Jean Delvare

[PATCH 3/3] ics932s401: Clean up detect function

[Jean Delvare]

As kind is now hard-coded to -1, there is room for code clean-ups.

Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Cc: "Darrick J. Wong" <djwong-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
---
 drivers/misc/ics932s401.c |   35 ++++++++++++++---------------------
 1 file changed, 14 insertions(+), 21 deletions(-)

--- linux-2.6.32-rc3.orig/drivers/misc/ics932s401.c	2009-10-06 10:33:15.000000000 +0200
+++ linux-2.6.32-rc3/drivers/misc/ics932s401.c	2009-10-06 10:34:10.000000000 +0200
@@ -417,32 +417,25 @@ static int ics932s401_detect(struct i2c_
 			  struct i2c_board_info *info)
 {
 	struct i2c_adapter *adapter = client->adapter;
+	int vendor, device, revision;
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
 		return -ENODEV;
 
-	if (kind <= 0) {
-		int vendor, device, revision;
+	vendor = i2c_smbus_read_word_data(client, ICS932S401_REG_VENDOR_REV);
+	vendor >>= 8;
+	revision = vendor >> ICS932S401_REV_SHIFT;
+	vendor &= ICS932S401_VENDOR_MASK;
+	if (vendor != ICS932S401_VENDOR)
+		return -ENODEV;
+
+	device = i2c_smbus_read_word_data(client, ICS932S401_REG_DEVICE);
+	device >>= 8;
+	if (device != ICS932S401_DEVICE)
+		return -ENODEV;
 
-		vendor = i2c_smbus_read_word_data(client,
-						  ICS932S401_REG_VENDOR_REV);
-		vendor >>= 8;
-		revision = vendor >> ICS932S401_REV_SHIFT;
-		vendor &= ICS932S401_VENDOR_MASK;
-		if (vendor != ICS932S401_VENDOR)
-			return -ENODEV;
-
-		device = i2c_smbus_read_word_data(client,
-						  ICS932S401_REG_DEVICE);
-		device >>= 8;
-		if (device != ICS932S401_DEVICE)
-			return -ENODEV;
-
-		if (revision != ICS932S401_REV)
-			dev_info(&adapter->dev, "Unknown revision %d\n",
-				 revision);
-	} else
-		dev_dbg(&adapter->dev, "detection forced\n");
+	if (revision != ICS932S401_REV)
+		dev_info(&adapter->dev, "Unknown revision %d\n", revision);
 
 	strlcpy(info->type, "ics932s401", I2C_NAME_SIZE);
 

-- 
Jean Delvare

Re: [PATCH 3/3] ics932s401: Clean up detect function

[Darrick J. Wong]

On Tue, Oct 06, 2009 at 06:06:23PM +0200, Jean Delvare wrote:
> As kind is now hard-coded to -1, there is room for code clean-ups.
> 
> Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> Cc: "Darrick J. Wong" <djwong-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

Based on some light testing, it looks like this is ok, so
Acked-by: "Darrick J. Wong" <djwong-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

--D