[PATCH v3 0/2] IIO version of INA2xx

[PATCH v3 0/2] IIO version of INA2xx

[Marc Titinger]

Version 3 of https://lkml.org/lkml/2015/11/25/245

  * rename ina2xx-iio to ian2xx-adc for consistency with other chips
    (ina2xx being the hwmon driver).
  * add an exclusion in the iio/adc/Kconfig: depends on !SENSORS_INA2XX
  * more typos and style fixes
  * make regmap config const with .max_register = INA2XX_MAX_REGISTERS

previous references:
  * following RFC started in https://lkml.org/lkml/2015/11/10/370
  * and feed back from https://lkml.org/lkml/2015/11/18/395
  * Version 2: https://lkml.org/lkml/2015/11/30/245

Marc Titinger (2):
  iio: ina2xx: add support for TI INA2xx Power Monitors
  iio: ina2xx: provide a sysfs parameter to allow async readout of the
    ADCs

 drivers/iio/adc/Kconfig      |  10 +
 drivers/iio/adc/Makefile     |   1 +
 drivers/iio/adc/ina2xx-adc.c | 706 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 717 insertions(+)
 create mode 100644 drivers/iio/adc/ina2xx-adc.c

-- 
1.9.1

[PATCH v3 1/2] iio: ina2xx: add support for TI INA2xx Power Monitors

[Marc Titinger]

in SOFTWARE buffer mode, a kthread will capture the active scan_elements
into a kfifo, then compute the remaining time until the next capture tick
and do an active wait (udelay).

This will produce a stream of up to fours channels plus a 64bits
timestamps (ns).

Tested with ina226, on BeagleBoneBlack.

Datasheet: http://www.ti.com/lit/gpn/ina226

Signed-off-by: Marc Titinger <mtitinger@baylibre.com>
---
 drivers/iio/adc/Kconfig      |  10 +
 drivers/iio/adc/Makefile     |   1 +
 drivers/iio/adc/ina2xx-adc.c | 670 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 681 insertions(+)
 create mode 100644 drivers/iio/adc/ina2xx-adc.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index eb0cd89..e5fdc7d 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -170,6 +170,16 @@ config EXYNOS_ADC
 	  of SoCs for drivers such as the touchscreen and hwmon to use to share
 	  this resource.
 
+config INA2XX_ADC
+	tristate "Texas Instruments INA2xx Power Monitors IIO driver"
+	depends on I2C && !SENSORS_INA2XX
+	select REGMAP_I2C
+	select IIO_BUFFER
+	select IIO_KFIFO_BUF
+	help
+	  Say yes here to build support for TI INA2xx family of Power Monitors.
+	  This driver is mutually exclusive with the HWMON version.
+
 config LP8788_ADC
 	tristate "LP8788 ADC driver"
 	depends on MFD_LP8788
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index a096210..2869b0ef 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_BERLIN2_ADC) += berlin2-adc.o
 obj-$(CONFIG_DA9150_GPADC) += da9150-gpadc.o
 obj-$(CONFIG_CC10001_ADC) += cc10001_adc.o
 obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o
+obj-$(CONFIG_INA2XX_ADC) += ina2xx-adc.o
 obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
 obj-$(CONFIG_MAX1027) += max1027.o
 obj-$(CONFIG_MAX1363) += max1363.o
diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
new file mode 100644
index 0000000..0e7c474
--- /dev/null
+++ b/drivers/iio/adc/ina2xx-adc.c
@@ -0,0 +1,670 @@
+/*
+ * INA2XX Current and Power Monitors
+ *
+ * Copyright 2015 Baylibre SAS.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Based on linux/drivers/iio/adc/ad7291.c
+ * Copyright 2010-2011 Analog Devices Inc.
+ *
+ * Based on linux/drivers/hwmon/ina2xx.c
+ * Copyright 2012 Lothar Felten <l-felten@ti.com>
+ *
+ * Licensed under the GPL-2 or later.
+ *
+ * IIO driver for INA219-220-226-230-231
+ *
+ * Configurable 7-bit I2C slave address from 0x40 to 0x4F
+ */
+#include <linux/module.h>
+#include <linux/kthread.h>
+#include <linux/delay.h>
+#include <linux/iio/kfifo_buf.h>
+#include <linux/iio/sysfs.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <linux/platform_data/ina2xx.h>
+
+#include <linux/util_macros.h>
+
+/* INA2XX registers definition */
+#define INA2XX_CONFIG                   0x00
+#define INA2XX_SHUNT_VOLTAGE            0x01	/* readonly */
+#define INA2XX_BUS_VOLTAGE              0x02	/* readonly */
+#define INA2XX_POWER                    0x03	/* readonly */
+#define INA2XX_CURRENT                  0x04	/* readonly */
+#define INA2XX_CALIBRATION              0x05
+
+#define INA226_ALERT_MASK		0x06
+#define INA266_CVRF			BIT(3)
+
+#define INA2XX_MAX_REGISTERS            8
+
+/* settings - depend on use case */
+#define INA219_CONFIG_DEFAULT           0x399F	/* PGA=8 */
+#define INA226_CONFIG_DEFAULT           0x4327
+#define INA226_DEFAULT_AVG              4
+#define INA226_DEFAULT_IT		1110
+
+#define INA2XX_RSHUNT_DEFAULT           10000
+
+/*
+ * bit mask for reading the averaging setting in the configuration register
+ * FIXME: use regmap_fields.
+ */
+#define INA2XX_MODE_MASK	GENMASK(3, 0)
+
+#define INA226_AVG_MASK		GENMASK(11, 9)
+#define INA226_SHIFT_AVG(val)	((val) << 9)
+
+/* Integration time for VBus */
+#define INA226_ITB_MASK		GENMASK(8, 6)
+#define INA226_SHIFT_ITB(val)	((val) << 6)
+
+/* Integration time for VShunt */
+#define INA226_ITS_MASK		GENMASK(5, 3)
+#define INA226_SHIFT_ITS(val)	((val) << 3)
+
+/* Cosmetic macro giving the sampling period for a full P=UxI cycle */
+#define SAMPLING_PERIOD(c)	((c->int_time_vbus + c->int_time_vshunt) \
+				 * c->avg)
+
+static bool ina2xx_is_writeable_reg(struct device *dev, unsigned int reg)
+{
+	return (reg == INA2XX_CONFIG) || (reg > INA2XX_CURRENT);
+}
+
+static bool ina2xx_is_volatile_reg(struct device *dev, unsigned int reg)
+{
+	return (reg != INA2XX_CONFIG);
+}
+
+static const struct regmap_config ina2xx_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 16,
+	.max_register = INA2XX_MAX_REGISTERS,
+	.writeable_reg = ina2xx_is_writeable_reg,
+	.volatile_reg = ina2xx_is_volatile_reg,
+};
+
+enum ina2xx_ids { ina219, ina226 };
+
+struct ina2xx_config {
+	u16 config_default;
+	int calibration_factor;
+	int shunt_div;
+	int bus_voltage_shift;
+	int bus_voltage_lsb;	/* uV */
+	int power_lsb;		/* uW */
+};
+
+struct ina2xx_chip_info {
+	struct regmap *regmap;
+	struct task_struct *task;
+	const struct ina2xx_config *config;
+	struct mutex state_lock;
+	long rshunt;
+	int avg;
+	s64 prev_ns;	/* track buffer capture time, check for underruns*/
+	int int_time_vbus; /* Bus voltage integration time uS */
+	int int_time_vshunt; /* Shunt voltage integration time uS */
+};
+
+static const struct ina2xx_config ina2xx_config[] = {
+	[ina219] = {
+		    .config_default = INA219_CONFIG_DEFAULT,
+		    .calibration_factor = 40960000,
+		    .shunt_div = 100,
+		    .bus_voltage_shift = 3,
+		    .bus_voltage_lsb = 4000,
+		    .power_lsb = 20000,
+		    },
+	[ina226] = {
+		    .config_default = INA226_CONFIG_DEFAULT,
+		    .calibration_factor = 5120000,
+		    .shunt_div = 400,
+		    .bus_voltage_shift = 0,
+		    .bus_voltage_lsb = 1250,
+		    .power_lsb = 25000,
+		    },
+};
+
+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)
+{
+	int ret;
+	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
+	unsigned int regval;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = regmap_read(chip->regmap, chan->address, &regval);
+		if (ret < 0)
+			return ret;
+
+		return ina2xx_get_value(chip, chan->address, regval, val, val2);
+
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		*val = chip->avg;
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_INT_TIME:
+		*val = 0;
+		if (chan->address == INA2XX_SHUNT_VOLTAGE)
+			*val2 = chip->int_time_vshunt;
+		else
+			*val2 = chip->int_time_vbus;
+
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		/*
+		 * Sample freq is read only, it is a consequence of
+		 * 1/AVG*(CT_bus+CT_shunt).
+		 */
+		*val = DIV_ROUND_CLOSEST(1000000, SAMPLING_PERIOD(chip));
+
+		return IIO_VAL_INT;
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/*
+ * 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 int ina226_set_average(struct ina2xx_chip_info *chip, unsigned int val,
+			      unsigned int *config)
+{
+	int bits;
+
+	if (val > 1024 || val < 1)
+		return -EINVAL;
+
+	bits = find_closest(val, ina226_avg_tab,
+			    ARRAY_SIZE(ina226_avg_tab));
+
+	chip->avg = ina226_avg_tab[bits];
+
+	*config &= ~INA226_AVG_MASK;
+	*config |= INA226_SHIFT_AVG(bits) & INA226_AVG_MASK;
+
+	return 0;
+}
+
+/* Conversion times in uS */
+static const int ina226_conv_time_tab[] = { 140, 204, 332, 588, 1100,
+					    2116, 4156, 8244 };
+
+static int ina226_set_int_time_vbus(struct ina2xx_chip_info *chip,
+				    unsigned int val_us, unsigned int *config)
+{
+	int bits;
+
+	if (val_us > 8244 || val_us < 140)
+		return -EINVAL;
+
+	bits = find_closest(val_us, ina226_conv_time_tab,
+			ARRAY_SIZE(ina226_conv_time_tab));
+
+	chip->int_time_vbus = ina226_conv_time_tab[bits];
+
+	*config &= ~INA226_ITB_MASK;
+	*config |= INA226_SHIFT_ITB(bits) & INA226_ITB_MASK;
+
+	return 0;
+}
+
+static int ina226_set_int_time_vshunt(struct ina2xx_chip_info *chip,
+				      unsigned int val_us, unsigned int *config)
+{
+	int bits;
+
+	if (val_us > 8244 || val_us < 140)
+		return -EINVAL;
+
+	bits = find_closest(val_us, ina226_conv_time_tab,
+			ARRAY_SIZE(ina226_conv_time_tab));
+
+	chip->int_time_vshunt = ina226_conv_time_tab[bits];
+
+	*config &= ~INA226_ITS_MASK;
+	*config |= INA226_SHIFT_ITS(bits) & INA226_ITS_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)
+{
+	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
+	int ret;
+	unsigned int config, tmp;
+
+	if (iio_buffer_enabled(indio_dev))
+		return -EBUSY;
+
+	mutex_lock(&chip->state_lock);
+
+	ret = regmap_read(chip->regmap, INA2XX_CONFIG, &config);
+	if (ret < 0)
+		goto _err;
+
+	tmp = config;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		ret = ina226_set_average(chip, val, &tmp);
+		break;
+
+	case IIO_CHAN_INFO_INT_TIME:
+		if (chan->address == INA2XX_SHUNT_VOLTAGE)
+			ret = ina226_set_int_time_vshunt(chip, val2, &tmp);
+		else
+			ret = ina226_set_int_time_vbus(chip, val2, &tmp);
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	if (!ret && (tmp != config))
+		ret = regmap_write(chip->regmap, INA2XX_CONFIG, tmp);
+_err:
+	mutex_unlock(&chip->state_lock);
+
+	return ret;
+}
+
+
+#define INA2XX_CHAN(_type, _index, _address) { \
+	.type = (_type), \
+	.address = (_address), \
+	.indexed = 1, \
+	.channel = (_index), \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+	.info_mask_shared_by_dir = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
+				   BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
+	.scan_index = (_index), \
+	.scan_type = { \
+		.sign = 'u', \
+		.realbits = 16, \
+		.storagebits = 16, \
+		.endianness = IIO_BE, \
+	} \
+}
+
+/*
+ * Sampling Freq is a consequence of the integration times of
+ * the Voltage channels.
+ */
+#define INA2XX_CHAN_VOLTAGE(_index, _address) { \
+	.type = IIO_VOLTAGE, \
+	.address = (_address), \
+	.indexed = 1, \
+	.channel = (_index), \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+			      BIT(IIO_CHAN_INFO_INT_TIME), \
+	.scan_index = (_index), \
+	.scan_type = { \
+		.sign = 'u', \
+		.realbits = 16, \
+		.storagebits = 16, \
+		.endianness = IIO_BE, \
+	} \
+}
+
+static const struct iio_chan_spec ina2xx_channels[] = {
+	INA2XX_CHAN_VOLTAGE(0, INA2XX_SHUNT_VOLTAGE),
+	INA2XX_CHAN_VOLTAGE(1, INA2XX_BUS_VOLTAGE),
+	INA2XX_CHAN(IIO_CURRENT, 2, INA2XX_CURRENT),
+	INA2XX_CHAN(IIO_POWER, 3, INA2XX_POWER),
+	IIO_CHAN_SOFT_TIMESTAMP(4),
+};
+
+static int ina2xx_work_buffer(struct iio_dev *indio_dev)
+{
+	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
+	unsigned short data[8];
+	int bit, ret, i = 0;
+	unsigned long buffer_us, elapsed_us;
+	s64 time_a, time_b;
+	unsigned int alert;
+
+	time_a = iio_get_time_ns();
+
+	/*
+	 * Because the timer thread and the chip conversion clock
+	 * are asynchronous, the period difference will eventually
+	 * result in reading V[k-1] again, or skip V[k] at time Tk.
+	 * In order to resync the timer with the conversion process
+	 * we check the ConVersionReadyFlag.
+	 * On hardware that supports using the ALERT pin to toggle a
+	 * GPIO a triggered buffer could be used instead.
+	 * For now, we pay for that extra read of the ALERT register
+	 */
+	do {
+		ret = regmap_read(chip->regmap, INA226_ALERT_MASK,
+				  &alert);
+		if (ret < 0)
+			return ret;
+
+		alert &= INA266_CVRF;
+		trace_printk("Conversion ready: %d\n", !!alert);
+
+	} while (!alert);
+
+	/*
+	 * Single register reads: bulk_read will not work with ina226
+	 * as there is no auto-increment of the address register for
+	 * data length longer than 16bits.
+	 */
+	for_each_set_bit(bit, indio_dev->active_scan_mask,
+			 indio_dev->masklength) {
+		unsigned int val;
+
+		ret = regmap_read(chip->regmap,
+				  INA2XX_SHUNT_VOLTAGE + bit, &val);
+		if (ret < 0)
+			return ret;
+
+		data[i++] = val;
+	}
+
+	time_b = iio_get_time_ns();
+
+	iio_push_to_buffers_with_timestamp(indio_dev,
+					   (unsigned int *)data, time_a);
+
+	buffer_us = (unsigned long)(time_b - time_a) / 1000;
+	elapsed_us = (unsigned long)(time_a - chip->prev_ns) / 1000;
+
+	trace_printk("uS: elapsed: %lu, buf: %lu\n", elapsed_us, buffer_us);
+
+	chip->prev_ns = time_a;
+
+	return buffer_us;
+};
+
+static int ina2xx_capture_thread(void *data)
+{
+	struct iio_dev *indio_dev = (struct iio_dev *)data;
+	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
+	unsigned int sampling_us = SAMPLING_PERIOD(chip);
+	int buffer_us;
+
+	/*
+	 * Poll a bit faster than the chip internal Fs, in case
+	 * we wish to sync with the conversion ready flag.
+	 */
+	sampling_us -= 200;
+
+	do {
+		buffer_us = ina2xx_work_buffer(indio_dev);
+		if (buffer_us < 0)
+			return buffer_us;
+
+		if (sampling_us > buffer_us)
+			udelay(sampling_us - buffer_us);
+
+	} while (!kthread_should_stop());
+
+	return 0;
+}
+
+int ina2xx_buffer_enable(struct iio_dev *indio_dev)
+{
+	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
+	unsigned int sampling_us = SAMPLING_PERIOD(chip);
+
+	trace_printk("Enabling buffer w/ scan_mask %02x, freq = %d, avg =%u\n",
+		     (unsigned int)(*indio_dev->active_scan_mask),
+		     1000000/sampling_us, chip->avg);
+
+	trace_printk("Expected work period: %u us\n", sampling_us);
+
+	chip->prev_ns = iio_get_time_ns();
+
+	chip->task = kthread_run(ina2xx_capture_thread, (void *)indio_dev,
+				 "ina2xx-%uus", sampling_us);
+
+	return PTR_ERR_OR_ZERO(chip->task);
+}
+
+int ina2xx_buffer_disable(struct iio_dev *indio_dev)
+{
+	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
+
+	if (chip->task) {
+		kthread_stop(chip->task);
+		chip->task = NULL;
+	}
+
+	return 0;
+}
+
+static const struct iio_buffer_setup_ops ina2xx_setup_ops = {
+	.postenable = &ina2xx_buffer_enable,
+	.postdisable = &ina2xx_buffer_disable,
+};
+
+static int ina2xx_debug_reg(struct iio_dev *indio_dev,
+			    unsigned reg, unsigned writeval, unsigned *readval)
+{
+	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
+
+	if (!readval)
+		return regmap_write(chip->regmap, reg, writeval);
+
+	return regmap_read(chip->regmap, reg, readval);
+}
+
+/* Possible integration times for vshunt and vbus */
+static IIO_CONST_ATTR_INT_TIME_AVAIL \
+ ("0.000140 0.000204 0.000332 0.000588 0.001100 0.002116 0.004156 0.008244");
+
+static struct attribute *ina2xx_attributes[] = {
+	&iio_const_attr_integration_time_available.dev_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group ina2xx_attribute_group = {
+	.attrs = ina2xx_attributes,
+};
+
+static const struct iio_info ina2xx_info = {
+	.debugfs_reg_access = &ina2xx_debug_reg,
+	.read_raw = &ina2xx_read_raw,
+	.write_raw = &ina2xx_write_raw,
+	.attrs = &ina2xx_attribute_group,
+	.driver_module = THIS_MODULE,
+};
+
+/* Initialize the configuration and calibration registers. */
+static int ina2xx_init(struct ina2xx_chip_info *chip, unsigned int config)
+{
+	u16 regval;
+	int ret = regmap_write(chip->regmap, INA2XX_CONFIG, config);
+
+	if (ret < 0)
+		return ret;
+	/*
+	 * Set current LSB to 1mA, shunt is in uOhms
+	 * (equation 13 in datasheet). We hardcode a Current_LSB
+	 * of 1.0 x10-6. The only remaining parameter is RShunt.
+	 * There is no need to expose the CALIBRATION register
+	 * to the user for now.
+	 */
+	regval = DIV_ROUND_CLOSEST(chip->config->calibration_factor,
+			    chip->rshunt);
+
+	return regmap_write(chip->regmap, INA2XX_CALIBRATION, regval);
+}
+
+static int ina2xx_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct ina2xx_chip_info *chip;
+	struct iio_dev *indio_dev;
+	struct iio_buffer *buffer;
+	int ret;
+	unsigned int val;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	chip = iio_priv(indio_dev);
+
+	chip->config = &ina2xx_config[id->driver_data];
+
+	if (of_property_read_u32(client->dev.of_node,
+				 "shunt-resistor", &val) < 0) {
+		struct ina2xx_platform_data *pdata =
+		    dev_get_platdata(&client->dev);
+
+		if (pdata)
+			val = pdata->shunt_uohms;
+		else
+			val = INA2XX_RSHUNT_DEFAULT;
+	}
+
+	if (val <= 0 || val > chip->config->calibration_factor)
+		return -ENODEV;
+
+	chip->rshunt = val;
+
+	mutex_init(&chip->state_lock);
+
+	/* This is only used for device removal purposes. */
+	i2c_set_clientdata(client, indio_dev);
+
+	indio_dev->name = id->name;
+	indio_dev->channels = ina2xx_channels;
+	indio_dev->num_channels = ARRAY_SIZE(ina2xx_channels);
+
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->info = &ina2xx_info;
+	indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
+
+	chip->regmap = devm_regmap_init_i2c(client, &ina2xx_regmap_config);
+	if (IS_ERR(chip->regmap)) {
+		dev_err(&client->dev, "failed to allocate register map\n");
+		return PTR_ERR(chip->regmap);
+	}
+
+	/* Patch the current config register with default. */
+	val = chip->config->config_default;
+
+	if (id->driver_data == ina226) {
+		ina226_set_average(chip, INA226_DEFAULT_AVG, &val);
+		ina226_set_int_time_vbus(chip, INA226_DEFAULT_IT, &val);
+		ina226_set_int_time_vshunt(chip, INA226_DEFAULT_IT, &val);
+	}
+
+	ret = ina2xx_init(chip, val);
+	if (ret < 0) {
+		dev_err(&client->dev, "error configuring the device: %d\n",
+			ret);
+		return -ENODEV;
+	}
+
+	buffer = devm_iio_kfifo_allocate(&indio_dev->dev);
+	if (!buffer)
+		return -ENOMEM;
+
+	indio_dev->setup_ops = &ina2xx_setup_ops;
+
+	iio_device_attach_buffer(indio_dev, buffer);
+
+	return iio_device_register(indio_dev);
+}
+
+
+static int ina2xx_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+
+	/* Powerdown */
+	return regmap_update_bits(chip->regmap, INA2XX_CONFIG,
+				  INA2XX_MODE_MASK, 0);
+}
+
+
+static const struct i2c_device_id ina2xx_id[] = {
+	{"ina219", ina219},
+	{"ina220", ina219},
+	{"ina226", ina226},
+	{"ina230", ina226},
+	{"ina231", ina226},
+	{}
+};
+
+MODULE_DEVICE_TABLE(i2c, ina2xx_id);
+
+static struct i2c_driver ina2xx_driver = {
+	.driver = {
+		   .name = KBUILD_MODNAME,
+	},
+	.probe = ina2xx_probe,
+	.remove = ina2xx_remove,
+	.id_table = ina2xx_id,
+};
+
+module_i2c_driver(ina2xx_driver);
+
+MODULE_AUTHOR("Marc Titinger <marc.titinger@baylibre.com>");
+MODULE_DESCRIPTION("Texas Instruments INA2XX ADC driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

Re: [PATCH v3 1/2] iio: ina2xx: add support for TI INA2xx Power Monitors

[Jonathan Cameron]

On 07/12/15 09:09, Marc Titinger wrote:
> in SOFTWARE buffer mode, a kthread will capture the active scan_elements
> into a kfifo, then compute the remaining time until the next capture tick
> and do an active wait (udelay).
> 
> This will produce a stream of up to fours channels plus a 64bits
> timestamps (ns).
> 
> Tested with ina226, on BeagleBoneBlack.
> 
> Datasheet: http://www.ti.com/lit/gpn/ina226
> 
> Signed-off-by: Marc Titinger <mtitinger@baylibre.com>
One small (largely semantic) issue with balanced post enable / pre disable
(rather than post disable) for the buffer.

Otherwise fine so I'll apply this and fix that up in the process.

Applied to the togreg branch of iio.git - pushed out as testing.
Was a bit of additional fuzz but I think the merge was straightforward.

Long term - might move this out to a meter directory along with the
various power monitors that are in staging/iio/meter.

Thanks,

Jonathan
> ---
>  drivers/iio/adc/Kconfig      |  10 +
>  drivers/iio/adc/Makefile     |   1 +
>  drivers/iio/adc/ina2xx-adc.c | 670 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 681 insertions(+)
>  create mode 100644 drivers/iio/adc/ina2xx-adc.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index eb0cd89..e5fdc7d 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -170,6 +170,16 @@ config EXYNOS_ADC
>  	  of SoCs for drivers such as the touchscreen and hwmon to use to share
>  	  this resource.
>  
> +config INA2XX_ADC
> +	tristate "Texas Instruments INA2xx Power Monitors IIO driver"
> +	depends on I2C && !SENSORS_INA2XX
> +	select REGMAP_I2C
> +	select IIO_BUFFER
> +	select IIO_KFIFO_BUF
> +	help
> +	  Say yes here to build support for TI INA2xx family of Power Monitors.
> +	  This driver is mutually exclusive with the HWMON version.
> +
>  config LP8788_ADC
>  	tristate "LP8788 ADC driver"
>  	depends on MFD_LP8788
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index a096210..2869b0ef 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_BERLIN2_ADC) += berlin2-adc.o
>  obj-$(CONFIG_DA9150_GPADC) += da9150-gpadc.o
>  obj-$(CONFIG_CC10001_ADC) += cc10001_adc.o
>  obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o
> +obj-$(CONFIG_INA2XX_ADC) += ina2xx-adc.o
>  obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
>  obj-$(CONFIG_MAX1027) += max1027.o
>  obj-$(CONFIG_MAX1363) += max1363.o
> diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
> new file mode 100644
> index 0000000..0e7c474
> --- /dev/null
> +++ b/drivers/iio/adc/ina2xx-adc.c
> @@ -0,0 +1,670 @@
> +/*
> + * INA2XX Current and Power Monitors
> + *
> + * Copyright 2015 Baylibre SAS.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Based on linux/drivers/iio/adc/ad7291.c
> + * Copyright 2010-2011 Analog Devices Inc.
> + *
> + * Based on linux/drivers/hwmon/ina2xx.c
> + * Copyright 2012 Lothar Felten <l-felten@ti.com>
> + *
> + * Licensed under the GPL-2 or later.
> + *
> + * IIO driver for INA219-220-226-230-231
> + *
> + * Configurable 7-bit I2C slave address from 0x40 to 0x4F
> + */
> +#include <linux/module.h>
> +#include <linux/kthread.h>
> +#include <linux/delay.h>
> +#include <linux/iio/kfifo_buf.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/i2c.h>
> +#include <linux/regmap.h>
> +#include <linux/platform_data/ina2xx.h>
> +
> +#include <linux/util_macros.h>
> +
> +/* INA2XX registers definition */
> +#define INA2XX_CONFIG                   0x00
> +#define INA2XX_SHUNT_VOLTAGE            0x01	/* readonly */
> +#define INA2XX_BUS_VOLTAGE              0x02	/* readonly */
> +#define INA2XX_POWER                    0x03	/* readonly */
> +#define INA2XX_CURRENT                  0x04	/* readonly */
> +#define INA2XX_CALIBRATION              0x05
> +
> +#define INA226_ALERT_MASK		0x06
> +#define INA266_CVRF			BIT(3)
> +
> +#define INA2XX_MAX_REGISTERS            8
> +
> +/* settings - depend on use case */
> +#define INA219_CONFIG_DEFAULT           0x399F	/* PGA=8 */
> +#define INA226_CONFIG_DEFAULT           0x4327
> +#define INA226_DEFAULT_AVG              4
> +#define INA226_DEFAULT_IT		1110
> +
> +#define INA2XX_RSHUNT_DEFAULT           10000
> +
> +/*
> + * bit mask for reading the averaging setting in the configuration register
> + * FIXME: use regmap_fields.
> + */
> +#define INA2XX_MODE_MASK	GENMASK(3, 0)
> +
> +#define INA226_AVG_MASK		GENMASK(11, 9)
> +#define INA226_SHIFT_AVG(val)	((val) << 9)
> +
> +/* Integration time for VBus */
> +#define INA226_ITB_MASK		GENMASK(8, 6)
> +#define INA226_SHIFT_ITB(val)	((val) << 6)
> +
> +/* Integration time for VShunt */
> +#define INA226_ITS_MASK		GENMASK(5, 3)
> +#define INA226_SHIFT_ITS(val)	((val) << 3)
> +
> +/* Cosmetic macro giving the sampling period for a full P=UxI cycle */
> +#define SAMPLING_PERIOD(c)	((c->int_time_vbus + c->int_time_vshunt) \
> +				 * c->avg)
> +
> +static bool ina2xx_is_writeable_reg(struct device *dev, unsigned int reg)
> +{
> +	return (reg == INA2XX_CONFIG) || (reg > INA2XX_CURRENT);
> +}
> +
> +static bool ina2xx_is_volatile_reg(struct device *dev, unsigned int reg)
> +{
> +	return (reg != INA2XX_CONFIG);
> +}
> +
> +static const struct regmap_config ina2xx_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 16,
> +	.max_register = INA2XX_MAX_REGISTERS,
> +	.writeable_reg = ina2xx_is_writeable_reg,
> +	.volatile_reg = ina2xx_is_volatile_reg,
> +};
> +
> +enum ina2xx_ids { ina219, ina226 };
> +
> +struct ina2xx_config {
> +	u16 config_default;
> +	int calibration_factor;
> +	int shunt_div;
> +	int bus_voltage_shift;
> +	int bus_voltage_lsb;	/* uV */
> +	int power_lsb;		/* uW */
> +};
> +
> +struct ina2xx_chip_info {
> +	struct regmap *regmap;
> +	struct task_struct *task;
> +	const struct ina2xx_config *config;
> +	struct mutex state_lock;
> +	long rshunt;
> +	int avg;
> +	s64 prev_ns;	/* track buffer capture time, check for underruns*/
> +	int int_time_vbus; /* Bus voltage integration time uS */
> +	int int_time_vshunt; /* Shunt voltage integration time uS */
> +};
> +
> +static const struct ina2xx_config ina2xx_config[] = {
> +	[ina219] = {
> +		    .config_default = INA219_CONFIG_DEFAULT,
> +		    .calibration_factor = 40960000,
> +		    .shunt_div = 100,
> +		    .bus_voltage_shift = 3,
> +		    .bus_voltage_lsb = 4000,
> +		    .power_lsb = 20000,
> +		    },
> +	[ina226] = {
> +		    .config_default = INA226_CONFIG_DEFAULT,
> +		    .calibration_factor = 5120000,
> +		    .shunt_div = 400,
> +		    .bus_voltage_shift = 0,
> +		    .bus_voltage_lsb = 1250,
> +		    .power_lsb = 25000,
> +		    },
> +};
> +
> +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)
> +{
> +	int ret;
> +	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
> +	unsigned int regval;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = regmap_read(chip->regmap, chan->address, &regval);
> +		if (ret < 0)
> +			return ret;
> +
> +		return ina2xx_get_value(chip, chan->address, regval, val, val2);
> +
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		*val = chip->avg;
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_INT_TIME:
> +		*val = 0;
> +		if (chan->address == INA2XX_SHUNT_VOLTAGE)
> +			*val2 = chip->int_time_vshunt;
> +		else
> +			*val2 = chip->int_time_vbus;
> +
> +		return IIO_VAL_INT_PLUS_MICRO;
> +
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		/*
> +		 * Sample freq is read only, it is a consequence of
> +		 * 1/AVG*(CT_bus+CT_shunt).
> +		 */
> +		*val = DIV_ROUND_CLOSEST(1000000, SAMPLING_PERIOD(chip));
> +
> +		return IIO_VAL_INT;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * 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 int ina226_set_average(struct ina2xx_chip_info *chip, unsigned int val,
> +			      unsigned int *config)
> +{
> +	int bits;
> +
> +	if (val > 1024 || val < 1)
> +		return -EINVAL;
> +
> +	bits = find_closest(val, ina226_avg_tab,
> +			    ARRAY_SIZE(ina226_avg_tab));
> +
> +	chip->avg = ina226_avg_tab[bits];
> +
> +	*config &= ~INA226_AVG_MASK;
> +	*config |= INA226_SHIFT_AVG(bits) & INA226_AVG_MASK;
> +
> +	return 0;
> +}
> +
> +/* Conversion times in uS */
> +static const int ina226_conv_time_tab[] = { 140, 204, 332, 588, 1100,
> +					    2116, 4156, 8244 };
> +
> +static int ina226_set_int_time_vbus(struct ina2xx_chip_info *chip,
> +				    unsigned int val_us, unsigned int *config)
> +{
> +	int bits;
> +
> +	if (val_us > 8244 || val_us < 140)
> +		return -EINVAL;
> +
> +	bits = find_closest(val_us, ina226_conv_time_tab,
> +			ARRAY_SIZE(ina226_conv_time_tab));
> +
> +	chip->int_time_vbus = ina226_conv_time_tab[bits];
> +
> +	*config &= ~INA226_ITB_MASK;
> +	*config |= INA226_SHIFT_ITB(bits) & INA226_ITB_MASK;
> +
> +	return 0;
> +}
> +
> +static int ina226_set_int_time_vshunt(struct ina2xx_chip_info *chip,
> +				      unsigned int val_us, unsigned int *config)
> +{
> +	int bits;
> +
> +	if (val_us > 8244 || val_us < 140)
> +		return -EINVAL;
> +
> +	bits = find_closest(val_us, ina226_conv_time_tab,
> +			ARRAY_SIZE(ina226_conv_time_tab));
> +
> +	chip->int_time_vshunt = ina226_conv_time_tab[bits];
> +
> +	*config &= ~INA226_ITS_MASK;
> +	*config |= INA226_SHIFT_ITS(bits) & INA226_ITS_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)
> +{
> +	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
> +	int ret;
> +	unsigned int config, tmp;
> +
> +	if (iio_buffer_enabled(indio_dev))
> +		return -EBUSY;
> +
> +	mutex_lock(&chip->state_lock);
> +
> +	ret = regmap_read(chip->regmap, INA2XX_CONFIG, &config);
> +	if (ret < 0)
> +		goto _err;
> +
> +	tmp = config;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		ret = ina226_set_average(chip, val, &tmp);
> +		break;
> +
> +	case IIO_CHAN_INFO_INT_TIME:
> +		if (chan->address == INA2XX_SHUNT_VOLTAGE)
> +			ret = ina226_set_int_time_vshunt(chip, val2, &tmp);
> +		else
> +			ret = ina226_set_int_time_vbus(chip, val2, &tmp);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +	if (!ret && (tmp != config))
> +		ret = regmap_write(chip->regmap, INA2XX_CONFIG, tmp);
> +_err:
> +	mutex_unlock(&chip->state_lock);
> +
> +	return ret;
> +}
> +
> +
> +#define INA2XX_CHAN(_type, _index, _address) { \
> +	.type = (_type), \
> +	.address = (_address), \
> +	.indexed = 1, \
> +	.channel = (_index), \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +	.info_mask_shared_by_dir = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
> +				   BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
> +	.scan_index = (_index), \
> +	.scan_type = { \
> +		.sign = 'u', \
> +		.realbits = 16, \
> +		.storagebits = 16, \
> +		.endianness = IIO_BE, \
> +	} \
> +}
> +
> +/*
> + * Sampling Freq is a consequence of the integration times of
> + * the Voltage channels.
> + */
> +#define INA2XX_CHAN_VOLTAGE(_index, _address) { \
> +	.type = IIO_VOLTAGE, \
> +	.address = (_address), \
> +	.indexed = 1, \
> +	.channel = (_index), \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> +			      BIT(IIO_CHAN_INFO_INT_TIME), \
> +	.scan_index = (_index), \
> +	.scan_type = { \
> +		.sign = 'u', \
> +		.realbits = 16, \
> +		.storagebits = 16, \
> +		.endianness = IIO_BE, \
> +	} \
> +}
> +
> +static const struct iio_chan_spec ina2xx_channels[] = {
> +	INA2XX_CHAN_VOLTAGE(0, INA2XX_SHUNT_VOLTAGE),
> +	INA2XX_CHAN_VOLTAGE(1, INA2XX_BUS_VOLTAGE),
> +	INA2XX_CHAN(IIO_CURRENT, 2, INA2XX_CURRENT),
> +	INA2XX_CHAN(IIO_POWER, 3, INA2XX_POWER),
> +	IIO_CHAN_SOFT_TIMESTAMP(4),
> +};
> +
> +static int ina2xx_work_buffer(struct iio_dev *indio_dev)
> +{
> +	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
> +	unsigned short data[8];
> +	int bit, ret, i = 0;
> +	unsigned long buffer_us, elapsed_us;
> +	s64 time_a, time_b;
> +	unsigned int alert;
> +
> +	time_a = iio_get_time_ns();
> +
> +	/*
> +	 * Because the timer thread and the chip conversion clock
> +	 * are asynchronous, the period difference will eventually
> +	 * result in reading V[k-1] again, or skip V[k] at time Tk.
> +	 * In order to resync the timer with the conversion process
> +	 * we check the ConVersionReadyFlag.
> +	 * On hardware that supports using the ALERT pin to toggle a
> +	 * GPIO a triggered buffer could be used instead.
> +	 * For now, we pay for that extra read of the ALERT register
> +	 */
> +	do {
> +		ret = regmap_read(chip->regmap, INA226_ALERT_MASK,
> +				  &alert);
> +		if (ret < 0)
> +			return ret;
> +
> +		alert &= INA266_CVRF;
> +		trace_printk("Conversion ready: %d\n", !!alert);
> +
> +	} while (!alert);
> +
> +	/*
> +	 * Single register reads: bulk_read will not work with ina226
> +	 * as there is no auto-increment of the address register for
> +	 * data length longer than 16bits.
> +	 */
> +	for_each_set_bit(bit, indio_dev->active_scan_mask,
> +			 indio_dev->masklength) {
> +		unsigned int val;
> +
> +		ret = regmap_read(chip->regmap,
> +				  INA2XX_SHUNT_VOLTAGE + bit, &val);
> +		if (ret < 0)
> +			return ret;
> +
> +		data[i++] = val;
> +	}
> +
> +	time_b = iio_get_time_ns();
> +
> +	iio_push_to_buffers_with_timestamp(indio_dev,
> +					   (unsigned int *)data, time_a);
> +
> +	buffer_us = (unsigned long)(time_b - time_a) / 1000;
> +	elapsed_us = (unsigned long)(time_a - chip->prev_ns) / 1000;
> +
> +	trace_printk("uS: elapsed: %lu, buf: %lu\n", elapsed_us, buffer_us);
> +
> +	chip->prev_ns = time_a;
> +
> +	return buffer_us;
> +};
> +
> +static int ina2xx_capture_thread(void *data)
> +{
> +	struct iio_dev *indio_dev = (struct iio_dev *)data;
> +	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
> +	unsigned int sampling_us = SAMPLING_PERIOD(chip);
> +	int buffer_us;
> +
> +	/*
> +	 * Poll a bit faster than the chip internal Fs, in case
> +	 * we wish to sync with the conversion ready flag.
> +	 */
> +	sampling_us -= 200;
> +
> +	do {
> +		buffer_us = ina2xx_work_buffer(indio_dev);
> +		if (buffer_us < 0)
> +			return buffer_us;
> +
> +		if (sampling_us > buffer_us)
> +			udelay(sampling_us - buffer_us);
> +
> +	} while (!kthread_should_stop());
> +
> +	return 0;
> +}
> +
> +int ina2xx_buffer_enable(struct iio_dev *indio_dev)
> +{
> +	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
> +	unsigned int sampling_us = SAMPLING_PERIOD(chip);
> +
> +	trace_printk("Enabling buffer w/ scan_mask %02x, freq = %d, avg =%u\n",
> +		     (unsigned int)(*indio_dev->active_scan_mask),
> +		     1000000/sampling_us, chip->avg);
> +
> +	trace_printk("Expected work period: %u us\n", sampling_us);
> +
> +	chip->prev_ns = iio_get_time_ns();
> +
> +	chip->task = kthread_run(ina2xx_capture_thread, (void *)indio_dev,
> +				 "ina2xx-%uus", sampling_us);
> +
> +	return PTR_ERR_OR_ZERO(chip->task);
> +}
> +
> +int ina2xx_buffer_disable(struct iio_dev *indio_dev)
> +{
> +	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
> +
> +	if (chip->task) {
> +		kthread_stop(chip->task);
> +		chip->task = NULL;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct iio_buffer_setup_ops ina2xx_setup_ops = {
> +	.postenable = &ina2xx_buffer_enable,
> +	.postdisable = &ina2xx_buffer_disable,
Sorry, only just noticed this.  The postenable pairs with predisable
(inside and outside the state change for the two pairs). I might just
fix this up when applying (should be predisable)

> +};
> +
> +static int ina2xx_debug_reg(struct iio_dev *indio_dev,
> +			    unsigned reg, unsigned writeval, unsigned *readval)
> +{
> +	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
> +
> +	if (!readval)
> +		return regmap_write(chip->regmap, reg, writeval);
> +
> +	return regmap_read(chip->regmap, reg, readval);
> +}
> +
> +/* Possible integration times for vshunt and vbus */
> +static IIO_CONST_ATTR_INT_TIME_AVAIL \
> + ("0.000140 0.000204 0.000332 0.000588 0.001100 0.002116 0.004156 0.008244");
> +
> +static struct attribute *ina2xx_attributes[] = {
> +	&iio_const_attr_integration_time_available.dev_attr.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group ina2xx_attribute_group = {
> +	.attrs = ina2xx_attributes,
> +};
> +
> +static const struct iio_info ina2xx_info = {
> +	.debugfs_reg_access = &ina2xx_debug_reg,
> +	.read_raw = &ina2xx_read_raw,
> +	.write_raw = &ina2xx_write_raw,
> +	.attrs = &ina2xx_attribute_group,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +/* Initialize the configuration and calibration registers. */
> +static int ina2xx_init(struct ina2xx_chip_info *chip, unsigned int config)
> +{
> +	u16 regval;
> +	int ret = regmap_write(chip->regmap, INA2XX_CONFIG, config);
> +
> +	if (ret < 0)
> +		return ret;
> +	/*
> +	 * Set current LSB to 1mA, shunt is in uOhms
> +	 * (equation 13 in datasheet). We hardcode a Current_LSB
> +	 * of 1.0 x10-6. The only remaining parameter is RShunt.
> +	 * There is no need to expose the CALIBRATION register
> +	 * to the user for now.
> +	 */
> +	regval = DIV_ROUND_CLOSEST(chip->config->calibration_factor,
> +			    chip->rshunt);
> +
> +	return regmap_write(chip->regmap, INA2XX_CALIBRATION, regval);
> +}
> +
> +static int ina2xx_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	struct ina2xx_chip_info *chip;
> +	struct iio_dev *indio_dev;
> +	struct iio_buffer *buffer;
> +	int ret;
> +	unsigned int val;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	chip = iio_priv(indio_dev);
> +
> +	chip->config = &ina2xx_config[id->driver_data];
> +
> +	if (of_property_read_u32(client->dev.of_node,
> +				 "shunt-resistor", &val) < 0) {
> +		struct ina2xx_platform_data *pdata =
> +		    dev_get_platdata(&client->dev);
> +
> +		if (pdata)
> +			val = pdata->shunt_uohms;
> +		else
> +			val = INA2XX_RSHUNT_DEFAULT;
> +	}
> +
> +	if (val <= 0 || val > chip->config->calibration_factor)
> +		return -ENODEV;
> +
> +	chip->rshunt = val;
> +
> +	mutex_init(&chip->state_lock);
> +
> +	/* This is only used for device removal purposes. */
> +	i2c_set_clientdata(client, indio_dev);
> +
> +	indio_dev->name = id->name;
> +	indio_dev->channels = ina2xx_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(ina2xx_channels);
> +
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->info = &ina2xx_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
> +
> +	chip->regmap = devm_regmap_init_i2c(client, &ina2xx_regmap_config);
> +	if (IS_ERR(chip->regmap)) {
> +		dev_err(&client->dev, "failed to allocate register map\n");
> +		return PTR_ERR(chip->regmap);
> +	}
> +
> +	/* Patch the current config register with default. */
> +	val = chip->config->config_default;
> +
> +	if (id->driver_data == ina226) {
> +		ina226_set_average(chip, INA226_DEFAULT_AVG, &val);
> +		ina226_set_int_time_vbus(chip, INA226_DEFAULT_IT, &val);
> +		ina226_set_int_time_vshunt(chip, INA226_DEFAULT_IT, &val);
> +	}
> +
> +	ret = ina2xx_init(chip, val);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "error configuring the device: %d\n",
> +			ret);
> +		return -ENODEV;
> +	}
> +
> +	buffer = devm_iio_kfifo_allocate(&indio_dev->dev);
> +	if (!buffer)
> +		return -ENOMEM;
> +
> +	indio_dev->setup_ops = &ina2xx_setup_ops;
> +
> +	iio_device_attach_buffer(indio_dev, buffer);
> +
> +	return iio_device_register(indio_dev);
> +}
> +
> +
> +static int ina2xx_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
> +
> +	/* Powerdown */
> +	return regmap_update_bits(chip->regmap, INA2XX_CONFIG,
> +				  INA2XX_MODE_MASK, 0);
> +}
> +
> +
> +static const struct i2c_device_id ina2xx_id[] = {
> +	{"ina219", ina219},
> +	{"ina220", ina219},
> +	{"ina226", ina226},
> +	{"ina230", ina226},
> +	{"ina231", ina226},
> +	{}
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, ina2xx_id);
> +
> +static struct i2c_driver ina2xx_driver = {
> +	.driver = {
> +		   .name = KBUILD_MODNAME,
> +	},
> +	.probe = ina2xx_probe,
> +	.remove = ina2xx_remove,
> +	.id_table = ina2xx_id,
> +};
> +
> +module_i2c_driver(ina2xx_driver);
> +
> +MODULE_AUTHOR("Marc Titinger <marc.titinger@baylibre.com>");
> +MODULE_DESCRIPTION("Texas Instruments INA2XX ADC driver");
> +MODULE_LICENSE("GPL v2");
> 

[PATCH] iio: ina2xx: fix channel order in software buffer

[Marc Titinger]

POWER and CURRENT were swapped out in the buffer:
was current2 and power3, correct order is power2 and current3.

Signed-off-by: Marc Titinger <mtitinger@baylibre.com>

---

Hi Jon,

we just found this while testing with the buffered mode.
My apologies for missing this bug ealier.

Cheers,
Marc.

---

 drivers/iio/adc/ina2xx-adc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
index 98939ba..91e1f2b 100644
--- a/drivers/iio/adc/ina2xx-adc.c
+++ b/drivers/iio/adc/ina2xx-adc.c
@@ -427,8 +427,8 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,
 static const struct iio_chan_spec ina2xx_channels[] = {
 	INA2XX_CHAN_VOLTAGE(0, INA2XX_SHUNT_VOLTAGE),
 	INA2XX_CHAN_VOLTAGE(1, INA2XX_BUS_VOLTAGE),
-	INA2XX_CHAN(IIO_CURRENT, 2, INA2XX_CURRENT),
-	INA2XX_CHAN(IIO_POWER, 3, INA2XX_POWER),
+	INA2XX_CHAN(IIO_POWER, 2, INA2XX_POWER),
+	INA2XX_CHAN(IIO_CURRENT, 3, INA2XX_CURRENT),
 	IIO_CHAN_SOFT_TIMESTAMP(4),
 };
 
-- 
1.9.1

Re: [PATCH] iio: ina2xx: fix channel order in software buffer

[Jonathan Cameron]

On 15/12/15 15:26, Marc Titinger wrote:
> POWER and CURRENT were swapped out in the buffer:
> was current2 and power3, correct order is power2 and current3.
> 
> Signed-off-by: Marc Titinger <mtitinger@baylibre.com>
> 
> ---
> 
> Hi Jon,
> 
> we just found this while testing with the buffered mode.
> My apologies for missing this bug ealier.
> 
> Cheers,
> Marc.
> 
> ---
>
Applied this one to the togreg branch of iio.git which will get
initially pushed out as testing for the autobuilders to play with it.
 
>  drivers/iio/adc/ina2xx-adc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
> index 98939ba..91e1f2b 100644
> --- a/drivers/iio/adc/ina2xx-adc.c
> +++ b/drivers/iio/adc/ina2xx-adc.c
> @@ -427,8 +427,8 @@ static ssize_t ina2xx_shunt_resistor_store(struct device *dev,
>  static const struct iio_chan_spec ina2xx_channels[] = {
>  	INA2XX_CHAN_VOLTAGE(0, INA2XX_SHUNT_VOLTAGE),
>  	INA2XX_CHAN_VOLTAGE(1, INA2XX_BUS_VOLTAGE),
> -	INA2XX_CHAN(IIO_CURRENT, 2, INA2XX_CURRENT),
> -	INA2XX_CHAN(IIO_POWER, 3, INA2XX_POWER),
> +	INA2XX_CHAN(IIO_POWER, 2, INA2XX_POWER),
> +	INA2XX_CHAN(IIO_CURRENT, 3, INA2XX_CURRENT),
>  	IIO_CHAN_SOFT_TIMESTAMP(4),
>  };
>  
> 

Re: [PATCH v3 1/2] iio: ina2xx: add support for TI INA2xx Power Monitors

[Andrew F. Davis]

On 12/07/2015 03:09 AM, Marc Titinger wrote:
> +#define INA2XX_CHAN(_type, _index, _address) { \
> +	.type = (_type), \
> +	.address = (_address), \
> +	.indexed = 1, \
> +	.channel = (_index), \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +	.info_mask_shared_by_dir = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
> +				   BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
> +	.scan_index = (_index), \
> +	.scan_type = { \
> +		.sign = 'u', \
> +		.realbits = 16, \
> +		.storagebits = 16, \
> +		.endianness = IIO_BE, \

^^^^ See below.

> +	} \
> +}
> +
> +/*
> + * Sampling Freq is a consequence of the integration times of
> + * the Voltage channels.
> + */
> +#define INA2XX_CHAN_VOLTAGE(_index, _address) { \
> +	.type = IIO_VOLTAGE, \
> +	.address = (_address), \
> +	.indexed = 1, \
> +	.channel = (_index), \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> +			      BIT(IIO_CHAN_INFO_INT_TIME), \
> +	.scan_index = (_index), \
> +	.scan_type = { \
> +		.sign = 'u', \
> +		.realbits = 16, \
> +		.storagebits = 16, \
> +		.endianness = IIO_BE, \
> +	} \
> +}
> +
> +static const struct iio_chan_spec ina2xx_channels[] = {
> +	INA2XX_CHAN_VOLTAGE(0, INA2XX_SHUNT_VOLTAGE),
> +	INA2XX_CHAN_VOLTAGE(1, INA2XX_BUS_VOLTAGE),
> +	INA2XX_CHAN(IIO_CURRENT, 2, INA2XX_CURRENT),
> +	INA2XX_CHAN(IIO_POWER, 3, INA2XX_POWER),
> +	IIO_CHAN_SOFT_TIMESTAMP(4),
> +};
> +
> +static int ina2xx_work_buffer(struct iio_dev *indio_dev)
> +{
> +	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
> +	unsigned short data[8];
> +	int bit, ret, i = 0;
> +	unsigned long buffer_us, elapsed_us;
> +	s64 time_a, time_b;
> +	unsigned int alert;
> +
> +	time_a = iio_get_time_ns();
> +
> +	/*
> +	 * Because the timer thread and the chip conversion clock
> +	 * are asynchronous, the period difference will eventually
> +	 * result in reading V[k-1] again, or skip V[k] at time Tk.
> +	 * In order to resync the timer with the conversion process
> +	 * we check the ConVersionReadyFlag.
> +	 * On hardware that supports using the ALERT pin to toggle a
> +	 * GPIO a triggered buffer could be used instead.
> +	 * For now, we pay for that extra read of the ALERT register
> +	 */
> +	do {
> +		ret = regmap_read(chip->regmap, INA226_ALERT_MASK,
> +				  &alert);
> +		if (ret < 0)
> +			return ret;
> +
> +		alert &= INA266_CVRF;
> +		trace_printk("Conversion ready: %d\n", !!alert);
> +
> +	} while (!alert);
> +
> +	/*
> +	 * Single register reads: bulk_read will not work with ina226
> +	 * as there is no auto-increment of the address register for
> +	 * data length longer than 16bits.
> +	 */
> +	for_each_set_bit(bit, indio_dev->active_scan_mask,
> +			 indio_dev->masklength) {
> +		unsigned int val;
> +
> +		ret = regmap_read(chip->regmap,
> +				  INA2XX_SHUNT_VOLTAGE + bit, &val);
> +		if (ret < 0)
> +			return ret;
> +
> +		data[i++] = val;

The read above seems to fill the buffer in CPU order, but above
IIO_BE is specified, this should probably be IIO_CPU to avoid
confusion for programs that check for IIO buffer endianness.

I'll push this in a fixup patch later. Just wanted to give a
heads up if you run into problems with some program.

Re: [PATCH v3 1/2] iio: ina2xx: add support for TI INA2xx Power Monitors

[Marc Titinger]

On 16/12/2015 18:45, Andrew F. Davis wrote:
> On 12/07/2015 03:09 AM, Marc Titinger wrote:
>> +#define INA2XX_CHAN(_type, _index, _address) { \
>> +    .type = (_type), \
>> +    .address = (_address), \
>> +    .indexed = 1, \
>> +    .channel = (_index), \
>> +    .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
>> +    .info_mask_shared_by_dir = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
>> +                   BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
>> +    .scan_index = (_index), \
>> +    .scan_type = { \
>> +        .sign = 'u', \
>> +        .realbits = 16, \
>> +        .storagebits = 16, \
>> +        .endianness = IIO_BE, \
>
> ^^^^ See below.
>
>> +    } \
>> +}
>> +
>> +/*
>> + * Sampling Freq is a consequence of the integration times of
>> + * the Voltage channels.
>> + */
>> +#define INA2XX_CHAN_VOLTAGE(_index, _address) { \
>> +    .type = IIO_VOLTAGE, \
>> +    .address = (_address), \
>> +    .indexed = 1, \
>> +    .channel = (_index), \
>> +    .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
>> +                  BIT(IIO_CHAN_INFO_INT_TIME), \
>> +    .scan_index = (_index), \
>> +    .scan_type = { \
>> +        .sign = 'u', \
>> +        .realbits = 16, \
>> +        .storagebits = 16, \
>> +        .endianness = IIO_BE, \
>> +    } \
>> +}
>> +
>> +static const struct iio_chan_spec ina2xx_channels[] = {
>> +    INA2XX_CHAN_VOLTAGE(0, INA2XX_SHUNT_VOLTAGE),
>> +    INA2XX_CHAN_VOLTAGE(1, INA2XX_BUS_VOLTAGE),
>> +    INA2XX_CHAN(IIO_CURRENT, 2, INA2XX_CURRENT),
>> +    INA2XX_CHAN(IIO_POWER, 3, INA2XX_POWER),
>> +    IIO_CHAN_SOFT_TIMESTAMP(4),
>> +};
>> +
>> +static int ina2xx_work_buffer(struct iio_dev *indio_dev)
>> +{
>> +    struct ina2xx_chip_info *chip = iio_priv(indio_dev);
>> +    unsigned short data[8];
>> +    int bit, ret, i = 0;
>> +    unsigned long buffer_us, elapsed_us;
>> +    s64 time_a, time_b;
>> +    unsigned int alert;
>> +
>> +    time_a = iio_get_time_ns();
>> +
>> +    /*
>> +     * Because the timer thread and the chip conversion clock
>> +     * are asynchronous, the period difference will eventually
>> +     * result in reading V[k-1] again, or skip V[k] at time Tk.
>> +     * In order to resync the timer with the conversion process
>> +     * we check the ConVersionReadyFlag.
>> +     * On hardware that supports using the ALERT pin to toggle a
>> +     * GPIO a triggered buffer could be used instead.
>> +     * For now, we pay for that extra read of the ALERT register
>> +     */
>> +    do {
>> +        ret = regmap_read(chip->regmap, INA226_ALERT_MASK,
>> +                  &alert);
>> +        if (ret < 0)
>> +            return ret;
>> +
>> +        alert &= INA266_CVRF;
>> +        trace_printk("Conversion ready: %d\n", !!alert);
>> +
>> +    } while (!alert);
>> +
>> +    /*
>> +     * Single register reads: bulk_read will not work with ina226
>> +     * as there is no auto-increment of the address register for
>> +     * data length longer than 16bits.
>> +     */
>> +    for_each_set_bit(bit, indio_dev->active_scan_mask,
>> +             indio_dev->masklength) {
>> +        unsigned int val;
>> +
>> +        ret = regmap_read(chip->regmap,
>> +                  INA2XX_SHUNT_VOLTAGE + bit, &val);
>> +        if (ret < 0)
>> +            return ret;
>> +
>> +        data[i++] = val;
>
> The read above seems to fill the buffer in CPU order, but above
> IIO_BE is specified, this should probably be IIO_CPU to avoid
> confusion for programs that check for IIO buffer endianness.

Hi Andrew,

Well spotted. I'm sorry it is getting a bit messy because I posted a fix 
for this in a separate patch (see https://lkml.org/lkml/2015/12/12/131)

Using IIO_CPU seems an even better option, I did not know about this 
value and used IIO_LE.

Thanks!
Marc.




>
> I'll push this in a fixup patch later. Just wanted to give a
> heads up if you run into problems with some program.

Re: [PATCH v3 1/2] iio: ina2xx: add support for TI INA2xx Power Monitors

[Andrew F. Davis]

On 12/16/2015 11:54 AM, Marc Titinger wrote:
> On 16/12/2015 18:45, Andrew F. Davis wrote:
>> On 12/07/2015 03:09 AM, Marc Titinger wrote:
>>> +#define INA2XX_CHAN(_type, _index, _address) { \
>>> +    .type = (_type), \
>>> +    .address = (_address), \
>>> +    .indexed = 1, \
>>> +    .channel = (_index), \
>>> +    .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
>>> +    .info_mask_shared_by_dir = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
>>> +                   BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
>>> +    .scan_index = (_index), \
>>> +    .scan_type = { \
>>> +        .sign = 'u', \
>>> +        .realbits = 16, \
>>> +        .storagebits = 16, \
>>> +        .endianness = IIO_BE, \
>>
>> ^^^^ See below.
>>
>>> +    } \
>>> +}
>>> +
>>> +/*
>>> + * Sampling Freq is a consequence of the integration times of
>>> + * the Voltage channels.
>>> + */
>>> +#define INA2XX_CHAN_VOLTAGE(_index, _address) { \
>>> +    .type = IIO_VOLTAGE, \
>>> +    .address = (_address), \
>>> +    .indexed = 1, \
>>> +    .channel = (_index), \
>>> +    .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
>>> +                  BIT(IIO_CHAN_INFO_INT_TIME), \
>>> +    .scan_index = (_index), \
>>> +    .scan_type = { \
>>> +        .sign = 'u', \
>>> +        .realbits = 16, \
>>> +        .storagebits = 16, \
>>> +        .endianness = IIO_BE, \
>>> +    } \
>>> +}
>>> +
>>> +static const struct iio_chan_spec ina2xx_channels[] = {
>>> +    INA2XX_CHAN_VOLTAGE(0, INA2XX_SHUNT_VOLTAGE),
>>> +    INA2XX_CHAN_VOLTAGE(1, INA2XX_BUS_VOLTAGE),
>>> +    INA2XX_CHAN(IIO_CURRENT, 2, INA2XX_CURRENT),
>>> +    INA2XX_CHAN(IIO_POWER, 3, INA2XX_POWER),
>>> +    IIO_CHAN_SOFT_TIMESTAMP(4),
>>> +};
>>> +
>>> +static int ina2xx_work_buffer(struct iio_dev *indio_dev)
>>> +{
>>> +    struct ina2xx_chip_info *chip = iio_priv(indio_dev);
>>> +    unsigned short data[8];
>>> +    int bit, ret, i = 0;
>>> +    unsigned long buffer_us, elapsed_us;
>>> +    s64 time_a, time_b;
>>> +    unsigned int alert;
>>> +
>>> +    time_a = iio_get_time_ns();
>>> +
>>> +    /*
>>> +     * Because the timer thread and the chip conversion clock
>>> +     * are asynchronous, the period difference will eventually
>>> +     * result in reading V[k-1] again, or skip V[k] at time Tk.
>>> +     * In order to resync the timer with the conversion process
>>> +     * we check the ConVersionReadyFlag.
>>> +     * On hardware that supports using the ALERT pin to toggle a
>>> +     * GPIO a triggered buffer could be used instead.
>>> +     * For now, we pay for that extra read of the ALERT register
>>> +     */
>>> +    do {
>>> +        ret = regmap_read(chip->regmap, INA226_ALERT_MASK,
>>> +                  &alert);
>>> +        if (ret < 0)
>>> +            return ret;
>>> +
>>> +        alert &= INA266_CVRF;
>>> +        trace_printk("Conversion ready: %d\n", !!alert);
>>> +
>>> +    } while (!alert);
>>> +
>>> +    /*
>>> +     * Single register reads: bulk_read will not work with ina226
>>> +     * as there is no auto-increment of the address register for
>>> +     * data length longer than 16bits.
>>> +     */
>>> +    for_each_set_bit(bit, indio_dev->active_scan_mask,
>>> +             indio_dev->masklength) {
>>> +        unsigned int val;
>>> +
>>> +        ret = regmap_read(chip->regmap,
>>> +                  INA2XX_SHUNT_VOLTAGE + bit, &val);
>>> +        if (ret < 0)
>>> +            return ret;
>>> +
>>> +        data[i++] = val;
>>
>> The read above seems to fill the buffer in CPU order, but above
>> IIO_BE is specified, this should probably be IIO_CPU to avoid
>> confusion for programs that check for IIO buffer endianness.
>
> Hi Andrew,
>
> Well spotted. I'm sorry it is getting a bit messy because I posted a fix for this in a separate patch (see https://lkml.org/lkml/2015/12/12/131)
>

Ah, wasn't following that.

> Using IIO_CPU seems an even better option, I did not know about this value and used IIO_LE.
>

Which will give incorrect results on BE systems, I'll wait till the dust settles for this
driver and push some more small fixups a bit later.

> Thanks!
> Marc.
>

[PATCH v3 2/2] iio: ina2xx: provide a sysfs parameter to allow async readout of the ADCs

[Marc Titinger]

This can lead to repeated or skipped samples depending on the clock beat
between the capture thread and the chip sampling clock, but will also spare
reading/waiting for the Capture Ready Flag and improve the available i2c
bandwidth for reading measurements.

Output of iio_info:
...snip...
4 device-specific attributes found:
  attr 0: in_oversampling_ratio value: 4
  attr 1: in_allow_async_readout value: 0
  attr 2: integration_time_available value: 140 204 332 588 1100 2116...
  attr 3: in_sampling_frequency value: 114

Signed-off-by: Marc Titinger <mtitinger@baylibre.com>
---
 drivers/iio/adc/ina2xx-adc.c | 54 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 45 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
index 0e7c474..615c203 100644
--- a/drivers/iio/adc/ina2xx-adc.c
+++ b/drivers/iio/adc/ina2xx-adc.c
@@ -111,6 +111,7 @@ struct ina2xx_chip_info {
 	s64 prev_ns;	/* track buffer capture time, check for underruns*/
 	int int_time_vbus; /* Bus voltage integration time uS */
 	int int_time_vshunt; /* Shunt voltage integration time uS */
+	bool allow_async_readout;
 };
 
 static const struct ina2xx_config ina2xx_config[] = {
@@ -326,6 +327,33 @@ _err:
 }
 
 
+static ssize_t ina2xx_allow_async_readout_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->allow_async_readout);
+}
+
+static ssize_t ina2xx_allow_async_readout_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));
+	bool val;
+	int ret;
+
+	ret = strtobool((const char *) buf, &val);
+	if (ret)
+		return ret;
+
+	chip->allow_async_readout = val;
+
+	return len;
+}
+
+
 #define INA2XX_CHAN(_type, _index, _address) { \
 	.type = (_type), \
 	.address = (_address), \
@@ -392,16 +420,17 @@ static int ina2xx_work_buffer(struct iio_dev *indio_dev)
 	 * GPIO a triggered buffer could be used instead.
 	 * For now, we pay for that extra read of the ALERT register
 	 */
-	do {
-		ret = regmap_read(chip->regmap, INA226_ALERT_MASK,
-				  &alert);
-		if (ret < 0)
-			return ret;
+	if (!chip->allow_async_readout)
+		do {
+			ret = regmap_read(chip->regmap, INA226_ALERT_MASK,
+					  &alert);
+			if (ret < 0)
+				return ret;
 
-		alert &= INA266_CVRF;
-		trace_printk("Conversion ready: %d\n", !!alert);
+			alert &= INA266_CVRF;
+			trace_printk("Conversion ready: %d\n", !!alert);
 
-	} while (!alert);
+		} while (!alert);
 
 	/*
 	 * Single register reads: bulk_read will not work with ina226
@@ -446,7 +475,8 @@ static int ina2xx_capture_thread(void *data)
 	 * Poll a bit faster than the chip internal Fs, in case
 	 * we wish to sync with the conversion ready flag.
 	 */
-	sampling_us -= 200;
+	if (!chip->allow_async_readout)
+		sampling_us -= 200;
 
 	do {
 		buffer_us = ina2xx_work_buffer(indio_dev);
@@ -471,6 +501,7 @@ int ina2xx_buffer_enable(struct iio_dev *indio_dev)
 		     1000000/sampling_us, chip->avg);
 
 	trace_printk("Expected work period: %u us\n", sampling_us);
+	trace_printk("Async readout mode: %d\n", chip->allow_async_readout);
 
 	chip->prev_ns = iio_get_time_ns();
 
@@ -512,7 +543,12 @@ static int ina2xx_debug_reg(struct iio_dev *indio_dev,
 static IIO_CONST_ATTR_INT_TIME_AVAIL \
  ("0.000140 0.000204 0.000332 0.000588 0.001100 0.002116 0.004156 0.008244");
 
+static IIO_DEVICE_ATTR(in_allow_async_readout, S_IRUGO | S_IWUSR,
+		       ina2xx_allow_async_readout_show,
+		       ina2xx_allow_async_readout_store, 0);
+
 static struct attribute *ina2xx_attributes[] = {
+	&iio_dev_attr_in_allow_async_readout.dev_attr.attr,
 	&iio_const_attr_integration_time_available.dev_attr.attr,
 	NULL,
 };
-- 
1.9.1

Re: [PATCH v3 2/2] iio: ina2xx: provide a sysfs parameter to allow async readout of the ADCs

[Jonathan Cameron]

On 07/12/15 09:09, Marc Titinger wrote:
> This can lead to repeated or skipped samples depending on the clock beat
> between the capture thread and the chip sampling clock, but will also spare
> reading/waiting for the Capture Ready Flag and improve the available i2c
> bandwidth for reading measurements.
> 
> Output of iio_info:
> ...snip...
> 4 device-specific attributes found:
>   attr 0: in_oversampling_ratio value: 4
>   attr 1: in_allow_async_readout value: 0
>   attr 2: integration_time_available value: 140 204 332 588 1100 2116...
>   attr 3: in_sampling_frequency value: 114
> 
> Signed-off-by: Marc Titinger <mtitinger@baylibre.com>
Applied to the togreg branch of iio.git - initially pushed out as testing for the autobuilders
to play with it.

Could you prepare a follow up patch adding abi docs to
Documentation/ABI/testing/sysfs-bus-iio-ina2xx-adc 
please.

I didn't want to hold this up by insisting on docs now, but it does need
doing asap.

Jonathan
> ---
>  drivers/iio/adc/ina2xx-adc.c | 54 ++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 45 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
> index 0e7c474..615c203 100644
> --- a/drivers/iio/adc/ina2xx-adc.c
> +++ b/drivers/iio/adc/ina2xx-adc.c
> @@ -111,6 +111,7 @@ struct ina2xx_chip_info {
>  	s64 prev_ns;	/* track buffer capture time, check for underruns*/
>  	int int_time_vbus; /* Bus voltage integration time uS */
>  	int int_time_vshunt; /* Shunt voltage integration time uS */
> +	bool allow_async_readout;
>  };
>  
>  static const struct ina2xx_config ina2xx_config[] = {
> @@ -326,6 +327,33 @@ _err:
>  }
>  
>  
> +static ssize_t ina2xx_allow_async_readout_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->allow_async_readout);
> +}
> +
> +static ssize_t ina2xx_allow_async_readout_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));
> +	bool val;
> +	int ret;
> +
> +	ret = strtobool((const char *) buf, &val);
> +	if (ret)
> +		return ret;
> +
> +	chip->allow_async_readout = val;
> +
> +	return len;
> +}
> +
> +
>  #define INA2XX_CHAN(_type, _index, _address) { \
>  	.type = (_type), \
>  	.address = (_address), \
> @@ -392,16 +420,17 @@ static int ina2xx_work_buffer(struct iio_dev *indio_dev)
>  	 * GPIO a triggered buffer could be used instead.
>  	 * For now, we pay for that extra read of the ALERT register
>  	 */
> -	do {
> -		ret = regmap_read(chip->regmap, INA226_ALERT_MASK,
> -				  &alert);
> -		if (ret < 0)
> -			return ret;
> +	if (!chip->allow_async_readout)
> +		do {
> +			ret = regmap_read(chip->regmap, INA226_ALERT_MASK,
> +					  &alert);
> +			if (ret < 0)
> +				return ret;
>  
> -		alert &= INA266_CVRF;
> -		trace_printk("Conversion ready: %d\n", !!alert);
> +			alert &= INA266_CVRF;
> +			trace_printk("Conversion ready: %d\n", !!alert);
>  
> -	} while (!alert);
> +		} while (!alert);
>  
>  	/*
>  	 * Single register reads: bulk_read will not work with ina226
> @@ -446,7 +475,8 @@ static int ina2xx_capture_thread(void *data)
>  	 * Poll a bit faster than the chip internal Fs, in case
>  	 * we wish to sync with the conversion ready flag.
>  	 */
> -	sampling_us -= 200;
> +	if (!chip->allow_async_readout)
> +		sampling_us -= 200;
>  
>  	do {
>  		buffer_us = ina2xx_work_buffer(indio_dev);
> @@ -471,6 +501,7 @@ int ina2xx_buffer_enable(struct iio_dev *indio_dev)
>  		     1000000/sampling_us, chip->avg);
>  
>  	trace_printk("Expected work period: %u us\n", sampling_us);
> +	trace_printk("Async readout mode: %d\n", chip->allow_async_readout);
>  
>  	chip->prev_ns = iio_get_time_ns();
>  
> @@ -512,7 +543,12 @@ static int ina2xx_debug_reg(struct iio_dev *indio_dev,
>  static IIO_CONST_ATTR_INT_TIME_AVAIL \
>   ("0.000140 0.000204 0.000332 0.000588 0.001100 0.002116 0.004156 0.008244");
>  
> +static IIO_DEVICE_ATTR(in_allow_async_readout, S_IRUGO | S_IWUSR,
> +		       ina2xx_allow_async_readout_show,
> +		       ina2xx_allow_async_readout_store, 0);
> +
>  static struct attribute *ina2xx_attributes[] = {
> +	&iio_dev_attr_in_allow_async_readout.dev_attr.attr,
>  	&iio_const_attr_integration_time_available.dev_attr.attr,
>  	NULL,
>  };
> 

[PATCH] iio: ina2xx: add ABI documentation entry sysfs-bus-iio-ina2xx-adc

[Marc Titinger]

Documentation for attributes:

 * in_allow_async_readout
 * in_shunt_resistor

Signed-off-by: Marc Titinger <mtitinger@baylibre.com>
---
 Documentation/ABI/testing/sysfs-bus-iio-ina2xx-adc | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-ina2xx-adc

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-ina2xx-adc b/Documentation/ABI/testing/sysfs-bus-iio-ina2xx-adc
new file mode 100644
index 0000000..8916f7e
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-ina2xx-adc
@@ -0,0 +1,24 @@
+What:		/sys/bus/iio/devices/iio:deviceX/in_allow_async_readout
+Date:		December 2015
+KernelVersion:	4.4
+Contact:	linux-iio@vger.kernel.org
+Description:
+		By default (value '0'), the capture thread checks for the Conversion
+		Ready Flag to being set prior to committing a new value to the sample
+		buffer. This synchronizes the in-chip conversion rate with the
+		in-driver readout rate at the cost of an additional register read.
+
+		Writing '1' will remove the polling for the Conversion Ready Flags to
+		save the additional i2c transaction, which will improve the bandwidth
+		available for reading data. However, samples can be occasionally skipped
+		or repeated, depending on the beat between the capture and conversion
+		rates.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_shunt_resistor
+Date:		December 2015
+KernelVersion:	4.4
+Contact:	linux-iio@vger.kernel.org
+Description:
+		The value of the shunt resistor may be known only at runtime fom an
+		eeprom content read by a client application. This attribute allows to
+		set its value in ohms.
-- 
1.9.1