[PATCH v5] hwmon: (lm90) split set&show temp as common codes

[PATCH v5] hwmon: (lm90) split set&show temp as common codes

[Wei Ni]

Split set&show temp codes as common functions, so we can use it
directly when implement linux thermal framework.
And handle error return value for the lm90_select_remote_channel
and write_tempx, then set_temp8 and set_temp11 could return it
to user-space.

Signed-off-by: Wei Ni <wni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/hwmon/lm90.c |  166 ++++++++++++++++++++++++++++++++++----------------
 1 file changed, 113 insertions(+), 53 deletions(-)

diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index 3f51680..82a1ca15 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -424,20 +424,29 @@ static int lm90_read16(struct i2c_client *client, u8 regh, u8 regl, u16 *value)
  * various registers have different meanings as a result of selecting a
  * non-default remote channel.
  */
-static inline void lm90_select_remote_channel(struct i2c_client *client,
+static inline int lm90_select_remote_channel(struct i2c_client *client,
 					      struct lm90_data *data,
 					      int channel)
 {
 	u8 config;
+	int err;
 
 	if (data->kind == max6696) {
 		lm90_read_reg(client, LM90_REG_R_CONFIG1, &config);
 		config &= ~0x08;
 		if (channel)
 			config |= 0x08;
-		i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1,
-					  config);
+		err = i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1,
+						config);
+		if (err < 0) {
+			dev_err(&client->dev,
+				"Failed to select remote channel %d, err %d\n",
+				channel, err);
+			return err;
+		}
 	}
+
+	return 0;
 }
 
 /*
@@ -704,29 +713,34 @@ static u16 temp_to_u16_adt7461(struct lm90_data *data, long val)
  * Sysfs stuff
  */
 
-static ssize_t show_temp8(struct device *dev, struct device_attribute *devattr,
-			  char *buf)
+static int read_temp8(struct device *dev, int index)
 {
-	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
 	struct lm90_data *data = lm90_update_device(dev);
 	int temp;
 
 	if (data->kind == adt7461)
-		temp = temp_from_u8_adt7461(data, data->temp8[attr->index]);
+		temp = temp_from_u8_adt7461(data, data->temp8[index]);
 	else if (data->kind == max6646)
-		temp = temp_from_u8(data->temp8[attr->index]);
+		temp = temp_from_u8(data->temp8[index]);
 	else
-		temp = temp_from_s8(data->temp8[attr->index]);
+		temp = temp_from_s8(data->temp8[index]);
 
 	/* +16 degrees offset for temp2 for the LM99 */
-	if (data->kind == lm99 && attr->index == 3)
+	if (data->kind == lm99 && index == 3)
 		temp += 16000;
 
-	return sprintf(buf, "%d\n", temp);
+	return temp;
 }
 
-static ssize_t set_temp8(struct device *dev, struct device_attribute *devattr,
-			 const char *buf, size_t count)
+static ssize_t show_temp8(struct device *dev, struct device_attribute *devattr,
+			  char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+
+	return sprintf(buf, "%d\n", read_temp8(dev, attr->index));
+}
+
+static int write_temp8(struct device *dev, int index, long val)
 {
 	static const u8 reg[8] = {
 		LM90_REG_W_LOCAL_LOW,
@@ -739,60 +753,79 @@ static ssize_t set_temp8(struct device *dev, struct device_attribute *devattr,
 		MAX6659_REG_W_REMOTE_EMERG,
 	};
 
-	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
 	struct i2c_client *client = to_i2c_client(dev);
 	struct lm90_data *data = i2c_get_clientdata(client);
-	int nr = attr->index;
-	long val;
 	int err;
 
-	err = kstrtol(buf, 10, &val);
-	if (err < 0)
-		return err;
-
 	/* +16 degrees offset for temp2 for the LM99 */
-	if (data->kind == lm99 && attr->index == 3)
+	if (data->kind == lm99 && index == 3)
 		val -= 16000;
 
 	mutex_lock(&data->update_lock);
 	if (data->kind == adt7461)
-		data->temp8[nr] = temp_to_u8_adt7461(data, val);
+		data->temp8[index] = temp_to_u8_adt7461(data, val);
 	else if (data->kind == max6646)
-		data->temp8[nr] = temp_to_u8(val);
+		data->temp8[index] = temp_to_u8(val);
 	else
-		data->temp8[nr] = temp_to_s8(val);
-
-	lm90_select_remote_channel(client, data, nr >= 6);
-	i2c_smbus_write_byte_data(client, reg[nr], data->temp8[nr]);
-	lm90_select_remote_channel(client, data, 0);
+		data->temp8[index] = temp_to_s8(val);
 
+	if ((err = lm90_select_remote_channel(client, data, index >= 6)) ||
+	    (err = i2c_smbus_write_byte_data(client, reg[index], data->temp8[index])) ||
+	    (err = lm90_select_remote_channel(client, data, 0))) {
+		dev_err(dev, "write_temp8 failed %d\n", err);
+	}
 	mutex_unlock(&data->update_lock);
+
+	return err;
+}
+
+static ssize_t set_temp8(struct device *dev, struct device_attribute *devattr,
+			 const char *buf, size_t count)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	int index = attr->index;
+	long val;
+	int err;
+
+	err = kstrtol(buf, 10, &val);
+	if (err < 0)
+		return err;
+
+	err = write_temp8(dev, index, val);
+	if (err < 0)
+		return err;
+
 	return count;
 }
 
-static ssize_t show_temp11(struct device *dev, struct device_attribute *devattr,
-			   char *buf)
+static int read_temp11(struct device *dev, int index)
 {
-	struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
 	struct lm90_data *data = lm90_update_device(dev);
 	int temp;
 
 	if (data->kind == adt7461)
-		temp = temp_from_u16_adt7461(data, data->temp11[attr->index]);
+		temp = temp_from_u16_adt7461(data, data->temp11[index]);
 	else if (data->kind == max6646)
-		temp = temp_from_u16(data->temp11[attr->index]);
+		temp = temp_from_u16(data->temp11[index]);
 	else
-		temp = temp_from_s16(data->temp11[attr->index]);
+		temp = temp_from_s16(data->temp11[index]);
 
 	/* +16 degrees offset for temp2 for the LM99 */
-	if (data->kind == lm99 &&  attr->index <= 2)
+	if (data->kind == lm99 && index <= 2)
 		temp += 16000;
 
-	return sprintf(buf, "%d\n", temp);
+	return temp;
 }
 
-static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr,
-			  const char *buf, size_t count)
+static ssize_t show_temp11(struct device *dev, struct device_attribute *devattr,
+			   char *buf)
+{
+	struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
+
+	return sprintf(buf, "%d\n", read_temp11(dev, attr->index));
+}
+
+static int write_temp11(struct device *dev, int nr, int index, long val)
 {
 	struct {
 		u8 high;
@@ -806,18 +839,10 @@ static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr,
 		{ LM90_REG_W_REMOTE_HIGHH, LM90_REG_W_REMOTE_HIGHL, 1 }
 	};
 
-	struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
 	struct i2c_client *client = to_i2c_client(dev);
 	struct lm90_data *data = i2c_get_clientdata(client);
-	int nr = attr->nr;
-	int index = attr->index;
-	long val;
 	int err;
 
-	err = kstrtol(buf, 10, &val);
-	if (err < 0)
-		return err;
-
 	/* +16 degrees offset for temp2 for the LM99 */
 	if (data->kind == lm99 && index <= 2)
 		val -= 16000;
@@ -832,15 +857,50 @@ static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr,
 	else
 		data->temp11[index] = temp_to_s8(val) << 8;
 
-	lm90_select_remote_channel(client, data, reg[nr].channel);
-	i2c_smbus_write_byte_data(client, reg[nr].high,
-				  data->temp11[index] >> 8);
-	if (data->flags & LM90_HAVE_REM_LIMIT_EXT)
-		i2c_smbus_write_byte_data(client, reg[nr].low,
-					  data->temp11[index] & 0xff);
-	lm90_select_remote_channel(client, data, 0);
+	err = lm90_select_remote_channel(client, data, reg[nr].channel);
+	if (err)
+		goto error;
+
+	err = i2c_smbus_write_byte_data(client, reg[nr].high,
+					data->temp11[index] >> 8);
+	if (err)
+		goto error;
+
+	if (data->flags & LM90_HAVE_REM_LIMIT_EXT) {
+		err = i2c_smbus_write_byte_data(client, reg[nr].low,
+						data->temp11[index] & 0xff);
+		if (err)
+			goto error;
+	}
+
+	err = lm90_select_remote_channel(client, data, 0);
+
+error:
+	if (err)
+		dev_err(dev, "write_temp11 failed %d\n", err);
 
 	mutex_unlock(&data->update_lock);
+
+	return err;
+}
+
+static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr,
+			  const char *buf, size_t count)
+{
+	struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
+	int nr = attr->nr;
+	int index = attr->index;
+	long val;
+	int err;
+
+	err = kstrtol(buf, 10, &val);
+	if (err < 0)
+		return err;
+
+	err = write_temp11(dev, nr, index, val);
+	if (err < 0)
+		return err;
+
 	return count;
 }
 
-- 
1.7.9.5

[lm-sensors] [PATCH v5] hwmon: (lm90) split set&show temp as common codes

[Wei Ni]

Split set&show temp codes as common functions, so we can use it
directly when implement linux thermal framework.
And handle error return value for the lm90_select_remote_channel
and write_tempx, then set_temp8 and set_temp11 could return it
to user-space.

Signed-off-by: Wei Ni <wni@nvidia.com>
---
 drivers/hwmon/lm90.c |  166 ++++++++++++++++++++++++++++++++++----------------
 1 file changed, 113 insertions(+), 53 deletions(-)

diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index 3f51680..82a1ca15 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -424,20 +424,29 @@ static int lm90_read16(struct i2c_client *client, u8 regh, u8 regl, u16 *value)
  * various registers have different meanings as a result of selecting a
  * non-default remote channel.
  */
-static inline void lm90_select_remote_channel(struct i2c_client *client,
+static inline int lm90_select_remote_channel(struct i2c_client *client,
 					      struct lm90_data *data,
 					      int channel)
 {
 	u8 config;
+	int err;
 
 	if (data->kind = max6696) {
 		lm90_read_reg(client, LM90_REG_R_CONFIG1, &config);
 		config &= ~0x08;
 		if (channel)
 			config |= 0x08;
-		i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1,
-					  config);
+		err = i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1,
+						config);
+		if (err < 0) {
+			dev_err(&client->dev,
+				"Failed to select remote channel %d, err %d\n",
+				channel, err);
+			return err;
+		}
 	}
+
+	return 0;
 }
 
 /*
@@ -704,29 +713,34 @@ static u16 temp_to_u16_adt7461(struct lm90_data *data, long val)
  * Sysfs stuff
  */
 
-static ssize_t show_temp8(struct device *dev, struct device_attribute *devattr,
-			  char *buf)
+static int read_temp8(struct device *dev, int index)
 {
-	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
 	struct lm90_data *data = lm90_update_device(dev);
 	int temp;
 
 	if (data->kind = adt7461)
-		temp = temp_from_u8_adt7461(data, data->temp8[attr->index]);
+		temp = temp_from_u8_adt7461(data, data->temp8[index]);
 	else if (data->kind = max6646)
-		temp = temp_from_u8(data->temp8[attr->index]);
+		temp = temp_from_u8(data->temp8[index]);
 	else
-		temp = temp_from_s8(data->temp8[attr->index]);
+		temp = temp_from_s8(data->temp8[index]);
 
 	/* +16 degrees offset for temp2 for the LM99 */
-	if (data->kind = lm99 && attr->index = 3)
+	if (data->kind = lm99 && index = 3)
 		temp += 16000;
 
-	return sprintf(buf, "%d\n", temp);
+	return temp;
 }
 
-static ssize_t set_temp8(struct device *dev, struct device_attribute *devattr,
-			 const char *buf, size_t count)
+static ssize_t show_temp8(struct device *dev, struct device_attribute *devattr,
+			  char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+
+	return sprintf(buf, "%d\n", read_temp8(dev, attr->index));
+}
+
+static int write_temp8(struct device *dev, int index, long val)
 {
 	static const u8 reg[8] = {
 		LM90_REG_W_LOCAL_LOW,
@@ -739,60 +753,79 @@ static ssize_t set_temp8(struct device *dev, struct device_attribute *devattr,
 		MAX6659_REG_W_REMOTE_EMERG,
 	};
 
-	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
 	struct i2c_client *client = to_i2c_client(dev);
 	struct lm90_data *data = i2c_get_clientdata(client);
-	int nr = attr->index;
-	long val;
 	int err;
 
-	err = kstrtol(buf, 10, &val);
-	if (err < 0)
-		return err;
-
 	/* +16 degrees offset for temp2 for the LM99 */
-	if (data->kind = lm99 && attr->index = 3)
+	if (data->kind = lm99 && index = 3)
 		val -= 16000;
 
 	mutex_lock(&data->update_lock);
 	if (data->kind = adt7461)
-		data->temp8[nr] = temp_to_u8_adt7461(data, val);
+		data->temp8[index] = temp_to_u8_adt7461(data, val);
 	else if (data->kind = max6646)
-		data->temp8[nr] = temp_to_u8(val);
+		data->temp8[index] = temp_to_u8(val);
 	else
-		data->temp8[nr] = temp_to_s8(val);
-
-	lm90_select_remote_channel(client, data, nr >= 6);
-	i2c_smbus_write_byte_data(client, reg[nr], data->temp8[nr]);
-	lm90_select_remote_channel(client, data, 0);
+		data->temp8[index] = temp_to_s8(val);
 
+	if ((err = lm90_select_remote_channel(client, data, index >= 6)) ||
+	    (err = i2c_smbus_write_byte_data(client, reg[index], data->temp8[index])) ||
+	    (err = lm90_select_remote_channel(client, data, 0))) {
+		dev_err(dev, "write_temp8 failed %d\n", err);
+	}
 	mutex_unlock(&data->update_lock);
+
+	return err;
+}
+
+static ssize_t set_temp8(struct device *dev, struct device_attribute *devattr,
+			 const char *buf, size_t count)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	int index = attr->index;
+	long val;
+	int err;
+
+	err = kstrtol(buf, 10, &val);
+	if (err < 0)
+		return err;
+
+	err = write_temp8(dev, index, val);
+	if (err < 0)
+		return err;
+
 	return count;
 }
 
-static ssize_t show_temp11(struct device *dev, struct device_attribute *devattr,
-			   char *buf)
+static int read_temp11(struct device *dev, int index)
 {
-	struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
 	struct lm90_data *data = lm90_update_device(dev);
 	int temp;
 
 	if (data->kind = adt7461)
-		temp = temp_from_u16_adt7461(data, data->temp11[attr->index]);
+		temp = temp_from_u16_adt7461(data, data->temp11[index]);
 	else if (data->kind = max6646)
-		temp = temp_from_u16(data->temp11[attr->index]);
+		temp = temp_from_u16(data->temp11[index]);
 	else
-		temp = temp_from_s16(data->temp11[attr->index]);
+		temp = temp_from_s16(data->temp11[index]);
 
 	/* +16 degrees offset for temp2 for the LM99 */
-	if (data->kind = lm99 &&  attr->index <= 2)
+	if (data->kind = lm99 && index <= 2)
 		temp += 16000;
 
-	return sprintf(buf, "%d\n", temp);
+	return temp;
 }
 
-static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr,
-			  const char *buf, size_t count)
+static ssize_t show_temp11(struct device *dev, struct device_attribute *devattr,
+			   char *buf)
+{
+	struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
+
+	return sprintf(buf, "%d\n", read_temp11(dev, attr->index));
+}
+
+static int write_temp11(struct device *dev, int nr, int index, long val)
 {
 	struct {
 		u8 high;
@@ -806,18 +839,10 @@ static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr,
 		{ LM90_REG_W_REMOTE_HIGHH, LM90_REG_W_REMOTE_HIGHL, 1 }
 	};
 
-	struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
 	struct i2c_client *client = to_i2c_client(dev);
 	struct lm90_data *data = i2c_get_clientdata(client);
-	int nr = attr->nr;
-	int index = attr->index;
-	long val;
 	int err;
 
-	err = kstrtol(buf, 10, &val);
-	if (err < 0)
-		return err;
-
 	/* +16 degrees offset for temp2 for the LM99 */
 	if (data->kind = lm99 && index <= 2)
 		val -= 16000;
@@ -832,15 +857,50 @@ static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr,
 	else
 		data->temp11[index] = temp_to_s8(val) << 8;
 
-	lm90_select_remote_channel(client, data, reg[nr].channel);
-	i2c_smbus_write_byte_data(client, reg[nr].high,
-				  data->temp11[index] >> 8);
-	if (data->flags & LM90_HAVE_REM_LIMIT_EXT)
-		i2c_smbus_write_byte_data(client, reg[nr].low,
-					  data->temp11[index] & 0xff);
-	lm90_select_remote_channel(client, data, 0);
+	err = lm90_select_remote_channel(client, data, reg[nr].channel);
+	if (err)
+		goto error;
+
+	err = i2c_smbus_write_byte_data(client, reg[nr].high,
+					data->temp11[index] >> 8);
+	if (err)
+		goto error;
+
+	if (data->flags & LM90_HAVE_REM_LIMIT_EXT) {
+		err = i2c_smbus_write_byte_data(client, reg[nr].low,
+						data->temp11[index] & 0xff);
+		if (err)
+			goto error;
+	}
+
+	err = lm90_select_remote_channel(client, data, 0);
+
+error:
+	if (err)
+		dev_err(dev, "write_temp11 failed %d\n", err);
 
 	mutex_unlock(&data->update_lock);
+
+	return err;
+}
+
+static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr,
+			  const char *buf, size_t count)
+{
+	struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
+	int nr = attr->nr;
+	int index = attr->index;
+	long val;
+	int err;
+
+	err = kstrtol(buf, 10, &val);
+	if (err < 0)
+		return err;
+
+	err = write_temp11(dev, nr, index, val);
+	if (err < 0)
+		return err;
+
 	return count;
 }
 
-- 
1.7.9.5


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

Re: [PATCH v5] hwmon: (lm90) split set&show temp as common codes

[Wei Ni]

The previous version is in https://lkml.org/lkml/2013/8/6/170 .

Changes from v4:
Couple of changes to get right error codes, as per Jean's review.

On 10/08/2013 03:04 PM, Wei Ni wrote:
> Split set&show temp codes as common functions, so we can use it
> directly when implement linux thermal framework.
> And handle error return value for the lm90_select_remote_channel
> and write_tempx, then set_temp8 and set_temp11 could return it
> to user-space.
> 
> Signed-off-by: Wei Ni <wni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/hwmon/lm90.c |  166 ++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 113 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
> index 3f51680..82a1ca15 100644
> --- a/drivers/hwmon/lm90.c
> +++ b/drivers/hwmon/lm90.c
> @@ -424,20 +424,29 @@ static int lm90_read16(struct i2c_client *client, u8 regh, u8 regl, u16 *value)
>   * various registers have different meanings as a result of selecting a
>   * non-default remote channel.
>   */
> -static inline void lm90_select_remote_channel(struct i2c_client *client,
> +static inline int lm90_select_remote_channel(struct i2c_client *client,
>  					      struct lm90_data *data,
>  					      int channel)
>  {
>  	u8 config;
> +	int err;
>  
>  	if (data->kind == max6696) {
>  		lm90_read_reg(client, LM90_REG_R_CONFIG1, &config);
>  		config &= ~0x08;
>  		if (channel)
>  			config |= 0x08;
> -		i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1,
> -					  config);
> +		err = i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1,
> +						config);
> +		if (err < 0) {
> +			dev_err(&client->dev,
> +				"Failed to select remote channel %d, err %d\n",
> +				channel, err);
> +			return err;
> +		}
>  	}
> +
> +	return 0;
>  }
>  
>  /*
> @@ -704,29 +713,34 @@ static u16 temp_to_u16_adt7461(struct lm90_data *data, long val)
>   * Sysfs stuff
>   */
>  
> -static ssize_t show_temp8(struct device *dev, struct device_attribute *devattr,
> -			  char *buf)
> +static int read_temp8(struct device *dev, int index)
>  {
> -	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
>  	struct lm90_data *data = lm90_update_device(dev);
>  	int temp;
>  
>  	if (data->kind == adt7461)
> -		temp = temp_from_u8_adt7461(data, data->temp8[attr->index]);
> +		temp = temp_from_u8_adt7461(data, data->temp8[index]);
>  	else if (data->kind == max6646)
> -		temp = temp_from_u8(data->temp8[attr->index]);
> +		temp = temp_from_u8(data->temp8[index]);
>  	else
> -		temp = temp_from_s8(data->temp8[attr->index]);
> +		temp = temp_from_s8(data->temp8[index]);
>  
>  	/* +16 degrees offset for temp2 for the LM99 */
> -	if (data->kind == lm99 && attr->index == 3)
> +	if (data->kind == lm99 && index == 3)
>  		temp += 16000;
>  
> -	return sprintf(buf, "%d\n", temp);
> +	return temp;
>  }
>  
> -static ssize_t set_temp8(struct device *dev, struct device_attribute *devattr,
> -			 const char *buf, size_t count)
> +static ssize_t show_temp8(struct device *dev, struct device_attribute *devattr,
> +			  char *buf)
> +{
> +	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> +
> +	return sprintf(buf, "%d\n", read_temp8(dev, attr->index));
> +}
> +
> +static int write_temp8(struct device *dev, int index, long val)
>  {
>  	static const u8 reg[8] = {
>  		LM90_REG_W_LOCAL_LOW,
> @@ -739,60 +753,79 @@ static ssize_t set_temp8(struct device *dev, struct device_attribute *devattr,
>  		MAX6659_REG_W_REMOTE_EMERG,
>  	};
>  
> -	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
>  	struct i2c_client *client = to_i2c_client(dev);
>  	struct lm90_data *data = i2c_get_clientdata(client);
> -	int nr = attr->index;
> -	long val;
>  	int err;
>  
> -	err = kstrtol(buf, 10, &val);
> -	if (err < 0)
> -		return err;
> -
>  	/* +16 degrees offset for temp2 for the LM99 */
> -	if (data->kind == lm99 && attr->index == 3)
> +	if (data->kind == lm99 && index == 3)
>  		val -= 16000;
>  
>  	mutex_lock(&data->update_lock);
>  	if (data->kind == adt7461)
> -		data->temp8[nr] = temp_to_u8_adt7461(data, val);
> +		data->temp8[index] = temp_to_u8_adt7461(data, val);
>  	else if (data->kind == max6646)
> -		data->temp8[nr] = temp_to_u8(val);
> +		data->temp8[index] = temp_to_u8(val);
>  	else
> -		data->temp8[nr] = temp_to_s8(val);
> -
> -	lm90_select_remote_channel(client, data, nr >= 6);
> -	i2c_smbus_write_byte_data(client, reg[nr], data->temp8[nr]);
> -	lm90_select_remote_channel(client, data, 0);
> +		data->temp8[index] = temp_to_s8(val);
>  
> +	if ((err = lm90_select_remote_channel(client, data, index >= 6)) ||
> +	    (err = i2c_smbus_write_byte_data(client, reg[index], data->temp8[index])) ||
> +	    (err = lm90_select_remote_channel(client, data, 0))) {
> +		dev_err(dev, "write_temp8 failed %d\n", err);
> +	}
>  	mutex_unlock(&data->update_lock);
> +
> +	return err;
> +}
> +
> +static ssize_t set_temp8(struct device *dev, struct device_attribute *devattr,
> +			 const char *buf, size_t count)
> +{
> +	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> +	int index = attr->index;
> +	long val;
> +	int err;
> +
> +	err = kstrtol(buf, 10, &val);
> +	if (err < 0)
> +		return err;
> +
> +	err = write_temp8(dev, index, val);
> +	if (err < 0)
> +		return err;
> +
>  	return count;
>  }
>  
> -static ssize_t show_temp11(struct device *dev, struct device_attribute *devattr,
> -			   char *buf)
> +static int read_temp11(struct device *dev, int index)
>  {
> -	struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
>  	struct lm90_data *data = lm90_update_device(dev);
>  	int temp;
>  
>  	if (data->kind == adt7461)
> -		temp = temp_from_u16_adt7461(data, data->temp11[attr->index]);
> +		temp = temp_from_u16_adt7461(data, data->temp11[index]);
>  	else if (data->kind == max6646)
> -		temp = temp_from_u16(data->temp11[attr->index]);
> +		temp = temp_from_u16(data->temp11[index]);
>  	else
> -		temp = temp_from_s16(data->temp11[attr->index]);
> +		temp = temp_from_s16(data->temp11[index]);
>  
>  	/* +16 degrees offset for temp2 for the LM99 */
> -	if (data->kind == lm99 &&  attr->index <= 2)
> +	if (data->kind == lm99 && index <= 2)
>  		temp += 16000;
>  
> -	return sprintf(buf, "%d\n", temp);
> +	return temp;
>  }
>  
> -static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr,
> -			  const char *buf, size_t count)
> +static ssize_t show_temp11(struct device *dev, struct device_attribute *devattr,
> +			   char *buf)
> +{
> +	struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
> +
> +	return sprintf(buf, "%d\n", read_temp11(dev, attr->index));
> +}
> +
> +static int write_temp11(struct device *dev, int nr, int index, long val)
>  {
>  	struct {
>  		u8 high;
> @@ -806,18 +839,10 @@ static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr,
>  		{ LM90_REG_W_REMOTE_HIGHH, LM90_REG_W_REMOTE_HIGHL, 1 }
>  	};
>  
> -	struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
>  	struct i2c_client *client = to_i2c_client(dev);
>  	struct lm90_data *data = i2c_get_clientdata(client);
> -	int nr = attr->nr;
> -	int index = attr->index;
> -	long val;
>  	int err;
>  
> -	err = kstrtol(buf, 10, &val);
> -	if (err < 0)
> -		return err;
> -
>  	/* +16 degrees offset for temp2 for the LM99 */
>  	if (data->kind == lm99 && index <= 2)
>  		val -= 16000;
> @@ -832,15 +857,50 @@ static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr,
>  	else
>  		data->temp11[index] = temp_to_s8(val) << 8;
>  
> -	lm90_select_remote_channel(client, data, reg[nr].channel);
> -	i2c_smbus_write_byte_data(client, reg[nr].high,
> -				  data->temp11[index] >> 8);
> -	if (data->flags & LM90_HAVE_REM_LIMIT_EXT)
> -		i2c_smbus_write_byte_data(client, reg[nr].low,
> -					  data->temp11[index] & 0xff);
> -	lm90_select_remote_channel(client, data, 0);
> +	err = lm90_select_remote_channel(client, data, reg[nr].channel);
> +	if (err)
> +		goto error;
> +
> +	err = i2c_smbus_write_byte_data(client, reg[nr].high,
> +					data->temp11[index] >> 8);
> +	if (err)
> +		goto error;
> +
> +	if (data->flags & LM90_HAVE_REM_LIMIT_EXT) {
> +		err = i2c_smbus_write_byte_data(client, reg[nr].low,
> +						data->temp11[index] & 0xff);
> +		if (err)
> +			goto error;
> +	}
> +
> +	err = lm90_select_remote_channel(client, data, 0);
> +
> +error:
> +	if (err)
> +		dev_err(dev, "write_temp11 failed %d\n", err);
>  
>  	mutex_unlock(&data->update_lock);
> +
> +	return err;
> +}
> +
> +static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr,
> +			  const char *buf, size_t count)
> +{
> +	struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
> +	int nr = attr->nr;
> +	int index = attr->index;
> +	long val;
> +	int err;
> +
> +	err = kstrtol(buf, 10, &val);
> +	if (err < 0)
> +		return err;
> +
> +	err = write_temp11(dev, nr, index, val);
> +	if (err < 0)
> +		return err;
> +
>  	return count;
>  }
>  
> 

Re: [lm-sensors] [PATCH v5] hwmon: (lm90) split set&show temp as common codes

[Wei Ni]

The previous version is in https://lkml.org/lkml/2013/8/6/170 .

Changes from v4:
Couple of changes to get right error codes, as per Jean's review.

On 10/08/2013 03:04 PM, Wei Ni wrote:
> Split set&show temp codes as common functions, so we can use it
> directly when implement linux thermal framework.
> And handle error return value for the lm90_select_remote_channel
> and write_tempx, then set_temp8 and set_temp11 could return it
> to user-space.
> 
> Signed-off-by: Wei Ni <wni@nvidia.com>
> ---
>  drivers/hwmon/lm90.c |  166 ++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 113 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
> index 3f51680..82a1ca15 100644
> --- a/drivers/hwmon/lm90.c
> +++ b/drivers/hwmon/lm90.c
> @@ -424,20 +424,29 @@ static int lm90_read16(struct i2c_client *client, u8 regh, u8 regl, u16 *value)
>   * various registers have different meanings as a result of selecting a
>   * non-default remote channel.
>   */
> -static inline void lm90_select_remote_channel(struct i2c_client *client,
> +static inline int lm90_select_remote_channel(struct i2c_client *client,
>  					      struct lm90_data *data,
>  					      int channel)
>  {
>  	u8 config;
> +	int err;
>  
>  	if (data->kind = max6696) {
>  		lm90_read_reg(client, LM90_REG_R_CONFIG1, &config);
>  		config &= ~0x08;
>  		if (channel)
>  			config |= 0x08;
> -		i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1,
> -					  config);
> +		err = i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1,
> +						config);
> +		if (err < 0) {
> +			dev_err(&client->dev,
> +				"Failed to select remote channel %d, err %d\n",
> +				channel, err);
> +			return err;
> +		}
>  	}
> +
> +	return 0;
>  }
>  
>  /*
> @@ -704,29 +713,34 @@ static u16 temp_to_u16_adt7461(struct lm90_data *data, long val)
>   * Sysfs stuff
>   */
>  
> -static ssize_t show_temp8(struct device *dev, struct device_attribute *devattr,
> -			  char *buf)
> +static int read_temp8(struct device *dev, int index)
>  {
> -	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
>  	struct lm90_data *data = lm90_update_device(dev);
>  	int temp;
>  
>  	if (data->kind = adt7461)
> -		temp = temp_from_u8_adt7461(data, data->temp8[attr->index]);
> +		temp = temp_from_u8_adt7461(data, data->temp8[index]);
>  	else if (data->kind = max6646)
> -		temp = temp_from_u8(data->temp8[attr->index]);
> +		temp = temp_from_u8(data->temp8[index]);
>  	else
> -		temp = temp_from_s8(data->temp8[attr->index]);
> +		temp = temp_from_s8(data->temp8[index]);
>  
>  	/* +16 degrees offset for temp2 for the LM99 */
> -	if (data->kind = lm99 && attr->index = 3)
> +	if (data->kind = lm99 && index = 3)
>  		temp += 16000;
>  
> -	return sprintf(buf, "%d\n", temp);
> +	return temp;
>  }
>  
> -static ssize_t set_temp8(struct device *dev, struct device_attribute *devattr,
> -			 const char *buf, size_t count)
> +static ssize_t show_temp8(struct device *dev, struct device_attribute *devattr,
> +			  char *buf)
> +{
> +	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> +
> +	return sprintf(buf, "%d\n", read_temp8(dev, attr->index));
> +}
> +
> +static int write_temp8(struct device *dev, int index, long val)
>  {
>  	static const u8 reg[8] = {
>  		LM90_REG_W_LOCAL_LOW,
> @@ -739,60 +753,79 @@ static ssize_t set_temp8(struct device *dev, struct device_attribute *devattr,
>  		MAX6659_REG_W_REMOTE_EMERG,
>  	};
>  
> -	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
>  	struct i2c_client *client = to_i2c_client(dev);
>  	struct lm90_data *data = i2c_get_clientdata(client);
> -	int nr = attr->index;
> -	long val;
>  	int err;
>  
> -	err = kstrtol(buf, 10, &val);
> -	if (err < 0)
> -		return err;
> -
>  	/* +16 degrees offset for temp2 for the LM99 */
> -	if (data->kind = lm99 && attr->index = 3)
> +	if (data->kind = lm99 && index = 3)
>  		val -= 16000;
>  
>  	mutex_lock(&data->update_lock);
>  	if (data->kind = adt7461)
> -		data->temp8[nr] = temp_to_u8_adt7461(data, val);
> +		data->temp8[index] = temp_to_u8_adt7461(data, val);
>  	else if (data->kind = max6646)
> -		data->temp8[nr] = temp_to_u8(val);
> +		data->temp8[index] = temp_to_u8(val);
>  	else
> -		data->temp8[nr] = temp_to_s8(val);
> -
> -	lm90_select_remote_channel(client, data, nr >= 6);
> -	i2c_smbus_write_byte_data(client, reg[nr], data->temp8[nr]);
> -	lm90_select_remote_channel(client, data, 0);
> +		data->temp8[index] = temp_to_s8(val);
>  
> +	if ((err = lm90_select_remote_channel(client, data, index >= 6)) ||
> +	    (err = i2c_smbus_write_byte_data(client, reg[index], data->temp8[index])) ||
> +	    (err = lm90_select_remote_channel(client, data, 0))) {
> +		dev_err(dev, "write_temp8 failed %d\n", err);
> +	}
>  	mutex_unlock(&data->update_lock);
> +
> +	return err;
> +}
> +
> +static ssize_t set_temp8(struct device *dev, struct device_attribute *devattr,
> +			 const char *buf, size_t count)
> +{
> +	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> +	int index = attr->index;
> +	long val;
> +	int err;
> +
> +	err = kstrtol(buf, 10, &val);
> +	if (err < 0)
> +		return err;
> +
> +	err = write_temp8(dev, index, val);
> +	if (err < 0)
> +		return err;
> +
>  	return count;
>  }
>  
> -static ssize_t show_temp11(struct device *dev, struct device_attribute *devattr,
> -			   char *buf)
> +static int read_temp11(struct device *dev, int index)
>  {
> -	struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
>  	struct lm90_data *data = lm90_update_device(dev);
>  	int temp;
>  
>  	if (data->kind = adt7461)
> -		temp = temp_from_u16_adt7461(data, data->temp11[attr->index]);
> +		temp = temp_from_u16_adt7461(data, data->temp11[index]);
>  	else if (data->kind = max6646)
> -		temp = temp_from_u16(data->temp11[attr->index]);
> +		temp = temp_from_u16(data->temp11[index]);
>  	else
> -		temp = temp_from_s16(data->temp11[attr->index]);
> +		temp = temp_from_s16(data->temp11[index]);
>  
>  	/* +16 degrees offset for temp2 for the LM99 */
> -	if (data->kind = lm99 &&  attr->index <= 2)
> +	if (data->kind = lm99 && index <= 2)
>  		temp += 16000;
>  
> -	return sprintf(buf, "%d\n", temp);
> +	return temp;
>  }
>  
> -static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr,
> -			  const char *buf, size_t count)
> +static ssize_t show_temp11(struct device *dev, struct device_attribute *devattr,
> +			   char *buf)
> +{
> +	struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
> +
> +	return sprintf(buf, "%d\n", read_temp11(dev, attr->index));
> +}
> +
> +static int write_temp11(struct device *dev, int nr, int index, long val)
>  {
>  	struct {
>  		u8 high;
> @@ -806,18 +839,10 @@ static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr,
>  		{ LM90_REG_W_REMOTE_HIGHH, LM90_REG_W_REMOTE_HIGHL, 1 }
>  	};
>  
> -	struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
>  	struct i2c_client *client = to_i2c_client(dev);
>  	struct lm90_data *data = i2c_get_clientdata(client);
> -	int nr = attr->nr;
> -	int index = attr->index;
> -	long val;
>  	int err;
>  
> -	err = kstrtol(buf, 10, &val);
> -	if (err < 0)
> -		return err;
> -
>  	/* +16 degrees offset for temp2 for the LM99 */
>  	if (data->kind = lm99 && index <= 2)
>  		val -= 16000;
> @@ -832,15 +857,50 @@ static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr,
>  	else
>  		data->temp11[index] = temp_to_s8(val) << 8;
>  
> -	lm90_select_remote_channel(client, data, reg[nr].channel);
> -	i2c_smbus_write_byte_data(client, reg[nr].high,
> -				  data->temp11[index] >> 8);
> -	if (data->flags & LM90_HAVE_REM_LIMIT_EXT)
> -		i2c_smbus_write_byte_data(client, reg[nr].low,
> -					  data->temp11[index] & 0xff);
> -	lm90_select_remote_channel(client, data, 0);
> +	err = lm90_select_remote_channel(client, data, reg[nr].channel);
> +	if (err)
> +		goto error;
> +
> +	err = i2c_smbus_write_byte_data(client, reg[nr].high,
> +					data->temp11[index] >> 8);
> +	if (err)
> +		goto error;
> +
> +	if (data->flags & LM90_HAVE_REM_LIMIT_EXT) {
> +		err = i2c_smbus_write_byte_data(client, reg[nr].low,
> +						data->temp11[index] & 0xff);
> +		if (err)
> +			goto error;
> +	}
> +
> +	err = lm90_select_remote_channel(client, data, 0);
> +
> +error:
> +	if (err)
> +		dev_err(dev, "write_temp11 failed %d\n", err);
>  
>  	mutex_unlock(&data->update_lock);
> +
> +	return err;
> +}
> +
> +static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr,
> +			  const char *buf, size_t count)
> +{
> +	struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
> +	int nr = attr->nr;
> +	int index = attr->index;
> +	long val;
> +	int err;
> +
> +	err = kstrtol(buf, 10, &val);
> +	if (err < 0)
> +		return err;
> +
> +	err = write_temp11(dev, nr, index, val);
> +	if (err < 0)
> +		return err;
> +
>  	return count;
>  }
>  
> 


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

Re: [PATCH v5] hwmon: (lm90) split set&show temp as common codes

[Jean Delvare]

On Tue, 8 Oct 2013 15:04:26 +0800, Wei Ni wrote:
> Split set&show temp codes as common functions, so we can use it
> directly when implement linux thermal framework.
> And handle error return value for the lm90_select_remote_channel
> and write_tempx, then set_temp8 and set_temp11 could return it
> to user-space.
> 
> Signed-off-by: Wei Ni <wni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/hwmon/lm90.c |  166 ++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 113 insertions(+), 53 deletions(-)
> (...)

Applied with minor edits, thanks.

-- 
Jean Delvare

Re: [lm-sensors] [PATCH v5] hwmon: (lm90) split set&show temp as common codes

[Jean Delvare]

On Tue, 8 Oct 2013 15:04:26 +0800, Wei Ni wrote:
> Split set&show temp codes as common functions, so we can use it
> directly when implement linux thermal framework.
> And handle error return value for the lm90_select_remote_channel
> and write_tempx, then set_temp8 and set_temp11 could return it
> to user-space.
> 
> Signed-off-by: Wei Ni <wni@nvidia.com>
> ---
>  drivers/hwmon/lm90.c |  166 ++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 113 insertions(+), 53 deletions(-)
> (...)

Applied with minor edits, thanks.

-- 
Jean Delvare

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

Re: [lm-sensors] [PATCH v5] hwmon: (lm90) split set&show temp as common codes

[Jean Delvare]

On Fri, 11 Oct 2013 09:17:41 +0200, Jean Delvare wrote:
> On Tue, 8 Oct 2013 15:04:26 +0800, Wei Ni wrote:
> > Split set&show temp codes as common functions, so we can use it
> > directly when implement linux thermal framework.
> > And handle error return value for the lm90_select_remote_channel
> > and write_tempx, then set_temp8 and set_temp11 could return it
> > to user-space.
> > 
> > Signed-off-by: Wei Ni <wni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > ---
> >  drivers/hwmon/lm90.c |  166 ++++++++++++++++++++++++++++++++++----------------
> >  1 file changed, 113 insertions(+), 53 deletions(-)
> > (...)
> 
> Applied with minor edits, thanks.

Wei, what's the status? With Guenter's recent cleanup of the lm90
driver, this patch no longer applies. I never sent it upstream as it
makes no sense without users of the new functions defined by the patch.
Will this ever happen?

For now I am discarding this patch. If it still needed, please rebase
on top of the current lm90 patch set:
http://jdelvare.nerim.net/devel/linux-3/jdelvare-hwmon/
and resend.

Thanks,
-- 
Jean Delvare

Re: [lm-sensors] [PATCH v5] hwmon: (lm90) split set&show temp as common codes

[Jean Delvare]

On Fri, 11 Oct 2013 09:17:41 +0200, Jean Delvare wrote:
> On Tue, 8 Oct 2013 15:04:26 +0800, Wei Ni wrote:
> > Split set&show temp codes as common functions, so we can use it
> > directly when implement linux thermal framework.
> > And handle error return value for the lm90_select_remote_channel
> > and write_tempx, then set_temp8 and set_temp11 could return it
> > to user-space.
> > 
> > Signed-off-by: Wei Ni <wni@nvidia.com>
> > ---
> >  drivers/hwmon/lm90.c |  166 ++++++++++++++++++++++++++++++++++----------------
> >  1 file changed, 113 insertions(+), 53 deletions(-)
> > (...)
> 
> Applied with minor edits, thanks.

Wei, what's the status? With Guenter's recent cleanup of the lm90
driver, this patch no longer applies. I never sent it upstream as it
makes no sense without users of the new functions defined by the patch.
Will this ever happen?

For now I am discarding this patch. If it still needed, please rebase
on top of the current lm90 patch set:
http://jdelvare.nerim.net/devel/linux-3/jdelvare-hwmon/
and resend.

Thanks,
-- 
Jean Delvare

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

Re: [lm-sensors] [PATCH v5] hwmon: (lm90) split set&show temp as common codes

[Guenter Roeck]

On 02/22/2014 02:47 AM, Jean Delvare wrote:
> On Fri, 11 Oct 2013 09:17:41 +0200, Jean Delvare wrote:
>> On Tue, 8 Oct 2013 15:04:26 +0800, Wei Ni wrote:
>>> Split set&show temp codes as common functions, so we can use it
>>> directly when implement linux thermal framework.
>>> And handle error return value for the lm90_select_remote_channel
>>> and write_tempx, then set_temp8 and set_temp11 could return it
>>> to user-space.
>>>
>>> Signed-off-by: Wei Ni <wni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>> ---
>>>   drivers/hwmon/lm90.c |  166 ++++++++++++++++++++++++++++++++++----------------
>>>   1 file changed, 113 insertions(+), 53 deletions(-)
>>> (...)
>>
>> Applied with minor edits, thanks.
>
> Wei, what's the status? With Guenter's recent cleanup of the lm90
> driver, this patch no longer applies. I never sent it upstream as it
> makes no sense without users of the new functions defined by the patch.
> Will this ever happen?
>
> For now I am discarding this patch. If it still needed, please rebase
> on top of the current lm90 patch set:
> http://jdelvare.nerim.net/devel/linux-3/jdelvare-hwmon/
> and resend.
>

I am still dreaming of somehow coming up with a new hwmon api that would move
the sysfs ABI into the hwmon core and only provide raw read/write functions
in the drivers. iio manages to do that, after all, so we should be able to
do it as well. Unfortunately everything I tried or have seen so far ended up
making the code more complex, defeating the idea.

On the other side, if we have more and more of those dual-mode drivers,
it might make sense to put more thought into it. After all, if the drivers
have to provide the raw access functions anyway, it would be neat if we
can just pass those to the hwmon core (and possibly instantiate the thermal
device from there and not from each driver).

Guenter

Re: [lm-sensors] [PATCH v5] hwmon: (lm90) split set&show temp as common codes

[Guenter Roeck]

On 02/22/2014 02:47 AM, Jean Delvare wrote:
> On Fri, 11 Oct 2013 09:17:41 +0200, Jean Delvare wrote:
>> On Tue, 8 Oct 2013 15:04:26 +0800, Wei Ni wrote:
>>> Split set&show temp codes as common functions, so we can use it
>>> directly when implement linux thermal framework.
>>> And handle error return value for the lm90_select_remote_channel
>>> and write_tempx, then set_temp8 and set_temp11 could return it
>>> to user-space.
>>>
>>> Signed-off-by: Wei Ni <wni@nvidia.com>
>>> ---
>>>   drivers/hwmon/lm90.c |  166 ++++++++++++++++++++++++++++++++++----------------
>>>   1 file changed, 113 insertions(+), 53 deletions(-)
>>> (...)
>>
>> Applied with minor edits, thanks.
>
> Wei, what's the status? With Guenter's recent cleanup of the lm90
> driver, this patch no longer applies. I never sent it upstream as it
> makes no sense without users of the new functions defined by the patch.
> Will this ever happen?
>
> For now I am discarding this patch. If it still needed, please rebase
> on top of the current lm90 patch set:
> http://jdelvare.nerim.net/devel/linux-3/jdelvare-hwmon/
> and resend.
>

I am still dreaming of somehow coming up with a new hwmon api that would move
the sysfs ABI into the hwmon core and only provide raw read/write functions
in the drivers. iio manages to do that, after all, so we should be able to
do it as well. Unfortunately everything I tried or have seen so far ended up
making the code more complex, defeating the idea.

On the other side, if we have more and more of those dual-mode drivers,
it might make sense to put more thought into it. After all, if the drivers
have to provide the raw access functions anyway, it would be neat if we
can just pass those to the hwmon core (and possibly instantiate the thermal
device from there and not from each driver).

Guenter


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

Re: [lm-sensors] [PATCH v5] hwmon: (lm90) split set&show temp as common codes

[Wei Ni]

On 02/22/2014 06:47 PM, Jean Delvare wrote:
> On Fri, 11 Oct 2013 09:17:41 +0200, Jean Delvare wrote:
>> On Tue, 8 Oct 2013 15:04:26 +0800, Wei Ni wrote:
>>> Split set&show temp codes as common functions, so we can use it
>>> directly when implement linux thermal framework.
>>> And handle error return value for the lm90_select_remote_channel
>>> and write_tempx, then set_temp8 and set_temp11 could return it
>>> to user-space.
>>>
>>> Signed-off-by: Wei Ni <wni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>> ---
>>>  drivers/hwmon/lm90.c |  166 ++++++++++++++++++++++++++++++++++----------------
>>>  1 file changed, 113 insertions(+), 53 deletions(-)
>>> (...)
>>
>> Applied with minor edits, thanks.
> 
> Wei, what's the status? With Guenter's recent cleanup of the lm90
> driver, this patch no longer applies. I never sent it upstream as it
> makes no sense without users of the new functions defined by the patch.
> Will this ever happen?
> 
> For now I am discarding this patch. If it still needed, please rebase
> on top of the current lm90 patch set:
> http://jdelvare.nerim.net/devel/linux-3/jdelvare-hwmon/
> and resend.

Hi, Jean
Sorry for no updates on this patch.

I noticed that in lm75.c, it expose itself as thermal zone device and
register on the thermal framework, so I will also implement it for
lm90.c, base on this patch.

Thanks.

> 
> Thanks,
> 

Re: [lm-sensors] [PATCH v5] hwmon: (lm90) split set&show temp as common codes

[Wei Ni]

On 02/22/2014 06:47 PM, Jean Delvare wrote:
> On Fri, 11 Oct 2013 09:17:41 +0200, Jean Delvare wrote:
>> On Tue, 8 Oct 2013 15:04:26 +0800, Wei Ni wrote:
>>> Split set&show temp codes as common functions, so we can use it
>>> directly when implement linux thermal framework.
>>> And handle error return value for the lm90_select_remote_channel
>>> and write_tempx, then set_temp8 and set_temp11 could return it
>>> to user-space.
>>>
>>> Signed-off-by: Wei Ni <wni@nvidia.com>
>>> ---
>>>  drivers/hwmon/lm90.c |  166 ++++++++++++++++++++++++++++++++++----------------
>>>  1 file changed, 113 insertions(+), 53 deletions(-)
>>> (...)
>>
>> Applied with minor edits, thanks.
> 
> Wei, what's the status? With Guenter's recent cleanup of the lm90
> driver, this patch no longer applies. I never sent it upstream as it
> makes no sense without users of the new functions defined by the patch.
> Will this ever happen?
> 
> For now I am discarding this patch. If it still needed, please rebase
> on top of the current lm90 patch set:
> http://jdelvare.nerim.net/devel/linux-3/jdelvare-hwmon/
> and resend.

Hi, Jean
Sorry for no updates on this patch.

I noticed that in lm75.c, it expose itself as thermal zone device and
register on the thermal framework, so I will also implement it for
lm90.c, base on this patch.

Thanks.

> 
> Thanks,
> 


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

Re: [PATCH v5] hwmon: (lm90) split set&show temp as common codes

[Wei Ni]

Hi, Jean
Sorry to trouble you, I recalled that you had applied this change, but I still
can't find it in the mainline.
Could you please check it or does I need to do anything else?

Thanks.
Wei.

On 2013年10月11日 15:17, Jean Delvare wrote:
> On Tue, 8 Oct 2013 15:04:26 +0800, Wei Ni wrote:
>> Split set&show temp codes as common functions, so we can use it
>> directly when implement linux thermal framework.
>> And handle error return value for the lm90_select_remote_channel
>> and write_tempx, then set_temp8 and set_temp11 could return it
>> to user-space.
>>
>> Signed-off-by: Wei Ni <wni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> ---
>>  drivers/hwmon/lm90.c |  166 ++++++++++++++++++++++++++++++++++----------------
>>  1 file changed, 113 insertions(+), 53 deletions(-)
>> (...)
> 
> Applied with minor edits, thanks.
> 

Re: [lm-sensors] [PATCH v5] hwmon: (lm90) split set&show temp as common codes

[Wei Ni]

SGksIEplYW4KU29ycnkgdG8gdHJvdWJsZSB5b3UsIEkgcmVjYWxsZWQgdGhhdCB5b3UgaGFkIGFw
cGxpZWQgdGhpcyBjaGFuZ2UsIGJ1dCBJIHN0aWxsCmNhbid0IGZpbmQgaXQgaW4gdGhlIG1haW5s
aW5lLgpDb3VsZCB5b3UgcGxlYXNlIGNoZWNrIGl0IG9yIGRvZXMgSSBuZWVkIHRvIGRvIGFueXRo
aW5nIGVsc2U/CgpUaGFua3MuCldlaS4KCk9uIDIwMTPlubQxMOaciDEx5pelIDE1OjE3LCBKZWFu
IERlbHZhcmUgd3JvdGU6Cj4gT24gVHVlLCA4IE9jdCAyMDEzIDE1OjA0OjI2ICswODAwLCBXZWkg
Tmkgd3JvdGU6Cj4+IFNwbGl0IHNldCZzaG93IHRlbXAgY29kZXMgYXMgY29tbW9uIGZ1bmN0aW9u
cywgc28gd2UgY2FuIHVzZSBpdAo+PiBkaXJlY3RseSB3aGVuIGltcGxlbWVudCBsaW51eCB0aGVy
bWFsIGZyYW1ld29yay4KPj4gQW5kIGhhbmRsZSBlcnJvciByZXR1cm4gdmFsdWUgZm9yIHRoZSBs
bTkwX3NlbGVjdF9yZW1vdGVfY2hhbm5lbAo+PiBhbmQgd3JpdGVfdGVtcHgsIHRoZW4gc2V0X3Rl
bXA4IGFuZCBzZXRfdGVtcDExIGNvdWxkIHJldHVybiBpdAo+PiB0byB1c2VyLXNwYWNlLgo+Pgo+
PiBTaWduZWQtb2ZmLWJ5OiBXZWkgTmkgPHduaUBudmlkaWEuY29tPgo+PiAtLS0KPj4gIGRyaXZl
cnMvaHdtb24vbG05MC5jIHwgIDE2NiArKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysr
LS0tLS0tLS0tLS0tLS0tLQo+PiAgMSBmaWxlIGNoYW5nZWQsIDExMyBpbnNlcnRpb25zKCspLCA1
MyBkZWxldGlvbnMoLSkKPj4gKC4uLikKPiAKPiBBcHBsaWVkIHdpdGggbWlub3IgZWRpdHMsIHRo
YW5rcy4KPiAKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f
CmxtLXNlbnNvcnMgbWFpbGluZyBsaXN0CmxtLXNlbnNvcnNAbG0tc2Vuc29ycy5vcmcKaHR0cDov
L2xpc3RzLmxtLXNlbnNvcnMub3JnL21haWxtYW4vbGlzdGluZm8vbG0tc2Vuc29ycw=

Re: [PATCH v5] hwmon: (lm90) split set&show temp as common codes

[Wei Ni]

Hi, Jean
Sorry to trouble you, I recalled that you had applied this change, but I still
can't find it in the mainline.
Could you please check it or does I need to do anything else?

Thanks.
Wei.

On 2013年10月11日 15:17, Jean Delvare wrote:
> On Tue, 8 Oct 2013 15:04:26 +0800, Wei Ni wrote:
>> Split set&show temp codes as common functions, so we can use it
>> directly when implement linux thermal framework.
>> And handle error return value for the lm90_select_remote_channel
>> and write_tempx, then set_temp8 and set_temp11 could return it
>> to user-space.
>>
>> Signed-off-by: Wei Ni <wni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> ---
>>  drivers/hwmon/lm90.c |  166 ++++++++++++++++++++++++++++++++++----------------
>>  1 file changed, 113 insertions(+), 53 deletions(-)
>> (...)
> 
> Applied with minor edits, thanks.
> 

Re: [lm-sensors] [PATCH v5] hwmon: (lm90) split set&show temp as common codes

[Wei Ni]

SGksIEplYW4KU29ycnkgdG8gdHJvdWJsZSB5b3UsIEkgcmVjYWxsZWQgdGhhdCB5b3UgaGFkIGFw
cGxpZWQgdGhpcyBjaGFuZ2UsIGJ1dCBJIHN0aWxsCmNhbid0IGZpbmQgaXQgaW4gdGhlIG1haW5s
aW5lLgpDb3VsZCB5b3UgcGxlYXNlIGNoZWNrIGl0IG9yIGRvZXMgSSBuZWVkIHRvIGRvIGFueXRo
aW5nIGVsc2U/CgpUaGFua3MuCldlaS4KCk9uIDIwMTPlubQxMOaciDEx5pelIDE1OjE3LCBKZWFu
IERlbHZhcmUgd3JvdGU6Cj4gT24gVHVlLCA4IE9jdCAyMDEzIDE1OjA0OjI2ICswODAwLCBXZWkg
Tmkgd3JvdGU6Cj4+IFNwbGl0IHNldCZzaG93IHRlbXAgY29kZXMgYXMgY29tbW9uIGZ1bmN0aW9u
cywgc28gd2UgY2FuIHVzZSBpdAo+PiBkaXJlY3RseSB3aGVuIGltcGxlbWVudCBsaW51eCB0aGVy
bWFsIGZyYW1ld29yay4KPj4gQW5kIGhhbmRsZSBlcnJvciByZXR1cm4gdmFsdWUgZm9yIHRoZSBs
bTkwX3NlbGVjdF9yZW1vdGVfY2hhbm5lbAo+PiBhbmQgd3JpdGVfdGVtcHgsIHRoZW4gc2V0X3Rl
bXA4IGFuZCBzZXRfdGVtcDExIGNvdWxkIHJldHVybiBpdAo+PiB0byB1c2VyLXNwYWNlLgo+Pgo+
PiBTaWduZWQtb2ZmLWJ5OiBXZWkgTmkgPHduaUBudmlkaWEuY29tPgo+PiAtLS0KPj4gIGRyaXZl
cnMvaHdtb24vbG05MC5jIHwgIDE2NiArKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysr
LS0tLS0tLS0tLS0tLS0tLQo+PiAgMSBmaWxlIGNoYW5nZWQsIDExMyBpbnNlcnRpb25zKCspLCA1
MyBkZWxldGlvbnMoLSkKPj4gKC4uLikKPiAKPiBBcHBsaWVkIHdpdGggbWlub3IgZWRpdHMsIHRo
YW5rcy4KPiAKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f
CmxtLXNlbnNvcnMgbWFpbGluZyBsaXN0CmxtLXNlbnNvcnNAbG0tc2Vuc29ycy5vcmcKaHR0cDov
L2xpc3RzLmxtLXNlbnNvcnMub3JnL21haWxtYW4vbGlzdGluZm8vbG0tc2Vuc29ycw=