* [lm-sensors] [PATCH 0/2] hwmon: Replace update_rate sysfs attribute @ 2010-09-16 2:07 ` Guenter Roeck 0 siblings, 0 replies; 28+ messages in thread From: Guenter Roeck @ 2010-09-16 2:07 UTC (permalink / raw) To: Jean Delvare Cc: Randy Dunlap, lm-sensors, linux-doc, linux-kernel, 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 0/2] hwmon: Replace update_rate sysfs attribute with update_interval @ 2010-09-16 2:07 ` Guenter Roeck 0 siblings, 0 replies; 28+ messages in thread From: Guenter Roeck @ 2010-09-16 2:07 UTC (permalink / raw) To: Jean Delvare Cc: Randy Dunlap, lm-sensors, linux-doc, linux-kernel, 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. ^ permalink raw reply [flat|nested] 28+ messages in thread
* [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs 2010-09-16 2:07 ` [PATCH 0/2] hwmon: Replace update_rate sysfs attribute with update_interval Guenter Roeck @ 2010-09-16 2:07 ` Guenter Roeck -1 siblings, 0 replies; 28+ messages in thread From: Guenter Roeck @ 2010-09-16 2:07 UTC (permalink / raw) To: Jean Delvare Cc: Randy Dunlap, lm-sensors, linux-doc, linux-kernel, 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 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval @ 2010-09-16 2:07 ` Guenter Roeck 0 siblings, 0 replies; 28+ messages in thread From: Guenter Roeck @ 2010-09-16 2:07 UTC (permalink / raw) To: Jean Delvare Cc: Randy Dunlap, lm-sensors, linux-doc, linux-kernel, 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 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate 2010-09-16 2:07 ` [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval Guenter Roeck @ 2010-09-16 15:14 ` Jean Delvare -1 siblings, 0 replies; 28+ messages in thread From: Jean Delvare @ 2010-09-16 15:14 UTC (permalink / raw) To: Guenter Roeck; +Cc: Randy Dunlap, lm-sensors, linux-doc, linux-kernel 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval @ 2010-09-16 15:14 ` Jean Delvare 0 siblings, 0 replies; 28+ messages in thread From: Jean Delvare @ 2010-09-16 15:14 UTC (permalink / raw) To: Guenter Roeck; +Cc: Randy Dunlap, lm-sensors, linux-doc, linux-kernel 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate 2010-09-16 15:14 ` [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval Jean Delvare @ 2010-09-16 15:37 ` Guenter Roeck -1 siblings, 0 replies; 28+ messages in thread From: Guenter Roeck @ 2010-09-16 15:37 UTC (permalink / raw) To: Jean Delvare Cc: Randy Dunlap, lm-sensors@lm-sensors.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval @ 2010-09-16 15:37 ` Guenter Roeck 0 siblings, 0 replies; 28+ messages in thread From: Guenter Roeck @ 2010-09-16 15:37 UTC (permalink / raw) To: Jean Delvare Cc: Randy Dunlap, lm-sensors@lm-sensors.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate 2010-09-16 15:37 ` [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval Guenter Roeck @ 2010-09-16 16:42 ` Jean Delvare -1 siblings, 0 replies; 28+ messages in thread From: Jean Delvare @ 2010-09-16 16:42 UTC (permalink / raw) To: Guenter Roeck; +Cc: Randy Dunlap, lm-sensors, linux-doc, linux-kernel 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval @ 2010-09-16 16:42 ` Jean Delvare 0 siblings, 0 replies; 28+ messages in thread From: Jean Delvare @ 2010-09-16 16:42 UTC (permalink / raw) To: Guenter Roeck; +Cc: Randy Dunlap, lm-sensors, linux-doc, linux-kernel 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate 2010-09-16 16:42 ` [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval Jean Delvare @ 2010-09-16 19:43 ` Guenter Roeck -1 siblings, 0 replies; 28+ messages in thread From: Guenter Roeck @ 2010-09-16 19:43 UTC (permalink / raw) To: Jean Delvare Cc: Randy Dunlap, lm-sensors@lm-sensors.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval @ 2010-09-16 19:43 ` Guenter Roeck 0 siblings, 0 replies; 28+ messages in thread From: Guenter Roeck @ 2010-09-16 19:43 UTC (permalink / raw) To: Jean Delvare Cc: Randy Dunlap, lm-sensors@lm-sensors.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate 2010-09-16 19:43 ` [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval Guenter Roeck @ 2010-09-16 21:02 ` Jean Delvare -1 siblings, 0 replies; 28+ messages in thread From: Jean Delvare @ 2010-09-16 21:02 UTC (permalink / raw) To: guenter.roeck; +Cc: Randy Dunlap, lm-sensors, linux-doc, linux-kernel 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval @ 2010-09-16 21:02 ` Jean Delvare 0 siblings, 0 replies; 28+ messages in thread From: Jean Delvare @ 2010-09-16 21:02 UTC (permalink / raw) To: guenter.roeck; +Cc: Randy Dunlap, lm-sensors, linux-doc, linux-kernel 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate 2010-09-16 21:02 ` [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval Jean Delvare @ 2010-09-16 21:39 ` Guenter Roeck -1 siblings, 0 replies; 28+ messages in thread From: Guenter Roeck @ 2010-09-16 21:39 UTC (permalink / raw) To: Jean Delvare Cc: Randy Dunlap, lm-sensors@lm-sensors.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval @ 2010-09-16 21:39 ` Guenter Roeck 0 siblings, 0 replies; 28+ messages in thread From: Guenter Roeck @ 2010-09-16 21:39 UTC (permalink / raw) To: Jean Delvare Cc: Randy Dunlap, lm-sensors@lm-sensors.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate 2010-09-16 15:14 ` [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval Jean Delvare @ 2010-09-16 15:42 ` Henrique de Moraes Holschuh -1 siblings, 0 replies; 28+ messages in thread From: Henrique de Moraes Holschuh @ 2010-09-16 15:42 UTC (permalink / raw) To: Jean Delvare Cc: Guenter Roeck, Randy Dunlap, linux-doc, linux-kernel, lm-sensors 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval @ 2010-09-16 15:42 ` Henrique de Moraes Holschuh 0 siblings, 0 replies; 28+ messages in thread From: Henrique de Moraes Holschuh @ 2010-09-16 15:42 UTC (permalink / raw) To: Jean Delvare Cc: Guenter Roeck, Randy Dunlap, linux-doc, linux-kernel, lm-sensors 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate 2010-09-16 15:42 ` [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval Henrique de Moraes Holschuh @ 2010-09-16 16:01 ` Jean Delvare -1 siblings, 0 replies; 28+ messages in thread From: Jean Delvare @ 2010-09-16 16:01 UTC (permalink / raw) To: Henrique de Moraes Holschuh Cc: Guenter Roeck, Randy Dunlap, linux-doc, linux-kernel, lm-sensors 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval @ 2010-09-16 16:01 ` Jean Delvare 0 siblings, 0 replies; 28+ messages in thread From: Jean Delvare @ 2010-09-16 16:01 UTC (permalink / raw) To: Henrique de Moraes Holschuh Cc: Guenter Roeck, Randy Dunlap, linux-doc, linux-kernel, lm-sensors 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate 2010-09-16 16:01 ` [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval Jean Delvare @ 2010-09-16 16:54 ` Henrique de Moraes Holschuh -1 siblings, 0 replies; 28+ messages in thread From: Henrique de Moraes Holschuh @ 2010-09-16 16:54 UTC (permalink / raw) To: Jean Delvare; +Cc: Randy Dunlap, linux-doc, lm-sensors, linux-kernel 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval @ 2010-09-16 16:54 ` Henrique de Moraes Holschuh 0 siblings, 0 replies; 28+ messages in thread From: Henrique de Moraes Holschuh @ 2010-09-16 16:54 UTC (permalink / raw) To: Jean Delvare; +Cc: Randy Dunlap, linux-doc, lm-sensors, linux-kernel 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate 2010-09-16 2:07 ` [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval Guenter Roeck @ 2010-09-16 15:14 ` Ira W. Snyder -1 siblings, 0 replies; 28+ messages in thread From: Ira W. Snyder @ 2010-09-16 15:14 UTC (permalink / raw) To: Guenter Roeck Cc: Jean Delvare, Randy Dunlap, linux-doc, linux-kernel, lm-sensors 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval @ 2010-09-16 15:14 ` Ira W. Snyder 0 siblings, 0 replies; 28+ messages in thread From: Ira W. Snyder @ 2010-09-16 15:14 UTC (permalink / raw) To: Guenter Roeck Cc: Jean Delvare, Randy Dunlap, linux-doc, linux-kernel, lm-sensors 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> ^ permalink raw reply [flat|nested] 28+ messages in thread
* [lm-sensors] [PATCH 2/2] hwmon: (lm95241) Replace rate sysfs 2010-09-16 2:07 ` [PATCH 0/2] hwmon: Replace update_rate sysfs attribute with update_interval Guenter Roeck @ 2010-09-16 2:07 ` Guenter Roeck -1 siblings, 0 replies; 28+ messages in thread From: Guenter Roeck @ 2010-09-16 2:07 UTC (permalink / raw) To: Jean Delvare Cc: Randy Dunlap, lm-sensors, linux-doc, linux-kernel, 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 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 2/2] hwmon: (lm95241) Replace rate sysfs attribute with update_interval @ 2010-09-16 2:07 ` Guenter Roeck 0 siblings, 0 replies; 28+ messages in thread From: Guenter Roeck @ 2010-09-16 2:07 UTC (permalink / raw) To: Jean Delvare Cc: Randy Dunlap, lm-sensors, linux-doc, linux-kernel, 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 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [lm-sensors] [PATCH 2/2] hwmon: (lm95241) Replace rate sysfs 2010-09-16 2:07 ` [PATCH 2/2] hwmon: (lm95241) Replace rate sysfs attribute with update_interval Guenter Roeck @ 2010-09-16 15:19 ` Jean Delvare -1 siblings, 0 replies; 28+ messages in thread From: Jean Delvare @ 2010-09-16 15:19 UTC (permalink / raw) To: Guenter Roeck; +Cc: Randy Dunlap, lm-sensors, linux-doc, linux-kernel 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/2] hwmon: (lm95241) Replace rate sysfs attribute with update_interval @ 2010-09-16 15:19 ` Jean Delvare 0 siblings, 0 replies; 28+ messages in thread From: Jean Delvare @ 2010-09-16 15:19 UTC (permalink / raw) To: Guenter Roeck; +Cc: Randy Dunlap, lm-sensors, linux-doc, linux-kernel 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2010-09-16 21:40 UTC | newest] Thread overview: 28+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-09-16 2:07 [lm-sensors] [PATCH 0/2] hwmon: Replace update_rate sysfs attribute Guenter Roeck 2010-09-16 2:07 ` [PATCH 0/2] hwmon: Replace update_rate sysfs attribute with update_interval Guenter Roeck 2010-09-16 2:07 ` [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs Guenter Roeck 2010-09-16 2:07 ` [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval Guenter Roeck 2010-09-16 15:14 ` [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate Jean Delvare 2010-09-16 15:14 ` [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval Jean Delvare 2010-09-16 15:37 ` [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate Guenter Roeck 2010-09-16 15:37 ` [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval Guenter Roeck 2010-09-16 16:42 ` [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate Jean Delvare 2010-09-16 16:42 ` [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval Jean Delvare 2010-09-16 19:43 ` [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate Guenter Roeck 2010-09-16 19:43 ` [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval Guenter Roeck 2010-09-16 21:02 ` [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate Jean Delvare 2010-09-16 21:02 ` [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval Jean Delvare 2010-09-16 21:39 ` [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate Guenter Roeck 2010-09-16 21:39 ` [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval Guenter Roeck 2010-09-16 15:42 ` [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate Henrique de Moraes Holschuh 2010-09-16 15:42 ` [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval Henrique de Moraes Holschuh 2010-09-16 16:01 ` [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate Jean Delvare 2010-09-16 16:01 ` [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval Jean Delvare 2010-09-16 16:54 ` [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate Henrique de Moraes Holschuh 2010-09-16 16:54 ` [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval Henrique de Moraes Holschuh 2010-09-16 15:14 ` [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate Ira W. Snyder 2010-09-16 15:14 ` [lm-sensors] [PATCH 1/2] hwmon: (adm1031) Replace update_rate sysfs attribute with update_interval Ira W. Snyder 2010-09-16 2:07 ` [lm-sensors] [PATCH 2/2] hwmon: (lm95241) Replace rate sysfs Guenter Roeck 2010-09-16 2:07 ` [PATCH 2/2] hwmon: (lm95241) Replace rate sysfs attribute with update_interval Guenter Roeck 2010-09-16 15:19 ` [lm-sensors] [PATCH 2/2] hwmon: (lm95241) Replace rate sysfs Jean Delvare 2010-09-16 15:19 ` [PATCH 2/2] hwmon: (lm95241) Replace rate sysfs attribute with update_interval Jean Delvare
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.