[PATCH 0/2] hwmon: Replace update_rate sysfs attribute with update_interval

[PATCH 0/2] hwmon: Replace update_rate sysfs attribute with update_interval

[Guenter Roeck]

Two sysfs attributes are currently used to reflect sensor reading update
intervals: update_rate and rate. In all uses, the attribute reflects
an interval, not a rate.

To reflect this, replace the sysfs ABI attribute update_rate with
update_interval, and modify affected drivers to use update_interval.

[lm-sensors] [PATCH 0/2] hwmon: Replace update_rate sysfs attribute

[Guenter Roeck]

Two sysfs attributes are currently used to reflect sensor reading update
intervals: update_rate and rate. In all uses, the attribute reflects
an interval, not a rate.

To reflect this, replace the sysfs ABI attribute update_rate with
update_interval, and modify affected drivers to use update_interval.

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

[PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval

[Guenter Roeck]

The attribute reflects an interval, not a rate.

Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
---
 Documentation/hwmon/sysfs-interface |   12 +++++-----
 drivers/hwmon/adm1031.c             |   43 +++++++++++++++++++---------------
 2 files changed, 30 insertions(+), 25 deletions(-)

diff --git a/Documentation/hwmon/sysfs-interface b/Documentation/hwmon/sysfs-interface
index ff45d1f..df0cdd2 100644
--- a/Documentation/hwmon/sysfs-interface
+++ b/Documentation/hwmon/sysfs-interface
@@ -91,13 +91,13 @@ name		The chip name.
 		I2C devices get this attribute created automatically.
 		RO
 
-update_rate	The rate at which the chip will update readings.
-		Unit: millisecond
+update_interval	The interval at which the chip or driver will update readings.
+		Unit: milliseconds
 		RW
-		Some devices have a variable update rate. This attribute
-		can be used to change the update rate to the desired
-		frequency.
-
+		Some devices have a variable update rate or interval.
+		Some drivers may want to make the update interval configurable
+		instead of using a hardcoded value. This attribute can be used
+		to change the update interval to the desired value.
 
 ************
 * Voltages *
diff --git a/drivers/hwmon/adm1031.c b/drivers/hwmon/adm1031.c
index 15c1a96..0683e6b 100644
--- a/drivers/hwmon/adm1031.c
+++ b/drivers/hwmon/adm1031.c
@@ -79,7 +79,7 @@ struct adm1031_data {
 	int chip_type;
 	char valid;		/* !=0 if following fields are valid */
 	unsigned long last_updated;	/* In jiffies */
-	unsigned int update_rate;	/* In milliseconds */
+	unsigned int update_interval;	/* In milliseconds */
 	/* The chan_select_table contains the possible configurations for
 	 * auto fan control.
 	 */
@@ -743,23 +743,23 @@ static SENSOR_DEVICE_ATTR(temp3_crit_alarm, S_IRUGO, show_alarm, NULL, 12);
 static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_alarm, NULL, 13);
 static SENSOR_DEVICE_ATTR(temp1_crit_alarm, S_IRUGO, show_alarm, NULL, 14);
 
-/* Update Rate */
-static const unsigned int update_rates[] = {
+/* Update Interval */
+static const unsigned int update_intervals[] = {
 	16000, 8000, 4000, 2000, 1000, 500, 250, 125,
 };
 
-static ssize_t show_update_rate(struct device *dev,
-				struct device_attribute *attr, char *buf)
+static ssize_t show_update_interval(struct device *dev,
+				    struct device_attribute *attr, char *buf)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	struct adm1031_data *data = i2c_get_clientdata(client);
 
-	return sprintf(buf, "%u\n", data->update_rate);
+	return sprintf(buf, "%u\n", data->update_interval);
 }
 
-static ssize_t set_update_rate(struct device *dev,
-			       struct device_attribute *attr,
-			       const char *buf, size_t count)
+static ssize_t set_update_interval(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf, size_t count)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	struct adm1031_data *data = i2c_get_clientdata(client);
@@ -771,12 +771,15 @@ static ssize_t set_update_rate(struct device *dev,
 	if (err)
 		return err;
 
-	/* find the nearest update rate from the table */
-	for (i = 0; i < ARRAY_SIZE(update_rates) - 1; i++) {
-		if (val >= update_rates[i])
+	/*
+	 * Find the nearest update interval from the table.
+	 * Use it to determine the matching update rate.
+	 */
+	for (i = 0; i < ARRAY_SIZE(update_intervals) - 1; i++) {
+		if (val >= update_intervals[i])
 			break;
 	}
-	/* if not found, we point to the last entry (lowest update rate) */
+	/* if not found, we point to the last entry (lowest update interval) */
 
 	/* set the new update rate while preserving other settings */
 	reg = adm1031_read_value(client, ADM1031_REG_FAN_FILTER);
@@ -785,14 +788,14 @@ static ssize_t set_update_rate(struct device *dev,
 	adm1031_write_value(client, ADM1031_REG_FAN_FILTER, reg);
 
 	mutex_lock(&data->update_lock);
-	data->update_rate = update_rates[i];
+	data->update_interval = update_intervals[i];
 	mutex_unlock(&data->update_lock);
 
 	return count;
 }
 
-static DEVICE_ATTR(update_rate, S_IRUGO | S_IWUSR, show_update_rate,
-		   set_update_rate);
+static DEVICE_ATTR(update_interval, S_IRUGO | S_IWUSR, show_update_interval,
+		   set_update_interval);
 
 static struct attribute *adm1031_attributes[] = {
 	&sensor_dev_attr_fan1_input.dev_attr.attr,
@@ -830,7 +833,7 @@ static struct attribute *adm1031_attributes[] = {
 
 	&sensor_dev_attr_auto_fan1_min_pwm.dev_attr.attr,
 
-	&dev_attr_update_rate.attr,
+	&dev_attr_update_interval.attr,
 	&dev_attr_alarms.attr,
 
 	NULL
@@ -981,7 +984,8 @@ static void adm1031_init_client(struct i2c_client *client)
 	mask = ADM1031_UPDATE_RATE_MASK;
 	read_val = adm1031_read_value(client, ADM1031_REG_FAN_FILTER);
 	i = (read_val & mask) >> ADM1031_UPDATE_RATE_SHIFT;
-	data->update_rate = update_rates[i];
+	/* Save it as update interval */
+	data->update_interval = update_intervals[i];
 }
 
 static struct adm1031_data *adm1031_update_device(struct device *dev)
@@ -993,7 +997,8 @@ static struct adm1031_data *adm1031_update_device(struct device *dev)
 
 	mutex_lock(&data->update_lock);
 
-	next_update = data->last_updated + msecs_to_jiffies(data->update_rate);
+	next_update = data->last_updated
+	  + msecs_to_jiffies(data->update_interval);
 	if (time_after(jiffies, next_update) || !data->valid) {
 
 		dev_dbg(&client->dev, "Starting adm1031 update\n");
-- 
1.7.0.87.g0901d

[lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs

[Guenter Roeck]

The attribute reflects an interval, not a rate.

Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
---
 Documentation/hwmon/sysfs-interface |   12 +++++-----
 drivers/hwmon/adm1031.c             |   43 +++++++++++++++++++---------------
 2 files changed, 30 insertions(+), 25 deletions(-)

diff --git a/Documentation/hwmon/sysfs-interface b/Documentation/hwmon/sysfs-interface
index ff45d1f..df0cdd2 100644
--- a/Documentation/hwmon/sysfs-interface
+++ b/Documentation/hwmon/sysfs-interface
@@ -91,13 +91,13 @@ name		The chip name.
 		I2C devices get this attribute created automatically.
 		RO
 
-update_rate	The rate at which the chip will update readings.
-		Unit: millisecond
+update_interval	The interval at which the chip or driver will update readings.
+		Unit: milliseconds
 		RW
-		Some devices have a variable update rate. This attribute
-		can be used to change the update rate to the desired
-		frequency.
-
+		Some devices have a variable update rate or interval.
+		Some drivers may want to make the update interval configurable
+		instead of using a hardcoded value. This attribute can be used
+		to change the update interval to the desired value.
 
 ************
 * Voltages *
diff --git a/drivers/hwmon/adm1031.c b/drivers/hwmon/adm1031.c
index 15c1a96..0683e6b 100644
--- a/drivers/hwmon/adm1031.c
+++ b/drivers/hwmon/adm1031.c
@@ -79,7 +79,7 @@ struct adm1031_data {
 	int chip_type;
 	char valid;		/* !=0 if following fields are valid */
 	unsigned long last_updated;	/* In jiffies */
-	unsigned int update_rate;	/* In milliseconds */
+	unsigned int update_interval;	/* In milliseconds */
 	/* The chan_select_table contains the possible configurations for
 	 * auto fan control.
 	 */
@@ -743,23 +743,23 @@ static SENSOR_DEVICE_ATTR(temp3_crit_alarm, S_IRUGO, show_alarm, NULL, 12);
 static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_alarm, NULL, 13);
 static SENSOR_DEVICE_ATTR(temp1_crit_alarm, S_IRUGO, show_alarm, NULL, 14);
 
-/* Update Rate */
-static const unsigned int update_rates[] = {
+/* Update Interval */
+static const unsigned int update_intervals[] = {
 	16000, 8000, 4000, 2000, 1000, 500, 250, 125,
 };
 
-static ssize_t show_update_rate(struct device *dev,
-				struct device_attribute *attr, char *buf)
+static ssize_t show_update_interval(struct device *dev,
+				    struct device_attribute *attr, char *buf)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	struct adm1031_data *data = i2c_get_clientdata(client);
 
-	return sprintf(buf, "%u\n", data->update_rate);
+	return sprintf(buf, "%u\n", data->update_interval);
 }
 
-static ssize_t set_update_rate(struct device *dev,
-			       struct device_attribute *attr,
-			       const char *buf, size_t count)
+static ssize_t set_update_interval(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf, size_t count)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	struct adm1031_data *data = i2c_get_clientdata(client);
@@ -771,12 +771,15 @@ static ssize_t set_update_rate(struct device *dev,
 	if (err)
 		return err;
 
-	/* find the nearest update rate from the table */
-	for (i = 0; i < ARRAY_SIZE(update_rates) - 1; i++) {
-		if (val >= update_rates[i])
+	/*
+	 * Find the nearest update interval from the table.
+	 * Use it to determine the matching update rate.
+	 */
+	for (i = 0; i < ARRAY_SIZE(update_intervals) - 1; i++) {
+		if (val >= update_intervals[i])
 			break;
 	}
-	/* if not found, we point to the last entry (lowest update rate) */
+	/* if not found, we point to the last entry (lowest update interval) */
 
 	/* set the new update rate while preserving other settings */
 	reg = adm1031_read_value(client, ADM1031_REG_FAN_FILTER);
@@ -785,14 +788,14 @@ static ssize_t set_update_rate(struct device *dev,
 	adm1031_write_value(client, ADM1031_REG_FAN_FILTER, reg);
 
 	mutex_lock(&data->update_lock);
-	data->update_rate = update_rates[i];
+	data->update_interval = update_intervals[i];
 	mutex_unlock(&data->update_lock);
 
 	return count;
 }
 
-static DEVICE_ATTR(update_rate, S_IRUGO | S_IWUSR, show_update_rate,
-		   set_update_rate);
+static DEVICE_ATTR(update_interval, S_IRUGO | S_IWUSR, show_update_interval,
+		   set_update_interval);
 
 static struct attribute *adm1031_attributes[] = {
 	&sensor_dev_attr_fan1_input.dev_attr.attr,
@@ -830,7 +833,7 @@ static struct attribute *adm1031_attributes[] = {
 
 	&sensor_dev_attr_auto_fan1_min_pwm.dev_attr.attr,
 
-	&dev_attr_update_rate.attr,
+	&dev_attr_update_interval.attr,
 	&dev_attr_alarms.attr,
 
 	NULL
@@ -981,7 +984,8 @@ static void adm1031_init_client(struct i2c_client *client)
 	mask = ADM1031_UPDATE_RATE_MASK;
 	read_val = adm1031_read_value(client, ADM1031_REG_FAN_FILTER);
 	i = (read_val & mask) >> ADM1031_UPDATE_RATE_SHIFT;
-	data->update_rate = update_rates[i];
+	/* Save it as update interval */
+	data->update_interval = update_intervals[i];
 }
 
 static struct adm1031_data *adm1031_update_device(struct device *dev)
@@ -993,7 +997,8 @@ static struct adm1031_data *adm1031_update_device(struct device *dev)
 
 	mutex_lock(&data->update_lock);
 
-	next_update = data->last_updated + msecs_to_jiffies(data->update_rate);
+	next_update = data->last_updated
+	  + msecs_to_jiffies(data->update_interval);
 	if (time_after(jiffies, next_update) || !data->valid) {
 
 		dev_dbg(&client->dev, "Starting adm1031 update\n");
-- 
1.7.0.87.g0901d


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

[PATCH 2/2] hwmon: (lm95241) Replace rate sysfs attribute with update_interval

[Guenter Roeck]

update_interval is the matching attribute defined in the hwmon sysfs ABI.
Use it.

Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
---
 drivers/hwmon/lm95241.c |   21 +++++++++++----------
 1 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/hwmon/lm95241.c b/drivers/hwmon/lm95241.c
index 94741d4..464340f 100644
--- a/drivers/hwmon/lm95241.c
+++ b/drivers/hwmon/lm95241.c
@@ -91,7 +91,7 @@ static struct lm95241_data *lm95241_update_device(struct device *dev);
 struct lm95241_data {
 	struct device *hwmon_dev;
 	struct mutex update_lock;
-	unsigned long last_updated, rate; /* in jiffies */
+	unsigned long last_updated, interval; /* in jiffies */
 	char valid; /* zero until following fields are valid */
 	/* registers values */
 	u8 local_h, local_l; /* local */
@@ -114,23 +114,23 @@ show_temp(local);
 show_temp(remote1);
 show_temp(remote2);
 
-static ssize_t show_rate(struct device *dev, struct device_attribute *attr,
+static ssize_t show_interval(struct device *dev, struct device_attribute *attr,
 			 char *buf)
 {
 	struct lm95241_data *data = lm95241_update_device(dev);
 
-	snprintf(buf, PAGE_SIZE - 1, "%lu\n", 1000 * data->rate / HZ);
+	snprintf(buf, PAGE_SIZE - 1, "%lu\n", 1000 * data->interval / HZ);
 	return strlen(buf);
 }
 
-static ssize_t set_rate(struct device *dev, struct device_attribute *attr,
+static ssize_t set_interval(struct device *dev, struct device_attribute *attr,
 			const char *buf, size_t count)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	struct lm95241_data *data = i2c_get_clientdata(client);
 
-	strict_strtol(buf, 10, &data->rate);
-	data->rate = data->rate * HZ / 1000;
+	strict_strtol(buf, 10, &data->interval);
+	data->interval = data->interval * HZ / 1000;
 
 	return count;
 }
@@ -286,7 +286,8 @@ static DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_min1, set_min1);
 static DEVICE_ATTR(temp3_min, S_IWUSR | S_IRUGO, show_min2, set_min2);
 static DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_max1, set_max1);
 static DEVICE_ATTR(temp3_max, S_IWUSR | S_IRUGO, show_max2, set_max2);
-static DEVICE_ATTR(rate, S_IWUSR | S_IRUGO, show_rate, set_rate);
+static DEVICE_ATTR(update_interval, S_IWUSR | S_IRUGO, show_interval,
+		   set_interval);
 
 static struct attribute *lm95241_attributes[] = {
 	&dev_attr_temp1_input.attr,
@@ -298,7 +299,7 @@ static struct attribute *lm95241_attributes[] = {
 	&dev_attr_temp3_min.attr,
 	&dev_attr_temp2_max.attr,
 	&dev_attr_temp3_max.attr,
-	&dev_attr_rate.attr,
+	&dev_attr_update_interval.attr,
 	NULL
 };
 
@@ -376,7 +377,7 @@ static void lm95241_init_client(struct i2c_client *client)
 {
 	struct lm95241_data *data = i2c_get_clientdata(client);
 
-	data->rate = HZ;    /* 1 sec default */
+	data->interval = HZ;    /* 1 sec default */
 	data->valid = 0;
 	data->config = CFG_CR0076;
 	data->model = 0;
@@ -410,7 +411,7 @@ static struct lm95241_data *lm95241_update_device(struct device *dev)
 
 	mutex_lock(&data->update_lock);
 
-	if (time_after(jiffies, data->last_updated + data->rate) ||
+	if (time_after(jiffies, data->last_updated + data->interval) ||
 	    !data->valid) {
 		dev_dbg(&client->dev, "Updating lm95241 data.\n");
 		data->local_h =
-- 
1.7.0.87.g0901d

[lm-sensors] [PATCH 2/2] hwmon: (lm95241) Replace rate sysfs

[Guenter Roeck]

update_interval is the matching attribute defined in the hwmon sysfs ABI.
Use it.

Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
---
 drivers/hwmon/lm95241.c |   21 +++++++++++----------
 1 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/hwmon/lm95241.c b/drivers/hwmon/lm95241.c
index 94741d4..464340f 100644
--- a/drivers/hwmon/lm95241.c
+++ b/drivers/hwmon/lm95241.c
@@ -91,7 +91,7 @@ static struct lm95241_data *lm95241_update_device(struct device *dev);
 struct lm95241_data {
 	struct device *hwmon_dev;
 	struct mutex update_lock;
-	unsigned long last_updated, rate; /* in jiffies */
+	unsigned long last_updated, interval; /* in jiffies */
 	char valid; /* zero until following fields are valid */
 	/* registers values */
 	u8 local_h, local_l; /* local */
@@ -114,23 +114,23 @@ show_temp(local);
 show_temp(remote1);
 show_temp(remote2);
 
-static ssize_t show_rate(struct device *dev, struct device_attribute *attr,
+static ssize_t show_interval(struct device *dev, struct device_attribute *attr,
 			 char *buf)
 {
 	struct lm95241_data *data = lm95241_update_device(dev);
 
-	snprintf(buf, PAGE_SIZE - 1, "%lu\n", 1000 * data->rate / HZ);
+	snprintf(buf, PAGE_SIZE - 1, "%lu\n", 1000 * data->interval / HZ);
 	return strlen(buf);
 }
 
-static ssize_t set_rate(struct device *dev, struct device_attribute *attr,
+static ssize_t set_interval(struct device *dev, struct device_attribute *attr,
 			const char *buf, size_t count)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	struct lm95241_data *data = i2c_get_clientdata(client);
 
-	strict_strtol(buf, 10, &data->rate);
-	data->rate = data->rate * HZ / 1000;
+	strict_strtol(buf, 10, &data->interval);
+	data->interval = data->interval * HZ / 1000;
 
 	return count;
 }
@@ -286,7 +286,8 @@ static DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_min1, set_min1);
 static DEVICE_ATTR(temp3_min, S_IWUSR | S_IRUGO, show_min2, set_min2);
 static DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_max1, set_max1);
 static DEVICE_ATTR(temp3_max, S_IWUSR | S_IRUGO, show_max2, set_max2);
-static DEVICE_ATTR(rate, S_IWUSR | S_IRUGO, show_rate, set_rate);
+static DEVICE_ATTR(update_interval, S_IWUSR | S_IRUGO, show_interval,
+		   set_interval);
 
 static struct attribute *lm95241_attributes[] = {
 	&dev_attr_temp1_input.attr,
@@ -298,7 +299,7 @@ static struct attribute *lm95241_attributes[] = {
 	&dev_attr_temp3_min.attr,
 	&dev_attr_temp2_max.attr,
 	&dev_attr_temp3_max.attr,
-	&dev_attr_rate.attr,
+	&dev_attr_update_interval.attr,
 	NULL
 };
 
@@ -376,7 +377,7 @@ static void lm95241_init_client(struct i2c_client *client)
 {
 	struct lm95241_data *data = i2c_get_clientdata(client);
 
-	data->rate = HZ;    /* 1 sec default */
+	data->interval = HZ;    /* 1 sec default */
 	data->valid = 0;
 	data->config = CFG_CR0076;
 	data->model = 0;
@@ -410,7 +411,7 @@ static struct lm95241_data *lm95241_update_device(struct device *dev)
 
 	mutex_lock(&data->update_lock);
 
-	if (time_after(jiffies, data->last_updated + data->rate) ||
+	if (time_after(jiffies, data->last_updated + data->interval) ||
 	    !data->valid) {
 		dev_dbg(&client->dev, "Updating lm95241 data.\n");
 		data->local_h -- 
1.7.0.87.g0901d


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

Re: [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval

[Jean Delvare]

On Wed, 15 Sep 2010 19:07:14 -0700, Guenter Roeck wrote:
> The attribute reflects an interval, not a rate.
> 
> Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
> ---
>  Documentation/hwmon/sysfs-interface |   12 +++++-----
>  drivers/hwmon/adm1031.c             |   43 +++++++++++++++++++---------------
>  2 files changed, 30 insertions(+), 25 deletions(-)
> 
> diff --git a/Documentation/hwmon/sysfs-interface b/Documentation/hwmon/sysfs-interface
> index ff45d1f..df0cdd2 100644
> --- a/Documentation/hwmon/sysfs-interface
> +++ b/Documentation/hwmon/sysfs-interface
> @@ -91,13 +91,13 @@ name		The chip name.
>  		I2C devices get this attribute created automatically.
>  		RO
>  
> -update_rate	The rate at which the chip will update readings.
> -		Unit: millisecond
> +update_interval	The interval at which the chip or driver will update readings.

I think I prefer the original wording. The attribute is really about
setting the register refresh rate at the hardware level. The fact that
register cache lifetime in the driver can be adjusted accordingly is an
optional implementation detail IMHO.

> +		Unit: milliseconds

Why change? All other units are singular in the file.

>  		RW
> -		Some devices have a variable update rate. This attribute
> -		can be used to change the update rate to the desired
> -		frequency.
> -

Killing an innocent blank line ;)

> +		Some devices have a variable update rate or interval.
> +		Some drivers may want to make the update interval configurable
> +		instead of using a hardcoded value. This attribute can be used
> +		to change the update interval to the desired value.
>  
>  ************
>  * Voltages *

I'm fine with all the rest.

-- 
Jean Delvare

Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate

[Jean Delvare]

On Wed, 15 Sep 2010 19:07:14 -0700, Guenter Roeck wrote:
> The attribute reflects an interval, not a rate.
> 
> Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
> ---
>  Documentation/hwmon/sysfs-interface |   12 +++++-----
>  drivers/hwmon/adm1031.c             |   43 +++++++++++++++++++---------------
>  2 files changed, 30 insertions(+), 25 deletions(-)
> 
> diff --git a/Documentation/hwmon/sysfs-interface b/Documentation/hwmon/sysfs-interface
> index ff45d1f..df0cdd2 100644
> --- a/Documentation/hwmon/sysfs-interface
> +++ b/Documentation/hwmon/sysfs-interface
> @@ -91,13 +91,13 @@ name		The chip name.
>  		I2C devices get this attribute created automatically.
>  		RO
>  
> -update_rate	The rate at which the chip will update readings.
> -		Unit: millisecond
> +update_interval	The interval at which the chip or driver will update readings.

I think I prefer the original wording. The attribute is really about
setting the register refresh rate at the hardware level. The fact that
register cache lifetime in the driver can be adjusted accordingly is an
optional implementation detail IMHO.

> +		Unit: milliseconds

Why change? All other units are singular in the file.

>  		RW
> -		Some devices have a variable update rate. This attribute
> -		can be used to change the update rate to the desired
> -		frequency.
> -

Killing an innocent blank line ;)

> +		Some devices have a variable update rate or interval.
> +		Some drivers may want to make the update interval configurable
> +		instead of using a hardcoded value. This attribute can be used
> +		to change the update interval to the desired value.
>  
>  ************
>  * Voltages *

I'm fine with all the rest.

-- 
Jean Delvare

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

Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval

[Ira W. Snyder]

On Wed, Sep 15, 2010 at 07:07:14PM -0700, Guenter Roeck wrote:
> The attribute reflects an interval, not a rate.
> 
> Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
> ---
>  Documentation/hwmon/sysfs-interface |   12 +++++-----
>  drivers/hwmon/adm1031.c             |   43 +++++++++++++++++++---------------
>  2 files changed, 30 insertions(+), 25 deletions(-)
> 

I'm the original author of both the update_rate text in sysfs-interface
and the adm1031 support. Your change looks good to me. Thanks for doing
the work.

Acked-by: Ira W. Snyder <iws@ovro.caltech.edu>

Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate

[Ira W. Snyder]

On Wed, Sep 15, 2010 at 07:07:14PM -0700, Guenter Roeck wrote:
> The attribute reflects an interval, not a rate.
> 
> Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
> ---
>  Documentation/hwmon/sysfs-interface |   12 +++++-----
>  drivers/hwmon/adm1031.c             |   43 +++++++++++++++++++---------------
>  2 files changed, 30 insertions(+), 25 deletions(-)
> 

I'm the original author of both the update_rate text in sysfs-interface
and the adm1031 support. Your change looks good to me. Thanks for doing
the work.

Acked-by: Ira W. Snyder <iws@ovro.caltech.edu>


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

Re: [PATCH 2/2] hwmon: (lm95241) Replace rate sysfs attribute with update_interval

[Jean Delvare]

On Wed, 15 Sep 2010 19:07:15 -0700, Guenter Roeck wrote:
> update_interval is the matching attribute defined in the hwmon sysfs ABI.
> Use it.
> 
> Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>

Applied, thanks.


-- 
Jean Delvare

Re: [lm-sensors] [PATCH 2/2] hwmon: (lm95241) Replace rate sysfs

[Jean Delvare]

On Wed, 15 Sep 2010 19:07:15 -0700, Guenter Roeck wrote:
> update_interval is the matching attribute defined in the hwmon sysfs ABI.
> Use it.
> 
> Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>

Applied, thanks.


-- 
Jean Delvare

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

Re: [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval

[Guenter Roeck]

On Thu, Sep 16, 2010 at 11:14:43AM -0400, Jean Delvare wrote:
> On Wed, 15 Sep 2010 19:07:14 -0700, Guenter Roeck wrote:
> > The attribute reflects an interval, not a rate.
> > 
> > Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
> > ---
> >  Documentation/hwmon/sysfs-interface |   12 +++++-----
> >  drivers/hwmon/adm1031.c             |   43 +++++++++++++++++++---------------
> >  2 files changed, 30 insertions(+), 25 deletions(-)
> > 
> > diff --git a/Documentation/hwmon/sysfs-interface b/Documentation/hwmon/sysfs-interface
> > index ff45d1f..df0cdd2 100644
> > --- a/Documentation/hwmon/sysfs-interface
> > +++ b/Documentation/hwmon/sysfs-interface
> > @@ -91,13 +91,13 @@ name		The chip name.
> >  		I2C devices get this attribute created automatically.
> >  		RO
> >  
> > -update_rate	The rate at which the chip will update readings.
> > -		Unit: millisecond
> > +update_interval	The interval at which the chip or driver will update readings.
> 
> I think I prefer the original wording. The attribute is really about
> setting the register refresh rate at the hardware level. The fact that
> register cache lifetime in the driver can be adjusted accordingly is an
> optional implementation detail IMHO.
> 
Ok, no problem.

> > +		Unit: milliseconds
> 
> Why change? All other units are singular in the file.
> 
Correct for anything but "milliseconds".

I changed it to milliseconds because all other attributes specify
"milliseconds". No other reason. Am I missing something ?

> >  		RW
> > -		Some devices have a variable update rate. This attribute
> > -		can be used to change the update rate to the desired
> > -		frequency.
> > -
> 
> Killing an innocent blank line ;)
> 
Just trying to sneak it through. Got me there.

> > +		Some devices have a variable update rate or interval.
> > +		Some drivers may want to make the update interval configurable
> > +		instead of using a hardcoded value. This attribute can be used
> > +		to change the update interval to the desired value.
> >  
> > +           instead of using a hardcoded value." 
> >  ************
> >  * Voltages *
> 
> I'm fine with all the rest.
> 
> -- 
> Jean Delvare

Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate

[Guenter Roeck]

On Thu, Sep 16, 2010 at 11:14:43AM -0400, Jean Delvare wrote:
> On Wed, 15 Sep 2010 19:07:14 -0700, Guenter Roeck wrote:
> > The attribute reflects an interval, not a rate.
> > 
> > Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
> > ---
> >  Documentation/hwmon/sysfs-interface |   12 +++++-----
> >  drivers/hwmon/adm1031.c             |   43 +++++++++++++++++++---------------
> >  2 files changed, 30 insertions(+), 25 deletions(-)
> > 
> > diff --git a/Documentation/hwmon/sysfs-interface b/Documentation/hwmon/sysfs-interface
> > index ff45d1f..df0cdd2 100644
> > --- a/Documentation/hwmon/sysfs-interface
> > +++ b/Documentation/hwmon/sysfs-interface
> > @@ -91,13 +91,13 @@ name		The chip name.
> >  		I2C devices get this attribute created automatically.
> >  		RO
> >  
> > -update_rate	The rate at which the chip will update readings.
> > -		Unit: millisecond
> > +update_interval	The interval at which the chip or driver will update readings.
> 
> I think I prefer the original wording. The attribute is really about
> setting the register refresh rate at the hardware level. The fact that
> register cache lifetime in the driver can be adjusted accordingly is an
> optional implementation detail IMHO.
> 
Ok, no problem.

> > +		Unit: milliseconds
> 
> Why change? All other units are singular in the file.
> 
Correct for anything but "milliseconds".

I changed it to milliseconds because all other attributes specify
"milliseconds". No other reason. Am I missing something ?

> >  		RW
> > -		Some devices have a variable update rate. This attribute
> > -		can be used to change the update rate to the desired
> > -		frequency.
> > -
> 
> Killing an innocent blank line ;)
> 
Just trying to sneak it through. Got me there.

> > +		Some devices have a variable update rate or interval.
> > +		Some drivers may want to make the update interval configurable
> > +		instead of using a hardcoded value. This attribute can be used
> > +		to change the update interval to the desired value.
> >  
> > +           instead of using a hardcoded value." 
> >  ************
> >  * Voltages *
> 
> I'm fine with all the rest.
> 
> -- 
> Jean Delvare

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

Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval

[Henrique de Moraes Holschuh]

On Thu, 16 Sep 2010, Jean Delvare wrote:
> On Wed, 15 Sep 2010 19:07:14 -0700, Guenter Roeck wrote:
> > The attribute reflects an interval, not a rate.
> > 
> > Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
> > ---
> >  Documentation/hwmon/sysfs-interface |   12 +++++-----
> >  drivers/hwmon/adm1031.c             |   43 +++++++++++++++++++---------------
> >  2 files changed, 30 insertions(+), 25 deletions(-)
> > 
> > diff --git a/Documentation/hwmon/sysfs-interface b/Documentation/hwmon/sysfs-interface
> > index ff45d1f..df0cdd2 100644
> > --- a/Documentation/hwmon/sysfs-interface
> > +++ b/Documentation/hwmon/sysfs-interface
> > @@ -91,13 +91,13 @@ name		The chip name.
> >  		I2C devices get this attribute created automatically.
> >  		RO
> >  
> > -update_rate	The rate at which the chip will update readings.
> > -		Unit: millisecond
> > +update_interval	The interval at which the chip or driver will update readings.
> 
> I think I prefer the original wording. The attribute is really about
> setting the register refresh rate at the hardware level. The fact that

Only, it doesn't set any rates in the hardware.  It sets the period
(interval).

If the unit of update_rate is changed to Hz, and the driver does
hardware_timer_milliseconds = 1000/update_rate_Hz, THEN it will be
correct to call it a rate...

I'd rather have it in Hz, actually.  I consider that more user-friendly.
But that's just personal preference.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate

[Henrique de Moraes Holschuh]

On Thu, 16 Sep 2010, Jean Delvare wrote:
> On Wed, 15 Sep 2010 19:07:14 -0700, Guenter Roeck wrote:
> > The attribute reflects an interval, not a rate.
> > 
> > Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
> > ---
> >  Documentation/hwmon/sysfs-interface |   12 +++++-----
> >  drivers/hwmon/adm1031.c             |   43 +++++++++++++++++++---------------
> >  2 files changed, 30 insertions(+), 25 deletions(-)
> > 
> > diff --git a/Documentation/hwmon/sysfs-interface b/Documentation/hwmon/sysfs-interface
> > index ff45d1f..df0cdd2 100644
> > --- a/Documentation/hwmon/sysfs-interface
> > +++ b/Documentation/hwmon/sysfs-interface
> > @@ -91,13 +91,13 @@ name		The chip name.
> >  		I2C devices get this attribute created automatically.
> >  		RO
> >  
> > -update_rate	The rate at which the chip will update readings.
> > -		Unit: millisecond
> > +update_interval	The interval at which the chip or driver will update readings.
> 
> I think I prefer the original wording. The attribute is really about
> setting the register refresh rate at the hardware level. The fact that

Only, it doesn't set any rates in the hardware.  It sets the period
(interval).

If the unit of update_rate is changed to Hz, and the driver does
hardware_timer_milliseconds = 1000/update_rate_Hz, THEN it will be
correct to call it a rate...

I'd rather have it in Hz, actually.  I consider that more user-friendly.
But that's just personal preference.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

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

Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval

[Jean Delvare]

On Thu, 16 Sep 2010 12:42:43 -0300, Henrique de Moraes Holschuh wrote:
> On Thu, 16 Sep 2010, Jean Delvare wrote:
> > On Wed, 15 Sep 2010 19:07:14 -0700, Guenter Roeck wrote:
> > > The attribute reflects an interval, not a rate.
> > > 
> > > Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
> > > ---
> > >  Documentation/hwmon/sysfs-interface |   12 +++++-----
> > >  drivers/hwmon/adm1031.c             |   43 +++++++++++++++++++---------------
> > >  2 files changed, 30 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/Documentation/hwmon/sysfs-interface b/Documentation/hwmon/sysfs-interface
> > > index ff45d1f..df0cdd2 100644
> > > --- a/Documentation/hwmon/sysfs-interface
> > > +++ b/Documentation/hwmon/sysfs-interface
> > > @@ -91,13 +91,13 @@ name		The chip name.
> > >  		I2C devices get this attribute created automatically.
> > >  		RO
> > >  
> > > -update_rate	The rate at which the chip will update readings.
> > > -		Unit: millisecond
> > > +update_interval	The interval at which the chip or driver will update readings.
> > 
> > I think I prefer the original wording. The attribute is really about
> > setting the register refresh rate at the hardware level. The fact that
> 
> Only, it doesn't set any rates in the hardware.  It sets the period
> (interval).
> 
> If the unit of update_rate is changed to Hz, and the driver does
> hardware_timer_milliseconds = 1000/update_rate_Hz, THEN it will be
> correct to call it a rate...

I agree. My point was really only about "chip or driver" vs. "chip".

> I'd rather have it in Hz, actually.  I consider that more user-friendly.
> But that's just personal preference.

The problem with Hz is that we need to be able to handle values lower
than 1, and mHz as a unit isn't exactly friendly. I would be very fine
with Hz (especially as we use it for pwmN_freq already) if we didn't
have to support frequencies below 1 Hz. For such low frequencies,
period (or interval) is clearer than frequency IMHO.

-- 
Jean Delvare

Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate

[Jean Delvare]

On Thu, 16 Sep 2010 12:42:43 -0300, Henrique de Moraes Holschuh wrote:
> On Thu, 16 Sep 2010, Jean Delvare wrote:
> > On Wed, 15 Sep 2010 19:07:14 -0700, Guenter Roeck wrote:
> > > The attribute reflects an interval, not a rate.
> > > 
> > > Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
> > > ---
> > >  Documentation/hwmon/sysfs-interface |   12 +++++-----
> > >  drivers/hwmon/adm1031.c             |   43 +++++++++++++++++++---------------
> > >  2 files changed, 30 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/Documentation/hwmon/sysfs-interface b/Documentation/hwmon/sysfs-interface
> > > index ff45d1f..df0cdd2 100644
> > > --- a/Documentation/hwmon/sysfs-interface
> > > +++ b/Documentation/hwmon/sysfs-interface
> > > @@ -91,13 +91,13 @@ name		The chip name.
> > >  		I2C devices get this attribute created automatically.
> > >  		RO
> > >  
> > > -update_rate	The rate at which the chip will update readings.
> > > -		Unit: millisecond
> > > +update_interval	The interval at which the chip or driver will update readings.
> > 
> > I think I prefer the original wording. The attribute is really about
> > setting the register refresh rate at the hardware level. The fact that
> 
> Only, it doesn't set any rates in the hardware.  It sets the period
> (interval).
> 
> If the unit of update_rate is changed to Hz, and the driver does
> hardware_timer_milliseconds = 1000/update_rate_Hz, THEN it will be
> correct to call it a rate...

I agree. My point was really only about "chip or driver" vs. "chip".

> I'd rather have it in Hz, actually.  I consider that more user-friendly.
> But that's just personal preference.

The problem with Hz is that we need to be able to handle values lower
than 1, and mHz as a unit isn't exactly friendly. I would be very fine
with Hz (especially as we use it for pwmN_freq already) if we didn't
have to support frequencies below 1 Hz. For such low frequencies,
period (or interval) is clearer than frequency IMHO.

-- 
Jean Delvare

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

Re: [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval

[Jean Delvare]

On Thu, 16 Sep 2010 08:37:15 -0700, Guenter Roeck wrote:
> On Thu, Sep 16, 2010 at 11:14:43AM -0400, Jean Delvare wrote:
> > On Wed, 15 Sep 2010 19:07:14 -0700, Guenter Roeck wrote:
> > > The attribute reflects an interval, not a rate.
> > > 
> > > Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
> > > ---
> > >  Documentation/hwmon/sysfs-interface |   12 +++++-----
> > >  drivers/hwmon/adm1031.c             |   43 +++++++++++++++++++---------------
> > >  2 files changed, 30 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/Documentation/hwmon/sysfs-interface b/Documentation/hwmon/sysfs-interface
> > > index ff45d1f..df0cdd2 100644
> > > --- a/Documentation/hwmon/sysfs-interface
> > > +++ b/Documentation/hwmon/sysfs-interface
> > > @@ -91,13 +91,13 @@ name		The chip name.
> > >  		I2C devices get this attribute created automatically.
> > >  		RO
> > >  
> > > -update_rate	The rate at which the chip will update readings.
> > > -		Unit: millisecond
> > > +update_interval	The interval at which the chip or driver will update readings.
> > 
> > I think I prefer the original wording. The attribute is really about
> > setting the register refresh rate at the hardware level. The fact that
> > register cache lifetime in the driver can be adjusted accordingly is an
> > optional implementation detail IMHO.
> > 
> Ok, no problem.
> 
> > > +		Unit: milliseconds
> > 
> > Why change? All other units are singular in the file.
> > 
> Correct for anything but "milliseconds".
> 
> I changed it to milliseconds because all other attributes specify
> "milliseconds". No other reason. Am I missing something ?

I'd rather change all these to "millisecond", so that the document
consistently uses singular for all units.

I also see that the indentation isn't consistent, with power and energy
having 4 tabs where the rest has only 2. Bah.

-- 
Jean Delvare

Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate

[Jean Delvare]

On Thu, 16 Sep 2010 08:37:15 -0700, Guenter Roeck wrote:
> On Thu, Sep 16, 2010 at 11:14:43AM -0400, Jean Delvare wrote:
> > On Wed, 15 Sep 2010 19:07:14 -0700, Guenter Roeck wrote:
> > > The attribute reflects an interval, not a rate.
> > > 
> > > Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
> > > ---
> > >  Documentation/hwmon/sysfs-interface |   12 +++++-----
> > >  drivers/hwmon/adm1031.c             |   43 +++++++++++++++++++---------------
> > >  2 files changed, 30 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/Documentation/hwmon/sysfs-interface b/Documentation/hwmon/sysfs-interface
> > > index ff45d1f..df0cdd2 100644
> > > --- a/Documentation/hwmon/sysfs-interface
> > > +++ b/Documentation/hwmon/sysfs-interface
> > > @@ -91,13 +91,13 @@ name		The chip name.
> > >  		I2C devices get this attribute created automatically.
> > >  		RO
> > >  
> > > -update_rate	The rate at which the chip will update readings.
> > > -		Unit: millisecond
> > > +update_interval	The interval at which the chip or driver will update readings.
> > 
> > I think I prefer the original wording. The attribute is really about
> > setting the register refresh rate at the hardware level. The fact that
> > register cache lifetime in the driver can be adjusted accordingly is an
> > optional implementation detail IMHO.
> > 
> Ok, no problem.
> 
> > > +		Unit: milliseconds
> > 
> > Why change? All other units are singular in the file.
> > 
> Correct for anything but "milliseconds".
> 
> I changed it to milliseconds because all other attributes specify
> "milliseconds". No other reason. Am I missing something ?

I'd rather change all these to "millisecond", so that the document
consistently uses singular for all units.

I also see that the indentation isn't consistent, with power and energy
having 4 tabs where the rest has only 2. Bah.

-- 
Jean Delvare

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

Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval

[Henrique de Moraes Holschuh]

On Thu, 16 Sep 2010, Jean Delvare wrote:
> > I'd rather have it in Hz, actually.  I consider that more user-friendly.
> > But that's just personal preference.
> 
> The problem with Hz is that we need to be able to handle values lower
> than 1, and mHz as a unit isn't exactly friendly. I would be very fine
> with Hz (especially as we use it for pwmN_freq already) if we didn't
> have to support frequencies below 1 Hz. For such low frequencies,
> period (or interval) is clearer than frequency IMHO.

We can always use fixed point math... ;-)

I am kidding, I am perfectly aware it is not worth the trouble.

And I agree with you, mHz would be horrible as far as usability goes.  And
prone to be confused with MHz, too.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate

[Henrique de Moraes Holschuh]

On Thu, 16 Sep 2010, Jean Delvare wrote:
> > I'd rather have it in Hz, actually.  I consider that more user-friendly.
> > But that's just personal preference.
> 
> The problem with Hz is that we need to be able to handle values lower
> than 1, and mHz as a unit isn't exactly friendly. I would be very fine
> with Hz (especially as we use it for pwmN_freq already) if we didn't
> have to support frequencies below 1 Hz. For such low frequencies,
> period (or interval) is clearer than frequency IMHO.

We can always use fixed point math... ;-)

I am kidding, I am perfectly aware it is not worth the trouble.

And I agree with you, mHz would be horrible as far as usability goes.  And
prone to be confused with MHz, too.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

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

Re: [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval

[Guenter Roeck]

On Thu, 2010-09-16 at 12:42 -0400, Jean Delvare wrote:
> On Thu, 16 Sep 2010 08:37:15 -0700, Guenter Roeck wrote:
> > On Thu, Sep 16, 2010 at 11:14:43AM -0400, Jean Delvare wrote:
> > > On Wed, 15 Sep 2010 19:07:14 -0700, Guenter Roeck wrote:
> > > > The attribute reflects an interval, not a rate.
> > > > 
> > > > Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
> > > > ---
> > > >  Documentation/hwmon/sysfs-interface |   12 +++++-----
> > > >  drivers/hwmon/adm1031.c             |   43 +++++++++++++++++++---------------
> > > >  2 files changed, 30 insertions(+), 25 deletions(-)
> > > > 
> > > > diff --git a/Documentation/hwmon/sysfs-interface b/Documentation/hwmon/sysfs-interface
> > > > index ff45d1f..df0cdd2 100644
> > > > --- a/Documentation/hwmon/sysfs-interface
> > > > +++ b/Documentation/hwmon/sysfs-interface
> > > > @@ -91,13 +91,13 @@ name		The chip name.
> > > >  		I2C devices get this attribute created automatically.
> > > >  		RO
> > > >  
> > > > -update_rate	The rate at which the chip will update readings.
> > > > -		Unit: millisecond
> > > > +update_interval	The interval at which the chip or driver will update readings.
> > > 
> > > I think I prefer the original wording. The attribute is really about
> > > setting the register refresh rate at the hardware level. The fact that
> > > register cache lifetime in the driver can be adjusted accordingly is an
> > > optional implementation detail IMHO.
> > > 
> > Ok, no problem.
> > 
> > > > +		Unit: milliseconds
> > > 
> > > Why change? All other units are singular in the file.
> > > 
> > Correct for anything but "milliseconds".
> > 
> > I changed it to milliseconds because all other attributes specify
> > "milliseconds". No other reason. Am I missing something ?
> 
> I'd rather change all these to "millisecond", so that the document
> consistently uses singular for all units.
> 
> I also see that the indentation isn't consistent, with power and energy
> having 4 tabs where the rest has only 2. Bah.
> 
To get beyond this ... how about you apply my rev2, and I make the other
changes (formatting and milliseconds --> millisecond) in a subsequent
patch ?

Guenter

Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate

[Guenter Roeck]

On Thu, 2010-09-16 at 12:42 -0400, Jean Delvare wrote:
> On Thu, 16 Sep 2010 08:37:15 -0700, Guenter Roeck wrote:
> > On Thu, Sep 16, 2010 at 11:14:43AM -0400, Jean Delvare wrote:
> > > On Wed, 15 Sep 2010 19:07:14 -0700, Guenter Roeck wrote:
> > > > The attribute reflects an interval, not a rate.
> > > > 
> > > > Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
> > > > ---
> > > >  Documentation/hwmon/sysfs-interface |   12 +++++-----
> > > >  drivers/hwmon/adm1031.c             |   43 +++++++++++++++++++---------------
> > > >  2 files changed, 30 insertions(+), 25 deletions(-)
> > > > 
> > > > diff --git a/Documentation/hwmon/sysfs-interface b/Documentation/hwmon/sysfs-interface
> > > > index ff45d1f..df0cdd2 100644
> > > > --- a/Documentation/hwmon/sysfs-interface
> > > > +++ b/Documentation/hwmon/sysfs-interface
> > > > @@ -91,13 +91,13 @@ name		The chip name.
> > > >  		I2C devices get this attribute created automatically.
> > > >  		RO
> > > >  
> > > > -update_rate	The rate at which the chip will update readings.
> > > > -		Unit: millisecond
> > > > +update_interval	The interval at which the chip or driver will update readings.
> > > 
> > > I think I prefer the original wording. The attribute is really about
> > > setting the register refresh rate at the hardware level. The fact that
> > > register cache lifetime in the driver can be adjusted accordingly is an
> > > optional implementation detail IMHO.
> > > 
> > Ok, no problem.
> > 
> > > > +		Unit: milliseconds
> > > 
> > > Why change? All other units are singular in the file.
> > > 
> > Correct for anything but "milliseconds".
> > 
> > I changed it to milliseconds because all other attributes specify
> > "milliseconds". No other reason. Am I missing something ?
> 
> I'd rather change all these to "millisecond", so that the document
> consistently uses singular for all units.
> 
> I also see that the indentation isn't consistent, with power and energy
> having 4 tabs where the rest has only 2. Bah.
> 
To get beyond this ... how about you apply my rev2, and I make the other
changes (formatting and milliseconds --> millisecond) in a subsequent
patch ?

Guenter



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

Re: [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval

[Jean Delvare]

On Thu, 16 Sep 2010 12:43:15 -0700, Guenter Roeck wrote:
> On Thu, 2010-09-16 at 12:42 -0400, Jean Delvare wrote:
> > I'd rather change all these to "millisecond", so that the document
> > consistently uses singular for all units.
> > 
> > I also see that the indentation isn't consistent, with power and energy
> > having 4 tabs where the rest has only 2. Bah.
> > 
> To get beyond this ... how about you apply my rev2, and I make the other
> changes (formatting and milliseconds --> millisecond) in a subsequent
> patch ?

I'll do the milliseconds --> millisecond myself, no need to resend.
Further changes would indeed belong to one or two separate patches, if
you have time for that. This is definitely low priority though.

-- 
Jean Delvare

Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate

[Jean Delvare]

On Thu, 16 Sep 2010 12:43:15 -0700, Guenter Roeck wrote:
> On Thu, 2010-09-16 at 12:42 -0400, Jean Delvare wrote:
> > I'd rather change all these to "millisecond", so that the document
> > consistently uses singular for all units.
> > 
> > I also see that the indentation isn't consistent, with power and energy
> > having 4 tabs where the rest has only 2. Bah.
> > 
> To get beyond this ... how about you apply my rev2, and I make the other
> changes (formatting and milliseconds --> millisecond) in a subsequent
> patch ?

I'll do the milliseconds --> millisecond myself, no need to resend.
Further changes would indeed belong to one or two separate patches, if
you have time for that. This is definitely low priority though.

-- 
Jean Delvare

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

Re: [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval

[Guenter Roeck]

On Thu, 2010-09-16 at 17:02 -0400, Jean Delvare wrote:
> On Thu, 16 Sep 2010 12:43:15 -0700, Guenter Roeck wrote:
> > On Thu, 2010-09-16 at 12:42 -0400, Jean Delvare wrote:
> > > I'd rather change all these to "millisecond", so that the document
> > > consistently uses singular for all units.
> > > 
> > > I also see that the indentation isn't consistent, with power and energy
> > > having 4 tabs where the rest has only 2. Bah.
> > > 
> > To get beyond this ... how about you apply my rev2, and I make the other
> > changes (formatting and milliseconds --> millisecond) in a subsequent
> > patch ?
> 
> I'll do the milliseconds --> millisecond myself, no need to resend.
> Further changes would indeed belong to one or two separate patches, if
> you have time for that. This is definitely low priority though.
> 
Ok, thanks

Guenter

Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate

[Guenter Roeck]

On Thu, 2010-09-16 at 17:02 -0400, Jean Delvare wrote:
> On Thu, 16 Sep 2010 12:43:15 -0700, Guenter Roeck wrote:
> > On Thu, 2010-09-16 at 12:42 -0400, Jean Delvare wrote:
> > > I'd rather change all these to "millisecond", so that the document
> > > consistently uses singular for all units.
> > > 
> > > I also see that the indentation isn't consistent, with power and energy
> > > having 4 tabs where the rest has only 2. Bah.
> > > 
> > To get beyond this ... how about you apply my rev2, and I make the other
> > changes (formatting and milliseconds --> millisecond) in a subsequent
> > patch ?
> 
> I'll do the milliseconds --> millisecond myself, no need to resend.
> Further changes would indeed belong to one or two separate patches, if
> you have time for that. This is definitely low priority though.
> 
Ok, thanks

Guenter



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