* [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
@ 2016-02-05 13:17 Daniel Baluta
2016-02-05 17:25 ` Michael Welling
` (3 more replies)
0 siblings, 4 replies; 45+ messages in thread
From: Daniel Baluta @ 2016-02-05 13:17 UTC (permalink / raw)
To: jic23
Cc: knaack.h, lars, pmeerw, linux-kernel, linux-iio, daniel.baluta,
lucas.demarchi, mwelling, linux, eibach
The driver has sysfs readings with runtime PM support for power saving.
It also offers buffer support that can be used together with IIO software
triggers.
Datasheet can be found here:
http://www.ti.com.cn/cn/lit/ds/symlink/ads1015.pdf
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
Changes since v3:
* fixed type connectd -> connected
* move mutex outside of switch in read_raw to be consistent
with write_raw
Changes since v2:
* push locking out of the switch in write_raw
* fix buf allocation in triggered buffer's handler
Changes since v1:
* addressed concerns about replacing the ads1015 hwmon driver
* For the moment the IIO driver is compatible with hwmon
driver (dt bindings) the only thing left is to also add
support for ads1115.
* DT bindings are described in Documentation/devicetree/
bindings/hwmon/ads1015.txt. If needed will copy this file
in a later patch to the IIO corresponding location.
* addressed comments from Jonathan
* added proper locking
* added timestamp channel
* removed magic constants
* added some new lines to increase readability
* used regmap_get_device instead of keeping a copy of
i2c_client pointer.
* used non-devm function to correct the cleanup order
drivers/iio/adc/Kconfig | 13 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ti-ads1015.c | 610 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 624 insertions(+)
create mode 100644 drivers/iio/adc/ti-ads1015.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 60673b4..fad7e6a 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -370,6 +370,19 @@ config TI_ADC128S052
This driver can also be built as a module. If so, the module will be
called ti-adc128s052.
+config TI_ADS1015
+ tristate "Texas Instruments ADS1015 ADC"
+ depends on I2C && !SENSORS_ADS1015
+ select REGMAP_I2C
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
+ help
+ If you say yes here you get support for Texas Instruments ADS1015
+ ADC chip.
+
+ This driver can also be built as a module. If so, the module will be
+ called ti-ads1015.
+
config TI_ADS8688
tristate "Texas Instruments ADS8688"
depends on SPI && OF
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index fb57e12..2ff70a3 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
+obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o
obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o
diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
new file mode 100644
index 0000000..596335f
--- /dev/null
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -0,0 +1,610 @@
+/*
+ * ADS1015 - Texas Instruments Analog-to-Digital Converter
+ *
+ * Copyright (c) 2016, Intel Corporation.
+ *
+ * This file is subject to the terms and conditions of version 2 of
+ * the GNU General Public License. See the file COPYING in the main
+ * directory of this archive for more details.
+ *
+ * IIO driver for ADS1015 ADC 7-bit I2C slave address:
+ * * 0x48 - ADDR connected to Ground
+ * * 0x49 - ADDR connected to Vdd
+ * * 0x4A - ADDR connected to SDA
+ * * 0x4B - ADDR connected to SCL
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <linux/pm_runtime.h>
+#include <linux/mutex.h>
+#include <linux/delay.h>
+
+#include <linux/i2c/ads1015.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/types.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
+
+#define ADS1015_DRV_NAME "ads1015"
+
+#define ADS1015_CONV_REG 0x00
+#define ADS1015_CFG_REG 0x01
+
+#define ADS1015_CFG_DR_SHIFT 5
+#define ADS1015_CFG_MOD_SHIFT 8
+#define ADS1015_CFG_PGA_SHIFT 9
+#define ADS1015_CFG_MUX_SHIFT 12
+
+#define ADS1015_CFG_DR_MASK GENMASK(7, 5)
+#define ADS1015_CFG_MOD_MASK BIT(8)
+#define ADS1015_CFG_PGA_MASK GENMASK(11, 9)
+#define ADS1015_CFG_MUX_MASK GENMASK(14, 12)
+
+/* device operating modes */
+#define ADS1015_CONTINUOUS 0
+#define ADS1015_SINGLESHOT 1
+
+#define ADS1015_SLEEP_DELAY_MS 2000
+#define ADS1015_DEFAULT_PGA 2
+#define ADS1015_DEFAULT_DATA_RATE 4
+#define ADS1015_DEFAULT_CHAN 0
+
+enum ads1015_channels {
+ ADS1015_AIN0_AIN1 = 0,
+ ADS1015_AIN0_AIN3,
+ ADS1015_AIN1_AIN3,
+ ADS1015_AIN2_AIN3,
+ ADS1015_AIN0,
+ ADS1015_AIN1,
+ ADS1015_AIN2,
+ ADS1015_AIN3,
+ ADS1015_TIMESTAMP,
+};
+
+static const unsigned int ads1015_data_rate[] = {
+ 128, 250, 490, 920, 1600, 2400, 3300, 3300
+};
+
+static const struct {
+ int scale;
+ int uscale;
+} ads1015_scale[] = {
+ {3, 0},
+ {2, 0},
+ {1, 0},
+ {0, 500000},
+ {0, 250000},
+ {0, 125000},
+ {0, 125000},
+ {0, 125000},
+};
+
+#define ADS1015_V_CHAN(_chan, _addr) { \
+ .type = IIO_VOLTAGE, \
+ .indexed = 1, \
+ .address = _addr, \
+ .channel = _chan, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_SCALE) | \
+ BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+ .scan_index = _addr, \
+ .scan_type = { \
+ .sign = 's', \
+ .realbits = 12, \
+ .storagebits = 16, \
+ .shift = 4, \
+ .endianness = IIO_CPU, \
+ }, \
+}
+
+#define ADS1015_V_DIFF_CHAN(_chan, _chan2, _addr) { \
+ .type = IIO_VOLTAGE, \
+ .differential = 1, \
+ .indexed = 1, \
+ .address = _addr, \
+ .channel = _chan, \
+ .channel2 = _chan2, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_SCALE) | \
+ BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+ .scan_index = _addr, \
+ .scan_type = { \
+ .sign = 's', \
+ .realbits = 12, \
+ .storagebits = 16, \
+ .shift = 4, \
+ .endianness = IIO_CPU, \
+ }, \
+}
+
+struct ads1015_data {
+ struct regmap *regmap;
+ /*
+ * Protects ADC ops, e.g: concurrent sysfs/buffered
+ * data reads, configuration updates
+ */
+ struct mutex lock;
+ struct ads1015_channel_data channel_data[ADS1015_CHANNELS];
+};
+
+static bool ads1015_is_writeable_reg(struct device *dev, unsigned int reg)
+{
+ return (reg == ADS1015_CFG_REG);
+}
+
+static const struct regmap_config ads1015_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 16,
+ .max_register = ADS1015_CFG_REG,
+ .writeable_reg = ads1015_is_writeable_reg,
+};
+
+static const struct iio_chan_spec ads1015_channels[] = {
+ ADS1015_V_DIFF_CHAN(0, 1, ADS1015_AIN0_AIN1),
+ ADS1015_V_DIFF_CHAN(0, 3, ADS1015_AIN0_AIN3),
+ ADS1015_V_DIFF_CHAN(1, 3, ADS1015_AIN1_AIN3),
+ ADS1015_V_DIFF_CHAN(2, 3, ADS1015_AIN2_AIN3),
+ ADS1015_V_CHAN(0, ADS1015_AIN0),
+ ADS1015_V_CHAN(1, ADS1015_AIN1),
+ ADS1015_V_CHAN(2, ADS1015_AIN2),
+ ADS1015_V_CHAN(3, ADS1015_AIN3),
+ IIO_CHAN_SOFT_TIMESTAMP(ADS1015_TIMESTAMP),
+};
+
+static int ads1015_set_power_state(struct ads1015_data *data, bool on)
+{
+ int ret;
+ struct device *dev = regmap_get_device(data->regmap);
+
+ if (on) {
+ ret = pm_runtime_get_sync(dev);
+ if (ret < 0)
+ pm_runtime_put_noidle(dev);
+ } else {
+ pm_runtime_mark_last_busy(dev);
+ ret = pm_runtime_put_autosuspend(dev);
+ }
+
+ return ret;
+}
+
+static
+int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val)
+{
+ int ret, pga, dr, conv_time;
+ bool change;
+
+ if (chan < 0 || chan >= ADS1015_CHANNELS)
+ return -EINVAL;
+
+ pga = data->channel_data[chan].pga;
+ dr = data->channel_data[chan].data_rate;
+
+ ret = regmap_update_bits_check(data->regmap, ADS1015_CFG_REG,
+ ADS1015_CFG_MUX_MASK |
+ ADS1015_CFG_PGA_MASK,
+ chan << ADS1015_CFG_MUX_SHIFT |
+ pga << ADS1015_CFG_PGA_SHIFT,
+ &change);
+ if (ret < 0)
+ return ret;
+
+ if (change) {
+ conv_time = DIV_ROUND_UP(USEC_PER_SEC, ads1015_data_rate[dr]);
+ usleep_range(conv_time, conv_time + 1);
+ }
+
+ return regmap_read(data->regmap, ADS1015_CONV_REG, val);
+}
+
+static irqreturn_t ads1015_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct ads1015_data *data = iio_priv(indio_dev);
+ s16 buf[8]; /* 1x s16 ADC val + 3x s16 padding + 4x s16 timestamp */
+ int chan, ret, res;
+
+ memset(buf, 0, sizeof(buf));
+
+ mutex_lock(&data->lock);
+ chan = find_first_bit(indio_dev->active_scan_mask,
+ indio_dev->masklength);
+ ret = ads1015_get_adc_result(data, chan, &res);
+ if (ret < 0) {
+ mutex_unlock(&data->lock);
+ goto err;
+ }
+
+ buf[0] = res;
+ mutex_unlock(&data->lock);
+
+ iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
+
+err:
+ iio_trigger_notify_done(indio_dev->trig);
+
+ return IRQ_HANDLED;
+}
+
+static int ads1015_set_scale(struct ads1015_data *data, int chan,
+ int scale, int uscale)
+{
+ int i, ret, rindex = -1;
+
+ for (i = 0; i < ARRAY_SIZE(ads1015_scale); i++)
+ if (ads1015_scale[i].scale == scale &&
+ ads1015_scale[i].uscale == uscale) {
+ rindex = i;
+ break;
+ }
+ if (rindex < 0)
+ return -EINVAL;
+
+ ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
+ ADS1015_CFG_PGA_MASK,
+ rindex << ADS1015_CFG_PGA_SHIFT);
+ if (ret < 0)
+ return ret;
+
+ data->channel_data[chan].pga = rindex;
+
+ return 0;
+}
+
+static int ads1015_set_data_rate(struct ads1015_data *data, int chan, int rate)
+{
+ int i, ret, rindex = -1;
+
+ for (i = 0; i < ARRAY_SIZE(ads1015_data_rate); i++)
+ if (ads1015_data_rate[i] == rate) {
+ rindex = i;
+ break;
+ }
+ if (rindex < 0)
+ return -EINVAL;
+
+ ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
+ ADS1015_CFG_DR_MASK,
+ rindex << ADS1015_CFG_DR_SHIFT);
+ if (ret < 0)
+ return ret;
+
+ data->channel_data[chan].data_rate = rindex;
+
+ return 0;
+}
+
+static int ads1015_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int *val,
+ int *val2, long mask)
+{
+ int ret, idx;
+ struct ads1015_data *data = iio_priv(indio_dev);
+
+ mutex_lock(&data->lock);
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ if (iio_buffer_enabled(indio_dev)) {
+ ret = -EBUSY;
+ break;
+ }
+
+ ret = ads1015_set_power_state(data, true);
+ if (ret < 0)
+ break;
+
+ ret = ads1015_get_adc_result(data, chan->address, val);
+ if (ret < 0) {
+ ads1015_set_power_state(data, false);
+ break;
+ }
+
+ /* 12 bit res, D0 is bit 4 in conversion register */
+ *val = sign_extend32(*val >> 4, 11);
+
+ ret = ads1015_set_power_state(data, false);
+ if (ret < 0)
+ break;
+
+ ret = IIO_VAL_INT;
+ break;
+ case IIO_CHAN_INFO_SCALE:
+ idx = data->channel_data[chan->address].pga;
+ *val = ads1015_scale[idx].scale;
+ *val2 = ads1015_scale[idx].uscale;
+ ret = IIO_VAL_INT_PLUS_MICRO;
+ break;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ idx = data->channel_data[chan->address].data_rate;
+ *val = ads1015_data_rate[idx];
+ ret = IIO_VAL_INT;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ mutex_unlock(&data->lock);
+
+ return ret;
+}
+
+static int ads1015_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int val,
+ int val2, long mask)
+{
+ struct ads1015_data *data = iio_priv(indio_dev);
+ int ret;
+
+ mutex_lock(&data->lock);
+ switch (mask) {
+ case IIO_CHAN_INFO_SCALE:
+ ret = ads1015_set_scale(data, chan->address, val, val2);
+ break;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ ret = ads1015_set_data_rate(data, chan->address, val);
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ mutex_unlock(&data->lock);
+
+ return ret;
+}
+
+static int ads1015_buffer_preenable(struct iio_dev *indio_dev)
+{
+ return ads1015_set_power_state(iio_priv(indio_dev), true);
+}
+
+static int ads1015_buffer_postdisable(struct iio_dev *indio_dev)
+{
+ return ads1015_set_power_state(iio_priv(indio_dev), false);
+}
+
+static const struct iio_buffer_setup_ops ads1015_buffer_setup_ops = {
+ .preenable = ads1015_buffer_preenable,
+ .postenable = iio_triggered_buffer_postenable,
+ .predisable = iio_triggered_buffer_predisable,
+ .postdisable = ads1015_buffer_postdisable,
+ .validate_scan_mask = &iio_validate_scan_mask_onehot,
+};
+
+static IIO_CONST_ATTR(scale_available, "3 2 1 0.5 0.25 0.125");
+static IIO_CONST_ATTR(sampling_frequency_available,
+ "128 250 490 920 1600 2400 3300");
+
+static struct attribute *ads1015_attributes[] = {
+ &iio_const_attr_scale_available.dev_attr.attr,
+ &iio_const_attr_sampling_frequency_available.dev_attr.attr,
+ NULL,
+};
+
+static const struct attribute_group ads1015_attribute_group = {
+ .attrs = ads1015_attributes,
+};
+
+static const struct iio_info ads1015_info = {
+ .driver_module = THIS_MODULE,
+ .read_raw = ads1015_read_raw,
+ .write_raw = ads1015_write_raw,
+ .attrs = &ads1015_attribute_group,
+};
+
+#ifdef CONFIG_OF
+static int ads1015_get_channels_config_of(struct i2c_client *client)
+{
+ struct ads1015_data *data = i2c_get_clientdata(client);
+ struct device_node *node;
+
+ if (!client->dev.of_node ||
+ !of_get_next_child(client->dev.of_node, NULL))
+ return -EINVAL;
+
+ for_each_child_of_node(client->dev.of_node, node) {
+ u32 pval;
+ unsigned int channel;
+ unsigned int pga = ADS1015_DEFAULT_PGA;
+ unsigned int data_rate = ADS1015_DEFAULT_DATA_RATE;
+
+ if (of_property_read_u32(node, "reg", &pval)) {
+ dev_err(&client->dev, "invalid reg on %s\n",
+ node->full_name);
+ continue;
+ }
+
+ channel = pval;
+ if (channel >= ADS1015_CHANNELS) {
+ dev_err(&client->dev,
+ "invalid channel index %d on %s\n",
+ channel, node->full_name);
+ continue;
+ }
+
+ if (!of_property_read_u32(node, "ti,gain", &pval)) {
+ pga = pval;
+ if (pga > 6) {
+ dev_err(&client->dev, "invalid gain on %s\n",
+ node->full_name);
+ return -EINVAL;
+ }
+ }
+
+ if (!of_property_read_u32(node, "ti,datarate", &pval)) {
+ data_rate = pval;
+ if (data_rate > 7) {
+ dev_err(&client->dev,
+ "invalid data_rate on %s\n",
+ node->full_name);
+ return -EINVAL;
+ }
+ }
+
+ data->channel_data[channel].pga = pga;
+ data->channel_data[channel].data_rate = data_rate;
+ }
+
+ return 0;
+}
+#endif
+
+static void ads1015_get_channels_config(struct i2c_client *client)
+{
+ unsigned int k;
+
+ struct iio_dev *indio_dev = i2c_get_clientdata(client);
+ struct ads1015_data *data = iio_priv(indio_dev);
+ struct ads1015_platform_data *pdata = dev_get_platdata(&client->dev);
+
+ /* prefer platform data */
+ if (pdata) {
+ memcpy(data->channel_data, pdata->channel_data,
+ sizeof(data->channel_data));
+ return;
+ }
+
+#ifdef CONFIG_OF
+ if (!ads1015_get_channels_config_of(client))
+ return;
+#endif
+ /* fallback on default configuration */
+ for (k = 0; k < ADS1015_CHANNELS; ++k) {
+ data->channel_data[k].pga = ADS1015_DEFAULT_PGA;
+ data->channel_data[k].data_rate = ADS1015_DEFAULT_DATA_RATE;
+ }
+}
+
+static int ads1015_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct iio_dev *indio_dev;
+ struct ads1015_data *data;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ data = iio_priv(indio_dev);
+ i2c_set_clientdata(client, indio_dev);
+
+ mutex_init(&data->lock);
+
+ indio_dev->dev.parent = &client->dev;
+ indio_dev->info = &ads1015_info;
+ indio_dev->name = ADS1015_DRV_NAME;
+ indio_dev->channels = ads1015_channels;
+ indio_dev->num_channels = ARRAY_SIZE(ads1015_channels);
+ indio_dev->modes = INDIO_DIRECT_MODE;
+
+ /* we need this be keep the same ABI with hwmon ADS1015 driver */
+ ads1015_get_channels_config(client);
+
+ data->regmap = devm_regmap_init_i2c(client, &ads1015_regmap_config);
+ if (IS_ERR(data->regmap)) {
+ dev_err(&client->dev, "Failed to allocate register map\n");
+ return PTR_ERR(data->regmap);
+ }
+
+ ret = iio_triggered_buffer_setup(indio_dev, NULL,
+ ads1015_trigger_handler,
+ &ads1015_buffer_setup_ops);
+ if (ret < 0) {
+ dev_err(&client->dev, "iio triggered buffer setup failed\n");
+ return ret;
+ }
+ ret = pm_runtime_set_active(&client->dev);
+ if (ret)
+ goto err_buffer_cleanup;
+ pm_runtime_set_autosuspend_delay(&client->dev, ADS1015_SLEEP_DELAY_MS);
+ pm_runtime_use_autosuspend(&client->dev);
+ pm_runtime_enable(&client->dev);
+
+ ret = iio_device_register(indio_dev);
+ if (ret < 0) {
+ dev_err(&client->dev, "Failed to register IIO device\n");
+ goto err_buffer_cleanup;
+ }
+
+ return 0;
+
+err_buffer_cleanup:
+ iio_triggered_buffer_cleanup(indio_dev);
+
+ return ret;
+}
+
+static int ads1015_remove(struct i2c_client *client)
+{
+ struct iio_dev *indio_dev = i2c_get_clientdata(client);
+ struct ads1015_data *data = iio_priv(indio_dev);
+
+ iio_device_unregister(indio_dev);
+
+ pm_runtime_disable(&client->dev);
+ pm_runtime_set_suspended(&client->dev);
+ pm_runtime_put_noidle(&client->dev);
+
+ iio_triggered_buffer_cleanup(indio_dev);
+
+ /* power down single shot mode */
+ return regmap_update_bits(data->regmap, ADS1015_CFG_REG,
+ ADS1015_CFG_MOD_MASK,
+ ADS1015_SINGLESHOT << ADS1015_CFG_MOD_SHIFT);
+}
+
+#ifdef CONFIG_PM
+static int ads1015_runtime_suspend(struct device *dev)
+{
+ struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+ struct ads1015_data *data = iio_priv(indio_dev);
+
+ return regmap_update_bits(data->regmap, ADS1015_CFG_REG,
+ ADS1015_CFG_MOD_MASK,
+ ADS1015_SINGLESHOT << ADS1015_CFG_MOD_SHIFT);
+}
+
+static int ads1015_runtime_resume(struct device *dev)
+{
+ struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+ struct ads1015_data *data = iio_priv(indio_dev);
+
+ return regmap_update_bits(data->regmap, ADS1015_CFG_REG,
+ ADS1015_CFG_MOD_MASK,
+ ADS1015_CONTINUOUS << ADS1015_CFG_MOD_SHIFT);
+}
+#endif
+
+static const struct dev_pm_ops ads1015_pm_ops = {
+ SET_RUNTIME_PM_OPS(ads1015_runtime_suspend,
+ ads1015_runtime_resume, NULL)
+};
+
+static const struct i2c_device_id ads1015_id[] = {
+ {"ads1015", 0},
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, ads1015_id);
+
+static struct i2c_driver ads1015_driver = {
+ .driver = {
+ .name = ADS1015_DRV_NAME,
+ .pm = &ads1015_pm_ops,
+ },
+ .probe = ads1015_probe,
+ .remove = ads1015_remove,
+ .id_table = ads1015_id,
+};
+
+module_i2c_driver(ads1015_driver);
+
+MODULE_AUTHOR("Daniel Baluta <daniel.baluta@intel.com>");
+MODULE_DESCRIPTION("Texas Instruments ADS1015 ADC driver");
+MODULE_LICENSE("GPL v2");
--
2.5.0
^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-05 13:17 [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support Daniel Baluta
@ 2016-02-05 17:25 ` Michael Welling
2016-02-05 19:32 ` Daniel Baluta
2016-02-05 21:02 ` Lucas De Marchi
` (2 subsequent siblings)
3 siblings, 1 reply; 45+ messages in thread
From: Michael Welling @ 2016-02-05 17:25 UTC (permalink / raw)
To: Daniel Baluta
Cc: jic23, knaack.h, lars, pmeerw, linux-kernel, linux-iio,
lucas.demarchi, linux, eibach
On Fri, Feb 05, 2016 at 03:17:18PM +0200, Daniel Baluta wrote:
> The driver has sysfs readings with runtime PM support for power saving.
> It also offers buffer support that can be used together with IIO software
> triggers.
>
> Datasheet can be found here:
> http://www.ti.com.cn/cn/lit/ds/symlink/ads1015.pdf
>
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> ---
> Changes since v3:
> * fixed type connectd -> connected
> * move mutex outside of switch in read_raw to be consistent
> with write_raw
>
> Changes since v2:
> * push locking out of the switch in write_raw
> * fix buf allocation in triggered buffer's handler
>
> Changes since v1:
> * addressed concerns about replacing the ads1015 hwmon driver
> * For the moment the IIO driver is compatible with hwmon
> driver (dt bindings) the only thing left is to also add
> support for ads1115.
> * DT bindings are described in Documentation/devicetree/
> bindings/hwmon/ads1015.txt. If needed will copy this file
> in a later patch to the IIO corresponding location.
> * addressed comments from Jonathan
> * added proper locking
> * added timestamp channel
> * removed magic constants
> * added some new lines to increase readability
> * used regmap_get_device instead of keeping a copy of
> i2c_client pointer.
> * used non-devm function to correct the cleanup order
>
> drivers/iio/adc/Kconfig | 13 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/ti-ads1015.c | 610 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 624 insertions(+)
> create mode 100644 drivers/iio/adc/ti-ads1015.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 60673b4..fad7e6a 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -370,6 +370,19 @@ config TI_ADC128S052
> This driver can also be built as a module. If so, the module will be
> called ti-adc128s052.
>
> +config TI_ADS1015
> + tristate "Texas Instruments ADS1015 ADC"
> + depends on I2C && !SENSORS_ADS1015
> + select REGMAP_I2C
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> + help
> + If you say yes here you get support for Texas Instruments ADS1015
> + ADC chip.
> +
> + This driver can also be built as a module. If so, the module will be
> + called ti-ads1015.
> +
> config TI_ADS8688
> tristate "Texas Instruments ADS8688"
> depends on SPI && OF
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index fb57e12..2ff70a3 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -36,6 +36,7 @@ obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
> obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
> obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
> obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
> +obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o
> obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
> obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
> obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o
> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
> new file mode 100644
> index 0000000..596335f
> --- /dev/null
> +++ b/drivers/iio/adc/ti-ads1015.c
> @@ -0,0 +1,610 @@
> +/*
> + * ADS1015 - Texas Instruments Analog-to-Digital Converter
> + *
> + * Copyright (c) 2016, Intel Corporation.
> + *
> + * This file is subject to the terms and conditions of version 2 of
> + * the GNU General Public License. See the file COPYING in the main
> + * directory of this archive for more details.
> + *
> + * IIO driver for ADS1015 ADC 7-bit I2C slave address:
> + * * 0x48 - ADDR connected to Ground
> + * * 0x49 - ADDR connected to Vdd
> + * * 0x4A - ADDR connected to SDA
> + * * 0x4B - ADDR connected to SCL
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/i2c.h>
> +#include <linux/regmap.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/mutex.h>
> +#include <linux/delay.h>
> +
> +#include <linux/i2c/ads1015.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/types.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +
> +#define ADS1015_DRV_NAME "ads1015"
> +
> +#define ADS1015_CONV_REG 0x00
> +#define ADS1015_CFG_REG 0x01
> +
> +#define ADS1015_CFG_DR_SHIFT 5
> +#define ADS1015_CFG_MOD_SHIFT 8
> +#define ADS1015_CFG_PGA_SHIFT 9
> +#define ADS1015_CFG_MUX_SHIFT 12
> +
> +#define ADS1015_CFG_DR_MASK GENMASK(7, 5)
> +#define ADS1015_CFG_MOD_MASK BIT(8)
> +#define ADS1015_CFG_PGA_MASK GENMASK(11, 9)
> +#define ADS1015_CFG_MUX_MASK GENMASK(14, 12)
> +
> +/* device operating modes */
> +#define ADS1015_CONTINUOUS 0
> +#define ADS1015_SINGLESHOT 1
> +
> +#define ADS1015_SLEEP_DELAY_MS 2000
> +#define ADS1015_DEFAULT_PGA 2
> +#define ADS1015_DEFAULT_DATA_RATE 4
> +#define ADS1015_DEFAULT_CHAN 0
> +
> +enum ads1015_channels {
> + ADS1015_AIN0_AIN1 = 0,
> + ADS1015_AIN0_AIN3,
> + ADS1015_AIN1_AIN3,
> + ADS1015_AIN2_AIN3,
> + ADS1015_AIN0,
> + ADS1015_AIN1,
> + ADS1015_AIN2,
> + ADS1015_AIN3,
> + ADS1015_TIMESTAMP,
> +};
> +
> +static const unsigned int ads1015_data_rate[] = {
> + 128, 250, 490, 920, 1600, 2400, 3300, 3300
> +};
> +
> +static const struct {
> + int scale;
> + int uscale;
> +} ads1015_scale[] = {
> + {3, 0},
> + {2, 0},
> + {1, 0},
> + {0, 500000},
> + {0, 250000},
> + {0, 125000},
> + {0, 125000},
> + {0, 125000},
> +};
> +
> +#define ADS1015_V_CHAN(_chan, _addr) { \
> + .type = IIO_VOLTAGE, \
> + .indexed = 1, \
> + .address = _addr, \
> + .channel = _chan, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_SCALE) | \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> + .scan_index = _addr, \
> + .scan_type = { \
> + .sign = 's', \
> + .realbits = 12, \
> + .storagebits = 16, \
> + .shift = 4, \
> + .endianness = IIO_CPU, \
> + }, \
> +}
> +
> +#define ADS1015_V_DIFF_CHAN(_chan, _chan2, _addr) { \
> + .type = IIO_VOLTAGE, \
> + .differential = 1, \
> + .indexed = 1, \
> + .address = _addr, \
> + .channel = _chan, \
> + .channel2 = _chan2, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_SCALE) | \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> + .scan_index = _addr, \
> + .scan_type = { \
> + .sign = 's', \
> + .realbits = 12, \
> + .storagebits = 16, \
> + .shift = 4, \
> + .endianness = IIO_CPU, \
> + }, \
> +}
> +
> +struct ads1015_data {
> + struct regmap *regmap;
> + /*
> + * Protects ADC ops, e.g: concurrent sysfs/buffered
> + * data reads, configuration updates
> + */
> + struct mutex lock;
> + struct ads1015_channel_data channel_data[ADS1015_CHANNELS];
> +};
> +
> +static bool ads1015_is_writeable_reg(struct device *dev, unsigned int reg)
> +{
> + return (reg == ADS1015_CFG_REG);
> +}
> +
> +static const struct regmap_config ads1015_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 16,
> + .max_register = ADS1015_CFG_REG,
> + .writeable_reg = ads1015_is_writeable_reg,
> +};
> +
> +static const struct iio_chan_spec ads1015_channels[] = {
> + ADS1015_V_DIFF_CHAN(0, 1, ADS1015_AIN0_AIN1),
> + ADS1015_V_DIFF_CHAN(0, 3, ADS1015_AIN0_AIN3),
> + ADS1015_V_DIFF_CHAN(1, 3, ADS1015_AIN1_AIN3),
> + ADS1015_V_DIFF_CHAN(2, 3, ADS1015_AIN2_AIN3),
> + ADS1015_V_CHAN(0, ADS1015_AIN0),
> + ADS1015_V_CHAN(1, ADS1015_AIN1),
> + ADS1015_V_CHAN(2, ADS1015_AIN2),
> + ADS1015_V_CHAN(3, ADS1015_AIN3),
> + IIO_CHAN_SOFT_TIMESTAMP(ADS1015_TIMESTAMP),
> +};
> +
> +static int ads1015_set_power_state(struct ads1015_data *data, bool on)
> +{
> + int ret;
> + struct device *dev = regmap_get_device(data->regmap);
> +
> + if (on) {
> + ret = pm_runtime_get_sync(dev);
> + if (ret < 0)
> + pm_runtime_put_noidle(dev);
> + } else {
> + pm_runtime_mark_last_busy(dev);
> + ret = pm_runtime_put_autosuspend(dev);
> + }
> +
> + return ret;
> +}
> +
> +static
> +int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val)
> +{
> + int ret, pga, dr, conv_time;
> + bool change;
> +
> + if (chan < 0 || chan >= ADS1015_CHANNELS)
> + return -EINVAL;
> +
> + pga = data->channel_data[chan].pga;
> + dr = data->channel_data[chan].data_rate;
> +
> + ret = regmap_update_bits_check(data->regmap, ADS1015_CFG_REG,
> + ADS1015_CFG_MUX_MASK |
> + ADS1015_CFG_PGA_MASK,
> + chan << ADS1015_CFG_MUX_SHIFT |
> + pga << ADS1015_CFG_PGA_SHIFT,
> + &change);
> + if (ret < 0)
> + return ret;
> +
> + if (change) {
> + conv_time = DIV_ROUND_UP(USEC_PER_SEC, ads1015_data_rate[dr]);
> + usleep_range(conv_time, conv_time + 1);
> + }
> +
> + return regmap_read(data->regmap, ADS1015_CONV_REG, val);
> +}
> +
> +static irqreturn_t ads1015_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct ads1015_data *data = iio_priv(indio_dev);
> + s16 buf[8]; /* 1x s16 ADC val + 3x s16 padding + 4x s16 timestamp */
> + int chan, ret, res;
> +
> + memset(buf, 0, sizeof(buf));
> +
> + mutex_lock(&data->lock);
> + chan = find_first_bit(indio_dev->active_scan_mask,
> + indio_dev->masklength);
> + ret = ads1015_get_adc_result(data, chan, &res);
> + if (ret < 0) {
> + mutex_unlock(&data->lock);
> + goto err;
> + }
> +
> + buf[0] = res;
> + mutex_unlock(&data->lock);
> +
> + iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
> +
> +err:
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int ads1015_set_scale(struct ads1015_data *data, int chan,
> + int scale, int uscale)
> +{
> + int i, ret, rindex = -1;
> +
> + for (i = 0; i < ARRAY_SIZE(ads1015_scale); i++)
> + if (ads1015_scale[i].scale == scale &&
> + ads1015_scale[i].uscale == uscale) {
> + rindex = i;
> + break;
> + }
> + if (rindex < 0)
> + return -EINVAL;
> +
> + ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
> + ADS1015_CFG_PGA_MASK,
> + rindex << ADS1015_CFG_PGA_SHIFT);
> + if (ret < 0)
> + return ret;
> +
> + data->channel_data[chan].pga = rindex;
> +
> + return 0;
> +}
> +
> +static int ads1015_set_data_rate(struct ads1015_data *data, int chan, int rate)
> +{
> + int i, ret, rindex = -1;
> +
> + for (i = 0; i < ARRAY_SIZE(ads1015_data_rate); i++)
> + if (ads1015_data_rate[i] == rate) {
> + rindex = i;
> + break;
> + }
> + if (rindex < 0)
> + return -EINVAL;
> +
> + ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
> + ADS1015_CFG_DR_MASK,
> + rindex << ADS1015_CFG_DR_SHIFT);
> + if (ret < 0)
> + return ret;
> +
> + data->channel_data[chan].data_rate = rindex;
> +
> + return 0;
> +}
> +
> +static int ads1015_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int *val,
> + int *val2, long mask)
> +{
> + int ret, idx;
> + struct ads1015_data *data = iio_priv(indio_dev);
> +
> + mutex_lock(&data->lock);
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + if (iio_buffer_enabled(indio_dev)) {
> + ret = -EBUSY;
> + break;
> + }
> +
> + ret = ads1015_set_power_state(data, true);
> + if (ret < 0)
> + break;
Just tested the driver on a Dragonboard 410C with a robotics mezzanine that I
designed.
The above ads1015_set_power_state(data, true) is always returning -EINVAL.
Any ideas why that would be happening?
I think it may be the return from pm_runtime_get_sync?
When I comment out the break the readings come back but are not updated continually.
If I read in_voltage0-voltage1_raw then in_voltage0_raw the value is updated.
> +
> + ret = ads1015_get_adc_result(data, chan->address, val);
> + if (ret < 0) {
> + ads1015_set_power_state(data, false);
> + break;
> + }
> +
> + /* 12 bit res, D0 is bit 4 in conversion register */
> + *val = sign_extend32(*val >> 4, 11);
> +
> + ret = ads1015_set_power_state(data, false);
> + if (ret < 0)
> + break;
> +
> + ret = IIO_VAL_INT;
> + break;
> + case IIO_CHAN_INFO_SCALE:
> + idx = data->channel_data[chan->address].pga;
> + *val = ads1015_scale[idx].scale;
> + *val2 = ads1015_scale[idx].uscale;
> + ret = IIO_VAL_INT_PLUS_MICRO;
> + break;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + idx = data->channel_data[chan->address].data_rate;
> + *val = ads1015_data_rate[idx];
> + ret = IIO_VAL_INT;
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> + mutex_unlock(&data->lock);
> +
> + return ret;
> +}
> +
> +static int ads1015_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int val,
> + int val2, long mask)
> +{
> + struct ads1015_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + mutex_lock(&data->lock);
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + ret = ads1015_set_scale(data, chan->address, val, val2);
> + break;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + ret = ads1015_set_data_rate(data, chan->address, val);
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> + mutex_unlock(&data->lock);
> +
> + return ret;
> +}
> +
> +static int ads1015_buffer_preenable(struct iio_dev *indio_dev)
> +{
> + return ads1015_set_power_state(iio_priv(indio_dev), true);
> +}
> +
> +static int ads1015_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> + return ads1015_set_power_state(iio_priv(indio_dev), false);
> +}
> +
> +static const struct iio_buffer_setup_ops ads1015_buffer_setup_ops = {
> + .preenable = ads1015_buffer_preenable,
> + .postenable = iio_triggered_buffer_postenable,
> + .predisable = iio_triggered_buffer_predisable,
> + .postdisable = ads1015_buffer_postdisable,
> + .validate_scan_mask = &iio_validate_scan_mask_onehot,
> +};
> +
> +static IIO_CONST_ATTR(scale_available, "3 2 1 0.5 0.25 0.125");
> +static IIO_CONST_ATTR(sampling_frequency_available,
> + "128 250 490 920 1600 2400 3300");
> +
> +static struct attribute *ads1015_attributes[] = {
> + &iio_const_attr_scale_available.dev_attr.attr,
> + &iio_const_attr_sampling_frequency_available.dev_attr.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group ads1015_attribute_group = {
> + .attrs = ads1015_attributes,
> +};
> +
> +static const struct iio_info ads1015_info = {
> + .driver_module = THIS_MODULE,
> + .read_raw = ads1015_read_raw,
> + .write_raw = ads1015_write_raw,
> + .attrs = &ads1015_attribute_group,
> +};
> +
> +#ifdef CONFIG_OF
> +static int ads1015_get_channels_config_of(struct i2c_client *client)
> +{
> + struct ads1015_data *data = i2c_get_clientdata(client);
> + struct device_node *node;
> +
> + if (!client->dev.of_node ||
> + !of_get_next_child(client->dev.of_node, NULL))
> + return -EINVAL;
> +
> + for_each_child_of_node(client->dev.of_node, node) {
> + u32 pval;
> + unsigned int channel;
> + unsigned int pga = ADS1015_DEFAULT_PGA;
> + unsigned int data_rate = ADS1015_DEFAULT_DATA_RATE;
> +
> + if (of_property_read_u32(node, "reg", &pval)) {
> + dev_err(&client->dev, "invalid reg on %s\n",
> + node->full_name);
> + continue;
> + }
> +
> + channel = pval;
> + if (channel >= ADS1015_CHANNELS) {
> + dev_err(&client->dev,
> + "invalid channel index %d on %s\n",
> + channel, node->full_name);
> + continue;
> + }
> +
> + if (!of_property_read_u32(node, "ti,gain", &pval)) {
> + pga = pval;
> + if (pga > 6) {
> + dev_err(&client->dev, "invalid gain on %s\n",
> + node->full_name);
> + return -EINVAL;
> + }
> + }
> +
> + if (!of_property_read_u32(node, "ti,datarate", &pval)) {
> + data_rate = pval;
> + if (data_rate > 7) {
> + dev_err(&client->dev,
> + "invalid data_rate on %s\n",
> + node->full_name);
> + return -EINVAL;
> + }
> + }
> +
> + data->channel_data[channel].pga = pga;
> + data->channel_data[channel].data_rate = data_rate;
> + }
> +
> + return 0;
> +}
> +#endif
> +
> +static void ads1015_get_channels_config(struct i2c_client *client)
> +{
> + unsigned int k;
> +
> + struct iio_dev *indio_dev = i2c_get_clientdata(client);
> + struct ads1015_data *data = iio_priv(indio_dev);
> + struct ads1015_platform_data *pdata = dev_get_platdata(&client->dev);
> +
> + /* prefer platform data */
> + if (pdata) {
> + memcpy(data->channel_data, pdata->channel_data,
> + sizeof(data->channel_data));
> + return;
> + }
> +
> +#ifdef CONFIG_OF
> + if (!ads1015_get_channels_config_of(client))
> + return;
> +#endif
> + /* fallback on default configuration */
> + for (k = 0; k < ADS1015_CHANNELS; ++k) {
> + data->channel_data[k].pga = ADS1015_DEFAULT_PGA;
> + data->channel_data[k].data_rate = ADS1015_DEFAULT_DATA_RATE;
> + }
> +}
> +
> +static int ads1015_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct iio_dev *indio_dev;
> + struct ads1015_data *data;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + data = iio_priv(indio_dev);
> + i2c_set_clientdata(client, indio_dev);
> +
> + mutex_init(&data->lock);
> +
> + indio_dev->dev.parent = &client->dev;
> + indio_dev->info = &ads1015_info;
> + indio_dev->name = ADS1015_DRV_NAME;
> + indio_dev->channels = ads1015_channels;
> + indio_dev->num_channels = ARRAY_SIZE(ads1015_channels);
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + /* we need this be keep the same ABI with hwmon ADS1015 driver */
> + ads1015_get_channels_config(client);
> +
> + data->regmap = devm_regmap_init_i2c(client, &ads1015_regmap_config);
> + if (IS_ERR(data->regmap)) {
> + dev_err(&client->dev, "Failed to allocate register map\n");
> + return PTR_ERR(data->regmap);
> + }
> +
> + ret = iio_triggered_buffer_setup(indio_dev, NULL,
> + ads1015_trigger_handler,
> + &ads1015_buffer_setup_ops);
> + if (ret < 0) {
> + dev_err(&client->dev, "iio triggered buffer setup failed\n");
> + return ret;
> + }
> + ret = pm_runtime_set_active(&client->dev);
> + if (ret)
> + goto err_buffer_cleanup;
> + pm_runtime_set_autosuspend_delay(&client->dev, ADS1015_SLEEP_DELAY_MS);
> + pm_runtime_use_autosuspend(&client->dev);
> + pm_runtime_enable(&client->dev);
> +
> + ret = iio_device_register(indio_dev);
> + if (ret < 0) {
> + dev_err(&client->dev, "Failed to register IIO device\n");
> + goto err_buffer_cleanup;
> + }
> +
> + return 0;
> +
> +err_buffer_cleanup:
> + iio_triggered_buffer_cleanup(indio_dev);
> +
> + return ret;
> +}
> +
> +static int ads1015_remove(struct i2c_client *client)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(client);
> + struct ads1015_data *data = iio_priv(indio_dev);
> +
> + iio_device_unregister(indio_dev);
> +
> + pm_runtime_disable(&client->dev);
> + pm_runtime_set_suspended(&client->dev);
> + pm_runtime_put_noidle(&client->dev);
> +
> + iio_triggered_buffer_cleanup(indio_dev);
> +
> + /* power down single shot mode */
> + return regmap_update_bits(data->regmap, ADS1015_CFG_REG,
> + ADS1015_CFG_MOD_MASK,
> + ADS1015_SINGLESHOT << ADS1015_CFG_MOD_SHIFT);
> +}
> +
> +#ifdef CONFIG_PM
> +static int ads1015_runtime_suspend(struct device *dev)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> + struct ads1015_data *data = iio_priv(indio_dev);
> +
> + return regmap_update_bits(data->regmap, ADS1015_CFG_REG,
> + ADS1015_CFG_MOD_MASK,
> + ADS1015_SINGLESHOT << ADS1015_CFG_MOD_SHIFT);
> +}
> +
> +static int ads1015_runtime_resume(struct device *dev)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> + struct ads1015_data *data = iio_priv(indio_dev);
> +
> + return regmap_update_bits(data->regmap, ADS1015_CFG_REG,
> + ADS1015_CFG_MOD_MASK,
> + ADS1015_CONTINUOUS << ADS1015_CFG_MOD_SHIFT);
> +}
> +#endif
> +
> +static const struct dev_pm_ops ads1015_pm_ops = {
> + SET_RUNTIME_PM_OPS(ads1015_runtime_suspend,
> + ads1015_runtime_resume, NULL)
> +};
> +
> +static const struct i2c_device_id ads1015_id[] = {
> + {"ads1015", 0},
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, ads1015_id);
> +
> +static struct i2c_driver ads1015_driver = {
> + .driver = {
> + .name = ADS1015_DRV_NAME,
> + .pm = &ads1015_pm_ops,
> + },
> + .probe = ads1015_probe,
> + .remove = ads1015_remove,
> + .id_table = ads1015_id,
> +};
> +
> +module_i2c_driver(ads1015_driver);
> +
> +MODULE_AUTHOR("Daniel Baluta <daniel.baluta@intel.com>");
> +MODULE_DESCRIPTION("Texas Instruments ADS1015 ADC driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.5.0
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-05 17:25 ` Michael Welling
@ 2016-02-05 19:32 ` Daniel Baluta
2016-02-05 19:50 ` Michael Welling
2016-02-06 0:32 ` Michael Welling
0 siblings, 2 replies; 45+ messages in thread
From: Daniel Baluta @ 2016-02-05 19:32 UTC (permalink / raw)
To: Michael Welling
Cc: Daniel Baluta, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler,
Linux Kernel Mailing List, linux-iio, Lucas De Marchi,
Guenter Roeck, eibach
On Fri, Feb 5, 2016 at 7:25 PM, Michael Welling <mwelling@ieee.org> wrote:
> On Fri, Feb 05, 2016 at 03:17:18PM +0200, Daniel Baluta wrote:
>> The driver has sysfs readings with runtime PM support for power saving.
>> It also offers buffer support that can be used together with IIO software
>> triggers.
>>
>> Datasheet can be found here:
>> http://www.ti.com.cn/cn/lit/ds/symlink/ads1015.pdf
>>
>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
>> ---
>> Changes since v3:
>> * fixed type connectd -> connected
>> * move mutex outside of switch in read_raw to be consistent
>> with write_raw
>>
>> Changes since v2:
>> * push locking out of the switch in write_raw
>> * fix buf allocation in triggered buffer's handler
>>
>> Changes since v1:
>> * addressed concerns about replacing the ads1015 hwmon driver
>> * For the moment the IIO driver is compatible with hwmon
>> driver (dt bindings) the only thing left is to also add
>> support for ads1115.
>> * DT bindings are described in Documentation/devicetree/
>> bindings/hwmon/ads1015.txt. If needed will copy this file
>> in a later patch to the IIO corresponding location.
>> * addressed comments from Jonathan
>> * added proper locking
>> * added timestamp channel
>> * removed magic constants
>> * added some new lines to increase readability
>> * used regmap_get_device instead of keeping a copy of
>> i2c_client pointer.
>> * used non-devm function to correct the cleanup order
>>
>> drivers/iio/adc/Kconfig | 13 +
>> drivers/iio/adc/Makefile | 1 +
>> drivers/iio/adc/ti-ads1015.c | 610 +++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 624 insertions(+)
>> create mode 100644 drivers/iio/adc/ti-ads1015.c
>>
>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>> index 60673b4..fad7e6a 100644
>> --- a/drivers/iio/adc/Kconfig
>> +++ b/drivers/iio/adc/Kconfig
>> @@ -370,6 +370,19 @@ config TI_ADC128S052
>> This driver can also be built as a module. If so, the module will be
>> called ti-adc128s052.
>>
>> +config TI_ADS1015
>> + tristate "Texas Instruments ADS1015 ADC"
>> + depends on I2C && !SENSORS_ADS1015
>> + select REGMAP_I2C
>> + select IIO_BUFFER
>> + select IIO_TRIGGERED_BUFFER
>> + help
>> + If you say yes here you get support for Texas Instruments ADS1015
>> + ADC chip.
>> +
>> + This driver can also be built as a module. If so, the module will be
>> + called ti-ads1015.
>> +
>> config TI_ADS8688
>> tristate "Texas Instruments ADS8688"
>> depends on SPI && OF
>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>> index fb57e12..2ff70a3 100644
>> --- a/drivers/iio/adc/Makefile
>> +++ b/drivers/iio/adc/Makefile
>> @@ -36,6 +36,7 @@ obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
>> obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
>> obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
>> obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
>> +obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o
>> obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
>> obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
>> obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o
>> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
>> new file mode 100644
>> index 0000000..596335f
>> --- /dev/null
>> +++ b/drivers/iio/adc/ti-ads1015.c
>> @@ -0,0 +1,610 @@
>> +/*
>> + * ADS1015 - Texas Instruments Analog-to-Digital Converter
>> + *
>> + * Copyright (c) 2016, Intel Corporation.
>> + *
>> + * This file is subject to the terms and conditions of version 2 of
>> + * the GNU General Public License. See the file COPYING in the main
>> + * directory of this archive for more details.
>> + *
>> + * IIO driver for ADS1015 ADC 7-bit I2C slave address:
>> + * * 0x48 - ADDR connected to Ground
>> + * * 0x49 - ADDR connected to Vdd
>> + * * 0x4A - ADDR connected to SDA
>> + * * 0x4B - ADDR connected to SCL
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/i2c.h>
>> +#include <linux/regmap.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/mutex.h>
>> +#include <linux/delay.h>
>> +
>> +#include <linux/i2c/ads1015.h>
>> +
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/types.h>
>> +#include <linux/iio/sysfs.h>
>> +#include <linux/iio/buffer.h>
>> +#include <linux/iio/triggered_buffer.h>
>> +#include <linux/iio/trigger_consumer.h>
>> +
>> +#define ADS1015_DRV_NAME "ads1015"
>> +
>> +#define ADS1015_CONV_REG 0x00
>> +#define ADS1015_CFG_REG 0x01
>> +
>> +#define ADS1015_CFG_DR_SHIFT 5
>> +#define ADS1015_CFG_MOD_SHIFT 8
>> +#define ADS1015_CFG_PGA_SHIFT 9
>> +#define ADS1015_CFG_MUX_SHIFT 12
>> +
>> +#define ADS1015_CFG_DR_MASK GENMASK(7, 5)
>> +#define ADS1015_CFG_MOD_MASK BIT(8)
>> +#define ADS1015_CFG_PGA_MASK GENMASK(11, 9)
>> +#define ADS1015_CFG_MUX_MASK GENMASK(14, 12)
>> +
>> +/* device operating modes */
>> +#define ADS1015_CONTINUOUS 0
>> +#define ADS1015_SINGLESHOT 1
>> +
>> +#define ADS1015_SLEEP_DELAY_MS 2000
>> +#define ADS1015_DEFAULT_PGA 2
>> +#define ADS1015_DEFAULT_DATA_RATE 4
>> +#define ADS1015_DEFAULT_CHAN 0
>> +
>> +enum ads1015_channels {
>> + ADS1015_AIN0_AIN1 = 0,
>> + ADS1015_AIN0_AIN3,
>> + ADS1015_AIN1_AIN3,
>> + ADS1015_AIN2_AIN3,
>> + ADS1015_AIN0,
>> + ADS1015_AIN1,
>> + ADS1015_AIN2,
>> + ADS1015_AIN3,
>> + ADS1015_TIMESTAMP,
>> +};
>> +
>> +static const unsigned int ads1015_data_rate[] = {
>> + 128, 250, 490, 920, 1600, 2400, 3300, 3300
>> +};
>> +
>> +static const struct {
>> + int scale;
>> + int uscale;
>> +} ads1015_scale[] = {
>> + {3, 0},
>> + {2, 0},
>> + {1, 0},
>> + {0, 500000},
>> + {0, 250000},
>> + {0, 125000},
>> + {0, 125000},
>> + {0, 125000},
>> +};
>> +
>> +#define ADS1015_V_CHAN(_chan, _addr) { \
>> + .type = IIO_VOLTAGE, \
>> + .indexed = 1, \
>> + .address = _addr, \
>> + .channel = _chan, \
>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
>> + BIT(IIO_CHAN_INFO_SCALE) | \
>> + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
>> + .scan_index = _addr, \
>> + .scan_type = { \
>> + .sign = 's', \
>> + .realbits = 12, \
>> + .storagebits = 16, \
>> + .shift = 4, \
>> + .endianness = IIO_CPU, \
>> + }, \
>> +}
>> +
>> +#define ADS1015_V_DIFF_CHAN(_chan, _chan2, _addr) { \
>> + .type = IIO_VOLTAGE, \
>> + .differential = 1, \
>> + .indexed = 1, \
>> + .address = _addr, \
>> + .channel = _chan, \
>> + .channel2 = _chan2, \
>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
>> + BIT(IIO_CHAN_INFO_SCALE) | \
>> + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
>> + .scan_index = _addr, \
>> + .scan_type = { \
>> + .sign = 's', \
>> + .realbits = 12, \
>> + .storagebits = 16, \
>> + .shift = 4, \
>> + .endianness = IIO_CPU, \
>> + }, \
>> +}
>> +
>> +struct ads1015_data {
>> + struct regmap *regmap;
>> + /*
>> + * Protects ADC ops, e.g: concurrent sysfs/buffered
>> + * data reads, configuration updates
>> + */
>> + struct mutex lock;
>> + struct ads1015_channel_data channel_data[ADS1015_CHANNELS];
>> +};
>> +
>> +static bool ads1015_is_writeable_reg(struct device *dev, unsigned int reg)
>> +{
>> + return (reg == ADS1015_CFG_REG);
>> +}
>> +
>> +static const struct regmap_config ads1015_regmap_config = {
>> + .reg_bits = 8,
>> + .val_bits = 16,
>> + .max_register = ADS1015_CFG_REG,
>> + .writeable_reg = ads1015_is_writeable_reg,
>> +};
>> +
>> +static const struct iio_chan_spec ads1015_channels[] = {
>> + ADS1015_V_DIFF_CHAN(0, 1, ADS1015_AIN0_AIN1),
>> + ADS1015_V_DIFF_CHAN(0, 3, ADS1015_AIN0_AIN3),
>> + ADS1015_V_DIFF_CHAN(1, 3, ADS1015_AIN1_AIN3),
>> + ADS1015_V_DIFF_CHAN(2, 3, ADS1015_AIN2_AIN3),
>> + ADS1015_V_CHAN(0, ADS1015_AIN0),
>> + ADS1015_V_CHAN(1, ADS1015_AIN1),
>> + ADS1015_V_CHAN(2, ADS1015_AIN2),
>> + ADS1015_V_CHAN(3, ADS1015_AIN3),
>> + IIO_CHAN_SOFT_TIMESTAMP(ADS1015_TIMESTAMP),
>> +};
>> +
>> +static int ads1015_set_power_state(struct ads1015_data *data, bool on)
>> +{
>> + int ret;
>> + struct device *dev = regmap_get_device(data->regmap);
>> +
>> + if (on) {
>> + ret = pm_runtime_get_sync(dev);
>> + if (ret < 0)
>> + pm_runtime_put_noidle(dev);
>> + } else {
>> + pm_runtime_mark_last_busy(dev);
>> + ret = pm_runtime_put_autosuspend(dev);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static
>> +int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val)
>> +{
>> + int ret, pga, dr, conv_time;
>> + bool change;
>> +
>> + if (chan < 0 || chan >= ADS1015_CHANNELS)
>> + return -EINVAL;
>> +
>> + pga = data->channel_data[chan].pga;
>> + dr = data->channel_data[chan].data_rate;
>> +
>> + ret = regmap_update_bits_check(data->regmap, ADS1015_CFG_REG,
>> + ADS1015_CFG_MUX_MASK |
>> + ADS1015_CFG_PGA_MASK,
>> + chan << ADS1015_CFG_MUX_SHIFT |
>> + pga << ADS1015_CFG_PGA_SHIFT,
>> + &change);
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (change) {
>> + conv_time = DIV_ROUND_UP(USEC_PER_SEC, ads1015_data_rate[dr]);
>> + usleep_range(conv_time, conv_time + 1);
>> + }
>> +
>> + return regmap_read(data->regmap, ADS1015_CONV_REG, val);
>> +}
>> +
>> +static irqreturn_t ads1015_trigger_handler(int irq, void *p)
>> +{
>> + struct iio_poll_func *pf = p;
>> + struct iio_dev *indio_dev = pf->indio_dev;
>> + struct ads1015_data *data = iio_priv(indio_dev);
>> + s16 buf[8]; /* 1x s16 ADC val + 3x s16 padding + 4x s16 timestamp */
>> + int chan, ret, res;
>> +
>> + memset(buf, 0, sizeof(buf));
>> +
>> + mutex_lock(&data->lock);
>> + chan = find_first_bit(indio_dev->active_scan_mask,
>> + indio_dev->masklength);
>> + ret = ads1015_get_adc_result(data, chan, &res);
>> + if (ret < 0) {
>> + mutex_unlock(&data->lock);
>> + goto err;
>> + }
>> +
>> + buf[0] = res;
>> + mutex_unlock(&data->lock);
>> +
>> + iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
>> +
>> +err:
>> + iio_trigger_notify_done(indio_dev->trig);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int ads1015_set_scale(struct ads1015_data *data, int chan,
>> + int scale, int uscale)
>> +{
>> + int i, ret, rindex = -1;
>> +
>> + for (i = 0; i < ARRAY_SIZE(ads1015_scale); i++)
>> + if (ads1015_scale[i].scale == scale &&
>> + ads1015_scale[i].uscale == uscale) {
>> + rindex = i;
>> + break;
>> + }
>> + if (rindex < 0)
>> + return -EINVAL;
>> +
>> + ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
>> + ADS1015_CFG_PGA_MASK,
>> + rindex << ADS1015_CFG_PGA_SHIFT);
>> + if (ret < 0)
>> + return ret;
>> +
>> + data->channel_data[chan].pga = rindex;
>> +
>> + return 0;
>> +}
>> +
>> +static int ads1015_set_data_rate(struct ads1015_data *data, int chan, int rate)
>> +{
>> + int i, ret, rindex = -1;
>> +
>> + for (i = 0; i < ARRAY_SIZE(ads1015_data_rate); i++)
>> + if (ads1015_data_rate[i] == rate) {
>> + rindex = i;
>> + break;
>> + }
>> + if (rindex < 0)
>> + return -EINVAL;
>> +
>> + ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
>> + ADS1015_CFG_DR_MASK,
>> + rindex << ADS1015_CFG_DR_SHIFT);
>> + if (ret < 0)
>> + return ret;
>> +
>> + data->channel_data[chan].data_rate = rindex;
>> +
>> + return 0;
>> +}
>> +
>> +static int ads1015_read_raw(struct iio_dev *indio_dev,
>> + struct iio_chan_spec const *chan, int *val,
>> + int *val2, long mask)
>> +{
>> + int ret, idx;
>> + struct ads1015_data *data = iio_priv(indio_dev);
>> +
>> + mutex_lock(&data->lock);
>> + switch (mask) {
>> + case IIO_CHAN_INFO_RAW:
>> + if (iio_buffer_enabled(indio_dev)) {
>> + ret = -EBUSY;
>> + break;
>> + }
>> +
>> + ret = ads1015_set_power_state(data, true);
>> + if (ret < 0)
>> + break;
>
> Just tested the driver on a Dragonboard 410C with a robotics mezzanine that I
> designed.
>
> The above ads1015_set_power_state(data, true) is always returning -EINVAL.
>
> Any ideas why that would be happening?
> I think it may be the return from pm_runtime_get_sync?
Can you confirm that pm_runtime_get_sync fails? Using some printk?
Also adding printks in suspend/resume function would be helpful. Do
you have CONFIG_PM enabled?
>
> When I comment out the break the readings come back but are not updated continually.
> If I read in_voltage0-voltage1_raw then in_voltage0_raw the value is updated.
I guess this is normal if set_power_state fails.
thanks,
Daniel.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-05 19:32 ` Daniel Baluta
@ 2016-02-05 19:50 ` Michael Welling
2016-02-06 0:32 ` Michael Welling
1 sibling, 0 replies; 45+ messages in thread
From: Michael Welling @ 2016-02-05 19:50 UTC (permalink / raw)
To: Daniel Baluta
Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Linux Kernel Mailing List, linux-iio,
Lucas De Marchi, Guenter Roeck, eibach
On Fri, Feb 05, 2016 at 09:32:34PM +0200, Daniel Baluta wrote:
> On Fri, Feb 5, 2016 at 7:25 PM, Michael Welling <mwelling@ieee.org> wrote:
> > On Fri, Feb 05, 2016 at 03:17:18PM +0200, Daniel Baluta wrote:
> >> The driver has sysfs readings with runtime PM support for power saving.
> >> It also offers buffer support that can be used together with IIO software
> >> triggers.
> >>
> >> Datasheet can be found here:
> >> http://www.ti.com.cn/cn/lit/ds/symlink/ads1015.pdf
> >>
> >> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> >> ---
> >> Changes since v3:
> >> * fixed type connectd -> connected
> >> * move mutex outside of switch in read_raw to be consistent
> >> with write_raw
> >>
> >> Changes since v2:
> >> * push locking out of the switch in write_raw
> >> * fix buf allocation in triggered buffer's handler
> >>
> >> Changes since v1:
> >> * addressed concerns about replacing the ads1015 hwmon driver
> >> * For the moment the IIO driver is compatible with hwmon
> >> driver (dt bindings) the only thing left is to also add
> >> support for ads1115.
> >> * DT bindings are described in Documentation/devicetree/
> >> bindings/hwmon/ads1015.txt. If needed will copy this file
> >> in a later patch to the IIO corresponding location.
> >> * addressed comments from Jonathan
> >> * added proper locking
> >> * added timestamp channel
> >> * removed magic constants
> >> * added some new lines to increase readability
> >> * used regmap_get_device instead of keeping a copy of
> >> i2c_client pointer.
> >> * used non-devm function to correct the cleanup order
> >>
> >> drivers/iio/adc/Kconfig | 13 +
> >> drivers/iio/adc/Makefile | 1 +
> >> drivers/iio/adc/ti-ads1015.c | 610 +++++++++++++++++++++++++++++++++++++++++++
> >> 3 files changed, 624 insertions(+)
> >> create mode 100644 drivers/iio/adc/ti-ads1015.c
> >>
> >> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> >> index 60673b4..fad7e6a 100644
> >> --- a/drivers/iio/adc/Kconfig
> >> +++ b/drivers/iio/adc/Kconfig
> >> @@ -370,6 +370,19 @@ config TI_ADC128S052
> >> This driver can also be built as a module. If so, the module will be
> >> called ti-adc128s052.
> >>
> >> +config TI_ADS1015
> >> + tristate "Texas Instruments ADS1015 ADC"
> >> + depends on I2C && !SENSORS_ADS1015
> >> + select REGMAP_I2C
> >> + select IIO_BUFFER
> >> + select IIO_TRIGGERED_BUFFER
> >> + help
> >> + If you say yes here you get support for Texas Instruments ADS1015
> >> + ADC chip.
> >> +
> >> + This driver can also be built as a module. If so, the module will be
> >> + called ti-ads1015.
> >> +
> >> config TI_ADS8688
> >> tristate "Texas Instruments ADS8688"
> >> depends on SPI && OF
> >> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> >> index fb57e12..2ff70a3 100644
> >> --- a/drivers/iio/adc/Makefile
> >> +++ b/drivers/iio/adc/Makefile
> >> @@ -36,6 +36,7 @@ obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
> >> obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
> >> obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
> >> obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
> >> +obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o
> >> obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
> >> obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
> >> obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o
> >> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
> >> new file mode 100644
> >> index 0000000..596335f
> >> --- /dev/null
> >> +++ b/drivers/iio/adc/ti-ads1015.c
> >> @@ -0,0 +1,610 @@
> >> +/*
> >> + * ADS1015 - Texas Instruments Analog-to-Digital Converter
> >> + *
> >> + * Copyright (c) 2016, Intel Corporation.
> >> + *
> >> + * This file is subject to the terms and conditions of version 2 of
> >> + * the GNU General Public License. See the file COPYING in the main
> >> + * directory of this archive for more details.
> >> + *
> >> + * IIO driver for ADS1015 ADC 7-bit I2C slave address:
> >> + * * 0x48 - ADDR connected to Ground
> >> + * * 0x49 - ADDR connected to Vdd
> >> + * * 0x4A - ADDR connected to SDA
> >> + * * 0x4B - ADDR connected to SCL
> >> + */
> >> +
> >> +#include <linux/module.h>
> >> +#include <linux/init.h>
> >> +#include <linux/i2c.h>
> >> +#include <linux/regmap.h>
> >> +#include <linux/pm_runtime.h>
> >> +#include <linux/mutex.h>
> >> +#include <linux/delay.h>
> >> +
> >> +#include <linux/i2c/ads1015.h>
> >> +
> >> +#include <linux/iio/iio.h>
> >> +#include <linux/iio/types.h>
> >> +#include <linux/iio/sysfs.h>
> >> +#include <linux/iio/buffer.h>
> >> +#include <linux/iio/triggered_buffer.h>
> >> +#include <linux/iio/trigger_consumer.h>
> >> +
> >> +#define ADS1015_DRV_NAME "ads1015"
> >> +
> >> +#define ADS1015_CONV_REG 0x00
> >> +#define ADS1015_CFG_REG 0x01
> >> +
> >> +#define ADS1015_CFG_DR_SHIFT 5
> >> +#define ADS1015_CFG_MOD_SHIFT 8
> >> +#define ADS1015_CFG_PGA_SHIFT 9
> >> +#define ADS1015_CFG_MUX_SHIFT 12
> >> +
> >> +#define ADS1015_CFG_DR_MASK GENMASK(7, 5)
> >> +#define ADS1015_CFG_MOD_MASK BIT(8)
> >> +#define ADS1015_CFG_PGA_MASK GENMASK(11, 9)
> >> +#define ADS1015_CFG_MUX_MASK GENMASK(14, 12)
> >> +
> >> +/* device operating modes */
> >> +#define ADS1015_CONTINUOUS 0
> >> +#define ADS1015_SINGLESHOT 1
> >> +
> >> +#define ADS1015_SLEEP_DELAY_MS 2000
> >> +#define ADS1015_DEFAULT_PGA 2
> >> +#define ADS1015_DEFAULT_DATA_RATE 4
> >> +#define ADS1015_DEFAULT_CHAN 0
> >> +
> >> +enum ads1015_channels {
> >> + ADS1015_AIN0_AIN1 = 0,
> >> + ADS1015_AIN0_AIN3,
> >> + ADS1015_AIN1_AIN3,
> >> + ADS1015_AIN2_AIN3,
> >> + ADS1015_AIN0,
> >> + ADS1015_AIN1,
> >> + ADS1015_AIN2,
> >> + ADS1015_AIN3,
> >> + ADS1015_TIMESTAMP,
> >> +};
> >> +
> >> +static const unsigned int ads1015_data_rate[] = {
> >> + 128, 250, 490, 920, 1600, 2400, 3300, 3300
> >> +};
> >> +
> >> +static const struct {
> >> + int scale;
> >> + int uscale;
> >> +} ads1015_scale[] = {
> >> + {3, 0},
> >> + {2, 0},
> >> + {1, 0},
> >> + {0, 500000},
> >> + {0, 250000},
> >> + {0, 125000},
> >> + {0, 125000},
> >> + {0, 125000},
> >> +};
> >> +
> >> +#define ADS1015_V_CHAN(_chan, _addr) { \
> >> + .type = IIO_VOLTAGE, \
> >> + .indexed = 1, \
> >> + .address = _addr, \
> >> + .channel = _chan, \
> >> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> >> + BIT(IIO_CHAN_INFO_SCALE) | \
> >> + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> >> + .scan_index = _addr, \
> >> + .scan_type = { \
> >> + .sign = 's', \
> >> + .realbits = 12, \
> >> + .storagebits = 16, \
> >> + .shift = 4, \
> >> + .endianness = IIO_CPU, \
> >> + }, \
> >> +}
> >> +
> >> +#define ADS1015_V_DIFF_CHAN(_chan, _chan2, _addr) { \
> >> + .type = IIO_VOLTAGE, \
> >> + .differential = 1, \
> >> + .indexed = 1, \
> >> + .address = _addr, \
> >> + .channel = _chan, \
> >> + .channel2 = _chan2, \
> >> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> >> + BIT(IIO_CHAN_INFO_SCALE) | \
> >> + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> >> + .scan_index = _addr, \
> >> + .scan_type = { \
> >> + .sign = 's', \
> >> + .realbits = 12, \
> >> + .storagebits = 16, \
> >> + .shift = 4, \
> >> + .endianness = IIO_CPU, \
> >> + }, \
> >> +}
> >> +
> >> +struct ads1015_data {
> >> + struct regmap *regmap;
> >> + /*
> >> + * Protects ADC ops, e.g: concurrent sysfs/buffered
> >> + * data reads, configuration updates
> >> + */
> >> + struct mutex lock;
> >> + struct ads1015_channel_data channel_data[ADS1015_CHANNELS];
> >> +};
> >> +
> >> +static bool ads1015_is_writeable_reg(struct device *dev, unsigned int reg)
> >> +{
> >> + return (reg == ADS1015_CFG_REG);
> >> +}
> >> +
> >> +static const struct regmap_config ads1015_regmap_config = {
> >> + .reg_bits = 8,
> >> + .val_bits = 16,
> >> + .max_register = ADS1015_CFG_REG,
> >> + .writeable_reg = ads1015_is_writeable_reg,
> >> +};
> >> +
> >> +static const struct iio_chan_spec ads1015_channels[] = {
> >> + ADS1015_V_DIFF_CHAN(0, 1, ADS1015_AIN0_AIN1),
> >> + ADS1015_V_DIFF_CHAN(0, 3, ADS1015_AIN0_AIN3),
> >> + ADS1015_V_DIFF_CHAN(1, 3, ADS1015_AIN1_AIN3),
> >> + ADS1015_V_DIFF_CHAN(2, 3, ADS1015_AIN2_AIN3),
> >> + ADS1015_V_CHAN(0, ADS1015_AIN0),
> >> + ADS1015_V_CHAN(1, ADS1015_AIN1),
> >> + ADS1015_V_CHAN(2, ADS1015_AIN2),
> >> + ADS1015_V_CHAN(3, ADS1015_AIN3),
> >> + IIO_CHAN_SOFT_TIMESTAMP(ADS1015_TIMESTAMP),
> >> +};
> >> +
> >> +static int ads1015_set_power_state(struct ads1015_data *data, bool on)
> >> +{
> >> + int ret;
> >> + struct device *dev = regmap_get_device(data->regmap);
> >> +
> >> + if (on) {
> >> + ret = pm_runtime_get_sync(dev);
> >> + if (ret < 0)
> >> + pm_runtime_put_noidle(dev);
> >> + } else {
> >> + pm_runtime_mark_last_busy(dev);
> >> + ret = pm_runtime_put_autosuspend(dev);
> >> + }
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +static
> >> +int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val)
> >> +{
> >> + int ret, pga, dr, conv_time;
> >> + bool change;
> >> +
> >> + if (chan < 0 || chan >= ADS1015_CHANNELS)
> >> + return -EINVAL;
> >> +
> >> + pga = data->channel_data[chan].pga;
> >> + dr = data->channel_data[chan].data_rate;
> >> +
> >> + ret = regmap_update_bits_check(data->regmap, ADS1015_CFG_REG,
> >> + ADS1015_CFG_MUX_MASK |
> >> + ADS1015_CFG_PGA_MASK,
> >> + chan << ADS1015_CFG_MUX_SHIFT |
> >> + pga << ADS1015_CFG_PGA_SHIFT,
> >> + &change);
> >> + if (ret < 0)
> >> + return ret;
> >> +
> >> + if (change) {
> >> + conv_time = DIV_ROUND_UP(USEC_PER_SEC, ads1015_data_rate[dr]);
> >> + usleep_range(conv_time, conv_time + 1);
> >> + }
> >> +
> >> + return regmap_read(data->regmap, ADS1015_CONV_REG, val);
> >> +}
> >> +
> >> +static irqreturn_t ads1015_trigger_handler(int irq, void *p)
> >> +{
> >> + struct iio_poll_func *pf = p;
> >> + struct iio_dev *indio_dev = pf->indio_dev;
> >> + struct ads1015_data *data = iio_priv(indio_dev);
> >> + s16 buf[8]; /* 1x s16 ADC val + 3x s16 padding + 4x s16 timestamp */
> >> + int chan, ret, res;
> >> +
> >> + memset(buf, 0, sizeof(buf));
> >> +
> >> + mutex_lock(&data->lock);
> >> + chan = find_first_bit(indio_dev->active_scan_mask,
> >> + indio_dev->masklength);
> >> + ret = ads1015_get_adc_result(data, chan, &res);
> >> + if (ret < 0) {
> >> + mutex_unlock(&data->lock);
> >> + goto err;
> >> + }
> >> +
> >> + buf[0] = res;
> >> + mutex_unlock(&data->lock);
> >> +
> >> + iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
> >> +
> >> +err:
> >> + iio_trigger_notify_done(indio_dev->trig);
> >> +
> >> + return IRQ_HANDLED;
> >> +}
> >> +
> >> +static int ads1015_set_scale(struct ads1015_data *data, int chan,
> >> + int scale, int uscale)
> >> +{
> >> + int i, ret, rindex = -1;
> >> +
> >> + for (i = 0; i < ARRAY_SIZE(ads1015_scale); i++)
> >> + if (ads1015_scale[i].scale == scale &&
> >> + ads1015_scale[i].uscale == uscale) {
> >> + rindex = i;
> >> + break;
> >> + }
> >> + if (rindex < 0)
> >> + return -EINVAL;
> >> +
> >> + ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
> >> + ADS1015_CFG_PGA_MASK,
> >> + rindex << ADS1015_CFG_PGA_SHIFT);
> >> + if (ret < 0)
> >> + return ret;
> >> +
> >> + data->channel_data[chan].pga = rindex;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int ads1015_set_data_rate(struct ads1015_data *data, int chan, int rate)
> >> +{
> >> + int i, ret, rindex = -1;
> >> +
> >> + for (i = 0; i < ARRAY_SIZE(ads1015_data_rate); i++)
> >> + if (ads1015_data_rate[i] == rate) {
> >> + rindex = i;
> >> + break;
> >> + }
> >> + if (rindex < 0)
> >> + return -EINVAL;
> >> +
> >> + ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
> >> + ADS1015_CFG_DR_MASK,
> >> + rindex << ADS1015_CFG_DR_SHIFT);
> >> + if (ret < 0)
> >> + return ret;
> >> +
> >> + data->channel_data[chan].data_rate = rindex;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int ads1015_read_raw(struct iio_dev *indio_dev,
> >> + struct iio_chan_spec const *chan, int *val,
> >> + int *val2, long mask)
> >> +{
> >> + int ret, idx;
> >> + struct ads1015_data *data = iio_priv(indio_dev);
> >> +
> >> + mutex_lock(&data->lock);
> >> + switch (mask) {
> >> + case IIO_CHAN_INFO_RAW:
> >> + if (iio_buffer_enabled(indio_dev)) {
> >> + ret = -EBUSY;
> >> + break;
> >> + }
> >> +
> >> + ret = ads1015_set_power_state(data, true);
> >> + if (ret < 0)
> >> + break;
> >
> > Just tested the driver on a Dragonboard 410C with a robotics mezzanine that I
> > designed.
> >
> > The above ads1015_set_power_state(data, true) is always returning -EINVAL.
> >
> > Any ideas why that would be happening?
> > I think it may be the return from pm_runtime_get_sync?
>
> Can you confirm that pm_runtime_get_sync fails? Using some printk?
>
root@dragonboard-410c:/sys/devices/platform/soc/78b6000.i2c/i2c-0/0-0048/iio:device0# cat in_voltage0_raw
[ 867.065084] mask = 0
[ 867.065104] IIO_CHAN_INFO_RAW
[ 867.066646] error set_power_state IIO_CHAN_INFO_RAW
265
I assumed the only way for the failure to occur is if pm_runtime_get_sync
returns an error in ads1015_set_power_state.
I will add more printks to see if I can find out more.
> Also adding printks in suspend/resume function would be helpful. Do
> you have CONFIG_PM enabled?
#
# Power management options
#
CONFIG_SUSPEND=y
CONFIG_SUSPEND_FREEZER=y
CONFIG_PM_SLEEP=y
CONFIG_PM_SLEEP_SMP=y
# CONFIG_PM_AUTOSLEEP is not set
# CONFIG_PM_WAKELOCKS is not set
CONFIG_PM=y
# CONFIG_PM_DEBUG is not set
CONFIG_PM_CLK=y
CONFIG_PM_GENERIC_DOMAINS=y
# CONFIG_WQ_POWER_EFFICIENT_DEFAULT is not set
CONFIG_PM_GENERIC_DOMAINS_SLEEP=y
CONFIG_PM_GENERIC_DOMAINS_OF=y
CONFIG_CPU_PM=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
>
> >
> > When I comment out the break the readings come back but are not updated continually.
> > If I read in_voltage0-voltage1_raw then in_voltage0_raw the value is updated.
>
> I guess this is normal if set_power_state fails.
>
Yeah the continuous mode is never enabled if the runtime resume is never called.
> thanks,
> Daniel.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-05 13:17 [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support Daniel Baluta
2016-02-05 17:25 ` Michael Welling
@ 2016-02-05 21:02 ` Lucas De Marchi
2016-02-05 21:44 ` Daniel Baluta
2016-02-06 11:05 ` Jonathan Cameron
2016-03-01 0:50 ` Michael Welling
3 siblings, 1 reply; 45+ messages in thread
From: Lucas De Marchi @ 2016-02-05 21:02 UTC (permalink / raw)
To: Daniel Baluta
Cc: jic23, knaack.h, lars, Peter Meerwald-Stadler, lkml, linux-iio,
Lucas De Marchi, mwelling, linux, eibach
Hi Daniel,
On Fri, Feb 5, 2016 at 11:17 AM, Daniel Baluta <daniel.baluta@intel.com> wrote:
> +
> +static const struct i2c_device_id ads1015_id[] = {
> + {"ads1015", 0},
> + {}
> +};
Since this module is not 100% compatible with the hwmon version,
wouldn't it be better to use another id?
Other than that, I tested it on the MinnowBoard Max with a Drone Lure
connected and it works fine.
thanks
Lucas De Marchi
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-05 21:02 ` Lucas De Marchi
@ 2016-02-05 21:44 ` Daniel Baluta
2016-02-06 10:52 ` Jonathan Cameron
0 siblings, 1 reply; 45+ messages in thread
From: Daniel Baluta @ 2016-02-05 21:44 UTC (permalink / raw)
To: Lucas De Marchi
Cc: Daniel Baluta, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, lkml, linux-iio,
Lucas De Marchi, Michael Welling, Guenter Roeck, eibach
On Fri, Feb 5, 2016 at 11:02 PM, Lucas De Marchi
<lucas.de.marchi@gmail.com> wrote:
> Hi Daniel,
>
> On Fri, Feb 5, 2016 at 11:17 AM, Daniel Baluta <daniel.baluta@intel.com> wrote:
>> +
>> +static const struct i2c_device_id ads1015_id[] = {
>> + {"ads1015", 0},
>> + {}
>> +};
>
> Since this module is not 100% compatible with the hwmon version,
> wouldn't it be better to use another id?
Hmm, as it is now it should be 100% compatible with the hwmon driver
for ads1015 device.
I don't have a strong preference for id here. Do you see any problems using
this id?
>
> Other than that, I tested it on the MinnowBoard Max with a Drone Lure
> connected and it works fine.
thanks,
Daniel.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-05 19:32 ` Daniel Baluta
2016-02-05 19:50 ` Michael Welling
@ 2016-02-06 0:32 ` Michael Welling
2016-02-08 10:25 ` Wolfram Sang
1 sibling, 1 reply; 45+ messages in thread
From: Michael Welling @ 2016-02-06 0:32 UTC (permalink / raw)
To: Daniel Baluta
Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Linux Kernel Mailing List, linux-iio,
Lucas De Marchi, Andy Gross, Wolfram Sang, Pramod Gurav,
Bjorn Andersson, Guenter Roeck, eibach
On Fri, Feb 05, 2016 at 09:32:34PM +0200, Daniel Baluta wrote:
> >> +static int ads1015_read_raw(struct iio_dev *indio_dev,
> >> + struct iio_chan_spec const *chan, int *val,
> >> + int *val2, long mask)
> >> +{
> >> + int ret, idx;
> >> + struct ads1015_data *data = iio_priv(indio_dev);
> >> +
> >> + mutex_lock(&data->lock);
> >> + switch (mask) {
> >> + case IIO_CHAN_INFO_RAW:
> >> + if (iio_buffer_enabled(indio_dev)) {
> >> + ret = -EBUSY;
> >> + break;
> >> + }
> >> +
> >> + ret = ads1015_set_power_state(data, true);
> >> + if (ret < 0)
> >> + break;
> >
> > Just tested the driver on a Dragonboard 410C with a robotics mezzanine that I
> > designed.
> >
> > The above ads1015_set_power_state(data, true) is always returning -EINVAL.
> >
> > Any ideas why that would be happening?
> > I think it may be the return from pm_runtime_get_sync?
>
> Can you confirm that pm_runtime_get_sync fails? Using some printk?
>
> Also adding printks in suspend/resume function would be helpful. Do
> you have CONFIG_PM enabled?
>
Indeed it is the pm_runtime_get_sync that fails with a -EINVAL.
> >
> > When I comment out the break the readings come back but are not updated continually.
> > If I read in_voltage0-voltage1_raw then in_voltage0_raw the value is updated.
>
> I guess this is normal if set_power_state fails.
The hwmod driver works fine BTW.
My guess is there is an issue with the qup i2c driver seeing as it has worked on
other system without issue.
CC'd some the latest developer on the qup i2c driver.
I2C guys have any ideas on this?
>
> thanks,
> Daniel.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-05 21:44 ` Daniel Baluta
@ 2016-02-06 10:52 ` Jonathan Cameron
0 siblings, 0 replies; 45+ messages in thread
From: Jonathan Cameron @ 2016-02-06 10:52 UTC (permalink / raw)
To: Daniel Baluta, Lucas De Marchi
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, lkml,
linux-iio, Lucas De Marchi, Michael Welling, Guenter Roeck,
eibach
On 05/02/16 21:44, Daniel Baluta wrote:
> On Fri, Feb 5, 2016 at 11:02 PM, Lucas De Marchi
> <lucas.de.marchi@gmail.com> wrote:
>> Hi Daniel,
>>
>> On Fri, Feb 5, 2016 at 11:17 AM, Daniel Baluta <daniel.baluta@intel.com> wrote:
>>> +
>>> +static const struct i2c_device_id ads1015_id[] = {
>>> + {"ads1015", 0},
>>> + {}
>>> +};
>>
>> Since this module is not 100% compatible with the hwmon version,
>> wouldn't it be better to use another id?
>
> Hmm, as it is now it should be 100% compatible with the hwmon driver
> for ads1015 device.
>
> I don't have a strong preference for id here. Do you see any problems using
> this id?
We definitely want to keep the ID if at all possible as otherwise
we end up with a driver choice having to be made at compile time (or possibly
via the device tree). If there is a compatibility issue, then Lucas,
please highlight it.
In the short term both drivers will exist and we'll rely on the Kconfig tricks
above to prevent them being both built. Longer term, if this driver supports
all features of the HWMON driver (via the iio-hwmon bridge) then then extra
support in here will make it sensible to drop the HWMON driver (just not for
at least a cycle or two I would guess).
Jonathan
>
>>
>> Other than that, I tested it on the MinnowBoard Max with a Drone Lure
>> connected and it works fine.
>
> thanks,
> Daniel.
> --
> 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] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-05 13:17 [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support Daniel Baluta
2016-02-05 17:25 ` Michael Welling
2016-02-05 21:02 ` Lucas De Marchi
@ 2016-02-06 11:05 ` Jonathan Cameron
2016-02-06 11:07 ` Jonathan Cameron
2016-03-01 0:50 ` Michael Welling
3 siblings, 1 reply; 45+ messages in thread
From: Jonathan Cameron @ 2016-02-06 11:05 UTC (permalink / raw)
To: Daniel Baluta
Cc: knaack.h, lars, pmeerw, linux-kernel, linux-iio, lucas.demarchi,
mwelling, linux, eibach
On 05/02/16 13:17, Daniel Baluta wrote:
> The driver has sysfs readings with runtime PM support for power saving.
> It also offers buffer support that can be used together with IIO software
> triggers.
>
> Datasheet can be found here:
> http://www.ti.com.cn/cn/lit/ds/symlink/ads1015.pdf
>
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Hmm. The existing hwmon driver device tree bindings are less than ideal!
(magic numbers left right and centre which always iritates me).
Ah well, perhaps we can later look at deprecating those (whilst maintaining
support) in favour of something a little more refined. I'm guessing that
binding has been around a while and we've all learnt more about device
tree bindings since then.
You need to be a little more careful around buffer / non buffered mode
state changes. mlock in the struct iio_dev should be taken whenever
you need to prevent a race in going in an out of buffered mode.
(here you need it round most of the raw_read function)
One trivial bit of english cleaning up in a comment.
Also would be ideal to have a resolution on Michael's issue as well
though it sounds like it is not an issue with this driver from his
debugging efforts.
Jonathan
> ---
> Changes since v3:
> * fixed type connectd -> connected
> * move mutex outside of switch in read_raw to be consistent
> with write_raw
>
> Changes since v2:
> * push locking out of the switch in write_raw
> * fix buf allocation in triggered buffer's handler
>
> Changes since v1:
> * addressed concerns about replacing the ads1015 hwmon driver
> * For the moment the IIO driver is compatible with hwmon
> driver (dt bindings) the only thing left is to also add
> support for ads1115.
> * DT bindings are described in Documentation/devicetree/
> bindings/hwmon/ads1015.txt. If needed will copy this file
> in a later patch to the IIO corresponding location.
> * addressed comments from Jonathan
> * added proper locking
> * added timestamp channel
> * removed magic constants
> * added some new lines to increase readability
> * used regmap_get_device instead of keeping a copy of
> i2c_client pointer.
> * used non-devm function to correct the cleanup order
>
> drivers/iio/adc/Kconfig | 13 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/ti-ads1015.c | 610 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 624 insertions(+)
> create mode 100644 drivers/iio/adc/ti-ads1015.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 60673b4..fad7e6a 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -370,6 +370,19 @@ config TI_ADC128S052
> This driver can also be built as a module. If so, the module will be
> called ti-adc128s052.
>
> +config TI_ADS1015
> + tristate "Texas Instruments ADS1015 ADC"
> + depends on I2C && !SENSORS_ADS1015
> + select REGMAP_I2C
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> + help
> + If you say yes here you get support for Texas Instruments ADS1015
> + ADC chip.
> +
> + This driver can also be built as a module. If so, the module will be
> + called ti-ads1015.
> +
> config TI_ADS8688
> tristate "Texas Instruments ADS8688"
> depends on SPI && OF
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index fb57e12..2ff70a3 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -36,6 +36,7 @@ obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
> obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
> obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
> obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
> +obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o
> obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
> obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
> obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o
> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
> new file mode 100644
> index 0000000..596335f
> --- /dev/null
> +++ b/drivers/iio/adc/ti-ads1015.c
> @@ -0,0 +1,610 @@
> +/*
> + * ADS1015 - Texas Instruments Analog-to-Digital Converter
> + *
> + * Copyright (c) 2016, Intel Corporation.
> + *
> + * This file is subject to the terms and conditions of version 2 of
> + * the GNU General Public License. See the file COPYING in the main
> + * directory of this archive for more details.
> + *
> + * IIO driver for ADS1015 ADC 7-bit I2C slave address:
> + * * 0x48 - ADDR connected to Ground
> + * * 0x49 - ADDR connected to Vdd
> + * * 0x4A - ADDR connected to SDA
> + * * 0x4B - ADDR connected to SCL
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/i2c.h>
> +#include <linux/regmap.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/mutex.h>
> +#include <linux/delay.h>
> +
> +#include <linux/i2c/ads1015.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/types.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +
> +#define ADS1015_DRV_NAME "ads1015"
> +
> +#define ADS1015_CONV_REG 0x00
> +#define ADS1015_CFG_REG 0x01
> +
> +#define ADS1015_CFG_DR_SHIFT 5
> +#define ADS1015_CFG_MOD_SHIFT 8
> +#define ADS1015_CFG_PGA_SHIFT 9
> +#define ADS1015_CFG_MUX_SHIFT 12
> +
> +#define ADS1015_CFG_DR_MASK GENMASK(7, 5)
> +#define ADS1015_CFG_MOD_MASK BIT(8)
> +#define ADS1015_CFG_PGA_MASK GENMASK(11, 9)
> +#define ADS1015_CFG_MUX_MASK GENMASK(14, 12)
> +
> +/* device operating modes */
> +#define ADS1015_CONTINUOUS 0
> +#define ADS1015_SINGLESHOT 1
> +
> +#define ADS1015_SLEEP_DELAY_MS 2000
> +#define ADS1015_DEFAULT_PGA 2
> +#define ADS1015_DEFAULT_DATA_RATE 4
> +#define ADS1015_DEFAULT_CHAN 0
> +
> +enum ads1015_channels {
> + ADS1015_AIN0_AIN1 = 0,
> + ADS1015_AIN0_AIN3,
> + ADS1015_AIN1_AIN3,
> + ADS1015_AIN2_AIN3,
> + ADS1015_AIN0,
> + ADS1015_AIN1,
> + ADS1015_AIN2,
> + ADS1015_AIN3,
> + ADS1015_TIMESTAMP,
> +};
> +
> +static const unsigned int ads1015_data_rate[] = {
> + 128, 250, 490, 920, 1600, 2400, 3300, 3300
> +};
> +
> +static const struct {
> + int scale;
> + int uscale;
> +} ads1015_scale[] = {
> + {3, 0},
> + {2, 0},
> + {1, 0},
> + {0, 500000},
> + {0, 250000},
> + {0, 125000},
> + {0, 125000},
> + {0, 125000},
> +};
> +
> +#define ADS1015_V_CHAN(_chan, _addr) { \
> + .type = IIO_VOLTAGE, \
> + .indexed = 1, \
> + .address = _addr, \
> + .channel = _chan, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_SCALE) | \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> + .scan_index = _addr, \
> + .scan_type = { \
> + .sign = 's', \
> + .realbits = 12, \
> + .storagebits = 16, \
> + .shift = 4, \
> + .endianness = IIO_CPU, \
> + }, \
> +}
> +
> +#define ADS1015_V_DIFF_CHAN(_chan, _chan2, _addr) { \
> + .type = IIO_VOLTAGE, \
> + .differential = 1, \
> + .indexed = 1, \
> + .address = _addr, \
> + .channel = _chan, \
> + .channel2 = _chan2, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_SCALE) | \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> + .scan_index = _addr, \
> + .scan_type = { \
> + .sign = 's', \
> + .realbits = 12, \
> + .storagebits = 16, \
> + .shift = 4, \
> + .endianness = IIO_CPU, \
> + }, \
> +}
> +
> +struct ads1015_data {
> + struct regmap *regmap;
> + /*
> + * Protects ADC ops, e.g: concurrent sysfs/buffered
> + * data reads, configuration updates
> + */
> + struct mutex lock;
> + struct ads1015_channel_data channel_data[ADS1015_CHANNELS];
> +};
> +
> +static bool ads1015_is_writeable_reg(struct device *dev, unsigned int reg)
> +{
> + return (reg == ADS1015_CFG_REG);
> +}
> +
> +static const struct regmap_config ads1015_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 16,
> + .max_register = ADS1015_CFG_REG,
> + .writeable_reg = ads1015_is_writeable_reg,
> +};
> +
> +static const struct iio_chan_spec ads1015_channels[] = {
> + ADS1015_V_DIFF_CHAN(0, 1, ADS1015_AIN0_AIN1),
> + ADS1015_V_DIFF_CHAN(0, 3, ADS1015_AIN0_AIN3),
> + ADS1015_V_DIFF_CHAN(1, 3, ADS1015_AIN1_AIN3),
> + ADS1015_V_DIFF_CHAN(2, 3, ADS1015_AIN2_AIN3),
> + ADS1015_V_CHAN(0, ADS1015_AIN0),
> + ADS1015_V_CHAN(1, ADS1015_AIN1),
> + ADS1015_V_CHAN(2, ADS1015_AIN2),
> + ADS1015_V_CHAN(3, ADS1015_AIN3),
> + IIO_CHAN_SOFT_TIMESTAMP(ADS1015_TIMESTAMP),
> +};
> +
> +static int ads1015_set_power_state(struct ads1015_data *data, bool on)
> +{
> + int ret;
> + struct device *dev = regmap_get_device(data->regmap);
> +
> + if (on) {
> + ret = pm_runtime_get_sync(dev);
> + if (ret < 0)
> + pm_runtime_put_noidle(dev);
> + } else {
> + pm_runtime_mark_last_busy(dev);
> + ret = pm_runtime_put_autosuspend(dev);
> + }
> +
> + return ret;
> +}
> +
> +static
> +int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val)
> +{
> + int ret, pga, dr, conv_time;
> + bool change;
> +
> + if (chan < 0 || chan >= ADS1015_CHANNELS)
> + return -EINVAL;
> +
> + pga = data->channel_data[chan].pga;
> + dr = data->channel_data[chan].data_rate;
> +
> + ret = regmap_update_bits_check(data->regmap, ADS1015_CFG_REG,
> + ADS1015_CFG_MUX_MASK |
> + ADS1015_CFG_PGA_MASK,
> + chan << ADS1015_CFG_MUX_SHIFT |
> + pga << ADS1015_CFG_PGA_SHIFT,
> + &change);
> + if (ret < 0)
> + return ret;
> +
> + if (change) {
> + conv_time = DIV_ROUND_UP(USEC_PER_SEC, ads1015_data_rate[dr]);
> + usleep_range(conv_time, conv_time + 1);
> + }
> +
> + return regmap_read(data->regmap, ADS1015_CONV_REG, val);
> +}
> +
> +static irqreturn_t ads1015_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct ads1015_data *data = iio_priv(indio_dev);
> + s16 buf[8]; /* 1x s16 ADC val + 3x s16 padding + 4x s16 timestamp */
> + int chan, ret, res;
> +
> + memset(buf, 0, sizeof(buf));
Hmm. I'd forgotten we should be doing this to avoid an information leak
in the unused parts of the buffer! Nice to be reminded of these things
from time to time!
> +
> + mutex_lock(&data->lock);
> + chan = find_first_bit(indio_dev->active_scan_mask,
> + indio_dev->masklength);
> + ret = ads1015_get_adc_result(data, chan, &res);
> + if (ret < 0) {
> + mutex_unlock(&data->lock);
> + goto err;
> + }
> +
> + buf[0] = res;
> + mutex_unlock(&data->lock);
> +
> + iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
> +
> +err:
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int ads1015_set_scale(struct ads1015_data *data, int chan,
> + int scale, int uscale)
> +{
> + int i, ret, rindex = -1;
> +
> + for (i = 0; i < ARRAY_SIZE(ads1015_scale); i++)
> + if (ads1015_scale[i].scale == scale &&
> + ads1015_scale[i].uscale == uscale) {
> + rindex = i;
> + break;
> + }
> + if (rindex < 0)
> + return -EINVAL;
> +
> + ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
> + ADS1015_CFG_PGA_MASK,
> + rindex << ADS1015_CFG_PGA_SHIFT);
> + if (ret < 0)
> + return ret;
> +
> + data->channel_data[chan].pga = rindex;
> +
> + return 0;
> +}
> +
> +static int ads1015_set_data_rate(struct ads1015_data *data, int chan, int rate)
> +{
> + int i, ret, rindex = -1;
> +
> + for (i = 0; i < ARRAY_SIZE(ads1015_data_rate); i++)
> + if (ads1015_data_rate[i] == rate) {
> + rindex = i;
> + break;
> + }
> + if (rindex < 0)
> + return -EINVAL;
> +
> + ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
> + ADS1015_CFG_DR_MASK,
> + rindex << ADS1015_CFG_DR_SHIFT);
> + if (ret < 0)
> + return ret;
> +
> + data->channel_data[chan].data_rate = rindex;
> +
> + return 0;
> +}
> +
> +static int ads1015_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int *val,
> + int *val2, long mask)
> +{
> + int ret, idx;
> + struct ads1015_data *data = iio_priv(indio_dev);
> +
> + mutex_lock(&data->lock);
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
This sanity check and related stuff should be done under iio_dev->mlock
as that's the lock used for buffer / non buffer state switches.
Your data lock will also be needed to prevent non core state change related
writes to the addresses. So leave that here as well.
> + if (iio_buffer_enabled(indio_dev)) {
> + ret = -EBUSY;
> + break;
> + }
> +
> + ret = ads1015_set_power_state(data, true);
> + if (ret < 0)
> + break;
> +
> + ret = ads1015_get_adc_result(data, chan->address, val);
> + if (ret < 0) {
> + ads1015_set_power_state(data, false);
> + break;
> + }
> +
> + /* 12 bit res, D0 is bit 4 in conversion register */
> + *val = sign_extend32(*val >> 4, 11);
> +
> + ret = ads1015_set_power_state(data, false);
> + if (ret < 0)
> + break;
> +
> + ret = IIO_VAL_INT;
> + break;
> + case IIO_CHAN_INFO_SCALE:
> + idx = data->channel_data[chan->address].pga;
> + *val = ads1015_scale[idx].scale;
> + *val2 = ads1015_scale[idx].uscale;
> + ret = IIO_VAL_INT_PLUS_MICRO;
> + break;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + idx = data->channel_data[chan->address].data_rate;
> + *val = ads1015_data_rate[idx];
> + ret = IIO_VAL_INT;
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> + mutex_unlock(&data->lock);
> +
> + return ret;
> +}
> +
> +static int ads1015_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int val,
> + int val2, long mask)
> +{
> + struct ads1015_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + mutex_lock(&data->lock);
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + ret = ads1015_set_scale(data, chan->address, val, val2);
> + break;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + ret = ads1015_set_data_rate(data, chan->address, val);
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> + mutex_unlock(&data->lock);
> +
> + return ret;
> +}
> +
> +static int ads1015_buffer_preenable(struct iio_dev *indio_dev)
> +{
> + return ads1015_set_power_state(iio_priv(indio_dev), true);
> +}
> +
> +static int ads1015_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> + return ads1015_set_power_state(iio_priv(indio_dev), false);
> +}
> +
> +static const struct iio_buffer_setup_ops ads1015_buffer_setup_ops = {
> + .preenable = ads1015_buffer_preenable,
> + .postenable = iio_triggered_buffer_postenable,
> + .predisable = iio_triggered_buffer_predisable,
> + .postdisable = ads1015_buffer_postdisable,
> + .validate_scan_mask = &iio_validate_scan_mask_onehot,
> +};
> +
> +static IIO_CONST_ATTR(scale_available, "3 2 1 0.5 0.25 0.125");
> +static IIO_CONST_ATTR(sampling_frequency_available,
> + "128 250 490 920 1600 2400 3300");
> +
> +static struct attribute *ads1015_attributes[] = {
> + &iio_const_attr_scale_available.dev_attr.attr,
> + &iio_const_attr_sampling_frequency_available.dev_attr.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group ads1015_attribute_group = {
> + .attrs = ads1015_attributes,
> +};
> +
> +static const struct iio_info ads1015_info = {
> + .driver_module = THIS_MODULE,
> + .read_raw = ads1015_read_raw,
> + .write_raw = ads1015_write_raw,
> + .attrs = &ads1015_attribute_group,
> +};
> +
> +#ifdef CONFIG_OF
> +static int ads1015_get_channels_config_of(struct i2c_client *client)
> +{
> + struct ads1015_data *data = i2c_get_clientdata(client);
> + struct device_node *node;
> +
> + if (!client->dev.of_node ||
> + !of_get_next_child(client->dev.of_node, NULL))
> + return -EINVAL;
> +
> + for_each_child_of_node(client->dev.of_node, node) {
> + u32 pval;
> + unsigned int channel;
> + unsigned int pga = ADS1015_DEFAULT_PGA;
> + unsigned int data_rate = ADS1015_DEFAULT_DATA_RATE;
> +
> + if (of_property_read_u32(node, "reg", &pval)) {
> + dev_err(&client->dev, "invalid reg on %s\n",
> + node->full_name);
> + continue;
> + }
> +
> + channel = pval;
> + if (channel >= ADS1015_CHANNELS) {
> + dev_err(&client->dev,
> + "invalid channel index %d on %s\n",
> + channel, node->full_name);
> + continue;
> + }
> +
> + if (!of_property_read_u32(node, "ti,gain", &pval)) {
> + pga = pval;
> + if (pga > 6) {
> + dev_err(&client->dev, "invalid gain on %s\n",
> + node->full_name);
> + return -EINVAL;
> + }
> + }
> +
> + if (!of_property_read_u32(node, "ti,datarate", &pval)) {
> + data_rate = pval;
> + if (data_rate > 7) {
> + dev_err(&client->dev,
> + "invalid data_rate on %s\n",
> + node->full_name);
> + return -EINVAL;
> + }
> + }
> +
> + data->channel_data[channel].pga = pga;
> + data->channel_data[channel].data_rate = data_rate;
> + }
> +
> + return 0;
> +}
> +#endif
> +
> +static void ads1015_get_channels_config(struct i2c_client *client)
> +{
> + unsigned int k;
> +
> + struct iio_dev *indio_dev = i2c_get_clientdata(client);
> + struct ads1015_data *data = iio_priv(indio_dev);
> + struct ads1015_platform_data *pdata = dev_get_platdata(&client->dev);
> +
> + /* prefer platform data */
> + if (pdata) {
> + memcpy(data->channel_data, pdata->channel_data,
> + sizeof(data->channel_data));
> + return;
> + }
> +
> +#ifdef CONFIG_OF
> + if (!ads1015_get_channels_config_of(client))
> + return;
> +#endif
> + /* fallback on default configuration */
> + for (k = 0; k < ADS1015_CHANNELS; ++k) {
> + data->channel_data[k].pga = ADS1015_DEFAULT_PGA;
> + data->channel_data[k].data_rate = ADS1015_DEFAULT_DATA_RATE;
> + }
> +}
> +
> +static int ads1015_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct iio_dev *indio_dev;
> + struct ads1015_data *data;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + data = iio_priv(indio_dev);
> + i2c_set_clientdata(client, indio_dev);
> +
> + mutex_init(&data->lock);
> +
> + indio_dev->dev.parent = &client->dev;
> + indio_dev->info = &ads1015_info;
> + indio_dev->name = ADS1015_DRV_NAME;
> + indio_dev->channels = ads1015_channels;
> + indio_dev->num_channels = ARRAY_SIZE(ads1015_channels);
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + /* we need this be keep the same ABI with hwmon ADS1015 driver */
We need to keep ths ABI the same as used by the hwmon ADS1015 driver.
> + ads1015_get_channels_config(client);
> +
> + data->regmap = devm_regmap_init_i2c(client, &ads1015_regmap_config);
> + if (IS_ERR(data->regmap)) {
> + dev_err(&client->dev, "Failed to allocate register map\n");
> + return PTR_ERR(data->regmap);
> + }
> +
> + ret = iio_triggered_buffer_setup(indio_dev, NULL,
> + ads1015_trigger_handler,
> + &ads1015_buffer_setup_ops);
> + if (ret < 0) {
> + dev_err(&client->dev, "iio triggered buffer setup failed\n");
> + return ret;
> + }
> + ret = pm_runtime_set_active(&client->dev);
> + if (ret)
> + goto err_buffer_cleanup;
> + pm_runtime_set_autosuspend_delay(&client->dev, ADS1015_SLEEP_DELAY_MS);
> + pm_runtime_use_autosuspend(&client->dev);
> + pm_runtime_enable(&client->dev);
> +
> + ret = iio_device_register(indio_dev);
> + if (ret < 0) {
> + dev_err(&client->dev, "Failed to register IIO device\n");
> + goto err_buffer_cleanup;
> + }
> +
> + return 0;
> +
> +err_buffer_cleanup:
> + iio_triggered_buffer_cleanup(indio_dev);
> +
> + return ret;
> +}
> +
> +static int ads1015_remove(struct i2c_client *client)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(client);
> + struct ads1015_data *data = iio_priv(indio_dev);
> +
> + iio_device_unregister(indio_dev);
> +
> + pm_runtime_disable(&client->dev);
> + pm_runtime_set_suspended(&client->dev);
> + pm_runtime_put_noidle(&client->dev);
> +
> + iio_triggered_buffer_cleanup(indio_dev);
> +
> + /* power down single shot mode */
> + return regmap_update_bits(data->regmap, ADS1015_CFG_REG,
> + ADS1015_CFG_MOD_MASK,
> + ADS1015_SINGLESHOT << ADS1015_CFG_MOD_SHIFT);
> +}
> +
> +#ifdef CONFIG_PM
> +static int ads1015_runtime_suspend(struct device *dev)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> + struct ads1015_data *data = iio_priv(indio_dev);
> +
> + return regmap_update_bits(data->regmap, ADS1015_CFG_REG,
> + ADS1015_CFG_MOD_MASK,
> + ADS1015_SINGLESHOT << ADS1015_CFG_MOD_SHIFT);
> +}
> +
> +static int ads1015_runtime_resume(struct device *dev)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> + struct ads1015_data *data = iio_priv(indio_dev);
> +
> + return regmap_update_bits(data->regmap, ADS1015_CFG_REG,
> + ADS1015_CFG_MOD_MASK,
> + ADS1015_CONTINUOUS << ADS1015_CFG_MOD_SHIFT);
> +}
> +#endif
> +
> +static const struct dev_pm_ops ads1015_pm_ops = {
> + SET_RUNTIME_PM_OPS(ads1015_runtime_suspend,
> + ads1015_runtime_resume, NULL)
> +};
> +
> +static const struct i2c_device_id ads1015_id[] = {
> + {"ads1015", 0},
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, ads1015_id);
> +
> +static struct i2c_driver ads1015_driver = {
> + .driver = {
> + .name = ADS1015_DRV_NAME,
> + .pm = &ads1015_pm_ops,
> + },
> + .probe = ads1015_probe,
> + .remove = ads1015_remove,
> + .id_table = ads1015_id,
> +};
> +
> +module_i2c_driver(ads1015_driver);
> +
> +MODULE_AUTHOR("Daniel Baluta <daniel.baluta@intel.com>");
> +MODULE_DESCRIPTION("Texas Instruments ADS1015 ADC driver");
> +MODULE_LICENSE("GPL v2");
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-06 11:05 ` Jonathan Cameron
@ 2016-02-06 11:07 ` Jonathan Cameron
0 siblings, 0 replies; 45+ messages in thread
From: Jonathan Cameron @ 2016-02-06 11:07 UTC (permalink / raw)
To: Daniel Baluta
Cc: knaack.h, lars, pmeerw, linux-kernel, linux-iio, lucas.demarchi,
mwelling, linux, eibach
On 06/02/16 11:05, Jonathan Cameron wrote:
> On 05/02/16 13:17, Daniel Baluta wrote:
>> The driver has sysfs readings with runtime PM support for power saving.
>> It also offers buffer support that can be used together with IIO software
>> triggers.
>>
>> Datasheet can be found here:
>> http://www.ti.com.cn/cn/lit/ds/symlink/ads1015.pdf
>>
>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> Hmm. The existing hwmon driver device tree bindings are less than ideal!
> (magic numbers left right and centre which always iritates me).
> Ah well, perhaps we can later look at deprecating those (whilst maintaining
> support) in favour of something a little more refined. I'm guessing that
> binding has been around a while and we've all learnt more about device
> tree bindings since then.
>
> You need to be a little more careful around buffer / non buffered mode
> state changes. mlock in the struct iio_dev should be taken whenever
> you need to prevent a race in going in an out of buffered mode.
> (here you need it round most of the raw_read function)
>
Ah, just noticed the docs in iio.h mark that lock as for core use only which
is wrong - it's used by both drivers and core.
Oops, patches welcome or I'll clean that up sometime.
> One trivial bit of english cleaning up in a comment.
>
> Also would be ideal to have a resolution on Michael's issue as well
> though it sounds like it is not an issue with this driver from his
> debugging efforts.
>
> Jonathan
>
>> ---
>> Changes since v3:
>> * fixed type connectd -> connected
>> * move mutex outside of switch in read_raw to be consistent
>> with write_raw
>>
>> Changes since v2:
>> * push locking out of the switch in write_raw
>> * fix buf allocation in triggered buffer's handler
>>
>> Changes since v1:
>> * addressed concerns about replacing the ads1015 hwmon driver
>> * For the moment the IIO driver is compatible with hwmon
>> driver (dt bindings) the only thing left is to also add
>> support for ads1115.
>> * DT bindings are described in Documentation/devicetree/
>> bindings/hwmon/ads1015.txt. If needed will copy this file
>> in a later patch to the IIO corresponding location.
>> * addressed comments from Jonathan
>> * added proper locking
>> * added timestamp channel
>> * removed magic constants
>> * added some new lines to increase readability
>> * used regmap_get_device instead of keeping a copy of
>> i2c_client pointer.
>> * used non-devm function to correct the cleanup order
>>
>> drivers/iio/adc/Kconfig | 13 +
>> drivers/iio/adc/Makefile | 1 +
>> drivers/iio/adc/ti-ads1015.c | 610 +++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 624 insertions(+)
>> create mode 100644 drivers/iio/adc/ti-ads1015.c
>>
>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>> index 60673b4..fad7e6a 100644
>> --- a/drivers/iio/adc/Kconfig
>> +++ b/drivers/iio/adc/Kconfig
>> @@ -370,6 +370,19 @@ config TI_ADC128S052
>> This driver can also be built as a module. If so, the module will be
>> called ti-adc128s052.
>>
>> +config TI_ADS1015
>> + tristate "Texas Instruments ADS1015 ADC"
>> + depends on I2C && !SENSORS_ADS1015
>> + select REGMAP_I2C
>> + select IIO_BUFFER
>> + select IIO_TRIGGERED_BUFFER
>> + help
>> + If you say yes here you get support for Texas Instruments ADS1015
>> + ADC chip.
>> +
>> + This driver can also be built as a module. If so, the module will be
>> + called ti-ads1015.
>> +
>> config TI_ADS8688
>> tristate "Texas Instruments ADS8688"
>> depends on SPI && OF
>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>> index fb57e12..2ff70a3 100644
>> --- a/drivers/iio/adc/Makefile
>> +++ b/drivers/iio/adc/Makefile
>> @@ -36,6 +36,7 @@ obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
>> obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
>> obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
>> obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
>> +obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o
>> obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
>> obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
>> obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o
>> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
>> new file mode 100644
>> index 0000000..596335f
>> --- /dev/null
>> +++ b/drivers/iio/adc/ti-ads1015.c
>> @@ -0,0 +1,610 @@
>> +/*
>> + * ADS1015 - Texas Instruments Analog-to-Digital Converter
>> + *
>> + * Copyright (c) 2016, Intel Corporation.
>> + *
>> + * This file is subject to the terms and conditions of version 2 of
>> + * the GNU General Public License. See the file COPYING in the main
>> + * directory of this archive for more details.
>> + *
>> + * IIO driver for ADS1015 ADC 7-bit I2C slave address:
>> + * * 0x48 - ADDR connected to Ground
>> + * * 0x49 - ADDR connected to Vdd
>> + * * 0x4A - ADDR connected to SDA
>> + * * 0x4B - ADDR connected to SCL
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/i2c.h>
>> +#include <linux/regmap.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/mutex.h>
>> +#include <linux/delay.h>
>> +
>> +#include <linux/i2c/ads1015.h>
>> +
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/types.h>
>> +#include <linux/iio/sysfs.h>
>> +#include <linux/iio/buffer.h>
>> +#include <linux/iio/triggered_buffer.h>
>> +#include <linux/iio/trigger_consumer.h>
>> +
>> +#define ADS1015_DRV_NAME "ads1015"
>> +
>> +#define ADS1015_CONV_REG 0x00
>> +#define ADS1015_CFG_REG 0x01
>> +
>> +#define ADS1015_CFG_DR_SHIFT 5
>> +#define ADS1015_CFG_MOD_SHIFT 8
>> +#define ADS1015_CFG_PGA_SHIFT 9
>> +#define ADS1015_CFG_MUX_SHIFT 12
>> +
>> +#define ADS1015_CFG_DR_MASK GENMASK(7, 5)
>> +#define ADS1015_CFG_MOD_MASK BIT(8)
>> +#define ADS1015_CFG_PGA_MASK GENMASK(11, 9)
>> +#define ADS1015_CFG_MUX_MASK GENMASK(14, 12)
>> +
>> +/* device operating modes */
>> +#define ADS1015_CONTINUOUS 0
>> +#define ADS1015_SINGLESHOT 1
>> +
>> +#define ADS1015_SLEEP_DELAY_MS 2000
>> +#define ADS1015_DEFAULT_PGA 2
>> +#define ADS1015_DEFAULT_DATA_RATE 4
>> +#define ADS1015_DEFAULT_CHAN 0
>> +
>> +enum ads1015_channels {
>> + ADS1015_AIN0_AIN1 = 0,
>> + ADS1015_AIN0_AIN3,
>> + ADS1015_AIN1_AIN3,
>> + ADS1015_AIN2_AIN3,
>> + ADS1015_AIN0,
>> + ADS1015_AIN1,
>> + ADS1015_AIN2,
>> + ADS1015_AIN3,
>> + ADS1015_TIMESTAMP,
>> +};
>> +
>> +static const unsigned int ads1015_data_rate[] = {
>> + 128, 250, 490, 920, 1600, 2400, 3300, 3300
>> +};
>> +
>> +static const struct {
>> + int scale;
>> + int uscale;
>> +} ads1015_scale[] = {
>> + {3, 0},
>> + {2, 0},
>> + {1, 0},
>> + {0, 500000},
>> + {0, 250000},
>> + {0, 125000},
>> + {0, 125000},
>> + {0, 125000},
>> +};
>> +
>> +#define ADS1015_V_CHAN(_chan, _addr) { \
>> + .type = IIO_VOLTAGE, \
>> + .indexed = 1, \
>> + .address = _addr, \
>> + .channel = _chan, \
>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
>> + BIT(IIO_CHAN_INFO_SCALE) | \
>> + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
>> + .scan_index = _addr, \
>> + .scan_type = { \
>> + .sign = 's', \
>> + .realbits = 12, \
>> + .storagebits = 16, \
>> + .shift = 4, \
>> + .endianness = IIO_CPU, \
>> + }, \
>> +}
>> +
>> +#define ADS1015_V_DIFF_CHAN(_chan, _chan2, _addr) { \
>> + .type = IIO_VOLTAGE, \
>> + .differential = 1, \
>> + .indexed = 1, \
>> + .address = _addr, \
>> + .channel = _chan, \
>> + .channel2 = _chan2, \
>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
>> + BIT(IIO_CHAN_INFO_SCALE) | \
>> + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
>> + .scan_index = _addr, \
>> + .scan_type = { \
>> + .sign = 's', \
>> + .realbits = 12, \
>> + .storagebits = 16, \
>> + .shift = 4, \
>> + .endianness = IIO_CPU, \
>> + }, \
>> +}
>> +
>> +struct ads1015_data {
>> + struct regmap *regmap;
>> + /*
>> + * Protects ADC ops, e.g: concurrent sysfs/buffered
>> + * data reads, configuration updates
>> + */
>> + struct mutex lock;
>> + struct ads1015_channel_data channel_data[ADS1015_CHANNELS];
>> +};
>> +
>> +static bool ads1015_is_writeable_reg(struct device *dev, unsigned int reg)
>> +{
>> + return (reg == ADS1015_CFG_REG);
>> +}
>> +
>> +static const struct regmap_config ads1015_regmap_config = {
>> + .reg_bits = 8,
>> + .val_bits = 16,
>> + .max_register = ADS1015_CFG_REG,
>> + .writeable_reg = ads1015_is_writeable_reg,
>> +};
>> +
>> +static const struct iio_chan_spec ads1015_channels[] = {
>> + ADS1015_V_DIFF_CHAN(0, 1, ADS1015_AIN0_AIN1),
>> + ADS1015_V_DIFF_CHAN(0, 3, ADS1015_AIN0_AIN3),
>> + ADS1015_V_DIFF_CHAN(1, 3, ADS1015_AIN1_AIN3),
>> + ADS1015_V_DIFF_CHAN(2, 3, ADS1015_AIN2_AIN3),
>> + ADS1015_V_CHAN(0, ADS1015_AIN0),
>> + ADS1015_V_CHAN(1, ADS1015_AIN1),
>> + ADS1015_V_CHAN(2, ADS1015_AIN2),
>> + ADS1015_V_CHAN(3, ADS1015_AIN3),
>> + IIO_CHAN_SOFT_TIMESTAMP(ADS1015_TIMESTAMP),
>> +};
>> +
>> +static int ads1015_set_power_state(struct ads1015_data *data, bool on)
>> +{
>> + int ret;
>> + struct device *dev = regmap_get_device(data->regmap);
>> +
>> + if (on) {
>> + ret = pm_runtime_get_sync(dev);
>> + if (ret < 0)
>> + pm_runtime_put_noidle(dev);
>> + } else {
>> + pm_runtime_mark_last_busy(dev);
>> + ret = pm_runtime_put_autosuspend(dev);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static
>> +int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val)
>> +{
>> + int ret, pga, dr, conv_time;
>> + bool change;
>> +
>> + if (chan < 0 || chan >= ADS1015_CHANNELS)
>> + return -EINVAL;
>> +
>> + pga = data->channel_data[chan].pga;
>> + dr = data->channel_data[chan].data_rate;
>> +
>> + ret = regmap_update_bits_check(data->regmap, ADS1015_CFG_REG,
>> + ADS1015_CFG_MUX_MASK |
>> + ADS1015_CFG_PGA_MASK,
>> + chan << ADS1015_CFG_MUX_SHIFT |
>> + pga << ADS1015_CFG_PGA_SHIFT,
>> + &change);
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (change) {
>> + conv_time = DIV_ROUND_UP(USEC_PER_SEC, ads1015_data_rate[dr]);
>> + usleep_range(conv_time, conv_time + 1);
>> + }
>> +
>> + return regmap_read(data->regmap, ADS1015_CONV_REG, val);
>> +}
>> +
>> +static irqreturn_t ads1015_trigger_handler(int irq, void *p)
>> +{
>> + struct iio_poll_func *pf = p;
>> + struct iio_dev *indio_dev = pf->indio_dev;
>> + struct ads1015_data *data = iio_priv(indio_dev);
>> + s16 buf[8]; /* 1x s16 ADC val + 3x s16 padding + 4x s16 timestamp */
>> + int chan, ret, res;
>> +
>> + memset(buf, 0, sizeof(buf));
> Hmm. I'd forgotten we should be doing this to avoid an information leak
> in the unused parts of the buffer! Nice to be reminded of these things
> from time to time!
>> +
>> + mutex_lock(&data->lock);
>> + chan = find_first_bit(indio_dev->active_scan_mask,
>> + indio_dev->masklength);
>> + ret = ads1015_get_adc_result(data, chan, &res);
>> + if (ret < 0) {
>> + mutex_unlock(&data->lock);
>> + goto err;
>> + }
>> +
>> + buf[0] = res;
>> + mutex_unlock(&data->lock);
>> +
>> + iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
>> +
>> +err:
>> + iio_trigger_notify_done(indio_dev->trig);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int ads1015_set_scale(struct ads1015_data *data, int chan,
>> + int scale, int uscale)
>> +{
>> + int i, ret, rindex = -1;
>> +
>> + for (i = 0; i < ARRAY_SIZE(ads1015_scale); i++)
>> + if (ads1015_scale[i].scale == scale &&
>> + ads1015_scale[i].uscale == uscale) {
>> + rindex = i;
>> + break;
>> + }
>> + if (rindex < 0)
>> + return -EINVAL;
>> +
>> + ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
>> + ADS1015_CFG_PGA_MASK,
>> + rindex << ADS1015_CFG_PGA_SHIFT);
>> + if (ret < 0)
>> + return ret;
>> +
>> + data->channel_data[chan].pga = rindex;
>> +
>> + return 0;
>> +}
>> +
>> +static int ads1015_set_data_rate(struct ads1015_data *data, int chan, int rate)
>> +{
>> + int i, ret, rindex = -1;
>> +
>> + for (i = 0; i < ARRAY_SIZE(ads1015_data_rate); i++)
>> + if (ads1015_data_rate[i] == rate) {
>> + rindex = i;
>> + break;
>> + }
>> + if (rindex < 0)
>> + return -EINVAL;
>> +
>> + ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
>> + ADS1015_CFG_DR_MASK,
>> + rindex << ADS1015_CFG_DR_SHIFT);
>> + if (ret < 0)
>> + return ret;
>> +
>> + data->channel_data[chan].data_rate = rindex;
>> +
>> + return 0;
>> +}
>> +
>> +static int ads1015_read_raw(struct iio_dev *indio_dev,
>> + struct iio_chan_spec const *chan, int *val,
>> + int *val2, long mask)
>> +{
>> + int ret, idx;
>> + struct ads1015_data *data = iio_priv(indio_dev);
>> +
>> + mutex_lock(&data->lock);
>> + switch (mask) {
>> + case IIO_CHAN_INFO_RAW:
>
> This sanity check and related stuff should be done under iio_dev->mlock
> as that's the lock used for buffer / non buffer state switches.
>
> Your data lock will also be needed to prevent non core state change related
> writes to the addresses. So leave that here as well.
>> + if (iio_buffer_enabled(indio_dev)) {
>> + ret = -EBUSY;
>> + break;
>> + }
>> +
>> + ret = ads1015_set_power_state(data, true);
>> + if (ret < 0)
>> + break;
>> +
>> + ret = ads1015_get_adc_result(data, chan->address, val);
>> + if (ret < 0) {
>> + ads1015_set_power_state(data, false);
>> + break;
>> + }
>> +
>> + /* 12 bit res, D0 is bit 4 in conversion register */
>> + *val = sign_extend32(*val >> 4, 11);
>> +
>> + ret = ads1015_set_power_state(data, false);
>> + if (ret < 0)
>> + break;
>> +
>> + ret = IIO_VAL_INT;
>> + break;
>> + case IIO_CHAN_INFO_SCALE:
>> + idx = data->channel_data[chan->address].pga;
>> + *val = ads1015_scale[idx].scale;
>> + *val2 = ads1015_scale[idx].uscale;
>> + ret = IIO_VAL_INT_PLUS_MICRO;
>> + break;
>> + case IIO_CHAN_INFO_SAMP_FREQ:
>> + idx = data->channel_data[chan->address].data_rate;
>> + *val = ads1015_data_rate[idx];
>> + ret = IIO_VAL_INT;
>> + break;
>> + default:
>> + ret = -EINVAL;
>> + break;
>> + }
>> + mutex_unlock(&data->lock);
>> +
>> + return ret;
>> +}
>> +
>> +static int ads1015_write_raw(struct iio_dev *indio_dev,
>> + struct iio_chan_spec const *chan, int val,
>> + int val2, long mask)
>> +{
>> + struct ads1015_data *data = iio_priv(indio_dev);
>> + int ret;
>> +
>> + mutex_lock(&data->lock);
>> + switch (mask) {
>> + case IIO_CHAN_INFO_SCALE:
>> + ret = ads1015_set_scale(data, chan->address, val, val2);
>> + break;
>> + case IIO_CHAN_INFO_SAMP_FREQ:
>> + ret = ads1015_set_data_rate(data, chan->address, val);
>> + break;
>> + default:
>> + ret = -EINVAL;
>> + break;
>> + }
>> + mutex_unlock(&data->lock);
>> +
>> + return ret;
>> +}
>> +
>> +static int ads1015_buffer_preenable(struct iio_dev *indio_dev)
>> +{
>> + return ads1015_set_power_state(iio_priv(indio_dev), true);
>> +}
>> +
>> +static int ads1015_buffer_postdisable(struct iio_dev *indio_dev)
>> +{
>> + return ads1015_set_power_state(iio_priv(indio_dev), false);
>> +}
>> +
>> +static const struct iio_buffer_setup_ops ads1015_buffer_setup_ops = {
>> + .preenable = ads1015_buffer_preenable,
>> + .postenable = iio_triggered_buffer_postenable,
>> + .predisable = iio_triggered_buffer_predisable,
>> + .postdisable = ads1015_buffer_postdisable,
>> + .validate_scan_mask = &iio_validate_scan_mask_onehot,
>> +};
>> +
>> +static IIO_CONST_ATTR(scale_available, "3 2 1 0.5 0.25 0.125");
>> +static IIO_CONST_ATTR(sampling_frequency_available,
>> + "128 250 490 920 1600 2400 3300");
>> +
>> +static struct attribute *ads1015_attributes[] = {
>> + &iio_const_attr_scale_available.dev_attr.attr,
>> + &iio_const_attr_sampling_frequency_available.dev_attr.attr,
>> + NULL,
>> +};
>> +
>> +static const struct attribute_group ads1015_attribute_group = {
>> + .attrs = ads1015_attributes,
>> +};
>> +
>> +static const struct iio_info ads1015_info = {
>> + .driver_module = THIS_MODULE,
>> + .read_raw = ads1015_read_raw,
>> + .write_raw = ads1015_write_raw,
>> + .attrs = &ads1015_attribute_group,
>> +};
>> +
>> +#ifdef CONFIG_OF
>> +static int ads1015_get_channels_config_of(struct i2c_client *client)
>> +{
>> + struct ads1015_data *data = i2c_get_clientdata(client);
>> + struct device_node *node;
>> +
>> + if (!client->dev.of_node ||
>> + !of_get_next_child(client->dev.of_node, NULL))
>> + return -EINVAL;
>> +
>> + for_each_child_of_node(client->dev.of_node, node) {
>> + u32 pval;
>> + unsigned int channel;
>> + unsigned int pga = ADS1015_DEFAULT_PGA;
>> + unsigned int data_rate = ADS1015_DEFAULT_DATA_RATE;
>> +
>> + if (of_property_read_u32(node, "reg", &pval)) {
>> + dev_err(&client->dev, "invalid reg on %s\n",
>> + node->full_name);
>> + continue;
>> + }
>> +
>> + channel = pval;
>> + if (channel >= ADS1015_CHANNELS) {
>> + dev_err(&client->dev,
>> + "invalid channel index %d on %s\n",
>> + channel, node->full_name);
>> + continue;
>> + }
>> +
>> + if (!of_property_read_u32(node, "ti,gain", &pval)) {
>> + pga = pval;
>> + if (pga > 6) {
>> + dev_err(&client->dev, "invalid gain on %s\n",
>> + node->full_name);
>> + return -EINVAL;
>> + }
>> + }
>> +
>> + if (!of_property_read_u32(node, "ti,datarate", &pval)) {
>> + data_rate = pval;
>> + if (data_rate > 7) {
>> + dev_err(&client->dev,
>> + "invalid data_rate on %s\n",
>> + node->full_name);
>> + return -EINVAL;
>> + }
>> + }
>> +
>> + data->channel_data[channel].pga = pga;
>> + data->channel_data[channel].data_rate = data_rate;
>> + }
>> +
>> + return 0;
>> +}
>> +#endif
>> +
>> +static void ads1015_get_channels_config(struct i2c_client *client)
>> +{
>> + unsigned int k;
>> +
>> + struct iio_dev *indio_dev = i2c_get_clientdata(client);
>> + struct ads1015_data *data = iio_priv(indio_dev);
>> + struct ads1015_platform_data *pdata = dev_get_platdata(&client->dev);
>> +
>> + /* prefer platform data */
>> + if (pdata) {
>> + memcpy(data->channel_data, pdata->channel_data,
>> + sizeof(data->channel_data));
>> + return;
>> + }
>> +
>> +#ifdef CONFIG_OF
>> + if (!ads1015_get_channels_config_of(client))
>> + return;
>> +#endif
>> + /* fallback on default configuration */
>> + for (k = 0; k < ADS1015_CHANNELS; ++k) {
>> + data->channel_data[k].pga = ADS1015_DEFAULT_PGA;
>> + data->channel_data[k].data_rate = ADS1015_DEFAULT_DATA_RATE;
>> + }
>> +}
>> +
>> +static int ads1015_probe(struct i2c_client *client,
>> + const struct i2c_device_id *id)
>> +{
>> + struct iio_dev *indio_dev;
>> + struct ads1015_data *data;
>> + int ret;
>> +
>> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>> + if (!indio_dev)
>> + return -ENOMEM;
>> +
>> + data = iio_priv(indio_dev);
>> + i2c_set_clientdata(client, indio_dev);
>> +
>> + mutex_init(&data->lock);
>> +
>> + indio_dev->dev.parent = &client->dev;
>> + indio_dev->info = &ads1015_info;
>> + indio_dev->name = ADS1015_DRV_NAME;
>> + indio_dev->channels = ads1015_channels;
>> + indio_dev->num_channels = ARRAY_SIZE(ads1015_channels);
>> + indio_dev->modes = INDIO_DIRECT_MODE;
>> +
>> + /* we need this be keep the same ABI with hwmon ADS1015 driver */
> We need to keep ths ABI the same as used by the hwmon ADS1015 driver.
>> + ads1015_get_channels_config(client);
>> +
>> + data->regmap = devm_regmap_init_i2c(client, &ads1015_regmap_config);
>> + if (IS_ERR(data->regmap)) {
>> + dev_err(&client->dev, "Failed to allocate register map\n");
>> + return PTR_ERR(data->regmap);
>> + }
>> +
>> + ret = iio_triggered_buffer_setup(indio_dev, NULL,
>> + ads1015_trigger_handler,
>> + &ads1015_buffer_setup_ops);
>> + if (ret < 0) {
>> + dev_err(&client->dev, "iio triggered buffer setup failed\n");
>> + return ret;
>> + }
>> + ret = pm_runtime_set_active(&client->dev);
>> + if (ret)
>> + goto err_buffer_cleanup;
>> + pm_runtime_set_autosuspend_delay(&client->dev, ADS1015_SLEEP_DELAY_MS);
>> + pm_runtime_use_autosuspend(&client->dev);
>> + pm_runtime_enable(&client->dev);
>> +
>> + ret = iio_device_register(indio_dev);
>> + if (ret < 0) {
>> + dev_err(&client->dev, "Failed to register IIO device\n");
>> + goto err_buffer_cleanup;
>> + }
>> +
>> + return 0;
>> +
>> +err_buffer_cleanup:
>> + iio_triggered_buffer_cleanup(indio_dev);
>> +
>> + return ret;
>> +}
>> +
>> +static int ads1015_remove(struct i2c_client *client)
>> +{
>> + struct iio_dev *indio_dev = i2c_get_clientdata(client);
>> + struct ads1015_data *data = iio_priv(indio_dev);
>> +
>> + iio_device_unregister(indio_dev);
>> +
>> + pm_runtime_disable(&client->dev);
>> + pm_runtime_set_suspended(&client->dev);
>> + pm_runtime_put_noidle(&client->dev);
>> +
>> + iio_triggered_buffer_cleanup(indio_dev);
>> +
>> + /* power down single shot mode */
>> + return regmap_update_bits(data->regmap, ADS1015_CFG_REG,
>> + ADS1015_CFG_MOD_MASK,
>> + ADS1015_SINGLESHOT << ADS1015_CFG_MOD_SHIFT);
>> +}
>> +
>> +#ifdef CONFIG_PM
>> +static int ads1015_runtime_suspend(struct device *dev)
>> +{
>> + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
>> + struct ads1015_data *data = iio_priv(indio_dev);
>> +
>> + return regmap_update_bits(data->regmap, ADS1015_CFG_REG,
>> + ADS1015_CFG_MOD_MASK,
>> + ADS1015_SINGLESHOT << ADS1015_CFG_MOD_SHIFT);
>> +}
>> +
>> +static int ads1015_runtime_resume(struct device *dev)
>> +{
>> + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
>> + struct ads1015_data *data = iio_priv(indio_dev);
>> +
>> + return regmap_update_bits(data->regmap, ADS1015_CFG_REG,
>> + ADS1015_CFG_MOD_MASK,
>> + ADS1015_CONTINUOUS << ADS1015_CFG_MOD_SHIFT);
>> +}
>> +#endif
>> +
>> +static const struct dev_pm_ops ads1015_pm_ops = {
>> + SET_RUNTIME_PM_OPS(ads1015_runtime_suspend,
>> + ads1015_runtime_resume, NULL)
>> +};
>> +
>> +static const struct i2c_device_id ads1015_id[] = {
>> + {"ads1015", 0},
>> + {}
>> +};
>> +MODULE_DEVICE_TABLE(i2c, ads1015_id);
>> +
>> +static struct i2c_driver ads1015_driver = {
>> + .driver = {
>> + .name = ADS1015_DRV_NAME,
>> + .pm = &ads1015_pm_ops,
>> + },
>> + .probe = ads1015_probe,
>> + .remove = ads1015_remove,
>> + .id_table = ads1015_id,
>> +};
>> +
>> +module_i2c_driver(ads1015_driver);
>> +
>> +MODULE_AUTHOR("Daniel Baluta <daniel.baluta@intel.com>");
>> +MODULE_DESCRIPTION("Texas Instruments ADS1015 ADC driver");
>> +MODULE_LICENSE("GPL v2");
>>
>
> --
> 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] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-06 0:32 ` Michael Welling
@ 2016-02-08 10:25 ` Wolfram Sang
2016-02-08 14:44 ` Daniel Baluta
2016-02-08 16:36 ` Michael Welling
0 siblings, 2 replies; 45+ messages in thread
From: Wolfram Sang @ 2016-02-08 10:25 UTC (permalink / raw)
To: Michael Welling
Cc: Daniel Baluta, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler,
Linux Kernel Mailing List, linux-iio, Lucas De Marchi,
Andy Gross, Pramod Gurav, Bjorn Andersson, Guenter Roeck, eibach,
Sricharan R, linux-arm-msm
[-- Attachment #1: Type: text/plain, Size: 2100 bytes --]
On Fri, Feb 05, 2016 at 06:32:45PM -0600, Michael Welling wrote:
> On Fri, Feb 05, 2016 at 09:32:34PM +0200, Daniel Baluta wrote:
> > >> +static int ads1015_read_raw(struct iio_dev *indio_dev,
> > >> + struct iio_chan_spec const *chan, int *val,
> > >> + int *val2, long mask)
> > >> +{
> > >> + int ret, idx;
> > >> + struct ads1015_data *data = iio_priv(indio_dev);
> > >> +
> > >> + mutex_lock(&data->lock);
> > >> + switch (mask) {
> > >> + case IIO_CHAN_INFO_RAW:
> > >> + if (iio_buffer_enabled(indio_dev)) {
> > >> + ret = -EBUSY;
> > >> + break;
> > >> + }
> > >> +
> > >> + ret = ads1015_set_power_state(data, true);
> > >> + if (ret < 0)
> > >> + break;
> > >
> > > Just tested the driver on a Dragonboard 410C with a robotics mezzanine that I
> > > designed.
> > >
> > > The above ads1015_set_power_state(data, true) is always returning -EINVAL.
> > >
> > > Any ideas why that would be happening?
> > > I think it may be the return from pm_runtime_get_sync?
> >
> > Can you confirm that pm_runtime_get_sync fails? Using some printk?
> >
> > Also adding printks in suspend/resume function would be helpful. Do
> > you have CONFIG_PM enabled?
> >
>
> Indeed it is the pm_runtime_get_sync that fails with a -EINVAL.
>
> > >
> > > When I comment out the break the readings come back but are not updated continually.
> > > If I read in_voltage0-voltage1_raw then in_voltage0_raw the value is updated.
> >
> > I guess this is normal if set_power_state fails.
>
> The hwmod driver works fine BTW.
>
> My guess is there is an issue with the qup i2c driver seeing as it has worked on
> other system without issue.
>
> CC'd some the latest developer on the qup i2c driver.
>
> I2C guys have any ideas on this?
>
Adding some more people who recently worked on this. Might be nice to
know which kernel version you are using.
> >
> > thanks,
> > Daniel.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-08 10:25 ` Wolfram Sang
@ 2016-02-08 14:44 ` Daniel Baluta
[not found] ` <CAEnQRZAQjhQRdzYUvo=aKgQScAyGUgp8Ni7nx9cEo1XhN_8Xyw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-02-08 16:37 ` Michael Welling
2016-02-08 16:36 ` Michael Welling
1 sibling, 2 replies; 45+ messages in thread
From: Daniel Baluta @ 2016-02-08 14:44 UTC (permalink / raw)
To: Wolfram Sang
Cc: Michael Welling, Daniel Baluta, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler,
Linux Kernel Mailing List, linux-iio, Lucas De Marchi,
Andy Gross, Pramod Gurav, Bjorn Andersson, Guenter Roeck, eibach,
Sricharan R, linux-arm-msm
On Mon, Feb 8, 2016 at 12:25 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> On Fri, Feb 05, 2016 at 06:32:45PM -0600, Michael Welling wrote:
>> On Fri, Feb 05, 2016 at 09:32:34PM +0200, Daniel Baluta wrote:
>> > >> +static int ads1015_read_raw(struct iio_dev *indio_dev,
>> > >> + struct iio_chan_spec const *chan, int *val,
>> > >> + int *val2, long mask)
>> > >> +{
>> > >> + int ret, idx;
>> > >> + struct ads1015_data *data = iio_priv(indio_dev);
>> > >> +
>> > >> + mutex_lock(&data->lock);
>> > >> + switch (mask) {
>> > >> + case IIO_CHAN_INFO_RAW:
>> > >> + if (iio_buffer_enabled(indio_dev)) {
>> > >> + ret = -EBUSY;
>> > >> + break;
>> > >> + }
>> > >> +
>> > >> + ret = ads1015_set_power_state(data, true);
>> > >> + if (ret < 0)
>> > >> + break;
>> > >
>> > > Just tested the driver on a Dragonboard 410C with a robotics mezzanine that I
>> > > designed.
>> > >
>> > > The above ads1015_set_power_state(data, true) is always returning -EINVAL.
>> > >
>> > > Any ideas why that would be happening?
>> > > I think it may be the return from pm_runtime_get_sync?
>> >
>> > Can you confirm that pm_runtime_get_sync fails? Using some printk?
>> >
>> > Also adding printks in suspend/resume function would be helpful. Do
>> > you have CONFIG_PM enabled?
>> >
>>
>> Indeed it is the pm_runtime_get_sync that fails with a -EINVAL.
Can you check the output of:
$ cat /sys/bus/iio/devices/iio:device0/power/runtime_status
* after insmod
* after a reading from sysfs
>>
>> > >
>> > > When I comment out the break the readings come back but are not updated continually.
>> > > If I read in_voltage0-voltage1_raw then in_voltage0_raw the value is updated.
>> >
>> > I guess this is normal if set_power_state fails.
>>
>> The hwmod driver works fine BTW.
>>
>> My guess is there is an issue with the qup i2c driver seeing as it has worked on
>> other system without issue.
>>
>> CC'd some the latest developer on the qup i2c driver.
>>
>> I2C guys have any ideas on this?
>>
>
> Adding some more people who recently worked on this. Might be nice to
> know which kernel version you are using.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-08 14:44 ` Daniel Baluta
@ 2016-02-08 15:03 ` Daniel Baluta
2016-02-08 16:37 ` Michael Welling
1 sibling, 0 replies; 45+ messages in thread
From: Daniel Baluta @ 2016-02-08 15:03 UTC (permalink / raw)
To: Daniel Baluta
Cc: Wolfram Sang, Michael Welling, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler,
Linux Kernel Mailing List, linux-iio-u79uwXL29TY76Z2rM5mHXA,
Lucas De Marchi, Andy Gross, Pramod Gurav, Bjorn Andersson,
Guenter Roeck, eibach-dJ+jgKLZIg4, Sricharan R,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
On Mon, Feb 8, 2016 at 4:44 PM, Daniel Baluta <daniel.baluta-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> On Mon, Feb 8, 2016 at 12:25 PM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
>> On Fri, Feb 05, 2016 at 06:32:45PM -0600, Michael Welling wrote:
>>> On Fri, Feb 05, 2016 at 09:32:34PM +0200, Daniel Baluta wrote:
>>> > >> +static int ads1015_read_raw(struct iio_dev *indio_dev,
>>> > >> + struct iio_chan_spec const *chan, int *val,
>>> > >> + int *val2, long mask)
>>> > >> +{
>>> > >> + int ret, idx;
>>> > >> + struct ads1015_data *data = iio_priv(indio_dev);
>>> > >> +
>>> > >> + mutex_lock(&data->lock);
>>> > >> + switch (mask) {
>>> > >> + case IIO_CHAN_INFO_RAW:
>>> > >> + if (iio_buffer_enabled(indio_dev)) {
>>> > >> + ret = -EBUSY;
>>> > >> + break;
>>> > >> + }
>>> > >> +
>>> > >> + ret = ads1015_set_power_state(data, true);
>>> > >> + if (ret < 0)
>>> > >> + break;
>>> > >
>>> > > Just tested the driver on a Dragonboard 410C with a robotics mezzanine that I
>>> > > designed.
>>> > >
>>> > > The above ads1015_set_power_state(data, true) is always returning -EINVAL.
>>> > >
>>> > > Any ideas why that would be happening?
>>> > > I think it may be the return from pm_runtime_get_sync?
>>> >
>>> > Can you confirm that pm_runtime_get_sync fails? Using some printk?
>>> >
>>> > Also adding printks in suspend/resume function would be helpful. Do
>>> > you have CONFIG_PM enabled?
>>> >
>>>
>>> Indeed it is the pm_runtime_get_sync that fails with a -EINVAL.
>
> Can you check the output of:
> $ cat /sys/bus/iio/devices/iio:device0/power/runtime_status
>
> * after insmod
> * after a reading from sysfs
>
>>>
>>> > >
>>> > > When I comment out the break the readings come back but are not updated continually.
>>> > > If I read in_voltage0-voltage1_raw then in_voltage0_raw the value is updated.
>>> >
>>> > I guess this is normal if set_power_state fails.
>>>
>>> The hwmod driver works fine BTW.
>>>
>>> My guess is there is an issue with the qup i2c driver seeing as it has worked on
>>> other system without issue.
>>>
>>> CC'd some the latest developer on the qup i2c driver.
>>>
>>> I2C guys have any ideas on this?
>>>
>>
>> Adding some more people who recently worked on this. Might be nice to
>> know which kernel version you are using.
Jonathan,
I will send an update version with your comments addressed once
we sort out this runtime pm issue.
Michael,
If you questions we can discuss on #linux-iio IRC channel
under irc.oftc.net server.
thanks,
Daniel.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
@ 2016-02-08 15:03 ` Daniel Baluta
0 siblings, 0 replies; 45+ messages in thread
From: Daniel Baluta @ 2016-02-08 15:03 UTC (permalink / raw)
To: Daniel Baluta
Cc: Wolfram Sang, Michael Welling, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler,
Linux Kernel Mailing List, linux-iio, Lucas De Marchi,
Andy Gross, Pramod Gurav, Bjorn Andersson, Guenter Roeck, eibach,
Sricharan R, linux-arm-msm
On Mon, Feb 8, 2016 at 4:44 PM, Daniel Baluta <daniel.baluta@intel.com> wrote:
> On Mon, Feb 8, 2016 at 12:25 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> On Fri, Feb 05, 2016 at 06:32:45PM -0600, Michael Welling wrote:
>>> On Fri, Feb 05, 2016 at 09:32:34PM +0200, Daniel Baluta wrote:
>>> > >> +static int ads1015_read_raw(struct iio_dev *indio_dev,
>>> > >> + struct iio_chan_spec const *chan, int *val,
>>> > >> + int *val2, long mask)
>>> > >> +{
>>> > >> + int ret, idx;
>>> > >> + struct ads1015_data *data = iio_priv(indio_dev);
>>> > >> +
>>> > >> + mutex_lock(&data->lock);
>>> > >> + switch (mask) {
>>> > >> + case IIO_CHAN_INFO_RAW:
>>> > >> + if (iio_buffer_enabled(indio_dev)) {
>>> > >> + ret = -EBUSY;
>>> > >> + break;
>>> > >> + }
>>> > >> +
>>> > >> + ret = ads1015_set_power_state(data, true);
>>> > >> + if (ret < 0)
>>> > >> + break;
>>> > >
>>> > > Just tested the driver on a Dragonboard 410C with a robotics mezzanine that I
>>> > > designed.
>>> > >
>>> > > The above ads1015_set_power_state(data, true) is always returning -EINVAL.
>>> > >
>>> > > Any ideas why that would be happening?
>>> > > I think it may be the return from pm_runtime_get_sync?
>>> >
>>> > Can you confirm that pm_runtime_get_sync fails? Using some printk?
>>> >
>>> > Also adding printks in suspend/resume function would be helpful. Do
>>> > you have CONFIG_PM enabled?
>>> >
>>>
>>> Indeed it is the pm_runtime_get_sync that fails with a -EINVAL.
>
> Can you check the output of:
> $ cat /sys/bus/iio/devices/iio:device0/power/runtime_status
>
> * after insmod
> * after a reading from sysfs
>
>>>
>>> > >
>>> > > When I comment out the break the readings come back but are not updated continually.
>>> > > If I read in_voltage0-voltage1_raw then in_voltage0_raw the value is updated.
>>> >
>>> > I guess this is normal if set_power_state fails.
>>>
>>> The hwmod driver works fine BTW.
>>>
>>> My guess is there is an issue with the qup i2c driver seeing as it has worked on
>>> other system without issue.
>>>
>>> CC'd some the latest developer on the qup i2c driver.
>>>
>>> I2C guys have any ideas on this?
>>>
>>
>> Adding some more people who recently worked on this. Might be nice to
>> know which kernel version you are using.
Jonathan,
I will send an update version with your comments addressed once
we sort out this runtime pm issue.
Michael,
If you questions we can discuss on #linux-iio IRC channel
under irc.oftc.net server.
thanks,
Daniel.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-08 10:25 ` Wolfram Sang
@ 2016-02-08 16:36 ` Michael Welling
2016-02-08 16:36 ` Michael Welling
1 sibling, 0 replies; 45+ messages in thread
From: Michael Welling @ 2016-02-08 16:36 UTC (permalink / raw)
To: Wolfram Sang
Cc: Daniel Baluta, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler,
Linux Kernel Mailing List, linux-iio-u79uwXL29TY76Z2rM5mHXA,
Lucas De Marchi, Andy Gross, Pramod Gurav, Bjorn Andersson,
Guenter Roeck, eibach-dJ+jgKLZIg4, Sricharan R,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
On Mon, Feb 08, 2016 at 11:25:00AM +0100, Wolfram Sang wrote:
> On Fri, Feb 05, 2016 at 06:32:45PM -0600, Michael Welling wrote:
> > On Fri, Feb 05, 2016 at 09:32:34PM +0200, Daniel Baluta wrote:
> > > >> +static int ads1015_read_raw(struct iio_dev *indio_dev,
> > > >> + struct iio_chan_spec const *chan, int *val,
> > > >> + int *val2, long mask)
> > > >> +{
> > > >> + int ret, idx;
> > > >> + struct ads1015_data *data = iio_priv(indio_dev);
> > > >> +
> > > >> + mutex_lock(&data->lock);
> > > >> + switch (mask) {
> > > >> + case IIO_CHAN_INFO_RAW:
> > > >> + if (iio_buffer_enabled(indio_dev)) {
> > > >> + ret = -EBUSY;
> > > >> + break;
> > > >> + }
> > > >> +
> > > >> + ret = ads1015_set_power_state(data, true);
> > > >> + if (ret < 0)
> > > >> + break;
> > > >
> > > > Just tested the driver on a Dragonboard 410C with a robotics mezzanine that I
> > > > designed.
> > > >
> > > > The above ads1015_set_power_state(data, true) is always returning -EINVAL.
> > > >
> > > > Any ideas why that would be happening?
> > > > I think it may be the return from pm_runtime_get_sync?
> > >
> > > Can you confirm that pm_runtime_get_sync fails? Using some printk?
> > >
> > > Also adding printks in suspend/resume function would be helpful. Do
> > > you have CONFIG_PM enabled?
> > >
> >
> > Indeed it is the pm_runtime_get_sync that fails with a -EINVAL.
> >
> > > >
> > > > When I comment out the break the readings come back but are not updated continually.
> > > > If I read in_voltage0-voltage1_raw then in_voltage0_raw the value is updated.
> > >
> > > I guess this is normal if set_power_state fails.
> >
> > The hwmod driver works fine BTW.
> >
> > My guess is there is an issue with the qup i2c driver seeing as it has worked on
> > other system without issue.
> >
> > CC'd some the latest developer on the qup i2c driver.
> >
> > I2C guys have any ideas on this?
> >
>
> Adding some more people who recently worked on this. Might be nice to
> know which kernel version you are using.
>
4.5.0-rc2
> > >
> > > thanks,
> > > Daniel.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
@ 2016-02-08 16:36 ` Michael Welling
0 siblings, 0 replies; 45+ messages in thread
From: Michael Welling @ 2016-02-08 16:36 UTC (permalink / raw)
To: Wolfram Sang
Cc: Daniel Baluta, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler,
Linux Kernel Mailing List, linux-iio, Lucas De Marchi,
Andy Gross, Pramod Gurav, Bjorn Andersson, Guenter Roeck, eibach,
Sricharan R, linux-arm-msm
On Mon, Feb 08, 2016 at 11:25:00AM +0100, Wolfram Sang wrote:
> On Fri, Feb 05, 2016 at 06:32:45PM -0600, Michael Welling wrote:
> > On Fri, Feb 05, 2016 at 09:32:34PM +0200, Daniel Baluta wrote:
> > > >> +static int ads1015_read_raw(struct iio_dev *indio_dev,
> > > >> + struct iio_chan_spec const *chan, int *val,
> > > >> + int *val2, long mask)
> > > >> +{
> > > >> + int ret, idx;
> > > >> + struct ads1015_data *data = iio_priv(indio_dev);
> > > >> +
> > > >> + mutex_lock(&data->lock);
> > > >> + switch (mask) {
> > > >> + case IIO_CHAN_INFO_RAW:
> > > >> + if (iio_buffer_enabled(indio_dev)) {
> > > >> + ret = -EBUSY;
> > > >> + break;
> > > >> + }
> > > >> +
> > > >> + ret = ads1015_set_power_state(data, true);
> > > >> + if (ret < 0)
> > > >> + break;
> > > >
> > > > Just tested the driver on a Dragonboard 410C with a robotics mezzanine that I
> > > > designed.
> > > >
> > > > The above ads1015_set_power_state(data, true) is always returning -EINVAL.
> > > >
> > > > Any ideas why that would be happening?
> > > > I think it may be the return from pm_runtime_get_sync?
> > >
> > > Can you confirm that pm_runtime_get_sync fails? Using some printk?
> > >
> > > Also adding printks in suspend/resume function would be helpful. Do
> > > you have CONFIG_PM enabled?
> > >
> >
> > Indeed it is the pm_runtime_get_sync that fails with a -EINVAL.
> >
> > > >
> > > > When I comment out the break the readings come back but are not updated continually.
> > > > If I read in_voltage0-voltage1_raw then in_voltage0_raw the value is updated.
> > >
> > > I guess this is normal if set_power_state fails.
> >
> > The hwmod driver works fine BTW.
> >
> > My guess is there is an issue with the qup i2c driver seeing as it has worked on
> > other system without issue.
> >
> > CC'd some the latest developer on the qup i2c driver.
> >
> > I2C guys have any ideas on this?
> >
>
> Adding some more people who recently worked on this. Might be nice to
> know which kernel version you are using.
>
4.5.0-rc2
> > >
> > > thanks,
> > > Daniel.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-08 14:44 ` Daniel Baluta
[not found] ` <CAEnQRZAQjhQRdzYUvo=aKgQScAyGUgp8Ni7nx9cEo1XhN_8Xyw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2016-02-08 16:37 ` Michael Welling
1 sibling, 0 replies; 45+ messages in thread
From: Michael Welling @ 2016-02-08 16:37 UTC (permalink / raw)
To: Daniel Baluta
Cc: Wolfram Sang, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler,
Linux Kernel Mailing List, linux-iio, Lucas De Marchi,
Andy Gross, Pramod Gurav, Bjorn Andersson, Guenter Roeck, eibach,
Sricharan R, linux-arm-msm
On Mon, Feb 08, 2016 at 04:44:02PM +0200, Daniel Baluta wrote:
> On Mon, Feb 8, 2016 at 12:25 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> > On Fri, Feb 05, 2016 at 06:32:45PM -0600, Michael Welling wrote:
> >> On Fri, Feb 05, 2016 at 09:32:34PM +0200, Daniel Baluta wrote:
> >> > >> +static int ads1015_read_raw(struct iio_dev *indio_dev,
> >> > >> + struct iio_chan_spec const *chan, int *val,
> >> > >> + int *val2, long mask)
> >> > >> +{
> >> > >> + int ret, idx;
> >> > >> + struct ads1015_data *data = iio_priv(indio_dev);
> >> > >> +
> >> > >> + mutex_lock(&data->lock);
> >> > >> + switch (mask) {
> >> > >> + case IIO_CHAN_INFO_RAW:
> >> > >> + if (iio_buffer_enabled(indio_dev)) {
> >> > >> + ret = -EBUSY;
> >> > >> + break;
> >> > >> + }
> >> > >> +
> >> > >> + ret = ads1015_set_power_state(data, true);
> >> > >> + if (ret < 0)
> >> > >> + break;
> >> > >
> >> > > Just tested the driver on a Dragonboard 410C with a robotics mezzanine that I
> >> > > designed.
> >> > >
> >> > > The above ads1015_set_power_state(data, true) is always returning -EINVAL.
> >> > >
> >> > > Any ideas why that would be happening?
> >> > > I think it may be the return from pm_runtime_get_sync?
> >> >
> >> > Can you confirm that pm_runtime_get_sync fails? Using some printk?
> >> >
> >> > Also adding printks in suspend/resume function would be helpful. Do
> >> > you have CONFIG_PM enabled?
> >> >
> >>
> >> Indeed it is the pm_runtime_get_sync that fails with a -EINVAL.
>
> Can you check the output of:
> $ cat /sys/bus/iio/devices/iio:device0/power/runtime_status
>
> * after insmod
root@dragonboard-410c:~# cat /sys/bus/iio/devices/iio:device0/power/runtime_status
unsupported
> * after a reading from sysfs
root@dragonboard-410c:~# cat /sys/bus/iio/devices/iio:device0/power/runtime_status
unsupported
>
> >>
> >> > >
> >> > > When I comment out the break the readings come back but are not updated continually.
> >> > > If I read in_voltage0-voltage1_raw then in_voltage0_raw the value is updated.
> >> >
> >> > I guess this is normal if set_power_state fails.
> >>
> >> The hwmod driver works fine BTW.
> >>
> >> My guess is there is an issue with the qup i2c driver seeing as it has worked on
> >> other system without issue.
> >>
> >> CC'd some the latest developer on the qup i2c driver.
> >>
> >> I2C guys have any ideas on this?
> >>
> >
> > Adding some more people who recently worked on this. Might be nice to
> > know which kernel version you are using.
^ permalink raw reply [flat|nested] 45+ messages in thread
* RE: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-08 16:36 ` Michael Welling
@ 2016-02-08 19:11 ` Sricharan
-1 siblings, 0 replies; 45+ messages in thread
From: Sricharan @ 2016-02-08 19:11 UTC (permalink / raw)
To: 'Michael Welling', 'Wolfram Sang'
Cc: 'Daniel Baluta', 'Jonathan Cameron',
'Hartmut Knaack', 'Lars-Peter Clausen',
'Peter Meerwald-Stadler',
'Linux Kernel Mailing List',
linux-iio, 'Lucas De Marchi', 'Andy Gross',
'Pramod Gurav', 'Bjorn Andersson',
'Guenter Roeck',
eibach, linux-arm-msm
Hi,
> -----Original Message-----
> From: Michael Welling [mailto:mwelling79@gmail.com] On Behalf Of Michael
> Welling
> Sent: Monday, February 08, 2016 10:07 PM
> To: Wolfram Sang
> Cc: Daniel Baluta; Jonathan Cameron; Hartmut Knaack; Lars-Peter Clausen;
> Peter Meerwald-Stadler; Linux Kernel Mailing List;
linux-iio@vger.kernel.org;
> Lucas De Marchi; Andy Gross; Pramod Gurav; Bjorn Andersson; Guenter
> Roeck; eibach@gdsys.de; Sricharan R; linux-arm-msm@vger.kernel.org
> Subject: Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
>
> On Mon, Feb 08, 2016 at 11:25:00AM +0100, Wolfram Sang wrote:
> > On Fri, Feb 05, 2016 at 06:32:45PM -0600, Michael Welling wrote:
> > > On Fri, Feb 05, 2016 at 09:32:34PM +0200, Daniel Baluta wrote:
> > > > >> +static int ads1015_read_raw(struct iio_dev *indio_dev,
> > > > >> + struct iio_chan_spec const *chan, int
*val,
> > > > >> + int *val2, long mask) {
> > > > >> + int ret, idx;
> > > > >> + struct ads1015_data *data = iio_priv(indio_dev);
> > > > >> +
> > > > >> + mutex_lock(&data->lock);
> > > > >> + switch (mask) {
> > > > >> + case IIO_CHAN_INFO_RAW:
> > > > >> + if (iio_buffer_enabled(indio_dev)) {
> > > > >> + ret = -EBUSY;
> > > > >> + break;
> > > > >> + }
> > > > >> +
> > > > >> + ret = ads1015_set_power_state(data, true);
> > > > >> + if (ret < 0)
> > > > >> + break;
> > > > >
> > > > > Just tested the driver on a Dragonboard 410C with a robotics
> > > > > mezzanine that I designed.
> > > > >
> > > > > The above ads1015_set_power_state(data, true) is always returning
-
> EINVAL.
> > > > >
> > > > > Any ideas why that would be happening?
> > > > > I think it may be the return from pm_runtime_get_sync?
> > > >
> > > > Can you confirm that pm_runtime_get_sync fails? Using some printk?
> > > >
> > > > Also adding printks in suspend/resume function would be helpful.
> > > > Do you have CONFIG_PM enabled?
> > > >
> > >
> > > Indeed it is the pm_runtime_get_sync that fails with a -EINVAL.
> > >
> > > > >
> > > > > When I comment out the break the readings come back but are not
> updated continually.
> > > > > If I read in_voltage0-voltage1_raw then in_voltage0_raw the value
is
> updated.
> > > >
> > > > I guess this is normal if set_power_state fails.
> > >
> > > The hwmod driver works fine BTW.
> > >
> > > My guess is there is an issue with the qup i2c driver seeing as it
> > > has worked on other system without issue.
> > >
> > > CC'd some the latest developer on the qup i2c driver.
> > >
> > > I2C guys have any ideas on this?
> > >
> >
> > Adding some more people who recently worked on this. Might be nice to
> > know which kernel version you are using.
> >
Which i2c bus is this connected to ? I can give a try with 410c to see why
pm_runtime_get_sync from qup
fails.
Regards,
Sricharan
^ permalink raw reply [flat|nested] 45+ messages in thread
* RE: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
@ 2016-02-08 19:11 ` Sricharan
0 siblings, 0 replies; 45+ messages in thread
From: Sricharan @ 2016-02-08 19:11 UTC (permalink / raw)
To: 'Michael Welling', 'Wolfram Sang'
Cc: 'Daniel Baluta', 'Jonathan Cameron',
'Hartmut Knaack', 'Lars-Peter Clausen',
'Peter Meerwald-Stadler',
'Linux Kernel Mailing List',
linux-iio, 'Lucas De Marchi', 'Andy Gross',
'Pramod Gurav', 'Bjorn Andersson',
'Guenter Roeck',
eibach, linux-arm-msm
Hi,
> -----Original Message-----
> From: Michael Welling [mailto:mwelling79@gmail.com] On Behalf Of Michael
> Welling
> Sent: Monday, February 08, 2016 10:07 PM
> To: Wolfram Sang
> Cc: Daniel Baluta; Jonathan Cameron; Hartmut Knaack; Lars-Peter Clausen;
> Peter Meerwald-Stadler; Linux Kernel Mailing List;
linux-iio@vger.kernel.org;
> Lucas De Marchi; Andy Gross; Pramod Gurav; Bjorn Andersson; Guenter
> Roeck; eibach@gdsys.de; Sricharan R; linux-arm-msm@vger.kernel.org
> Subject: Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
>
> On Mon, Feb 08, 2016 at 11:25:00AM +0100, Wolfram Sang wrote:
> > On Fri, Feb 05, 2016 at 06:32:45PM -0600, Michael Welling wrote:
> > > On Fri, Feb 05, 2016 at 09:32:34PM +0200, Daniel Baluta wrote:
> > > > >> +static int ads1015_read_raw(struct iio_dev *indio_dev,
> > > > >> + struct iio_chan_spec const *chan, int
*val,
> > > > >> + int *val2, long mask) {
> > > > >> + int ret, idx;
> > > > >> + struct ads1015_data *data = iio_priv(indio_dev);
> > > > >> +
> > > > >> + mutex_lock(&data->lock);
> > > > >> + switch (mask) {
> > > > >> + case IIO_CHAN_INFO_RAW:
> > > > >> + if (iio_buffer_enabled(indio_dev)) {
> > > > >> + ret = -EBUSY;
> > > > >> + break;
> > > > >> + }
> > > > >> +
> > > > >> + ret = ads1015_set_power_state(data, true);
> > > > >> + if (ret < 0)
> > > > >> + break;
> > > > >
> > > > > Just tested the driver on a Dragonboard 410C with a robotics
> > > > > mezzanine that I designed.
> > > > >
> > > > > The above ads1015_set_power_state(data, true) is always returning
-
> EINVAL.
> > > > >
> > > > > Any ideas why that would be happening?
> > > > > I think it may be the return from pm_runtime_get_sync?
> > > >
> > > > Can you confirm that pm_runtime_get_sync fails? Using some printk?
> > > >
> > > > Also adding printks in suspend/resume function would be helpful.
> > > > Do you have CONFIG_PM enabled?
> > > >
> > >
> > > Indeed it is the pm_runtime_get_sync that fails with a -EINVAL.
> > >
> > > > >
> > > > > When I comment out the break the readings come back but are not
> updated continually.
> > > > > If I read in_voltage0-voltage1_raw then in_voltage0_raw the value
is
> updated.
> > > >
> > > > I guess this is normal if set_power_state fails.
> > >
> > > The hwmod driver works fine BTW.
> > >
> > > My guess is there is an issue with the qup i2c driver seeing as it
> > > has worked on other system without issue.
> > >
> > > CC'd some the latest developer on the qup i2c driver.
> > >
> > > I2C guys have any ideas on this?
> > >
> >
> > Adding some more people who recently worked on this. Might be nice to
> > know which kernel version you are using.
> >
Which i2c bus is this connected to ? I can give a try with 410c to see why
pm_runtime_get_sync from qup
fails.
Regards,
Sricharan
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-08 19:11 ` Sricharan
@ 2016-02-08 19:16 ` Michael Welling
-1 siblings, 0 replies; 45+ messages in thread
From: Michael Welling @ 2016-02-08 19:16 UTC (permalink / raw)
To: Sricharan
Cc: 'Wolfram Sang', 'Daniel Baluta',
'Jonathan Cameron', 'Hartmut Knaack',
'Lars-Peter Clausen', 'Peter Meerwald-Stadler',
'Linux Kernel Mailing List',
linux-iio-u79uwXL29TY76Z2rM5mHXA, 'Lucas De Marchi',
'Andy Gross', 'Pramod Gurav',
'Bjorn Andersson', 'Guenter Roeck',
eibach-dJ+jgKLZIg4, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
On Tue, Feb 09, 2016 at 12:41:35AM +0530, Sricharan wrote:
> Hi,
>
> > -----Original Message-----
> > From: Michael Welling [mailto:mwelling79-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org] On Behalf Of Michael
> > Welling
> > Sent: Monday, February 08, 2016 10:07 PM
> > To: Wolfram Sang
> > Cc: Daniel Baluta; Jonathan Cameron; Hartmut Knaack; Lars-Peter Clausen;
> > Peter Meerwald-Stadler; Linux Kernel Mailing List;
> linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> > Lucas De Marchi; Andy Gross; Pramod Gurav; Bjorn Andersson; Guenter
> > Roeck; eibach-dJ+jgKLZIg4@public.gmane.org; Sricharan R; linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Subject: Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
> >
> > On Mon, Feb 08, 2016 at 11:25:00AM +0100, Wolfram Sang wrote:
> > > On Fri, Feb 05, 2016 at 06:32:45PM -0600, Michael Welling wrote:
> > > > On Fri, Feb 05, 2016 at 09:32:34PM +0200, Daniel Baluta wrote:
> > > > > >> +static int ads1015_read_raw(struct iio_dev *indio_dev,
> > > > > >> + struct iio_chan_spec const *chan, int
> *val,
> > > > > >> + int *val2, long mask) {
> > > > > >> + int ret, idx;
> > > > > >> + struct ads1015_data *data = iio_priv(indio_dev);
> > > > > >> +
> > > > > >> + mutex_lock(&data->lock);
> > > > > >> + switch (mask) {
> > > > > >> + case IIO_CHAN_INFO_RAW:
> > > > > >> + if (iio_buffer_enabled(indio_dev)) {
> > > > > >> + ret = -EBUSY;
> > > > > >> + break;
> > > > > >> + }
> > > > > >> +
> > > > > >> + ret = ads1015_set_power_state(data, true);
> > > > > >> + if (ret < 0)
> > > > > >> + break;
> > > > > >
> > > > > > Just tested the driver on a Dragonboard 410C with a robotics
> > > > > > mezzanine that I designed.
> > > > > >
> > > > > > The above ads1015_set_power_state(data, true) is always returning
> -
> > EINVAL.
> > > > > >
> > > > > > Any ideas why that would be happening?
> > > > > > I think it may be the return from pm_runtime_get_sync?
> > > > >
> > > > > Can you confirm that pm_runtime_get_sync fails? Using some printk?
> > > > >
> > > > > Also adding printks in suspend/resume function would be helpful.
> > > > > Do you have CONFIG_PM enabled?
> > > > >
> > > >
> > > > Indeed it is the pm_runtime_get_sync that fails with a -EINVAL.
> > > >
> > > > > >
> > > > > > When I comment out the break the readings come back but are not
> > updated continually.
> > > > > > If I read in_voltage0-voltage1_raw then in_voltage0_raw the value
> is
> > updated.
> > > > >
> > > > > I guess this is normal if set_power_state fails.
> > > >
> > > > The hwmod driver works fine BTW.
> > > >
> > > > My guess is there is an issue with the qup i2c driver seeing as it
> > > > has worked on other system without issue.
> > > >
> > > > CC'd some the latest developer on the qup i2c driver.
> > > >
> > > > I2C guys have any ideas on this?
> > > >
> > >
> > > Adding some more people who recently worked on this. Might be nice to
> > > know which kernel version you are using.
> > >
> Which i2c bus is this connected to ? I can give a try with 410c to see why
> pm_runtime_get_sync from qup
> fails.
It is on the lowspeed header. Here is my devicetree entry:
i2c@78b6000 {
/* On Low speed expansion */
label = "LS-I2C0";
status = "okay";
pca: pca@40 {
compatible = "nxp,pca9685-pwm";
#pwm-cells = <2>;
reg = <0x40>;
};
adc: adc@48 {
compatible = "ti,ads1015";
reg = <0x48>;
};
};
>
> Regards,
> Sricharan
>
>
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
@ 2016-02-08 19:16 ` Michael Welling
0 siblings, 0 replies; 45+ messages in thread
From: Michael Welling @ 2016-02-08 19:16 UTC (permalink / raw)
To: Sricharan
Cc: 'Wolfram Sang', 'Daniel Baluta',
'Jonathan Cameron', 'Hartmut Knaack',
'Lars-Peter Clausen', 'Peter Meerwald-Stadler',
'Linux Kernel Mailing List',
linux-iio, 'Lucas De Marchi', 'Andy Gross',
'Pramod Gurav', 'Bjorn Andersson',
'Guenter Roeck',
eibach, linux-arm-msm
On Tue, Feb 09, 2016 at 12:41:35AM +0530, Sricharan wrote:
> Hi,
>
> > -----Original Message-----
> > From: Michael Welling [mailto:mwelling79@gmail.com] On Behalf Of Michael
> > Welling
> > Sent: Monday, February 08, 2016 10:07 PM
> > To: Wolfram Sang
> > Cc: Daniel Baluta; Jonathan Cameron; Hartmut Knaack; Lars-Peter Clausen;
> > Peter Meerwald-Stadler; Linux Kernel Mailing List;
> linux-iio@vger.kernel.org;
> > Lucas De Marchi; Andy Gross; Pramod Gurav; Bjorn Andersson; Guenter
> > Roeck; eibach@gdsys.de; Sricharan R; linux-arm-msm@vger.kernel.org
> > Subject: Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
> >
> > On Mon, Feb 08, 2016 at 11:25:00AM +0100, Wolfram Sang wrote:
> > > On Fri, Feb 05, 2016 at 06:32:45PM -0600, Michael Welling wrote:
> > > > On Fri, Feb 05, 2016 at 09:32:34PM +0200, Daniel Baluta wrote:
> > > > > >> +static int ads1015_read_raw(struct iio_dev *indio_dev,
> > > > > >> + struct iio_chan_spec const *chan, int
> *val,
> > > > > >> + int *val2, long mask) {
> > > > > >> + int ret, idx;
> > > > > >> + struct ads1015_data *data = iio_priv(indio_dev);
> > > > > >> +
> > > > > >> + mutex_lock(&data->lock);
> > > > > >> + switch (mask) {
> > > > > >> + case IIO_CHAN_INFO_RAW:
> > > > > >> + if (iio_buffer_enabled(indio_dev)) {
> > > > > >> + ret = -EBUSY;
> > > > > >> + break;
> > > > > >> + }
> > > > > >> +
> > > > > >> + ret = ads1015_set_power_state(data, true);
> > > > > >> + if (ret < 0)
> > > > > >> + break;
> > > > > >
> > > > > > Just tested the driver on a Dragonboard 410C with a robotics
> > > > > > mezzanine that I designed.
> > > > > >
> > > > > > The above ads1015_set_power_state(data, true) is always returning
> -
> > EINVAL.
> > > > > >
> > > > > > Any ideas why that would be happening?
> > > > > > I think it may be the return from pm_runtime_get_sync?
> > > > >
> > > > > Can you confirm that pm_runtime_get_sync fails? Using some printk?
> > > > >
> > > > > Also adding printks in suspend/resume function would be helpful.
> > > > > Do you have CONFIG_PM enabled?
> > > > >
> > > >
> > > > Indeed it is the pm_runtime_get_sync that fails with a -EINVAL.
> > > >
> > > > > >
> > > > > > When I comment out the break the readings come back but are not
> > updated continually.
> > > > > > If I read in_voltage0-voltage1_raw then in_voltage0_raw the value
> is
> > updated.
> > > > >
> > > > > I guess this is normal if set_power_state fails.
> > > >
> > > > The hwmod driver works fine BTW.
> > > >
> > > > My guess is there is an issue with the qup i2c driver seeing as it
> > > > has worked on other system without issue.
> > > >
> > > > CC'd some the latest developer on the qup i2c driver.
> > > >
> > > > I2C guys have any ideas on this?
> > > >
> > >
> > > Adding some more people who recently worked on this. Might be nice to
> > > know which kernel version you are using.
> > >
> Which i2c bus is this connected to ? I can give a try with 410c to see why
> pm_runtime_get_sync from qup
> fails.
It is on the lowspeed header. Here is my devicetree entry:
i2c@78b6000 {
/* On Low speed expansion */
label = "LS-I2C0";
status = "okay";
pca: pca@40 {
compatible = "nxp,pca9685-pwm";
#pwm-cells = <2>;
reg = <0x40>;
};
adc: adc@48 {
compatible = "ti,ads1015";
reg = <0x48>;
};
};
>
> Regards,
> Sricharan
>
>
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-08 19:16 ` Michael Welling
@ 2016-02-08 20:00 ` Wolfram Sang
-1 siblings, 0 replies; 45+ messages in thread
From: Wolfram Sang @ 2016-02-08 20:00 UTC (permalink / raw)
To: Michael Welling
Cc: Sricharan, 'Daniel Baluta', 'Jonathan Cameron',
'Hartmut Knaack', 'Lars-Peter Clausen',
'Peter Meerwald-Stadler',
'Linux Kernel Mailing List',
linux-iio-u79uwXL29TY76Z2rM5mHXA, 'Lucas De Marchi',
'Andy Gross', 'Pramod Gurav',
'Bjorn Andersson', 'Guenter Roeck',
eibach-dJ+jgKLZIg4, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: Type: text/plain, Size: 680 bytes --]
> > > On Mon, Feb 08, 2016 at 11:25:00AM +0100, Wolfram Sang wrote:
> > > > On Fri, Feb 05, 2016 at 06:32:45PM -0600, Michael Welling wrote:
> > > > > On Fri, Feb 05, 2016 at 09:32:34PM +0200, Daniel Baluta wrote:
> > > > > > >> +static int ads1015_read_raw(struct iio_dev *indio_dev,
> > > > > > >> + struct iio_chan_spec const *chan, int
Guys, please quote only relevant sections.
> pca: pca@40 {
> compatible = "nxp,pca9685-pwm";
> #pwm-cells = <2>;
> reg = <0x40>;
> };
Does accessing the pca work?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
@ 2016-02-08 20:00 ` Wolfram Sang
0 siblings, 0 replies; 45+ messages in thread
From: Wolfram Sang @ 2016-02-08 20:00 UTC (permalink / raw)
To: Michael Welling
Cc: Sricharan, 'Daniel Baluta', 'Jonathan Cameron',
'Hartmut Knaack', 'Lars-Peter Clausen',
'Peter Meerwald-Stadler',
'Linux Kernel Mailing List',
linux-iio, 'Lucas De Marchi', 'Andy Gross',
'Pramod Gurav', 'Bjorn Andersson',
'Guenter Roeck',
eibach, linux-arm-msm
[-- Attachment #1: Type: text/plain, Size: 680 bytes --]
> > > On Mon, Feb 08, 2016 at 11:25:00AM +0100, Wolfram Sang wrote:
> > > > On Fri, Feb 05, 2016 at 06:32:45PM -0600, Michael Welling wrote:
> > > > > On Fri, Feb 05, 2016 at 09:32:34PM +0200, Daniel Baluta wrote:
> > > > > > >> +static int ads1015_read_raw(struct iio_dev *indio_dev,
> > > > > > >> + struct iio_chan_spec const *chan, int
Guys, please quote only relevant sections.
> pca: pca@40 {
> compatible = "nxp,pca9685-pwm";
> #pwm-cells = <2>;
> reg = <0x40>;
> };
Does accessing the pca work?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-08 20:00 ` Wolfram Sang
@ 2016-02-08 20:06 ` Michael Welling
-1 siblings, 0 replies; 45+ messages in thread
From: Michael Welling @ 2016-02-08 20:06 UTC (permalink / raw)
To: Wolfram Sang
Cc: Sricharan, 'Daniel Baluta', 'Jonathan Cameron',
'Hartmut Knaack', 'Lars-Peter Clausen',
'Peter Meerwald-Stadler',
'Linux Kernel Mailing List',
linux-iio-u79uwXL29TY76Z2rM5mHXA, 'Lucas De Marchi',
'Andy Gross', 'Pramod Gurav',
'Bjorn Andersson', 'Guenter Roeck',
eibach-dJ+jgKLZIg4, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
On Mon, Feb 08, 2016 at 09:00:27PM +0100, Wolfram Sang wrote:
> > > > On Mon, Feb 08, 2016 at 11:25:00AM +0100, Wolfram Sang wrote:
> > > > > On Fri, Feb 05, 2016 at 06:32:45PM -0600, Michael Welling wrote:
> > > > > > On Fri, Feb 05, 2016 at 09:32:34PM +0200, Daniel Baluta wrote:
> > > > > > > >> +static int ads1015_read_raw(struct iio_dev *indio_dev,
> > > > > > > >> + struct iio_chan_spec const *chan, int
>
> Guys, please quote only relevant sections.
>
Sorry.
> > pca: pca@40 {
> > compatible = "nxp,pca9685-pwm";
> > #pwm-cells = <2>;
> > reg = <0x40>;
> > };
>
> Does accessing the pca work?
>
Yes.
https://plus.google.com/+MichaelWelling79/posts/PdmGysBfZp4
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
@ 2016-02-08 20:06 ` Michael Welling
0 siblings, 0 replies; 45+ messages in thread
From: Michael Welling @ 2016-02-08 20:06 UTC (permalink / raw)
To: Wolfram Sang
Cc: Sricharan, 'Daniel Baluta', 'Jonathan Cameron',
'Hartmut Knaack', 'Lars-Peter Clausen',
'Peter Meerwald-Stadler',
'Linux Kernel Mailing List',
linux-iio, 'Lucas De Marchi', 'Andy Gross',
'Pramod Gurav', 'Bjorn Andersson',
'Guenter Roeck',
eibach, linux-arm-msm
On Mon, Feb 08, 2016 at 09:00:27PM +0100, Wolfram Sang wrote:
> > > > On Mon, Feb 08, 2016 at 11:25:00AM +0100, Wolfram Sang wrote:
> > > > > On Fri, Feb 05, 2016 at 06:32:45PM -0600, Michael Welling wrote:
> > > > > > On Fri, Feb 05, 2016 at 09:32:34PM +0200, Daniel Baluta wrote:
> > > > > > > >> +static int ads1015_read_raw(struct iio_dev *indio_dev,
> > > > > > > >> + struct iio_chan_spec const *chan, int
>
> Guys, please quote only relevant sections.
>
Sorry.
> > pca: pca@40 {
> > compatible = "nxp,pca9685-pwm";
> > #pwm-cells = <2>;
> > reg = <0x40>;
> > };
>
> Does accessing the pca work?
>
Yes.
https://plus.google.com/+MichaelWelling79/posts/PdmGysBfZp4
^ permalink raw reply [flat|nested] 45+ messages in thread
* RE: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-08 19:16 ` Michael Welling
@ 2016-02-10 12:22 ` Sricharan
-1 siblings, 0 replies; 45+ messages in thread
From: Sricharan @ 2016-02-10 12:22 UTC (permalink / raw)
To: 'Michael Welling'
Cc: 'Wolfram Sang', 'Daniel Baluta',
'Jonathan Cameron', 'Hartmut Knaack',
'Lars-Peter Clausen', 'Peter Meerwald-Stadler',
'Linux Kernel Mailing List',
linux-iio, 'Lucas De Marchi', 'Andy Gross',
'Pramod Gurav', 'Bjorn Andersson',
'Guenter Roeck',
eibach, linux-arm-msm
Hi,
> -----Original Message-----
> From: linux-arm-msm-owner@vger.kernel.org [mailto:linux-arm-msm-
> owner@vger.kernel.org] On Behalf Of Michael Welling
> Sent: Tuesday, February 09, 2016 12:47 AM
> To: Sricharan
> Cc: 'Wolfram Sang'; 'Daniel Baluta'; 'Jonathan Cameron'; 'Hartmut Knaack';
> 'Lars-Peter Clausen'; 'Peter Meerwald-Stadler'; 'Linux Kernel Mailing
List';
> linux-iio@vger.kernel.org; 'Lucas De Marchi'; 'Andy Gross'; 'Pramod
Gurav';
> 'Bjorn Andersson'; 'Guenter Roeck'; eibach@gdsys.de; linux-arm-
> msm@vger.kernel.org
> Subject: Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
>
> On Tue, Feb 09, 2016 at 12:41:35AM +0530, Sricharan wrote:
> > Hi,
> >
> > > -----Original Message-----
> > > From: Michael Welling [mailto:mwelling79@gmail.com] On Behalf Of
> > > Michael Welling
> > > Sent: Monday, February 08, 2016 10:07 PM
> > > To: Wolfram Sang
> > > Cc: Daniel Baluta; Jonathan Cameron; Hartmut Knaack; Lars-Peter
> > > Clausen; Peter Meerwald-Stadler; Linux Kernel Mailing List;
> > linux-iio@vger.kernel.org;
> > > Lucas De Marchi; Andy Gross; Pramod Gurav; Bjorn Andersson; Guenter
> > > Roeck; eibach@gdsys.de; Sricharan R; linux-arm-msm@vger.kernel.org
> > > Subject: Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
> > >
> > > On Mon, Feb 08, 2016 at 11:25:00AM +0100, Wolfram Sang wrote:
> > > > On Fri, Feb 05, 2016 at 06:32:45PM -0600, Michael Welling wrote:
> > > > > On Fri, Feb 05, 2016 at 09:32:34PM +0200, Daniel Baluta wrote:
> > > > > > >> +static int ads1015_read_raw(struct iio_dev *indio_dev,
> > > > > > >> + struct iio_chan_spec const *chan,
> > > > > > >> +int
> > *val,
> > > > > > >> + int *val2, long mask) {
> > > > > > >> + int ret, idx;
> > > > > > >> + struct ads1015_data *data = iio_priv(indio_dev);
> > > > > > >> +
> > > > > > >> + mutex_lock(&data->lock);
> > > > > > >> + switch (mask) {
> > > > > > >> + case IIO_CHAN_INFO_RAW:
> > > > > > >> + if (iio_buffer_enabled(indio_dev)) {
> > > > > > >> + ret = -EBUSY;
> > > > > > >> + break;
> > > > > > >> + }
> > > > > > >> +
> > > > > > >> + ret = ads1015_set_power_state(data, true);
> > > > > > >> + if (ret < 0)
> > > > > > >> + break;
> > > > > > >
> > > > > > > Just tested the driver on a Dragonboard 410C with a robotics
> > > > > > > mezzanine that I designed.
> > > > > > >
> > > > > > > The above ads1015_set_power_state(data, true) is always
> > > > > > > returning
> > -
> > > EINVAL.
> > > > > > >
> > > > > > > Any ideas why that would be happening?
> > > > > > > I think it may be the return from pm_runtime_get_sync?
> > > > > >
> > > > > > Can you confirm that pm_runtime_get_sync fails? Using some
> printk?
> > > > > >
> > > > > > Also adding printks in suspend/resume function would be helpful.
> > > > > > Do you have CONFIG_PM enabled?
> > > > > >
> > > > >
> > > > > Indeed it is the pm_runtime_get_sync that fails with a -EINVAL.
> > > > >
> > > > > > >
> > > > > > > When I comment out the break the readings come back but are
> > > > > > > not
> > > updated continually.
> > > > > > > If I read in_voltage0-voltage1_raw then in_voltage0_raw the
> > > > > > > value
> > is
> > > updated.
> > > > > >
> > > > > > I guess this is normal if set_power_state fails.
> > > > >
> > > > > The hwmod driver works fine BTW.
> > > > >
> > > > > My guess is there is an issue with the qup i2c driver seeing as
> > > > > it has worked on other system without issue.
> > > > >
> > > > > CC'd some the latest developer on the qup i2c driver.
> > > > >
> > > > > I2C guys have any ideas on this?
> > > > >
> > > >
> > > > Adding some more people who recently worked on this. Might be nice
> > > > to know which kernel version you are using.
> > > >
> > Which i2c bus is this connected to ? I can give a try with 410c to
> > see why pm_runtime_get_sync from qup fails.
>
> It is on the lowspeed header. Here is my devicetree entry:
>
> i2c@78b6000 {
> /* On Low speed expansion */
> label = "LS-I2C0";
> status = "okay";
>
> pca: pca@40 {
> compatible = "nxp,pca9685-pwm";
> #pwm-cells = <2>;
> reg = <0x40>;
> };
>
> adc: adc@48 {
> compatible = "ti,ads1015";
> reg = <0x48>;
> };
> };
Whats the sequence in which the failure happens ?
I tested on DB410c by adding the DT entry that you mentioned above on
4.5-rc2 and rc3.
I see that the i2c transfers call from pca9685 during pca9685_pwm_probe
did
go through and no failure from pm_runtime_get_sync
Regards,
Sricharan
^ permalink raw reply [flat|nested] 45+ messages in thread
* RE: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
@ 2016-02-10 12:22 ` Sricharan
0 siblings, 0 replies; 45+ messages in thread
From: Sricharan @ 2016-02-10 12:22 UTC (permalink / raw)
To: 'Michael Welling'
Cc: 'Wolfram Sang', 'Daniel Baluta',
'Jonathan Cameron', 'Hartmut Knaack',
'Lars-Peter Clausen', 'Peter Meerwald-Stadler',
'Linux Kernel Mailing List',
linux-iio, 'Lucas De Marchi', 'Andy Gross',
'Pramod Gurav', 'Bjorn Andersson',
'Guenter Roeck',
eibach, linux-arm-msm
Hi,
> -----Original Message-----
> From: linux-arm-msm-owner@vger.kernel.org [mailto:linux-arm-msm-
> owner@vger.kernel.org] On Behalf Of Michael Welling
> Sent: Tuesday, February 09, 2016 12:47 AM
> To: Sricharan
> Cc: 'Wolfram Sang'; 'Daniel Baluta'; 'Jonathan Cameron'; 'Hartmut Knaack';
> 'Lars-Peter Clausen'; 'Peter Meerwald-Stadler'; 'Linux Kernel Mailing
List';
> linux-iio@vger.kernel.org; 'Lucas De Marchi'; 'Andy Gross'; 'Pramod
Gurav';
> 'Bjorn Andersson'; 'Guenter Roeck'; eibach@gdsys.de; linux-arm-
> msm@vger.kernel.org
> Subject: Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
>
> On Tue, Feb 09, 2016 at 12:41:35AM +0530, Sricharan wrote:
> > Hi,
> >
> > > -----Original Message-----
> > > From: Michael Welling [mailto:mwelling79@gmail.com] On Behalf Of
> > > Michael Welling
> > > Sent: Monday, February 08, 2016 10:07 PM
> > > To: Wolfram Sang
> > > Cc: Daniel Baluta; Jonathan Cameron; Hartmut Knaack; Lars-Peter
> > > Clausen; Peter Meerwald-Stadler; Linux Kernel Mailing List;
> > linux-iio@vger.kernel.org;
> > > Lucas De Marchi; Andy Gross; Pramod Gurav; Bjorn Andersson; Guenter
> > > Roeck; eibach@gdsys.de; Sricharan R; linux-arm-msm@vger.kernel.org
> > > Subject: Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
> > >
> > > On Mon, Feb 08, 2016 at 11:25:00AM +0100, Wolfram Sang wrote:
> > > > On Fri, Feb 05, 2016 at 06:32:45PM -0600, Michael Welling wrote:
> > > > > On Fri, Feb 05, 2016 at 09:32:34PM +0200, Daniel Baluta wrote:
> > > > > > >> +static int ads1015_read_raw(struct iio_dev *indio_dev,
> > > > > > >> + struct iio_chan_spec const *chan,
> > > > > > >> +int
> > *val,
> > > > > > >> + int *val2, long mask) {
> > > > > > >> + int ret, idx;
> > > > > > >> + struct ads1015_data *data = iio_priv(indio_dev);
> > > > > > >> +
> > > > > > >> + mutex_lock(&data->lock);
> > > > > > >> + switch (mask) {
> > > > > > >> + case IIO_CHAN_INFO_RAW:
> > > > > > >> + if (iio_buffer_enabled(indio_dev)) {
> > > > > > >> + ret = -EBUSY;
> > > > > > >> + break;
> > > > > > >> + }
> > > > > > >> +
> > > > > > >> + ret = ads1015_set_power_state(data, true);
> > > > > > >> + if (ret < 0)
> > > > > > >> + break;
> > > > > > >
> > > > > > > Just tested the driver on a Dragonboard 410C with a robotics
> > > > > > > mezzanine that I designed.
> > > > > > >
> > > > > > > The above ads1015_set_power_state(data, true) is always
> > > > > > > returning
> > -
> > > EINVAL.
> > > > > > >
> > > > > > > Any ideas why that would be happening?
> > > > > > > I think it may be the return from pm_runtime_get_sync?
> > > > > >
> > > > > > Can you confirm that pm_runtime_get_sync fails? Using some
> printk?
> > > > > >
> > > > > > Also adding printks in suspend/resume function would be helpful.
> > > > > > Do you have CONFIG_PM enabled?
> > > > > >
> > > > >
> > > > > Indeed it is the pm_runtime_get_sync that fails with a -EINVAL.
> > > > >
> > > > > > >
> > > > > > > When I comment out the break the readings come back but are
> > > > > > > not
> > > updated continually.
> > > > > > > If I read in_voltage0-voltage1_raw then in_voltage0_raw the
> > > > > > > value
> > is
> > > updated.
> > > > > >
> > > > > > I guess this is normal if set_power_state fails.
> > > > >
> > > > > The hwmod driver works fine BTW.
> > > > >
> > > > > My guess is there is an issue with the qup i2c driver seeing as
> > > > > it has worked on other system without issue.
> > > > >
> > > > > CC'd some the latest developer on the qup i2c driver.
> > > > >
> > > > > I2C guys have any ideas on this?
> > > > >
> > > >
> > > > Adding some more people who recently worked on this. Might be nice
> > > > to know which kernel version you are using.
> > > >
> > Which i2c bus is this connected to ? I can give a try with 410c to
> > see why pm_runtime_get_sync from qup fails.
>
> It is on the lowspeed header. Here is my devicetree entry:
>
> i2c@78b6000 {
> /* On Low speed expansion */
> label = "LS-I2C0";
> status = "okay";
>
> pca: pca@40 {
> compatible = "nxp,pca9685-pwm";
> #pwm-cells = <2>;
> reg = <0x40>;
> };
>
> adc: adc@48 {
> compatible = "ti,ads1015";
> reg = <0x48>;
> };
> };
Whats the sequence in which the failure happens ?
I tested on DB410c by adding the DT entry that you mentioned above on
4.5-rc2 and rc3.
I see that the i2c transfers call from pca9685 during pca9685_pwm_probe
did
go through and no failure from pm_runtime_get_sync
Regards,
Sricharan
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-10 12:22 ` Sricharan
(?)
@ 2016-02-10 12:56 ` Daniel Baluta
[not found] ` <CAEnQRZCzkq5wFsS7Ydby+bsFJTbGVq=AX9TLkkhfMKOG8gAZHA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
-1 siblings, 1 reply; 45+ messages in thread
From: Daniel Baluta @ 2016-02-10 12:56 UTC (permalink / raw)
To: Sricharan
Cc: Michael Welling, Wolfram Sang, Daniel Baluta, Jonathan Cameron,
Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
Linux Kernel Mailing List, linux-iio, Lucas De Marchi,
Andy Gross, Pramod Gurav, Bjorn Andersson, Guenter Roeck, eibach,
linux-arm-msm
<snap headers>
>> > > > > > >> +static int ads1015_read_raw(struct iio_dev *indio_dev,
>> > > > > > >> + struct iio_chan_spec const *chan,
>> > > > > > >> +int
>> > *val,
>> > > > > > >> + int *val2, long mask) {
>> > > > > > >> + int ret, idx;
>> > > > > > >> + struct ads1015_data *data = iio_priv(indio_dev);
>> > > > > > >> +
>> > > > > > >> + mutex_lock(&data->lock);
>> > > > > > >> + switch (mask) {
>> > > > > > >> + case IIO_CHAN_INFO_RAW:
>> > > > > > >> + if (iio_buffer_enabled(indio_dev)) {
>> > > > > > >> + ret = -EBUSY;
>> > > > > > >> + break;
>> > > > > > >> + }
>> > > > > > >> +
>> > > > > > >> + ret = ads1015_set_power_state(data, true);
>> > > > > > >> + if (ret < 0)
>> > > > > > >> + break;
>> > > > > > >
>> > > > > > > Just tested the driver on a Dragonboard 410C with a robotics
>> > > > > > > mezzanine that I designed.
>> > > > > > >
>> > > > > > > The above ads1015_set_power_state(data, true) is always
>> > > > > > > returning
>> > -
>> > > EINVAL.
>> > > > > > >
>> > > > > > > Any ideas why that would be happening?
>> > > > > > > I think it may be the return from pm_runtime_get_sync?
>> > > > > >
>> > > > > > Can you confirm that pm_runtime_get_sync fails? Using some
>> printk?
>> > > > > >
>> > > > > > Also adding printks in suspend/resume function would be helpful.
>> > > > > > Do you have CONFIG_PM enabled?
>> > > > > >
>> > > > >
>> > > > > Indeed it is the pm_runtime_get_sync that fails with a -EINVAL.
>> > > > >
>> > > > > > >
>> > > > > > > When I comment out the break the readings come back but are
>> > > > > > > not
>> > > updated continually.
>> > > > > > > If I read in_voltage0-voltage1_raw then in_voltage0_raw the
>> > > > > > > value
>> > is
>> > > updated.
>> > > > > >
>> > > > > > I guess this is normal if set_power_state fails.
>> > > > >
>> > > > > The hwmod driver works fine BTW.
>> > > > >
>> > > > > My guess is there is an issue with the qup i2c driver seeing as
>> > > > > it has worked on other system without issue.
>> > > > >
>> > > > > CC'd some the latest developer on the qup i2c driver.
>> > > > >
>> > > > > I2C guys have any ideas on this?
>> > > > >
>> > > >
>> > > > Adding some more people who recently worked on this. Might be nice
>> > > > to know which kernel version you are using.
>> > > >
>> > Which i2c bus is this connected to ? I can give a try with 410c to
>> > see why pm_runtime_get_sync from qup fails.
>>
>> It is on the lowspeed header. Here is my devicetree entry:
>>
>> i2c@78b6000 {
>> /* On Low speed expansion */
>> label = "LS-I2C0";
>> status = "okay";
>>
>> pca: pca@40 {
>> compatible = "nxp,pca9685-pwm";
>> #pwm-cells = <2>;
>> reg = <0x40>;
>> };
>>
>> adc: adc@48 {
>> compatible = "ti,ads1015";
>> reg = <0x48>;
>> };
>> };
>
> Whats the sequence in which the failure happens ?
>
> I tested on DB410c by adding the DT entry that you mentioned above on
> 4.5-rc2 and rc3.
> I see that the i2c transfers call from pca9685 during pca9685_pwm_probe
> did
> go through and no failure from pm_runtime_get_sync
Hi Sricharan,
Are you looking at pca9685_pwm_probe in drivers/pwm/pwm-pca9685.c right?
I'm asking this because this driver doesn't seem to support runtime
pm and there is no check for regmap_write/regmap_write return
code in the probe function.
Daniel.
^ permalink raw reply [flat|nested] 45+ messages in thread
* RE: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-10 12:56 ` Daniel Baluta
@ 2016-02-10 15:09 ` Sricharan
0 siblings, 0 replies; 45+ messages in thread
From: Sricharan @ 2016-02-10 15:09 UTC (permalink / raw)
To: 'Daniel Baluta'
Cc: 'Michael Welling', 'Wolfram Sang',
'Jonathan Cameron', 'Hartmut Knaack',
'Lars-Peter Clausen', 'Peter Meerwald-Stadler',
'Linux Kernel Mailing List',
linux-iio-u79uwXL29TY76Z2rM5mHXA, 'Lucas De Marchi',
'Andy Gross', 'Pramod Gurav',
'Bjorn Andersson', 'Guenter Roeck',
eibach-dJ+jgKLZIg4, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
<snip>...
> >> > > > >
> >> > > > > Indeed it is the pm_runtime_get_sync that fails with a -EINVAL.
> >> > > > >
> >> > > > > > >
> >> > > > > > > When I comment out the break the readings come back but
> >> > > > > > > are not
> >> > > updated continually.
> >> > > > > > > If I read in_voltage0-voltage1_raw then in_voltage0_raw
> >> > > > > > > the value
> >> > is
> >> > > updated.
> >> > > > > >
> >> > > > > > I guess this is normal if set_power_state fails.
> >> > > > >
> >> > > > > The hwmod driver works fine BTW.
> >> > > > >
> >> > > > > My guess is there is an issue with the qup i2c driver seeing
> >> > > > > as it has worked on other system without issue.
> >> > > > >
> >> > > > > CC'd some the latest developer on the qup i2c driver.
> >> > > > >
> >> > > > > I2C guys have any ideas on this?
> >> > > > >
> >> > > >
> >> > > > Adding some more people who recently worked on this. Might be
> >> > > > nice to know which kernel version you are using.
> >> > > >
> >> > Which i2c bus is this connected to ? I can give a try with 410c to
> >> > see why pm_runtime_get_sync from qup fails.
> >>
> >> It is on the lowspeed header. Here is my devicetree entry:
> >>
> >> i2c@78b6000 {
> >> /* On Low speed expansion */
> >> label = "LS-I2C0";
> >> status = "okay";
> >>
> >> pca: pca@40 {
> >> compatible = "nxp,pca9685-pwm";
> >> #pwm-cells = <2>;
> >> reg = <0x40>;
> >> };
> >>
> >> adc: adc@48 {
> >> compatible = "ti,ads1015";
> >> reg = <0x48>;
> >> };
> >> };
> >
> > Whats the sequence in which the failure happens ?
> >
> > I tested on DB410c by adding the DT entry that you mentioned above on
> > 4.5-rc2 and rc3.
> > I see that the i2c transfers call from pca9685 during
> > pca9685_pwm_probe did go through and no failure from
> > pm_runtime_get_sync
>
> Hi Sricharan,
>
> Are you looking at pca9685_pwm_probe in drivers/pwm/pwm-pca9685.c
> right?
>
Yes.
> I'm asking this because this driver doesn't seem to support runtime pm and
> there is no check for regmap_write/regmap_write return code in the probe
> function.
Hmm to be clear, so it’s the pm_runtime_getsync from i2c-qup which fails right ?
I was tracking that when there are i2c_xfers from pwm. I did not see any failures there.
So wanted to know the correct sequence to reproduce.
Regards,
Sricharan
^ permalink raw reply [flat|nested] 45+ messages in thread
* RE: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
@ 2016-02-10 15:09 ` Sricharan
0 siblings, 0 replies; 45+ messages in thread
From: Sricharan @ 2016-02-10 15:09 UTC (permalink / raw)
To: 'Daniel Baluta'
Cc: 'Michael Welling', 'Wolfram Sang',
'Jonathan Cameron', 'Hartmut Knaack',
'Lars-Peter Clausen', 'Peter Meerwald-Stadler',
'Linux Kernel Mailing List',
linux-iio, 'Lucas De Marchi', 'Andy Gross',
'Pramod Gurav', 'Bjorn Andersson',
'Guenter Roeck',
eibach, linux-arm-msm
<snip>...
> >> > > > >
> >> > > > > Indeed it is the pm_runtime_get_sync that fails with a -EINVAL.
> >> > > > >
> >> > > > > > >
> >> > > > > > > When I comment out the break the readings come back but
> >> > > > > > > are not
> >> > > updated continually.
> >> > > > > > > If I read in_voltage0-voltage1_raw then in_voltage0_raw
> >> > > > > > > the value
> >> > is
> >> > > updated.
> >> > > > > >
> >> > > > > > I guess this is normal if set_power_state fails.
> >> > > > >
> >> > > > > The hwmod driver works fine BTW.
> >> > > > >
> >> > > > > My guess is there is an issue with the qup i2c driver seeing
> >> > > > > as it has worked on other system without issue.
> >> > > > >
> >> > > > > CC'd some the latest developer on the qup i2c driver.
> >> > > > >
> >> > > > > I2C guys have any ideas on this?
> >> > > > >
> >> > > >
> >> > > > Adding some more people who recently worked on this. Might be
> >> > > > nice to know which kernel version you are using.
> >> > > >
> >> > Which i2c bus is this connected to ? I can give a try with 410c to
> >> > see why pm_runtime_get_sync from qup fails.
> >>
> >> It is on the lowspeed header. Here is my devicetree entry:
> >>
> >> i2c@78b6000 {
> >> /* On Low speed expansion */
> >> label = "LS-I2C0";
> >> status = "okay";
> >>
> >> pca: pca@40 {
> >> compatible = "nxp,pca9685-pwm";
> >> #pwm-cells = <2>;
> >> reg = <0x40>;
> >> };
> >>
> >> adc: adc@48 {
> >> compatible = "ti,ads1015";
> >> reg = <0x48>;
> >> };
> >> };
> >
> > Whats the sequence in which the failure happens ?
> >
> > I tested on DB410c by adding the DT entry that you mentioned above on
> > 4.5-rc2 and rc3.
> > I see that the i2c transfers call from pca9685 during
> > pca9685_pwm_probe did go through and no failure from
> > pm_runtime_get_sync
>
> Hi Sricharan,
>
> Are you looking at pca9685_pwm_probe in drivers/pwm/pwm-pca9685.c
> right?
>
Yes.
> I'm asking this because this driver doesn't seem to support runtime pm and
> there is no check for regmap_write/regmap_write return code in the probe
> function.
Hmm to be clear, so it’s the pm_runtime_getsync from i2c-qup which fails right ?
I was tracking that when there are i2c_xfers from pwm. I did not see any failures there.
So wanted to know the correct sequence to reproduce.
Regards,
Sricharan
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-10 15:09 ` Sricharan
@ 2016-02-10 16:36 ` Michael Welling
-1 siblings, 0 replies; 45+ messages in thread
From: Michael Welling @ 2016-02-10 16:36 UTC (permalink / raw)
To: Sricharan
Cc: 'Daniel Baluta', 'Wolfram Sang',
'Jonathan Cameron', 'Hartmut Knaack',
'Lars-Peter Clausen', 'Peter Meerwald-Stadler',
'Linux Kernel Mailing List',
linux-iio-u79uwXL29TY76Z2rM5mHXA, 'Lucas De Marchi',
'Andy Gross', 'Pramod Gurav',
'Bjorn Andersson', 'Guenter Roeck',
eibach-dJ+jgKLZIg4, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
On Wed, Feb 10, 2016 at 08:39:04PM +0530, Sricharan wrote:
> > Hi Sricharan,
> >
> > Are you looking at pca9685_pwm_probe in drivers/pwm/pwm-pca9685.c
> > right?
> >
> Yes.
>
> > I'm asking this because this driver doesn't seem to support runtime pm and
> > there is no check for regmap_write/regmap_write return code in the probe
> > function.
> Hmm to be clear, so it’s the pm_runtime_getsync from i2c-qup which fails right ?
> I was tracking that when there are i2c_xfers from pwm. I did not see any failures there.
> So wanted to know the correct sequence to reproduce.
>
The problem was discovered using the patch that this thread is on. The PWM driver does
not have the problem.
When the driver in this patch called pm_runtime_get_sync you got -EINVAL back.
> Regards,
> Sricharan
>
>
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
@ 2016-02-10 16:36 ` Michael Welling
0 siblings, 0 replies; 45+ messages in thread
From: Michael Welling @ 2016-02-10 16:36 UTC (permalink / raw)
To: Sricharan
Cc: 'Daniel Baluta', 'Wolfram Sang',
'Jonathan Cameron', 'Hartmut Knaack',
'Lars-Peter Clausen', 'Peter Meerwald-Stadler',
'Linux Kernel Mailing List',
linux-iio, 'Lucas De Marchi', 'Andy Gross',
'Pramod Gurav', 'Bjorn Andersson',
'Guenter Roeck',
eibach, linux-arm-msm
On Wed, Feb 10, 2016 at 08:39:04PM +0530, Sricharan wrote:
> > Hi Sricharan,
> >
> > Are you looking at pca9685_pwm_probe in drivers/pwm/pwm-pca9685.c
> > right?
> >
> Yes.
>
> > I'm asking this because this driver doesn't seem to support runtime pm and
> > there is no check for regmap_write/regmap_write return code in the probe
> > function.
> Hmm to be clear, so it’s the pm_runtime_getsync from i2c-qup which fails right ?
> I was tracking that when there are i2c_xfers from pwm. I did not see any failures there.
> So wanted to know the correct sequence to reproduce.
>
The problem was discovered using the patch that this thread is on. The PWM driver does
not have the problem.
When the driver in this patch called pm_runtime_get_sync you got -EINVAL back.
> Regards,
> Sricharan
>
>
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-10 16:36 ` Michael Welling
(?)
@ 2016-02-17 23:53 ` Michael Welling
2016-02-18 8:45 ` Sricharan
-1 siblings, 1 reply; 45+ messages in thread
From: Michael Welling @ 2016-02-17 23:53 UTC (permalink / raw)
To: Sricharan
Cc: 'Daniel Baluta', 'Wolfram Sang',
'Jonathan Cameron', 'Hartmut Knaack',
'Lars-Peter Clausen', 'Peter Meerwald-Stadler',
'Linux Kernel Mailing List',
linux-iio, 'Lucas De Marchi', 'Andy Gross',
'Pramod Gurav', 'Bjorn Andersson',
'Guenter Roeck',
eibach, linux-arm-msm
On Wed, Feb 10, 2016 at 10:36:22AM -0600, Michael Welling wrote:
> On Wed, Feb 10, 2016 at 08:39:04PM +0530, Sricharan wrote:
> > > Hi Sricharan,
> > >
> > > Are you looking at pca9685_pwm_probe in drivers/pwm/pwm-pca9685.c
> > > right?
> > >
> > Yes.
> >
> > > I'm asking this because this driver doesn't seem to support runtime pm and
> > > there is no check for regmap_write/regmap_write return code in the probe
> > > function.
> > Hmm to be clear, so it’s the pm_runtime_getsync from i2c-qup which fails right ?
> > I was tracking that when there are i2c_xfers from pwm. I did not see any failures there.
> > So wanted to know the correct sequence to reproduce.
> >
>
> The problem was discovered using the patch that this thread is on. The PWM driver does
> not have the problem.
>
> When the driver in this patch called pm_runtime_get_sync you got -EINVAL back.
I noticed some patches for the QUP I2C driver in linux-next so I built against it.
The ADC driver now appears to work as desired.
root@dragonboard-410c:~# cat /sys/bus/iio/devices/iio\:device0/in_voltage0_raw
287
root@dragonboard-410c:~# cat /sys/bus/iio/devices/iio\:device0/in_voltage1_raw
269
root@dragonboard-410c:~# cat /sys/bus/iio/devices/iio\:device0/in_voltage2_raw
270
root@dragonboard-410c:~# cat /sys/bus/iio/devices/iio\:device0/in_voltage3_raw
271
>
> > Regards,
> > Sricharan
> >
> >
> >
^ permalink raw reply [flat|nested] 45+ messages in thread
* RE: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-17 23:53 ` Michael Welling
@ 2016-02-18 8:45 ` Sricharan
0 siblings, 0 replies; 45+ messages in thread
From: Sricharan @ 2016-02-18 8:45 UTC (permalink / raw)
To: 'Michael Welling'
Cc: 'Daniel Baluta', 'Wolfram Sang',
'Jonathan Cameron', 'Hartmut Knaack',
'Lars-Peter Clausen', 'Peter Meerwald-Stadler',
'Linux Kernel Mailing List',
linux-iio, 'Lucas De Marchi', 'Andy Gross',
'Pramod Gurav', 'Bjorn Andersson',
'Guenter Roeck',
eibach, linux-arm-msm
Hi,
> On Wed, Feb 10, 2016 at 10:36:22AM -0600, Michael Welling wrote:
> > On Wed, Feb 10, 2016 at 08:39:04PM +0530, Sricharan wrote:
> > > > Hi Sricharan,
> > > >
> > > > Are you looking at pca9685_pwm_probe in drivers/pwm/pwm-
> pca9685.c
> > > > right?
> > > >
> > > Yes.
> > >
> > > > I'm asking this because this driver doesn't seem to support
> > > > runtime pm and there is no check for regmap_write/regmap_write
> > > > return code in the probe function.
> > > Hmm to be clear, so it’s the pm_runtime_getsync from i2c-qup which
> fails right ?
> > > I was tracking that when there are i2c_xfers from pwm. I did not see
> any failures there.
> > > So wanted to know the correct sequence to reproduce.
> > >
> >
> > The problem was discovered using the patch that this thread is on. The
> > PWM driver does not have the problem.
> >
> > When the driver in this patch called pm_runtime_get_sync you got -EINVAL
> back.
>
> I noticed some patches for the QUP I2C driver in linux-next so I built against
> it.
>
> The ADC driver now appears to work as desired.
>
Ok, I suppose those were changes merged last week to add qup V2 tags support, but
not sure how that is helping to solve the pm_runtime issue here.
Regards,
Sricharan
^ permalink raw reply [flat|nested] 45+ messages in thread
* RE: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
@ 2016-02-18 8:45 ` Sricharan
0 siblings, 0 replies; 45+ messages in thread
From: Sricharan @ 2016-02-18 8:45 UTC (permalink / raw)
To: 'Michael Welling'
Cc: 'Daniel Baluta', 'Wolfram Sang',
'Jonathan Cameron', 'Hartmut Knaack',
'Lars-Peter Clausen', 'Peter Meerwald-Stadler',
'Linux Kernel Mailing List',
linux-iio, 'Lucas De Marchi', 'Andy Gross',
'Pramod Gurav', 'Bjorn Andersson',
'Guenter Roeck',
eibach, linux-arm-msm
Hi,
> On Wed, Feb 10, 2016 at 10:36:22AM -0600, Michael Welling wrote:
> > On Wed, Feb 10, 2016 at 08:39:04PM +0530, Sricharan wrote:
> > > > Hi Sricharan,
> > > >
> > > > Are you looking at pca9685_pwm_probe in drivers/pwm/pwm-
> pca9685.c
> > > > right?
> > > >
> > > Yes.
> > >
> > > > I'm asking this because this driver doesn't seem to support
> > > > runtime pm and there is no check for regmap_write/regmap_write
> > > > return code in the probe function.
> > > Hmm to be clear, so it’s the pm_runtime_getsync from i2c-qup which
> fails right ?
> > > I was tracking that when there are i2c_xfers from pwm. I did not see
> any failures there.
> > > So wanted to know the correct sequence to reproduce.
> > >
> >
> > The problem was discovered using the patch that this thread is on. The
> > PWM driver does not have the problem.
> >
> > When the driver in this patch called pm_runtime_get_sync you got -EINVAL
> back.
>
> I noticed some patches for the QUP I2C driver in linux-next so I built against
> it.
>
> The ADC driver now appears to work as desired.
>
Ok, I suppose those were changes merged last week to add qup V2 tags support, but
not sure how that is helping to solve the pm_runtime issue here.
Regards,
Sricharan
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-02-05 13:17 [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support Daniel Baluta
` (2 preceding siblings ...)
2016-02-06 11:05 ` Jonathan Cameron
@ 2016-03-01 0:50 ` Michael Welling
2016-03-01 1:09 ` Lucas De Marchi
3 siblings, 1 reply; 45+ messages in thread
From: Michael Welling @ 2016-03-01 0:50 UTC (permalink / raw)
To: Daniel Baluta
Cc: jic23, knaack.h, lars, pmeerw, linux-kernel, linux-iio,
lucas.demarchi, linux, eibach
On Fri, Feb 05, 2016 at 03:17:18PM +0200, Daniel Baluta wrote:
> The driver has sysfs readings with runtime PM support for power saving.
> It also offers buffer support that can be used together with IIO software
> triggers.
>
Daniel,
So I noticed something yesterday while testing new boards.
The channels are occassionally swapping when accessing data from multiple channels.
I wrote a simple bash script to demonstrate.
root@dragonboard-410c:~# cat test-analog.sh
while [ 1 ]; do
voltage0=`cat /sys/bus/iio/devices/iio\:device0/in_voltage0_raw`
voltage1=`cat /sys/bus/iio/devices/iio\:device0/in_voltage1_raw`
voltage2=`cat /sys/bus/iio/devices/iio\:device0/in_voltage2_raw`
voltage3=`cat /sys/bus/iio/devices/iio\:device0/in_voltage3_raw`
echo ain0 = $voltage0 ain1 = $voltage1 ain2 = $voltage2 ain3 = $voltage3
done
root@dragonboard-410c:~# ./test-analog.sh
ain0 = 266 ain1 = 291 ain2 = 268 ain3 = 291
ain0 = 287 ain1 = 294 ain2 = 289 ain3 = 292
ain0 = 284 ain1 = 294 ain2 = 286 ain3 = 0
ain0 = 285 ain1 = 288 ain2 = 287 ain3 = 0
ain0 = 287 ain1 = 286 ain2 = 287 ain3 = 0
ain0 = 285 ain1 = 287 ain2 = 293 ain3 = 0
ain0 = 0 ain1 = 289 ain2 = 293 ain3 = 0
ain0 = 0 ain1 = 289 ain2 = 290 ain3 = 291
ain0 = 287 ain1 = 289 ain2 = 289 ain3 = 0
ain0 = 0 ain1 = 285 ain2 = 288 ain3 = 0
ain0 = 285 ain1 = 286 ain2 = 288 ain3 = 0
ain0 = 286 ain1 = 288 ain2 = 285 ain3 = 0
ain0 = 287 ain1 = 293 ain2 = 289 ain3 = 292
ain0 = 288 ain1 = 292 ain2 = 287 ain3 = 293
ain0 = 286 ain1 = 287 ain2 = 289 ain3 = 0
ain0 = 283 ain1 = 289 ain2 = 289 ain3 = 0
ain0 = 0 ain1 = 287 ain2 = 287 ain3 = 0
ain0 = 0 ain1 = 286 ain2 = 288 ain3 = 0
ain0 = 286 ain1 = 288 ain2 = 289 ain3 = 0
ain0 = 287 ain1 = 286 ain2 = 290 ain3 = 0
ain0 = 289 ain1 = 291 ain2 = 287 ain3 = 0
ain0 = 284 ain1 = 286 ain2 = 292 ain3 = 0
ain0 = 286 ain1 = 291 ain2 = 289 ain3 = 292
ain0 = 284 ain1 = 292 ain2 = 291 ain3 = 291
ain0 = 285 ain1 = 287 ain2 = 287 ain3 = 0
ain0 = 0 ain1 = 289 ain2 = 291 ain3 = 0
ain0 = 0 ain1 = 288 ain2 = 291 ain3 = 0
ain0 = 0 ain1 = 286 ain2 = 288 ain3 = 0
ain0 = 286 ain1 = 287 ain2 = 290 ain3 = 0
ain0 = 286 ain1 = 286 ain2 = 288 ain3 = 0
ain0 = 286 ain1 = 286 ain2 = 287 ain3 = 0
ain0 = 285 ain1 = 289 ain2 = 288 ain3 = 288
ain0 = 289 ain1 = 292 ain2 = 289 ain3 = 293
ain0 = 287 ain1 = 292 ain2 = 290 ain3 = 0
ain0 = 286 ain1 = 286 ain2 = 291 ain3 = 0
ain0 = 0 ain1 = 287 ain2 = 290 ain3 = 0
ain0 = 0 ain1 = 286 ain2 = 289 ain3 = 0
ain0 = 284 ain1 = 285 ain2 = 292 ain3 = 0
ain0 = 285 ain1 = 289 ain2 = 289 ain3 = 0
ain0 = 287 ain1 = 287 ain2 = 287 ain3 = 0
ain0 = 287 ain1 = 291 ain2 = 287 ain3 = 291
ain0 = 288 ain1 = 291 ain2 = 290 ain3 = 293
ain0 = 285 ain1 = 290 ain2 = 287 ain3 = 292
ain0 = 285 ain1 = 289 ain2 = 289 ain3 = 292
ain0 = 286 ain1 = 289 ain2 = 288 ain3 = 0
ain0 = 0 ain1 = 286 ain2 = 291 ain3 = 0
ain0 = 0 ain1 = 287 ain2 = 291 ain3 = 0
ain0 = 287 ain1 = 286 ain2 = 291 ain3 = 0
ain0 = 288 ain1 = 286 ain2 = 289 ain3 = 0
ain0 = 284 ain1 = 289 ain2 = 288 ain3 = 0
ain0 = 287 ain1 = 290 ain2 = 287 ain3 = 292
ain0 = 285 ain1 = 292 ain2 = 289 ain3 = 292
ain0 = 289 ain1 = 292 ain2 = 290 ain3 = 0
.
.
The in_voltage3_raw channel is connected to GND the rest are left floating.
This does not happen if only one channels is accessed at a time.
I have not yet found the solution. Please left me know if this can be
duplicated on your end.
Regards,
Michael
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-03-01 0:50 ` Michael Welling
@ 2016-03-01 1:09 ` Lucas De Marchi
2016-03-01 1:16 ` Michael Welling
` (2 more replies)
0 siblings, 3 replies; 45+ messages in thread
From: Lucas De Marchi @ 2016-03-01 1:09 UTC (permalink / raw)
To: Michael Welling
Cc: Daniel Baluta, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, lkml, linux-iio,
Lucas De Marchi, Guenter Roeck, eibach
On Mon, Feb 29, 2016 at 9:50 PM, Michael Welling <mwelling@ieee.org> wrote:
> On Fri, Feb 05, 2016 at 03:17:18PM +0200, Daniel Baluta wrote:
>> The driver has sysfs readings with runtime PM support for power saving.
>> It also offers buffer support that can be used together with IIO software
>> triggers.
>>
>
> Daniel,
>
> So I noticed something yesterday while testing new boards.
> The channels are occassionally swapping when accessing data from multiple channels.
>
> I wrote a simple bash script to demonstrate.
This happened to me in a previous version of the patch. I remember it
being fixed in the last version (or at least I could not reproduce).
I'll test again tomorrow with your script.
Lucas De Marchi
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-03-01 1:09 ` Lucas De Marchi
@ 2016-03-01 1:16 ` Michael Welling
2016-03-01 1:33 ` Michael Welling
2016-03-01 2:42 ` Michael Welling
2 siblings, 0 replies; 45+ messages in thread
From: Michael Welling @ 2016-03-01 1:16 UTC (permalink / raw)
To: Lucas De Marchi
Cc: Daniel Baluta, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, lkml, linux-iio,
Lucas De Marchi, Guenter Roeck, eibach
On Mon, Feb 29, 2016 at 10:09:10PM -0300, Lucas De Marchi wrote:
> On Mon, Feb 29, 2016 at 9:50 PM, Michael Welling <mwelling@ieee.org> wrote:
> > On Fri, Feb 05, 2016 at 03:17:18PM +0200, Daniel Baluta wrote:
> >> The driver has sysfs readings with runtime PM support for power saving.
> >> It also offers buffer support that can be used together with IIO software
> >> triggers.
> >>
> >
> > Daniel,
> >
> > So I noticed something yesterday while testing new boards.
> > The channels are occassionally swapping when accessing data from multiple channels.
> >
> > I wrote a simple bash script to demonstrate.
>
> This happened to me in a previous version of the patch. I remember it
> being fixed in the last version (or at least I could not reproduce).
> I'll test again tomorrow with your script.
>
Okay thanks. I will see if I am using a dated version of the driver for some reason.
> Lucas De Marchi
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-03-01 1:09 ` Lucas De Marchi
2016-03-01 1:16 ` Michael Welling
@ 2016-03-01 1:33 ` Michael Welling
2016-03-01 2:42 ` Michael Welling
2 siblings, 0 replies; 45+ messages in thread
From: Michael Welling @ 2016-03-01 1:33 UTC (permalink / raw)
To: Lucas De Marchi
Cc: Daniel Baluta, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, lkml, linux-iio,
Lucas De Marchi, Guenter Roeck, eibach
On Mon, Feb 29, 2016 at 10:09:10PM -0300, Lucas De Marchi wrote:
> On Mon, Feb 29, 2016 at 9:50 PM, Michael Welling <mwelling@ieee.org> wrote:
> > On Fri, Feb 05, 2016 at 03:17:18PM +0200, Daniel Baluta wrote:
> >> The driver has sysfs readings with runtime PM support for power saving.
> >> It also offers buffer support that can be used together with IIO software
> >> triggers.
> >>
> >
> > Daniel,
> >
> > So I noticed something yesterday while testing new boards.
> > The channels are occassionally swapping when accessing data from multiple channels.
> >
> > I wrote a simple bash script to demonstrate.
>
> This happened to me in a previous version of the patch. I remember it
> being fixed in the last version (or at least I could not reproduce).
> I'll test again tomorrow with your script.
>
Just verified that it is happening to me will the latest in linux-next.
root@dragonboard-410c:~# cat /proc/version
Linux version 4.5.0-rc6-next-20160229+ (michael@deathstar) (gcc version 5.2.0 (GCC) ) #1 SMP PREEMPT Mon Feb 29 19:27:11 CST 2016
root@dragonboard-410c:~# ./test-analog.sh
ain0 = 267 ain1 = 286 ain2 = 268 ain3 = 288
ain0 = 283 ain1 = 284 ain2 = 284 ain3 = 0
ain0 = 0 ain1 = 284 ain2 = 287 ain3 = 0
ain0 = 0 ain1 = 285 ain2 = 287 ain3 = 0
ain0 = 282 ain1 = 284 ain2 = 287 ain3 = 0
ain0 = 283 ain1 = 284 ain2 = 286 ain3 = 0
ain0 = 283 ain1 = 290 ain2 = 285 ain3 = 288
ain0 = 284 ain1 = 286 ain2 = 285 ain3 = 0
ain0 = 0 ain1 = 284 ain2 = 285 ain3 = 0
ain0 = 0 ain1 = 286 ain2 = 287 ain3 = 0
ain0 = 0 ain1 = 284 ain2 = 289 ain3 = 0
.
.
> Lucas De Marchi
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-03-01 1:09 ` Lucas De Marchi
2016-03-01 1:16 ` Michael Welling
2016-03-01 1:33 ` Michael Welling
@ 2016-03-01 2:42 ` Michael Welling
2016-03-01 8:35 ` jic23
2016-03-01 11:28 ` Daniel Baluta
2 siblings, 2 replies; 45+ messages in thread
From: Michael Welling @ 2016-03-01 2:42 UTC (permalink / raw)
To: Lucas De Marchi
Cc: Daniel Baluta, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, lkml, linux-iio,
Lucas De Marchi, Guenter Roeck, eibach
On Mon, Feb 29, 2016 at 10:09:10PM -0300, Lucas De Marchi wrote:
> On Mon, Feb 29, 2016 at 9:50 PM, Michael Welling <mwelling@ieee.org> wrote:
> > On Fri, Feb 05, 2016 at 03:17:18PM +0200, Daniel Baluta wrote:
> >> The driver has sysfs readings with runtime PM support for power saving.
> >> It also offers buffer support that can be used together with IIO software
> >> triggers.
> >>
> >
> > Daniel,
> >
> > So I noticed something yesterday while testing new boards.
> > The channels are occassionally swapping when accessing data from multiple channels.
> >
> > I wrote a simple bash script to demonstrate.
>
> This happened to me in a previous version of the patch. I remember it
> being fixed in the last version (or at least I could not reproduce).
> I'll test again tomorrow with your script.
>
Here is what I believe is happening.
The request for a conversion on a new channel comes in while the conversion
for the previous channel is still converting. The driver waits approximately
one conversion cycle. The previous channel completes within this timeframe
and the MUX is changed and the new sample is started. The new sample is still
converting and the driver returns the value from the previous conversion.
For a test I multiplied the conv_time value by 2 in the ads1015_get_adc_result
function. This allows time for the current sample flush out and always returns
the appropriate channel's value.
Looking at the buffered mode it appears that only one channel is being accessed
at any time. This being the first one in the active_scan_mask found by
find_first_bit. So the MUX would never change is buffered mode as far I can
tell.
Don't we typically want to read all of enabled channels in buffered mode?
> Lucas De Marchi
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-03-01 2:42 ` Michael Welling
@ 2016-03-01 8:35 ` jic23
2016-03-01 16:02 ` Michael Welling
2016-03-01 11:28 ` Daniel Baluta
1 sibling, 1 reply; 45+ messages in thread
From: jic23 @ 2016-03-01 8:35 UTC (permalink / raw)
To: Michael Welling
Cc: Lucas De Marchi, Daniel Baluta, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, lkml, linux-iio,
Lucas De Marchi, Guenter Roeck, eibach, linux-iio-owner
On 01.03.2016 02:42, Michael Welling wrote:
> On Mon, Feb 29, 2016 at 10:09:10PM -0300, Lucas De Marchi wrote:
>> On Mon, Feb 29, 2016 at 9:50 PM, Michael Welling <mwelling@ieee.org>
>> wrote:
>> > On Fri, Feb 05, 2016 at 03:17:18PM +0200, Daniel Baluta wrote:
>> >> The driver has sysfs readings with runtime PM support for power saving.
>> >> It also offers buffer support that can be used together with IIO software
>> >> triggers.
>> >>
>> >
>> > Daniel,
>> >
>> > So I noticed something yesterday while testing new boards.
>> > The channels are occassionally swapping when accessing data from multiple channels.
>> >
>> > I wrote a simple bash script to demonstrate.
>>
>> This happened to me in a previous version of the patch. I remember it
>> being fixed in the last version (or at least I could not reproduce).
>> I'll test again tomorrow with your script.
>>
>
> Here is what I believe is happening.
>
> The request for a conversion on a new channel comes in while the
> conversion
> for the previous channel is still converting. The driver waits
> approximately
> one conversion cycle. The previous channel completes within this
> timeframe
> and the MUX is changed and the new sample is started. The new sample is
> still
> converting and the driver returns the value from the previous
> conversion.
>
> For a test I multiplied the conv_time value by 2 in the
> ads1015_get_adc_result
> function. This allows time for the current sample flush out and always
> returns
> the appropriate channel's value.
>
> Looking at the buffered mode it appears that only one channel is being
> accessed
> at any time. This being the first one in the active_scan_mask found by
> find_first_bit. So the MUX would never change is buffered mode as far I
> can
> tell.
>
> Don't we typically want to read all of enabled channels in buffered
> mode?
In some devices that is effectively impossible (fifo's that fill only
from the currently
selected channel). That is what the onehot validation callback is about.
In this particular case it looks like doing a multichannel read is fair
bit more time
consuming than a single channel read and so will result in a
considerable reduction in
throughput. This is of course why many parts include a simple
sequencer!
Anyhow, supporting multi channel buffered reads could be done (probably)
but you
would want to fall back to the single channel case as it is now.
Jonathan
>> Lucas De Marchi
> --
> 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] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-03-01 2:42 ` Michael Welling
2016-03-01 8:35 ` jic23
@ 2016-03-01 11:28 ` Daniel Baluta
2016-03-01 15:57 ` Michael Welling
1 sibling, 1 reply; 45+ messages in thread
From: Daniel Baluta @ 2016-03-01 11:28 UTC (permalink / raw)
To: Michael Welling
Cc: Lucas De Marchi, Daniel Baluta, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, lkml, linux-iio,
Lucas De Marchi, Guenter Roeck, eibach
On Tue, Mar 1, 2016 at 4:42 AM, Michael Welling <mwelling@ieee.org> wrote:
> On Mon, Feb 29, 2016 at 10:09:10PM -0300, Lucas De Marchi wrote:
>> On Mon, Feb 29, 2016 at 9:50 PM, Michael Welling <mwelling@ieee.org> wrote:
>> > On Fri, Feb 05, 2016 at 03:17:18PM +0200, Daniel Baluta wrote:
>> >> The driver has sysfs readings with runtime PM support for power saving.
>> >> It also offers buffer support that can be used together with IIO software
>> >> triggers.
>> >>
>> >
>> > Daniel,
>> >
>> > So I noticed something yesterday while testing new boards.
>> > The channels are occassionally swapping when accessing data from multiple channels.
>> >
>> > I wrote a simple bash script to demonstrate.
>>
>> This happened to me in a previous version of the patch. I remember it
>> being fixed in the last version (or at least I could not reproduce).
>> I'll test again tomorrow with your script.
>>
>
> Here is what I believe is happening.
>
> The request for a conversion on a new channel comes in while the conversion
> for the previous channel is still converting. The driver waits approximately
> one conversion cycle. The previous channel completes within this timeframe
> and the MUX is changed and the new sample is started. The new sample is still
> converting and the driver returns the value from the previous conversion.
>
Yes, this was the problem in the initial driver. The fix was to wait
for the conversion to complete
but it seems that it doesn't always work on your setup :(.
if (change) {
+ conv_time = DIV_ROUND_UP(USEC_PER_SEC, ads1015_data_rate[dr]);
+ usleep_range(conv_time, conv_time + 1);
+ }
+
> For a test I multiplied the conv_time value by 2 in the ads1015_get_adc_result
> function. This allows time for the current sample flush out and always returns
> the appropriate channel's value.
>
> Looking at the buffered mode it appears that only one channel is being accessed
> at any time. This being the first one in the active_scan_mask found by
> find_first_bit. So the MUX would never change is buffered mode as far I can
> tell.
>
> Don't we typically want to read all of enabled channels in buffered mode?
For the moment we allow only a single channel to be enabled in buffer mode.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-03-01 11:28 ` Daniel Baluta
@ 2016-03-01 15:57 ` Michael Welling
0 siblings, 0 replies; 45+ messages in thread
From: Michael Welling @ 2016-03-01 15:57 UTC (permalink / raw)
To: Daniel Baluta
Cc: Lucas De Marchi, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, lkml, linux-iio,
Lucas De Marchi, Guenter Roeck, eibach
On Tue, Mar 01, 2016 at 01:28:26PM +0200, Daniel Baluta wrote:
> On Tue, Mar 1, 2016 at 4:42 AM, Michael Welling <mwelling@ieee.org> wrote:
> > On Mon, Feb 29, 2016 at 10:09:10PM -0300, Lucas De Marchi wrote:
> >> On Mon, Feb 29, 2016 at 9:50 PM, Michael Welling <mwelling@ieee.org> wrote:
> >> > On Fri, Feb 05, 2016 at 03:17:18PM +0200, Daniel Baluta wrote:
> >> >> The driver has sysfs readings with runtime PM support for power saving.
> >> >> It also offers buffer support that can be used together with IIO software
> >> >> triggers.
> >> >>
> >> >
> >> > Daniel,
> >> >
> >> > So I noticed something yesterday while testing new boards.
> >> > The channels are occassionally swapping when accessing data from multiple channels.
> >> >
> >> > I wrote a simple bash script to demonstrate.
> >>
> >> This happened to me in a previous version of the patch. I remember it
> >> being fixed in the last version (or at least I could not reproduce).
> >> I'll test again tomorrow with your script.
> >>
> >
> > Here is what I believe is happening.
> >
> > The request for a conversion on a new channel comes in while the conversion
> > for the previous channel is still converting. The driver waits approximately
> > one conversion cycle. The previous channel completes within this timeframe
> > and the MUX is changed and the new sample is started. The new sample is still
> > converting and the driver returns the value from the previous conversion.
> >
>
> Yes, this was the problem in the initial driver. The fix was to wait
> for the conversion to complete
> but it seems that it doesn't always work on your setup :(.
>
> if (change) {
> + conv_time = DIV_ROUND_UP(USEC_PER_SEC, ads1015_data_rate[dr]);
> + usleep_range(conv_time, conv_time + 1);
> + }
> +
>
The result can theoretically take up to nearly 2 conversion cycles if the
request for a new channel comes in just as the previous conversion starts.
>
> > For a test I multiplied the conv_time value by 2 in the ads1015_get_adc_result
> > function. This allows time for the current sample flush out and always returns
> > the appropriate channel's value.
> >
> > Looking at the buffered mode it appears that only one channel is being accessed
> > at any time. This being the first one in the active_scan_mask found by
> > find_first_bit. So the MUX would never change is buffered mode as far I can
> > tell.
> >
> > Don't we typically want to read all of enabled channels in buffered mode?
>
> For the moment we allow only a single channel to be enabled in buffer mode.
Okay just making sure. Adding more channels would effectively divide the sampling
rate and increase the complexity of the driver.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-03-01 8:35 ` jic23
@ 2016-03-01 16:02 ` Michael Welling
2016-03-01 17:24 ` Jonathan Cameron
0 siblings, 1 reply; 45+ messages in thread
From: Michael Welling @ 2016-03-01 16:02 UTC (permalink / raw)
To: jic23
Cc: Lucas De Marchi, Daniel Baluta, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, lkml, linux-iio,
Lucas De Marchi, Guenter Roeck, eibach, linux-iio-owner
On Tue, Mar 01, 2016 at 08:35:01AM +0000, jic23@jic23.retrosnub.co.uk wrote:
> On 01.03.2016 02:42, Michael Welling wrote:
> >On Mon, Feb 29, 2016 at 10:09:10PM -0300, Lucas De Marchi wrote:
> >>On Mon, Feb 29, 2016 at 9:50 PM, Michael Welling <mwelling@ieee.org>
> >>wrote:
> >>> On Fri, Feb 05, 2016 at 03:17:18PM +0200, Daniel Baluta wrote:
> >>>> The driver has sysfs readings with runtime PM support for power saving.
> >>>> It also offers buffer support that can be used together with IIO software
> >>>> triggers.
> >>>>
> >>>
> >>> Daniel,
> >>>
> >>> So I noticed something yesterday while testing new boards.
> >>> The channels are occassionally swapping when accessing data from multiple channels.
> >>>
> >>> I wrote a simple bash script to demonstrate.
> >>
> >>This happened to me in a previous version of the patch. I remember it
> >>being fixed in the last version (or at least I could not reproduce).
> >>I'll test again tomorrow with your script.
> >>
> >
> >Here is what I believe is happening.
> >
> >The request for a conversion on a new channel comes in while the
> >conversion
> >for the previous channel is still converting. The driver waits
> >approximately
> >one conversion cycle. The previous channel completes within this timeframe
> >and the MUX is changed and the new sample is started. The new sample is
> >still
> >converting and the driver returns the value from the previous conversion.
> >
> >For a test I multiplied the conv_time value by 2 in the
> >ads1015_get_adc_result
> >function. This allows time for the current sample flush out and always
> >returns
> >the appropriate channel's value.
> >
> >Looking at the buffered mode it appears that only one channel is being
> >accessed
> >at any time. This being the first one in the active_scan_mask found by
> >find_first_bit. So the MUX would never change is buffered mode as far I
> >can
> >tell.
> >
> >Don't we typically want to read all of enabled channels in buffered mode?
> In some devices that is effectively impossible (fifo's that fill only from
> the currently
> selected channel). That is what the onehot validation callback is about.
> In this particular case it looks like doing a multichannel read is fair bit
> more time
> consuming than a single channel read and so will result in a considerable
> reduction in
> throughput. This is of course why many parts include a simple sequencer!
Yes the sampling rate would effectively be divided by the number of channels
enabled.
>
> Anyhow, supporting multi channel buffered reads could be done (probably) but
> you
> would want to fall back to the single channel case as it is now.
Understood.
>
> Jonathan
> >>Lucas De Marchi
> >--
> >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] 45+ messages in thread
* Re: [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support
2016-03-01 16:02 ` Michael Welling
@ 2016-03-01 17:24 ` Jonathan Cameron
0 siblings, 0 replies; 45+ messages in thread
From: Jonathan Cameron @ 2016-03-01 17:24 UTC (permalink / raw)
To: Michael Welling
Cc: Lucas De Marchi, Daniel Baluta, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, lkml, linux-iio,
Lucas De Marchi, Guenter Roeck, eibach, linux-iio-owner
On 1 March 2016 16:02:00 GMT+00:00, Michael Welling <mwelling@ieee.org> wrote:
>On Tue, Mar 01, 2016 at 08:35:01AM +0000, jic23@jic23.retrosnub.co.uk
>wrote:
>> On 01.03.2016 02:42, Michael Welling wrote:
>> >On Mon, Feb 29, 2016 at 10:09:10PM -0300, Lucas De Marchi wrote:
>> >>On Mon, Feb 29, 2016 at 9:50 PM, Michael Welling
><mwelling@ieee.org>
>> >>wrote:
>> >>> On Fri, Feb 05, 2016 at 03:17:18PM +0200, Daniel Baluta wrote:
>> >>>> The driver has sysfs readings with runtime PM support for power
>saving.
>> >>>> It also offers buffer support that can be used together with IIO
>software
>> >>>> triggers.
>> >>>>
>> >>>
>> >>> Daniel,
>> >>>
>> >>> So I noticed something yesterday while testing new boards.
>> >>> The channels are occassionally swapping when accessing data from
>multiple channels.
>> >>>
>> >>> I wrote a simple bash script to demonstrate.
>> >>
>> >>This happened to me in a previous version of the patch. I remember
>it
>> >>being fixed in the last version (or at least I could not
>reproduce).
>> >>I'll test again tomorrow with your script.
>> >>
>> >
>> >Here is what I believe is happening.
>> >
>> >The request for a conversion on a new channel comes in while the
>> >conversion
>> >for the previous channel is still converting. The driver waits
>> >approximately
>> >one conversion cycle. The previous channel completes within this
>timeframe
>> >and the MUX is changed and the new sample is started. The new sample
>is
>> >still
>> >converting and the driver returns the value from the previous
>conversion.
>> >
>> >For a test I multiplied the conv_time value by 2 in the
>> >ads1015_get_adc_result
>> >function. This allows time for the current sample flush out and
>always
>> >returns
>> >the appropriate channel's value.
>> >
>> >Looking at the buffered mode it appears that only one channel is
>being
>> >accessed
>> >at any time. This being the first one in the active_scan_mask found
>by
>> >find_first_bit. So the MUX would never change is buffered mode as
>far I
>> >can
>> >tell.
>> >
>> >Don't we typically want to read all of enabled channels in buffered
>mode?
>> In some devices that is effectively impossible (fifo's that fill only
>from
>> the currently
>> selected channel). That is what the onehot validation callback is
>about.
>> In this particular case it looks like doing a multichannel read is
>fair bit
>> more time
>> consuming than a single channel read and so will result in a
>considerable
>> reduction in
>> throughput. This is of course why many parts include a simple
>sequencer!
>
>Yes the sampling rate would effectively be divided by the number of
>channels
>enabled.
Worse than that as you have to explicitly change the mux for each channel requiring an i2c write.
>
>>
>> Anyhow, supporting multi channel buffered reads could be done
>(probably) but
>> you
>> would want to fall back to the single channel case as it is now.
>
>Understood.
>
>>
>> Jonathan
>> >>Lucas De Marchi
>> >--
>> >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
>--
>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
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply [flat|nested] 45+ messages in thread
end of thread, other threads:[~2016-03-01 17:25 UTC | newest]
Thread overview: 45+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-05 13:17 [PATCH v4] iio: adc: Add TI ADS1015 ADC driver support Daniel Baluta
2016-02-05 17:25 ` Michael Welling
2016-02-05 19:32 ` Daniel Baluta
2016-02-05 19:50 ` Michael Welling
2016-02-06 0:32 ` Michael Welling
2016-02-08 10:25 ` Wolfram Sang
2016-02-08 14:44 ` Daniel Baluta
[not found] ` <CAEnQRZAQjhQRdzYUvo=aKgQScAyGUgp8Ni7nx9cEo1XhN_8Xyw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-02-08 15:03 ` Daniel Baluta
2016-02-08 15:03 ` Daniel Baluta
2016-02-08 16:37 ` Michael Welling
2016-02-08 16:36 ` Michael Welling
2016-02-08 16:36 ` Michael Welling
2016-02-08 19:11 ` Sricharan
2016-02-08 19:11 ` Sricharan
2016-02-08 19:16 ` Michael Welling
2016-02-08 19:16 ` Michael Welling
2016-02-08 20:00 ` Wolfram Sang
2016-02-08 20:00 ` Wolfram Sang
2016-02-08 20:06 ` Michael Welling
2016-02-08 20:06 ` Michael Welling
2016-02-10 12:22 ` Sricharan
2016-02-10 12:22 ` Sricharan
2016-02-10 12:56 ` Daniel Baluta
[not found] ` <CAEnQRZCzkq5wFsS7Ydby+bsFJTbGVq=AX9TLkkhfMKOG8gAZHA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-02-10 15:09 ` Sricharan
2016-02-10 15:09 ` Sricharan
2016-02-10 16:36 ` Michael Welling
2016-02-10 16:36 ` Michael Welling
2016-02-17 23:53 ` Michael Welling
2016-02-18 8:45 ` Sricharan
2016-02-18 8:45 ` Sricharan
2016-02-05 21:02 ` Lucas De Marchi
2016-02-05 21:44 ` Daniel Baluta
2016-02-06 10:52 ` Jonathan Cameron
2016-02-06 11:05 ` Jonathan Cameron
2016-02-06 11:07 ` Jonathan Cameron
2016-03-01 0:50 ` Michael Welling
2016-03-01 1:09 ` Lucas De Marchi
2016-03-01 1:16 ` Michael Welling
2016-03-01 1:33 ` Michael Welling
2016-03-01 2:42 ` Michael Welling
2016-03-01 8:35 ` jic23
2016-03-01 16:02 ` Michael Welling
2016-03-01 17:24 ` Jonathan Cameron
2016-03-01 11:28 ` Daniel Baluta
2016-03-01 15:57 ` Michael Welling
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.