| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5474A7D0.9050207@roeck-us.net>
Date: Tue, 25 Nov 2014 08:01:20 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Bartosz Golaszewski <bgolaszewski@...libre.com>
CC: LKML <linux-kernel@...r.kernel.org>,
Benoit Cousson <bcousson@...libre.com>,
Patrick Titiano <ptitiano@...libre.com>
Subject: Re: [PATCH 3/5] hwmon: ina2xx: allow to change the averaging rate
at run-time
On 11/25/2014 07:47 AM, Bartosz Golaszewski wrote:
> The averaging rate of ina226 is hardcoded to 16 in the driver.
>
> Make it modifiable at run-time via a new sysfs attribute.
I don't see enough value in this to make it configurable.
Guenter
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@...libre.com>
> ---
> drivers/hwmon/ina2xx.c | 115 +++++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 106 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
> index eb66081..0914a72 100644
> --- a/drivers/hwmon/ina2xx.c
> +++ b/drivers/hwmon/ina2xx.c
> @@ -54,6 +54,9 @@
> /* shunt resistor sysfs attribute index */
> #define INA2XX_RSHUNT 0x8
>
> +/* INA226 averaging sysfs index */
> +#define INA226_AVG 0x9
> +
> /* register count */
> #define INA219_REGISTERS 6
> #define INA226_REGISTERS 8
> @@ -70,6 +73,12 @@
> /* default shunt resistance */
> #define INA2XX_RSHUNT_DEFAULT 10000
>
> +/* bit masks for the averaging setting in the configuration register */
> +#define INA226_AVG_RD_MASK 0x0E00
> +#define INA226_AVG_WR_MASK 0xF1FF
> +
> +#define INA226_READ_AVG(reg) ((reg & INA226_AVG_RD_MASK) >> 9)
> +
> enum ina2xx_ids { ina219, ina226 };
>
> struct ina2xx_config {
> @@ -117,6 +126,13 @@ static const struct ina2xx_config ina2xx_config[] = {
> },
> };
>
> +/* Available averaging rates for ina226. The indices correspond with
> + * the bit values expected by the chip (according to the ina226 datasheet,
> + * table 3 AVG bit settings, found at
> + * http://www.ti.com/lit/ds/symlink/ina226.pdf.
> + */
> +static const int ina226_avg_tab[] = { 1, 4, 16, 64, 128, 256, 512, 1024 };
> +
> static u16 ina2xx_calibration_val(const struct ina2xx_data *data)
> {
> return (u16)(data->config->calibration_factor / data->rshunt);
> @@ -156,6 +172,45 @@ static int ina2xx_calibrate(const struct i2c_client *client, u16 value)
> return status;
> }
>
> +static int ina226_avg_bits(int avg)
> +{
> + int i;
> +
> + for (i = 0; i <= ARRAY_SIZE(ina226_avg_tab); i++) {
> + if (avg == ina226_avg_tab[i])
> + return i;
> + }
> +
> + /* Invalid value */
> + return -1;
> +}
> +
> +static int ina226_avg_val(int bits)
> +{
> + /* Value read from the config register should be correct, but do check
> + * the boundary just in case.
> + */
> + if (bits >= ARRAY_SIZE(ina226_avg_tab)) {
> + WARN_ON_ONCE(1);
> + return -1;
> + }
> +
> + return ina226_avg_tab[bits];
> +}
> +
> +static inline int ina226_update_avg(struct ina2xx_data *data, int avg)
> +{
> + int status;
> + u16 conf;
> +
> + mutex_lock(&data->update_lock);
> + conf = (data->regs[INA2XX_CONFIG] & INA226_AVG_WR_MASK) | (avg << 9);
> + status = ina2xx_configure(data->client, conf);
> + mutex_unlock(&data->update_lock);
> +
> + return status;
> +}
> +
> static struct ina2xx_data *ina2xx_update_device(struct device *dev)
> {
> struct ina2xx_data *data = dev_get_drvdata(dev);
> @@ -213,6 +268,10 @@ static int ina2xx_get_value(struct ina2xx_data *data, u8 reg)
> case INA2XX_RSHUNT:
> val = data->rshunt;
> break;
> + case INA226_AVG:
> + val = ina226_avg_val(INA226_READ_AVG(
> + data->regs[INA2XX_CONFIG]));
> + break;
> default:
> /* programmer goofed */
> WARN_ON_ONCE(1);
> @@ -236,13 +295,25 @@ static ssize_t ina2xx_show_value(struct device *dev,
> ina2xx_get_value(data, attr->index));
> }
>
> -static ssize_t ina2xx_set_shunt(struct device *dev,
> +static ssize_t ina226_show_avg(struct device *dev,
> + struct device_attribute *da, char *buf)
> +{
> + struct ina2xx_data *data = dev_get_drvdata(dev);
> +
> + if (data->kind != ina226)
> + return -ENXIO;
> +
> + return ina2xx_show_value(dev, da, buf);
> +}
> +
> +static ssize_t ina2xx_set_value(struct device *dev,
> struct device_attribute *da,
> const char *buf, size_t count)
> {
> + struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> struct ina2xx_data *data = ina2xx_update_device(dev);
> long val;
> - s32 status;
> + int status;
>
> if (IS_ERR(data))
> return PTR_ERR(data);
> @@ -250,12 +321,33 @@ static ssize_t ina2xx_set_shunt(struct device *dev,
> if ((kstrtol(buf, 10, &val) == -EINVAL) || (val <= 0))
> return -EINVAL;
>
> - mutex_lock(&data->update_lock);
> - data->rshunt = val;
> - status = ina2xx_calibrate(data->client, ina2xx_calibration_val(data));
> - mutex_unlock(&data->update_lock);
> - if (status < 0)
> - return -ENODEV;
> + switch (attr->index) {
> + case INA2XX_RSHUNT:
> + mutex_lock(&data->update_lock);
> + data->rshunt = val;
> + status = ina2xx_calibrate(data->client,
> + ina2xx_calibration_val(data));
> + mutex_unlock(&data->update_lock);
> + if (status < 0)
> + return -ENODEV;
> + break;
> + case INA226_AVG:
> + if (data->kind != ina226)
> + return -ENXIO;
> +
> + status = ina226_avg_bits(val);
> + if (status < 0)
> + return -EINVAL;
> +
> + if (ina226_update_avg(data, status) < 0)
> + return -ENODEV;
> + break;
> + default:
> + /* programmer goofed */
> + WARN_ON_ONCE(1);
> + val = 0;
> + break;
> + }
>
> return count;
> }
> @@ -278,7 +370,11 @@ static SENSOR_DEVICE_ATTR(power1_input, S_IRUGO, ina2xx_show_value, NULL,
>
> /* shunt resistance */
> static SENSOR_DEVICE_ATTR(rshunt, S_IRUGO | S_IWUSR | S_IWGRP,
> - ina2xx_show_value, ina2xx_set_shunt, INA2XX_RSHUNT);
> + ina2xx_show_value, ina2xx_set_value, INA2XX_RSHUNT);
> +
> +/* averaging rate */
> +static SENSOR_DEVICE_ATTR(avg, S_IRUGO | S_IWUSR | S_IWGRP,
> + ina226_show_avg, ina2xx_set_value, INA226_AVG);
>
> /* pointers to created device attributes */
> static struct attribute *ina2xx_attrs[] = {
> @@ -287,6 +383,7 @@ static struct attribute *ina2xx_attrs[] = {
> &sensor_dev_attr_curr1_input.dev_attr.attr,
> &sensor_dev_attr_power1_input.dev_attr.attr,
> &sensor_dev_attr_rshunt.dev_attr.attr,
> + &sensor_dev_attr_avg.dev_attr.attr,
> NULL,
> };
> ATTRIBUTE_GROUPS(ina2xx);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists