* [RFC 0/4] IIO: add support for INA2xx power monitor @ 2015-11-10 16:07 Marc Titinger 2015-11-10 16:07 ` [RFC 1/4] iio: ina2xx: add direct IO support for TI INA2xx Power Monitors Marc Titinger ` (3 more replies) 0 siblings, 4 replies; 20+ messages in thread From: Marc Titinger @ 2015-11-10 16:07 UTC (permalink / raw) To: jic23, knaack.h, lars, pmeerw Cc: linux-iio, linux-kernel, mturquette, bcousson, ptitiano, Marc Titinger This chip has fair support in the hwmon stack already, this work is more as a pathfinder for me, hence I post it as RFC to digg some more into 'does and donts' with IIO. Nevertheless, it provides a working streaming scheme for capturing power/voltage/current with this chip. It works in local and remote mode with iio_readdev and I did some sniffing tests with iio-oscilloscope with promising results inspite of timeout issues for long temporal buffers presumably due to the slow rates for this chip compared to expected high-speed CoDecs (and maybe the lack of a proper plugin?). The kthread I'm using does an active waiting to allow for sampling periods shorter than a tick. I have not experienced performance issues with it on the board (BeagleBoneBlack), IIOD could always schedule on time as far as I could see, maybe with other i2c backends a schedule() could be mandatory ? Many thanks, Marc Titinger (4): iio: ina2xx: add direct IO support for TI INA2xx Power Monitors iio: ina2xx: add SAMP_FREQ attribute. iio: ina2xx: add debugfs reg access iio: ina2xx: add SOFTWARE buffer mode using an iio kfifo. drivers/iio/adc/Kconfig | 11 + drivers/iio/adc/Makefile | 1 + drivers/iio/adc/ina2xx-iio.c | 579 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 591 insertions(+) create mode 100644 drivers/iio/adc/ina2xx-iio.c -- 1.9.1 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC 1/4] iio: ina2xx: add direct IO support for TI INA2xx Power Monitors 2015-11-10 16:07 [RFC 0/4] IIO: add support for INA2xx power monitor Marc Titinger @ 2015-11-10 16:07 ` Marc Titinger 2015-11-11 10:14 ` Daniel Baluta 2015-11-11 12:09 ` Daniel Baluta 2015-11-10 16:07 ` [RFC 2/4] iio: ina2xx: add SAMP_FREQ attribute Marc Titinger ` (2 subsequent siblings) 3 siblings, 2 replies; 20+ messages in thread From: Marc Titinger @ 2015-11-10 16:07 UTC (permalink / raw) To: jic23, knaack.h, lars, pmeerw Cc: linux-iio, linux-kernel, mturquette, bcousson, ptitiano, Marc Titinger Basic support or direct IO raw read, with averaging attribute. Values are RAW, INT_PLUS_MICRO (Volt/Ampere/Watt). IIO context has 1 devices: 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 Signed-off-by: Marc Titinger <mtitinger@baylibre.com> --- drivers/iio/adc/Kconfig | 9 + drivers/iio/adc/Makefile | 1 + drivers/iio/adc/ina2xx-iio.c | 404 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 414 insertions(+) create mode 100644 drivers/iio/adc/ina2xx-iio.c diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index eb0cd89..b648051 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -10,6 +10,15 @@ config AD_SIGMA_DELTA select IIO_BUFFER select IIO_TRIGGERED_BUFFER +config INA2XX_IIO + tristate "Texas Instruments INA2xx Power Monitors IIO driver" + depends on I2C + help + Say yes here to build support for TI INA2xx familly Power Monitors. + + Note that this is not the existing hwmon interface for this device. + + config AD7266 tristate "Analog Devices AD7265/AD7266 ADC driver" depends on SPI_MASTER diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile index a096210..e6a844a 100644 --- a/drivers/iio/adc/Makefile +++ b/drivers/iio/adc/Makefile @@ -5,6 +5,7 @@ # When adding new entries keep the list in alphabetical order obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o obj-$(CONFIG_AD7266) += ad7266.o +obj-$(CONFIG_INA2XX_IIO) += ina2xx-iio.o obj-$(CONFIG_AD7291) += ad7291.o obj-$(CONFIG_AD7298) += ad7298.o obj-$(CONFIG_AD7923) += ad7923.o diff --git a/drivers/iio/adc/ina2xx-iio.c b/drivers/iio/adc/ina2xx-iio.c new file mode 100644 index 0000000..257d8d5 --- /dev/null +++ b/drivers/iio/adc/ina2xx-iio.c @@ -0,0 +1,404 @@ +/* + * 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. + */ +#include <linux/module.h> +#include <linux/iio/iio.h> +#include <linux/i2c.h> +#include <linux/regmap.h> +#include <linux/platform_data/ina2xx.h> + +#include <linux/util_macros.h> + +/* + * INA2XX registers definition + */ +/* common register definitions */ +#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 + +/* register count */ +#define INA219_REGISTERS 6 +#define INA226_REGISTERS 8 +#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 INA2XX_RSHUNT_DEFAULT 10000 + +/* bit mask for reading the averaging setting in the configuration register */ +#define INA226_AVG_RD_MASK 0x0E00 +#define INA226_READ_AVG(reg) (((reg) & INA226_AVG_RD_MASK) >> 9) +#define INA226_SHIFT_AVG(val) ((val) << 9) + +static struct regmap_config ina2xx_regmap_config = { + .reg_bits = 8, + .val_bits = 16, +}; + +enum ina2xx_ids { ina219, ina226 }; + +struct ina2xx_config { + u16 config_default; + int calibration_factor; + int registers; + int shunt_div; + int bus_voltage_shift; + int bus_voltage_lsb; /* uV */ + int power_lsb; /* uW */ +}; + +struct ina2xx_chip_info { + struct iio_dev *indio_dev; + const struct ina2xx_config *config; + struct mutex state_lock; + long rshunt; + int avg; + int freq; + int period_us; + struct regmap *regmap; +}; + +static const struct ina2xx_config ina2xx_config[] = { + [ina219] = { + .config_default = INA219_CONFIG_DEFAULT, + .calibration_factor = 40960000, + .registers = INA219_REGISTERS, + .shunt_div = 100, + .bus_voltage_shift = 3, + .bus_voltage_lsb = 4000, + .power_lsb = 20000, + }, + [ina226] = { + .config_default = INA226_CONFIG_DEFAULT, + .calibration_factor = 5120000, + .registers = INA226_REGISTERS, + .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); + *val = *uval/1000000; + *uval = *uval - *val*1000000; + 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 - *val*1000000; + return IIO_VAL_INT_PLUS_MICRO; + + case INA2XX_POWER: + *uval = regval * chip->config->power_lsb; + *val = *uval/1000000; + *uval = *uval - *val*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; + + case INA2XX_CALIBRATION: + *val = DIV_ROUND_CLOSEST(chip->config->calibration_factor, + regval); + return IIO_VAL_INT; + + 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, ®val); + if (ret < 0) + return ret; + + return ina2xx_get_value(chip, chan->address, regval, val, val2); + + case IIO_CHAN_INFO_AVERAGE_RAW: + *val = chip->avg; + return IIO_VAL_INT; + + case IIO_CHAN_INFO_CALIBSCALE: + ret = regmap_read(chip->regmap, INA2XX_CALIBRATION, ®val); + if (ret < 0) + return ret; + + return ina2xx_get_value(chip, INA2XX_CALIBRATION, regval, + val, val2); + + default: + return -EINVAL; + } + + return 0; +} + +static int ina2xx_calibrate(struct ina2xx_chip_info *chip) +{ + u16 val = DIV_ROUND_CLOSEST(chip->config->calibration_factor, + chip->rshunt); + + return regmap_write(chip->regmap, INA2XX_CALIBRATION, val); +} + + +/* + * 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 unsigned 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_RD_MASK; + *config |= INA226_SHIFT_AVG(bits) & INA226_AVG_RD_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 = 0; + unsigned int config, tmp; + + 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_AVERAGE_RAW: + + ret = ina226_set_average(chip, val, &tmp); + break; + + default: + ret = -EINVAL; + } + + if (!ret && (tmp != config)) + ret = regmap_write(chip->regmap, INA2XX_CONFIG, config); +_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_AVERAGE_RAW) | \ + BIT(IIO_CHAN_INFO_CALIBSCALE), \ + .scan_index = (_index), \ + .scan_type = { \ + .sign = 'u', \ + .realbits = 16, \ + .storagebits = 16, \ + .shift = 0, \ + .endianness = IIO_BE, \ + } \ +} + +static const struct iio_chan_spec ina2xx_channels[] = { + INA2XX_CHAN(IIO_VOLTAGE, 0, INA2XX_SHUNT_VOLTAGE), + INA2XX_CHAN(IIO_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 const struct iio_info ina2xx_info = { + .read_raw = &ina2xx_read_raw, + .write_raw = &ina2xx_write_raw, + .driver_module = THIS_MODULE, +}; + +/* +* Initialize the configuration and calibration registers. +*/ +static int ina2xx_init(struct ina2xx_chip_info *chip, unsigned int config) +{ + 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). + */ + return ina2xx_calibrate(chip); +} + +static int ina2xx_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct ina2xx_chip_info *chip; + struct iio_dev *indio_dev; + 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->indio_dev = indio_dev; + + /* set the device type */ + 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; + + ina2xx_regmap_config.max_register = chip->config->registers; + + 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; + + 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); + + ret = ina2xx_init(chip, val); + if (ret < 0) { + dev_err(&client->dev, "error configuring the device: %d\n", + ret); + return -ENODEV; + } + + return iio_device_register(indio_dev); +} + +static int ina2xx_remove(struct i2c_client *client) +{ + struct iio_dev *indio_dev = i2c_get_clientdata(client); + + iio_device_unregister(indio_dev); + + return 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [RFC 1/4] iio: ina2xx: add direct IO support for TI INA2xx Power Monitors 2015-11-10 16:07 ` [RFC 1/4] iio: ina2xx: add direct IO support for TI INA2xx Power Monitors Marc Titinger @ 2015-11-11 10:14 ` Daniel Baluta 2015-11-12 9:25 ` Marc Titinger 2015-11-11 12:09 ` Daniel Baluta 1 sibling, 1 reply; 20+ messages in thread From: Daniel Baluta @ 2015-11-11 10:14 UTC (permalink / raw) To: Marc Titinger Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald, linux-iio, Linux Kernel Mailing List, mturquette, bcousson, ptitiano On Tue, Nov 10, 2015 at 6:07 PM, Marc Titinger <mtitinger@baylibre.com> wrote: > Basic support or direct IO raw read, with averaging attribute. > Values are RAW, INT_PLUS_MICRO (Volt/Ampere/Watt). > > IIO context has 1 devices: Is this from libiio right? Perhaps you should specify this: "libiio context has 1 devices" > 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 > > Link to datasheet? > Signed-off-by: Marc Titinger <mtitinger@baylibre.com> > --- > drivers/iio/adc/Kconfig | 9 + > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/ina2xx-iio.c | 404 +++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 414 insertions(+) > create mode 100644 drivers/iio/adc/ina2xx-iio.c > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index eb0cd89..b648051 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -10,6 +10,15 @@ config AD_SIGMA_DELTA > select IIO_BUFFER > select IIO_TRIGGERED_BUFFER > > +config INA2XX_IIO > + tristate "Texas Instruments INA2xx Power Monitors IIO driver" > + depends on I2C Since you are using regmap you should select it here. > + help > + Say yes here to build support for TI INA2xx familly Power Monitors. > + > + Note that this is not the existing hwmon interface for this device. > + > + Config symbol should be in alphabetical order. > config AD7266 > tristate "Analog Devices AD7265/AD7266 ADC driver" > depends on SPI_MASTER > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > index a096210..e6a844a 100644 > --- a/drivers/iio/adc/Makefile > +++ b/drivers/iio/adc/Makefile > @@ -5,6 +5,7 @@ > # When adding new entries keep the list in alphabetical order > obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o > obj-$(CONFIG_AD7266) += ad7266.o > +obj-$(CONFIG_INA2XX_IIO) += ina2xx-iio.o The same here. > obj-$(CONFIG_AD7291) += ad7291.o > obj-$(CONFIG_AD7298) += ad7298.o > obj-$(CONFIG_AD7923) += ad7923.o > diff --git a/drivers/iio/adc/ina2xx-iio.c b/drivers/iio/adc/ina2xx-iio.c > new file mode 100644 > index 0000000..257d8d5 > --- /dev/null > +++ b/drivers/iio/adc/ina2xx-iio.c > @@ -0,0 +1,404 @@ > +/* > + * 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. If you know the I2C address its recommended to mention it here. > + */ > +#include <linux/module.h> > +#include <linux/iio/iio.h> > +#include <linux/i2c.h> > +#include <linux/regmap.h> > +#include <linux/platform_data/ina2xx.h> > + > +#include <linux/util_macros.h> Nice, didn't know about this :). > + > +/* > + * INA2XX registers definition > + */ > +/* common register definitions */ > +#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 > + > +/* register count */ > +#define INA219_REGISTERS 6 > +#define INA226_REGISTERS 8 > +#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 INA2XX_RSHUNT_DEFAULT 10000 > + > +/* bit mask for reading the averaging setting in the configuration register */ > +#define INA226_AVG_RD_MASK 0x0E00 > +#define INA226_READ_AVG(reg) (((reg) & INA226_AVG_RD_MASK) >> 9) > +#define INA226_SHIFT_AVG(val) ((val) << 9) > + > +static struct regmap_config ina2xx_regmap_config = { > + .reg_bits = 8, > + .val_bits = 16, Specify here which registers are cacheable, read/write. > +}; > + > +enum ina2xx_ids { ina219, ina226 }; > + > +struct ina2xx_config { > + u16 config_default; > + int calibration_factor; > + int registers; > + int shunt_div; > + int bus_voltage_shift; > + int bus_voltage_lsb; /* uV */ > + int power_lsb; /* uW */ > +}; > + > +struct ina2xx_chip_info { > + struct iio_dev *indio_dev; > + const struct ina2xx_config *config; > + struct mutex state_lock; > + long rshunt; > + int avg; > + int freq; > + int period_us; > + struct regmap *regmap; > +}; > + > +static const struct ina2xx_config ina2xx_config[] = { > + [ina219] = { > + .config_default = INA219_CONFIG_DEFAULT, > + .calibration_factor = 40960000, > + .registers = INA219_REGISTERS, > + .shunt_div = 100, > + .bus_voltage_shift = 3, > + .bus_voltage_lsb = 4000, > + .power_lsb = 20000, > + }, > + [ina226] = { > + .config_default = INA226_CONFIG_DEFAULT, > + .calibration_factor = 5120000, > + .registers = INA226_REGISTERS, > + .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); > + *val = *uval/1000000; > + *uval = *uval - *val*1000000; > + 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 - *val*1000000; > + return IIO_VAL_INT_PLUS_MICRO; > + > + case INA2XX_POWER: > + *uval = regval * chip->config->power_lsb; > + *val = *uval/1000000; > + *uval = *uval - *val*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; > + > + case INA2XX_CALIBRATION: > + *val = DIV_ROUND_CLOSEST(chip->config->calibration_factor, > + regval); > + return IIO_VAL_INT; > + > + 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, ®val); > + if (ret < 0) > + return ret; > + > + return ina2xx_get_value(chip, chan->address, regval, val, val2); > + > + case IIO_CHAN_INFO_AVERAGE_RAW: > + *val = chip->avg; > + return IIO_VAL_INT; > + > + case IIO_CHAN_INFO_CALIBSCALE: > + ret = regmap_read(chip->regmap, INA2XX_CALIBRATION, ®val); > + if (ret < 0) > + return ret; > + > + return ina2xx_get_value(chip, INA2XX_CALIBRATION, regval, > + val, val2); > + > + default: > + return -EINVAL; > + } > + > + return 0; > +} > + > +static int ina2xx_calibrate(struct ina2xx_chip_info *chip) > +{ > + u16 val = DIV_ROUND_CLOSEST(chip->config->calibration_factor, > + chip->rshunt); > + > + return regmap_write(chip->regmap, INA2XX_CALIBRATION, val); > +} > + > + > +/* > + * 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 unsigned 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_RD_MASK; > + *config |= INA226_SHIFT_AVG(bits) & INA226_AVG_RD_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 = 0; > + unsigned int config, tmp; > + > + 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_AVERAGE_RAW: > + > + ret = ina226_set_average(chip, val, &tmp); > + break; > + > + default: > + ret = -EINVAL; > + } > + > + if (!ret && (tmp != config)) > + ret = regmap_write(chip->regmap, INA2XX_CONFIG, config); > +_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_AVERAGE_RAW) | \ > + BIT(IIO_CHAN_INFO_CALIBSCALE), \ > + .scan_index = (_index), \ > + .scan_type = { \ > + .sign = 'u', \ > + .realbits = 16, \ > + .storagebits = 16, \ > + .shift = 0, \ > + .endianness = IIO_BE, \ > + } \ > +} > + > +static const struct iio_chan_spec ina2xx_channels[] = { > + INA2XX_CHAN(IIO_VOLTAGE, 0, INA2XX_SHUNT_VOLTAGE), > + INA2XX_CHAN(IIO_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 const struct iio_info ina2xx_info = { > + .read_raw = &ina2xx_read_raw, > + .write_raw = &ina2xx_write_raw, > + .driver_module = THIS_MODULE, > +}; > + > +/* > +* Initialize the configuration and calibration registers. > +*/ > +static int ina2xx_init(struct ina2xx_chip_info *chip, unsigned int config) > +{ > + 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). > + */ > + return ina2xx_calibrate(chip); > +} > + > +static int ina2xx_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct ina2xx_chip_info *chip; > + struct iio_dev *indio_dev; > + 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->indio_dev = indio_dev; > + > + /* set the device type */ > + 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; > + > + ina2xx_regmap_config.max_register = chip->config->registers; > + > + 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; > + > + 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); > + > + ret = ina2xx_init(chip, val); > + if (ret < 0) { > + dev_err(&client->dev, "error configuring the device: %d\n", > + ret); > + return -ENODEV; > + } > + > + return iio_device_register(indio_dev); If there is no reverse operation for ina2xx_init (e.g ina2xx_poweroff) then here you can use devm_iio_device_register and completely remove ina2xx_remove function. > +} > + > +static int ina2xx_remove(struct i2c_client *client) > +{ > + struct iio_dev *indio_dev = i2c_get_clientdata(client); > + > + iio_device_unregister(indio_dev); > + > + return 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"); ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC 1/4] iio: ina2xx: add direct IO support for TI INA2xx Power Monitors 2015-11-11 10:14 ` Daniel Baluta @ 2015-11-12 9:25 ` Marc Titinger 0 siblings, 0 replies; 20+ messages in thread From: Marc Titinger @ 2015-11-12 9:25 UTC (permalink / raw) To: Daniel Baluta Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald, linux-iio, Linux Kernel Mailing List, mturquette, bcousson, ptitiano On 11/11/2015 11:14, Daniel Baluta wrote: > On Tue, Nov 10, 2015 at 6:07 PM, Marc Titinger <mtitinger@baylibre.com> wrote: >> Basic support or direct IO raw read, with averaging attribute. >> Values are RAW, INT_PLUS_MICRO (Volt/Ampere/Watt). >> Hi Daniel, thanks for the review, comments bellow, Marc. >> IIO context has 1 devices: > Is this from libiio right? Perhaps you should specify this: > > "libiio context has 1 devices" It's the raw output from iio_info to give a quick overview of the features. > >> 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 >> >> > > Link to datasheet? Ok, will add it. http://www.ti.com/lit/gpn/ina226 ... >> >> +config INA2XX_IIO >> + tristate "Texas Instruments INA2xx Power Monitors IIO driver" >> + depends on I2C > > Since you are using regmap you should select it here. Right. > >> + help >> + Say yes here to build support for TI INA2xx familly Power Monitors. >> + >> + Note that this is not the existing hwmon interface for this device. >> + >> + > > Config symbol should be in alphabetical order. Ok > ... >> obj-$(CONFIG_AD7266) += ad7266.o >> +obj-$(CONFIG_INA2XX_IIO) += ina2xx-iio.o > > The same here. Ok. > >> obj-$(CONFIG_AD7291) += ad7291.o >> obj-$(CONFIG_AD7298) += ad7298.o >> obj-$(CONFIG_AD7923) += ad7923.o >> diff --git a/drivers/iio/adc/ina2xx-iio.c b/drivers/iio/adc/ina2xx-iio.c >> new file mode 100644 >> index 0000000..257d8d5 >> --- /dev/null >> +++ b/drivers/iio/adc/ina2xx-iio.c >> @@ -0,0 +1,404 @@ >> +/* >> + * INA2XX Current and Power Monitors >> + * ... >> + * >> + * Licensed under the GPL-2 or later. > > If you know the I2C address its recommended to mention it here. ok. ... >> + >> +static struct regmap_config ina2xx_regmap_config = { >> + .reg_bits = 8, >> + .val_bits = 16, > Specify here which registers are cacheable, read/write. ok. ... >> + >> + return iio_device_register(indio_dev); > > If there is no reverse operation for ina2xx_init (e.g ina2xx_poweroff) then here > you can use devm_iio_device_register and completely remove > ina2xx_remove function. right, thanks! ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC 1/4] iio: ina2xx: add direct IO support for TI INA2xx Power Monitors 2015-11-10 16:07 ` [RFC 1/4] iio: ina2xx: add direct IO support for TI INA2xx Power Monitors Marc Titinger 2015-11-11 10:14 ` Daniel Baluta @ 2015-11-11 12:09 ` Daniel Baluta 2015-11-12 9:38 ` Marc Titinger ` (2 more replies) 1 sibling, 3 replies; 20+ messages in thread From: Daniel Baluta @ 2015-11-11 12:09 UTC (permalink / raw) To: Marc Titinger Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald, linux-iio, Linux Kernel Mailing List, mturquette, bcousson, ptitiano <snip> > Signed-off-by: Marc Titinger <mtitinger@baylibre.com> > --- > drivers/iio/adc/Kconfig | 9 + > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/ina2xx-iio.c | 404 +++++++++++++++++++++++++++++++++++++++++++ One more thing. In IIO we do not prefer generic names for files. Lets name this after the first device in the family. Here I would say we should call it drivers/iio/adc/ina219.c. Skip the -iio suffix from file name, because it is already under the iio/ directory. If there is a module name conflict with hwmon module we can name the .ko resulted from this file ina219-iio.ko. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC 1/4] iio: ina2xx: add direct IO support for TI INA2xx Power Monitors 2015-11-11 12:09 ` Daniel Baluta @ 2015-11-12 9:38 ` Marc Titinger 2015-11-12 12:57 ` [RFC v2 1/2] " Marc Titinger 2015-11-12 12:57 ` [RFC v2 2/2] iio: ina2xx: add SOFTWARE buffer mode using an iio kfifo Marc Titinger 2 siblings, 0 replies; 20+ messages in thread From: Marc Titinger @ 2015-11-12 9:38 UTC (permalink / raw) To: Daniel Baluta Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald, linux-iio, Linux Kernel Mailing List, mturquette, bcousson, ptitiano On 11/11/2015 13:09, Daniel Baluta wrote: > <snip> > >> Signed-off-by: Marc Titinger <mtitinger@baylibre.com> >> --- >> drivers/iio/adc/Kconfig | 9 + >> drivers/iio/adc/Makefile | 1 + >> drivers/iio/adc/ina2xx-iio.c | 404 +++++++++++++++++++++++++++++++++++++++++++ > > One more thing. In IIO we do not prefer generic names for files. > Lets name this after the first device in the family. > > Here I would say we should call it drivers/iio/adc/ina219.c. Skip the > -iio suffix > from file name, because it is already under the iio/ directory. I get your point, but in this specific case there is already support for this chip family in hwmon and both drivers make sense because both applications exist (hardware monitoring or power measurement equipement). To avoid confusion it may be good to have at least a common radix for the CONFIG option, so that searching returns both options. > > If there is a module name conflict with hwmon module we can name the > .ko resulted > from this file ina219-iio.ko. > ^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC v2 1/2] iio: ina2xx: add direct IO support for TI INA2xx Power Monitors 2015-11-11 12:09 ` Daniel Baluta 2015-11-12 9:38 ` Marc Titinger @ 2015-11-12 12:57 ` Marc Titinger 2015-11-14 18:59 ` Jonathan Cameron 2015-11-12 12:57 ` [RFC v2 2/2] iio: ina2xx: add SOFTWARE buffer mode using an iio kfifo Marc Titinger 2 siblings, 1 reply; 20+ messages in thread From: Marc Titinger @ 2015-11-12 12:57 UTC (permalink / raw) To: jic23, knaack.h, lars, pmeerw Cc: linux-iio, linux-kernel, mturquette, bcousson, ptitiano, Marc Titinger Basic support or direct IO raw read, with averaging attribute. 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@baylibre.com> --- v2 of series https://lkml.org/lkml/2015/11/10/370 : - squash patches adding SAMPL_FREQ and debugfs interface - add regmap is_volatile and is_writeable callbacks - fix Kconfig deps and alphabetical sorting - simplification: use devm_iio_device_register --- drivers/iio/adc/Kconfig | 9 + drivers/iio/adc/Makefile | 1 + drivers/iio/adc/ina2xx-iio.c | 472 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 482 insertions(+) create mode 100644 drivers/iio/adc/ina2xx-iio.c diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index eb0cd89..087e5bd 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -170,6 +170,15 @@ config EXYNOS_ADC of SoCs for drivers such as the touchscreen and hwmon to use to share this resource. +config INA2XX_IIO + tristate "Texas Instruments INA2xx Power Monitors IIO driver" + depends on I2C + select REGMAP_I2C + help + Say yes here to build support for TI INA2xx familly Power Monitors. + + Note that this is not the existing hwmon interface for this device. + 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..74e4341 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_IIO) += ina2xx-iio.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-iio.c b/drivers/iio/adc/ina2xx-iio.c new file mode 100644 index 0000000..28b7919 --- /dev/null +++ b/drivers/iio/adc/ina2xx-iio.c @@ -0,0 +1,472 @@ +/* + * 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/iio/iio.h> +#include <linux/i2c.h> +#include <linux/regmap.h> +#include <linux/platform_data/ina2xx.h> + +#include <linux/util_macros.h> + +/* + * INA2XX registers definition + */ +/* common register definitions */ +#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 + +/* register count */ +#define INA219_REGISTERS 6 +#define INA226_REGISTERS 8 +#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_FREQ 454 + +#define INA2XX_RSHUNT_DEFAULT 10000 + +/* bit mask for reading the averaging setting in the configuration register */ +#define INA226_AVG_RD_MASK 0x0E00 +#define INA226_READ_AVG(reg) (((reg) & INA226_AVG_RD_MASK) >> 9) +#define INA226_SHIFT_AVG(val) ((val) << 9) + +#define INA226_SFREQ_RD_MASK 0x01f8 + + +static bool ina2xx_is_writeable_reg(struct device *dev, unsigned int reg) +{ + return (reg == INA2XX_CONFIG) + || (reg == INA2XX_CALIBRATION); +} + +static bool ina2xx_is_volatile_reg(struct device *dev, unsigned int reg) +{ + return (reg != INA2XX_CONFIG); +} + +static struct regmap_config ina2xx_regmap_config = { + .reg_bits = 8, + .val_bits = 16, + + .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 registers; + int shunt_div; + int bus_voltage_shift; + int bus_voltage_lsb; /* uV */ + int power_lsb; /* uW */ +}; + +struct ina2xx_chip_info { + struct iio_dev *indio_dev; + const struct ina2xx_config *config; + struct mutex state_lock; + long rshunt; + int avg; + int freq; + int period_us; + struct regmap *regmap; +}; + +static const struct ina2xx_config ina2xx_config[] = { + [ina219] = { + .config_default = INA219_CONFIG_DEFAULT, + .calibration_factor = 40960000, + .registers = INA219_REGISTERS, + .shunt_div = 100, + .bus_voltage_shift = 3, + .bus_voltage_lsb = 4000, + .power_lsb = 20000, + }, + [ina226] = { + .config_default = INA226_CONFIG_DEFAULT, + .calibration_factor = 5120000, + .registers = INA226_REGISTERS, + .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); + *val = *uval/1000000; + *uval = *uval - *val*1000000; + 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 - *val*1000000; + return IIO_VAL_INT_PLUS_MICRO; + + case INA2XX_POWER: + *uval = regval * chip->config->power_lsb; + *val = *uval/1000000; + *uval = *uval - *val*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; + + case INA2XX_CALIBRATION: + *val = DIV_ROUND_CLOSEST(chip->config->calibration_factor, + regval); + return IIO_VAL_INT; + + 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, ®val); + if (ret < 0) + return ret; + + return ina2xx_get_value(chip, chan->address, regval, val, val2); + + case IIO_CHAN_INFO_AVERAGE_RAW: + *val = chip->avg; + return IIO_VAL_INT; + + case IIO_CHAN_INFO_CALIBSCALE: + ret = regmap_read(chip->regmap, INA2XX_CALIBRATION, ®val); + if (ret < 0) + return ret; + + return ina2xx_get_value(chip, INA2XX_CALIBRATION, regval, + val, val2); + + case IIO_CHAN_INFO_SAMP_FREQ: + *val = chip->freq; + return IIO_VAL_INT; + + default: + return -EINVAL; + } + + return 0; +} + +static int ina2xx_calibrate(struct ina2xx_chip_info *chip) +{ + u16 val = DIV_ROUND_CLOSEST(chip->config->calibration_factor, + chip->rshunt); + + return regmap_write(chip->regmap, INA2XX_CALIBRATION, val); +} + + +/* + * 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 unsigned 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_RD_MASK; + *config |= INA226_SHIFT_AVG(bits) & INA226_AVG_RD_MASK; + + return 0; +} + +/* + * Conversion times in uS + */ +static const int ina226_conv_time_tab[] = { 140, 204, 332, 588, 1100, + 2116, 4156, 8244}; + +static unsigned int ina226_set_frequency(struct ina2xx_chip_info *chip, + unsigned int val, + unsigned int *config) +{ + int bits; + + if (val > 3550 || val < 50) + return -EINVAL; + + /* integration time in uS, for both voltage channels */ + val = DIV_ROUND_CLOSEST(1000000, 2 * val); + + bits = find_closest(val, ina226_conv_time_tab, + ARRAY_SIZE(ina226_conv_time_tab)); + + chip->period_us = 2 * ina226_conv_time_tab[bits]; + + chip->freq = DIV_ROUND_CLOSEST(1000000, chip->period_us); + + *config &= ~INA226_SFREQ_RD_MASK; + *config |= (bits << 3) | (bits << 6); + + 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 = 0; + unsigned int config, tmp; + + 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_AVERAGE_RAW: + + ret = ina226_set_average(chip, val, &tmp); + break; + + case IIO_CHAN_INFO_SAMP_FREQ: + + ret = ina226_set_frequency(chip, val, &tmp); + break; + + default: + ret = -EINVAL; + } + + if (!ret && (tmp != config)) + ret = regmap_write(chip->regmap, INA2XX_CONFIG, config); +_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_AVERAGE_RAW) | \ + BIT(IIO_CHAN_INFO_SAMP_FREQ) | \ + BIT(IIO_CHAN_INFO_CALIBSCALE), \ + .scan_index = (_index), \ + .scan_type = { \ + .sign = 'u', \ + .realbits = 16, \ + .storagebits = 16, \ + .shift = 0, \ + .endianness = IIO_BE, \ + } \ +} + +static const struct iio_chan_spec ina2xx_channels[] = { + INA2XX_CHAN(IIO_VOLTAGE, 0, INA2XX_SHUNT_VOLTAGE), + INA2XX_CHAN(IIO_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_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); +} + +static const struct iio_info ina2xx_info = { + .debugfs_reg_access = &ina2xx_debug_reg, + .read_raw = &ina2xx_read_raw, + .write_raw = &ina2xx_write_raw, + .driver_module = THIS_MODULE, +}; + +/* +* Initialize the configuration and calibration registers. +*/ +static int ina2xx_init(struct ina2xx_chip_info *chip, unsigned int config) +{ + 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). + */ + return ina2xx_calibrate(chip); +} + +static int ina2xx_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct ina2xx_chip_info *chip; + struct iio_dev *indio_dev; + 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->indio_dev = indio_dev; + + /* set the device type */ + 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; + + ina2xx_regmap_config.max_register = chip->config->registers; + + 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; + + 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_frequency(chip, INA226_DEFAULT_FREQ, &val); + } + + ret = ina2xx_init(chip, val); + if (ret < 0) { + dev_err(&client->dev, "error configuring the device: %d\n", + ret); + return -ENODEV; + } + + return devm_iio_device_register(&client->dev, indio_dev); +} + +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, + .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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [RFC v2 1/2] iio: ina2xx: add direct IO support for TI INA2xx Power Monitors 2015-11-12 12:57 ` [RFC v2 1/2] " Marc Titinger @ 2015-11-14 18:59 ` Jonathan Cameron 2015-11-16 9:31 ` Marc Titinger 0 siblings, 1 reply; 20+ messages in thread From: Jonathan Cameron @ 2015-11-14 18:59 UTC (permalink / raw) To: Marc Titinger, knaack.h, lars, pmeerw Cc: linux-iio, linux-kernel, mturquette, bcousson, ptitiano On 12/11/15 12:57, Marc Titinger wrote: > Basic support or direct IO raw read, with averaging attribute. > 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@baylibre.com> Hi Marc To express a personal preference, please don't have series of patches as 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 > --- > > v2 of series https://lkml.org/lkml/2015/11/10/370 : > > - squash patches adding SAMPL_FREQ and debugfs interface > - add regmap is_volatile and is_writeable callbacks > - fix Kconfig deps and alphabetical sorting > - simplification: use devm_iio_device_register > > --- > > drivers/iio/adc/Kconfig | 9 + > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/ina2xx-iio.c | 472 +++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 482 insertions(+) > create mode 100644 drivers/iio/adc/ina2xx-iio.c > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index eb0cd89..087e5bd 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -170,6 +170,15 @@ config EXYNOS_ADC > of SoCs for drivers such as the touchscreen and hwmon to use to share > this resource. > > +config INA2XX_IIO > + tristate "Texas Instruments INA2xx Power Monitors IIO driver" > + depends on I2C > + select REGMAP_I2C > + help > + Say yes here to build support for TI INA2xx familly Power Monitors. > + > + Note that this is not the existing hwmon interface for this device. > + > 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..74e4341 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_IIO) += ina2xx-iio.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-iio.c b/drivers/iio/adc/ina2xx-iio.c > new file mode 100644 > index 0000000..28b7919 > --- /dev/null > +++ b/drivers/iio/adc/ina2xx-iio.c > @@ -0,0 +1,472 @@ > +/* > + * 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/iio/iio.h> > +#include <linux/i2c.h> > +#include <linux/regmap.h> > +#include <linux/platform_data/ina2xx.h> > + > +#include <linux/util_macros.h> > + > +/* > + * INA2XX registers definition > + */ > +/* common register definitions */ > +#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 > + > +/* register count */ > +#define INA219_REGISTERS 6 > +#define INA226_REGISTERS 8 > +#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_FREQ 454 > + > +#define INA2XX_RSHUNT_DEFAULT 10000 > + > +/* bit mask for reading the averaging setting in the configuration register */ > +#define INA226_AVG_RD_MASK 0x0E00 genmask is always good for these. > +#define INA226_READ_AVG(reg) (((reg) & INA226_AVG_RD_MASK) >> 9) > +#define INA226_SHIFT_AVG(val) ((val) << 9) > + > +#define INA226_SFREQ_RD_MASK 0x01f8 > + > + > +static bool ina2xx_is_writeable_reg(struct device *dev, unsigned int reg) > +{ > + return (reg == INA2XX_CONFIG) > + || (reg == INA2XX_CALIBRATION); On one line I think this is still way less than 80 chars.. > +} > + > +static bool ina2xx_is_volatile_reg(struct device *dev, unsigned int reg) > +{ > + return (reg != INA2XX_CONFIG); > +} > + > +static struct regmap_config ina2xx_regmap_config = { > + .reg_bits = 8, > + .val_bits = 16, > + > + .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 registers; > + int shunt_div; > + int bus_voltage_shift; > + int bus_voltage_lsb; /* uV */ > + int power_lsb; /* uW */ > +}; > + > +struct ina2xx_chip_info { > + struct iio_dev *indio_dev; Having a pointer back to the indio_dev is usually a sign of 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. > + const struct ina2xx_config *config; > + struct mutex state_lock; > + long rshunt; > + int avg; > + int freq; > + int period_us; > + struct regmap *regmap; > +}; > + > +static const struct ina2xx_config ina2xx_config[] = { > + [ina219] = { > + .config_default = INA219_CONFIG_DEFAULT, > + .calibration_factor = 40960000, > + .registers = INA219_REGISTERS, > + .shunt_div = 100, > + .bus_voltage_shift = 3, > + .bus_voltage_lsb = 4000, > + .power_lsb = 20000, > + }, > + [ina226] = { > + .config_default = INA226_CONFIG_DEFAULT, > + .calibration_factor = 5120000, > + .registers = INA226_REGISTERS, > + .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); > + *val = *uval/1000000; > + *uval = *uval - *val*1000000; > + return IIO_VAL_INT_PLUS_MICRO; I'm somewhat unconvinced that this wrapper is adding anything over 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; > + > + case INA2XX_POWER: > + *uval = regval * chip->config->power_lsb; > + *val = *uval/1000000; > + *uval = *uval - *val*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; > + > + case INA2XX_CALIBRATION: > + *val = DIV_ROUND_CLOSEST(chip->config->calibration_factor, > + regval); > + return IIO_VAL_INT; > + > + 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, ®val); > + if (ret < 0) > + return ret; > + > + return ina2xx_get_value(chip, chan->address, regval, val, val2); > + > + case IIO_CHAN_INFO_AVERAGE_RAW: > + *val = chip->avg; > + return IIO_VAL_INT; > + > + case IIO_CHAN_INFO_CALIBSCALE: > + ret = regmap_read(chip->regmap, INA2XX_CALIBRATION, ®val); > + if (ret < 0) > + return ret; > + > + return ina2xx_get_value(chip, INA2XX_CALIBRATION, regval, > + val, val2); > + > + case IIO_CHAN_INFO_SAMP_FREQ: > + *val = chip->freq; > + return IIO_VAL_INT; > + > + default: > + return -EINVAL; > + } > + > + return 0; > +} > + > +static int ina2xx_calibrate(struct ina2xx_chip_info *chip) > +{ > + u16 val = DIV_ROUND_CLOSEST(chip->config->calibration_factor, > + chip->rshunt); > + > + return regmap_write(chip->regmap, INA2XX_CALIBRATION, val); > +} > + > + > +/* > + * 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 unsigned 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_RD_MASK; > + *config |= INA226_SHIFT_AVG(bits) & INA226_AVG_RD_MASK; > + > + return 0; > +} > + > +/* > + * Conversion times in uS > + */ > +static const int ina226_conv_time_tab[] = { 140, 204, 332, 588, 1100, > + 2116, 4156, 8244}; > + > +static unsigned int ina226_set_frequency(struct ina2xx_chip_info *chip, > + unsigned int val, > + unsigned int *config) > +{ > + int bits; > + > + if (val > 3550 || val < 50) > + return -EINVAL; > + > + /* integration time in uS, for both voltage channels */ > + val = DIV_ROUND_CLOSEST(1000000, 2 * val); > + > + bits = find_closest(val, ina226_conv_time_tab, > + ARRAY_SIZE(ina226_conv_time_tab)); > + > + chip->period_us = 2 * ina226_conv_time_tab[bits]; > + > + chip->freq = DIV_ROUND_CLOSEST(1000000, chip->period_us); > + > + *config &= ~INA226_SFREQ_RD_MASK; > + *config |= (bits << 3) | (bits << 6); > + > + 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 = 0; > + unsigned int config, tmp; > + > + 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_AVERAGE_RAW: > + > + ret = ina226_set_average(chip, val, &tmp); This isn't what the ABI uses the info_average_raw attribute for - it's 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! > + break; > + > + case IIO_CHAN_INFO_SAMP_FREQ: > + > + ret = ina226_set_frequency(chip, val, &tmp); > + break; > + > + default: > + ret = -EINVAL; > + } > + > + if (!ret && (tmp != config)) > + ret = regmap_write(chip->regmap, INA2XX_CONFIG, config); > +_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_AVERAGE_RAW) | \ > + BIT(IIO_CHAN_INFO_SAMP_FREQ) | \ > + BIT(IIO_CHAN_INFO_CALIBSCALE), \ > + .scan_index = (_index), \ > + .scan_type = { \ > + .sign = 'u', \ > + .realbits = 16, \ > + .storagebits = 16, \ > + .shift = 0, \ > + .endianness = IIO_BE, \ > + } \ > +} > + > +static const struct iio_chan_spec ina2xx_channels[] = { > + INA2XX_CHAN(IIO_VOLTAGE, 0, INA2XX_SHUNT_VOLTAGE), > + INA2XX_CHAN(IIO_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_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); > +} > + > +static const struct iio_info ina2xx_info = { > + .debugfs_reg_access = &ina2xx_debug_reg, > + .read_raw = &ina2xx_read_raw, > + .write_raw = &ina2xx_write_raw, > + .driver_module = THIS_MODULE, > +}; > + > +/* Single line comment, use single line comment syntax. > +* Initialize the configuration and calibration registers. > +*/ > +static int ina2xx_init(struct ina2xx_chip_info *chip, unsigned int config) > +{ > + 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). > + */ > + return ina2xx_calibrate(chip); > +} > + > +static int ina2xx_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct ina2xx_chip_info *chip; > + struct iio_dev *indio_dev; > + 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->indio_dev = indio_dev; > + > + /* set the device type */ > + 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; > + > + ina2xx_regmap_config.max_register = chip->config->registers; > + > + 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; > + > + 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_frequency(chip, INA226_DEFAULT_FREQ, &val); > + } > + > + ret = ina2xx_init(chip, val); > + if (ret < 0) { > + dev_err(&client->dev, "error configuring the device: %d\n", > + ret); > + return -ENODEV; > + } > + > + return devm_iio_device_register(&client->dev, indio_dev); > +} > + > +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, > + .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"); > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC v2 1/2] iio: ina2xx: add direct IO support for TI INA2xx Power Monitors 2015-11-14 18:59 ` Jonathan Cameron @ 2015-11-16 9:31 ` Marc Titinger 2015-11-16 17:27 ` Jonathan Cameron 0 siblings, 1 reply; 20+ messages in thread From: Marc Titinger @ 2015-11-16 9:31 UTC (permalink / raw) To: Jonathan Cameron, knaack.h, lars, pmeerw Cc: linux-iio, linux-kernel, mturquette, bcousson, ptitiano On 14/11/2015 19:59, Jonathan Cameron wrote: > On 12/11/15 12:57, Marc Titinger wrote: >> Basic support or direct IO raw read, with averaging attribute. >> 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@baylibre.com> > Hi Marc > Hi Jonathan, > To express a personal preference, please don't have series of patches as > replies to the original thread. It gets really hard to follow after > a couple of revisions! > Ok, sorry for that. > May seem a random question, but what do you want to gain from an IIO > driver over what the hwmon provides? Good question. In the frame of Baylibre's ACME power and temperature monitoring demo based on Sigrock, we wish to add a remote mode for the GUI and processing front-end as well as explore other possibilities like using an on-board accelerator to capture the sensor data and stream it back to the front-end. This work is a pathfinder as IIO seems appropriate for what we want to do. > > Various bits inline.. Thanks for the review, further questions below! Marc. > > Mostly looks pretty good though. > > Jonathan >> --- >> ... >> +#define INA2XX_RSHUNT_DEFAULT 10000 >> + >> +/* bit mask for reading the averaging setting in the configuration register */ >> +#define INA226_AVG_RD_MASK 0x0E00 > genmask is always good for these. > ok. .. >> + >> +static bool ina2xx_is_writeable_reg(struct device *dev, unsigned int reg) >> +{ >> + return (reg == INA2XX_CONFIG) >> + || (reg == INA2XX_CALIBRATION); > On one line I think this is still way less than 80 chars.. >> +} ok ... >> +struct ina2xx_chip_info { >> + struct iio_dev *indio_dev; > Having a pointer back to the indio_dev is usually a sign of > 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. > Ah, forgot to remove it when splitting into two patches, but yeah you're right, I shall pass the indio_dev ptr as data in the first place. ... >> + >> +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; > I'm somewhat unconvinced that this wrapper is adding anything over > 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. got it! A practical question: can you specify a divider rather than a multiplier as a scale so that userspace does the division? >> + >> + 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; ... >> + tmp = config; >> + >> + switch (mask) { >> + case IIO_CHAN_INFO_AVERAGE_RAW: >> + >> + ret = ina226_set_average(chip, val, &tmp); > This isn't what the ABI uses the info_average_raw attribute for - it's > 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! > 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 ? ... .driver_module = THIS_MODULE, >> +}; >> + >> +/* > Single line comment, use single line comment syntax. ok ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC v2 1/2] iio: ina2xx: add direct IO support for TI INA2xx Power Monitors 2015-11-16 9:31 ` Marc Titinger @ 2015-11-16 17:27 ` Jonathan Cameron 0 siblings, 0 replies; 20+ messages in thread From: Jonathan Cameron @ 2015-11-16 17:27 UTC (permalink / raw) To: Marc Titinger, Jonathan Cameron, knaack.h, lars, pmeerw Cc: linux-iio, linux-kernel, mturquette, bcousson, ptitiano On 16 November 2015 09:31:17 GMT+00:00, Marc Titinger <mtitinger@baylibre.com> wrote: >On 14/11/2015 19:59, Jonathan Cameron wrote: >> On 12/11/15 12:57, Marc Titinger wrote: >>> Basic support or direct IO raw read, with averaging attribute. >>> 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@baylibre.com> >> Hi Marc >> > >Hi Jonathan, > >> To express a personal preference, please don't have series of patches >as >> replies to the original thread. It gets really hard to follow after >> a couple of revisions! >> >Ok, sorry for that. > >> May seem a random question, but what do you want to gain from an IIO >> driver over what the hwmon provides? > >Good question. In the frame of Baylibre's ACME power and temperature >monitoring demo based on Sigrock, we wish to add a remote mode for the >GUI and processing front-end as well as explore other possibilities >like >using an on-board accelerator to capture the sensor data and stream it >back to the front-end. This work is a pathfinder as IIO seems >appropriate for what we want to do. Fair enough I guess. Will need to check we aren't stepping on too many toes in hwmon if we merge a second driver... > >> >> Various bits inline.. > >Thanks for the review, further questions below! > >Marc. > >> >> Mostly looks pretty good though. >> >> Jonathan >>> --- >>> > >... > >>> +#define INA2XX_RSHUNT_DEFAULT 10000 >>> + >>> +/* bit mask for reading the averaging setting in the configuration >register */ >>> +#define INA226_AVG_RD_MASK 0x0E00 >> genmask is always good for these. >> > >ok. > >.. > >>> + >>> +static bool ina2xx_is_writeable_reg(struct device *dev, unsigned >int reg) >>> +{ >>> + return (reg == INA2XX_CONFIG) >>> + || (reg == INA2XX_CALIBRATION); >> On one line I think this is still way less than 80 chars.. >>> +} > >ok > >... > > >>> +struct ina2xx_chip_info { >>> + struct iio_dev *indio_dev; >> Having a pointer back to the indio_dev is usually a sign of >> 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. >> > >Ah, forgot to remove it when splitting into two patches, but yeah >you're >right, I shall pass the indio_dev ptr as data in the first place. > >... > >>> + >>> +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; >> I'm somewhat unconvinced that this wrapper is adding anything over >> 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. > >got it! A practical question: can you specify a divider rather than a >multiplier as a scale so that userspace does the division? Sort of see IIO_TYPE_FRACTIONAL > >>> + >>> + 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; >... > >>> + tmp = config; >>> + >>> + switch (mask) { >>> + case IIO_CHAN_INFO_AVERAGE_RAW: >>> + >>> + ret = ina226_set_average(chip, val, &tmp); >> This isn't what the ABI uses the info_average_raw attribute for - >it's >> 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! >> >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 ? Go ahead and have a go at defining a suitable ABI :) Post it as a separate RFC though as discussion might be rather in depth and not really driver related. > >... > >.driver_module = THIS_MODULE, >>> +}; >>> + >>> +/* >> Single line comment, use single line comment syntax. > >ok -- Sent from my Android device with K-9 Mail. Please excuse my brevity. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC v2 2/2] iio: ina2xx: add SOFTWARE buffer mode using an iio kfifo. 2015-11-11 12:09 ` Daniel Baluta 2015-11-12 9:38 ` Marc Titinger 2015-11-12 12:57 ` [RFC v2 1/2] " Marc Titinger @ 2015-11-12 12:57 ` Marc Titinger 2 siblings, 0 replies; 20+ messages in thread From: Marc Titinger @ 2015-11-12 12:57 UTC (permalink / raw) To: jic23, knaack.h, lars, pmeerw Cc: linux-iio, linux-kernel, mturquette, bcousson, ptitiano, Marc Titinger Capture the active scan_elements into a kfifo. The capture thread will 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). # iio_readdev ina226 | od -x WARNING: High-speed mode not enabled 0000000 042f 0d5a 002e 010c 4be8 4eb4 0013 0000 0000020 0430 0d5a 002e 010c a704 4f3e 0013 0000 0000040 0430 0d5a 002e 010c b477 4fc7 0013 0000 0000060 042f 0d5b 002e 010c 8052 5050 0013 0000 0000100 042f 0d5b 002e 010c 5d92 50d8 0013 0000 0000120 0430 0d5a 002e 010c fa59 515e 0013 0000 0000140 0430 0d5b 002e 010c 95d2 51e5 0013 0000 Signed-off-by: Marc Titinger <mtitinger@baylibre.com> --- drivers/iio/adc/Kconfig | 1 + drivers/iio/adc/ina2xx-iio.c | 119 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 118 insertions(+), 2 deletions(-) diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index 087e5bd..9b87372 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -174,6 +174,7 @@ config INA2XX_IIO tristate "Texas Instruments INA2xx Power Monitors IIO driver" depends on I2C select REGMAP_I2C + select IIO_BUFFER help Say yes here to build support for TI INA2xx familly Power Monitors. diff --git a/drivers/iio/adc/ina2xx-iio.c b/drivers/iio/adc/ina2xx-iio.c index 28b7919..4c11c80 100644 --- a/drivers/iio/adc/ina2xx-iio.c +++ b/drivers/iio/adc/ina2xx-iio.c @@ -20,7 +20,9 @@ * Configurable 7-bit I2C slave address from 0x40 to 0x4F */ #include <linux/module.h> -#include <linux/iio/iio.h> +#include <linux/kthread.h> +#include <linux/delay.h> +#include <linux/iio/kfifo_buf.h> #include <linux/i2c.h> #include <linux/regmap.h> #include <linux/platform_data/ina2xx.h> @@ -92,6 +94,7 @@ struct ina2xx_config { struct ina2xx_chip_info { struct iio_dev *indio_dev; + struct task_struct *task; const struct ina2xx_config *config; struct mutex state_lock; long rshunt; @@ -282,6 +285,9 @@ static int ina2xx_write_raw(struct iio_dev *indio_dev, int ret = 0; 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); @@ -340,6 +346,106 @@ static const struct iio_chan_spec ina2xx_channels[] = { IIO_CHAN_SOFT_TIMESTAMP(4), }; +/* + * return uS until next due sampling. + */ + +static s64 prev_ns; + +static int ina2xx_work_buffer(struct ina2xx_chip_info *chip) +{ + unsigned short data[8]; + struct iio_dev *indio_dev = chip->indio_dev; + int bit, ret = 0, i = 0; + unsigned long buffer_us = 0, elapsed_us = 0; + s64 time_a, time_b; + + time_a = iio_get_time_ns(); + + /* 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) + goto _err; + + data[i++] = val; + } + + time_b = iio_get_time_ns(); + + iio_push_to_buffers_with_timestamp(chip->indio_dev, + (unsigned int *)data, time_a); + + buffer_us = (unsigned long)(time_b - time_a) / 1000; + elapsed_us = (unsigned long)(time_a - prev_ns) / 1000; + + trace_printk("uS: elapsed: %lu, buf: %lu\n", elapsed_us, buffer_us); + + prev_ns = time_a; +_err: + return buffer_us; +}; + +static int ina2xx_capture_thread(void *data) +{ + struct ina2xx_chip_info *chip = (struct ina2xx_chip_info *)data; + unsigned int sampling_us = chip->period_us * chip->avg; + unsigned long buffer_us; + + do { + buffer_us = ina2xx_work_buffer(chip); + + if (sampling_us > buffer_us) + udelay(sampling_us - buffer_us); + + } while (!kthread_should_stop()); + + chip->task = NULL; + + return 0; +} + +int ina2xx_buffer_enable(struct iio_dev *indio_dev) +{ + struct ina2xx_chip_info *chip = iio_priv(indio_dev); + unsigned int sampling_us = chip->period_us * chip->avg; + + trace_printk("Enabling buffer w/ scan_mask %02x, freq = %d, avg =%u\n", + (unsigned int)(*indio_dev->active_scan_mask), chip->freq, + chip->avg); + + trace_printk("Expected work period is %u us\n", sampling_us); + + prev_ns = iio_get_time_ns(); + + chip->task = kthread_run(ina2xx_capture_thread, (void *)chip, + "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); + + 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) { @@ -379,6 +485,7 @@ static int ina2xx_probe(struct i2c_client *client, { struct ina2xx_chip_info *chip; struct iio_dev *indio_dev; + struct iio_buffer *buffer; int ret; unsigned int val; @@ -421,7 +528,7 @@ static int ina2xx_probe(struct i2c_client *client, indio_dev->dev.parent = &client->dev; indio_dev->info = &ina2xx_info; - indio_dev->modes = INDIO_DIRECT_MODE; + indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE; chip->regmap = devm_regmap_init_i2c(client, &ina2xx_regmap_config); if (IS_ERR(chip->regmap)) { @@ -443,6 +550,14 @@ static int ina2xx_probe(struct i2c_client *client, 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 devm_iio_device_register(&client->dev, indio_dev); } -- 1.9.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [RFC 2/4] iio: ina2xx: add SAMP_FREQ attribute. 2015-11-10 16:07 [RFC 0/4] IIO: add support for INA2xx power monitor Marc Titinger 2015-11-10 16:07 ` [RFC 1/4] iio: ina2xx: add direct IO support for TI INA2xx Power Monitors Marc Titinger @ 2015-11-10 16:07 ` Marc Titinger 2015-11-11 10:17 ` Daniel Baluta 2015-11-10 16:07 ` [RFC 3/4] iio: ina2xx: add debugfs reg access Marc Titinger 2015-11-10 16:07 ` [RFC 4/4] iio: ina2xx: add SOFTWARE buffer mode using an iio kfifo Marc Titinger 3 siblings, 1 reply; 20+ messages in thread From: Marc Titinger @ 2015-11-10 16:07 UTC (permalink / raw) To: jic23, knaack.h, lars, pmeerw Cc: linux-iio, linux-kernel, mturquette, bcousson, ptitiano, Marc Titinger Signed-off-by: Marc Titinger <mtitinger@baylibre.com> --- drivers/iio/adc/ina2xx-iio.c | 51 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/drivers/iio/adc/ina2xx-iio.c b/drivers/iio/adc/ina2xx-iio.c index 257d8d5..92169e1 100644 --- a/drivers/iio/adc/ina2xx-iio.c +++ b/drivers/iio/adc/ina2xx-iio.c @@ -42,7 +42,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_AVG 4 +#define INA226_DEFAULT_FREQ 454 #define INA2XX_RSHUNT_DEFAULT 10000 @@ -51,6 +52,8 @@ #define INA226_READ_AVG(reg) (((reg) & INA226_AVG_RD_MASK) >> 9) #define INA226_SHIFT_AVG(val) ((val) << 9) +#define INA226_SFREQ_RD_MASK 0x01f8 + static struct regmap_config ina2xx_regmap_config = { .reg_bits = 8, .val_bits = 16, @@ -172,6 +175,10 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev, return ina2xx_get_value(chip, INA2XX_CALIBRATION, regval, val, val2); + case IIO_CHAN_INFO_SAMP_FREQ: + *val = chip->freq; + return IIO_VAL_INT; + default: return -EINVAL; } @@ -216,6 +223,38 @@ static unsigned int ina226_set_average(struct ina2xx_chip_info *chip, return 0; } +/* + * Conversion times in uS + */ +static const int ina226_conv_time_tab[] = { 140, 204, 332, 588, 1100, + 2116, 4156, 8244}; + +static unsigned int ina226_set_frequency(struct ina2xx_chip_info *chip, + unsigned int val, + unsigned int *config) +{ + int bits; + + if (val > 3550 || val < 50) + return -EINVAL; + + /* integration time in uS, for both voltage channels */ + val = DIV_ROUND_CLOSEST(1000000, 2 * val); + + bits = find_closest(val, ina226_conv_time_tab, + ARRAY_SIZE(ina226_conv_time_tab)); + + chip->period_us = 2 * ina226_conv_time_tab[bits]; + + chip->freq = DIV_ROUND_CLOSEST(1000000, chip->period_us); + + *config &= ~INA226_SFREQ_RD_MASK; + *config |= (bits << 3) | (bits << 6); + + return 0; +} + + static int ina2xx_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, int val, int val2, long mask) @@ -238,6 +277,11 @@ static int ina2xx_write_raw(struct iio_dev *indio_dev, ret = ina226_set_average(chip, val, &tmp); break; + case IIO_CHAN_INFO_SAMP_FREQ: + + ret = ina226_set_frequency(chip, val, &tmp); + break; + default: ret = -EINVAL; } @@ -257,6 +301,7 @@ _err: .channel = (_index), \ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ .info_mask_shared_by_dir = BIT(IIO_CHAN_INFO_AVERAGE_RAW) | \ + BIT(IIO_CHAN_INFO_SAMP_FREQ) | \ BIT(IIO_CHAN_INFO_CALIBSCALE), \ .scan_index = (_index), \ .scan_type = { \ @@ -355,8 +400,10 @@ static int ina2xx_probe(struct i2c_client *client, /* Patch the current config register with default. */ val = chip->config->config_default; - if (id->driver_data == ina226) + if (id->driver_data == ina226) { ina226_set_average(chip, INA226_DEFAULT_AVG, &val); + ina226_set_frequency(chip, INA226_DEFAULT_FREQ, &val); + } ret = ina2xx_init(chip, val); if (ret < 0) { -- 1.9.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [RFC 2/4] iio: ina2xx: add SAMP_FREQ attribute. 2015-11-10 16:07 ` [RFC 2/4] iio: ina2xx: add SAMP_FREQ attribute Marc Titinger @ 2015-11-11 10:17 ` Daniel Baluta 0 siblings, 0 replies; 20+ messages in thread From: Daniel Baluta @ 2015-11-11 10:17 UTC (permalink / raw) To: Marc Titinger Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald, linux-iio, Linux Kernel Mailing List, mturquette, bcousson, ptitiano On Tue, Nov 10, 2015 at 6:07 PM, Marc Titinger <mtitinger@baylibre.com> wrote: > Signed-off-by: Marc Titinger <mtitinger@baylibre.com> I'm in favor of small incremental patches, anyhow since this is the initial submission I can't see why this patch isn't squashed in the previous one? > --- > drivers/iio/adc/ina2xx-iio.c | 51 ++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 49 insertions(+), 2 deletions(-) > > diff --git a/drivers/iio/adc/ina2xx-iio.c b/drivers/iio/adc/ina2xx-iio.c > index 257d8d5..92169e1 100644 > --- a/drivers/iio/adc/ina2xx-iio.c > +++ b/drivers/iio/adc/ina2xx-iio.c > @@ -42,7 +42,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_AVG 4 > +#define INA226_DEFAULT_FREQ 454 > > #define INA2XX_RSHUNT_DEFAULT 10000 > > @@ -51,6 +52,8 @@ > #define INA226_READ_AVG(reg) (((reg) & INA226_AVG_RD_MASK) >> 9) > #define INA226_SHIFT_AVG(val) ((val) << 9) > > +#define INA226_SFREQ_RD_MASK 0x01f8 > + > static struct regmap_config ina2xx_regmap_config = { > .reg_bits = 8, > .val_bits = 16, > @@ -172,6 +175,10 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev, > return ina2xx_get_value(chip, INA2XX_CALIBRATION, regval, > val, val2); > > + case IIO_CHAN_INFO_SAMP_FREQ: > + *val = chip->freq; > + return IIO_VAL_INT; > + > default: > return -EINVAL; > } > @@ -216,6 +223,38 @@ static unsigned int ina226_set_average(struct ina2xx_chip_info *chip, > return 0; > } > > +/* > + * Conversion times in uS > + */ > +static const int ina226_conv_time_tab[] = { 140, 204, 332, 588, 1100, > + 2116, 4156, 8244}; > + > +static unsigned int ina226_set_frequency(struct ina2xx_chip_info *chip, > + unsigned int val, > + unsigned int *config) > +{ > + int bits; > + > + if (val > 3550 || val < 50) > + return -EINVAL; > + > + /* integration time in uS, for both voltage channels */ > + val = DIV_ROUND_CLOSEST(1000000, 2 * val); > + > + bits = find_closest(val, ina226_conv_time_tab, > + ARRAY_SIZE(ina226_conv_time_tab)); > + > + chip->period_us = 2 * ina226_conv_time_tab[bits]; > + > + chip->freq = DIV_ROUND_CLOSEST(1000000, chip->period_us); > + > + *config &= ~INA226_SFREQ_RD_MASK; > + *config |= (bits << 3) | (bits << 6); > + > + return 0; > +} > + > + > static int ina2xx_write_raw(struct iio_dev *indio_dev, > struct iio_chan_spec const *chan, > int val, int val2, long mask) > @@ -238,6 +277,11 @@ static int ina2xx_write_raw(struct iio_dev *indio_dev, > ret = ina226_set_average(chip, val, &tmp); > break; > > + case IIO_CHAN_INFO_SAMP_FREQ: > + > + ret = ina226_set_frequency(chip, val, &tmp); > + break; > + > default: > ret = -EINVAL; > } > @@ -257,6 +301,7 @@ _err: > .channel = (_index), \ > .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > .info_mask_shared_by_dir = BIT(IIO_CHAN_INFO_AVERAGE_RAW) | \ > + BIT(IIO_CHAN_INFO_SAMP_FREQ) | \ > BIT(IIO_CHAN_INFO_CALIBSCALE), \ > .scan_index = (_index), \ > .scan_type = { \ > @@ -355,8 +400,10 @@ static int ina2xx_probe(struct i2c_client *client, > > /* Patch the current config register with default. */ > val = chip->config->config_default; > - if (id->driver_data == ina226) > + if (id->driver_data == ina226) { > ina226_set_average(chip, INA226_DEFAULT_AVG, &val); > + ina226_set_frequency(chip, INA226_DEFAULT_FREQ, &val); > + } > > ret = ina2xx_init(chip, val); > if (ret < 0) { > -- > 1.9.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC 3/4] iio: ina2xx: add debugfs reg access 2015-11-10 16:07 [RFC 0/4] IIO: add support for INA2xx power monitor Marc Titinger 2015-11-10 16:07 ` [RFC 1/4] iio: ina2xx: add direct IO support for TI INA2xx Power Monitors Marc Titinger 2015-11-10 16:07 ` [RFC 2/4] iio: ina2xx: add SAMP_FREQ attribute Marc Titinger @ 2015-11-10 16:07 ` Marc Titinger 2015-11-10 16:07 ` [RFC 4/4] iio: ina2xx: add SOFTWARE buffer mode using an iio kfifo Marc Titinger 3 siblings, 0 replies; 20+ messages in thread From: Marc Titinger @ 2015-11-10 16:07 UTC (permalink / raw) To: jic23, knaack.h, lars, pmeerw Cc: linux-iio, linux-kernel, mturquette, bcousson, ptitiano, Marc Titinger iio:device0: ina226 ... 3 device-specific attributes found: attr 0: in_calibscale value: 10000 attr 1: in_mean_raw value: 4 attr 2: in_sampling_frequency value: 455 Signed-off-by: Marc Titinger <mtitinger@baylibre.com> --- drivers/iio/adc/ina2xx-iio.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/iio/adc/ina2xx-iio.c b/drivers/iio/adc/ina2xx-iio.c index 92169e1..3e4a4b0 100644 --- a/drivers/iio/adc/ina2xx-iio.c +++ b/drivers/iio/adc/ina2xx-iio.c @@ -321,7 +321,19 @@ static const struct iio_chan_spec ina2xx_channels[] = { IIO_CHAN_SOFT_TIMESTAMP(4), }; +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); +} + static const struct iio_info ina2xx_info = { + .debugfs_reg_access = &ina2xx_debug_reg, .read_raw = &ina2xx_read_raw, .write_raw = &ina2xx_write_raw, .driver_module = THIS_MODULE, -- 1.9.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [RFC 4/4] iio: ina2xx: add SOFTWARE buffer mode using an iio kfifo. 2015-11-10 16:07 [RFC 0/4] IIO: add support for INA2xx power monitor Marc Titinger ` (2 preceding siblings ...) 2015-11-10 16:07 ` [RFC 3/4] iio: ina2xx: add debugfs reg access Marc Titinger @ 2015-11-10 16:07 ` Marc Titinger 2015-11-10 18:23 ` Lars-Peter Clausen 3 siblings, 1 reply; 20+ messages in thread From: Marc Titinger @ 2015-11-10 16:07 UTC (permalink / raw) To: jic23, knaack.h, lars, pmeerw Cc: linux-iio, linux-kernel, mturquette, bcousson, ptitiano, Marc Titinger Capture the active scan_elements into a kfifo. The capture thread will 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). # iio_readdev ina226 | od -x WARNING: High-speed mode not enabled 0000000 042f 0d5a 002e 010c 4be8 4eb4 0013 0000 0000020 0430 0d5a 002e 010c a704 4f3e 0013 0000 0000040 0430 0d5a 002e 010c b477 4fc7 0013 0000 0000060 042f 0d5b 002e 010c 8052 5050 0013 0000 0000100 042f 0d5b 002e 010c 5d92 50d8 0013 0000 0000120 0430 0d5a 002e 010c fa59 515e 0013 0000 0000140 0430 0d5b 002e 010c 95d2 51e5 0013 0000 Signed-off-by: Marc Titinger <mtitinger@baylibre.com> --- Ina2xx does not support auto-increment, hence the capture threads sticks with single register reads instead of regmap_bulk_read. The proper scales must be applied to those raw register values, I'm in favor of doing the conversion in userland in a client plugin for instance a sigrok, of iio-osciloscope plugin, rather than converting in kernel. --- drivers/iio/adc/Kconfig | 2 + drivers/iio/adc/ina2xx-iio.c | 120 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 120 insertions(+), 2 deletions(-) diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index b648051..6ef8ce5 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -13,6 +13,8 @@ config AD_SIGMA_DELTA config INA2XX_IIO tristate "Texas Instruments INA2xx Power Monitors IIO driver" depends on I2C + select IIO_BUFFER + select IIO_KFIFO_BUF help Say yes here to build support for TI INA2xx familly Power Monitors. diff --git a/drivers/iio/adc/ina2xx-iio.c b/drivers/iio/adc/ina2xx-iio.c index 3e4a4b0..132b8a9 100644 --- a/drivers/iio/adc/ina2xx-iio.c +++ b/drivers/iio/adc/ina2xx-iio.c @@ -16,7 +16,10 @@ * Licensed under the GPL-2 or later. */ #include <linux/module.h> -#include <linux/iio/iio.h> +#include <linux/kthread.h> +#include <linux/delay.h> +#include <linux/iio/kfifo_buf.h> + #include <linux/i2c.h> #include <linux/regmap.h> #include <linux/platform_data/ina2xx.h> @@ -73,6 +76,7 @@ struct ina2xx_config { struct ina2xx_chip_info { struct iio_dev *indio_dev; + struct task_struct *task; const struct ina2xx_config *config; struct mutex state_lock; long rshunt; @@ -263,6 +267,9 @@ static int ina2xx_write_raw(struct iio_dev *indio_dev, int ret = 0; 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); @@ -321,6 +328,106 @@ static const struct iio_chan_spec ina2xx_channels[] = { IIO_CHAN_SOFT_TIMESTAMP(4), }; +/* + * return uS until next due sampling. + */ + +static s64 prev_ns; + +static int ina2xx_work_buffer(struct ina2xx_chip_info *chip) +{ + unsigned short data[8]; + struct iio_dev *indio_dev = chip->indio_dev; + int bit, ret = 0, i = 0; + unsigned long buffer_us = 0, elapsed_us = 0; + s64 time_a, time_b; + + time_a = iio_get_time_ns(); + + /* 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) + goto _err; + + data[i++] = val; + } + + time_b = iio_get_time_ns(); + + iio_push_to_buffers_with_timestamp(chip->indio_dev, + (unsigned int *)data, time_a); + + buffer_us = (unsigned long)(time_b - time_a) / 1000; + elapsed_us = (unsigned long)(time_a - prev_ns) / 1000; + + trace_printk("uS: elapsed: %lu, buf: %lu\n", elapsed_us, buffer_us); + + prev_ns = time_a; +_err: + return buffer_us; +}; + +static int ina2xx_capture_thread(void *data) +{ + struct ina2xx_chip_info *chip = (struct ina2xx_chip_info *)data; + unsigned int sampling_us = chip->period_us * chip->avg; + unsigned long buffer_us; + + do { + buffer_us = ina2xx_work_buffer(chip); + + if (sampling_us > buffer_us) + udelay(sampling_us - buffer_us); + + } while (!kthread_should_stop()); + + chip->task = NULL; + + return 0; +} + +int ina2xx_buffer_enable(struct iio_dev *indio_dev) +{ + struct ina2xx_chip_info *chip = iio_priv(indio_dev); + unsigned int sampling_us = chip->period_us * chip->avg; + + trace_printk("Enabling buffer w/ scan_mask %02x, freq = %d, avg =%u\n", + (unsigned int)(*indio_dev->active_scan_mask), chip->freq, + chip->avg); + + trace_printk("Expected work period is %u us\n", sampling_us); + + prev_ns = iio_get_time_ns(); + + chip->task = kthread_run(ina2xx_capture_thread, (void *)chip, + "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); + + 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) { @@ -360,6 +467,7 @@ static int ina2xx_probe(struct i2c_client *client, { struct ina2xx_chip_info *chip; struct iio_dev *indio_dev; + struct iio_buffer *buffer; int ret; unsigned int val; @@ -402,7 +510,7 @@ static int ina2xx_probe(struct i2c_client *client, indio_dev->dev.parent = &client->dev; indio_dev->info = &ina2xx_info; - indio_dev->modes = INDIO_DIRECT_MODE; + indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE; chip->regmap = devm_regmap_init_i2c(client, &ina2xx_regmap_config); if (IS_ERR(chip->regmap)) { @@ -424,6 +532,14 @@ static int ina2xx_probe(struct i2c_client *client, 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); } -- 1.9.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [RFC 4/4] iio: ina2xx: add SOFTWARE buffer mode using an iio kfifo. 2015-11-10 16:07 ` [RFC 4/4] iio: ina2xx: add SOFTWARE buffer mode using an iio kfifo Marc Titinger @ 2015-11-10 18:23 ` Lars-Peter Clausen 2015-11-12 10:18 ` Marc Titinger 0 siblings, 1 reply; 20+ messages in thread From: Lars-Peter Clausen @ 2015-11-10 18:23 UTC (permalink / raw) To: Marc Titinger, jic23, knaack.h, pmeerw Cc: linux-iio, linux-kernel, mturquette, bcousson, ptitiano On 11/10/2015 05:07 PM, Marc Titinger wrote: > Capture the active scan_elements into a kfifo. > The capture thread will 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). > > # iio_readdev ina226 | od -x > WARNING: High-speed mode not enabled > 0000000 042f 0d5a 002e 010c 4be8 4eb4 0013 0000 > 0000020 0430 0d5a 002e 010c a704 4f3e 0013 0000 > 0000040 0430 0d5a 002e 010c b477 4fc7 0013 0000 > 0000060 042f 0d5b 002e 010c 8052 5050 0013 0000 > 0000100 042f 0d5b 002e 010c 5d92 50d8 0013 0000 > 0000120 0430 0d5a 002e 010c fa59 515e 0013 0000 > 0000140 0430 0d5b 002e 010c 95d2 51e5 0013 0000 > > Signed-off-by: Marc Titinger <mtitinger@baylibre.com> Interesting approach. I think if we are going to due this we want to make this kind of emulation generic. Have you seen the software trigger and configfs support patches[1] from Daniel? It kind of achieves the same as you do, but using hrtimers. > > --- > Ina2xx does not support auto-increment, hence the capture threads sticks > with single register reads instead of regmap_bulk_read. > > The proper scales must be applied to those raw register > values, I'm in favor of doing the conversion in userland in a client plugin Yes, conversion should not be done in kernel space, we don't want to impose the performance penalty on users which don't need it and you can typically do it faster in userspace anyway where you have floats and SSE and what not. > for instance a sigrok Slightly OT, but do you already have some Sigrok IIO support? I have this scheduled for end of the month, maybe we can align our strategies here and avoid duplicated work. - Lars [1] https://lkml.org/lkml/2015/8/10/877 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC 4/4] iio: ina2xx: add SOFTWARE buffer mode using an iio kfifo. 2015-11-10 18:23 ` Lars-Peter Clausen @ 2015-11-12 10:18 ` Marc Titinger 2015-11-12 10:20 ` Lars-Peter Clausen 2015-11-14 18:44 ` Jonathan Cameron 0 siblings, 2 replies; 20+ messages in thread From: Marc Titinger @ 2015-11-12 10:18 UTC (permalink / raw) To: Lars-Peter Clausen, jic23, knaack.h, pmeerw Cc: linux-iio, linux-kernel, mturquette, bcousson, ptitiano On 10/11/2015 19:23, Lars-Peter Clausen wrote: > On 11/10/2015 05:07 PM, Marc Titinger wrote: >> Capture the active scan_elements into a kfifo. >> The capture thread will 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). >> >> # iio_readdev ina226 | od -x >> WARNING: High-speed mode not enabled >> 0000000 042f 0d5a 002e 010c 4be8 4eb4 0013 0000 >> 0000020 0430 0d5a 002e 010c a704 4f3e 0013 0000 >> 0000040 0430 0d5a 002e 010c b477 4fc7 0013 0000 >> 0000060 042f 0d5b 002e 010c 8052 5050 0013 0000 >> 0000100 042f 0d5b 002e 010c 5d92 50d8 0013 0000 >> 0000120 0430 0d5a 002e 010c fa59 515e 0013 0000 >> 0000140 0430 0d5b 002e 010c 95d2 51e5 0013 0000 >> >> Signed-off-by: Marc Titinger <mtitinger@baylibre.com> Hi Lars, > > Interesting approach. I think if we are going to due this we want to make > this kind of emulation generic. Have you seen the software trigger and > configfs support patches[1] from Daniel? It kind of achieves the same as you > do, but using hrtimers. I totally agree, let me have a look on those patches maybe I could add an active waiting scheme for platforms w/o hrtimers ? > >> >> --- >> Ina2xx does not support auto-increment, hence the capture threads sticks >> with single register reads instead of regmap_bulk_read. >> >> The proper scales must be applied to those raw register >> values, I'm in favor of doing the conversion in userland in a client plugin > > Yes, conversion should not be done in kernel space, we don't want to impose > the performance penalty on users which don't need it and you can typically > do it faster in userspace anyway where you have floats and SSE and what not. > >> for instance a sigrok > > Slightly OT, but do you already have some Sigrok IIO support? I have this > scheduled for end of the month, maybe we can align our strategies here and > avoid duplicated work. How fortunate! I've started some preliminary work like cloning the demo driver into a skeletton for 'hardware/generic-iio/api.c', adding the build/ac plumbing, and linking to libiio with the idea of using iio_info to create a generic enumeration of the iio-context into sigrok channels. Now, I'm not familiar with Glib and it might not be my prio until a couple of weeks, so I'd be super happy to wait for you if you are keen to do that part :) What would be the best spot to chat about this ? Marc. > > - Lars > > [1] https://lkml.org/lkml/2015/8/10/877 > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC 4/4] iio: ina2xx: add SOFTWARE buffer mode using an iio kfifo. 2015-11-12 10:18 ` Marc Titinger @ 2015-11-12 10:20 ` Lars-Peter Clausen 2015-11-14 18:44 ` Jonathan Cameron 1 sibling, 0 replies; 20+ messages in thread From: Lars-Peter Clausen @ 2015-11-12 10:20 UTC (permalink / raw) To: Marc Titinger, jic23, knaack.h, pmeerw Cc: linux-iio, linux-kernel, mturquette, bcousson, ptitiano On 11/12/2015 11:18 AM, Marc Titinger wrote: [...] >> Slightly OT, but do you already have some Sigrok IIO support? I have this >> scheduled for end of the month, maybe we can align our strategies here and >> avoid duplicated work. > > How fortunate! I've started some preliminary work like cloning the demo > driver into a skeletton for 'hardware/generic-iio/api.c', adding the > build/ac plumbing, and linking to libiio with the idea of using iio_info to > create a generic enumeration of the iio-context into sigrok channels. > > Now, I'm not familiar with Glib and it might not be my prio until a couple > of weeks, so I'd be super happy to wait for you if you are keen to do that > part :) > > What would be the best spot to chat about this ? #sigrok on irc.freenode.net ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC 4/4] iio: ina2xx: add SOFTWARE buffer mode using an iio kfifo. 2015-11-12 10:18 ` Marc Titinger 2015-11-12 10:20 ` Lars-Peter Clausen @ 2015-11-14 18:44 ` Jonathan Cameron 2015-11-16 9:37 ` Marc Titinger 1 sibling, 1 reply; 20+ messages in thread From: Jonathan Cameron @ 2015-11-14 18:44 UTC (permalink / raw) To: Marc Titinger, Lars-Peter Clausen, knaack.h, pmeerw Cc: linux-iio, linux-kernel, mturquette, bcousson, ptitiano On 12/11/15 10:18, Marc Titinger wrote: > On 10/11/2015 19:23, Lars-Peter Clausen wrote: >> On 11/10/2015 05:07 PM, Marc Titinger wrote: >>> Capture the active scan_elements into a kfifo. >>> The capture thread will 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). >>> >>> # iio_readdev ina226 | od -x >>> WARNING: High-speed mode not enabled >>> 0000000 042f 0d5a 002e 010c 4be8 4eb4 0013 0000 >>> 0000020 0430 0d5a 002e 010c a704 4f3e 0013 0000 >>> 0000040 0430 0d5a 002e 010c b477 4fc7 0013 0000 >>> 0000060 042f 0d5b 002e 010c 8052 5050 0013 0000 >>> 0000100 042f 0d5b 002e 010c 5d92 50d8 0013 0000 >>> 0000120 0430 0d5a 002e 010c fa59 515e 0013 0000 >>> 0000140 0430 0d5b 002e 010c 95d2 51e5 0013 0000 >>> >>> Signed-off-by: Marc Titinger <mtitinger@baylibre.com> > > Hi Lars, > >> >> Interesting approach. I think if we are going to due this we want to make >> this kind of emulation generic. Have you seen the software trigger and >> configfs support patches[1] from Daniel? It kind of achieves the same as you >> do, but using hrtimers. > > I totally agree, let me have a look on those patches maybe I could > add an active waiting scheme for platforms w/o hrtimers ? I've no idea if this is a remotely common case any more but in theory I'd have no objection to such a patch though I would like it to be a stand alone trigger similar to that used in the patch Daniel has submitted. >> >>> >>> --- >>> Ina2xx does not support auto-increment, hence the capture threads sticks >>> with single register reads instead of regmap_bulk_read. >>> >>> The proper scales must be applied to those raw register >>> values, I'm in favor of doing the conversion in userland in a client plugin >> >> Yes, conversion should not be done in kernel space, we don't want to impose >> the performance penalty on users which don't need it and you can typically >> do it faster in userspace anyway where you have floats and SSE and what not. >> >>> for instance a sigrok >> >> Slightly OT, but do you already have some Sigrok IIO support? I have this >> scheduled for end of the month, maybe we can align our strategies here and >> avoid duplicated work. > > How fortunate! I've started some preliminary work like cloning the > demo driver into a skeletton for 'hardware/generic-iio/api.c', adding > the build/ac plumbing, and linking to libiio with the idea of using > iio_info to create a generic enumeration of the iio-context into > sigrok channels. > > Now, I'm not familiar with Glib and it might not be my prio until a > couple of weeks, so I'd be super happy to wait for you if you are > keen to do that part :) > > What would be the best spot to chat about this ? > > Marc. > > >> >> - Lars >> >> [1] https://lkml.org/lkml/2015/8/10/877 >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC 4/4] iio: ina2xx: add SOFTWARE buffer mode using an iio kfifo. 2015-11-14 18:44 ` Jonathan Cameron @ 2015-11-16 9:37 ` Marc Titinger 0 siblings, 0 replies; 20+ messages in thread From: Marc Titinger @ 2015-11-16 9:37 UTC (permalink / raw) To: Jonathan Cameron, Lars-Peter Clausen, knaack.h, pmeerw Cc: linux-iio, linux-kernel, mturquette, bcousson, ptitiano On 14/11/2015 19:44, Jonathan Cameron wrote: > On 12/11/15 10:18, Marc Titinger wrote: >> On 10/11/2015 19:23, Lars-Peter Clausen wrote: >>> On 11/10/2015 05:07 PM, Marc Titinger wrote: >>>> Capture the active scan_elements into a kfifo. >>>> The capture thread will 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). >>>> >>>> # iio_readdev ina226 | od -x >>>> WARNING: High-speed mode not enabled >>>> 0000000 042f 0d5a 002e 010c 4be8 4eb4 0013 0000 >>>> 0000020 0430 0d5a 002e 010c a704 4f3e 0013 0000 >>>> 0000040 0430 0d5a 002e 010c b477 4fc7 0013 0000 >>>> 0000060 042f 0d5b 002e 010c 8052 5050 0013 0000 >>>> 0000100 042f 0d5b 002e 010c 5d92 50d8 0013 0000 >>>> 0000120 0430 0d5a 002e 010c fa59 515e 0013 0000 >>>> 0000140 0430 0d5b 002e 010c 95d2 51e5 0013 0000 >>>> >>>> Signed-off-by: Marc Titinger <mtitinger@baylibre.com> >> >> Hi Lars, >> >>> >>> Interesting approach. I think if we are going to due this we want to make >>> this kind of emulation generic. Have you seen the software trigger and >>> configfs support patches[1] from Daniel? It kind of achieves the same as you >>> do, but using hrtimers. >> > >> I totally agree, let me have a look on those patches maybe I could >> add an active waiting scheme for platforms w/o hrtimers ? > I've no idea if this is a remotely common case any more but in theory > I'd have no objection to such a patch though I would like it to be > a stand alone trigger similar to that used in the patch Daniel has > submitted. > Yes, I've started looking into something like a 'polling' trigger class. I understand that I'd need to support triggered buffer mode in the driver in this case. The current patch is rather simple, but having a generic and documented solution seems a good idea. >>> >>>> >>>> --- >>>> Ina2xx does not support auto-increment, hence the capture threads sticks >>>> with single register reads instead of regmap_bulk_read. >>>> >>>> The proper scales must be applied to those raw register >>>> values, I'm in favor of doing the conversion in userland in a client plugin >>> >>> Yes, conversion should not be done in kernel space, we don't want to impose >>> the performance penalty on users which don't need it and you can typically >>> do it faster in userspace anyway where you have floats and SSE and what not. >>> >>>> for instance a sigrok >>> >>> Slightly OT, but do you already have some Sigrok IIO support? I have this >>> scheduled for end of the month, maybe we can align our strategies here and >>> avoid duplicated work. >> > >> How fortunate! I've started some preliminary work like cloning the >> demo driver into a skeletton for 'hardware/generic-iio/api.c', adding >> the build/ac plumbing, and linking to libiio with the idea of using >> iio_info to create a generic enumeration of the iio-context into >> sigrok channels. >> >> Now, I'm not familiar with Glib and it might not be my prio until a >> couple of weeks, so I'd be super happy to wait for you if you are >> keen to do that part :) >> >> What would be the best spot to chat about this ? >> > >> Marc. >> >> >>> >>> - Lars >>> >>> [1] https://lkml.org/lkml/2015/8/10/877 >>> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-iio" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2015-11-16 17:28 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-11-10 16:07 [RFC 0/4] IIO: add support for INA2xx power monitor Marc Titinger 2015-11-10 16:07 ` [RFC 1/4] iio: ina2xx: add direct IO support for TI INA2xx Power Monitors Marc Titinger 2015-11-11 10:14 ` Daniel Baluta 2015-11-12 9:25 ` Marc Titinger 2015-11-11 12:09 ` Daniel Baluta 2015-11-12 9:38 ` Marc Titinger 2015-11-12 12:57 ` [RFC v2 1/2] " Marc Titinger 2015-11-14 18:59 ` Jonathan Cameron 2015-11-16 9:31 ` Marc Titinger 2015-11-16 17:27 ` Jonathan Cameron 2015-11-12 12:57 ` [RFC v2 2/2] iio: ina2xx: add SOFTWARE buffer mode using an iio kfifo Marc Titinger 2015-11-10 16:07 ` [RFC 2/4] iio: ina2xx: add SAMP_FREQ attribute Marc Titinger 2015-11-11 10:17 ` Daniel Baluta 2015-11-10 16:07 ` [RFC 3/4] iio: ina2xx: add debugfs reg access Marc Titinger 2015-11-10 16:07 ` [RFC 4/4] iio: ina2xx: add SOFTWARE buffer mode using an iio kfifo Marc Titinger 2015-11-10 18:23 ` Lars-Peter Clausen 2015-11-12 10:18 ` Marc Titinger 2015-11-12 10:20 ` Lars-Peter Clausen 2015-11-14 18:44 ` Jonathan Cameron 2015-11-16 9:37 ` Marc Titinger
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.