Reducing shunt and bus voltage range improves the accuracy, so allow
altering the default settings.
Signed-off-by: Stefan Brüns <[email protected]>
---
drivers/iio/adc/ina2xx-adc.c | 165 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 164 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
index d1678f886297..856409ecceb3 100644
--- a/drivers/iio/adc/ina2xx-adc.c
+++ b/drivers/iio/adc/ina2xx-adc.c
@@ -47,8 +47,10 @@
#define INA2XX_MAX_REGISTERS 8
/* settings - depend on use case */
-#define INA219_CONFIG_DEFAULT 0x399F /* PGA=8 */
+#define INA219_CONFIG_DEFAULT 0x399F /* PGA=1/8, BRNG=32V */
#define INA219_DEFAULT_IT 532
+#define INA219_DEFAULT_BRNG 32
+#define INA219_DEFAULT_PGA 125 /* 1000/8 */
#define INA226_CONFIG_DEFAULT 0x4327
#define INA226_DEFAULT_AVG 4
#define INA226_DEFAULT_IT 1110
@@ -61,6 +63,14 @@
*/
#define INA2XX_MODE_MASK GENMASK(3, 0)
+/* Gain for VShunt: 1/8 (default), 1/4, 1/2, 1 */
+#define INA219_PGA_MASK GENMASK(12, 11)
+#define INA219_SHIFT_PGA(val) ((val) << 11)
+
+/* VBus range: 32V (default), 16V */
+#define INA219_BRNG_MASK BIT(13)
+#define INA219_SHIFT_BRNG(val) ((val) << 13)
+
/* Averaging for VBus/VShunt/Power */
#define INA226_AVG_MASK GENMASK(11, 9)
#define INA226_SHIFT_AVG(val) ((val) << 9)
@@ -125,6 +135,8 @@ struct ina2xx_chip_info {
int avg;
int int_time_vbus; /* Bus voltage integration time uS */
int int_time_vshunt; /* Shunt voltage integration time uS */
+ int range_vbus; /* Bus voltage maximum in V */
+ int pga_gain_vshunt; /* Shunt voltage PGA gain */
bool allow_async_readout;
};
@@ -351,6 +363,44 @@ static int ina219_set_int_time_vshunt(struct ina2xx_chip_info *chip,
return 0;
}
+static int ina219_set_vbus_range(struct ina2xx_chip_info *chip,
+ unsigned int range,
+ unsigned int *config)
+{
+ if (range != 16 && range != 32)
+ return -EINVAL;
+
+ chip->range_vbus = range;
+
+ *config &= ~INA219_BRNG_MASK;
+ *config |= INA219_SHIFT_BRNG(range == 32 ? 1 : 0) & INA219_BRNG_MASK;
+
+ return 0;
+}
+
+static const int ina219_vshunt_gain_tab[] = { 125, 250, 500, 1000 };
+
+static int ina219_set_vshunt_pga_gain(struct ina2xx_chip_info *chip,
+ unsigned int gain,
+ unsigned int *config)
+{
+ int bits;
+
+ if (gain < 125 || gain > 1000)
+ return -EINVAL;
+
+ bits = find_closest(gain, ina219_vshunt_gain_tab,
+ ARRAY_SIZE(ina219_vshunt_gain_tab));
+
+ chip->pga_gain_vshunt = ina219_vshunt_gain_tab[bits];
+ bits = 3 - bits;
+
+ *config &= ~INA219_PGA_MASK;
+ *config |= INA219_SHIFT_PGA(bits) & INA219_PGA_MASK;
+
+ return 0;
+}
+
static int ina2xx_write_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
int val, int val2, long mask)
@@ -485,6 +535,92 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,
return len;
}
+static ssize_t ina219_bus_voltage_range_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));
+
+ return sprintf(buf, "%d\n", chip->range_vbus);
+}
+
+static ssize_t ina219_bus_voltage_range_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));
+ unsigned long val;
+ unsigned int config, tmp;
+ int ret;
+
+ ret = kstrtoul((const char *) buf, 10, &val);
+ if (ret)
+ return ret;
+
+ mutex_lock(&chip->state_lock);
+
+ ret = regmap_read(chip->regmap, INA2XX_CONFIG, &config);
+ if (ret)
+ goto err;
+
+ tmp = config;
+
+ ret = ina219_set_vbus_range(chip, val, &config);
+
+ if (!ret && (tmp != config))
+ ret = regmap_write(chip->regmap, INA2XX_CONFIG, config);
+
+ if (!ret)
+ ret = len;
+err:
+ mutex_unlock(&chip->state_lock);
+
+ return ret;
+}
+
+static ssize_t ina219_shunt_voltage_gain_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));
+ int vals[2] = { chip->pga_gain_vshunt, 1000 };
+
+ return iio_format_value(buf, IIO_VAL_FRACTIONAL, 1, vals);
+}
+
+static ssize_t ina219_shunt_voltage_gain_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));
+ unsigned int config, tmp;
+ int val, val_fract, ret;
+
+ ret = iio_str_to_fixpoint(buf, 100, &val, &val_fract);
+ if (ret)
+ return ret;
+
+ mutex_lock(&chip->state_lock);
+
+ ret = regmap_read(chip->regmap, INA2XX_CONFIG, &config);
+ if (ret)
+ goto err;
+
+ tmp = config;
+
+ ret = ina219_set_vshunt_pga_gain(chip, val * 1000 + val_fract, &config);
+
+ if (!ret && (tmp != config))
+ ret = regmap_write(chip->regmap, INA2XX_CONFIG, config);
+
+ if (!ret)
+ ret = len;
+err:
+ mutex_unlock(&chip->state_lock);
+
+ return ret;
+}
+
#define INA2XX_CHAN(_type, _index, _address) { \
.type = (_type), \
.address = (_address), \
@@ -698,6 +834,15 @@ static IIO_CONST_ATTR_NAMED(ina226_integration_time_available,
integration_time_available,
"0.000140 0.000204 0.000332 0.000588 0.001100 0.002116 0.004156 0.008244");
+/* INA219/220 Bus voltage range */
+static IIO_CONST_ATTR_NAMED(ina219_bus_voltage_range_available,
+ in_bus_voltage_range_available,
+ "16 32");
+
+/* INA219/220 Shunt voltage PGA gain */
+static IIO_CONST_ATTR_NAMED(ina219_shunt_voltage_gain_available,
+ in_shunt_voltage_gain_available,
+ "0.125 0.25 0.5 1");
static IIO_DEVICE_ATTR(in_allow_async_readout, S_IRUGO | S_IWUSR,
ina2xx_allow_async_readout_show,
@@ -707,9 +852,25 @@ static IIO_DEVICE_ATTR(in_shunt_resistor, S_IRUGO | S_IWUSR,
ina2xx_shunt_resistor_show,
ina2xx_shunt_resistor_store, 0);
+static IIO_DEVICE_ATTR_NAMED(ina219_bus_voltage_range,
+ in_bus_voltage_range,
+ S_IRUGO | S_IWUSR,
+ ina219_bus_voltage_range_show,
+ ina219_bus_voltage_range_store, 0);
+
+static IIO_DEVICE_ATTR_NAMED(ina219_shunt_voltage_gain,
+ in_shunt_voltage_gain,
+ S_IRUGO | S_IWUSR,
+ ina219_shunt_voltage_gain_show,
+ ina219_shunt_voltage_gain_store, 0);
+
static struct attribute *ina219_attributes[] = {
&iio_dev_attr_in_allow_async_readout.dev_attr.attr,
&iio_const_attr_ina219_integration_time_available.dev_attr.attr,
+ &iio_dev_attr_ina219_bus_voltage_range.dev_attr.attr,
+ &iio_const_attr_ina219_bus_voltage_range_available.dev_attr.attr,
+ &iio_dev_attr_ina219_shunt_voltage_gain.dev_attr.attr,
+ &iio_const_attr_ina219_shunt_voltage_gain_available.dev_attr.attr,
&iio_dev_attr_in_shunt_resistor.dev_attr.attr,
NULL,
};
@@ -809,6 +970,8 @@ static int ina2xx_probe(struct i2c_client *client,
chip->avg = 1;
ina219_set_int_time_vbus(chip, INA219_DEFAULT_IT, &val);
ina219_set_int_time_vshunt(chip, INA219_DEFAULT_IT, &val);
+ ina219_set_vbus_range(chip, INA219_DEFAULT_BRNG, &val);
+ ina219_set_vshunt_pga_gain(chip, INA219_DEFAULT_PGA, &val);
}
ret = ina2xx_init(chip, val);
--
2.12.0
On 12/04/17 04:01, Stefan Brüns wrote:
> Reducing shunt and bus voltage range improves the accuracy, so allow
> altering the default settings.
>
> Signed-off-by: Stefan Brüns <[email protected]>
Hi Stefan,
There is new userspace ABI in here, so starting point is to document that.
That would allow the discussion of whether it is the right ABI to begin.
In particular can one of these at least be rolled into the standard
scale attributes that are already supported by the driver?
It looks to me like they both probably can - perhaps in conjunction with
use of the _available callback to notify userspace the range available from
_raw thus allowing easy computation of the range you are providing.
Keeping new ABI to a minimum makes life a lot easier for userspace tooling!
I particularly love the way it's described in the datasheet as a gain
for the shunt voltage but a range for the bus voltage - despite being the
same PGA (at least in the symbolic representation).
Thanks,
Jonathan
> ---
> drivers/iio/adc/ina2xx-adc.c | 165 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 164 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
> index d1678f886297..856409ecceb3 100644
> --- a/drivers/iio/adc/ina2xx-adc.c
> +++ b/drivers/iio/adc/ina2xx-adc.c
> @@ -47,8 +47,10 @@
> #define INA2XX_MAX_REGISTERS 8
>
> /* settings - depend on use case */
> -#define INA219_CONFIG_DEFAULT 0x399F /* PGA=8 */
> +#define INA219_CONFIG_DEFAULT 0x399F /* PGA=1/8, BRNG=32V */
> #define INA219_DEFAULT_IT 532
> +#define INA219_DEFAULT_BRNG 32
> +#define INA219_DEFAULT_PGA 125 /* 1000/8 */
> #define INA226_CONFIG_DEFAULT 0x4327
> #define INA226_DEFAULT_AVG 4
> #define INA226_DEFAULT_IT 1110
> @@ -61,6 +63,14 @@
> */
> #define INA2XX_MODE_MASK GENMASK(3, 0)
>
> +/* Gain for VShunt: 1/8 (default), 1/4, 1/2, 1 */
> +#define INA219_PGA_MASK GENMASK(12, 11)
> +#define INA219_SHIFT_PGA(val) ((val) << 11)
> +
> +/* VBus range: 32V (default), 16V */
> +#define INA219_BRNG_MASK BIT(13)
> +#define INA219_SHIFT_BRNG(val) ((val) << 13)
> +
> /* Averaging for VBus/VShunt/Power */
> #define INA226_AVG_MASK GENMASK(11, 9)
> #define INA226_SHIFT_AVG(val) ((val) << 9)
> @@ -125,6 +135,8 @@ struct ina2xx_chip_info {
> int avg;
> int int_time_vbus; /* Bus voltage integration time uS */
> int int_time_vshunt; /* Shunt voltage integration time uS */
> + int range_vbus; /* Bus voltage maximum in V */
> + int pga_gain_vshunt; /* Shunt voltage PGA gain */
> bool allow_async_readout;
> };
>
> @@ -351,6 +363,44 @@ static int ina219_set_int_time_vshunt(struct ina2xx_chip_info *chip,
> return 0;
> }
>
> +static int ina219_set_vbus_range(struct ina2xx_chip_info *chip,
> + unsigned int range,
> + unsigned int *config)
> +{
> + if (range != 16 && range != 32)
> + return -EINVAL;
> +
> + chip->range_vbus = range;
> +
> + *config &= ~INA219_BRNG_MASK;
> + *config |= INA219_SHIFT_BRNG(range == 32 ? 1 : 0) & INA219_BRNG_MASK;
> +
> + return 0;
> +}
> +
> +static const int ina219_vshunt_gain_tab[] = { 125, 250, 500, 1000 };
> +
> +static int ina219_set_vshunt_pga_gain(struct ina2xx_chip_info *chip,
> + unsigned int gain,
> + unsigned int *config)
> +{
> + int bits;
> +
> + if (gain < 125 || gain > 1000)
> + return -EINVAL;
> +
> + bits = find_closest(gain, ina219_vshunt_gain_tab,
> + ARRAY_SIZE(ina219_vshunt_gain_tab));
> +
> + chip->pga_gain_vshunt = ina219_vshunt_gain_tab[bits];
> + bits = 3 - bits;
> +
> + *config &= ~INA219_PGA_MASK;
> + *config |= INA219_SHIFT_PGA(bits) & INA219_PGA_MASK;
> +
> + return 0;
> +}
> +
> static int ina2xx_write_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> int val, int val2, long mask)
> @@ -485,6 +535,92 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,
> return len;
> }
>
> +static ssize_t ina219_bus_voltage_range_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));
> +
> + return sprintf(buf, "%d\n", chip->range_vbus);
> +}
> +
> +static ssize_t ina219_bus_voltage_range_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));
> + unsigned long val;
> + unsigned int config, tmp;
> + int ret;
> +
> + ret = kstrtoul((const char *) buf, 10, &val);
> + if (ret)
> + return ret;
> +
> + mutex_lock(&chip->state_lock);
> +
> + ret = regmap_read(chip->regmap, INA2XX_CONFIG, &config);
> + if (ret)
> + goto err;
> +
> + tmp = config;
> +
> + ret = ina219_set_vbus_range(chip, val, &config);
> +
> + if (!ret && (tmp != config))
> + ret = regmap_write(chip->regmap, INA2XX_CONFIG, config);
> +
> + if (!ret)
> + ret = len;
> +err:
> + mutex_unlock(&chip->state_lock);
> +
> + return ret;
> +}
> +
> +static ssize_t ina219_shunt_voltage_gain_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));
> + int vals[2] = { chip->pga_gain_vshunt, 1000 };
> +
> + return iio_format_value(buf, IIO_VAL_FRACTIONAL, 1, vals);
> +}
> +
> +static ssize_t ina219_shunt_voltage_gain_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));
> + unsigned int config, tmp;
> + int val, val_fract, ret;
> +
> + ret = iio_str_to_fixpoint(buf, 100, &val, &val_fract);
> + if (ret)
> + return ret;
> +
> + mutex_lock(&chip->state_lock);
> +
> + ret = regmap_read(chip->regmap, INA2XX_CONFIG, &config);
> + if (ret)
> + goto err;
> +
> + tmp = config;
> +
> + ret = ina219_set_vshunt_pga_gain(chip, val * 1000 + val_fract, &config);
> +
> + if (!ret && (tmp != config))
> + ret = regmap_write(chip->regmap, INA2XX_CONFIG, config);
> +
> + if (!ret)
> + ret = len;
> +err:
> + mutex_unlock(&chip->state_lock);
> +
> + return ret;
> +}
> +
> #define INA2XX_CHAN(_type, _index, _address) { \
> .type = (_type), \
> .address = (_address), \
> @@ -698,6 +834,15 @@ static IIO_CONST_ATTR_NAMED(ina226_integration_time_available,
> integration_time_available,
> "0.000140 0.000204 0.000332 0.000588 0.001100 0.002116 0.004156 0.008244");
>
> +/* INA219/220 Bus voltage range */
> +static IIO_CONST_ATTR_NAMED(ina219_bus_voltage_range_available,
> + in_bus_voltage_range_available,
> + "16 32");
> +
> +/* INA219/220 Shunt voltage PGA gain */
> +static IIO_CONST_ATTR_NAMED(ina219_shunt_voltage_gain_available,
> + in_shunt_voltage_gain_available,
> + "0.125 0.25 0.5 1");
>
> static IIO_DEVICE_ATTR(in_allow_async_readout, S_IRUGO | S_IWUSR,
> ina2xx_allow_async_readout_show,
> @@ -707,9 +852,25 @@ static IIO_DEVICE_ATTR(in_shunt_resistor, S_IRUGO | S_IWUSR,
> ina2xx_shunt_resistor_show,
> ina2xx_shunt_resistor_store, 0);
>
> +static IIO_DEVICE_ATTR_NAMED(ina219_bus_voltage_range,
> + in_bus_voltage_range,
> + S_IRUGO | S_IWUSR,
> + ina219_bus_voltage_range_show,
> + ina219_bus_voltage_range_store, 0);
> +
> +static IIO_DEVICE_ATTR_NAMED(ina219_shunt_voltage_gain,
> + in_shunt_voltage_gain,
> + S_IRUGO | S_IWUSR,
> + ina219_shunt_voltage_gain_show,
> + ina219_shunt_voltage_gain_store, 0);
> +
> static struct attribute *ina219_attributes[] = {
> &iio_dev_attr_in_allow_async_readout.dev_attr.attr,
> &iio_const_attr_ina219_integration_time_available.dev_attr.attr,
> + &iio_dev_attr_ina219_bus_voltage_range.dev_attr.attr,
> + &iio_const_attr_ina219_bus_voltage_range_available.dev_attr.attr,
> + &iio_dev_attr_ina219_shunt_voltage_gain.dev_attr.attr,
> + &iio_const_attr_ina219_shunt_voltage_gain_available.dev_attr.attr,
Whole load of new ABI here which must be documented under
Documentation/ABI/testing/sysfs-bus-iio-*
> &iio_dev_attr_in_shunt_resistor.dev_attr.attr,
> NULL,
> };
> @@ -809,6 +970,8 @@ static int ina2xx_probe(struct i2c_client *client,
> chip->avg = 1;
> ina219_set_int_time_vbus(chip, INA219_DEFAULT_IT, &val);
> ina219_set_int_time_vshunt(chip, INA219_DEFAULT_IT, &val);
> + ina219_set_vbus_range(chip, INA219_DEFAULT_BRNG, &val);
> + ina219_set_vshunt_pga_gain(chip, INA219_DEFAULT_PGA, &val);
> }
>
> ret = ina2xx_init(chip, val);
>
On Freitag, 14. April 2017 17:12:03 CEST Jonathan Cameron wrote:
> On 12/04/17 04:01, Stefan Br?ns wrote:
> > Reducing shunt and bus voltage range improves the accuracy, so allow
> > altering the default settings.
> >
> > Signed-off-by: Stefan Br?ns <[email protected]>
>
> Hi Stefan,
>
> There is new userspace ABI in here, so starting point is to document that.
>
> That would allow the discussion of whether it is the right ABI to begin.
>
> In particular can one of these at least be rolled into the standard
> scale attributes that are already supported by the driver?
> It looks to me like they both probably can - perhaps in conjunction with
> use of the _available callback to notify userspace the range available from
> _raw thus allowing easy computation of the range you are providing.
>
> Keeping new ABI to a minimum makes life a lot easier for userspace tooling!
>
> I particularly love the way it's described in the datasheet as a gain
> for the shunt voltage but a range for the bus voltage - despite being the
> same PGA (at least in the symbolic representation).
Unfortunately, correct use of raw and scale is somewhat underdocumented. I
would expect the raw values to reflect the value read from the device,
unaltered.
For the INA226, all value registers are 16 bit, while for the INA219 the
voltage register is 13bit (msb aligned, lowest 3 bits from the register are
masked), the other 3 registers are 16 bit as well.
The INA219 incorporates the bus range and shunt voltage gain in the register
value, i.e. the shunt voltage value 0x0100 always corresponds to 256 * 10uV,
irrespective of the PGA setting.
I think its a bad idea to incorporate the gain settings into the scale
attribute:
1. Raw values would no longer be raw values
2. Scale for the INA219 would be settable, but readonly for the INA226
3. If the device has a gain setting, it should be exposed as such, and names
should correspond to the datasheet
4. Any user of the gain settings had to be made aware of the possibility to
change it, no matter how it is exposed. Making it part of the scale, and thus
changing the meaning of the raw values, would be breaking the existing ABI.
Kind regards,
Stefan
--
Stefan Br?ns / Bergstra?e 21 / 52062 Aachen
home: +49 241 53809034 mobile: +49 151 50412019
work: +49 2405 49936-424
On 17/04/17 23:08, Stefan Bruens wrote:
> On Freitag, 14. April 2017 17:12:03 CEST Jonathan Cameron wrote:
>> On 12/04/17 04:01, Stefan Brüns wrote:
>>> Reducing shunt and bus voltage range improves the accuracy, so allow
>>> altering the default settings.
>>>
>>> Signed-off-by: Stefan Brüns <[email protected]>
>>
>> Hi Stefan,
>>
>> There is new userspace ABI in here, so starting point is to document that.
>>
>> That would allow the discussion of whether it is the right ABI to begin.
>>
>> In particular can one of these at least be rolled into the standard
>> scale attributes that are already supported by the driver?
>> It looks to me like they both probably can - perhaps in conjunction with
>> use of the _available callback to notify userspace the range available from
>> _raw thus allowing easy computation of the range you are providing.
>>
>> Keeping new ABI to a minimum makes life a lot easier for userspace tooling!
>>
>> I particularly love the way it's described in the datasheet as a gain
>> for the shunt voltage but a range for the bus voltage - despite being the
>> same PGA (at least in the symbolic representation).
>
> Unfortunately, correct use of raw and scale is somewhat underdocumented. I
> would expect the raw values to reflect the value read from the device,
> unaltered.
The raw value will indeed give you that. The _available callbacks provide
the range of a particular value. They are rather undocumented though and
rather new which is indeed less than ideal.
Correct use of raw and scale themselves is pretty well covered by the ABI
docs.
Documentation/ABI/testing/sysfs-bus-iio*
>
> For the INA226, all value registers are 16 bit, while for the INA219 the
> voltage register is 13bit (msb aligned, lowest 3 bits from the register are
> masked), the other 3 registers are 16 bit as well.
>
> The INA219 incorporates the bus range and shunt voltage gain in the register
> value, i.e. the shunt voltage value 0x0100 always corresponds to 256 * 10uV,
> irrespective of the PGA setting.
Ah. I hadn't realised that. In that case the standard approach for this
is to use calibscale which reflects changes that are within the hardware
but not in the resulting _raw values (i.e. devices that apply the scale
internally only)
>
> I think its a bad idea to incorporate the gain settings into the scale
> attribute:
> 1. Raw values would no longer be raw values
I think this one is me being unclear in my original response.
> 2. Scale for the INA219 would be settable, but readonly for the INA226
That's not really a problem...
> 3. If the device has a gain setting, it should be exposed as such, and names
> should correspond to the datasheet
Not necessarily. The aim here is to produce a single unified (and normally
minimal) ABI for all devices. If a particular datasheet takes one particular
naming they user should not be obliged to get out said datasheet to find
out what it means. Some of the early parts we supported did everything in
terms of scaling in the datasheets. They got in first and so we are stuck
with that ABI if we can possibly use it.
> 4. Any user of the gain settings had to be made aware of the possibility to
> change it, no matter how it is exposed. Making it part of the scale, and thus
> changing the meaning of the raw values, would be breaking the existing ABI.
The raw values should indeed not change. That was a missunderstanding on my
part. Usually when a device has a PGA it is not compensated for in the
output. So normally it's up to the driver to 'apply' the effective gain to
the incoming reading. When that isn't the case, it can be considered some
sort of internal trim - hence the use of calibscale for this case.
Sorry for the delay in replying, I'm travelling at the moment and reviewing
with jet lag isn't much fun!
Thanks,
Jonathan
>
> Kind regards,
>
> Stefan
>
On 26/04/17 07:19, Jonathan Cameron wrote:
> On 17/04/17 23:08, Stefan Bruens wrote:
>> On Freitag, 14. April 2017 17:12:03 CEST Jonathan Cameron wrote:
>>> On 12/04/17 04:01, Stefan Brüns wrote:
>>>> Reducing shunt and bus voltage range improves the accuracy, so allow
>>>> altering the default settings.
>>>>
>>>> Signed-off-by: Stefan Brüns <[email protected]>
>>>
>>> Hi Stefan,
>>>
>>> There is new userspace ABI in here, so starting point is to document that.
>>>
>>> That would allow the discussion of whether it is the right ABI to begin.
>>>
>>> In particular can one of these at least be rolled into the standard
>>> scale attributes that are already supported by the driver?
>>> It looks to me like they both probably can - perhaps in conjunction with
>>> use of the _available callback to notify userspace the range available from
>>> _raw thus allowing easy computation of the range you are providing.
>>>
>>> Keeping new ABI to a minimum makes life a lot easier for userspace tooling!
>>>
>>> I particularly love the way it's described in the datasheet as a gain
>>> for the shunt voltage but a range for the bus voltage - despite being the
>>> same PGA (at least in the symbolic representation).
>>
>> Unfortunately, correct use of raw and scale is somewhat underdocumented. I
>> would expect the raw values to reflect the value read from the device,
>> unaltered.
> The raw value will indeed give you that. The _available callbacks provide
> the range of a particular value. They are rather undocumented though and
> rather new which is indeed less than ideal.
>
> Correct use of raw and scale themselves is pretty well covered by the ABI
> docs.
> Documentation/ABI/testing/sysfs-bus-iio*
>
>>
>> For the INA226, all value registers are 16 bit, while for the INA219 the
>> voltage register is 13bit (msb aligned, lowest 3 bits from the register are
>> masked), the other 3 registers are 16 bit as well.
>>
>> The INA219 incorporates the bus range and shunt voltage gain in the register
>> value, i.e. the shunt voltage value 0x0100 always corresponds to 256 * 10uV,
>> irrespective of the PGA setting.
> Ah. I hadn't realised that. In that case the standard approach for this
> is to use calibscale which reflects changes that are within the hardware
> but not in the resulting _raw values (i.e. devices that apply the scale
> internally only)
>>
>> I think its a bad idea to incorporate the gain settings into the scale
>> attribute:
>> 1. Raw values would no longer be raw values
> I think this one is me being unclear in my original response.
>> 2. Scale for the INA219 would be settable, but readonly for the INA226
> That's not really a problem...
>
>> 3. If the device has a gain setting, it should be exposed as such, and names
>> should correspond to the datasheet
> Not necessarily. The aim here is to produce a single unified (and normally
> minimal) ABI for all devices. If a particular datasheet takes one particular
> naming they user should not be obliged to get out said datasheet to find
> out what it means. Some of the early parts we supported did everything in
> terms of scaling in the datasheets. They got in first and so we are stuck
> with that ABI if we can possibly use it.
>> 4. Any user of the gain settings had to be made aware of the possibility to
>> change it, no matter how it is exposed. Making it part of the scale, and thus
>> changing the meaning of the raw values, would be breaking the existing ABI.
> The raw values should indeed not change. That was a missunderstanding on my
> part. Usually when a device has a PGA it is not compensated for in the
> output. So normally it's up to the driver to 'apply' the effective gain to
> the incoming reading. When that isn't the case, it can be considered some
> sort of internal trim - hence the use of calibscale for this case.
Mulling this over, calibscale might not work either in this case. The datasheet
helpfully sometimes uses ranges and sometimes uses scale factors.
There is also obviously the calibration register kicking around which would
also be handled with calibscale if exposed to userspace (currently it isn't)
I'm out of time tonight so will think it bit more about this and get back to you
in the next few days...
Jonathan
>
> Sorry for the delay in replying, I'm travelling at the moment and reviewing
> with jet lag isn't much fun!
>
> Thanks,
>
> Jonathan
>>
>> Kind regards,
>>
>> Stefan
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On Mittwoch, 26. April 2017 08:59:47 CEST Jonathan Cameron wrote:
> On 26/04/17 07:19, Jonathan Cameron wrote:
> > On 17/04/17 23:08, Stefan Bruens wrote:
> >> On Freitag, 14. April 2017 17:12:03 CEST Jonathan Cameron wrote:
[...]
> >
> >> 4. Any user of the gain settings had to be made aware of the possibility
> >> to
> >> change it, no matter how it is exposed. Making it part of the scale, and
> >> thus changing the meaning of the raw values, would be breaking the
> >> existing ABI.>
> > The raw values should indeed not change. That was a missunderstanding on
> > my part. Usually when a device has a PGA it is not compensated for in
> > the output. So normally it's up to the driver to 'apply' the effective
> > gain to the incoming reading. When that isn't the case, it can be
> > considered some sort of internal trim - hence the use of calibscale for
> > this case.
> Mulling this over, calibscale might not work either in this case. The
> datasheet helpfully sometimes uses ranges and sometimes uses scale factors.
> There is also obviously the calibration register kicking around which would
> also be handled with calibscale if exposed to userspace (currently it isn't)
>
> I'm out of time tonight so will think it bit more about this and get back to
> you in the next few days...
hardwaregain may be a viable option. For the shunt voltage, available values
would be [0.125, 0.25, 0.5, 1.0], for the bus range we would have either [0.5,
1.0] or [1.0, 2.0] for bus ranges [32V, 16V].
Does hardwaregain have the right semantics for shunt voltage gain and/or bus
range?
Kind regards,
Stefan
--
Stefan Br?ns / Bergstra?e 21 / 52062 Aachen
home: +49 241 53809034 mobile: +49 151 50412019
work: +49 2405 49936-424
On 29/04/17 21:37, Stefan Bruens wrote:
> On Mittwoch, 26. April 2017 08:59:47 CEST Jonathan Cameron wrote:
>> On 26/04/17 07:19, Jonathan Cameron wrote:
>>> On 17/04/17 23:08, Stefan Bruens wrote:
>>>> On Freitag, 14. April 2017 17:12:03 CEST Jonathan Cameron wrote:
> [...]
>>>
>>>> 4. Any user of the gain settings had to be made aware of the possibility
>>>> to
>>>> change it, no matter how it is exposed. Making it part of the scale, and
>>>> thus changing the meaning of the raw values, would be breaking the
>>>> existing ABI.>
>>> The raw values should indeed not change. That was a missunderstanding on
>>> my part. Usually when a device has a PGA it is not compensated for in
>>> the output. So normally it's up to the driver to 'apply' the effective
>>> gain to the incoming reading. When that isn't the case, it can be
>>> considered some sort of internal trim - hence the use of calibscale for
>>> this case.
>> Mulling this over, calibscale might not work either in this case. The
>> datasheet helpfully sometimes uses ranges and sometimes uses scale factors.
>> There is also obviously the calibration register kicking around which would
>> also be handled with calibscale if exposed to userspace (currently it isn't)
>>
>> I'm out of time tonight so will think it bit more about this and get back to
>> you in the next few days...
>
> hardwaregain may be a viable option. For the shunt voltage, available values
> would be [0.125, 0.25, 0.5, 1.0], for the bus range we would have either [0.5,
> 1.0] or [1.0, 2.0] for bus ranges [32V, 16V].
>
> Does hardwaregain have the right semantics for shunt voltage gain and/or bus
> range?
Description we currently have in
Documentation/ABI/testing/sysfs-bus-iio is:
Hardware applied gain factor. If shared across all channels,
<type>_hardwaregain is used.
Just thinking about the use cases, it is mostly used for cases where the
gain is not of the measurement being acquired, but rather of something related
(like the gain on time of flight sensors or pulse counters).
It also gets used for output devices and amplifiers though so kind of similar
as in those cases we felt calibrationscale was a bit of a stretch!
So yes, I can see that working. Whether it is a better choice than
simply allowing the range attributes (documented for this narrow
case to say they should only be used when the range is independent of
the scale) is an open question. Given we have always preferred scales
to ranges if you think you can make hardwaregain fit well then lets
go with that, perhaps updating the docs to make this usecase explicit.
Looking back at the original emails we were actually thinking of
transistioning calibscale to hardwaregain in general as it covered
describing both uses, but it never happened...
J
>
> Kind regards,
>
> Stefan
>
On Sonntag, 30. April 2017 18:19:39 CEST Jonathan Cameron wrote:
> On 29/04/17 21:37, Stefan Bruens wrote:
> > On Mittwoch, 26. April 2017 08:59:47 CEST Jonathan Cameron wrote:
> >> On 26/04/17 07:19, Jonathan Cameron wrote:
> >>> On 17/04/17 23:08, Stefan Bruens wrote:
> >>>> On Freitag, 14. April 2017 17:12:03 CEST Jonathan Cameron wrote:
> > [...]
> >
> >>>> 4. Any user of the gain settings had to be made aware of the
> >>>> possibility
> >>>> to
> >>>> change it, no matter how it is exposed. Making it part of the scale,
> >>>> and
> >>>> thus changing the meaning of the raw values, would be breaking the
> >>>> existing ABI.>
> >>>
> >>> The raw values should indeed not change. That was a missunderstanding
> >>> on
> >>> my part. Usually when a device has a PGA it is not compensated for in
> >>> the output. So normally it's up to the driver to 'apply' the effective
> >>> gain to the incoming reading. When that isn't the case, it can be
> >>> considered some sort of internal trim - hence the use of calibscale for
> >>> this case.
> >>
> >> Mulling this over, calibscale might not work either in this case. The
> >> datasheet helpfully sometimes uses ranges and sometimes uses scale
> >> factors.
> >> There is also obviously the calibration register kicking around which
> >> would
> >> also be handled with calibscale if exposed to userspace (currently it
> >> isn't)
> >>
> >> I'm out of time tonight so will think it bit more about this and get back
> >> to you in the next few days...
> >
> > hardwaregain may be a viable option. For the shunt voltage, available
> > values would be [0.125, 0.25, 0.5, 1.0], for the bus range we would have
> > either [0.5, 1.0] or [1.0, 2.0] for bus ranges [32V, 16V].
> >
> > Does hardwaregain have the right semantics for shunt voltage gain and/or
> > bus range?
>
> Description we currently have in
> Documentation/ABI/testing/sysfs-bus-iio is:
> Hardware applied gain factor. If shared across all channels,
> <type>_hardwaregain is used.
>
> Just thinking about the use cases, it is mostly used for cases where the
> gain is not of the measurement being acquired, but rather of something
> related (like the gain on time of flight sensors or pulse counters).
>
> It also gets used for output devices and amplifiers though so kind of
> similar as in those cases we felt calibrationscale was a bit of a stretch!
>
> So yes, I can see that working. Whether it is a better choice than
> simply allowing the range attributes (documented for this narrow
> case to say they should only be used when the range is independent of
> the scale) is an open question. Given we have always preferred scales
> to ranges if you think you can make hardwaregain fit well then lets
> go with that, perhaps updating the docs to make this usecase explicit.
>
> Looking back at the original emails we were actually thinking of
> transistioning calibscale to hardwaregain in general as it covered
> describing both uses, but it never happened...
Hi John,
as all other patches for INA2xx went into or on their way into mainline, its
time to revisit the INA219/220 bus range and shunt voltage gain again.
TLDR: Using HARDWAREGAIN fits existing uses/semantics.
I had a look at current users of IIO_CHAN_INFO_HARDWAREGAIN:
amplifiers/ad8366.c: Variable gain amplifier without ADC or DAC, so no SCALE
attribute
light/vl6180.c: ToF sensor with ambient light sensor. The ALS sensor has two
settings affecting the RAW sensor readout, HARDWAREGAIN and INTegration_TIME.
Baseline settings are gain=1 and integration time=0.1(seconds), with a
corresponding raw reading of 1 ^= 0.32 lux.
The SCALE value is correct for the baseline setting, but although modifying
HARDWAREGAIN and/or INT_TIME affects the RAW readout, this is not reflected in
the SCALE attribute, i.e. to get the correct physical value, one has to use:
Light[lux] = raw_value * SCALE * (0.1s/INT_TIME) / HARDWAREGAIN
light/adjd_s311.c: HARDWAREGAIN affects the RAW readout, but as there is no
given fixed relationship between RAW values and irradiance, there is no SCALE
attribute.
adc/stx104.c: The ADC has a software controllable HARDWAREGAIN and a hardware
controlled (jumper) offset and single ended/differential setting with software
readback. HARDWAREGAIN and offset/differential are reflected in the SCALE and
OFFSET attributes, i.e. the physical value can be determined by:
U[V] = (raw_value * SCALE) + OFFSET
So we have two users of HARDWAREGAIN with contradicting behaviour regarding
SCALE. IMHO, the stx104 behaviour is the correct one.
For the INA2xx, neither INT_TIME nor AVERAGE affect the RAW <-> physical value
relationship, i.e. the SCALE is fixed. The same is true for the INA219/220 bus
range/shunt voltage gain. So using HARDWAREGAIN for both shunt voltage gain
and bus voltage range does match existing semantics.
Kind regards,
Stefan
--
Stefan Br?ns / Bergstra?e 21 / 52062 Aachen
home: +49 241 53809034 mobile: +49 151 50412019
On Mon, 17 Jul 2017 00:08:32 +0200
Stefan Bruens <[email protected]> wrote:
> On Sonntag, 30. April 2017 18:19:39 CEST Jonathan Cameron wrote:
> > On 29/04/17 21:37, Stefan Bruens wrote:
> > > On Mittwoch, 26. April 2017 08:59:47 CEST Jonathan Cameron wrote:
> > >> On 26/04/17 07:19, Jonathan Cameron wrote:
> > >>> On 17/04/17 23:08, Stefan Bruens wrote:
> > >>>> On Freitag, 14. April 2017 17:12:03 CEST Jonathan Cameron wrote:
> > > [...]
> > >
> > >>>> 4. Any user of the gain settings had to be made aware of the
> > >>>> possibility
> > >>>> to
> > >>>> change it, no matter how it is exposed. Making it part of the scale,
> > >>>> and
> > >>>> thus changing the meaning of the raw values, would be breaking the
> > >>>> existing ABI.>
> > >>>
> > >>> The raw values should indeed not change. That was a missunderstanding
> > >>> on
> > >>> my part. Usually when a device has a PGA it is not compensated for in
> > >>> the output. So normally it's up to the driver to 'apply' the effective
> > >>> gain to the incoming reading. When that isn't the case, it can be
> > >>> considered some sort of internal trim - hence the use of calibscale for
> > >>> this case.
> > >>
> > >> Mulling this over, calibscale might not work either in this case. The
> > >> datasheet helpfully sometimes uses ranges and sometimes uses scale
> > >> factors.
> > >> There is also obviously the calibration register kicking around which
> > >> would
> > >> also be handled with calibscale if exposed to userspace (currently it
> > >> isn't)
> > >>
> > >> I'm out of time tonight so will think it bit more about this and get back
> > >> to you in the next few days...
> > >
> > > hardwaregain may be a viable option. For the shunt voltage, available
> > > values would be [0.125, 0.25, 0.5, 1.0], for the bus range we would have
> > > either [0.5, 1.0] or [1.0, 2.0] for bus ranges [32V, 16V].
> > >
> > > Does hardwaregain have the right semantics for shunt voltage gain and/or
> > > bus range?
> >
> > Description we currently have in
> > Documentation/ABI/testing/sysfs-bus-iio is:
> > Hardware applied gain factor. If shared across all channels,
> > <type>_hardwaregain is used.
> >
> > Just thinking about the use cases, it is mostly used for cases where the
> > gain is not of the measurement being acquired, but rather of something
> > related (like the gain on time of flight sensors or pulse counters).
> >
> > It also gets used for output devices and amplifiers though so kind of
> > similar as in those cases we felt calibrationscale was a bit of a stretch!
> >
> > So yes, I can see that working. Whether it is a better choice than
> > simply allowing the range attributes (documented for this narrow
> > case to say they should only be used when the range is independent of
> > the scale) is an open question. Given we have always preferred scales
> > to ranges if you think you can make hardwaregain fit well then lets
> > go with that, perhaps updating the docs to make this usecase explicit.
> >
> > Looking back at the original emails we were actually thinking of
> > transistioning calibscale to hardwaregain in general as it covered
> > describing both uses, but it never happened...
>
> Hi John,
>
> as all other patches for INA2xx went into or on their way into mainline, its
> time to revisit the INA219/220 bus range and shunt voltage gain again.
>
> TLDR: Using HARDWAREGAIN fits existing uses/semantics.
>
> I had a look at current users of IIO_CHAN_INFO_HARDWAREGAIN:
>
> amplifiers/ad8366.c: Variable gain amplifier without ADC or DAC, so no SCALE
> attribute
>
> light/vl6180.c: ToF sensor with ambient light sensor. The ALS sensor has two
> settings affecting the RAW sensor readout, HARDWAREGAIN and INTegration_TIME.
> Baseline settings are gain=1 and integration time=0.1(seconds), with a
> corresponding raw reading of 1 ^= 0.32 lux.
> The SCALE value is correct for the baseline setting, but although modifying
> HARDWAREGAIN and/or INT_TIME affects the RAW readout, this is not reflected in
> the SCALE attribute, i.e. to get the correct physical value, one has to use:
> Light[lux] = raw_value * SCALE * (0.1s/INT_TIME) / HARDWAREGAIN
This isn't right. User space should be able to just apply the single scale
value to get the correct real world value, not this complex interaction.
So I'd count this driver as buggy unfortunately.
>
> light/adjd_s311.c: HARDWAREGAIN affects the RAW readout, but as there is no
> given fixed relationship between RAW values and irradiance, there is no SCALE
> attribute.
>
> adc/stx104.c: The ADC has a software controllable HARDWAREGAIN and a hardware
> controlled (jumper) offset and single ended/differential setting with software
> readback. HARDWAREGAIN and offset/differential are reflected in the SCALE and
> OFFSET attributes, i.e. the physical value can be determined by:
> U[V] = (raw_value * SCALE) + OFFSET
>
> So we have two users of HARDWAREGAIN with contradicting behaviour regarding
> SCALE. IMHO, the stx104 behaviour is the correct one.
I go the other way, simply because we don't want to complicate the userspace
interface if we don't have to.
>
> For the INA2xx, neither INT_TIME nor AVERAGE affect the RAW <-> physical value
> relationship, i.e. the SCALE is fixed. The same is true for the INA219/220 bus
> range/shunt voltage gain. So using HARDWAREGAIN for both shunt voltage gain
> and bus voltage range does match existing semantics.
I'm uncomfortable with applying a second scaling within all user space code.
That should be handled in the kernel rather than pushing on the burden.
It's not a fast path so doesn't matter if we have to some nasty maths to
work out the right value.
Jonathan
>
> Kind regards,
>
> Stefan
>
On Mon, 17 Jul 2017 20:04:57 +0800
Jonathan Cameron <[email protected]> wrote:
> On Mon, 17 Jul 2017 00:08:32 +0200
> Stefan Bruens <[email protected]> wrote:
>
> > On Sonntag, 30. April 2017 18:19:39 CEST Jonathan Cameron wrote:
> > > On 29/04/17 21:37, Stefan Bruens wrote:
> > > > On Mittwoch, 26. April 2017 08:59:47 CEST Jonathan Cameron wrote:
> > > >> On 26/04/17 07:19, Jonathan Cameron wrote:
> > > >>> On 17/04/17 23:08, Stefan Bruens wrote:
> > > >>>> On Freitag, 14. April 2017 17:12:03 CEST Jonathan Cameron wrote:
> > > > [...]
> > > >
> > > >>>> 4. Any user of the gain settings had to be made aware of the
> > > >>>> possibility
> > > >>>> to
> > > >>>> change it, no matter how it is exposed. Making it part of the scale,
> > > >>>> and
> > > >>>> thus changing the meaning of the raw values, would be breaking the
> > > >>>> existing ABI.>
> > > >>>
> > > >>> The raw values should indeed not change. That was a missunderstanding
> > > >>> on
> > > >>> my part. Usually when a device has a PGA it is not compensated for in
> > > >>> the output. So normally it's up to the driver to 'apply' the effective
> > > >>> gain to the incoming reading. When that isn't the case, it can be
> > > >>> considered some sort of internal trim - hence the use of calibscale for
> > > >>> this case.
> > > >>
> > > >> Mulling this over, calibscale might not work either in this case. The
> > > >> datasheet helpfully sometimes uses ranges and sometimes uses scale
> > > >> factors.
> > > >> There is also obviously the calibration register kicking around which
> > > >> would
> > > >> also be handled with calibscale if exposed to userspace (currently it
> > > >> isn't)
> > > >>
> > > >> I'm out of time tonight so will think it bit more about this and get back
> > > >> to you in the next few days...
> > > >
> > > > hardwaregain may be a viable option. For the shunt voltage, available
> > > > values would be [0.125, 0.25, 0.5, 1.0], for the bus range we would have
> > > > either [0.5, 1.0] or [1.0, 2.0] for bus ranges [32V, 16V].
> > > >
> > > > Does hardwaregain have the right semantics for shunt voltage gain and/or
> > > > bus range?
> > >
> > > Description we currently have in
> > > Documentation/ABI/testing/sysfs-bus-iio is:
> > > Hardware applied gain factor. If shared across all channels,
> > > <type>_hardwaregain is used.
> > >
> > > Just thinking about the use cases, it is mostly used for cases where the
> > > gain is not of the measurement being acquired, but rather of something
> > > related (like the gain on time of flight sensors or pulse counters).
> > >
> > > It also gets used for output devices and amplifiers though so kind of
> > > similar as in those cases we felt calibrationscale was a bit of a stretch!
> > >
> > > So yes, I can see that working. Whether it is a better choice than
> > > simply allowing the range attributes (documented for this narrow
> > > case to say they should only be used when the range is independent of
> > > the scale) is an open question. Given we have always preferred scales
> > > to ranges if you think you can make hardwaregain fit well then lets
> > > go with that, perhaps updating the docs to make this usecase explicit.
> > >
> > > Looking back at the original emails we were actually thinking of
> > > transistioning calibscale to hardwaregain in general as it covered
> > > describing both uses, but it never happened...
> >
> > Hi John,
> >
> > as all other patches for INA2xx went into or on their way into mainline, its
> > time to revisit the INA219/220 bus range and shunt voltage gain again.
> >
> > TLDR: Using HARDWAREGAIN fits existing uses/semantics.
> >
> > I had a look at current users of IIO_CHAN_INFO_HARDWAREGAIN:
> >
> > amplifiers/ad8366.c: Variable gain amplifier without ADC or DAC, so no SCALE
> > attribute
> >
> > light/vl6180.c: ToF sensor with ambient light sensor. The ALS sensor has two
> > settings affecting the RAW sensor readout, HARDWAREGAIN and INTegration_TIME.
> > Baseline settings are gain=1 and integration time=0.1(seconds), with a
> > corresponding raw reading of 1 ^= 0.32 lux.
> > The SCALE value is correct for the baseline setting, but although modifying
> > HARDWAREGAIN and/or INT_TIME affects the RAW readout, this is not reflected in
> > the SCALE attribute, i.e. to get the correct physical value, one has to use:
> > Light[lux] = raw_value * SCALE * (0.1s/INT_TIME) / HARDWAREGAIN
> This isn't right. User space should be able to just apply the single scale
> value to get the correct real world value, not this complex interaction.
>
> So I'd count this driver as buggy unfortunately.
>
> >
> > light/adjd_s311.c: HARDWAREGAIN affects the RAW readout, but as there is no
> > given fixed relationship between RAW values and irradiance, there is no SCALE
> > attribute.
>
> >
> > adc/stx104.c: The ADC has a software controllable HARDWAREGAIN and a hardware
> > controlled (jumper) offset and single ended/differential setting with software
> > readback. HARDWAREGAIN and offset/differential are reflected in the SCALE and
> > OFFSET attributes, i.e. the physical value can be determined by:
> > U[V] = (raw_value * SCALE) + OFFSET
> >
> > So we have two users of HARDWAREGAIN with contradicting behaviour regarding
> > SCALE. IMHO, the stx104 behaviour is the correct one.
> I go the other way, simply because we don't want to complicate the userspace
> interface if we don't have to.
Sorry, I was clearly talking rubbish here.
The stx104 is the right way.
> >
> > For the INA2xx, neither INT_TIME nor AVERAGE affect the RAW <-> physical value
> > relationship, i.e. the SCALE is fixed. The same is true for the INA219/220 bus
> > range/shunt voltage gain. So using HARDWAREGAIN for both shunt voltage gain
> > and bus voltage range does match existing semantics.
>
> I'm uncomfortable with applying a second scaling within all user space code.
> That should be handled in the kernel rather than pushing on the burden.
> It's not a fast path so doesn't matter if we have to some nasty maths to
> work out the right value.
Again complete rubbish. I'll blame lack of coffee :(
If it's not effecting the output, then hardware gain is absolutely fine.
Jonathan
>
> Jonathan
> >
> > Kind regards,
> >
> > Stefan
> >
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html