Skip to contentSkip to conversation

sensor chip driver refactoring

PATCH New Mark M. Hoffman 3 participants 3 messages May 19, 2005

lore
Share & tools
lore

Mark M. Hoffman

author
#0 lore raw

PATCH 1/1 sensor chip driver refactoring

Hi Greg:

I think this patch will apply (without fuzz) to your current I2C tree. If it doesn't, please post a plain diff of your tree and I'll redo.

* * * * *

This patch is a refactoring of some common code among all sensors chip drivers (except asb100, which was written this way to begin with.) It saves a handful of lines and ~100-300 bytes per module. It compiles ok. I've only tested it against one of the drivers, but the changes are similar across the board and quite mechanical. Please apply.

drivers/i2c/chips/adm1021.c +13 -18
diff -Nru linux-2.6.4-rc3-gkh/drivers/i2c/chips/adm1021.c linux-2.6.4-rc3-mmh/drivers/i2c/chips/adm1021.c
@@ -139,7 +139,7 @@
 static int adm1021_read_value(struct i2c_client *client, u8 reg);
 static int adm1021_write_value(struct i2c_client *client, u8 reg,
 			       u16 value);
-static void adm1021_update_client(struct i2c_client *client);
+static struct adm1021_data *adm1021_update_device(struct device *dev);
 
 /* (amalysh) read only mode, otherwise any limit's writing confuse BIOS */
 static int read_only = 0;

@@ -161,15 +161,10 @@
 static int adm1021_id = 0;
 
 #define show(value)	\
-static ssize_t show_##value(struct device *dev, char *buf)	\
-{								\
-	struct i2c_client *client = to_i2c_client(dev);		\
-	struct adm1021_data *data = i2c_get_clientdata(client);	\
-	int temp;						\
-								\
-	adm1021_update_client(client);				\
-	temp = TEMP_FROM_REG(data->value);			\
-	return sprintf(buf, "%d\n", temp);			\
+static ssize_t show_##value(struct device *dev, char *buf)		\
+{									\
+	struct adm1021_data *data = adm1021_update_device(dev);		\
+	return sprintf(buf, "%d\n", TEMP_FROM_REG(data->value));	\
 }
 show(temp_max);
 show(temp_hyst);

@@ -179,13 +174,10 @@
 show(remote_temp_input);
 
 #define show2(value)	\
-static ssize_t show_##value(struct device *dev, char *buf)	\
-{								\
-	struct i2c_client *client = to_i2c_client(dev);		\
-	struct adm1021_data *data = i2c_get_clientdata(client);	\
-								\
-	adm1021_update_client(client);				\
-	return sprintf(buf, "%d\n", data->value);		\
+static ssize_t show_##value(struct device *dev, char *buf)		\
+{									\
+	struct adm1021_data *data = adm1021_update_device(dev);		\
+	return sprintf(buf, "%d\n", data->value);			\
 }
 show2(alarms);
 show2(die_code);

@@ -393,8 +385,9 @@
 	return 0;
 }
 
-static void adm1021_update_client(struct i2c_client *client)
+static struct adm1021_data *adm1021_update_device(struct device *dev)
 {
+	struct i2c_client *client = to_i2c_client(dev);
 	struct adm1021_data *data = i2c_get_clientdata(client);
 
 	down(&data->update_lock);

@@ -424,6 +417,8 @@
 	}
 
 	up(&data->update_lock);
+
+	return data;
 }
 
 static int __init sensors_adm1021_init(void)
drivers/i2c/chips/fscher.c +6 -5
diff -Nru linux-2.6.4-rc3-gkh/drivers/i2c/chips/fscher.c linux-2.6.4-rc3-mmh/drivers/i2c/chips/fscher.c
@@ -113,7 +113,7 @@
 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 void fscher_update_client(struct i2c_client *client);
+static struct fscher_data *fscher_update_device(struct device *dev);
 static void fscher_init_client(struct i2c_client *client);
 
 static int fscher_read_value(struct i2c_client *client, u8 reg);

@@ -170,9 +170,7 @@
 static ssize_t show_##kind##offset##sub (struct device *, char *); \
 static ssize_t show_##kind##offset##sub (struct device *dev, char *buf) \
 { \
-	struct i2c_client *client = to_i2c_client(dev); \
-	struct fscher_data *data = i2c_get_clientdata(client); \
-	fscher_update_client(client); \
+	struct fscher_data *data = fscher_update_device(dev); \
 	return show_##kind##sub(data, buf, (offset)); \
 }
 

@@ -420,8 +418,9 @@
 	data->revision =  fscher_read_value(client, FSCHER_REG_REVISION);
 }
 
-static void fscher_update_client(struct i2c_client *client)
+static struct fscher_data *fscher_update_device(struct device *dev)
 {
+	struct i2c_client *client = to_i2c_client(dev);
 	struct fscher_data *data = i2c_get_clientdata(client);
 
 	down(&data->update_lock);

@@ -466,6 +465,8 @@
 	}
 
 	up(&data->update_lock);
+
+	return data;
 }
drivers/i2c/chips/gl518sm.c +7 -8
diff -Nru linux-2.6.4-rc3-gkh/drivers/i2c/chips/gl518sm.c linux-2.6.4-rc3-mmh/drivers/i2c/chips/gl518sm.c
@@ -151,7 +151,7 @@
 static int gl518_detach_client(struct i2c_client *client);
 static int gl518_read_value(struct i2c_client *client, u8 reg);
 static int gl518_write_value(struct i2c_client *client, u8 reg, u16 value);
-static void gl518_update_client(struct i2c_client *client);
+static struct gl518_data *gl518_update_device(struct device *dev);
 
 /* This is the driver that will be inserted */
 static struct i2c_driver gl518_driver = {

@@ -176,18 +176,14 @@
 #define show(type, suffix, value)					\
 static ssize_t show_##suffix(struct device *dev, char *buf)		\
 {									\
-	struct i2c_client *client = to_i2c_client(dev);			\
-	struct gl518_data *data = i2c_get_clientdata(client);		\
-	gl518_update_client(client);					\
+	struct gl518_data *data = gl518_update_device(dev);		\
 	return sprintf(buf, "%d\n", type##_FROM_REG(data->value));	\
 }
 
 #define show_fan(suffix, value, index)					\
 static ssize_t show_##suffix(struct device *dev, char *buf)		\
 {									\
-	struct i2c_client *client = to_i2c_client(dev);			\
-	struct gl518_data *data = i2c_get_clientdata(client);		\
-	gl518_update_client(client);					\
+	struct gl518_data *data = gl518_update_device(dev);		\
 	return sprintf(buf, "%d\n", FAN_FROM_REG(data->value[index],	\
 		DIV_FROM_REG(data->fan_div[index])));			\
 }

@@ -523,8 +519,9 @@
 		return i2c_smbus_write_byte_data(client, reg, value);
 }
 
-static void gl518_update_client(struct i2c_client *client)
+static struct gl518_data *gl518_update_device(struct device *dev)
 {
+	struct i2c_client *client = to_i2c_client(dev);
 	struct gl518_data *data = i2c_get_clientdata(client);
 	int val;
 

@@ -590,6 +587,8 @@
 	}
 
 	up(&data->update_lock);
+
+	return data;
 }
 
 static int __init sensors_gl518sm_init(void)
drivers/i2c/chips/it87.c +16 -35

128 lines not on this page · raw message

drivers/i2c/chips/lm75.c +6 -5
diff -Nru linux-2.6.4-rc3-gkh/drivers/i2c/chips/lm75.c linux-2.6.4-rc3-mmh/drivers/i2c/chips/lm75.c
@@ -64,7 +64,7 @@
 static int lm75_detach_client(struct i2c_client *client);
 static int lm75_read_value(struct i2c_client *client, u8 reg);
 static int lm75_write_value(struct i2c_client *client, u8 reg, u16 value);
-static void lm75_update_client(struct i2c_client *client);
+static struct lm75_data *lm75_update_device(struct device *dev);
 
 
 /* This is the driver that will be inserted */

@@ -82,9 +82,7 @@
 #define show(value)	\
 static ssize_t show_##value(struct device *dev, char *buf)		\
 {									\
-	struct i2c_client *client = to_i2c_client(dev);			\
-	struct lm75_data *data = i2c_get_clientdata(client);		\
-	lm75_update_client(client);					\
+	struct lm75_data *data = lm75_update_device(dev);		\
 	return sprintf(buf, "%d\n", LM75_TEMP_FROM_REG(data->value));	\
 }
 show(temp_max);

@@ -250,8 +248,9 @@
 	lm75_write_value(client, LM75_REG_CONF, 0);
 }
 
-static void lm75_update_client(struct i2c_client *client)
+static struct lm75_data *lm75_update_device(struct device *dev)
 {
+	struct i2c_client *client = to_i2c_client(dev);
 	struct lm75_data *data = i2c_get_clientdata(client);
 
 	down(&data->update_lock);

@@ -268,6 +267,8 @@
 	}
 
 	up(&data->update_lock);
+
+	return data;
 }
 
 static int __init sensors_lm75_init(void)
drivers/i2c/chips/lm78.c +16 -35

133 lines not on this page · raw message

drivers/i2c/chips/lm80.c +11 -20
diff -Nru linux-2.6.4-rc3-gkh/drivers/i2c/chips/lm80.c linux-2.6.4-rc3-mmh/drivers/i2c/chips/lm80.c
@@ -141,7 +141,7 @@
 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);
-static void lm80_update_client(struct i2c_client *client);
+static struct lm80_data *lm80_update_device(struct device *dev);
 static int lm80_read_value(struct i2c_client *client, u8 reg);
 static int lm80_write_value(struct i2c_client *client, u8 reg, u8 value);
 

@@ -171,9 +171,7 @@
 #define show_in(suffix, value) \
 static ssize_t show_in_##suffix(struct device *dev, char *buf) \
 { \
-	struct i2c_client *client = to_i2c_client(dev); \
-	struct lm80_data *data = i2c_get_clientdata(client); \
-	lm80_update_client(client); \
+	struct lm80_data *data = lm80_update_device(dev); \
 	return sprintf(buf, "%d\n", IN_FROM_REG(data->value)); \
 }
 show_in(min0, in_min[0]);

@@ -227,9 +225,7 @@
 #define show_fan(suffix, value, div) \
 static ssize_t show_fan_##suffix(struct device *dev, char *buf) \
 { \
-	struct i2c_client *client = to_i2c_client(dev); \
-	struct lm80_data *data = i2c_get_clientdata(client); \
-	lm80_update_client(client); \
+	struct lm80_data *data = lm80_update_device(dev); \
 	return sprintf(buf, "%d\n", FAN_FROM_REG(data->value, \
 		       DIV_FROM_REG(data->div))); \
 }

@@ -241,9 +237,7 @@
 #define show_fan_div(suffix, value) \
 static ssize_t show_fan_div##suffix(struct device *dev, char *buf) \
 { \
-	struct i2c_client *client = to_i2c_client(dev); \
-	struct lm80_data *data = i2c_get_clientdata(client); \
-	lm80_update_client(client); \
+	struct lm80_data *data = lm80_update_device(dev); \
 	return sprintf(buf, "%d\n", DIV_FROM_REG(data->value)); \
 }
 show_fan_div(1, fan_div[0]);

@@ -265,18 +259,14 @@
 
 static ssize_t show_temp_input1(struct device *dev, char *buf)
 {
-	struct i2c_client *client = to_i2c_client(dev);
-	struct lm80_data *data = i2c_get_clientdata(client);
-	lm80_update_client(client);
+	struct lm80_data *data = lm80_update_device(dev);
 	return sprintf(buf, "%ld\n", TEMP_FROM_REG(data->temp));
 }
 
 #define show_temp(suffix, value) \
 static ssize_t show_temp_##suffix(struct device *dev, char *buf) \
 { \
-	struct i2c_client *client = to_i2c_client(dev); \
-	struct lm80_data *data = i2c_get_clientdata(client); \
-	lm80_update_client(client); \
+	struct lm80_data *data = lm80_update_device(dev); \
 	return sprintf(buf, "%d\n", TEMP_LIMIT_FROM_REG(data->value)); \
 }
 show_temp(hot_max, temp_hot_max);

@@ -302,9 +292,7 @@
 
 static ssize_t show_alarms(struct device *dev, char *buf)
 {
-	struct i2c_client *client = to_i2c_client(dev);
-	struct lm80_data *data = i2c_get_clientdata(client);
-	lm80_update_client(client);
+	struct lm80_data *data = lm80_update_device(dev);
 	return sprintf(buf, "%d\n", ALARMS_FROM_REG(data->alarms));
 }
 

@@ -498,8 +486,9 @@
 	lm80_write_value(client, LM80_REG_CONFIG, 0x01);
 }
 
-static void lm80_update_client(struct i2c_client *client)
+static struct lm80_data *lm80_update_device(struct device *dev)
 {
+	struct i2c_client *client = to_i2c_client(dev);
 	struct lm80_data *data = i2c_get_clientdata(client);
 	int i;
 

@@ -546,6 +535,8 @@
 	}
 
 	up(&data->update_lock);
+
+	return data;
 }
 
 static int __init sensors_lm80_init(void)
drivers/i2c/chips/lm83.c +7 -8
diff -Nru linux-2.6.4-rc3-gkh/drivers/i2c/chips/lm83.c linux-2.6.4-rc3-mmh/drivers/i2c/chips/lm83.c
@@ -118,7 +118,7 @@
 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 void lm83_update_client(struct i2c_client *client);
+static struct lm83_data *lm83_update_device(struct device *dev);
 
 /*
  * Driver data (common to all clients)

@@ -162,9 +162,7 @@
 #define show_temp(suffix, value) \
 static ssize_t show_temp_##suffix(struct device *dev, char *buf) \
 { \
-	struct i2c_client *client = to_i2c_client(dev); \
-	struct lm83_data *data = i2c_get_clientdata(client); \
-	lm83_update_client(client); \
+	struct lm83_data *data = lm83_update_device(dev); \
 	return sprintf(buf, "%d\n", TEMP_FROM_REG(data->value)); \
 }
 show_temp(input1, temp_input[0]);

@@ -195,9 +193,7 @@
 
 static ssize_t show_alarms(struct device *dev, char *buf)
 {
-	struct i2c_client *client = to_i2c_client(dev);
-	struct lm83_data *data = i2c_get_clientdata(client);
-	lm83_update_client(client);
+	struct lm83_data *data = lm83_update_device(dev);
 	return sprintf(buf, "%d\n", data->alarms);
 }
 

@@ -353,8 +349,9 @@
 	return 0;
 }
 
-static void lm83_update_client(struct i2c_client *client)
+static struct lm83_data *lm83_update_device(struct device *dev)
 {
+	struct i2c_client *client = to_i2c_client(dev);
 	struct lm83_data *data = i2c_get_clientdata(client);
 
 	down(&data->update_lock);

@@ -385,6 +382,8 @@
 	}
 
 	up(&data->update_lock);
+
+	return data;
 }
 
 static int __init sensors_lm83_init(void)
drivers/i2c/chips/lm85.c +18 -53

164 lines not on this page · raw message

drivers/i2c/chips/lm90.c +8 -11
diff -Nru linux-2.6.4-rc3-gkh/drivers/i2c/chips/lm90.c linux-2.6.4-rc3-mmh/drivers/i2c/chips/lm90.c
@@ -126,7 +126,7 @@
 	int kind);
 static void lm90_init_client(struct i2c_client *client);
 static int lm90_detach_client(struct i2c_client *client);
-static void lm90_update_client(struct i2c_client *client);
+static struct lm90_data *lm90_update_device(struct device *dev);
 
 /*
  * Driver data (common to all clients)

@@ -171,9 +171,7 @@
 #define show_temp(value, converter) \
 static ssize_t show_##value(struct device *dev, char *buf) \
 { \
-	struct i2c_client *client = to_i2c_client(dev); \
-	struct lm90_data *data = i2c_get_clientdata(client); \
-	lm90_update_client(client); \
+	struct lm90_data *data = lm90_update_device(dev); \
 	return sprintf(buf, "%d\n", converter(data->value)); \
 }
 show_temp(temp_input1, TEMP1_FROM_REG);

@@ -216,9 +214,7 @@
 #define show_temp_hyst(value, basereg) \
 static ssize_t show_##value(struct device *dev, char *buf) \
 { \
-	struct i2c_client *client = to_i2c_client(dev); \
-	struct lm90_data *data = i2c_get_clientdata(client); \
-	lm90_update_client(client); \
+	struct lm90_data *data = lm90_update_device(dev); \
 	return sprintf(buf, "%d\n", TEMP1_FROM_REG(data->basereg) \
 		       - HYST_FROM_REG(data->temp_hyst)); \
 }

@@ -239,9 +235,7 @@
 
 static ssize_t show_alarms(struct device *dev, char *buf)
 {
-	struct i2c_client *client = to_i2c_client(dev);
-	struct lm90_data *data = i2c_get_clientdata(client);
-	lm90_update_client(client);
+	struct lm90_data *data = lm90_update_device(dev);
 	return sprintf(buf, "%d\n", data->alarms);
 }
 

@@ -430,8 +424,9 @@
 	return 0;
 }
 
-static void lm90_update_client(struct i2c_client *client)
+static struct lm90_data *lm90_update_device(struct device *dev)
 {
+	struct i2c_client *client = to_i2c_client(dev);
 	struct lm90_data *data = i2c_get_clientdata(client);
 
 	down(&data->update_lock);

@@ -505,6 +500,8 @@
 	}
 
 	up(&data->update_lock);
+
+	return data;
 }
 
 static int __init sensors_lm90_init(void)
drivers/i2c/chips/via686a.c +15 -32

112 lines not on this page · raw message

drivers/i2c/chips/w83627hf.c +15 -52

146 lines not on this page · raw message

drivers/i2c/chips/w83781d.c +17 -62

172 lines not on this page · raw message

drivers/i2c/chips/w83l785ts.c +7 -8
diff -Nru linux-2.6.4-rc3-gkh/drivers/i2c/chips/w83l785ts.c linux-2.6.4-rc3-mmh/drivers/i2c/chips/w83l785ts.c
@@ -89,7 +89,7 @@
 	int kind);
 static int w83l785ts_detach_client(struct i2c_client *client);
 static u8 w83l785ts_read_value(struct i2c_client *client, u8 reg, u8 defval);
-static void w83l785ts_update_client(struct i2c_client *client);
+static struct w83l785ts_data *w83l785ts_update_device(struct device *dev);
 
 /*
  * Driver data (common to all clients)

@@ -130,17 +130,13 @@
 
 static ssize_t show_temp(struct device *dev, char *buf)
 {
-	struct i2c_client *client = to_i2c_client(dev);
-	struct w83l785ts_data *data = i2c_get_clientdata(client);
-	w83l785ts_update_client(client);
+	struct w83l785ts_data *data = w83l785ts_update_device(dev);
 	return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp));
 }
 
 static ssize_t show_temp_over(struct device *dev, char *buf)
 {
-	struct i2c_client *client = to_i2c_client(dev);
-	struct w83l785ts_data *data = i2c_get_clientdata(client);
-	w83l785ts_update_client(client);
+	struct w83l785ts_data *data = w83l785ts_update_device(dev);
 	return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp_over));
 }
 

@@ -304,8 +300,9 @@
 	return defval;
 }
 
-static void w83l785ts_update_client(struct i2c_client *client)
+static struct w83l785ts_data *w83l785ts_update_device(struct device *dev)
 {
+	struct i2c_client *client = to_i2c_client(dev);
 	struct w83l785ts_data *data = i2c_get_clientdata(client);
 
 	down(&data->update_lock);

@@ -324,6 +321,8 @@
 	}
 
 	up(&data->update_lock);
+
+	return data;
 }
 
 static int __init sensors_w83l785ts_init(void)
Signature · 2 lines
Mark M. Hoffman
[email protected]
#1 lore raw

PATCH 1/1 sensor chip driver refactoring

Quoted 2 lines

I think this patch will apply (without fuzz) to your current I2C tree. If it doesn't, please post a plain diff of your tree and I'll redo.

2.6.4-mm1 can be used as a base since it contains Greg's i2c tree.

Also, you can get my patch set here:
http://delvare.nerim.net/i2c/linux-2.6/linux-2.6.4-i2c.patch.gz

It should never be really far from Greg's as long as patches are sent to the mailing-list before he applies them.

Your patch actually applies, with some fuzz on it87 due to the fact that Takeru's first patch had some broken indentation (not really his fault since they came from the original driver in the first place). I've fixed my own patches collection to reflect that and fixed some more noise too, so it now applies against 2.6.4 cleanly and brings the i2c tree to the same point as 2.6.4-mm1 does, as far as I can tell.

Quoted 6 lines

This patch is a refactoring of some common code among all sensors chip drivers (except asb100, which was written this way to begin with.) It saves a handful of lines and ~100-300 bytes per module. It compiles ok. I've only tested it against one of the drivers, but the changes are similar across the board and quite mechanical. Please apply.

Tested, compiles fine and works OK (AS99127F rev.1, ADM1032). I like the idea much and just have to say I support Mark here :)

Signature · 2 lines
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/
#2 lore raw

PATCH 1/1 sensor chip driver refactoring

In reply to Mark M. Hoffman
Quoted 4 lines · Mark M. Hoffman

Hi Greg:

I think this patch will apply (without fuzz) to your current I2C tree. If it doesn't, please post a plain diff of your tree and I'll redo.

It applied just fine, thanks.

greg k-h