[lm-sensors] [PATCH 3/3] lm87: Add support for configuration

[lm-sensors] [PATCH 3/3] lm87: Add support for configuration

[Ben Hutchings]

The lm87 driver normally assumes that firmware configured the chip
correctly.  Since this is not always the case, alllow platform code to
set the channel register value via platform_data.  All other
configuration registers can be changed after driver initialisation.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 Documentation/hwmon/lm87 |    9 ++++-----
 drivers/hwmon/lm87.c     |   20 +++++++++++++-------
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/Documentation/hwmon/lm87 b/Documentation/hwmon/lm87
index ec27aa1..6b47b67 100644
--- a/Documentation/hwmon/lm87
+++ b/Documentation/hwmon/lm87
@@ -65,11 +65,10 @@ The LM87 has four pins which can serve one of two possible functions,
 depending on the hardware configuration.
 
 Some functions share pins, so not all functions are available at the same
-time. Which are depends on the hardware setup. This driver assumes that
-the BIOS configured the chip correctly. In that respect, it differs from
-the original driver (from lm_sensors for Linux 2.4), which would force the
-LM87 to an arbitrary, compile-time chosen mode, regardless of the actual
-chipset wiring.
+time. Which are depends on the hardware setup. This driver normally
+assumes that firmware configured the chip correctly. Where this is not
+the case, platform code must set the I2C client's platform_data to point
+to a u8 value to be written to the channel register.
 
 For reference, here is the list of exclusive functions:
  - in0+in5 (default) or temp3
diff --git a/drivers/hwmon/lm87.c b/drivers/hwmon/lm87.c
index 695f5f2..197a842 100644
--- a/drivers/hwmon/lm87.c
+++ b/drivers/hwmon/lm87.c
@@ -21,11 +21,10 @@
  *   http://www.national.com/pf/LM/LM87.html
  *
  * Some functions share pins, so not all functions are available at the same
- * time. Which are depends on the hardware setup. This driver assumes that
- * the BIOS configured the chip correctly. In that respect, it  differs from
- * the original driver (from lm_sensors for Linux 2.4), which would force the
- * LM87 to an arbitrary, compile-time chosen mode, regardless of the actual
- * chipset wiring.
+ * time. Which are depends on the hardware setup. This driver normally
+ * assumes that firmware configured the chip correctly. Where this is not
+ * the case, platform code must set the I2C client's platform_data to point
+ * to a u8 value to be written to the channel register.
  * For reference, here is the list of exclusive functions:
  *  - in0+in5 (default) or temp3
  *  - fan1 (default) or in6
@@ -843,8 +842,15 @@ static void lm87_init_client(struct i2c_client *client)
 {
 	struct lm87_data *data = i2c_get_clientdata(client);
 
-	data->channel = lm87_read_value(client, LM87_REG_CHANNEL_MODE);
-	data->config = lm87_read_value(client, LM87_REG_CONFIG);
+	if (client->dev.platform_data) {
+		data->channel = *(u8 *)client->dev.platform_data;
+		lm87_write_value(client,
+				 LM87_REG_CHANNEL_MODE, data->channel);
+		data->config = 0x08; /* reset value */
+	} else {
+		data->channel = lm87_read_value(client, LM87_REG_CHANNEL_MODE);
+		data->config = lm87_read_value(client, LM87_REG_CONFIG);
+	}
 
 	if (!(data->config & 0x01)) {
 		int i;

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

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

Re: [lm-sensors] [PATCH 3/3] lm87: Add support for configuration

[Jean Delvare]

Hi Ben,

On Wed, 20 Aug 2008 21:21:14 +0100, Ben Hutchings wrote:
> The lm87 driver normally assumes that firmware configured the chip
> correctly.  Since this is not always the case, alllow platform code to
> set the channel register value via platform_data.  All other
> configuration registers can be changed after driver initialisation.
> 
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
>  Documentation/hwmon/lm87 |    9 ++++-----
>  drivers/hwmon/lm87.c     |   20 +++++++++++++-------
>  2 files changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/hwmon/lm87 b/Documentation/hwmon/lm87
> index ec27aa1..6b47b67 100644
> --- a/Documentation/hwmon/lm87
> +++ b/Documentation/hwmon/lm87
> @@ -65,11 +65,10 @@ The LM87 has four pins which can serve one of two possible functions,
>  depending on the hardware configuration.
>  
>  Some functions share pins, so not all functions are available at the same
> -time. Which are depends on the hardware setup. This driver assumes that
> -the BIOS configured the chip correctly. In that respect, it differs from
> -the original driver (from lm_sensors for Linux 2.4), which would force the
> -LM87 to an arbitrary, compile-time chosen mode, regardless of the actual
> -chipset wiring.
> +time. Which are depends on the hardware setup. This driver normally
> +assumes that firmware configured the chip correctly. Where this is not
> +the case, platform code must set the I2C client's platform_data to point
> +to a u8 value to be written to the channel register.
>  
>  For reference, here is the list of exclusive functions:
>   - in0+in5 (default) or temp3
> diff --git a/drivers/hwmon/lm87.c b/drivers/hwmon/lm87.c
> index 695f5f2..197a842 100644
> --- a/drivers/hwmon/lm87.c
> +++ b/drivers/hwmon/lm87.c
> @@ -21,11 +21,10 @@
>   *   http://www.national.com/pf/LM/LM87.html
>   *
>   * Some functions share pins, so not all functions are available at the same
> - * time. Which are depends on the hardware setup. This driver assumes that
> - * the BIOS configured the chip correctly. In that respect, it  differs from
> - * the original driver (from lm_sensors for Linux 2.4), which would force the
> - * LM87 to an arbitrary, compile-time chosen mode, regardless of the actual
> - * chipset wiring.
> + * time. Which are depends on the hardware setup. This driver normally
> + * assumes that firmware configured the chip correctly. Where this is not
> + * the case, platform code must set the I2C client's platform_data to point
> + * to a u8 value to be written to the channel register.
>   * For reference, here is the list of exclusive functions:
>   *  - in0+in5 (default) or temp3
>   *  - fan1 (default) or in6
> @@ -843,8 +842,15 @@ static void lm87_init_client(struct i2c_client *client)
>  {
>  	struct lm87_data *data = i2c_get_clientdata(client);
>  
> -	data->channel = lm87_read_value(client, LM87_REG_CHANNEL_MODE);
> -	data->config = lm87_read_value(client, LM87_REG_CONFIG);
> +	if (client->dev.platform_data) {
> +		data->channel = *(u8 *)client->dev.platform_data;
> +		lm87_write_value(client,
> +				 LM87_REG_CHANNEL_MODE, data->channel);
> +		data->config = 0x08; /* reset value */

I don't get the point of hard-coding this value. Why don't you read it
from the chip as you do in the no-platform-data case? This would be
more flexible and safer.

> +	} else {
> +		data->channel = lm87_read_value(client, LM87_REG_CHANNEL_MODE);
> +		data->config = lm87_read_value(client, LM87_REG_CONFIG);
> +	}
>  
>  	if (!(data->config & 0x01)) {
>  		int i;
> 

Other than that, this patch looks OK to me.

-- 
Jean Delvare

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