[lm-sensors] [patch 2.6.25-rc9 1/5] lm75: cleanup/reorg

[lm-sensors] [patch 2.6.25-rc9 1/5] lm75: cleanup/reorg

[David Brownell]

Minor cleanup and reorg of the lm75 code.

 - Kconfig provides a larger list of lm75-compatible chips

 - A top comment now says what the driver does (!) ... as in, just
   what sort of sensor is this??

 - Section comments now delineate the various sections of the driver:
   hwmon attributes, driver binding, register access, module glue.
   One driver binding function moved out of the attribute section,
   as did the driver struct itself.

 - Minor tweaks to legacy probe logic:  correct a comment, and
   remove a pointless variable.

 - Whitespace, linelength, and comment fixes.

This patch should include no functional changes.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
 drivers/hwmon/Kconfig |   13 +++++-
 drivers/hwmon/lm75.c  |   97 +++++++++++++++++++++++++++-----------------------
 2 files changed, 63 insertions(+), 47 deletions(-)

--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -379,9 +379,16 @@ config SENSORS_LM75
 	tristate "National Semiconductor LM75 and compatibles"
 	depends on I2C
 	help
-	  If you say yes here you get support for National Semiconductor LM75
-	  sensor chips and clones: Dallas Semiconductor DS75 and DS1775 (in
-	  9-bit precision mode), and TelCom (now Microchip) TCN75.
+	  If you say yes here you get support for one common type of
+	  temperature sensor chip, with models including:
+
+		- Dallas Semiconductor DS75 and DS1775
+		- Maxim MAX6625 and MAX6626
+		- Microchip MCP980x
+		- National Semiconductor LM75
+		- ST Microelectronics STDS75
+		- TelCom (now Microchip) TCN75
+		- Texas Instruments TMP100, TMP101, TMP75, TMP175, TMP275
 
 	  The DS75 and DS1775 in 10- to 12-bit precision modes will require
 	  a force module parameter. The driver will not handle the extra
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -30,14 +30,19 @@
 #include "lm75.h"
 
 
-/* Addresses to scan */
+/*
+ * This driver handles the LM75 and compatible digital temperature sensors.
+ * Compatibles include at least the DS75, DS1775, MCP980x, STDS75, TCN75,
+ * TMP100, TMP101, TMP75, TMP175, and TMP275.
+ */
+
+/* Addresses scanned by legacy style driver binding */
 static const unsigned short normal_i2c[] = { 0x48, 0x49, 0x4a, 0x4b, 0x4c,
 					0x4d, 0x4e, 0x4f, I2C_CLIENT_END };
 
-/* Insmod parameters */
+/* Insmod parameters (only for legacy style driver binding) */
 I2C_CLIENT_INSMOD_1(lm75);
 
-/* Many LM75 constants specified below */
 
 /* The LM75 registers */
 #define LM75_REG_CONF		0x01
@@ -50,9 +55,9 @@ static const u8 LM75_REG_TEMP[3] = {
 /* Each client has this additional data */
 struct lm75_data {
 	struct i2c_client	client;
-	struct device *hwmon_dev;
+	struct device		*hwmon_dev;
 	struct mutex		update_lock;
-	char			valid;		/* !=0 if following fields are valid */
+	char			valid;		/* !=0 if registers are valid */
 	unsigned long		last_updated;	/* In jiffies */
 	u16			temp[3];	/* Register values,
 						   0 = input
@@ -60,23 +65,15 @@ struct lm75_data {
 						   2 = 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);
 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 struct lm75_data *lm75_update_device(struct device *dev);
 
 
-/* This is the driver that will be inserted */
-static struct i2c_driver lm75_driver = {
-	.driver = {
-		.name	= "lm75",
-	},
-	.attach_adapter	= lm75_attach_adapter,
-	.detach_client	= lm75_detach_client,
-};
+/*-----------------------------------------------------------------------*/
+
+/* sysfs attributes for hwmon */
 
 static ssize_t show_temp(struct device *dev, struct device_attribute *da,
 			 char *buf)
@@ -109,13 +106,6 @@ static SENSOR_DEVICE_ATTR(temp1_max_hyst
 			show_temp, set_temp, 2);
 static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp, NULL, 0);
 
-static int lm75_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, lm75_detect);
-}
-
 static struct attribute *lm75_attributes[] = {
 	&sensor_dev_attr_temp1_input.dev_attr.attr,
 	&sensor_dev_attr_temp1_max.dev_attr.attr,
@@ -128,6 +118,12 @@ static const struct attribute_group lm75
 	.attrs = lm75_attributes,
 };
 
+/*-----------------------------------------------------------------------*/
+
+/* "Legacy" I2C driver binding */
+
+static struct i2c_driver lm75_driver;
+
 /* This function is called by i2c_probe */
 static int lm75_detect(struct i2c_adapter *adapter, int address, int kind)
 {
@@ -135,15 +131,14 @@ static int lm75_detect(struct i2c_adapte
 	struct i2c_client *new_client;
 	struct lm75_data *data;
 	int err = 0;
-	const char *name = "";
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
 				     I2C_FUNC_SMBUS_WORD_DATA))
 		goto exit;
 
-	/* OK. For now, we presume we have a valid client. We now create the
-	   client structure, even though we cannot fill it completely yet.
-	   But it allows us to access lm75_{read,write}_value. */
+	/* OK. For now, we presume we have a valid address. We create the
+	   client structure, even though there may be no sensor present.
+	   But it allows us to access i2c_smbus_read_*_data() calls. */
 	if (!(data = kzalloc(sizeof(struct lm75_data), GFP_KERNEL))) {
 		err = -ENOMEM;
 		goto exit;
@@ -174,17 +169,17 @@ static int lm75_detect(struct i2c_adapte
 		 || i2c_smbus_read_word_data(new_client, 5) != hyst
 		 || i2c_smbus_read_word_data(new_client, 6) != hyst
 		 || i2c_smbus_read_word_data(new_client, 7) != hyst)
-		 	goto exit_free;
+			goto exit_free;
 		os = i2c_smbus_read_word_data(new_client, 3);
 		if (i2c_smbus_read_word_data(new_client, 4) != os
 		 || i2c_smbus_read_word_data(new_client, 5) != os
 		 || i2c_smbus_read_word_data(new_client, 6) != os
 		 || i2c_smbus_read_word_data(new_client, 7) != os)
-		 	goto exit_free;
+			goto exit_free;
 
 		/* Unused bits */
 		if (conf & 0xe0)
-		 	goto exit_free;
+			goto exit_free;
 
 		/* Addresses cycling */
 		for (i = 8; i < 0xff; i += 8)
@@ -194,16 +189,10 @@ static int lm75_detect(struct i2c_adapte
 				goto exit_free;
 	}
 
-	/* Determine the chip type - only one kind supported! */
-	if (kind <= 0)
-		kind = lm75;
-
-	if (kind = lm75) {
-		name = "lm75";
-	}
+	/* NOTE: we treat "force=..." and "force_lm75=..." the same. */
+	strlcpy(new_client->name, "lm75", I2C_NAME_SIZE);
 
 	/* Fill in the remaining client fields and put it into the global list */
-	strlcpy(new_client->name, name, I2C_NAME_SIZE);
 	data->valid = 0;
 	mutex_init(&data->update_lock);
 
@@ -213,7 +202,7 @@ static int lm75_detect(struct i2c_adapte
 
 	/* Initialize the LM75 chip */
 	lm75_init_client(new_client);
-	
+
 	/* Register sysfs hooks */
 	if ((err = sysfs_create_group(&new_client->dev.kobj, &lm75_group)))
 		goto exit_detach;
@@ -236,6 +225,13 @@ exit:
 	return err;
 }
 
+static int lm75_attach_adapter(struct i2c_adapter *adapter)
+{
+	if (!(adapter->class & I2C_CLASS_HWMON))
+		return 0;
+	return i2c_probe(adapter, &addr_data, lm75_detect);
+}
+
 static int lm75_detach_client(struct i2c_client *client)
 {
 	struct lm75_data *data = i2c_get_clientdata(client);
@@ -246,9 +242,21 @@ static int lm75_detach_client(struct i2c
 	return 0;
 }
 
+static struct i2c_driver lm75_driver = {
+	.driver = {
+		.name	= "lm75",
+	},
+	.attach_adapter	= lm75_attach_adapter,
+	.detach_client	= lm75_detach_client,
+};
+
+/*-----------------------------------------------------------------------*/
+
+/* register access */
+
 /* All registers are word-sized, except for the configuration register.
-   LM75 uses a high-byte first convention, which is exactly opposite to
-   the usual practice. */
+ * LM75 uses a high-byte first convention (not the SMBus convention).
+ */
 static int lm75_read_value(struct i2c_client *client, u8 reg)
 {
 	if (reg = LM75_REG_CONF)
@@ -257,9 +265,6 @@ static int lm75_read_value(struct i2c_cl
 		return swab16(i2c_smbus_read_word_data(client, reg));
 }
 
-/* All registers are word-sized, except for the configuration register.
-   LM75 uses a high-byte first convention, which is exactly opposite to
-   the usual practice. */
 static int lm75_write_value(struct i2c_client *client, u8 reg, u16 value)
 {
 	if (reg = LM75_REG_CONF)
@@ -302,6 +307,10 @@ static struct lm75_data *lm75_update_dev
 	return data;
 }
 
+/*-----------------------------------------------------------------------*/
+
+/* module glue */
+
 static int __init sensors_lm75_init(void)
 {
 	return i2c_add_driver(&lm75_driver);

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [patch 2.6.25-rc9 1/5] lm75: cleanup/reorg

[Jean Delvare]

Hi David,

On Wed, 16 Apr 2008 10:29:23 -0700, David Brownell wrote:
> Minor cleanup and reorg of the lm75 code.
> 
>  - Kconfig provides a larger list of lm75-compatible chips
> 
>  - A top comment now says what the driver does (!) ... as in, just
>    what sort of sensor is this??
> 
>  - Section comments now delineate the various sections of the driver:
>    hwmon attributes, driver binding, register access, module glue.
>    One driver binding function moved out of the attribute section,
>    as did the driver struct itself.
> 
>  - Minor tweaks to legacy probe logic:  correct a comment, and
>    remove a pointless variable.
> 
>  - Whitespace, linelength, and comment fixes.
> 
> This patch should include no functional changes.
> 
> Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> ---
>  drivers/hwmon/Kconfig |   13 +++++-
>  drivers/hwmon/lm75.c  |   97 +++++++++++++++++++++++++++-----------------------
>  2 files changed, 63 insertions(+), 47 deletions(-)

The following patch in Mark's tree:
http://lm-sensors.org/kernel?p=kernel/mhoffman/hwmon-2.6.git;a=commit;h¦17156848e99b93660f0da08fc401661d5cddbf
conflicts with this patch. You'll have to adjust your patch so that it
applies cleanly on top of Mark's tree (shouldn't be difficult.)

Over than that, I had already reviewed this patch and it looked OK to
me. Additional comments while I'm here:

> 
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -379,9 +379,16 @@ config SENSORS_LM75
>  	tristate "National Semiconductor LM75 and compatibles"
>  	depends on I2C
>  	help
> -	  If you say yes here you get support for National Semiconductor LM75
> -	  sensor chips and clones: Dallas Semiconductor DS75 and DS1775 (in
> -	  9-bit precision mode), and TelCom (now Microchip) TCN75.
> +	  If you say yes here you get support for one common type of
> +	  temperature sensor chip, with models including:
> +
> +		- Dallas Semiconductor DS75 and DS1775
> +		- Maxim MAX6625 and MAX6626
> +		- Microchip MCP980x
> +		- National Semiconductor LM75
> +		- ST Microelectronics STDS75
> +		- TelCom (now Microchip) TCN75
> +		- Texas Instruments TMP100, TMP101, TMP75, TMP175, TMP275

It seems you can add the Philips (NXP) LM75A to the list.

>  
>  	  The DS75 and DS1775 in 10- to 12-bit precision modes will require
>  	  a force module parameter. The driver will not handle the extra

In fact almost all chips above (except the original NatSemi LM75) need a
force module parameter.

> --- a/drivers/hwmon/lm75.c
> +++ b/drivers/hwmon/lm75.c
> @@ -30,14 +30,19 @@
>  #include "lm75.h"
>  
>  
> -/* Addresses to scan */
> +/*
> + * This driver handles the LM75 and compatible digital temperature sensors.
> + * Compatibles include at least the DS75, DS1775, MCP980x, STDS75, TCN75,
> + * TMP100, TMP101, TMP75, TMP175, and TMP275.
> + */
> +
> +/* Addresses scanned by legacy style driver binding */
>  static const unsigned short normal_i2c[] = { 0x48, 0x49, 0x4a, 0x4b, 0x4c,
>  					0x4d, 0x4e, 0x4f, I2C_CLIENT_END };
>  
> -/* Insmod parameters */
> +/* Insmod parameters (only for legacy style driver binding) */
>  I2C_CLIENT_INSMOD_1(lm75);
>  
> -/* Many LM75 constants specified below */
>  
>  /* The LM75 registers */
>  #define LM75_REG_CONF		0x01
> @@ -50,9 +55,9 @@ static const u8 LM75_REG_TEMP[3] = {
>  /* Each client has this additional data */
>  struct lm75_data {
>  	struct i2c_client	client;
> -	struct device *hwmon_dev;
> +	struct device		*hwmon_dev;
>  	struct mutex		update_lock;
> -	char			valid;		/* !=0 if following fields are valid */
> +	char			valid;		/* !=0 if registers are valid */
>  	unsigned long		last_updated;	/* In jiffies */
>  	u16			temp[3];	/* Register values,
>  						   0 = input
> @@ -60,23 +65,15 @@ struct lm75_data {
>  						   2 = 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);
>  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 struct lm75_data *lm75_update_device(struct device *dev);
>  
>  
> -/* This is the driver that will be inserted */
> -static struct i2c_driver lm75_driver = {
> -	.driver = {
> -		.name	= "lm75",
> -	},
> -	.attach_adapter	= lm75_attach_adapter,
> -	.detach_client	= lm75_detach_client,
> -};
> +/*-----------------------------------------------------------------------*/
> +
> +/* sysfs attributes for hwmon */
>  
>  static ssize_t show_temp(struct device *dev, struct device_attribute *da,
>  			 char *buf)
> @@ -109,13 +106,6 @@ static SENSOR_DEVICE_ATTR(temp1_max_hyst
>  			show_temp, set_temp, 2);
>  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp, NULL, 0);
>  
> -static int lm75_attach_adapter(struct i2c_adapter *adapter)
> -{
> -	if (!(adapter->class & I2C_CLASS_HWMON))
> -		return 0;
> -	return i2c_probe(adapter, &addr_data, lm75_detect);
> -}
> -
>  static struct attribute *lm75_attributes[] = {
>  	&sensor_dev_attr_temp1_input.dev_attr.attr,
>  	&sensor_dev_attr_temp1_max.dev_attr.attr,
> @@ -128,6 +118,12 @@ static const struct attribute_group lm75
>  	.attrs = lm75_attributes,
>  };
>  
> +/*-----------------------------------------------------------------------*/
> +
> +/* "Legacy" I2C driver binding */
> +
> +static struct i2c_driver lm75_driver;
> +
>  /* This function is called by i2c_probe */
>  static int lm75_detect(struct i2c_adapter *adapter, int address, int kind)
>  {
> @@ -135,15 +131,14 @@ static int lm75_detect(struct i2c_adapte
>  	struct i2c_client *new_client;
>  	struct lm75_data *data;
>  	int err = 0;
> -	const char *name = "";
>  
>  	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
>  				     I2C_FUNC_SMBUS_WORD_DATA))
>  		goto exit;
>  
> -	/* OK. For now, we presume we have a valid client. We now create the
> -	   client structure, even though we cannot fill it completely yet.
> -	   But it allows us to access lm75_{read,write}_value. */
> +	/* OK. For now, we presume we have a valid address. We create the
> +	   client structure, even though there may be no sensor present.
> +	   But it allows us to access i2c_smbus_read_*_data() calls. */

"accessing a call" sounds strange.

>  	if (!(data = kzalloc(sizeof(struct lm75_data), GFP_KERNEL))) {
>  		err = -ENOMEM;
>  		goto exit;
> @@ -174,17 +169,17 @@ static int lm75_detect(struct i2c_adapte
>  		 || i2c_smbus_read_word_data(new_client, 5) != hyst
>  		 || i2c_smbus_read_word_data(new_client, 6) != hyst
>  		 || i2c_smbus_read_word_data(new_client, 7) != hyst)
> -		 	goto exit_free;
> +			goto exit_free;
>  		os = i2c_smbus_read_word_data(new_client, 3);
>  		if (i2c_smbus_read_word_data(new_client, 4) != os
>  		 || i2c_smbus_read_word_data(new_client, 5) != os
>  		 || i2c_smbus_read_word_data(new_client, 6) != os
>  		 || i2c_smbus_read_word_data(new_client, 7) != os)
> -		 	goto exit_free;
> +			goto exit_free;
>  
>  		/* Unused bits */
>  		if (conf & 0xe0)
> -		 	goto exit_free;
> +			goto exit_free;
>  
>  		/* Addresses cycling */
>  		for (i = 8; i < 0xff; i += 8)
> @@ -194,16 +189,10 @@ static int lm75_detect(struct i2c_adapte
>  				goto exit_free;
>  	}
>  
> -	/* Determine the chip type - only one kind supported! */
> -	if (kind <= 0)
> -		kind = lm75;
> -
> -	if (kind = lm75) {
> -		name = "lm75";
> -	}
> +	/* NOTE: we treat "force=..." and "force_lm75=..." the same. */
> +	strlcpy(new_client->name, "lm75", I2C_NAME_SIZE);
>  
>  	/* Fill in the remaining client fields and put it into the global list */
> -	strlcpy(new_client->name, name, I2C_NAME_SIZE);
>  	data->valid = 0;
>  	mutex_init(&data->update_lock);
>  
> @@ -213,7 +202,7 @@ static int lm75_detect(struct i2c_adapte
>  
>  	/* Initialize the LM75 chip */
>  	lm75_init_client(new_client);
> -	
> +
>  	/* Register sysfs hooks */
>  	if ((err = sysfs_create_group(&new_client->dev.kobj, &lm75_group)))
>  		goto exit_detach;
> @@ -236,6 +225,13 @@ exit:
>  	return err;
>  }
>  
> +static int lm75_attach_adapter(struct i2c_adapter *adapter)
> +{
> +	if (!(adapter->class & I2C_CLASS_HWMON))
> +		return 0;
> +	return i2c_probe(adapter, &addr_data, lm75_detect);
> +}
> +
>  static int lm75_detach_client(struct i2c_client *client)
>  {
>  	struct lm75_data *data = i2c_get_clientdata(client);
> @@ -246,9 +242,21 @@ static int lm75_detach_client(struct i2c
>  	return 0;
>  }
>  
> +static struct i2c_driver lm75_driver = {
> +	.driver = {
> +		.name	= "lm75",
> +	},
> +	.attach_adapter	= lm75_attach_adapter,
> +	.detach_client	= lm75_detach_client,
> +};
> +
> +/*-----------------------------------------------------------------------*/
> +
> +/* register access */
> +
>  /* All registers are word-sized, except for the configuration register.
> -   LM75 uses a high-byte first convention, which is exactly opposite to
> -   the usual practice. */
> + * LM75 uses a high-byte first convention (not the SMBus convention).
> + */

This is the part which conflicts with my own cleanup patch. Basically
my patch does exactly what you were doing here, so you can just drop
this part of your patch.

>  static int lm75_read_value(struct i2c_client *client, u8 reg)
>  {
>  	if (reg = LM75_REG_CONF)
> @@ -257,9 +265,6 @@ static int lm75_read_value(struct i2c_cl
>  		return swab16(i2c_smbus_read_word_data(client, reg));
>  }
>  
> -/* All registers are word-sized, except for the configuration register.
> -   LM75 uses a high-byte first convention, which is exactly opposite to
> -   the usual practice. */
>  static int lm75_write_value(struct i2c_client *client, u8 reg, u16 value)
>  {
>  	if (reg = LM75_REG_CONF)
> @@ -302,6 +307,10 @@ static struct lm75_data *lm75_update_dev
>  	return data;
>  }
>  
> +/*-----------------------------------------------------------------------*/
> +
> +/* module glue */
> +
>  static int __init sensors_lm75_init(void)
>  {
>  	return i2c_add_driver(&lm75_driver);


-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [patch 2.6.25-rc9 1/5] lm75: cleanup/reorg

[David Brownell]

On Friday 18 April 2008, Jean Delvare wrote:
> The following patch in Mark's tree:
> http://lm-sensors.org/kernel?p=kernel/mhoffman/hwmon-2.6.git;a=commit;h¦17156848e99b93660f0da08fc401661d5cddbf
> conflicts with this patch. You'll have to adjust your patch so that it
> applies cleanly on top of Mark's tree (shouldn't be difficult.)
> 
> Over than that, I had already reviewed this patch and it looked OK to
> me. Additional comments while I'm here:

Updated patch appended.  The second patch needs corresponding
fixups; I'll post them after I get any comments on that.

===== CUT HERE
From: David Brownell <dbrownell@users.sourceforge.net>

Minor cleanup and reorg of the lm75 code.

 - Kconfig provides a larger list of lm75-compatible chips

 - A top comment now says what the driver does (!) ... as in, just
   what sort of sensor is this??

 - Section comments now delineate the various sections of the driver:
   hwmon attributes, driver binding, register access, module glue.
   One driver binding function moved out of the attribute section,
   as did the driver struct itself.

 - Minor tweaks to legacy probe logic:  correct a comment, and
   remove a pointless variable.

 - Whitespace, linelength, and comment fixes.

This patch should include no functional changes.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
 drivers/hwmon/Kconfig |   17 ++++++---
 drivers/hwmon/lm75.c  |   90 ++++++++++++++++++++++++++++----------------------
 2 files changed, 62 insertions(+), 45 deletions(-)

--- ngw.orig/drivers/hwmon/Kconfig	2008-04-18 11:16:46.000000000 -0700
+++ ngw/drivers/hwmon/Kconfig	2008-04-18 11:24:59.000000000 -0700
@@ -380,13 +380,18 @@ config SENSORS_LM75
 	tristate "National Semiconductor LM75 and compatibles"
 	depends on I2C
 	help
-	  If you say yes here you get support for National Semiconductor LM75
-	  sensor chips and clones: Dallas Semiconductor DS75 and DS1775 (in
-	  9-bit precision mode), and TelCom (now Microchip) TCN75.
+	  If you say yes here you get support for one common type of
+	  temperature sensor chip, with models including:
 
-	  The DS75 and DS1775 in 10- to 12-bit precision modes will require
-	  a force module parameter. The driver will not handle the extra
-	  precision anyhow.
+		- Dallas Semiconductor DS75 and DS1775
+		- Maxim MAX6625 and MAX6626
+		- Microchip MCP980x
+		- National Semiconductor LM75 and LM75A
+		- ST Microelectronics STDS75
+		- TelCom (now Microchip) TCN75
+		- Texas Instruments TMP100, TMP101, TMP75, TMP175, TMP275
+
+	  Most of these chips will require a "force" module parameter.
 
 	  This driver can also be built as a module.  If so, the module
 	  will be called lm75.
--- ngw.orig/drivers/hwmon/lm75.c	2008-04-18 11:18:48.000000000 -0700
+++ ngw/drivers/hwmon/lm75.c	2008-04-18 11:26:02.000000000 -0700
@@ -30,14 +30,19 @@
 #include "lm75.h"
 
 
-/* Addresses to scan */
+/*
+ * This driver handles the LM75 and compatible digital temperature sensors.
+ * Compatibles include at least the DS75, DS1775, MCP980x, STDS75, TCN75,
+ * TMP100, TMP101, TMP75, TMP175, and TMP275.
+ */
+
+/* Addresses scanned by legacy style driver binding */
 static const unsigned short normal_i2c[] = { 0x48, 0x49, 0x4a, 0x4b, 0x4c,
 					0x4d, 0x4e, 0x4f, I2C_CLIENT_END };
 
-/* Insmod parameters */
+/* Insmod parameters (only for legacy style driver binding) */
 I2C_CLIENT_INSMOD_1(lm75);
 
-/* Many LM75 constants specified below */
 
 /* The LM75 registers */
 #define LM75_REG_CONF		0x01
@@ -50,9 +55,9 @@ static const u8 LM75_REG_TEMP[3] = {
 /* Each client has this additional data */
 struct lm75_data {
 	struct i2c_client	client;
-	struct device *hwmon_dev;
+	struct device		*hwmon_dev;
 	struct mutex		update_lock;
-	char			valid;		/* !=0 if following fields are valid */
+	char			valid;		/* !=0 if registers are valid */
 	unsigned long		last_updated;	/* In jiffies */
 	u16			temp[3];	/* Register values,
 						   0 = input
@@ -60,23 +65,15 @@ struct lm75_data {
 						   2 = 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);
 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 struct lm75_data *lm75_update_device(struct device *dev);
 
 
-/* This is the driver that will be inserted */
-static struct i2c_driver lm75_driver = {
-	.driver = {
-		.name	= "lm75",
-	},
-	.attach_adapter	= lm75_attach_adapter,
-	.detach_client	= lm75_detach_client,
-};
+/*-----------------------------------------------------------------------*/
+
+/* sysfs attributes for hwmon */
 
 static ssize_t show_temp(struct device *dev, struct device_attribute *da,
 			 char *buf)
@@ -109,13 +106,6 @@ static SENSOR_DEVICE_ATTR(temp1_max_hyst
 			show_temp, set_temp, 2);
 static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp, NULL, 0);
 
-static int lm75_attach_adapter(struct i2c_adapter *adapter)
-{
-	if (!(adapter->class & I2C_CLASS_HWMON))
-		return 0;
-	return i2c_probe(adapter, &addr_data, lm75_detect);
-}
-
 static struct attribute *lm75_attributes[] = {
 	&sensor_dev_attr_temp1_input.dev_attr.attr,
 	&sensor_dev_attr_temp1_max.dev_attr.attr,
@@ -128,6 +118,12 @@ static const struct attribute_group lm75
 	.attrs = lm75_attributes,
 };
 
+/*-----------------------------------------------------------------------*/
+
+/* "Legacy" I2C driver binding */
+
+static struct i2c_driver lm75_driver;
+
 /* This function is called by i2c_probe */
 static int lm75_detect(struct i2c_adapter *adapter, int address, int kind)
 {
@@ -135,15 +131,14 @@ static int lm75_detect(struct i2c_adapte
 	struct i2c_client *new_client;
 	struct lm75_data *data;
 	int err = 0;
-	const char *name = "";
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
 				     I2C_FUNC_SMBUS_WORD_DATA))
 		goto exit;
 
-	/* OK. For now, we presume we have a valid client. We now create the
-	   client structure, even though we cannot fill it completely yet.
-	   But it allows us to access lm75_{read,write}_value. */
+	/* OK. For now, we presume we have a valid address. We create the
+	   client structure, even though there may be no sensor present.
+	   But it allows us to use i2c_smbus_read_*_data() calls. */
 	if (!(data = kzalloc(sizeof(struct lm75_data), GFP_KERNEL))) {
 		err = -ENOMEM;
 		goto exit;
@@ -174,17 +169,17 @@ static int lm75_detect(struct i2c_adapte
 		 || i2c_smbus_read_word_data(new_client, 5) != hyst
 		 || i2c_smbus_read_word_data(new_client, 6) != hyst
 		 || i2c_smbus_read_word_data(new_client, 7) != hyst)
-		 	goto exit_free;
+			goto exit_free;
 		os = i2c_smbus_read_word_data(new_client, 3);
 		if (i2c_smbus_read_word_data(new_client, 4) != os
 		 || i2c_smbus_read_word_data(new_client, 5) != os
 		 || i2c_smbus_read_word_data(new_client, 6) != os
 		 || i2c_smbus_read_word_data(new_client, 7) != os)
-		 	goto exit_free;
+			goto exit_free;
 
 		/* Unused bits */
 		if (conf & 0xe0)
-		 	goto exit_free;
+			goto exit_free;
 
 		/* Addresses cycling */
 		for (i = 8; i < 0xff; i += 8)
@@ -194,16 +189,10 @@ static int lm75_detect(struct i2c_adapte
 				goto exit_free;
 	}
 
-	/* Determine the chip type - only one kind supported! */
-	if (kind <= 0)
-		kind = lm75;
-
-	if (kind = lm75) {
-		name = "lm75";
-	}
+	/* NOTE: we treat "force=..." and "force_lm75=..." the same. */
+	strlcpy(new_client->name, "lm75", I2C_NAME_SIZE);
 
 	/* Fill in the remaining client fields and put it into the global list */
-	strlcpy(new_client->name, name, I2C_NAME_SIZE);
 	data->valid = 0;
 	mutex_init(&data->update_lock);
 
@@ -213,7 +202,7 @@ static int lm75_detect(struct i2c_adapte
 
 	/* Initialize the LM75 chip */
 	lm75_init_client(new_client);
-	
+
 	/* Register sysfs hooks */
 	if ((err = sysfs_create_group(&new_client->dev.kobj, &lm75_group)))
 		goto exit_detach;
@@ -236,6 +225,13 @@ exit:
 	return err;
 }
 
+static int lm75_attach_adapter(struct i2c_adapter *adapter)
+{
+	if (!(adapter->class & I2C_CLASS_HWMON))
+		return 0;
+	return i2c_probe(adapter, &addr_data, lm75_detect);
+}
+
 static int lm75_detach_client(struct i2c_client *client)
 {
 	struct lm75_data *data = i2c_get_clientdata(client);
@@ -246,6 +242,18 @@ static int lm75_detach_client(struct i2c
 	return 0;
 }
 
+static struct i2c_driver lm75_driver = {
+	.driver = {
+		.name	= "lm75",
+	},
+	.attach_adapter	= lm75_attach_adapter,
+	.detach_client	= lm75_detach_client,
+};
+
+/*-----------------------------------------------------------------------*/
+
+/* register access */
+
 /* All registers are word-sized, except for the configuration register.
    LM75 uses a high-byte first convention, which is exactly opposite to
    the SMBus standard. */
@@ -299,6 +307,10 @@ static struct lm75_data *lm75_update_dev
 	return data;
 }
 
+/*-----------------------------------------------------------------------*/
+
+/* module glue */
+
 static int __init sensors_lm75_init(void)
 {
 	return i2c_add_driver(&lm75_driver);


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [patch 2.6.25-rc9 1/5] lm75: cleanup/reorg

[Jean Delvare]

On Fri, 18 Apr 2008 13:49:31 -0700, David Brownell wrote:
> Updated patch appended.  The second patch needs corresponding
> fixups; I'll post them after I get any comments on that.

Good idea. I hope to have some time for this later today, if Laurent
doesn't beat me at it.

> Minor cleanup and reorg of the lm75 code.
> 
>  - Kconfig provides a larger list of lm75-compatible chips
> 
>  - A top comment now says what the driver does (!) ... as in, just
>    what sort of sensor is this??
> 
>  - Section comments now delineate the various sections of the driver:
>    hwmon attributes, driver binding, register access, module glue.
>    One driver binding function moved out of the attribute section,
>    as did the driver struct itself.
> 
>  - Minor tweaks to legacy probe logic:  correct a comment, and
>    remove a pointless variable.
> 
>  - Whitespace, linelength, and comment fixes.
> 
> This patch should include no functional changes.
> 
> Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> ---
>  drivers/hwmon/Kconfig |   17 ++++++---
>  drivers/hwmon/lm75.c  |   90 ++++++++++++++++++++++++++++----------------------
>  2 files changed, 62 insertions(+), 45 deletions(-)
> 
> --- ngw.orig/drivers/hwmon/Kconfig	2008-04-18 11:16:46.000000000 -0700
> +++ ngw/drivers/hwmon/Kconfig	2008-04-18 11:24:59.000000000 -0700
> @@ -380,13 +380,18 @@ config SENSORS_LM75
>  	tristate "National Semiconductor LM75 and compatibles"
>  	depends on I2C
>  	help
> -	  If you say yes here you get support for National Semiconductor LM75
> -	  sensor chips and clones: Dallas Semiconductor DS75 and DS1775 (in
> -	  9-bit precision mode), and TelCom (now Microchip) TCN75.
> +	  If you say yes here you get support for one common type of
> +	  temperature sensor chip, with models including:
>  
> -	  The DS75 and DS1775 in 10- to 12-bit precision modes will require
> -	  a force module parameter. The driver will not handle the extra
> -	  precision anyhow.
> +		- Dallas Semiconductor DS75 and DS1775
> +		- Maxim MAX6625 and MAX6626
> +		- Microchip MCP980x
> +		- National Semiconductor LM75 and LM75A

The LM75A is actually a Philips (NXP) part, not a National
Semiconductor part.

> +		- ST Microelectronics STDS75
> +		- TelCom (now Microchip) TCN75
> +		- Texas Instruments TMP100, TMP101, TMP75, TMP175, TMP275
> +
> +	  Most of these chips will require a "force" module parameter.
>  
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called lm75.

But anyway, all the rest is correct so

Acked-by: Jean Delvare <khali@linux-fr.org>

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [patch 2.6.25-rc9 1/5] lm75: cleanup/reorg

[Laurent Pinchart]

[-- Attachment #1.1: Type: text/plain, Size: 3141 bytes --]

On Saturday 19 April 2008 14:34, Jean Delvare wrote:
> On Fri, 18 Apr 2008 13:49:31 -0700, David Brownell wrote:
> > Updated patch appended.  The second patch needs corresponding
> > fixups; I'll post them after I get any comments on that.
> 
> Good idea. I hope to have some time for this later today, if Laurent
> doesn't beat me at it.

I work on PowerPC- and I2C-related code during week days only. Sorry for the
delay.

> > Minor cleanup and reorg of the lm75 code.
> > 
> >  - Kconfig provides a larger list of lm75-compatible chips
> > 
> >  - A top comment now says what the driver does (!) ... as in, just
> >    what sort of sensor is this??
> > 
> >  - Section comments now delineate the various sections of the driver:
> >    hwmon attributes, driver binding, register access, module glue.
> >    One driver binding function moved out of the attribute section,
> >    as did the driver struct itself.
> > 
> >  - Minor tweaks to legacy probe logic:  correct a comment, and
> >    remove a pointless variable.
> > 
> >  - Whitespace, linelength, and comment fixes.
> > 
> > This patch should include no functional changes.
> > 
> > Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> > ---
> >  drivers/hwmon/Kconfig |   17 ++++++---
> >  drivers/hwmon/lm75.c  |   90 
++++++++++++++++++++++++++++----------------------
> >  2 files changed, 62 insertions(+), 45 deletions(-)
> > 
> > --- ngw.orig/drivers/hwmon/Kconfig	2008-04-18 11:16:46.000000000 -0700
> > +++ ngw/drivers/hwmon/Kconfig	2008-04-18 11:24:59.000000000 -0700
> > @@ -380,13 +380,18 @@ config SENSORS_LM75
> >  	tristate "National Semiconductor LM75 and compatibles"
> >  	depends on I2C
> >  	help
> > -	  If you say yes here you get support for National Semiconductor LM75
> > -	  sensor chips and clones: Dallas Semiconductor DS75 and DS1775 (in
> > -	  9-bit precision mode), and TelCom (now Microchip) TCN75.
> > +	  If you say yes here you get support for one common type of
> > +	  temperature sensor chip, with models including:
> >  
> > -	  The DS75 and DS1775 in 10- to 12-bit precision modes will require
> > -	  a force module parameter. The driver will not handle the extra
> > -	  precision anyhow.
> > +		- Dallas Semiconductor DS75 and DS1775
> > +		- Maxim MAX6625 and MAX6626
> > +		- Microchip MCP980x
> > +		- National Semiconductor LM75 and LM75A
> 
> The LM75A is actually a Philips (NXP) part, not a National
> Semiconductor part.
> 
> > +		- ST Microelectronics STDS75
> > +		- TelCom (now Microchip) TCN75
> > +		- Texas Instruments TMP100, TMP101, TMP75, TMP175, TMP275
> > +
> > +	  Most of these chips will require a "force" module parameter.
> >  
> >  	  This driver can also be built as a module.  If so, the module
> >  	  will be called lm75.
> 
> But anyway, all the rest is correct so
> 
> Acked-by: Jean Delvare <khali@linux-fr.org>

Acked-by: Laurent Pinchart <laurentp@cse-semaphore.com>

-- 
Laurent Pinchart
CSE Semaphore Belgium

Chaussee de Bruxelles, 732A
B-1410 Waterloo
Belgium

T +32 (2) 387 42 59
F +32 (2) 387 42 75

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors