Re: [PATCH 3/3] iio: ina2xx: add support for CHAN_INFO_SCALE
From: Marc Titinger
Date: Mon Dec 14 2015 - 05:15:40 EST
On 12/12/2015 18:14, Jonathan Cameron wrote:
On 11/12/15 16:49, Marc Titinger wrote:
Provide client apps with the scales to apply to the register values
read from the software buffer.
Follow the ABI documentation so that values are in milli-unit after scales
are applied.
Umm. The below looks like it is doing rather more than this..
There is an endian switch! I'm going to assume that is correct
for now and merge it into the original patch (rather than as a follow
up). If this is wrong and should not be here shout quickly and loudly!
I know...I think the endianess issue was not spotted in my tests until I
had the scales coded-in: values in direct read mode were processed
before, and hence ok.
The endianness hint was wrong in streamed mode, but I did only
functional check so far. This is now tested with he sigrok-iio draft
from my colleague and values are now fine with scales applied on the
buffer samples.
Will take the rest of this patch as is though I would much have
prefered that this one was rolled into the original driver as
I hadn't taken that when you sent this change...
Actually never mind I'll smash it into the original driver as git
seems to be happy to handle that bit of reordering and I haven't
pushed out yet.
Yes thank you for doing that, it's the good option I think.
So applied as a fixup to the original driver patch.
Hmm. a few mor bits came up build testing this - such as lack of static on the
the buffer enable / disable functions.
Please check nothing went wrong in my making various minor changes.
I've done basic build tests but obviously that doesn't catch everything!
Tested ok with iio utils (using iio_monitor for scales) and and the
sigrok-iio draft using 3 instances of the driver.
Many thanks,
Marc.
Jonathan
Signed-off-by: Marc Titinger <mtitinger@xxxxxxxxxxxx>
---
drivers/iio/adc/ina2xx-adc.c | 85 +++++++++++++++++++++-----------------------
1 file changed, 41 insertions(+), 44 deletions(-)
diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
index 99afa6e..98939ba 100644
--- a/drivers/iio/adc/ina2xx-adc.c
+++ b/drivers/iio/adc/ina2xx-adc.c
@@ -82,6 +82,11 @@ static bool ina2xx_is_volatile_reg(struct device *dev, unsigned int reg)
return (reg != INA2XX_CONFIG);
}
+static inline bool is_signed_reg(unsigned int reg)
+{
+ return (reg == INA2XX_SHUNT_VOLTAGE) || (reg == INA2XX_CURRENT);
+}
+
static const struct regmap_config ina2xx_regmap_config = {
.reg_bits = 8,
.val_bits = 16,
@@ -133,43 +138,6 @@ static const struct ina2xx_config ina2xx_config[] = {
},
};
-static int ina2xx_get_value(struct ina2xx_chip_info *chip, u8 reg,
- unsigned int regval, int *val, int *uval)
-{
- *val = 0;
-
- switch (reg) {
- case INA2XX_SHUNT_VOLTAGE:
- /* signed register */
- *uval = DIV_ROUND_CLOSEST((s16) regval,
- chip->config->shunt_div);
- return IIO_VAL_INT_PLUS_MICRO;
-
- case INA2XX_BUS_VOLTAGE:
- *uval = (regval >> chip->config->bus_voltage_shift)
- * chip->config->bus_voltage_lsb;
- *val = *uval / 1000000;
- *uval = *uval % 1000000;
- return IIO_VAL_INT_PLUS_MICRO;
-
- case INA2XX_POWER:
- *uval = regval * chip->config->power_lsb;
- *val = *uval / 1000000;
- *uval = *uval % 1000000;
- return IIO_VAL_INT_PLUS_MICRO;
-
- case INA2XX_CURRENT:
- /* signed register, LSB=1mA (selected), in mA */
- *uval = (s16) regval * 1000;
- return IIO_VAL_INT_PLUS_MICRO;
-
- default:
- /* programmer goofed */
- WARN_ON_ONCE(1);
- }
- return -EINVAL;
-}
-
static int ina2xx_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
int *val, int *val2, long mask)
@@ -184,7 +152,12 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev,
if (ret < 0)
return ret;
- return ina2xx_get_value(chip, chan->address, regval, val, val2);
+ if (is_signed_reg(chan->address))
+ *val = (s16) regval;
+ else
+ *val = regval;
+
+ return IIO_VAL_INT;
case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
*val = chip->avg;
@@ -208,11 +181,34 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev,
return IIO_VAL_INT;
- default:
- return -EINVAL;
+ case IIO_CHAN_INFO_SCALE:
+ switch (chan->address) {
+ case INA2XX_SHUNT_VOLTAGE:
+ /* processed (mV) = raw*1000/shunt_div */
+ *val2 = chip->config->shunt_div;
+ *val = 1000;
+ return IIO_VAL_FRACTIONAL;
+
+ case INA2XX_BUS_VOLTAGE:
+ /* processed (mV) = raw*lsb (uV) / (1000 << shift) */
+ *val = chip->config->bus_voltage_lsb;
+ *val2 = 1000 << chip->config->bus_voltage_shift;
+ return IIO_VAL_FRACTIONAL;
+
+ case INA2XX_POWER:
+ /* processed (mW) = raw*lsb (uW) / 1000 */
+ *val = chip->config->power_lsb;
+ *val2 = 1000;
+ return IIO_VAL_FRACTIONAL;
+
+ case INA2XX_CURRENT:
+ /* processed (mA) = raw (mA) */
+ *val = 1;
+ return IIO_VAL_INT;
+ }
}
- return 0;
+ return -EINVAL;
}
/*
@@ -395,7 +391,7 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,
.address = (_address), \
.indexed = 1, \
.channel = (_index), \
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW)|BIT(IIO_CHAN_INFO_SCALE), \
Spacing wrong about the |
.info_mask_shared_by_dir = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
.scan_index = (_index), \
@@ -403,7 +399,7 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,
.sign = 'u', \
.realbits = 16, \
.storagebits = 16, \
- .endianness = IIO_BE, \
+ .endianness = IIO_LE, \
} \
}
@@ -417,13 +413,14 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,
.indexed = 1, \
.channel = (_index), \
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_SCALE) | \
BIT(IIO_CHAN_INFO_INT_TIME), \
.scan_index = (_index), \
.scan_type = { \
.sign = 'u', \
.realbits = 16, \
.storagebits = 16, \
- .endianness = IIO_BE, \
+ .endianness = IIO_LE, \
} \
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/