[RFC 0/9] spawn hrtimer trigger from client driver upon enabling buffer

[RFC 0/9] spawn hrtimer trigger from client driver upon enabling buffer

[Marc Titinger]

this builds upon the sw-trigger / hrtimer series from Daniel[1]
in order to create the trigger upon enabling the buffer when the
sampling frequency is known. This is useful for devices without
hardware streaming scheme, to avoid the complexity of creating
a separate trigger through configfs, and then setting
the sw-trigger frequency from both device0 and the trigger.

[1]: https://lkml.org/lkml/2015/10/23/369

Daniel Baluta (5):
  configfs: Allow dynamic group creation
  iio: core: Introduce IIO configfs support
  iio: core: Introduce IIO software triggers
  iio: trigger: Introduce IIO hrtimer based trigger
  iio: Documentation: Add IIO configfs documentation

Marc Titinger (4):
  iio: ina2xx: add direct IO support for TI INA2xx Power Monitors
  iio: ina2xx: add triggered buffer
  iio: buffer: allow for last-second trigger spawning from device driver
  iio: (RFC) illustrate creation/destruction of hrtimer trigger upon
    buffer enable

 Documentation/ABI/testing/configfs-iio |  21 ++
 Documentation/iio/iio_configfs.txt     |  93 +++++
 drivers/iio/Kconfig                    |  16 +
 drivers/iio/Makefile                   |   2 +
 drivers/iio/adc/Kconfig                |  12 +
 drivers/iio/adc/Makefile               |   1 +
 drivers/iio/adc/ina2xx-iio.c           | 661 +++++++++++++++++++++++++++++++++
 drivers/iio/industrialio-buffer.c      |   5 +
 drivers/iio/industrialio-configfs.c    |  50 +++
 drivers/iio/industrialio-sw-trigger.c  | 183 +++++++++
 drivers/iio/trigger/Kconfig            |  10 +
 drivers/iio/trigger/Makefile           |   2 +
 drivers/iio/trigger/iio-trig-hrtimer.c | 193 ++++++++++
 fs/configfs/dir.c                      | 110 ++++++
 include/linux/configfs.h               |  10 +
 include/linux/iio/iio.h                |   3 +
 include/linux/iio/sw_trigger.h         |  71 ++++
 17 files changed, 1443 insertions(+)
 create mode 100644 Documentation/ABI/testing/configfs-iio
 create mode 100644 Documentation/iio/iio_configfs.txt
 create mode 100644 drivers/iio/adc/ina2xx-iio.c
 create mode 100644 drivers/iio/industrialio-configfs.c
 create mode 100644 drivers/iio/industrialio-sw-trigger.c
 create mode 100644 drivers/iio/trigger/iio-trig-hrtimer.c
 create mode 100644 include/linux/iio/sw_trigger.h

-- 
1.9.1

[RFC 1/9] configfs: Allow dynamic group creation

[Marc Titinger]

From: Daniel Baluta <daniel.baluta@intel.com>

We don't want to hardcode default groups at subsystem
creation time. We export:
	* configfs_register_group
	* configfs_unregister_group
to allow drivers to programatically create/destroy groups
later, after module init time.

This is needed for IIO configfs support.

Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Acked-by: Joel Becker <jlbec@evilplan.org>
---
 fs/configfs/dir.c        | 110 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/configfs.h |  10 +++++
 2 files changed, 120 insertions(+)

diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index c81ce7f..a7a1b21 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -1636,6 +1636,116 @@ const struct file_operations configfs_dir_operations = {
 	.iterate	= configfs_readdir,
 };
 
+/**
+ * configfs_register_group - creates a parent-child relation between two groups
+ * @parent_group:	parent group
+ * @group:		child group
+ *
+ * link groups, creates dentry for the child and attaches it to the
+ * parent dentry.
+ *
+ * Return: 0 on success, negative errno code on error
+ */
+int configfs_register_group(struct config_group *parent_group,
+			    struct config_group *group)
+{
+	struct configfs_subsystem *subsys = parent_group->cg_subsys;
+	struct dentry *parent;
+	int ret;
+
+	mutex_lock(&subsys->su_mutex);
+	link_group(parent_group, group);
+	mutex_unlock(&subsys->su_mutex);
+
+	parent = parent_group->cg_item.ci_dentry;
+
+	mutex_lock_nested(&d_inode(parent)->i_mutex, I_MUTEX_PARENT);
+	ret = create_default_group(parent_group, group);
+	if (!ret) {
+		spin_lock(&configfs_dirent_lock);
+		configfs_dir_set_ready(group->cg_item.ci_dentry->d_fsdata);
+		spin_unlock(&configfs_dirent_lock);
+	}
+	mutex_unlock(&d_inode(parent)->i_mutex);
+	return ret;
+}
+EXPORT_SYMBOL(configfs_register_group);
+
+/**
+ * configfs_unregister_group() - unregisters a child group from its parent
+ * @group: parent group to be unregistered
+ *
+ * Undoes configfs_register_group()
+ */
+void configfs_unregister_group(struct config_group *group)
+{
+	struct configfs_subsystem *subsys = group->cg_subsys;
+	struct dentry *dentry = group->cg_item.ci_dentry;
+	struct dentry *parent = group->cg_item.ci_parent->ci_dentry;
+
+	mutex_lock_nested(&d_inode(parent)->i_mutex, I_MUTEX_PARENT);
+	spin_lock(&configfs_dirent_lock);
+	configfs_detach_prep(dentry, NULL);
+	spin_unlock(&configfs_dirent_lock);
+
+	configfs_detach_group(&group->cg_item);
+	d_inode(dentry)->i_flags |= S_DEAD;
+	dont_mount(dentry);
+	d_delete(dentry);
+	mutex_unlock(&d_inode(parent)->i_mutex);
+
+	dput(dentry);
+
+	mutex_lock(&subsys->su_mutex);
+	unlink_group(group);
+	mutex_unlock(&subsys->su_mutex);
+}
+EXPORT_SYMBOL(configfs_unregister_group);
+
+/**
+ * configfs_register_default_group() - allocates and registers a child group
+ * @parent_group:	parent group
+ * @name:		child group name
+ * @item_type:		child item type description
+ *
+ * boilerplate to allocate and register a child group with its parent. We need
+ * kzalloc'ed memory because child's default_group is initially empty.
+ *
+ * Return: allocated config group or ERR_PTR() on error
+ */
+struct config_group *
+configfs_register_default_group(struct config_group *parent_group,
+				const char *name,
+				struct config_item_type *item_type)
+{
+	int ret;
+	struct config_group *group;
+
+	group = kzalloc(sizeof(*group), GFP_KERNEL);
+	if (!group)
+		return ERR_PTR(-ENOMEM);
+	config_group_init_type_name(group, name, item_type);
+
+	ret = configfs_register_group(parent_group, group);
+	if (ret) {
+		kfree(group);
+		return ERR_PTR(ret);
+	}
+	return group;
+}
+EXPORT_SYMBOL(configfs_register_default_group);
+
+/**
+ * configfs_unregister_default_group() - unregisters and frees a child group
+ * @group:	the group to act on
+ */
+void configfs_unregister_default_group(struct config_group *group)
+{
+	configfs_unregister_group(group);
+	kfree(group);
+}
+EXPORT_SYMBOL(configfs_unregister_default_group);
+
 int configfs_register_subsystem(struct configfs_subsystem *subsys)
 {
 	int err;
diff --git a/include/linux/configfs.h b/include/linux/configfs.h
index 63a36e8..931ca25 100644
--- a/include/linux/configfs.h
+++ b/include/linux/configfs.h
@@ -252,6 +252,16 @@ static inline struct configfs_subsystem *to_configfs_subsystem(struct config_gro
 int configfs_register_subsystem(struct configfs_subsystem *subsys);
 void configfs_unregister_subsystem(struct configfs_subsystem *subsys);
 
+int configfs_register_group(struct config_group *parent_group,
+			    struct config_group *group);
+void configfs_unregister_group(struct config_group *group);
+
+struct config_group *
+configfs_register_default_group(struct config_group *parent_group,
+				const char *name,
+				struct config_item_type *item_type);
+void configfs_unregister_default_group(struct config_group *group);
+
 /* These functions can sleep and can alloc with GFP_KERNEL */
 /* WARNING: These cannot be called underneath configfs callbacks!! */
 int configfs_depend_item(struct configfs_subsystem *subsys, struct config_item *target);
-- 
1.9.1

[RFC 2/9] iio: core: Introduce IIO configfs support

[Marc Titinger]

From: Daniel Baluta <daniel.baluta@intel.com>

This patch creates the IIO configfs root group. The group
will appear under <mount-point>/iio/, usually /config/iio.

We introduce configfs support in IIO in order to be able to easily
create IIO objects from userspace. The first supported IIO objects
are triggers introduced with next patches.

Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Tested-by: Matt Ranostay <matt.ranostay@intel.com>
---
 drivers/iio/Kconfig                 |  8 ++++++
 drivers/iio/Makefile                |  1 +
 drivers/iio/industrialio-configfs.c | 50 +++++++++++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+)
 create mode 100644 drivers/iio/industrialio-configfs.c

diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index 4011eff..17cac8e 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -42,6 +42,14 @@ config IIO_TRIGGERED_BUFFER
 
 endif # IIO_BUFFER
 
+config IIO_CONFIGFS
+	tristate "Enable IIO configuration via configfs"
+	select CONFIGFS_FS
+	help
+	  This allows configuring various IIO bits through configfs
+	  (e.g. software triggers). For more info see
+	  Documentation/iio/iio_configfs.txt.
+
 config IIO_TRIGGER
 	bool "Enable triggered sampling support"
 	help
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index 698afc2..72e85c42 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -8,6 +8,7 @@ industrialio-$(CONFIG_IIO_BUFFER) += industrialio-buffer.o
 industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o
 industrialio-$(CONFIG_IIO_BUFFER_CB) += buffer_cb.o
 
+obj-$(CONFIG_IIO_CONFIGFS) += industrialio-configfs.o
 obj-$(CONFIG_IIO_TRIGGERED_BUFFER) += industrialio-triggered-buffer.o
 obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o
 
diff --git a/drivers/iio/industrialio-configfs.c b/drivers/iio/industrialio-configfs.c
new file mode 100644
index 0000000..83563dd
--- /dev/null
+++ b/drivers/iio/industrialio-configfs.c
@@ -0,0 +1,50 @@
+/*
+ * Industrial I/O configfs bits
+ *
+ * Copyright (c) 2015 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#include <linux/configfs.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kmod.h>
+#include <linux/slab.h>
+
+#include <linux/iio/iio.h>
+
+static struct config_item_type iio_root_group_type = {
+	.ct_owner       = THIS_MODULE,
+};
+
+struct configfs_subsystem iio_configfs_subsys = {
+	.su_group = {
+		.cg_item = {
+			.ci_namebuf = "iio",
+			.ci_type = &iio_root_group_type,
+		},
+	},
+	.su_mutex = __MUTEX_INITIALIZER(iio_configfs_subsys.su_mutex),
+};
+EXPORT_SYMBOL(iio_configfs_subsys);
+
+static int __init iio_configfs_init(void)
+{
+	config_group_init(&iio_configfs_subsys.su_group);
+
+	return configfs_register_subsystem(&iio_configfs_subsys);
+}
+module_init(iio_configfs_init);
+
+static void __exit iio_configfs_exit(void)
+{
+	configfs_unregister_subsystem(&iio_configfs_subsys);
+}
+module_exit(iio_configfs_exit);
+
+MODULE_AUTHOR("Daniel Baluta <daniel.baluta@intel.com>");
+MODULE_DESCRIPTION("Industrial I/O configfs support");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

[RFC 3/9] iio: core: Introduce IIO software triggers

[Marc Titinger]

From: Daniel Baluta <daniel.baluta@intel.com>

A software trigger associates an IIO device trigger with a software
interrupt source (e.g: timer, sysfs). This patch adds the generic
infrastructure for handling software triggers.

Software interrupts sources are kept in a iio_trigger_types_list and
registered separately when the associated kernel module is loaded.

Software triggers can be created directly from drivers or from user
space via configfs interface.

To sum up, this dynamically creates "triggers" group to be found under
/config/iio/triggers and offers the possibility of dynamically
creating trigger types groups. The first supported trigger type is
"hrtimer" found under /config/iio/triggers/hrtimer.

Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
 drivers/iio/Kconfig                   |   8 ++
 drivers/iio/Makefile                  |   1 +
 drivers/iio/industrialio-sw-trigger.c | 183 ++++++++++++++++++++++++++++++++++
 include/linux/iio/sw_trigger.h        |  71 +++++++++++++
 4 files changed, 263 insertions(+)
 create mode 100644 drivers/iio/industrialio-sw-trigger.c
 create mode 100644 include/linux/iio/sw_trigger.h

diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index 17cac8e..5dce2cf 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -66,6 +66,14 @@ config IIO_CONSUMERS_PER_TRIGGER
 	This value controls the maximum number of consumers that a
 	given trigger may handle. Default is 2.
 
+config IIO_SW_TRIGGER
+	tristate "Enable software triggers support"
+	select IIO_CONFIGFS
+	help
+	 Provides IIO core support for software triggers. A software
+	 trigger can be created via configfs or directly by a driver
+	 using the API provided.
+
 source "drivers/iio/accel/Kconfig"
 source "drivers/iio/adc/Kconfig"
 source "drivers/iio/amplifiers/Kconfig"
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index 72e85c42..43b95e9 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -9,6 +9,7 @@ industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o
 industrialio-$(CONFIG_IIO_BUFFER_CB) += buffer_cb.o
 
 obj-$(CONFIG_IIO_CONFIGFS) += industrialio-configfs.o
+obj-$(CONFIG_IIO_SW_TRIGGER) += industrialio-sw-trigger.o
 obj-$(CONFIG_IIO_TRIGGERED_BUFFER) += industrialio-triggered-buffer.o
 obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o
 
diff --git a/drivers/iio/industrialio-sw-trigger.c b/drivers/iio/industrialio-sw-trigger.c
new file mode 100644
index 0000000..4825cfd
--- /dev/null
+++ b/drivers/iio/industrialio-sw-trigger.c
@@ -0,0 +1,183 @@
+/*
+ * The Industrial I/O core, software trigger functions
+ *
+ * Copyright (c) 2015 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kmod.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+
+#include <linux/iio/sw_trigger.h>
+#include <linux/configfs.h>
+
+static struct config_group *iio_triggers_group;
+static struct config_item_type iio_trigger_type_group_type;
+
+static struct config_item_type iio_triggers_group_type = {
+	.ct_owner = THIS_MODULE,
+};
+
+static LIST_HEAD(iio_trigger_types_list);
+static DEFINE_MUTEX(iio_trigger_types_lock);
+
+static
+struct iio_sw_trigger_type *__iio_find_sw_trigger_type(const char *name,
+						       unsigned len)
+{
+	struct iio_sw_trigger_type *t = NULL, *iter;
+
+	list_for_each_entry(iter, &iio_trigger_types_list, list)
+		if (!strcmp(iter->name, name)) {
+			t = iter;
+			break;
+		}
+
+	return t;
+}
+
+int iio_register_sw_trigger_type(struct iio_sw_trigger_type *t)
+{
+	struct iio_sw_trigger_type *iter;
+	int ret = 0;
+
+	mutex_lock(&iio_trigger_types_lock);
+	iter = __iio_find_sw_trigger_type(t->name, strlen(t->name));
+	if (iter)
+		ret = -EBUSY;
+	else
+		list_add_tail(&t->list, &iio_trigger_types_list);
+	mutex_unlock(&iio_trigger_types_lock);
+
+	if (ret)
+		return ret;
+
+	t->group = configfs_register_default_group(iio_triggers_group, t->name,
+						&iio_trigger_type_group_type);
+	if (IS_ERR(t->group))
+		ret = PTR_ERR(t->group);
+
+	return ret;
+}
+EXPORT_SYMBOL(iio_register_sw_trigger_type);
+
+void iio_unregister_sw_trigger_type(struct iio_sw_trigger_type *t)
+{
+	struct iio_sw_trigger_type *iter;
+
+	mutex_lock(&iio_trigger_types_lock);
+	iter = __iio_find_sw_trigger_type(t->name, strlen(t->name));
+	if (iter)
+		list_del(&t->list);
+	mutex_unlock(&iio_trigger_types_lock);
+
+	configfs_unregister_default_group(t->group);
+}
+EXPORT_SYMBOL(iio_unregister_sw_trigger_type);
+
+static
+struct iio_sw_trigger_type *iio_get_sw_trigger_type(const char *name)
+{
+	struct iio_sw_trigger_type *t;
+
+	mutex_lock(&iio_trigger_types_lock);
+	t = __iio_find_sw_trigger_type(name, strlen(name));
+	if (t && !try_module_get(t->owner))
+		t = NULL;
+	mutex_unlock(&iio_trigger_types_lock);
+
+	return t;
+}
+
+struct iio_sw_trigger *iio_sw_trigger_create(const char *type, const char *name)
+{
+	struct iio_sw_trigger *t;
+	struct iio_sw_trigger_type *tt;
+
+	tt = iio_get_sw_trigger_type(type);
+	if (!tt) {
+		pr_err("Invalid trigger type: %s\n", type);
+		return ERR_PTR(-EINVAL);
+	}
+	t = tt->ops->probe(name);
+	if (IS_ERR(t))
+		goto out_module_put;
+
+	t->trigger_type = tt;
+
+	return t;
+out_module_put:
+	module_put(tt->owner);
+	return t;
+}
+EXPORT_SYMBOL(iio_sw_trigger_create);
+
+void iio_sw_trigger_destroy(struct iio_sw_trigger *t)
+{
+	struct iio_sw_trigger_type *tt = t->trigger_type;
+
+	tt->ops->remove(t);
+	module_put(tt->owner);
+}
+EXPORT_SYMBOL(iio_sw_trigger_destroy);
+
+static struct config_group *trigger_make_group(struct config_group *group,
+					       const char *name)
+{
+	struct iio_sw_trigger *t;
+
+	t = iio_sw_trigger_create(group->cg_item.ci_name, name);
+	if (IS_ERR(t))
+		return ERR_CAST(t);
+
+	config_item_set_name(&t->group.cg_item, "%s", name);
+
+	return &t->group;
+}
+
+static void trigger_drop_group(struct config_group *group,
+			       struct config_item *item)
+{
+	struct iio_sw_trigger *t = to_iio_sw_trigger(item);
+
+	iio_sw_trigger_destroy(t);
+	config_item_put(item);
+}
+
+static struct configfs_group_operations trigger_ops = {
+	.make_group	= &trigger_make_group,
+	.drop_item	= &trigger_drop_group,
+};
+
+static struct config_item_type iio_trigger_type_group_type = {
+	.ct_group_ops = &trigger_ops,
+	.ct_owner       = THIS_MODULE,
+};
+
+static int __init iio_sw_trigger_init(void)
+{
+	iio_triggers_group =
+		configfs_register_default_group(&iio_configfs_subsys.su_group,
+						"triggers",
+						&iio_triggers_group_type);
+	if (IS_ERR(iio_triggers_group))
+		return PTR_ERR(iio_triggers_group);
+	return 0;
+}
+module_init(iio_sw_trigger_init);
+
+static void __exit iio_sw_trigger_exit(void)
+{
+	configfs_unregister_default_group(iio_triggers_group);
+}
+module_exit(iio_sw_trigger_exit);
+
+MODULE_AUTHOR("Daniel Baluta <daniel.baluta@intel.com>");
+MODULE_DESCRIPTION("Industrial I/O software triggers support");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/iio/sw_trigger.h b/include/linux/iio/sw_trigger.h
new file mode 100644
index 0000000..c2f33b2
--- /dev/null
+++ b/include/linux/iio/sw_trigger.h
@@ -0,0 +1,71 @@
+/*
+ * Industrial I/O software trigger interface
+ *
+ * Copyright (c) 2015 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#ifndef __IIO_SW_TRIGGER
+#define __IIO_SW_TRIGGER
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/iio/iio.h>
+#include <linux/configfs.h>
+
+#define module_iio_sw_trigger_driver(__iio_sw_trigger_type) \
+	module_driver(__iio_sw_trigger_type, iio_register_sw_trigger_type, \
+		      iio_unregister_sw_trigger_type)
+
+extern struct configfs_subsystem iio_configfs_subsys;
+struct iio_sw_trigger_ops;
+
+struct iio_sw_trigger_type {
+	const char *name;
+	struct module *owner;
+	const struct iio_sw_trigger_ops *ops;
+	struct list_head list;
+	struct config_group *group;
+};
+
+struct iio_sw_trigger {
+	struct iio_trigger *trigger;
+	struct iio_sw_trigger_type *trigger_type;
+	struct config_group group;
+};
+
+struct iio_sw_trigger_ops {
+	struct iio_sw_trigger* (*probe)(const char *);
+	int (*remove)(struct iio_sw_trigger *);
+};
+
+static inline
+struct iio_sw_trigger *to_iio_sw_trigger(struct config_item *item)
+{
+	return container_of(to_config_group(item), struct iio_sw_trigger,
+			    group);
+}
+
+int iio_register_sw_trigger_type(struct iio_sw_trigger_type *tt);
+void iio_unregister_sw_trigger_type(struct iio_sw_trigger_type *tt);
+
+struct iio_sw_trigger *iio_sw_trigger_create(const char *, const char *);
+void iio_sw_trigger_destroy(struct iio_sw_trigger *);
+
+int iio_sw_trigger_type_configfs_register(struct iio_sw_trigger_type *tt);
+void iio_sw_trigger_type_configfs_unregister(struct iio_sw_trigger_type *tt);
+
+static inline
+void iio_swt_group_init_type_name(struct iio_sw_trigger *t,
+				  const char *name,
+				  struct config_item_type *type)
+{
+#ifdef CONFIG_CONFIGFS_FS
+	config_group_init_type_name(&t->group, name, type);
+#endif
+}
+
+#endif /* __IIO_SW_TRIGGER */
-- 
1.9.1

[RFC 4/9] iio: trigger: Introduce IIO hrtimer based trigger

[Marc Titinger]

From: Daniel Baluta <daniel.baluta@intel.com>

This patch registers a new IIO software trigger interrupt source
based on high resolution timers.

Notice that if configfs is enabled we create sampling_frequency
attribute allowing users to change hrtimer period (1/sampling_frequency).

The IIO hrtimer trigger has a long history, this patch is based on
an older version from Marten and Lars-Peter.

Signed-off-by: Marten Svanfeldt <marten@intuitiveaerial.com>
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
 drivers/iio/trigger/Kconfig            |  10 ++
 drivers/iio/trigger/Makefile           |   2 +
 drivers/iio/trigger/iio-trig-hrtimer.c | 193 +++++++++++++++++++++++++++++++++
 3 files changed, 205 insertions(+)
 create mode 100644 drivers/iio/trigger/iio-trig-hrtimer.c

diff --git a/drivers/iio/trigger/Kconfig b/drivers/iio/trigger/Kconfig
index 7999612..519e677 100644
--- a/drivers/iio/trigger/Kconfig
+++ b/drivers/iio/trigger/Kconfig
@@ -5,6 +5,16 @@
 
 menu "Triggers - standalone"
 
+config IIO_HRTIMER_TRIGGER
+	tristate "High resolution timer trigger"
+	depends on IIO_SW_TRIGGER
+	help
+	  Provides a frequency based IIO trigger using high resolution
+	  timers as interrupt source.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called iio-trig-hrtimer.
+
 config IIO_INTERRUPT_TRIGGER
 	tristate "Generic interrupt trigger"
 	help
diff --git a/drivers/iio/trigger/Makefile b/drivers/iio/trigger/Makefile
index 0694dae..fe06eb5 100644
--- a/drivers/iio/trigger/Makefile
+++ b/drivers/iio/trigger/Makefile
@@ -3,5 +3,7 @@
 #
 
 # When adding new entries keep the list in alphabetical order
+
+obj-$(CONFIG_IIO_HRTIMER_TRIGGER) += iio-trig-hrtimer.o
 obj-$(CONFIG_IIO_INTERRUPT_TRIGGER) += iio-trig-interrupt.o
 obj-$(CONFIG_IIO_SYSFS_TRIGGER) += iio-trig-sysfs.o
diff --git a/drivers/iio/trigger/iio-trig-hrtimer.c b/drivers/iio/trigger/iio-trig-hrtimer.c
new file mode 100644
index 0000000..5e6d451
--- /dev/null
+++ b/drivers/iio/trigger/iio-trig-hrtimer.c
@@ -0,0 +1,193 @@
+/**
+ * The industrial I/O periodic hrtimer trigger driver
+ *
+ * Copyright (C) Intuitive Aerial AB
+ * Written by Marten Svanfeldt, marten@intuitiveaerial.com
+ * Copyright (C) 2012, Analog Device Inc.
+ *	Author: Lars-Peter Clausen <lars@metafoo.de>
+ * Copyright (C) 2015, Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ */
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/hrtimer.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/sw_trigger.h>
+
+/* default sampling frequency - 100Hz */
+#define HRTIMER_DEFAULT_SAMPLING_FREQUENCY 100
+
+struct iio_hrtimer_info {
+	struct iio_sw_trigger swt;
+	struct hrtimer timer;
+	unsigned long sampling_frequency;
+	ktime_t period;
+};
+
+static struct config_item_type iio_hrtimer_type = {
+	.ct_owner = THIS_MODULE,
+};
+
+static
+ssize_t iio_hrtimer_show_sampling_frequency(struct device *dev,
+					    struct device_attribute *attr,
+					    char *buf)
+{
+	struct iio_trigger *trig = to_iio_trigger(dev);
+	struct iio_hrtimer_info *info = iio_trigger_get_drvdata(trig);
+
+	return snprintf(buf, PAGE_SIZE, "%lu\n", info->sampling_frequency);
+}
+
+static
+ssize_t iio_hrtimer_store_sampling_frequency(struct device *dev,
+					     struct device_attribute *attr,
+					     const char *buf, size_t len)
+{
+	struct iio_trigger *trig = to_iio_trigger(dev);
+	struct iio_hrtimer_info *info = iio_trigger_get_drvdata(trig);
+	unsigned long val;
+	int ret;
+
+	ret = kstrtoul(buf, 10, &val);
+	if (ret)
+		return ret;
+
+	if (!val || val > NSEC_PER_SEC)
+		return -EINVAL;
+
+	info->sampling_frequency = val;
+	info->period = ktime_set(0, NSEC_PER_SEC / val);
+
+	return len;
+}
+
+static DEVICE_ATTR(sampling_frequency, S_IRUGO | S_IWUSR,
+		   iio_hrtimer_show_sampling_frequency,
+		   iio_hrtimer_store_sampling_frequency);
+
+static struct attribute *iio_hrtimer_attrs[] = {
+	&dev_attr_sampling_frequency.attr,
+	NULL
+};
+
+static const struct attribute_group iio_hrtimer_attr_group = {
+	.attrs = iio_hrtimer_attrs,
+};
+
+static const struct attribute_group *iio_hrtimer_attr_groups[] = {
+	&iio_hrtimer_attr_group,
+	NULL
+};
+
+static enum hrtimer_restart iio_hrtimer_trig_handler(struct hrtimer *timer)
+{
+	struct iio_hrtimer_info *info;
+
+	info = container_of(timer, struct iio_hrtimer_info, timer);
+
+	hrtimer_forward_now(timer, info->period);
+	iio_trigger_poll(info->swt.trigger);
+
+	return HRTIMER_RESTART;
+}
+
+static int iio_trig_hrtimer_set_state(struct iio_trigger *trig, bool state)
+{
+	struct iio_hrtimer_info *trig_info;
+
+	trig_info = iio_trigger_get_drvdata(trig);
+
+	if (state)
+		hrtimer_start(&trig_info->timer, trig_info->period,
+			      HRTIMER_MODE_REL);
+	else
+		hrtimer_cancel(&trig_info->timer);
+
+	return 0;
+}
+
+static const struct iio_trigger_ops iio_hrtimer_trigger_ops = {
+	.owner = THIS_MODULE,
+	.set_trigger_state = iio_trig_hrtimer_set_state,
+};
+
+static struct iio_sw_trigger *iio_trig_hrtimer_probe(const char *name)
+{
+	struct iio_hrtimer_info *trig_info;
+	int ret;
+
+	trig_info = kzalloc(sizeof(*trig_info), GFP_KERNEL);
+	if (!trig_info)
+		return ERR_PTR(-ENOMEM);
+
+	trig_info->swt.trigger = iio_trigger_alloc("%s", name);
+	if (!trig_info->swt.trigger) {
+		ret = -ENOMEM;
+		goto err_free_trig_info;
+	}
+
+	iio_trigger_set_drvdata(trig_info->swt.trigger, trig_info);
+	trig_info->swt.trigger->ops = &iio_hrtimer_trigger_ops;
+	trig_info->swt.trigger->dev.groups = iio_hrtimer_attr_groups;
+
+	hrtimer_init(&trig_info->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	trig_info->timer.function = iio_hrtimer_trig_handler;
+
+	trig_info->sampling_frequency = HRTIMER_DEFAULT_SAMPLING_FREQUENCY;
+	trig_info->period = ktime_set(0, NSEC_PER_SEC /
+				      trig_info->sampling_frequency);
+
+	ret = iio_trigger_register(trig_info->swt.trigger);
+	if (ret)
+		goto err_free_trigger;
+
+	iio_swt_group_init_type_name(&trig_info->swt, name, &iio_hrtimer_type);
+	return &trig_info->swt;
+err_free_trigger:
+	iio_trigger_free(trig_info->swt.trigger);
+err_free_trig_info:
+	kfree(trig_info);
+
+	return ERR_PTR(ret);
+}
+
+static int iio_trig_hrtimer_remove(struct iio_sw_trigger *swt)
+{
+	struct iio_hrtimer_info *trig_info;
+
+	trig_info = iio_trigger_get_drvdata(swt->trigger);
+
+	iio_trigger_unregister(swt->trigger);
+
+	/* cancel the timer after unreg to make sure no one rearms it */
+	hrtimer_cancel(&trig_info->timer);
+	iio_trigger_free(swt->trigger);
+	kfree(trig_info);
+
+	return 0;
+}
+
+static const struct iio_sw_trigger_ops iio_trig_hrtimer_ops = {
+	.probe		= iio_trig_hrtimer_probe,
+	.remove		= iio_trig_hrtimer_remove,
+};
+
+static struct iio_sw_trigger_type iio_trig_hrtimer = {
+	.name = "hrtimer",
+	.owner = THIS_MODULE,
+	.ops = &iio_trig_hrtimer_ops,
+};
+
+module_iio_sw_trigger_driver(iio_trig_hrtimer);
+
+MODULE_AUTHOR("Marten Svanfeldt <marten@intuitiveaerial.com>");
+MODULE_AUTHOR("Daniel Baluta <daniel.baluta@intel.com>");
+MODULE_DESCRIPTION("Periodic hrtimer trigger for the IIO subsystem");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

[RFC 5/9] iio: Documentation: Add IIO configfs documentation

[Marc Titinger]

From: Daniel Baluta <daniel.baluta@intel.com>

Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Acked-by: Crt Mori <cmo@melexis.com>
---
 Documentation/ABI/testing/configfs-iio | 21 ++++++++
 Documentation/iio/iio_configfs.txt     | 93 ++++++++++++++++++++++++++++++++++
 2 files changed, 114 insertions(+)
 create mode 100644 Documentation/ABI/testing/configfs-iio
 create mode 100644 Documentation/iio/iio_configfs.txt

diff --git a/Documentation/ABI/testing/configfs-iio b/Documentation/ABI/testing/configfs-iio
new file mode 100644
index 0000000..2483756
--- /dev/null
+++ b/Documentation/ABI/testing/configfs-iio
@@ -0,0 +1,21 @@
+What:		/config/iio
+Date:		October 2015
+KernelVersion:	4.4
+Contact:	linux-iio@vger.kernel.org
+Description:
+		This represents Industrial IO configuration entry point
+		directory. It contains sub-groups corresponding to IIO
+		objects.
+
+What:		/config/iio/triggers
+Date:		October 2015
+KernelVersion:	4.4
+Description:
+		Industrial IO software triggers directory.
+
+What:		/config/iio/triggers/hrtimers
+Date:		October 2015
+KernelVersion:	4.4
+Description:
+		High resolution timers directory. Creating a directory here
+		will result in creating a hrtimer trigger in the IIO subsystem.
diff --git a/Documentation/iio/iio_configfs.txt b/Documentation/iio/iio_configfs.txt
new file mode 100644
index 0000000..f0add35
--- /dev/null
+++ b/Documentation/iio/iio_configfs.txt
@@ -0,0 +1,93 @@
+Industrial IIO configfs support
+
+1. Overview
+
+Configfs is a filesystem-based manager of kernel objects. IIO uses some
+objects that could be easily configured using configfs (e.g.: devices,
+triggers).
+
+See Documentation/filesystems/configfs/configfs.txt for more information
+about how configfs works.
+
+2. Usage
+
+In order to use configfs support in IIO we need to select it at compile
+time via CONFIG_IIO_CONFIGFS config option.
+
+Then, mount the configfs filesystem (usually under /config directory):
+
+$ mkdir /config
+$ mount -t configfs none /config
+
+At this point, all default IIO groups will be created and can be accessed
+under /config/iio. Next chapters will describe available IIO configuration
+objects.
+
+3. Software triggers
+
+One of the IIO default configfs groups is the "triggers" group. It is
+automagically accessible when the configfs is mounted and can be found
+under /config/iio/triggers.
+
+IIO software triggers implementation offers support for creating multiple
+trigger types. A new trigger type is usually implemented as a separate
+kernel module following the interface in include/linux/iio/sw_trigger.h:
+
+/*
+ * drivers/iio/trigger/iio-trig-sample.c
+ * sample kernel module implementing a new trigger type
+ */
+#include <linux/iio/sw_trigger.h>
+
+
+static struct iio_sw_trigger *iio_trig_sample_probe(const char *name)
+{
+	/*
+	 * This allocates and registers an IIO trigger plus other
+	 * trigger type specific initialization.
+	 */
+}
+
+static int iio_trig_hrtimer_remove(struct iio_sw_trigger *swt)
+{
+	/*
+	 * This undoes the actions in iio_trig_sample_probe
+	 */
+}
+
+static const struct iio_sw_trigger_ops iio_trig_sample_ops = {
+	.probe		= iio_trig_sample_probe,
+	.remove		= iio_trig_sample_remove,
+};
+
+static struct iio_sw_trigger_type iio_trig_sample = {
+	.name = "trig-sample",
+	.owner = THIS_MODULE,
+	.ops = &iio_trig_sample_ops,
+};
+
+module_iio_sw_trigger_driver(iio_trig_sample);
+
+Each trigger type has its own directory under /config/iio/triggers. Loading
+iio-trig-sample module will create 'trig-sample' trigger type directory
+/config/iio/triggers/trig-sample.
+
+We support the following interrupt sources (trigger types):
+	* hrtimer, uses high resolution timers as interrupt source
+
+3.1 Hrtimer triggers creation and destruction
+
+Loading iio-trig-hrtimer module will register hrtimer trigger types allowing
+users to create hrtimer triggers under /config/iio/triggers/hrtimer.
+
+e.g:
+
+$ mkdir /config/triggers/hrtimer/instance1
+$ rmdir /config/triggers/hrtimer/instance1
+
+Each trigger can have one or more attributes specific to the trigger type.
+
+3.2 "hrtimer" trigger types attributes
+
+"hrtimer" trigger type doesn't have any configurable attribute from /config dir.
+It does introduce the sampling_frequency attribute to trigger directory.
-- 
1.9.1

[RFC 6/9] iio: ina2xx: add direct IO support for TI INA2xx Power Monitors

[Marc Titinger]

Basic support or direct IO raw read, with averaging attribute.
Values are RAW, INT_PLUS_MICRO (Volt/Ampere/Watt).

Output of iio_info:

     iio:device0: ina226
      4 channels found:
        power3:  (input)
        1 channel-specific attributes found:
                attr 0: raw value: 1.150000
        voltage0:  (input)
        1 channel-specific attributes found:
                attr 0: raw value: 0.000003
        voltage1:  (input)
        1 channel-specific attributes found:
                attr 0: raw value: 4.277500
        current2:  (input)
        1 channel-specific attributes found:
                attr 0: raw value: 0.268000
        4 device-specific attributes found:
                attr 0: sampling_frequency_available value: 61 120 236...
                attr 1: in_averaging_steps value: 4
                attr 2: in_calibscale value: 10000
                attr 3: in_sampling_frequency value: 1506

Tested with ina226, on BeagleBoneBlack.

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

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

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 50c103d..ebbfff9 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -183,6 +183,15 @@ config EXYNOS_ADC
 	  To compile this driver as a module, choose M here: the module will be
 	  called exynos_adc.
 
+config INA2XX_IIO
+	tristate "Texas Instruments INA2xx Power Monitors IIO driver"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  Say yes here to build support for TI INA2xx familly Power Monitors.
+
+	  Note that this is not the existing hwmon interface for this device.
+
 config LP8788_ADC
 	tristate "LP8788 ADC driver"
 	depends on MFD_LP8788
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index a096210..74e4341 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_BERLIN2_ADC) += berlin2-adc.o
 obj-$(CONFIG_DA9150_GPADC) += da9150-gpadc.o
 obj-$(CONFIG_CC10001_ADC) += cc10001_adc.o
 obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o
+obj-$(CONFIG_INA2XX_IIO) += ina2xx-iio.o
 obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
 obj-$(CONFIG_MAX1027) += max1027.o
 obj-$(CONFIG_MAX1363) += max1363.o
diff --git a/drivers/iio/adc/ina2xx-iio.c b/drivers/iio/adc/ina2xx-iio.c
new file mode 100644
index 0000000..d4dd908
--- /dev/null
+++ b/drivers/iio/adc/ina2xx-iio.c
@@ -0,0 +1,515 @@
+/*
+ * INA2XX Current and Power Monitors
+ *
+ * Copyright 2015 Baylibre SAS.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Based on linux/drivers/iio/adc/ad7291.c
+ * Copyright 2010-2011 Analog Devices Inc.
+ *
+ * Based on linux/drivers/hwmon/ina2xx.c
+ * Copyright 2012 Lothar Felten <l-felten@ti.com>
+ *
+ * Licensed under the GPL-2 or later.
+ *
+ * IIO driver for INA219-220-226-230-231
+ *
+ * Configurable 7-bit I2C slave address from 0x40 to 0x4F
+ */
+#include <linux/module.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <linux/platform_data/ina2xx.h>
+
+#include <linux/util_macros.h>
+
+/*
+ * INA2XX registers definition
+ */
+/* common register definitions */
+#define INA2XX_CONFIG                   0x00
+#define INA2XX_SHUNT_VOLTAGE            0x01	/* readonly */
+#define INA2XX_BUS_VOLTAGE              0x02	/* readonly */
+#define INA2XX_POWER                    0x03	/* readonly */
+#define INA2XX_CURRENT                  0x04	/* readonly */
+#define INA2XX_CALIBRATION              0x05
+
+/* register count */
+#define INA219_REGISTERS                6
+#define INA226_REGISTERS                8
+#define INA2XX_MAX_REGISTERS            8
+
+/* settings - depend on use case */
+#define INA219_CONFIG_DEFAULT           0x399F	/* PGA=8 */
+#define INA226_CONFIG_DEFAULT           0x4327
+#define INA226_DEFAULT_AVG              4
+#define INA226_DEFAULT_FREQ             455
+
+#define INA2XX_RSHUNT_DEFAULT           10000
+
+/* bit mask for reading the averaging setting in the configuration register */
+#define INA226_AVG_RD_MASK              GENMASK(11, 9)
+#define INA226_READ_AVG(reg)            (((reg) & INA226_AVG_RD_MASK) >> 9)
+#define INA226_SHIFT_AVG(val)           ((val) << 9)
+
+#define INA226_SFREQ_RD_MASK            GENMASK(8, 3)
+
+
+static bool ina2xx_is_writeable_reg(struct device *dev, unsigned int reg)
+{
+	return (reg == INA2XX_CONFIG) || (reg == INA2XX_CALIBRATION);
+}
+
+static bool ina2xx_is_volatile_reg(struct device *dev, unsigned int reg)
+{
+	return (reg != INA2XX_CONFIG);
+}
+
+static struct regmap_config ina2xx_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 16,
+
+	.writeable_reg = ina2xx_is_writeable_reg,
+	.volatile_reg = ina2xx_is_volatile_reg,
+};
+
+enum ina2xx_ids { ina219, ina226 };
+
+struct ina2xx_config {
+	u16 config_default;
+	int calibration_factor;
+	int registers;
+	int shunt_div;
+	int bus_voltage_shift;
+	int bus_voltage_lsb;	/* uV */
+	int power_lsb;		/* uW */
+};
+
+struct ina2xx_chip_info {
+	const struct ina2xx_config *config;
+	struct mutex state_lock;
+	long rshunt;
+	int avg;
+	int freq;
+	int period_us;
+	struct regmap *regmap;
+};
+
+static const struct ina2xx_config ina2xx_config[] = {
+	[ina219] = {
+		    .config_default = INA219_CONFIG_DEFAULT,
+		    .calibration_factor = 40960000,
+		    .registers = INA219_REGISTERS,
+		    .shunt_div = 100,
+		    .bus_voltage_shift = 3,
+		    .bus_voltage_lsb = 4000,
+		    .power_lsb = 20000,
+		    },
+	[ina226] = {
+		    .config_default = INA226_CONFIG_DEFAULT,
+		    .calibration_factor = 5120000,
+		    .registers = INA226_REGISTERS,
+		    .shunt_div = 400,
+		    .bus_voltage_shift = 0,
+		    .bus_voltage_lsb = 1250,
+		    .power_lsb = 25000,
+		    },
+};
+
+static int ina2xx_get_value(struct ina2xx_chip_info *chip, u8 reg,
+			    unsigned int regval, int *val, int *uval)
+{
+	*val = 0;
+
+	switch (reg) {
+	case INA2XX_SHUNT_VOLTAGE:
+		/* signed register */
+		*uval = DIV_ROUND_CLOSEST((s16) regval,
+					  chip->config->shunt_div);
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	case INA2XX_BUS_VOLTAGE:
+		*uval = (regval >> chip->config->bus_voltage_shift)
+			* chip->config->bus_voltage_lsb;
+		*val = *uval/1000000;
+		*uval = *uval % 1000000;
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	case INA2XX_POWER:
+		*uval = regval * chip->config->power_lsb;
+		*val = *uval/1000000;
+		*uval = *uval % 1000000;
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	case INA2XX_CURRENT:
+		/* signed register, LSB=1mA (selected), in mA */
+		*uval = (s16) regval * 1000;
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	case INA2XX_CALIBRATION:
+		*val = DIV_ROUND_CLOSEST(chip->config->calibration_factor,
+					regval);
+		return IIO_VAL_INT;
+
+	default:
+		/* programmer goofed */
+		WARN_ON_ONCE(1);
+	}
+	return -EINVAL;
+}
+
+static int ina2xx_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	int ret;
+	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
+	unsigned int regval;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = regmap_read(chip->regmap, chan->address, &regval);
+		if (ret < 0)
+			return ret;
+
+		return ina2xx_get_value(chip, chan->address, regval, val, val2);
+
+	case IIO_CHAN_INFO_AVERAGE_RAW:
+		*val = chip->avg;
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_CALIBSCALE:
+		ret = regmap_read(chip->regmap, INA2XX_CALIBRATION, &regval);
+		if (ret < 0)
+			return ret;
+
+		return ina2xx_get_value(chip, INA2XX_CALIBRATION, regval,
+					val, val2);
+
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*val = chip->freq;
+		return IIO_VAL_INT;
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int ina2xx_calibrate(struct ina2xx_chip_info *chip)
+{
+	u16 val = DIV_ROUND_CLOSEST(chip->config->calibration_factor,
+				    chip->rshunt);
+
+	return regmap_write(chip->regmap, INA2XX_CALIBRATION, val);
+}
+
+
+/*
+ * Available averaging rates for ina226. The indices correspond with
+ * the bit values expected by the chip (according to the ina226 datasheet,
+ * table 3 AVG bit settings, found at
+ * http://www.ti.com/lit/ds/symlink/ina226.pdf.
+ */
+static const int ina226_avg_tab[] = { 1, 4, 16, 64, 128, 256, 512, 1024 };
+
+static unsigned int ina226_set_average(struct ina2xx_chip_info *chip,
+				       unsigned int val,
+				       unsigned int *config)
+{
+	int bits;
+
+	if (val > 1024 || val < 1)
+		return -EINVAL;
+
+	bits = find_closest(val, ina226_avg_tab,
+			    ARRAY_SIZE(ina226_avg_tab));
+
+	chip->avg = ina226_avg_tab[bits];
+
+	*config &= ~INA226_AVG_RD_MASK;
+	*config |= INA226_SHIFT_AVG(bits) & INA226_AVG_RD_MASK;
+
+	return 0;
+}
+
+/* Conversion times in uS */
+static const int ina226_conv_time_tab[] = { 140, 204, 332, 588, 1100,
+	2116, 4156, 8244};
+
+static unsigned int ina226_set_frequency(struct ina2xx_chip_info *chip,
+					 unsigned int val,
+					 unsigned int *config)
+{
+	int bits;
+
+	if (val > 3550  || val < 50)
+		return -EINVAL;
+
+	/* integration time in uS, for both voltage channels */
+	val = DIV_ROUND_CLOSEST(1000000, 2 * val);
+
+	bits = find_closest(val, ina226_conv_time_tab,
+			    ARRAY_SIZE(ina226_conv_time_tab));
+
+	chip->period_us = 2 * ina226_conv_time_tab[bits];
+
+	chip->freq = DIV_ROUND_CLOSEST(1000000, chip->period_us);
+
+	*config &= ~INA226_SFREQ_RD_MASK;
+	*config |= (bits << 3) | (bits << 6);
+
+	return 0;
+}
+
+
+static int ina2xx_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long mask)
+{
+	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
+	int ret = 0;
+	unsigned int config, tmp;
+
+	mutex_lock(&chip->state_lock);
+
+	ret = regmap_read(chip->regmap, INA2XX_CONFIG, &config);
+	if (ret < 0)
+		goto _err;
+
+	tmp = config;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_AVERAGE_RAW:
+
+		ret = ina226_set_average(chip, val, &tmp);
+		break;
+
+	case IIO_CHAN_INFO_SAMP_FREQ:
+
+		ret = ina226_set_frequency(chip, val, &tmp);
+		break;
+
+	default:
+		ret = -EINVAL;
+	}
+
+	if (!ret && (tmp != config))
+		ret = regmap_write(chip->regmap, INA2XX_CONFIG, config);
+_err:
+	mutex_unlock(&chip->state_lock);
+
+	return ret;
+}
+
+
+static ssize_t ina2xx_averaging_steps_show(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf)
+{
+	struct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));
+
+	return sprintf(buf, "%d\n", chip->avg);
+}
+
+
+static ssize_t ina2xx_averaging_steps_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t len)
+{
+	unsigned long val;
+	int ret;
+
+	ret = kstrtoul((const char *) buf, 10, &val);
+	if (ret)
+		return -EINVAL;
+
+	/* unexposed missuse of INFO_AVERAGE_RAW, until a proper ABI for the
+	 * averaging steps setting is specified.
+	 */
+	ret  = ina2xx_write_raw(dev_to_iio_dev(dev), NULL, val, 0,
+				IIO_CHAN_INFO_AVERAGE_RAW);
+	if (ret < 0)
+		return ret;
+
+	return len;
+}
+
+
+#define INA2XX_CHAN(_type, _index, _address) { \
+	.type = _type, \
+	.address = _address, \
+	.indexed = 1, \
+	.channel = (_index), \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+	.info_mask_shared_by_dir = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
+				   BIT(IIO_CHAN_INFO_CALIBSCALE), \
+	.scan_index = (_index), \
+	.scan_type = { \
+		.sign = 'u', \
+		.realbits = 16, \
+		.storagebits = 16, \
+		.shift = 0, \
+	.endianness = IIO_BE, \
+	} \
+}
+
+static const struct iio_chan_spec ina2xx_channels[] = {
+	INA2XX_CHAN(IIO_VOLTAGE, 0, INA2XX_SHUNT_VOLTAGE),
+	INA2XX_CHAN(IIO_VOLTAGE, 1, INA2XX_BUS_VOLTAGE),
+	INA2XX_CHAN(IIO_CURRENT, 2, INA2XX_CURRENT),
+	INA2XX_CHAN(IIO_POWER, 3, INA2XX_POWER),
+	IIO_CHAN_SOFT_TIMESTAMP(4),
+};
+
+static int ina2xx_debug_reg(struct iio_dev *indio_dev,
+			    unsigned reg, unsigned writeval, unsigned *readval)
+{
+	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
+
+	if (!readval)
+		return regmap_write(chip->regmap, reg, writeval);
+
+	return regmap_read(chip->regmap, reg, readval);
+}
+
+/* frequencies matching the cummulated integration times for vshunt and vbus */
+static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("61 120 236 455 850 1506 2450 3571");
+
+static IIO_DEVICE_ATTR(in_averaging_steps, S_IRUGO | S_IWUSR,
+		       ina2xx_averaging_steps_show,
+		       ina2xx_averaging_steps_store, 0);
+
+static struct attribute *ina2xx_attributes[] = {
+	&iio_dev_attr_in_averaging_steps.dev_attr.attr,
+	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group ina2xx_attribute_group = {
+	.attrs = ina2xx_attributes,
+};
+
+static const struct iio_info ina2xx_info = {
+	.debugfs_reg_access = &ina2xx_debug_reg,
+	.read_raw = &ina2xx_read_raw,
+	.write_raw = &ina2xx_write_raw,
+	.attrs = &ina2xx_attribute_group,
+	.driver_module = THIS_MODULE,
+};
+
+/* Initialize the configuration and calibration registers. */
+static int ina2xx_init(struct ina2xx_chip_info *chip, unsigned int config)
+{
+	int ret = regmap_write(chip->regmap, INA2XX_CONFIG, config);
+
+	if (ret < 0)
+		return ret;
+	/*
+	 * Set current LSB to 1mA, shunt is in uOhms
+	 * (equation 13 in datasheet).
+	 */
+	return ina2xx_calibrate(chip);
+}
+
+static int ina2xx_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct ina2xx_chip_info *chip;
+	struct iio_dev *indio_dev;
+	int ret;
+	unsigned int val;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	chip = iio_priv(indio_dev);
+
+	/* set the device type */
+	chip->config = &ina2xx_config[id->driver_data];
+
+	if (of_property_read_u32(client->dev.of_node,
+				 "shunt-resistor", &val) < 0) {
+		struct ina2xx_platform_data *pdata =
+		    dev_get_platdata(&client->dev);
+
+		if (pdata)
+			val = pdata->shunt_uohms;
+		else
+			val = INA2XX_RSHUNT_DEFAULT;
+	}
+
+	if (val <= 0 || val > chip->config->calibration_factor)
+		return -ENODEV;
+
+	chip->rshunt = val;
+
+	ina2xx_regmap_config.max_register = chip->config->registers;
+
+	mutex_init(&chip->state_lock);
+
+	/* this is only used for device removal purposes */
+	i2c_set_clientdata(client, indio_dev);
+
+	indio_dev->name = id->name;
+	indio_dev->channels = ina2xx_channels;
+	indio_dev->num_channels = ARRAY_SIZE(ina2xx_channels);
+
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->info = &ina2xx_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	chip->regmap = devm_regmap_init_i2c(client, &ina2xx_regmap_config);
+	if (IS_ERR(chip->regmap)) {
+		dev_err(&client->dev, "failed to allocate register map\n");
+		return PTR_ERR(chip->regmap);
+	}
+
+	/* Patch the current config register with default. */
+	val = chip->config->config_default;
+	if (id->driver_data == ina226) {
+		ina226_set_average(chip, INA226_DEFAULT_AVG, &val);
+		ina226_set_frequency(chip, INA226_DEFAULT_FREQ, &val);
+	}
+
+	ret = ina2xx_init(chip, val);
+	if (ret < 0) {
+		dev_err(&client->dev, "error configuring the device: %d\n",
+			ret);
+		return -ENODEV;
+	}
+
+	return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static const struct i2c_device_id ina2xx_id[] = {
+	{"ina219", ina219},
+	{"ina220", ina219},
+	{"ina226", ina226},
+	{"ina230", ina226},
+	{"ina231", ina226},
+	{}
+};
+
+MODULE_DEVICE_TABLE(i2c, ina2xx_id);
+
+static struct i2c_driver ina2xx_driver = {
+	.driver = {
+		   .name = KBUILD_MODNAME,
+		   },
+	.probe = ina2xx_probe,
+	.id_table = ina2xx_id,
+};
+
+module_i2c_driver(ina2xx_driver);
+
+MODULE_AUTHOR("Marc Titinger <marc.titinger@baylibre.com>");
+MODULE_DESCRIPTION("Texas Instruments INA2XX ADC driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

[RFC 7/9] iio: ina2xx: add triggered buffer

[Marc Titinger]

Signed-off-by: Marc Titinger <mtitinger@baylibre.com>
---
 drivers/iio/adc/Kconfig      |  3 ++
 drivers/iio/adc/ina2xx-iio.c | 74 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index ebbfff9..929cfb0 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -187,6 +187,9 @@ config INA2XX_IIO
 	tristate "Texas Instruments INA2xx Power Monitors IIO driver"
 	depends on I2C
 	select REGMAP_I2C
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	select IIO_HRTIMER_TRIGGER
 	help
 	  Say yes here to build support for TI INA2xx familly Power Monitors.
 
diff --git a/drivers/iio/adc/ina2xx-iio.c b/drivers/iio/adc/ina2xx-iio.c
index d4dd908..e47f30d 100644
--- a/drivers/iio/adc/ina2xx-iio.c
+++ b/drivers/iio/adc/ina2xx-iio.c
@@ -28,6 +28,10 @@
 
 #include <linux/util_macros.h>
 
+#include <linux/iio/buffer.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+
 /*
  * INA2XX registers definition
  */
@@ -294,6 +298,10 @@ static int ina2xx_write_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_SAMP_FREQ:
 
 		ret = ina226_set_frequency(chip, val, &tmp);
+
+		trace_printk("Enabling buffer w/ freq = %d, avg =%u, period= %u\n",
+                    chip->freq, chip->avg, chip->period_us );
+
 		break;
 
 	default:
@@ -379,6 +387,58 @@ static int ina2xx_debug_reg(struct iio_dev *indio_dev,
 	return regmap_read(chip->regmap, reg, readval);
 }
 
+static s64 prev_ns;
+
+static irqreturn_t ina2xx_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
+	unsigned short data[8];
+	int bit, ret = 0, i = 0;
+
+	unsigned long buffer_us = 0, elapsed_us = 0;
+	s64 time_a, time_b;
+
+	time_a = iio_get_time_ns();
+
+	/* Single register reads: bulk_read will not work with ina226
+	* as there is no auto-increment of the address register for
+	* data length longer than 16bits.
+	*/
+	for_each_set_bit(bit, indio_dev->active_scan_mask,
+					indio_dev->masklength) {
+		unsigned int val;
+
+		ret = regmap_read(chip->regmap,
+				  INA2XX_SHUNT_VOLTAGE + bit, &val);
+		if (ret < 0)
+			goto _err;
+
+		data[i++] = val;
+	}
+
+	time_b = iio_get_time_ns();
+
+	iio_push_to_buffers_with_timestamp(indio_dev, (unsigned int *)data,
+					   time_b);
+
+	buffer_us = (unsigned long)(time_b - time_a) / 1000;
+	elapsed_us = (unsigned long)(time_a - prev_ns) / 1000;
+
+	/* delais in uS */
+	trace_printk("T[k]-T[k_1] = %lu, xfer %lu", elapsed_us, buffer_us);
+
+	ret = IRQ_HANDLED;
+
+        prev_ns = time_a;
+
+_err:
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return ret;
+}
+
 /* frequencies matching the cummulated integration times for vshunt and vbus */
 static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("61 120 236 455 850 1506 2450 3571");
 
@@ -486,9 +546,22 @@ static int ina2xx_probe(struct i2c_client *client,
 		return -ENODEV;
 	}
 
+	ret = iio_triggered_buffer_setup(indio_dev, NULL,
+					&ina2xx_trigger_handler, NULL);
+	if (ret)
+		return ret;
+
 	return devm_iio_device_register(&client->dev, indio_dev);
 }
 
+static int ina2xx_remove(struct i2c_client *client)
+{
+	iio_triggered_buffer_cleanup(dev_get_drvdata(&client->dev));
+
+	return 0;
+}
+
+
 static const struct i2c_device_id ina2xx_id[] = {
 	{"ina219", ina219},
 	{"ina220", ina219},
@@ -505,6 +578,7 @@ static struct i2c_driver ina2xx_driver = {
 		   .name = KBUILD_MODNAME,
 		   },
 	.probe = ina2xx_probe,
+	.remove = ina2xx_remove,
 	.id_table = ina2xx_id,
 };
 
-- 
1.9.1

[RFC 8/9] iio: buffer: allow for last-second trigger spawning from device driver

[Marc Titinger]

The hrtimer sw-trigger allow for polling mode on devices w/o hard irq
trigger source, but setting the frequency from userland for both the
hrtimer trigger device and the adc is error prone.

Make adc drivers able to setup the sw-trigger at the last second when the
buffer is enabled, and the sampling frequency is known.

enable_trigger is called from verify_update, before the classical setup_ops
are called in buffers_enable. This gives a chance to complete the setup of
indio_dev->trig.

Signed-off-by: Marc Titinger <mtitinger@baylibre.com>
---
 drivers/iio/industrialio-buffer.c | 5 +++++
 include/linux/iio/iio.h           | 3 +++
 2 files changed, 8 insertions(+)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index d7e908a..ba7abd4 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -647,6 +647,11 @@ static int iio_verify_update(struct iio_dev *indio_dev,
 	if (insert_buffer)
 		modes &= insert_buffer->access->modes;
 
+	if (indio_dev->setup_ops &&
+	    indio_dev->setup_ops->enable_trigger &&
+	   (indio_dev->setup_ops->enable_trigger(indio_dev) < 0))
+		return -ENXIO;
+
 	/* Definitely possible for devices to support both of these. */
 	if ((modes & INDIO_BUFFER_TRIGGERED) && indio_dev->trig) {
 		config->mode = INDIO_BUFFER_TRIGGERED;
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 7bb7f67..8f82113 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -419,6 +419,8 @@ struct iio_info {
 
 /**
  * struct iio_buffer_setup_ops - buffer setup related callbacks
+ * @enable_trigger:	[DRIVER] function to call if a trigger is instancied
+ *				 upon enabling the buffer (sw triggers)
  * @preenable:		[DRIVER] function to run prior to marking buffer enabled
  * @postenable:		[DRIVER] function to run after marking buffer enabled
  * @predisable:		[DRIVER] function to run prior to marking buffer
@@ -428,6 +430,7 @@ struct iio_info {
  *			scan mask is valid for the device.
  */
 struct iio_buffer_setup_ops {
+	int (*enable_trigger)(struct iio_dev *);
 	int (*preenable)(struct iio_dev *);
 	int (*postenable)(struct iio_dev *);
 	int (*predisable)(struct iio_dev *);
-- 
1.9.1

[RFC 9/9] iio: (RFC) illustrate creation/destruction of hrtimer trigger upon buffer enable

[Marc Titinger]

This also raises the question of how to programmatically set the period of
the hrtimer from the owner driver, I had to locally copy iio_hrtimer_info
Maybe this should go to linux/iio/hrtimer_trigger.h ?

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

diff --git a/drivers/iio/adc/ina2xx-iio.c b/drivers/iio/adc/ina2xx-iio.c
index e47f30d..5f61296b 100644
--- a/drivers/iio/adc/ina2xx-iio.c
+++ b/drivers/iio/adc/ina2xx-iio.c
@@ -19,6 +19,7 @@
  *
  * Configurable 7-bit I2C slave address from 0x40 to 0x4F
  */
+
 #include <linux/module.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
@@ -32,6 +33,10 @@
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/triggered_buffer.h>
 
+#include <linux/iio/trigger.h>
+#include <linux/iio/sw_trigger.h>
+
+
 /*
  * INA2XX registers definition
  */
@@ -96,6 +101,7 @@ struct ina2xx_config {
 
 struct ina2xx_chip_info {
 	const struct ina2xx_config *config;
+	struct iio_sw_trigger *swtrig;
 	struct mutex state_lock;
 	long rshunt;
 	int avg;
@@ -283,6 +289,11 @@ static int ina2xx_write_raw(struct iio_dev *indio_dev,
 
 	mutex_lock(&chip->state_lock);
 
+	if (iio_buffer_enabled(indio_dev)) {
+		ret = -EBUSY;
+		goto _err;
+	}
+
 	ret = regmap_read(chip->regmap, INA2XX_CONFIG, &config);
 	if (ret < 0)
 		goto _err;
@@ -316,6 +327,13 @@ _err:
 	return ret;
 }
 
+/* FIXME */
+struct iio_hrtimer_info {
+        struct iio_sw_trigger swt;
+        struct hrtimer timer;
+        unsigned long sampling_frequency;
+        ktime_t period;
+};
 
 static ssize_t ina2xx_averaging_steps_show(struct device *dev,
 					   struct device_attribute *attr,
@@ -387,6 +405,7 @@ static int ina2xx_debug_reg(struct iio_dev *indio_dev,
 	return regmap_read(chip->regmap, reg, readval);
 }
 
+
 static s64 prev_ns;
 
 static irqreturn_t ina2xx_trigger_handler(int irq, void *p)
@@ -478,6 +497,58 @@ static int ina2xx_init(struct ina2xx_chip_info *chip, unsigned int config)
 	return ina2xx_calibrate(chip);
 }
 
+
+static int ina2xx_trigger_create(struct iio_dev *indio_dev)
+{
+	struct iio_sw_trigger *swtrig;
+	struct iio_hrtimer_info *info;
+	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
+
+	swtrig = iio_sw_trigger_create("hrtimer", indio_dev->name);
+	if (IS_ERR(swtrig))
+		return -EINVAL;
+
+	info = iio_trigger_get_drvdata(swtrig->trigger);
+
+	mutex_lock(&chip->state_lock);
+
+	info->sampling_frequency = chip->freq;
+	info->period = ktime_set(0, NSEC_PER_SEC / chip->freq);
+
+	chip->swtrig = swtrig;
+	indio_dev->trig = swtrig->trigger;
+
+	mutex_unlock(&chip->state_lock);
+
+	iio_trigger_get(indio_dev->trig);
+
+	return 0;
+}
+
+int ina2xx_trigger_destroy(struct iio_dev *indio_dev)
+{
+	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
+
+	mutex_lock(&chip->state_lock);
+
+	iio_trigger_put(indio_dev->trig);
+	iio_sw_trigger_destroy(chip->swtrig);
+
+	indio_dev->trig = NULL;
+
+	mutex_unlock(&chip->state_lock);
+
+	return 0;
+}
+
+
+static const struct iio_buffer_setup_ops ina2xx_buffer_setup_ops = {
+	.enable_trigger = &ina2xx_trigger_create,
+	.postenable = &iio_triggered_buffer_postenable,
+	.predisable = &iio_triggered_buffer_predisable,
+	.postdisable = &ina2xx_trigger_destroy,
+};
+
 static int ina2xx_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
@@ -547,7 +618,8 @@ static int ina2xx_probe(struct i2c_client *client,
 	}
 
 	ret = iio_triggered_buffer_setup(indio_dev, NULL,
-					&ina2xx_trigger_handler, NULL);
+					&ina2xx_trigger_handler,
+					&ina2xx_buffer_setup_ops);
 	if (ret)
 		return ret;
 
-- 
1.9.1

Re: [RFC 5/9] iio: Documentation: Add IIO configfs documentation

[Crt Mori]

This has my acked-by, yet it is missing a commit message? Can we get that
and the removed From: as I think this is a new patch and not resend of Daniel's?
Also it would need your signed-off-by in that case

On 18 November 2015 at 15:38, Marc Titinger <mtitinger@baylibre.com> wrote:
> From: Daniel Baluta <daniel.baluta@intel.com>
>
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> Acked-by: Crt Mori <cmo@melexis.com>
> ---
>  Documentation/ABI/testing/configfs-iio | 21 ++++++++
>  Documentation/iio/iio_configfs.txt     | 93 ++++++++++++++++++++++++++++++++++
>  2 files changed, 114 insertions(+)
>  create mode 100644 Documentation/ABI/testing/configfs-iio
>  create mode 100644 Documentation/iio/iio_configfs.txt
>
> diff --git a/Documentation/ABI/testing/configfs-iio b/Documentation/ABI/testing/configfs-iio
> new file mode 100644
> index 0000000..2483756
> --- /dev/null
> +++ b/Documentation/ABI/testing/configfs-iio
> @@ -0,0 +1,21 @@
> +What:          /config/iio
> +Date:          October 2015
> +KernelVersion: 4.4
> +Contact:       linux-iio@vger.kernel.org
> +Description:
> +               This represents Industrial IO configuration entry point
> +               directory. It contains sub-groups corresponding to IIO
> +               objects.
> +
> +What:          /config/iio/triggers
> +Date:          October 2015
> +KernelVersion: 4.4
> +Description:
> +               Industrial IO software triggers directory.
> +
> +What:          /config/iio/triggers/hrtimers
> +Date:          October 2015
> +KernelVersion: 4.4
> +Description:
> +               High resolution timers directory. Creating a directory here
> +               will result in creating a hrtimer trigger in the IIO subsystem.
> diff --git a/Documentation/iio/iio_configfs.txt b/Documentation/iio/iio_configfs.txt
> new file mode 100644
> index 0000000..f0add35
> --- /dev/null
> +++ b/Documentation/iio/iio_configfs.txt
> @@ -0,0 +1,93 @@
> +Industrial IIO configfs support
> +
> +1. Overview
> +
> +Configfs is a filesystem-based manager of kernel objects. IIO uses some
> +objects that could be easily configured using configfs (e.g.: devices,
> +triggers).
> +
> +See Documentation/filesystems/configfs/configfs.txt for more information
> +about how configfs works.
> +
> +2. Usage
> +
> +In order to use configfs support in IIO we need to select it at compile
> +time via CONFIG_IIO_CONFIGFS config option.
> +
> +Then, mount the configfs filesystem (usually under /config directory):
> +
> +$ mkdir /config
> +$ mount -t configfs none /config
> +
> +At this point, all default IIO groups will be created and can be accessed
> +under /config/iio. Next chapters will describe available IIO configuration
> +objects.
> +
> +3. Software triggers
> +
> +One of the IIO default configfs groups is the "triggers" group. It is
> +automagically accessible when the configfs is mounted and can be found
> +under /config/iio/triggers.
> +
> +IIO software triggers implementation offers support for creating multiple
> +trigger types. A new trigger type is usually implemented as a separate
> +kernel module following the interface in include/linux/iio/sw_trigger.h:
> +
> +/*
> + * drivers/iio/trigger/iio-trig-sample.c
> + * sample kernel module implementing a new trigger type
> + */
> +#include <linux/iio/sw_trigger.h>
> +
> +
> +static struct iio_sw_trigger *iio_trig_sample_probe(const char *name)
> +{
> +       /*
> +        * This allocates and registers an IIO trigger plus other
> +        * trigger type specific initialization.
> +        */
> +}
> +
> +static int iio_trig_hrtimer_remove(struct iio_sw_trigger *swt)
> +{
> +       /*
> +        * This undoes the actions in iio_trig_sample_probe
> +        */
> +}
> +
> +static const struct iio_sw_trigger_ops iio_trig_sample_ops = {
> +       .probe          = iio_trig_sample_probe,
> +       .remove         = iio_trig_sample_remove,
> +};
> +
> +static struct iio_sw_trigger_type iio_trig_sample = {
> +       .name = "trig-sample",
> +       .owner = THIS_MODULE,
> +       .ops = &iio_trig_sample_ops,
> +};
> +
> +module_iio_sw_trigger_driver(iio_trig_sample);
> +
> +Each trigger type has its own directory under /config/iio/triggers. Loading
> +iio-trig-sample module will create 'trig-sample' trigger type directory
> +/config/iio/triggers/trig-sample.
> +
> +We support the following interrupt sources (trigger types):
> +       * hrtimer, uses high resolution timers as interrupt source
> +
> +3.1 Hrtimer triggers creation and destruction
> +
> +Loading iio-trig-hrtimer module will register hrtimer trigger types allowing
> +users to create hrtimer triggers under /config/iio/triggers/hrtimer.
> +
> +e.g:
> +
> +$ mkdir /config/triggers/hrtimer/instance1
> +$ rmdir /config/triggers/hrtimer/instance1
> +
> +Each trigger can have one or more attributes specific to the trigger type.
> +
> +3.2 "hrtimer" trigger types attributes
> +
> +"hrtimer" trigger type doesn't have any configurable attribute from /config dir.
> +It does introduce the sampling_frequency attribute to trigger directory.
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 5/9] iio: Documentation: Add IIO configfs documentation

[Marc Titinger]

On 18/11/2015 16:38, Crt Mori wrote:
> This has my acked-by, yet it is missing a commit message? Can we get that
> and the removed From: as I think this is a new patch and not resend of Daniel's?
> Also it would need your signed-off-by in that case

You've lost me: I think the commit message is the brief line only.
The patch is as-is from Daniel's series.

Cheers,
M.


>
> On 18 November 2015 at 15:38, Marc Titinger <mtitinger@baylibre.com> wrote:
>> From: Daniel Baluta <daniel.baluta@intel.com>
>>
>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
>> Acked-by: Crt Mori <cmo@melexis.com>
>> ---
>>   Documentation/ABI/testing/configfs-iio | 21 ++++++++
>>   Documentation/iio/iio_configfs.txt     | 93 ++++++++++++++++++++++++++++++++++
>>   2 files changed, 114 insertions(+)
>>   create mode 100644 Documentation/ABI/testing/configfs-iio
>>   create mode 100644 Documentation/iio/iio_configfs.txt

Re: [RFC 5/9] iio: Documentation: Add IIO configfs documentation

[Daniel Baluta]

On Wed, Nov 18, 2015 at 6:06 PM, Marc Titinger <mtitinger@baylibre.com> wrote:
> On 18/11/2015 16:38, Crt Mori wrote:
>>
>> This has my acked-by, yet it is missing a commit message? Can we get that
>> and the removed From: as I think this is a new patch and not resend of
>> Daniel's?
>> Also it would need your signed-off-by in that case
>
>
> You've lost me: I think the commit message is the brief line only.
> The patch is as-is from Daniel's series.

Hi Marc, Crt,

Configfs patches should soon be merged into linux-next tree, so there is
no need for them to be resent here.

Not sure how/when/where they'll be pulled back to linux-iio tree.

Daniel.

Re: [RFC 5/9] iio: Documentation: Add IIO configfs documentation

[Jonathan Cameron]

On 18 November 2015 16:15:57 GMT+00:00, Daniel Baluta <daniel.baluta@intel.com> wrote:
>On Wed, Nov 18, 2015 at 6:06 PM, Marc Titinger <mtitinger@baylibre.com>
>wrote:
>> On 18/11/2015 16:38, Crt Mori wrote:
>>>
>>> This has my acked-by, yet it is missing a commit message? Can we get
>that
>>> and the removed From: as I think this is a new patch and not resend
>of
>>> Daniel's?
>>> Also it would need your signed-off-by in that case
>>
>>
>> You've lost me: I think the commit message is the brief line only.
>> The patch is as-is from Daniel's series.
>
>Hi Marc, Crt,
>
>Configfs patches should soon be merged into linux-next tree, so there
>is
>no need for them to be resent here.
>
>Not sure how/when/where they'll be pulled back to linux-iio tree.
>
>Daniel.
It got a little complex as Christoph wanted the configfs change to hit
 mainline asap.

Andrew Morton will be sending Linus the change to configfs sometime in the next
 week. The other patches might show in Linux next after that via Andrew's tree.
Once I fast forward to a version of Greg's
 tree which has picked up the configfs change I will apply the patches to the IIO
 tree and send Greg a pull request sharpish to get them into Linux next.
 Andrew will then drop them from his tree and all will be lined up for the next merge
 window.

Simple :)

At a guess we are talking 2 to 3 weeks, maybe more, unfortunately but we'll
 before the next merge window.

Jonathan
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [RFC 8/9] iio: buffer: allow for last-second trigger spawning from device driver

[Jonathan Cameron]

On 18/11/15 14:38, Marc Titinger wrote:
> The hrtimer sw-trigger allow for polling mode on devices w/o hard irq
> trigger source, but setting the frequency from userland for both the
> hrtimer trigger device and the adc is error prone.
> 
> Make adc drivers able to setup the sw-trigger at the last second when the
> buffer is enabled, and the sampling frequency is known.
Hi Marc,

This statement rang alarm bells.  We are trying to cross synchronize a timer
on the processor with that on the device.  Doing this is ALWAYS going to end
up dropping or duplicating samples depending on whether we happen to run faster
or slower than the sample rate.

Now I've done a spot of datasheet diving to see if there is a status register or
other indication that we are looking at new data (if there isn't one, then any
attempt to get a stream of data off the device is going to give us a mess unfortunately)

Anyhow, on the INA219 we have a CNVR bit in the bus voltage register which will do as it
it's semantics are described in the datasheet and it's basically a 'you haven't read
me before bit'

The INA226 seems to have a CVRF bit (nothing like consistency in naming ;) that indicates
the 'dataready / alert pin' was set high to indicate new data - so you may need to enable
the interrupt pin even if you don't have it wired to anything useful.

Anyhow, so we have discussed how to handle this in the past (though right now I can't
remember the device in question so will be fun to find).  The case it came up for was
a board where the interrupt pin on a device wasn't wired to anything (or perhaps a stripped
down package in which the sensor didn't have a pin for it - I can't recall).

 First thing to note is that you 'don't' have to use a trigger to drive a buffer.
This makes our life rather easier!  Here we don't have a timing signal that is terribly
useful for any other sensors to use so a trigger won't be much real use.

Once we have dropped that requirement you do end up with something close to the
strategy you adopted in the first patch with a small twist.

You need to check those 'dataready' register bits to decide whether to push anything
at all to the buffer.

So basically you need your thread to always query significantly faster than the sampling
rate and to push data directly into the buffer iff the device indicates it is new data.
(not the significantly is to ensure that the you get one clean full set of readings within the sample period - I'm not sure the docs were all that specific on what happens if you hit
the sampling point in your read - maybe I missed it!)

The hrtimer trigger etc make a lot of sense for sample of demand devices, but
they will result in inconsistent data if used to sample a self clocking device like
this one.

I recall we discussed once how to do this generically and concluded that it really
isn't easy to do so as it involves polling a local register on a given device.

Sorry I didn't pick up on this earlier.

Jonathan

> 
> enable_trigger is called from verify_update, before the classical setup_ops
> are called in buffers_enable. This gives a chance to complete the setup of
> indio_dev->trig.
> 
> Signed-off-by: Marc Titinger <mtitinger@baylibre.com>
> ---
>  drivers/iio/industrialio-buffer.c | 5 +++++
>  include/linux/iio/iio.h           | 3 +++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index d7e908a..ba7abd4 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -647,6 +647,11 @@ static int iio_verify_update(struct iio_dev *indio_dev,
>  	if (insert_buffer)
>  		modes &= insert_buffer->access->modes;
>  
> +	if (indio_dev->setup_ops &&
> +	    indio_dev->setup_ops->enable_trigger &&
> +	   (indio_dev->setup_ops->enable_trigger(indio_dev) < 0))
> +		return -ENXIO;
> +
>  	/* Definitely possible for devices to support both of these. */
>  	if ((modes & INDIO_BUFFER_TRIGGERED) && indio_dev->trig) {
>  		config->mode = INDIO_BUFFER_TRIGGERED;
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index 7bb7f67..8f82113 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -419,6 +419,8 @@ struct iio_info {
>  
>  /**
>   * struct iio_buffer_setup_ops - buffer setup related callbacks
> + * @enable_trigger:	[DRIVER] function to call if a trigger is instancied
> + *				 upon enabling the buffer (sw triggers)
>   * @preenable:		[DRIVER] function to run prior to marking buffer enabled
>   * @postenable:		[DRIVER] function to run after marking buffer enabled
>   * @predisable:		[DRIVER] function to run prior to marking buffer
> @@ -428,6 +430,7 @@ struct iio_info {
>   *			scan mask is valid for the device.
>   */
>  struct iio_buffer_setup_ops {
> +	int (*enable_trigger)(struct iio_dev *);
>  	int (*preenable)(struct iio_dev *);
>  	int (*postenable)(struct iio_dev *);
>  	int (*predisable)(struct iio_dev *);
> 

Re: [RFC 8/9] iio: buffer: allow for last-second trigger spawning from device driver

[Marc Titinger]

On 18/11/2015 19:55, Jonathan Cameron wrote:
> On 18/11/15 14:38, Marc Titinger wrote:
>> The hrtimer sw-trigger allow for polling mode on devices w/o hard irq
>> trigger source, but setting the frequency from userland for both the
>> hrtimer trigger device and the adc is error prone.
>>
>> Make adc drivers able to setup the sw-trigger at the last second when the
>> buffer is enabled, and the sampling frequency is known.
> Hi Marc,
>
> This statement rang alarm bells.  We are trying to cross synchronize a timer
> on the processor with that on the device.  Doing this is ALWAYS going to end
> up dropping or duplicating samples depending on whether we happen to run faster
> or slower than the sample rate.
>
Yup, I had a hunch this was an issue, I understand this is not something 
you guys want to generally support in IIO since it should allow for 
reliable signal processing!

> Now I've done a spot of datasheet diving to see if there is a status register or
> other indication that we are looking at new data (if there isn't one, then any
> attempt to get a stream of data off the device is going to give us a mess unfortunately)
>
> Anyhow, on the INA219 we have a CNVR bit in the bus voltage register which will do as it
> it's semantics are described in the datasheet and it's basically a 'you haven't read
> me before bit'
>
> The INA226 seems to have a CVRF bit (nothing like consistency in naming ;) that indicates
> the 'dataready / alert pin' was set high to indicate new data - so you may need to enable
> the interrupt pin even if you don't have it wired to anything useful.
>
> Anyhow, so we have discussed how to handle this in the past (though right now I can't
> remember the device in question so will be fun to find).  The case it came up for was
> a board where the interrupt pin on a device wasn't wired to anything (or perhaps a stripped
> down package in which the sensor didn't have a pin for it - I can't recall).
>
>   First thing to note is that you 'don't' have to use a trigger to drive a buffer.
> This makes our life rather easier!  Here we don't have a timing signal that is terribly
> useful for any other sensors to use so a trigger won't be much real use.
>
> Once we have dropped that requirement you do end up with something close to the
> strategy you adopted in the first patch with a small twist.
>
> You need to check those 'dataready' register bits to decide whether to push anything
> at all to the buffer.

Ok yes, I can check that dataready bit, and only push new values with a 
usable timestamp. So I shall go back to my polling thread version with 
that addition. I'm just concerned that It is one more register to read 
while part of this work was to increase the bandwidth of our 
apllication. Our hwmon sigrok layer would reissue the last value anyhow 
if the hardware is not ready for a new sample, so a bit of clock jitter 
was ok for my purpose.

Alternatively, I could check if the cnvr signal can be configured as an 
alarm and routed to a gpio irq, and then use a conventional trigger.

>
> So basically you need your thread to always query significantly faster than the sampling
> rate and to push data directly into the buffer iff the device indicates it is new data.
> (not the significantly is to ensure that the you get one clean full set of readings within the sample period - I'm not sure the docs were all that specific on what happens if you hit
> the sampling point in your read - maybe I missed it!)
>
> The hrtimer trigger etc make a lot of sense for sample of demand devices, but
> they will result in inconsistent data if used to sample a self clocking device like
> this one.
>
> I recall we discussed once how to do this generically and concluded that it really
> isn't easy to do so as it involves polling a local register on a given device.
>
> Sorry I didn't pick up on this earlier.

No worries, I'm happy to experiment and gain understanding of the 
features of IIO, I'm sorry it cost you guys review time though, _and_ it 
is still getting onto something useful :)

Marc.


>
> Jonathan
>
>>
>> enable_trigger is called from verify_update, before the classical setup_ops
>> are called in buffers_enable. This gives a chance to complete the setup of
>> indio_dev->trig.
>>
>> Signed-off-by: Marc Titinger <mtitinger@baylibre.com>
>> ---
>>   drivers/iio/industrialio-buffer.c | 5 +++++
>>   include/linux/iio/iio.h           | 3 +++
>>   2 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
>> index d7e908a..ba7abd4 100644
>> --- a/drivers/iio/industrialio-buffer.c
>> +++ b/drivers/iio/industrialio-buffer.c
>> @@ -647,6 +647,11 @@ static int iio_verify_update(struct iio_dev *indio_dev,
>>   	if (insert_buffer)
>>   		modes &= insert_buffer->access->modes;
>>
>> +	if (indio_dev->setup_ops &&
>> +	    indio_dev->setup_ops->enable_trigger &&
>> +	   (indio_dev->setup_ops->enable_trigger(indio_dev) < 0))
>> +		return -ENXIO;
>> +
>>   	/* Definitely possible for devices to support both of these. */
>>   	if ((modes & INDIO_BUFFER_TRIGGERED) && indio_dev->trig) {
>>   		config->mode = INDIO_BUFFER_TRIGGERED;
>> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
>> index 7bb7f67..8f82113 100644
>> --- a/include/linux/iio/iio.h
>> +++ b/include/linux/iio/iio.h
>> @@ -419,6 +419,8 @@ struct iio_info {
>>
>>   /**
>>    * struct iio_buffer_setup_ops - buffer setup related callbacks
>> + * @enable_trigger:	[DRIVER] function to call if a trigger is instancied
>> + *				 upon enabling the buffer (sw triggers)
>>    * @preenable:		[DRIVER] function to run prior to marking buffer enabled
>>    * @postenable:		[DRIVER] function to run after marking buffer enabled
>>    * @predisable:		[DRIVER] function to run prior to marking buffer
>> @@ -428,6 +430,7 @@ struct iio_info {
>>    *			scan mask is valid for the device.
>>    */
>>   struct iio_buffer_setup_ops {
>> +	int (*enable_trigger)(struct iio_dev *);
>>   	int (*preenable)(struct iio_dev *);
>>   	int (*postenable)(struct iio_dev *);
>>   	int (*predisable)(struct iio_dev *);
>>
>

Re: [RFC 6/9] iio: ina2xx: add direct IO support for TI INA2xx Power Monitors

[Jonathan Cameron]

On 18/11/15 14:38, Marc Titinger wrote:
> Basic support or direct IO raw read, with averaging attribute.
> Values are RAW, INT_PLUS_MICRO (Volt/Ampere/Watt).
> 
> Output of iio_info:
> 
>      iio:device0: ina226
>       4 channels found:
>         power3:  (input)
>         1 channel-specific attributes found:
>                 attr 0: raw value: 1.150000
>         voltage0:  (input)
>         1 channel-specific attributes found:
>                 attr 0: raw value: 0.000003
>         voltage1:  (input)
>         1 channel-specific attributes found:
>                 attr 0: raw value: 4.277500
>         current2:  (input)
>         1 channel-specific attributes found:
>                 attr 0: raw value: 0.268000
>         4 device-specific attributes found:
>                 attr 0: sampling_frequency_available value: 61 120 236...
>                 attr 1: in_averaging_steps value: 4
>                 attr 2: in_calibscale value: 10000
>                 attr 3: in_sampling_frequency value: 1506
> 
> Tested with ina226, on BeagleBoneBlack.
> 
> Datasheet: http://www.ti.com/lit/gpn/ina226
> 
> Signed-off-by: Marc Titinger <mtitinger@baylibre.com>
You have added some new ABI in here, but I'm not seeing any documentation
for averaging_steps.  Does this map onto the existing oversampling_ratio?

Also we need to start bringing the hwmon guys in on these patches.
I don't want to tread on any toes so we need to play carefully!
Please cc their maintainers and list on the next version and make sure
you include a cover letter explaining exactly why you want a second
driver for this same part.

A few other bits inline..

Thanks,

Jonathan
> ---
>  drivers/iio/adc/Kconfig      |   9 +
>  drivers/iio/adc/Makefile     |   1 +
>  drivers/iio/adc/ina2xx-iio.c | 515 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 525 insertions(+)
>  create mode 100644 drivers/iio/adc/ina2xx-iio.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 50c103d..ebbfff9 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -183,6 +183,15 @@ config EXYNOS_ADC
>  	  To compile this driver as a module, choose M here: the module will be
>  	  called exynos_adc.
>  
> +config INA2XX_IIO
> +	tristate "Texas Instruments INA2xx Power Monitors IIO driver"
> +	depends on I2C
> +	select REGMAP_I2C
> +	help
> +	  Say yes here to build support for TI INA2xx familly Power Monitors.
> +
> +	  Note that this is not the existing hwmon interface for this device.
> +
>  config LP8788_ADC
>  	tristate "LP8788 ADC driver"
>  	depends on MFD_LP8788
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index a096210..74e4341 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_BERLIN2_ADC) += berlin2-adc.o
>  obj-$(CONFIG_DA9150_GPADC) += da9150-gpadc.o
>  obj-$(CONFIG_CC10001_ADC) += cc10001_adc.o
>  obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o
> +obj-$(CONFIG_INA2XX_IIO) += ina2xx-iio.o
>  obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
>  obj-$(CONFIG_MAX1027) += max1027.o
>  obj-$(CONFIG_MAX1363) += max1363.o
> diff --git a/drivers/iio/adc/ina2xx-iio.c b/drivers/iio/adc/ina2xx-iio.c
> new file mode 100644
> index 0000000..d4dd908
> --- /dev/null
> +++ b/drivers/iio/adc/ina2xx-iio.c
> @@ -0,0 +1,515 @@
> +/*
> + * INA2XX Current and Power Monitors
> + *
> + * Copyright 2015 Baylibre SAS.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Based on linux/drivers/iio/adc/ad7291.c
> + * Copyright 2010-2011 Analog Devices Inc.
> + *
> + * Based on linux/drivers/hwmon/ina2xx.c
> + * Copyright 2012 Lothar Felten <l-felten@ti.com>
> + *
> + * Licensed under the GPL-2 or later.
> + *
> + * IIO driver for INA219-220-226-230-231
> + *
> + * Configurable 7-bit I2C slave address from 0x40 to 0x4F
> + */
> +#include <linux/module.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/i2c.h>
> +#include <linux/regmap.h>
> +#include <linux/platform_data/ina2xx.h>
> +
> +#include <linux/util_macros.h>
> +
> +/*
> + * INA2XX registers definition
> + */
> +/* common register definitions */
> +#define INA2XX_CONFIG                   0x00
> +#define INA2XX_SHUNT_VOLTAGE            0x01	/* readonly */
> +#define INA2XX_BUS_VOLTAGE              0x02	/* readonly */
> +#define INA2XX_POWER                    0x03	/* readonly */
> +#define INA2XX_CURRENT                  0x04	/* readonly */
> +#define INA2XX_CALIBRATION              0x05
> +
> +/* register count */
> +#define INA219_REGISTERS                6
> +#define INA226_REGISTERS                8
> +#define INA2XX_MAX_REGISTERS            8
> +
> +/* settings - depend on use case */
> +#define INA219_CONFIG_DEFAULT           0x399F	/* PGA=8 */
> +#define INA226_CONFIG_DEFAULT           0x4327
> +#define INA226_DEFAULT_AVG              4
> +#define INA226_DEFAULT_FREQ             455
> +
> +#define INA2XX_RSHUNT_DEFAULT           10000
> +
> +/* bit mask for reading the averaging setting in the configuration register */
> +#define INA226_AVG_RD_MASK              GENMASK(11, 9)
> +#define INA226_READ_AVG(reg)            (((reg) & INA226_AVG_RD_MASK) >> 9)
> +#define INA226_SHIFT_AVG(val)           ((val) << 9)
> +
> +#define INA226_SFREQ_RD_MASK            GENMASK(8, 3)
> +
> +
> +static bool ina2xx_is_writeable_reg(struct device *dev, unsigned int reg)
> +{
> +	return (reg == INA2XX_CONFIG) || (reg == INA2XX_CALIBRATION);
> +}
> +
> +static bool ina2xx_is_volatile_reg(struct device *dev, unsigned int reg)
> +{
> +	return (reg != INA2XX_CONFIG);
> +}
> +
> +static struct regmap_config ina2xx_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 16,
> +
> +	.writeable_reg = ina2xx_is_writeable_reg,
> +	.volatile_reg = ina2xx_is_volatile_reg,
> +};
> +
> +enum ina2xx_ids { ina219, ina226 };
> +
> +struct ina2xx_config {
> +	u16 config_default;
> +	int calibration_factor;
> +	int registers;
> +	int shunt_div;
> +	int bus_voltage_shift;
> +	int bus_voltage_lsb;	/* uV */
> +	int power_lsb;		/* uW */
> +};
> +
> +struct ina2xx_chip_info {
> +	const struct ina2xx_config *config;
> +	struct mutex state_lock;
> +	long rshunt;
> +	int avg;
> +	int freq;
> +	int period_us;
> +	struct regmap *regmap;
> +};
> +
> +static const struct ina2xx_config ina2xx_config[] = {
> +	[ina219] = {
> +		    .config_default = INA219_CONFIG_DEFAULT,
> +		    .calibration_factor = 40960000,
> +		    .registers = INA219_REGISTERS,
> +		    .shunt_div = 100,
> +		    .bus_voltage_shift = 3,
> +		    .bus_voltage_lsb = 4000,
> +		    .power_lsb = 20000,
> +		    },
> +	[ina226] = {
> +		    .config_default = INA226_CONFIG_DEFAULT,
> +		    .calibration_factor = 5120000,
> +		    .registers = INA226_REGISTERS,
> +		    .shunt_div = 400,
> +		    .bus_voltage_shift = 0,
> +		    .bus_voltage_lsb = 1250,
> +		    .power_lsb = 25000,
> +		    },
> +};
> +
> +static int ina2xx_get_value(struct ina2xx_chip_info *chip, u8 reg,
> +			    unsigned int regval, int *val, int *uval)
> +{
> +	*val = 0;
> +
> +	switch (reg) {
> +	case INA2XX_SHUNT_VOLTAGE:
> +		/* signed register */
> +		*uval = DIV_ROUND_CLOSEST((s16) regval,
> +					  chip->config->shunt_div);
> +		return IIO_VAL_INT_PLUS_MICRO;
> +
> +	case INA2XX_BUS_VOLTAGE:
> +		*uval = (regval >> chip->config->bus_voltage_shift)
> +			* chip->config->bus_voltage_lsb;
> +		*val = *uval/1000000;
> +		*uval = *uval % 1000000;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +
> +	case INA2XX_POWER:
> +		*uval = regval * chip->config->power_lsb;
> +		*val = *uval/1000000;
> +		*uval = *uval % 1000000;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +
> +	case INA2XX_CURRENT:
> +		/* signed register, LSB=1mA (selected), in mA */
> +		*uval = (s16) regval * 1000;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +
Having the calibration in here doesn't make a lot of sense.  Just read the
value directly where needed. It'll be easier to follow.
> +	case INA2XX_CALIBRATION:
> +		*val = DIV_ROUND_CLOSEST(chip->config->calibration_factor,
> +					regval);
> +		return IIO_VAL_INT;
> +
> +	default:
> +		/* programmer goofed */
> +		WARN_ON_ONCE(1);
> +	}
> +	return -EINVAL;
> +}
> +
> +static int ina2xx_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long mask)
> +{
> +	int ret;
> +	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
> +	unsigned int regval;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = regmap_read(chip->regmap, chan->address, &regval);
> +		if (ret < 0)
> +			return ret;
> +
> +		return ina2xx_get_value(chip, chan->address, regval, val, val2);
> +
> +	case IIO_CHAN_INFO_AVERAGE_RAW:
as in the write oversampling_ratio seems more appropriate to me.
Please do read the ABI docs. Documentation/ABI/testing/sysfs-bus-iio*

> +		*val = chip->avg;
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_CALIBSCALE:
> +		ret = regmap_read(chip->regmap, INA2XX_CALIBRATION, &regval);
> +		if (ret < 0)
> +			return ret;
> +
> +		return ina2xx_get_value(chip, INA2XX_CALIBRATION, regval,
> +					val, val2);
> +
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*val = chip->freq;
> +		return IIO_VAL_INT;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ina2xx_calibrate(struct ina2xx_chip_info *chip)
> +{
> +	u16 val = DIV_ROUND_CLOSEST(chip->config->calibration_factor,
> +				    chip->rshunt);
> +
> +	return regmap_write(chip->regmap, INA2XX_CALIBRATION, val);
> +}
> +
> +
> +/*
> + * Available averaging rates for ina226. The indices correspond with
> + * the bit values expected by the chip (according to the ina226 datasheet,
> + * table 3 AVG bit settings, found at
> + * http://www.ti.com/lit/ds/symlink/ina226.pdf.
> + */
> +static const int ina226_avg_tab[] = { 1, 4, 16, 64, 128, 256, 512, 1024 };
> +
> +static unsigned int ina226_set_average(struct ina2xx_chip_info *chip,
> +				       unsigned int val,
> +				       unsigned int *config)
> +{
> +	int bits;
> +
> +	if (val > 1024 || val < 1)
> +		return -EINVAL;
> +
> +	bits = find_closest(val, ina226_avg_tab,
> +			    ARRAY_SIZE(ina226_avg_tab));
> +
> +	chip->avg = ina226_avg_tab[bits];
> +
> +	*config &= ~INA226_AVG_RD_MASK;
> +	*config |= INA226_SHIFT_AVG(bits) & INA226_AVG_RD_MASK;
> +
> +	return 0;
> +}
> +
> +/* Conversion times in uS */
> +static const int ina226_conv_time_tab[] = { 140, 204, 332, 588, 1100,
> +	2116, 4156, 8244};
> +
> +static unsigned int ina226_set_frequency(struct ina2xx_chip_info *chip,
> +					 unsigned int val,
> +					 unsigned int *config)
> +{
> +	int bits;
> +
> +	if (val > 3550  || val < 50)
> +		return -EINVAL;
> +
> +	/* integration time in uS, for both voltage channels */
> +	val = DIV_ROUND_CLOSEST(1000000, 2 * val);
> +
> +	bits = find_closest(val, ina226_conv_time_tab,
> +			    ARRAY_SIZE(ina226_conv_time_tab));
> +
> +	chip->period_us = 2 * ina226_conv_time_tab[bits];
> +
> +	chip->freq = DIV_ROUND_CLOSEST(1000000, chip->period_us);
> +
> +	*config &= ~INA226_SFREQ_RD_MASK;
> +	*config |= (bits << 3) | (bits << 6);
So right now you are restricting the conversion times for the two channels
to be linked?  I'd be tempted to separate them out for greater flexibility.
> +
> +	return 0;
> +}
> +
> +
> +static int ina2xx_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int val, int val2, long mask)
> +{
> +	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
> +	int ret = 0;
> +	unsigned int config, tmp;
> +
> +	mutex_lock(&chip->state_lock);
> +
> +	ret = regmap_read(chip->regmap, INA2XX_CONFIG, &config);
> +	if (ret < 0)
> +		goto _err;
> +
> +	tmp = config;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_AVERAGE_RAW:
> +
> +		ret = ina226_set_average(chip, val, &tmp);
This isn't normally considered a write parameter. It's usually
for output of a calculated average.  I think you want oversampling
ratio for this...
> +		break;
> +
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +
> +		ret = ina226_set_frequency(chip, val, &tmp);
> +		break;
> +
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +	if (!ret && (tmp != config))
> +		ret = regmap_write(chip->regmap, INA2XX_CONFIG, config);
> +_err:
> +	mutex_unlock(&chip->state_lock);
> +
> +	return ret;
> +}
> +
> +
> +static ssize_t ina2xx_averaging_steps_show(struct device *dev,
> +					   struct device_attribute *attr,
> +					   char *buf)
> +{
> +	struct ina2xx_chip_info *chip = iio_priv(dev_to_iio_dev(dev));
> +
> +	return sprintf(buf, "%d\n", chip->avg);
> +}
> +
> +
> +static ssize_t ina2xx_averaging_steps_store(struct device *dev,
> +				struct device_attribute *attr,
> +				const char *buf, size_t len)
> +{
> +	unsigned long val;
> +	int ret;
> +
> +	ret = kstrtoul((const char *) buf, 10, &val);
> +	if (ret)
> +		return -EINVAL;
> +
> +	/* unexposed missuse of INFO_AVERAGE_RAW, until a proper ABI for the
> +	 * averaging steps setting is specified.
Incorrect multiline comment syntax...
> +	 */
> +	ret  = ina2xx_write_raw(dev_to_iio_dev(dev), NULL, val, 0,
> +				IIO_CHAN_INFO_AVERAGE_RAW);
> +	if (ret < 0)
> +		return ret;
> +
> +	return len;
> +}
> +
> +
> +#define INA2XX_CHAN(_type, _index, _address) { \
> +	.type = _type, \
> +	.address = _address, \
> +	.indexed = 1, \
> +	.channel = (_index), \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +	.info_mask_shared_by_dir = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
> +				   BIT(IIO_CHAN_INFO_CALIBSCALE), \
> +	.scan_index = (_index), \
> +	.scan_type = { \
> +		.sign = 'u', \
> +		.realbits = 16, \
> +		.storagebits = 16, \
> +		.shift = 0, \
No need to specify shift if it is 0
> +	.endianness = IIO_BE, \
> +	} \
> +}
> +
> +static const struct iio_chan_spec ina2xx_channels[] = {
> +	INA2XX_CHAN(IIO_VOLTAGE, 0, INA2XX_SHUNT_VOLTAGE),
> +	INA2XX_CHAN(IIO_VOLTAGE, 1, INA2XX_BUS_VOLTAGE),
> +	INA2XX_CHAN(IIO_CURRENT, 2, INA2XX_CURRENT),
> +	INA2XX_CHAN(IIO_POWER, 3, INA2XX_POWER),
> +	IIO_CHAN_SOFT_TIMESTAMP(4),
> +};
> +
> +static int ina2xx_debug_reg(struct iio_dev *indio_dev,
> +			    unsigned reg, unsigned writeval, unsigned *readval)
> +{
> +	struct ina2xx_chip_info *chip = iio_priv(indio_dev);
> +
> +	if (!readval)
> +		return regmap_write(chip->regmap, reg, writeval);
> +
> +	return regmap_read(chip->regmap, reg, readval);
> +}
> +
> +/* frequencies matching the cummulated integration times for vshunt and vbus */
> +static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("61 120 236 455 850 1506 2450 3571");
> +
> +static IIO_DEVICE_ATTR(in_averaging_steps, S_IRUGO | S_IWUSR,
> +		       ina2xx_averaging_steps_show,
> +		       ina2xx_averaging_steps_store, 0);
> +
> +static struct attribute *ina2xx_attributes[] = {
> +	&iio_dev_attr_in_averaging_steps.dev_attr.attr,
> +	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group ina2xx_attribute_group = {
> +	.attrs = ina2xx_attributes,
> +};
> +
> +static const struct iio_info ina2xx_info = {
> +	.debugfs_reg_access = &ina2xx_debug_reg,
> +	.read_raw = &ina2xx_read_raw,
> +	.write_raw = &ina2xx_write_raw,
> +	.attrs = &ina2xx_attribute_group,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +/* Initialize the configuration and calibration registers. */
> +static int ina2xx_init(struct ina2xx_chip_info *chip, unsigned int config)
> +{
> +	int ret = regmap_write(chip->regmap, INA2XX_CONFIG, config);
> +
> +	if (ret < 0)
> +		return ret;
> +	/*
> +	 * Set current LSB to 1mA, shunt is in uOhms
> +	 * (equation 13 in datasheet).
> +	 */
> +	return ina2xx_calibrate(chip);
> +}
> +
> +static int ina2xx_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	struct ina2xx_chip_info *chip;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +	unsigned int val;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	chip = iio_priv(indio_dev);
> +
> +	/* set the device type */
> +	chip->config = &ina2xx_config[id->driver_data];
> +
> +	if (of_property_read_u32(client->dev.of_node,
> +				 "shunt-resistor", &val) < 0) {
> +		struct ina2xx_platform_data *pdata =
> +		    dev_get_platdata(&client->dev);
> +
> +		if (pdata)
> +			val = pdata->shunt_uohms;
> +		else
> +			val = INA2XX_RSHUNT_DEFAULT;
> +	}
> +
> +	if (val <= 0 || val > chip->config->calibration_factor)
> +		return -ENODEV;
> +
> +	chip->rshunt = val;
> +
> +	ina2xx_regmap_config.max_register = chip->config->registers;
> +
> +	mutex_init(&chip->state_lock);
> +
> +	/* this is only used for device removal purposes */
> +	i2c_set_clientdata(client, indio_dev);
> +
> +	indio_dev->name = id->name;
> +	indio_dev->channels = ina2xx_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(ina2xx_channels);
> +
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->info = &ina2xx_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	chip->regmap = devm_regmap_init_i2c(client, &ina2xx_regmap_config);
> +	if (IS_ERR(chip->regmap)) {
> +		dev_err(&client->dev, "failed to allocate register map\n");
> +		return PTR_ERR(chip->regmap);
> +	}
> +
> +	/* Patch the current config register with default. */
> +	val = chip->config->config_default;
> +	if (id->driver_data == ina226) {
> +		ina226_set_average(chip, INA226_DEFAULT_AVG, &val);
> +		ina226_set_frequency(chip, INA226_DEFAULT_FREQ, &val);
> +	}
> +
> +	ret = ina2xx_init(chip, val);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "error configuring the device: %d\n",
> +			ret);
> +		return -ENODEV;
> +	}
> +
> +	return devm_iio_device_register(&client->dev, indio_dev);

It's a small point, but I'd suggest you should really have a remove. The
INA226 at least has a power-down or shutdown mode.  I'd advocate a remove
that at least sets that.  Might as well save a bit of power for a few
lines of additional code!

Can be added at a later stage however.
> +}
> +
> +static const struct i2c_device_id ina2xx_id[] = {
> +	{"ina219", ina219},
> +	{"ina220", ina219},
> +	{"ina226", ina226},
> +	{"ina230", ina226},
> +	{"ina231", ina226},
> +	{}
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, ina2xx_id);
> +
> +static struct i2c_driver ina2xx_driver = {
> +	.driver = {
> +		   .name = KBUILD_MODNAME,
> +		   },
> +	.probe = ina2xx_probe,
> +	.id_table = ina2xx_id,
> +};
> +
> +module_i2c_driver(ina2xx_driver);
> +
> +MODULE_AUTHOR("Marc Titinger <marc.titinger@baylibre.com>");
> +MODULE_DESCRIPTION("Texas Instruments INA2XX ADC driver");
> +MODULE_LICENSE("GPL v2");
> 

Re: [RFC 8/9] iio: buffer: allow for last-second trigger spawning from device driver

[Jonathan Cameron]

On 19/11/15 09:15, Marc Titinger wrote:
> On 18/11/2015 19:55, Jonathan Cameron wrote:
>> On 18/11/15 14:38, Marc Titinger wrote:
>>> The hrtimer sw-trigger allow for polling mode on devices w/o hard irq
>>> trigger source, but setting the frequency from userland for both the
>>> hrtimer trigger device and the adc is error prone.
>>>
>>> Make adc drivers able to setup the sw-trigger at the last second when the
>>> buffer is enabled, and the sampling frequency is known.
>> Hi Marc,
>>
>> This statement rang alarm bells.  We are trying to cross synchronize a timer
>> on the processor with that on the device.  Doing this is ALWAYS going to end
>> up dropping or duplicating samples depending on whether we happen to run faster
>> or slower than the sample rate.
>>
> Yup, I had a hunch this was an issue, I understand this is not something you guys want to generally support in IIO since it should allow for reliable signal processing!
> 
>> Now I've done a spot of datasheet diving to see if there is a status register or
>> other indication that we are looking at new data (if there isn't one, then any
>> attempt to get a stream of data off the device is going to give us a mess unfortunately)
>>
>> Anyhow, on the INA219 we have a CNVR bit in the bus voltage register which will do as it
>> it's semantics are described in the datasheet and it's basically a 'you haven't read
>> me before bit'
>>
>> The INA226 seems to have a CVRF bit (nothing like consistency in naming ;) that indicates
>> the 'dataready / alert pin' was set high to indicate new data - so you may need to enable
>> the interrupt pin even if you don't have it wired to anything useful.
>>
>> Anyhow, so we have discussed how to handle this in the past (though right now I can't
>> remember the device in question so will be fun to find).  The case it came up for was
>> a board where the interrupt pin on a device wasn't wired to anything (or perhaps a stripped
>> down package in which the sensor didn't have a pin for it - I can't recall).
>>
>>   First thing to note is that you 'don't' have to use a trigger to drive a buffer.
>> This makes our life rather easier!  Here we don't have a timing signal that is terribly
>> useful for any other sensors to use so a trigger won't be much real use.
>>
>> Once we have dropped that requirement you do end up with something close to the
>> strategy you adopted in the first patch with a small twist.
>>
>> You need to check those 'dataready' register bits to decide whether to push anything
>> at all to the buffer.
> 
> Ok yes, I can check that dataready bit, and only push new values with
> a usable timestamp.
Hmm. Timestamping accuracy is going to be terrible whatever you do, but I
guess if that is well documented and you need it in your application we'll
need to go with it as whatever is possible.

> So I shall go back to my polling thread version
> with that addition. I'm just concerned that It is one more register
> to read while part of this work was to increase the bandwidth of our
> apllication.
Give it a try and see if you can get away with it.. In some parts cases you
don't need to even read an extra register and if you pick a sensible poll frequency
might get data you want say 3 out of 4 times.

 
> Our hwmon sigrok layer would reissue the last value
> anyhow if the hardware is not ready for a new sample, so a bit of
> clock jitter was ok for my purpose.

> Alternatively, I could check if the cnvr signal can be configured as
> an alarm and routed to a gpio irq, and then use a conventional
> trigger.
That would be preferable, though I'm not sure all the parts
actually have a hardware irq line for it so you may need the register
read value as well.

> 
>>
>> So basically you need your thread to always query significantly faster than the sampling
>> rate and to push data directly into the buffer iff the device indicates it is new data.
>> (not the significantly is to ensure that the you get one clean full set of readings within the sample period - I'm not sure the docs were all that specific on what happens if you hit
>> the sampling point in your read - maybe I missed it!)
>>
>> The hrtimer trigger etc make a lot of sense for sample of demand devices, but
>> they will result in inconsistent data if used to sample a self clocking device like
>> this one.
>>
>> I recall we discussed once how to do this generically and concluded that it really
>> isn't easy to do so as it involves polling a local register on a given device.
>>
>> Sorry I didn't pick up on this earlier.
> 
> No worries, I'm happy to experiment and gain understanding of the features of IIO, I'm sorry it cost you guys review time though, _and_ it is still getting onto something useful :)
> 
> Marc.
> 
> 
>>
>> Jonathan
>>
>>>
>>> enable_trigger is called from verify_update, before the classical setup_ops
>>> are called in buffers_enable. This gives a chance to complete the setup of
>>> indio_dev->trig.
>>>
>>> Signed-off-by: Marc Titinger <mtitinger@baylibre.com>
>>> ---
>>>   drivers/iio/industrialio-buffer.c | 5 +++++
>>>   include/linux/iio/iio.h           | 3 +++
>>>   2 files changed, 8 insertions(+)
>>>
>>> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
>>> index d7e908a..ba7abd4 100644
>>> --- a/drivers/iio/industrialio-buffer.c
>>> +++ b/drivers/iio/industrialio-buffer.c
>>> @@ -647,6 +647,11 @@ static int iio_verify_update(struct iio_dev *indio_dev,
>>>       if (insert_buffer)
>>>           modes &= insert_buffer->access->modes;
>>>
>>> +    if (indio_dev->setup_ops &&
>>> +        indio_dev->setup_ops->enable_trigger &&
>>> +       (indio_dev->setup_ops->enable_trigger(indio_dev) < 0))
>>> +        return -ENXIO;
>>> +
>>>       /* Definitely possible for devices to support both of these. */
>>>       if ((modes & INDIO_BUFFER_TRIGGERED) && indio_dev->trig) {
>>>           config->mode = INDIO_BUFFER_TRIGGERED;
>>> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
>>> index 7bb7f67..8f82113 100644
>>> --- a/include/linux/iio/iio.h
>>> +++ b/include/linux/iio/iio.h
>>> @@ -419,6 +419,8 @@ struct iio_info {
>>>
>>>   /**
>>>    * struct iio_buffer_setup_ops - buffer setup related callbacks
>>> + * @enable_trigger:    [DRIVER] function to call if a trigger is instancied
>>> + *                 upon enabling the buffer (sw triggers)
>>>    * @preenable:        [DRIVER] function to run prior to marking buffer enabled
>>>    * @postenable:        [DRIVER] function to run after marking buffer enabled
>>>    * @predisable:        [DRIVER] function to run prior to marking buffer
>>> @@ -428,6 +430,7 @@ struct iio_info {
>>>    *            scan mask is valid for the device.
>>>    */
>>>   struct iio_buffer_setup_ops {
>>> +    int (*enable_trigger)(struct iio_dev *);
>>>       int (*preenable)(struct iio_dev *);
>>>       int (*postenable)(struct iio_dev *);
>>>       int (*predisable)(struct iio_dev *);
>>>
>>
> 
> -- 
> 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

Re: [RFC 6/9] iio: ina2xx: add direct IO support for TI INA2xx Power Monitors

[Marc Titinger]

On 21/11/2015 19:13, Jonathan Cameron wrote:
> On 18/11/15 14:38, Marc Titinger wrote:
>> Basic support or direct IO raw read, with averaging attribute.
>> Values are RAW, INT_PLUS_MICRO (Volt/Ampere/Watt).
>>
>> Output of iio_info:
>>
>>       iio:device0: ina226
>>        4 channels found:
>>          power3:  (input)
>>          1 channel-specific attributes found:
>>                  attr 0: raw value: 1.150000
>>          voltage0:  (input)
>>          1 channel-specific attributes found:
>>                  attr 0: raw value: 0.000003
>>          voltage1:  (input)
>>          1 channel-specific attributes found:
>>                  attr 0: raw value: 4.277500
>>          current2:  (input)
>>          1 channel-specific attributes found:
>>                  attr 0: raw value: 0.268000
>>          4 device-specific attributes found:
>>                  attr 0: sampling_frequency_available value: 61 120 236...
>>                  attr 1: in_averaging_steps value: 4
>>                  attr 2: in_calibscale value: 10000
>>                  attr 3: in_sampling_frequency value: 1506
>>
>> Tested with ina226, on BeagleBoneBlack.
>>
>> Datasheet: http://www.ti.com/lit/gpn/ina226
>>
>> Signed-off-by: Marc Titinger <mtitinger@baylibre.com>
> You have added some new ABI in here, but I'm not seeing any documentation
> for averaging_steps.  Does this map onto the existing oversampling_ratio?
>

I am not sure normal averaging maps well with oversampling. Normal 
averaging will provide one value every N samples (this is what this chip 
does), while oversampling will interpolate N value between sample 'k' 
and 'k-1', and decimate to provide a less-noisy version of sample 'k', 
the resulting sampling frequency is not lower.

Re: [RFC 6/9] iio: ina2xx: add direct IO support for TI INA2xx Power Monitors

[Jonathan Cameron]

On 23/11/15 16:15, Marc Titinger wrote:
> On 21/11/2015 19:13, Jonathan Cameron wrote:
>> On 18/11/15 14:38, Marc Titinger wrote:
>>> Basic support or direct IO raw read, with averaging attribute.
>>> Values are RAW, INT_PLUS_MICRO (Volt/Ampere/Watt).
>>>
>>> Output of iio_info:
>>>
>>>       iio:device0: ina226
>>>        4 channels found:
>>>          power3:  (input)
>>>          1 channel-specific attributes found:
>>>                  attr 0: raw value: 1.150000
>>>          voltage0:  (input)
>>>          1 channel-specific attributes found:
>>>                  attr 0: raw value: 0.000003
>>>          voltage1:  (input)
>>>          1 channel-specific attributes found:
>>>                  attr 0: raw value: 4.277500
>>>          current2:  (input)
>>>          1 channel-specific attributes found:
>>>                  attr 0: raw value: 0.268000
>>>          4 device-specific attributes found:
>>>                  attr 0: sampling_frequency_available value: 61 120 236...
>>>                  attr 1: in_averaging_steps value: 4
>>>                  attr 2: in_calibscale value: 10000
>>>                  attr 3: in_sampling_frequency value: 1506
>>>
>>> Tested with ina226, on BeagleBoneBlack.
>>>
>>> Datasheet: http://www.ti.com/lit/gpn/ina226
>>>
>>> Signed-off-by: Marc Titinger <mtitinger@baylibre.com>
>> You have added some new ABI in here, but I'm not seeing any documentation
>> for averaging_steps.  Does this map onto the existing oversampling_ratio?
>>
> 
> I am not sure normal averaging maps well with oversampling. Normal
> averaging will provide one value every N samples (this is what this
> chip does), while oversampling will interpolate N value between
> sample 'k' and 'k-1', and decimate to provide a less-noisy version of
> sample 'k', the resulting sampling frequency is not lower.
> 
Oversampling is wonderfully badly defined as a term.  Often it is use precisely for the
method or noise reduction that is infact just an average.   Usually it implies:
1) Sampling at a higher rate than the eventual output will be provided at. This is the oversampling bit.
2) Sometimes adding carefully controlled noise - effectively dithering but ensuring the
noise is not at a frequency of interest so it can be removed later without effecting
what we are after - this allows obtaining a theoretically higher bit rate signal than
what you are measuring with - in extreme cases it allows the one bit adc trick - more
commonly it's just about reducing the noise as a result of the measurement process.
3) ADC conversion - post noise addition if we are deliberately adding some.
4) Applying digital filters as desired.
5) Finally outputing at the reduced data rate.

So - in the simplest form we actually have a straight forward average filter just like
the one your device is applying.  

Now you are correct that the data goes digital at a much higher rate than the output
rate in oversampling - however if we assume a chip is providing an oversampling function
inherently it must then be dropping it down to a more reasonable level in chip (rather than
leaving it to the CPU)  Hence if we have an iio chip with oversampling it's applying a
filter - whether that is a simple average filter, or something a touch more clever is the
only real question.

Now I've probably confused this discussion even more ;)

Jonathan