On 12/11/15 12:57, Marc Titinger wrote:
Basic support or direct IO raw read, with averaging attribute.Hi Marc
Values are RAW, INT_PLUS_MICRO (Volt/Ampere/Watt).
Output of iio_info:
iio:device0: ina226
4 channels found:
power3: (input)
1 channel-specific attributes found:
attr 0: raw value: 1.150000
voltage0: (input)
1 channel-specific attributes found:
attr 0: raw value: 0.000003
voltage1: (input)
1 channel-specific attributes found:
attr 0: raw value: 4.277500
current2: (input)
1 channel-specific attributes found:
attr 0: raw value: 0.268000
2 device-specific attributes found:
attr 0: in_calibscale value: 10000
attr 1: in_mean_raw value: 4
attr 2: in_sampling_frequency value: 455
Tested with ina226, on BeagleBoneBlack.
Datasheet: http://www.ti.com/lit/gpn/ina226
Signed-off-by: Marc Titinger <mtitinger@xxxxxxxxxxxx>
To express a personal preference, please don't have series of patches asOk, sorry for that.
replies to the original thread. It gets really hard to follow after
a couple of revisions!
May seem a random question, but what do you want to gain from an IIO
driver over what the hwmon provides?
Various bits inline..
Mostly looks pretty good though.
Jonathan
---
+#define INA2XX_RSHUNT_DEFAULT 10000genmask is always good for these.
+
+/* bit mask for reading the averaging setting in the configuration register */
+#define INA226_AVG_RD_MASK 0x0E00
+On one line I think this is still way less than 80 chars..
+static bool ina2xx_is_writeable_reg(struct device *dev, unsigned int reg)
+{
+ return (reg == INA2XX_CONFIG)
+ || (reg == INA2XX_CALIBRATION);
+}
+struct ina2xx_chip_info {Having a pointer back to the indio_dev is usually a sign of
+ struct iio_dev *indio_dev;
something 'unusual' going on... Will be interesting to see what it is for ;)
Ah, that was easy, you don't use it that I can see.
+I'm somewhat unconvinced that this wrapper is adding anything over
+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);
+ *val = *uval/1000000;
+ *uval = *uval - *val*1000000;
+ return IIO_VAL_INT_PLUS_MICRO;
just doing this where you care about the result.
Also, this is a straight forward linear conversion. Do it in userspace
by providing the relevant 'scale' element.
...+
+ case INA2XX_BUS_VOLTAGE:
+ *uval = (regval >> chip->config->bus_voltage_shift)
+ * chip->config->bus_voltage_lsb;
+ *val = *uval/1000000;
+ *uval = *uval - *val*1000000;
+ return IIO_VAL_INT_PLUS_MICRO;
Ah ok, should this go into a sysfs attribute then, until the ABI section is defined ? How about specifying the ABI section while we are at it ?+ tmp = config;This isn't what the ABI uses the info_average_raw attribute for - it's
+
+ switch (mask) {
+ case IIO_CHAN_INFO_AVERAGE_RAW:
+
+ ret = ina226_set_average(chip, val, &tmp);
for outputing the average of a channel rather than setting a mean
filter width (which is what you have here). Probably need some new ABI
for this as our current filter description ABI is rather limited!
+};Single line comment, use single line comment syntax.
+
+/*