[PATCH 0/3] Add configfs support for software triggers in IIO

[PATCH 0/3] Add configfs support for software triggers in IIO

[Daniel Baluta]

This patchset adds initial support for configfs in IIO. It is structured in 
the following way:

	* patch 1 - adds configfs infrastructure creating an "iio" configfs
	subystem + an initial "triggers" group.
	* patch 2 - adds the first "real" trigger type - hrtimer.
	* patch 3 - adds Documentation

Daniel Baluta (3):
  iio: configfs: Add configfs support to IIO
  iio: trigger: Add support for highres timer trigger
  iio: Documentation: Add initial documentaton for IIO

 Documentation/iio/iio_configfs.txt       |  74 ++++++++
 drivers/iio/Kconfig                      |   8 +
 drivers/iio/Makefile                     |   1 +
 drivers/iio/industrialio-configfs.c      | 301 +++++++++++++++++++++++++++++++
 drivers/iio/trigger/Kconfig              |   8 +
 drivers/iio/trigger/Makefile             |   2 +
 drivers/iio/trigger/iio-trig-hrtimer.c   | 146 +++++++++++++++
 include/linux/iio/iio_configfs_trigger.h |  49 +++++
 8 files changed, 589 insertions(+)
 create mode 100644 Documentation/iio/iio_configfs.txt
 create mode 100644 drivers/iio/industrialio-configfs.c
 create mode 100644 drivers/iio/trigger/iio-trig-hrtimer.c
 create mode 100644 include/linux/iio/iio_configfs_trigger.h

-- 
1.9.1

[PATCH 1/3] iio: configfs: Add configfs support to IIO

[Daniel Baluta]

This creates an IIO configfs subsystem named "iio", which has one default
group named "triggers". This allows us to easily create/destroy software
triggers. One must create a driver which implements iio_configfs_trigger.h
interface and then add its trigger type to IIO configfs core.

See Documentation/iio/iio_configfs.txt for more details on how configfs
support for IIO works.

Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
 drivers/iio/Kconfig                      |   8 +
 drivers/iio/Makefile                     |   1 +
 drivers/iio/industrialio-configfs.c      | 297 +++++++++++++++++++++++++++++++
 include/linux/iio/iio_configfs_trigger.h |  48 +++++
 4 files changed, 354 insertions(+)
 create mode 100644 drivers/iio/industrialio-configfs.c
 create mode 100644 include/linux/iio/iio_configfs_trigger.h

diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index 4132935..39f1b69 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -18,6 +18,14 @@ config IIO_BUFFER
 	  Provide core support for various buffer based data
 	  acquisition methods.
 
+config IIO_CONFIGFS
+	tristate "Enable IIO configuration via configfs"
+	select CONFIGFS_FS
+	help
+	  This allows configuring various IIO bits through configfs
+	  (e.g software trigger creation / destruction). For more info
+	  see Documentation/iio/iio_configfs.txt.
+
 if IIO_BUFFER
 
 config IIO_BUFFER_CB
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index 698afc2..90cc407 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -5,6 +5,7 @@
 obj-$(CONFIG_IIO) += industrialio.o
 industrialio-y := industrialio-core.o industrialio-event.o inkern.o
 industrialio-$(CONFIG_IIO_BUFFER) += industrialio-buffer.o
+industrialio-$(CONFIG_IIO_CONFIGFS) += industrialio-configfs.o
 industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o
 industrialio-$(CONFIG_IIO_BUFFER_CB) += buffer_cb.o
 
diff --git a/drivers/iio/industrialio-configfs.c b/drivers/iio/industrialio-configfs.c
new file mode 100644
index 0000000..4d2133a
--- /dev/null
+++ b/drivers/iio/industrialio-configfs.c
@@ -0,0 +1,297 @@
+/*
+ * 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/slab.h>
+
+#include <linux/iio/iio_configfs_trigger.h>
+
+static const char *trigger_types[] =
+{
+	"none",
+};
+
+struct iio_configfs_ops iio_none_ops = {
+	.get_freq	= iio_none_get_freq,
+	.set_freq	= iio_none_set_freq,
+	.probe		= iio_none_probe,
+	.remove		= iio_none_remove,
+};
+
+struct iio_trigger_item {
+	struct config_item item;
+	struct iio_configfs_trigger_info *trigger_info;
+};
+
+static
+inline struct iio_trigger_item *to_iio_trigger_item(struct config_item *item)
+{
+	if (!item)
+		return NULL;
+	return container_of(item, struct iio_trigger_item, item);
+}
+
+static unsigned int iio_trigger_get_type(const char *type_str)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(trigger_types); i++) {
+		if (!strncmp(trigger_types[i], type_str,
+			    strlen(trigger_types[i])))
+			return i;
+	}
+	return -EINVAL;
+}
+
+static
+void iio_trigger_set_configfs_ops(struct iio_configfs_trigger_info *trig_info,
+				  unsigned int type)
+{
+	switch (type) {
+	case IIO_TRIGGER_TYPE_NONE:
+		trig_info->configfs_ops = &iio_none_ops;
+		break;
+	default:
+		pr_err("Setting configfs ops failed! Unknown type %d\n", type);
+		break;
+	}
+}
+
+CONFIGFS_ATTR_STRUCT(iio_trigger_item);
+
+#define IIO_TRIGGER_ITEM_ATTR(_name, _mode, _show, _store) \
+struct iio_trigger_item_attribute iio_trigger_item_attr_##_name = \
+	__CONFIGFS_ATTR(_name, _mode, _show, _store)
+
+static ssize_t iio_trigger_item_type_read(struct iio_trigger_item *item,
+					  char *page)
+{
+	return sprintf(page, "%s\n", trigger_types[item->trigger_info->type]);
+}
+
+static ssize_t iio_trigger_item_type_write(struct iio_trigger_item *item,
+					   const char *page, size_t count)
+{
+	int type;
+
+	if (item->trigger_info->active)
+		return -EBUSY;
+
+	type = iio_trigger_get_type(page);
+	if (type < 0)
+		return -EINVAL;
+
+	item->trigger_info->type = type;
+
+	iio_trigger_set_configfs_ops(item->trigger_info, type);
+
+	return count;
+}
+
+static ssize_t iio_trigger_item_activate_read(struct iio_trigger_item *item,
+					      char *page)
+{
+	return sprintf(page, "%d\n", item->trigger_info->active);
+}
+
+static ssize_t iio_trigger_item_activate_write(struct iio_trigger_item *item,
+					       const char *page, size_t count)
+{
+	bool requested_action;
+	int ret;
+
+	ret = strtobool(page, &requested_action);
+	if (ret < 0)
+		return ret;
+
+	if (requested_action == item->trigger_info->active)
+		return -EINVAL;
+
+	if (requested_action)
+		item->trigger_info->configfs_ops->probe(item->trigger_info);
+	else
+		item->trigger_info->configfs_ops->remove(item->trigger_info);
+
+	item->trigger_info->active = requested_action;
+
+	return count;
+}
+
+static
+ssize_t iio_trigger_item_get_sampling_freq(struct iio_trigger_item *item,
+					   char *page)
+{
+	int ret;
+
+	if (!item->trigger_info->active)
+		return -ENODEV;
+
+	ret = item->trigger_info->configfs_ops->get_freq(item->trigger_info);
+	if (ret < 0)
+		return ret;
+	return sprintf(page, "%d\n", ret);
+}
+
+static
+ssize_t iio_trigger_item_set_sampling_freq(struct iio_trigger_item *item,
+					   const char *page,
+					   size_t count)
+{
+	int ret;
+	unsigned long freq;
+
+	if (!item->trigger_info->active)
+		return -ENODEV;
+
+	ret = kstrtoul(page, 10, &freq);
+	if (ret < 0)
+		return 0;
+
+	ret = item->trigger_info->configfs_ops->set_freq(item->trigger_info,
+							 freq);
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+IIO_TRIGGER_ITEM_ATTR(type, S_IRUGO | S_IWUSR,
+		      iio_trigger_item_type_read,
+		      iio_trigger_item_type_write);
+
+IIO_TRIGGER_ITEM_ATTR(activate, S_IRUGO | S_IWUSR,
+		      iio_trigger_item_activate_read,
+		      iio_trigger_item_activate_write);
+
+IIO_TRIGGER_ITEM_ATTR(sampling_frequency, S_IRUGO | S_IWUSR,
+		      iio_trigger_item_get_sampling_freq,
+		      iio_trigger_item_set_sampling_freq);
+
+static struct configfs_attribute *iio_trigger_item_attrs[] = {
+	&iio_trigger_item_attr_type.attr,
+	&iio_trigger_item_attr_sampling_frequency.attr,
+	&iio_trigger_item_attr_activate.attr,
+	NULL
+};
+
+CONFIGFS_ATTR_OPS(iio_trigger_item);
+
+static struct configfs_item_operations iio_trigger_item_ops = {
+	.show_attribute = iio_trigger_item_attr_show,
+	.store_attribute = iio_trigger_item_attr_store,
+};
+
+static struct config_item_type iio_triggers_item_type;
+
+static struct config_item *iio_triggers_make_item(struct config_group *group,
+						  const char *name)
+{
+	struct iio_trigger_item *trigger_item;
+	struct iio_configfs_trigger_info *trigger_info;
+
+	trigger_item = kzalloc(sizeof(*trigger_item), GFP_KERNEL);
+	if (!trigger_item)
+		return ERR_PTR(-ENOMEM);
+	trigger_info = kzalloc(sizeof(*trigger_info), GFP_KERNEL);
+	if (!trigger_info)
+		goto out_free_trigger_item;
+
+	trigger_info->name = kstrdup(name, GFP_KERNEL);
+	if (!trigger_info->name)
+		goto out_free_trigger_info;
+
+	trigger_info->type = IIO_TRIGGER_TYPE_NONE;
+	trigger_info->configfs_ops = &iio_none_ops;
+
+	trigger_item->trigger_info = trigger_info;
+	config_item_init_type_name(&trigger_item->item, name,
+				   &iio_triggers_item_type);
+
+	return &trigger_item->item;
+out_free_trigger_info:
+	kfree(trigger_info);
+out_free_trigger_item:
+	kfree(trigger_item);
+	return ERR_PTR(-ENOMEM);
+}
+
+static void iio_triggers_drop_item(struct config_group *group,
+				   struct config_item *item)
+{
+	struct iio_trigger_item *trigger_item;
+
+	trigger_item = container_of(item, struct iio_trigger_item, item);
+
+	kfree(trigger_item->trigger_info->name);
+	kfree(trigger_item->trigger_info);
+	kfree(trigger_item);
+}
+
+static struct configfs_group_operations iio_triggers_group_ops = {
+	.make_item = iio_triggers_make_item,
+	.drop_item = iio_triggers_drop_item,
+};
+
+static struct config_item_type iio_triggers_item_type = {
+	.ct_owner	= THIS_MODULE,
+	.ct_item_ops	= &iio_trigger_item_ops,
+	.ct_attrs	= iio_trigger_item_attrs,
+};
+
+static struct config_item_type iio_triggers_root_type = {
+	.ct_owner	= THIS_MODULE,
+	.ct_group_ops	= &iio_triggers_group_ops,
+};
+
+static struct config_group iio_triggers_group = {
+	.cg_item = {
+		.ci_namebuf = "triggers",
+		.ci_type = &iio_triggers_root_type,
+	},
+};
+
+static struct config_group *iio_default_groups[] = {
+	&iio_triggers_group,
+	NULL
+};
+
+static struct config_item_type iio_root_group_type = {
+	.ct_owner       = THIS_MODULE,
+};
+
+static struct configfs_subsystem iio_configfs_subsys = {
+	.su_group = {
+		.cg_item = {
+			.ci_namebuf = "iio",
+			.ci_type = &iio_root_group_type,
+		},
+		.default_groups = iio_default_groups,
+	},
+	.su_mutex = __MUTEX_INITIALIZER(iio_configfs_subsys.su_mutex),
+};
+
+static int __init iio_configfs_init(void)
+{
+	config_group_init(&iio_triggers_group);
+	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");
diff --git a/include/linux/iio/iio_configfs_trigger.h b/include/linux/iio/iio_configfs_trigger.h
new file mode 100644
index 0000000..e7b5777
--- /dev/null
+++ b/include/linux/iio/iio_configfs_trigger.h
@@ -0,0 +1,48 @@
+#ifndef __IIO_CONFIGFS_TRIGGER
+#define __IIO_CONFIGFS_TRIGGER
+
+#define IIO_TRIGGER_TYPE_NONE		0
+
+struct iio_configfs_trigger_info {
+	const char *name;
+	unsigned int type;
+	unsigned int active;
+	struct iio_trigger *trigger;
+	struct iio_configfs_ops *configfs_ops;
+};
+
+struct iio_configfs_ops {
+	int (*get_freq)(struct iio_configfs_trigger_info* );
+	int (*set_freq)(struct iio_configfs_trigger_info *, unsigned long);
+	int (*probe)(struct iio_configfs_trigger_info* );
+	int (*remove)(struct iio_configfs_trigger_info* );
+};
+
+static 
+inline int iio_none_get_freq(struct iio_configfs_trigger_info *trigger_info)
+{ 
+	return 0; 
+}
+
+static 
+inline int iio_none_set_freq(struct iio_configfs_trigger_info *trigger_info,
+			     unsigned long f)
+{
+	return 0; 
+}
+
+static
+inline int iio_none_probe(struct iio_configfs_trigger_info *trigger_info)
+{
+	return 0; 
+}
+
+static 
+inline int iio_none_remove(struct iio_configfs_trigger_info *trigger_info)
+{
+	return 0; 
+}
+
+extern struct iio_configfs_ops iio_hrtimer_ops;
+
+#endif /* __IIO_CONFIGFS_TRIGGER */
-- 
1.9.1

[PATCH 2/3] iio: trigger: Add support for highres timer trigger

[Daniel Baluta]

This patch adds an IIO trigger driver which uses a high resolution
timer to provide a frequency based trigger.

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/industrialio-configfs.c      |   4 +
 drivers/iio/trigger/Kconfig              |   8 ++
 drivers/iio/trigger/Makefile             |   2 +
 drivers/iio/trigger/iio-trig-hrtimer.c   | 146 +++++++++++++++++++++++++++++++
 include/linux/iio/iio_configfs_trigger.h |   1 +
 5 files changed, 161 insertions(+)
 create mode 100644 drivers/iio/trigger/iio-trig-hrtimer.c

diff --git a/drivers/iio/industrialio-configfs.c b/drivers/iio/industrialio-configfs.c
index 4d2133a..22740c0 100644
--- a/drivers/iio/industrialio-configfs.c
+++ b/drivers/iio/industrialio-configfs.c
@@ -17,6 +17,7 @@
 static const char *trigger_types[] =
 {
 	"none",
+	"hrtimer",
 };
 
 struct iio_configfs_ops iio_none_ops = {
@@ -59,6 +60,9 @@ void iio_trigger_set_configfs_ops(struct iio_configfs_trigger_info *trig_info,
 	case IIO_TRIGGER_TYPE_NONE:
 		trig_info->configfs_ops = &iio_none_ops;
 		break;
+	case IIO_TRIGGER_TYPE_HRTIMER:
+		trig_info->configfs_ops = &iio_hrtimer_ops;
+		break;
 	default:
 		pr_err("Setting configfs ops failed! Unknown type %d\n", type);
 		break;
diff --git a/drivers/iio/trigger/Kconfig b/drivers/iio/trigger/Kconfig
index 7999612..c856664 100644
--- a/drivers/iio/trigger/Kconfig
+++ b/drivers/iio/trigger/Kconfig
@@ -5,6 +5,14 @@
 
 menu "Triggers - standalone"
 
+config IIO_TRIGGER_HRTIMER
+	tristate "High resolution timer trigger"
+	help
+	  Provides a frequency based IIO trigger using hrtimers.
+
+	  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..3d289f6 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_TRIGGER_HRTIMER) += 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..1a8beaf
--- /dev/null
+++ b/drivers/iio/trigger/iio-trig-hrtimer.c
@@ -0,0 +1,146 @@
+/**
+ * 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>
+ *
+ * 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/platform_device.h>
+#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/iio_configfs_trigger.h>
+
+struct iio_hrtimer_trig_info {
+	struct iio_configfs_trigger_info *configfs_info;
+	unsigned int frequency;
+	struct hrtimer timer;
+	ktime_t period;
+};
+
+static enum hrtimer_restart iio_trig_hrtimer_trig(struct hrtimer *timer)
+{
+	struct iio_hrtimer_trig_info *trig_info;
+
+	trig_info = container_of(timer, struct iio_hrtimer_trig_info, timer);
+
+	hrtimer_forward_now(timer, trig_info->period);
+	iio_trigger_poll(trig_info->configfs_info->trigger);
+
+	return HRTIMER_RESTART;
+}
+
+static int iio_trig_hrtimer_set_state(struct iio_trigger *trig, bool state)
+{
+	struct iio_hrtimer_trig_info *trig_info;
+
+	trig_info = iio_trigger_get_drvdata(trig);
+
+	if (trig_info->frequency == 0)
+		return -EINVAL;
+
+	if (state) {
+		hrtimer_start(&trig_info->timer, trig_info->period,
+			HRTIMER_MODE_REL);
+	} else {
+		hrtimer_cancel(&trig_info->timer);
+	}
+
+	return 0;
+}
+
+int iio_trig_hrtimer_read_freq(struct iio_configfs_trigger_info *cinfo)
+{
+	struct iio_hrtimer_trig_info *trig_info;
+
+	trig_info = iio_trigger_get_drvdata(cinfo->trigger);
+	return trig_info->frequency;
+}
+
+int iio_trig_hrtimer_write_freq(struct iio_configfs_trigger_info *cinfo,
+				unsigned long freq)
+{
+	struct iio_hrtimer_trig_info *trig_info;
+
+	trig_info = iio_trigger_get_drvdata(cinfo->trigger);
+
+	if (freq > NSEC_PER_SEC)
+		return -EINVAL;
+
+	if (freq != 0)
+		trig_info->period = ktime_set(0, NSEC_PER_SEC / freq);
+	trig_info->frequency = freq;
+
+	return 0;
+}
+
+static const struct iio_trigger_ops iio_hrtimer_trigger_ops = {
+	.owner = THIS_MODULE,
+	.set_trigger_state = iio_trig_hrtimer_set_state,
+};
+
+static int iio_trig_hrtimer_probe(struct iio_configfs_trigger_info *cinfo)
+{
+	struct iio_hrtimer_trig_info *trig_info;
+	int ret;
+
+	trig_info = kzalloc(sizeof(*trig_info), GFP_KERNEL);
+	if (!trig_info)
+		return -ENOMEM;
+
+	trig_info->configfs_info = cinfo;
+
+	cinfo->trigger = iio_trigger_alloc("%s", cinfo->name);
+	if (!cinfo->trigger)
+		return -ENOMEM;
+
+	iio_trigger_set_drvdata(cinfo->trigger, trig_info);
+	cinfo->trigger->ops = &iio_hrtimer_trigger_ops;
+
+	hrtimer_init(&trig_info->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	trig_info->timer.function = iio_trig_hrtimer_trig;
+	ret = iio_trigger_register(cinfo->trigger);
+	if (ret)
+		goto err_free_trigger;
+
+	return 0;
+err_free_trigger:
+	iio_trigger_free(cinfo->trigger);
+
+	return ret;
+}
+
+static int iio_trig_hrtimer_remove(struct iio_configfs_trigger_info *cinfo)
+{
+	struct iio_hrtimer_trig_info *trig_info;
+
+	trig_info = iio_trigger_get_drvdata(cinfo->trigger);
+
+	iio_trigger_unregister(cinfo->trigger);
+	hrtimer_cancel(&trig_info->timer);
+	iio_trigger_free(cinfo->trigger);
+
+	return 0;
+}
+
+struct iio_configfs_ops iio_hrtimer_ops = {
+	.get_freq = iio_trig_hrtimer_read_freq,
+	.set_freq = iio_trig_hrtimer_write_freq,
+	.probe = iio_trig_hrtimer_probe,
+	.remove = iio_trig_hrtimer_remove,
+};
+EXPORT_SYMBOL(iio_hrtimer_ops);
+
+MODULE_AUTHOR("Marten Svanfeldt <marten@intuitiveaerial.com>");
+MODULE_DESCRIPTION("Periodic hrtimer trigger for the IIO subsystem");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/iio/iio_configfs_trigger.h b/include/linux/iio/iio_configfs_trigger.h
index e7b5777..1390474 100644
--- a/include/linux/iio/iio_configfs_trigger.h
+++ b/include/linux/iio/iio_configfs_trigger.h
@@ -2,6 +2,7 @@
 #define __IIO_CONFIGFS_TRIGGER
 
 #define IIO_TRIGGER_TYPE_NONE		0
+#define IIO_TRIGGER_TYPE_HRTIMER	1
 
 struct iio_configfs_trigger_info {
 	const char *name;
-- 
1.9.1

[PATCH 3/3] iio: Documentation: Add initial documentaton for IIO

[Daniel Baluta]

This file wants to be a starting point document for anyone wanting
to use IIO configfs support or adding new IIO configfs functionality.

Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
 Documentation/iio/iio_configfs.txt | 74 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 Documentation/iio/iio_configfs.txt

diff --git a/Documentation/iio/iio_configfs.txt b/Documentation/iio/iio_configfs.txt
new file mode 100644
index 0000000..494f4ff
--- /dev/null
+++ b/Documentation/iio/iio_configfs.txt
@@ -0,0 +1,74 @@
+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 creatd and can be accessed under
+/config/iio. Next chapters will describe available IIO configurable objects.
+
+2.1. Software triggers creation and destruction
+
+One of the IIO default configfs groups is the "triggers" groups. It is
+automagically accessible when the configfs is mounted and can be found under
+/config/iio/triggers.
+
+2.1.1. Trigger creation
+
+As simply as:
+
+$ mkdir /config/triggers/my_trigger
+
+This will create a directory associated with a trigger. To understand how this
+works we first need to see "my_triggers"'s attributes:
+
+$ ls /config/triggers/my_trigger
+activate            sampling_frequency  type
+
+Available types for triggers are:
+	* none, this is a default dummy trigger that does nothing.
+	* hrtimer, this is trigger based on high resolution timer.
+
+Order of operations in order to create a "hrtimer" trigger:
+
+$ echo hrtimer > /config/triggers/my_trigger/type
+$ echo 1 > /config/triggers/my_trigger/activate
+$ echo 100 > /config/triggers/my_trigger/sampling_frequency
+
+At this point the trigger can be used by an IIO device.
+
+2.1.2 Trigger destruction
+
+$ echo 1 > /config/triggers/my_trigger/activate
+
+3. Misc
+
+In order to add a new trigger type, one need to implement a driver that creates
+an instance of struct iio_configfs_ops (see iio_configfs_trigger.h header file
+in include/linux/iio) and then support it in iio_trigger_set_configfs_ops
+function from industrialiio-configfs.c file.
+
+These are the existing drivers implementing new trigger types:
+	* hrtimer => iio/trigger/iio-trig-hrtimer.c
+
+4. Further work
+
+* IIO dummy device creation
+* Mappings to 'soft' in kernel users such as iio_input and iio_hwmon
+* IIO on IIO drivers
+
-- 
1.9.1

Re: [PATCH 3/3] iio: Documentation: Add initial documentaton for IIO

[Peter Meerwald]

On Wed, 25 Mar 2015, Daniel Baluta wrote:

> This file wants to be a starting point document for anyone wanting
> to use IIO configfs support or adding new IIO configfs functionality.

typos below; I'm just trying to understand what's going on...
 
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> ---
>  Documentation/iio/iio_configfs.txt | 74 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 74 insertions(+)
>  create mode 100644 Documentation/iio/iio_configfs.txt
> 
> diff --git a/Documentation/iio/iio_configfs.txt b/Documentation/iio/iio_configfs.txt
> new file mode 100644
> index 0000000..494f4ff
> --- /dev/null
> +++ b/Documentation/iio/iio_configfs.txt
> @@ -0,0 +1,74 @@
> +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,

e.g.

> +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 creatd and can be accessed under

created

> +/config/iio. Next chapters will describe available IIO configurable objects.

IIO configuration objects?

> +
> +2.1. Software triggers creation and destruction

trigger

> +
> +One of the IIO default configfs groups is the "triggers" groups. It is
> +automagically accessible when the configfs is mounted and can be found under
> +/config/iio/triggers.
> +
> +2.1.1. Trigger creation
> +
> +As simply as:
> +
> +$ mkdir /config/triggers/my_trigger
> +
> +This will create a directory associated with a trigger. To understand how this
> +works we first need to see "my_triggers"'s attributes:
> +
> +$ ls /config/triggers/my_trigger

should be config/iio/triggers? here and elsewhere?

> +activate            sampling_frequency  type
> +
> +Available types for triggers are:
> +	* none, this is a default dummy trigger that does nothing.
> +	* hrtimer, this is trigger based on high resolution timer.
> +
> +Order of operations in order to create a "hrtimer" trigger:

Sequence of operations, or just 'operations'

> +
> +$ echo hrtimer > /config/triggers/my_trigger/type
> +$ echo 1 > /config/triggers/my_trigger/activate
> +$ echo 100 > /config/triggers/my_trigger/sampling_frequency
> +
> +At this point the trigger can be used by an IIO device.
> +
> +2.1.2 Trigger destruction
> +
> +$ echo 1 > /config/triggers/my_trigger/activate

echo 0
probably

> +
> +3. Misc
> +
> +In order to add a new trigger type, one need to implement a driver that creates

needs

> +an instance of struct iio_configfs_ops (see iio_configfs_trigger.h header file
> +in include/linux/iio) and then support it in iio_trigger_set_configfs_ops
> +function from industrialiio-configfs.c file.
> +
> +These are the existing drivers implementing new trigger types:
> +	* hrtimer => iio/trigger/iio-trig-hrtimer.c
> +
> +4. Further work
> +
> +* IIO dummy device creation
> +* Mappings to 'soft' in kernel users such as iio_input and iio_hwmon

in-kernel

> +* IIO on IIO drivers

??

> +
> 

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [PATCH 1/3] iio: configfs: Add configfs support to IIO

[Peter Meerwald]

> This creates an IIO configfs subsystem named "iio", which has one default
> group named "triggers". This allows us to easily create/destroy software
> triggers. One must create a driver which implements iio_configfs_trigger.h
> interface and then add its trigger type to IIO configfs core.

some minor things noted below... I'm just trying to understand what this 
is about :)
 
> See Documentation/iio/iio_configfs.txt for more details on how configfs
> support for IIO works.
> 
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> ---
>  drivers/iio/Kconfig                      |   8 +
>  drivers/iio/Makefile                     |   1 +
>  drivers/iio/industrialio-configfs.c      | 297 +++++++++++++++++++++++++++++++
>  include/linux/iio/iio_configfs_trigger.h |  48 +++++
>  4 files changed, 354 insertions(+)
>  create mode 100644 drivers/iio/industrialio-configfs.c
>  create mode 100644 include/linux/iio/iio_configfs_trigger.h
> 
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index 4132935..39f1b69 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -18,6 +18,14 @@ config IIO_BUFFER
>  	  Provide core support for various buffer based data
>  	  acquisition methods.
>  
> +config IIO_CONFIGFS
> +	tristate "Enable IIO configuration via configfs"
> +	select CONFIGFS_FS
> +	help
> +	  This allows configuring various IIO bits through configfs
> +	  (e.g software trigger creation / destruction). For more info
> +	  see Documentation/iio/iio_configfs.txt.
> +
>  if IIO_BUFFER
>  
>  config IIO_BUFFER_CB
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index 698afc2..90cc407 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -5,6 +5,7 @@
>  obj-$(CONFIG_IIO) += industrialio.o
>  industrialio-y := industrialio-core.o industrialio-event.o inkern.o
>  industrialio-$(CONFIG_IIO_BUFFER) += industrialio-buffer.o
> +industrialio-$(CONFIG_IIO_CONFIGFS) += industrialio-configfs.o
>  industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o
>  industrialio-$(CONFIG_IIO_BUFFER_CB) += buffer_cb.o
>  
> diff --git a/drivers/iio/industrialio-configfs.c b/drivers/iio/industrialio-configfs.c
> new file mode 100644
> index 0000000..4d2133a
> --- /dev/null
> +++ b/drivers/iio/industrialio-configfs.c
> @@ -0,0 +1,297 @@
> +/*
> + * 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/slab.h>
> +
> +#include <linux/iio/iio_configfs_trigger.h>
> +
> +static const char *trigger_types[] =
> +{
> +	"none",
> +};
> +
> +struct iio_configfs_ops iio_none_ops = {
> +	.get_freq	= iio_none_get_freq,
> +	.set_freq	= iio_none_set_freq,
> +	.probe		= iio_none_probe,
> +	.remove		= iio_none_remove,
> +};
> +
> +struct iio_trigger_item {
> +	struct config_item item;
> +	struct iio_configfs_trigger_info *trigger_info;
> +};
> +
> +static
> +inline struct iio_trigger_item *to_iio_trigger_item(struct config_item *item)
> +{
> +	if (!item)
> +		return NULL;
> +	return container_of(item, struct iio_trigger_item, item);
> +}
> +
> +static unsigned int iio_trigger_get_type(const char *type_str)
> +{
> +	int i;

unsigned
the function returns an index, not a type; probably get_type_index()?

> +
> +	for (i = 0; i < ARRAY_SIZE(trigger_types); i++) {
> +		if (!strncmp(trigger_types[i], type_str,
> +			    strlen(trigger_types[i])))
> +			return i;
> +	}
> +	return -EINVAL;
> +}
> +
> +static
> +void iio_trigger_set_configfs_ops(struct iio_configfs_trigger_info *trig_info,
> +				  unsigned int type)
> +{
> +	switch (type) {
> +	case IIO_TRIGGER_TYPE_NONE:
> +		trig_info->configfs_ops = &iio_none_ops;
> +		break;
> +	default:
> +		pr_err("Setting configfs ops failed! Unknown type %d\n", type);

%u

> +		break;
> +	}
> +}
> +
> +CONFIGFS_ATTR_STRUCT(iio_trigger_item);
> +
> +#define IIO_TRIGGER_ITEM_ATTR(_name, _mode, _show, _store) \
> +struct iio_trigger_item_attribute iio_trigger_item_attr_##_name = \
> +	__CONFIGFS_ATTR(_name, _mode, _show, _store)
> +
> +static ssize_t iio_trigger_item_type_read(struct iio_trigger_item *item,
> +					  char *page)
> +{
> +	return sprintf(page, "%s\n", trigger_types[item->trigger_info->type]);
> +}
> +
> +static ssize_t iio_trigger_item_type_write(struct iio_trigger_item *item,
> +					   const char *page, size_t count)
> +{
> +	int type;
> +
> +	if (item->trigger_info->active)
> +		return -EBUSY;
> +
> +	type = iio_trigger_get_type(page);
> +	if (type < 0)
> +		return -EINVAL;
> +
> +	item->trigger_info->type = type;
> +
> +	iio_trigger_set_configfs_ops(item->trigger_info, type);
> +
> +	return count;
> +}
> +
> +static ssize_t iio_trigger_item_activate_read(struct iio_trigger_item *item,
> +					      char *page)
> +{
> +	return sprintf(page, "%d\n", item->trigger_info->active);
> +}
> +
> +static ssize_t iio_trigger_item_activate_write(struct iio_trigger_item *item,
> +					       const char *page, size_t count)
> +{
> +	bool requested_action;
> +	int ret;
> +
> +	ret = strtobool(page, &requested_action);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (requested_action == item->trigger_info->active)
> +		return -EINVAL;
> +
> +	if (requested_action)
> +		item->trigger_info->configfs_ops->probe(item->trigger_info);
> +	else
> +		item->trigger_info->configfs_ops->remove(item->trigger_info);
> +
> +	item->trigger_info->active = requested_action;
> +
> +	return count;
> +}
> +
> +static
> +ssize_t iio_trigger_item_get_sampling_freq(struct iio_trigger_item *item,
> +					   char *page)
> +{
> +	int ret;
> +
> +	if (!item->trigger_info->active)
> +		return -ENODEV;
> +
> +	ret = item->trigger_info->configfs_ops->get_freq(item->trigger_info);
> +	if (ret < 0)
> +		return ret;
> +	return sprintf(page, "%d\n", ret);
> +}
> +
> +static
> +ssize_t iio_trigger_item_set_sampling_freq(struct iio_trigger_item *item,
> +					   const char *page,
> +					   size_t count)

count is size_t, return type is ssize_t

> +{
> +	int ret;
> +	unsigned long freq;
> +
> +	if (!item->trigger_info->active)
> +		return -ENODEV;
> +
> +	ret = kstrtoul(page, 10, &freq);
> +	if (ret < 0)
> +		return 0;
> +
> +	ret = item->trigger_info->configfs_ops->set_freq(item->trigger_info,
> +							 freq);
> +	if (ret < 0)
> +		return ret;
> +
> +	return count;
> +}
> +
> +IIO_TRIGGER_ITEM_ATTR(type, S_IRUGO | S_IWUSR,
> +		      iio_trigger_item_type_read,
> +		      iio_trigger_item_type_write);
> +
> +IIO_TRIGGER_ITEM_ATTR(activate, S_IRUGO | S_IWUSR,
> +		      iio_trigger_item_activate_read,
> +		      iio_trigger_item_activate_write);
> +
> +IIO_TRIGGER_ITEM_ATTR(sampling_frequency, S_IRUGO | S_IWUSR,
> +		      iio_trigger_item_get_sampling_freq,
> +		      iio_trigger_item_set_sampling_freq);
> +
> +static struct configfs_attribute *iio_trigger_item_attrs[] = {
> +	&iio_trigger_item_attr_type.attr,
> +	&iio_trigger_item_attr_sampling_frequency.attr,
> +	&iio_trigger_item_attr_activate.attr,
> +	NULL
> +};
> +
> +CONFIGFS_ATTR_OPS(iio_trigger_item);
> +
> +static struct configfs_item_operations iio_trigger_item_ops = {
> +	.show_attribute = iio_trigger_item_attr_show,
> +	.store_attribute = iio_trigger_item_attr_store,
> +};
> +
> +static struct config_item_type iio_triggers_item_type;
> +
> +static struct config_item *iio_triggers_make_item(struct config_group *group,
> +						  const char *name)
> +{
> +	struct iio_trigger_item *trigger_item;
> +	struct iio_configfs_trigger_info *trigger_info;
> +
> +	trigger_item = kzalloc(sizeof(*trigger_item), GFP_KERNEL);
> +	if (!trigger_item)
> +		return ERR_PTR(-ENOMEM);
> +	trigger_info = kzalloc(sizeof(*trigger_info), GFP_KERNEL);
> +	if (!trigger_info)
> +		goto out_free_trigger_item;
> +
> +	trigger_info->name = kstrdup(name, GFP_KERNEL);
> +	if (!trigger_info->name)
> +		goto out_free_trigger_info;
> +
> +	trigger_info->type = IIO_TRIGGER_TYPE_NONE;
> +	trigger_info->configfs_ops = &iio_none_ops;
> +
> +	trigger_item->trigger_info = trigger_info;
> +	config_item_init_type_name(&trigger_item->item, name,
> +				   &iio_triggers_item_type);
> +
> +	return &trigger_item->item;
> +out_free_trigger_info:
> +	kfree(trigger_info);
> +out_free_trigger_item:
> +	kfree(trigger_item);
> +	return ERR_PTR(-ENOMEM);
> +}
> +
> +static void iio_triggers_drop_item(struct config_group *group,
> +				   struct config_item *item)
> +{
> +	struct iio_trigger_item *trigger_item;
> +
> +	trigger_item = container_of(item, struct iio_trigger_item, item);
> +
> +	kfree(trigger_item->trigger_info->name);
> +	kfree(trigger_item->trigger_info);
> +	kfree(trigger_item);
> +}
> +
> +static struct configfs_group_operations iio_triggers_group_ops = {
> +	.make_item = iio_triggers_make_item,
> +	.drop_item = iio_triggers_drop_item,
> +};
> +
> +static struct config_item_type iio_triggers_item_type = {
> +	.ct_owner	= THIS_MODULE,
> +	.ct_item_ops	= &iio_trigger_item_ops,
> +	.ct_attrs	= iio_trigger_item_attrs,
> +};
> +
> +static struct config_item_type iio_triggers_root_type = {
> +	.ct_owner	= THIS_MODULE,
> +	.ct_group_ops	= &iio_triggers_group_ops,
> +};
> +
> +static struct config_group iio_triggers_group = {
> +	.cg_item = {
> +		.ci_namebuf = "triggers",
> +		.ci_type = &iio_triggers_root_type,
> +	},
> +};
> +
> +static struct config_group *iio_default_groups[] = {
> +	&iio_triggers_group,
> +	NULL
> +};
> +
> +static struct config_item_type iio_root_group_type = {
> +	.ct_owner       = THIS_MODULE,
> +};
> +
> +static struct configfs_subsystem iio_configfs_subsys = {
> +	.su_group = {
> +		.cg_item = {
> +			.ci_namebuf = "iio",
> +			.ci_type = &iio_root_group_type,
> +		},
> +		.default_groups = iio_default_groups,
> +	},
> +	.su_mutex = __MUTEX_INITIALIZER(iio_configfs_subsys.su_mutex),
> +};
> +
> +static int __init iio_configfs_init(void)
> +{
> +	config_group_init(&iio_triggers_group);
> +	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");
> diff --git a/include/linux/iio/iio_configfs_trigger.h b/include/linux/iio/iio_configfs_trigger.h
> new file mode 100644
> index 0000000..e7b5777
> --- /dev/null
> +++ b/include/linux/iio/iio_configfs_trigger.h
> @@ -0,0 +1,48 @@
> +#ifndef __IIO_CONFIGFS_TRIGGER
> +#define __IIO_CONFIGFS_TRIGGER
> +
> +#define IIO_TRIGGER_TYPE_NONE		0
> +
> +struct iio_configfs_trigger_info {
> +	const char *name;
> +	unsigned int type;
> +	unsigned int active;

bool?

> +	struct iio_trigger *trigger;
> +	struct iio_configfs_ops *configfs_ops;
> +};
> +
> +struct iio_configfs_ops {
> +	int (*get_freq)(struct iio_configfs_trigger_info* );

should return unsigned long?
extra space after * and before ) looks weird

> +	int (*set_freq)(struct iio_configfs_trigger_info *, unsigned long);
> +	int (*probe)(struct iio_configfs_trigger_info* );
> +	int (*remove)(struct iio_configfs_trigger_info* );
> +};
> +
> +static 
> +inline int iio_none_get_freq(struct iio_configfs_trigger_info *trigger_info)
> +{ 
> +	return 0; 
> +}
> +
> +static 
> +inline int iio_none_set_freq(struct iio_configfs_trigger_info *trigger_info,
> +			     unsigned long f)
> +{
> +	return 0; 
> +}
> +
> +static
> +inline int iio_none_probe(struct iio_configfs_trigger_info *trigger_info)
> +{
> +	return 0; 
> +}
> +
> +static 
> +inline int iio_none_remove(struct iio_configfs_trigger_info *trigger_info)
> +{
> +	return 0; 
> +}
> +
> +extern struct iio_configfs_ops iio_hrtimer_ops;

shouldn't probably be here yet

> +
> +#endif /* __IIO_CONFIGFS_TRIGGER */
> 

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [PATCH 3/3] iio: Documentation: Add initial documentaton for IIO

[Daniel Baluta]

On Wed, Mar 25, 2015 at 7:22 PM, Peter Meerwald <pmeerw@pmeerw.net> wrote:
> On Wed, 25 Mar 2015, Daniel Baluta wrote:
>
>> This file wants to be a starting point document for anyone wanting
>> to use IIO configfs support or adding new IIO configfs functionality.
>
> typos below; I'm just trying to understand what's going on...

Hi Peter,

Thanks a lot for your time. I agree with all your comments for this patch
and will fix in v2.
<snip>

>> +* IIO on IIO drivers
>
> ??

I will add more details about this. I was trying to describe the IIO
elements that
would make sense to be configured via configfs.

Have a look at Jonathan's email:

http://marc.info/?l=linux-iio&m=142693832028677&w=2

thanks,
Daniel.

Re: [PATCH 1/3] iio: configfs: Add configfs support to IIO

[Paul Bolle]

A license nit.

On Wed, 2015-03-25 at 19:00 +0200, Daniel Baluta wrote:
> --- /dev/null
> +++ b/drivers/iio/industrialio-configfs.c

> + * 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.

> +MODULE_AUTHOR("Daniel Baluta <daniel.baluta@intel.com>");
> +MODULE_DESCRIPTION("Industrial I/O configfs support");

You probably want
    MODULE_LICENSE("GPL v2");

here too. Because now, at module load time, the license will default to
"unspecified" which will taint the kernel.


Paul Bolle

Re: [PATCH 1/3] iio: configfs: Add configfs support to IIO

[Daniel Baluta]

On 03/25/2015 10:14 PM, Paul Bolle wrote:
> A license nit.
>
> On Wed, 2015-03-25 at 19:00 +0200, Daniel Baluta wrote:
>> --- /dev/null
>> +++ b/drivers/iio/industrialio-configfs.c
>
>> + * 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.
>
>> +MODULE_AUTHOR("Daniel Baluta <daniel.baluta@intel.com>");
>> +MODULE_DESCRIPTION("Industrial I/O configfs support");
>
> You probably want
>      MODULE_LICENSE("GPL v2");
>
> here too. Because now, at module load time, the license will default to
> "unspecified" which will taint the kernel.

:), correct! I forgot about it.

Daniel.

Re: [PATCH 1/3] iio: configfs: Add configfs support to IIO

[Lars-Peter Clausen]

On 03/25/2015 06:00 PM, Daniel Baluta wrote:
> This creates an IIO configfs subsystem named "iio", which has one default
> group named "triggers". This allows us to easily create/destroy software
> triggers. One must create a driver which implements iio_configfs_trigger.h
> interface and then add its trigger type to IIO configfs core.
>
> See Documentation/iio/iio_configfs.txt for more details on how configfs
> support for IIO works.
>
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>

Very good stuff, thanks.

[...]
> diff --git a/drivers/iio/industrialio-configfs.c b/drivers/iio/industrialio-configfs.c
> new file mode 100644
> index 0000000..4d2133a
> --- /dev/null
> +++ b/drivers/iio/industrialio-configfs.c
> @@ -0,0 +1,297 @@
> +/*
> + * 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/slab.h>
> +
> +#include <linux/iio/iio_configfs_trigger.h>
> +
> +static const char *trigger_types[] =
> +{
> +	"none",
> +};

This doesn't necessarily need to be in the initial implementation, but it 
would be good instead of having a global registry inside the core module to 
have a set of functions that allow to register a configfs trigger. These 
functions can be called from the module that implements the trigger in the 
init and exit section. A helper macro similar to module_platform_driver that 
auto-generates those section would be helpful.

Cause right now we do have the dependencies wrong. The core module has a 
symbol dependency on the trigger modules implementing the trigger. That 
means you need to load the trigger module before the core module and also 
means that if at least one trigger is build as a module the core also needs 
to be built as a module.

E.g. lets say there are triggerA, triggerB and triggerC. All build as a 
module. If you want to use any of them you still have to load all of them 
since the core module has a dependency on all of them.

> +
> +struct iio_configfs_ops iio_none_ops = {
> +	.get_freq	= iio_none_get_freq,
> +	.set_freq	= iio_none_set_freq,
> +	.probe		= iio_none_probe,
> +	.remove		= iio_none_remove,
> +};
> +
> +struct iio_trigger_item {
> +	struct config_item item;
> +	struct iio_configfs_trigger_info *trigger_info;
> +};
> +
> +static
> +inline struct iio_trigger_item *to_iio_trigger_item(struct config_item *item)
> +{
> +	if (!item)
> +		return NULL;
> +	return container_of(item, struct iio_trigger_item, item);
> +}
> +
> +static unsigned int iio_trigger_get_type(const char *type_str)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(trigger_types); i++) {
> +		if (!strncmp(trigger_types[i], type_str,
> +			    strlen(trigger_types[i])))
> +			return i;
> +	}
> +	return -EINVAL;
> +}
> +
> +static
> +void iio_trigger_set_configfs_ops(struct iio_configfs_trigger_info *trig_info,
> +				  unsigned int type)
> +{
> +	switch (type) {
> +	case IIO_TRIGGER_TYPE_NONE:
> +		trig_info->configfs_ops = &iio_none_ops;
> +		break;
> +	default:
> +		pr_err("Setting configfs ops failed! Unknown type %d\n", type);
> +		break;
> +	}
> +}

I wonder if it is not better to have a sub-directory per trigger type and if 
you create a trigger in the sub-directory it is automatically of that type.

The advantage is that you can have e.g. trigger specific properties.

The downside is that you no longer have a global namespace and there could 
be name collisions by creating triggers with the same name, but with 
different types. This could be solved though by making sure that the 
internal name is pre-fixed by the type. E.g. "hrtimer-foobar" or 
"hrtimer/foobar".

> +
> +CONFIGFS_ATTR_STRUCT(iio_trigger_item);
> +
> +#define IIO_TRIGGER_ITEM_ATTR(_name, _mode, _show, _store) \
> +struct iio_trigger_item_attribute iio_trigger_item_attr_##_name = \
> +	__CONFIGFS_ATTR(_name, _mode, _show, _store)
> +
> +static ssize_t iio_trigger_item_type_read(struct iio_trigger_item *item,
> +					  char *page)
> +{
> +	return sprintf(page, "%s\n", trigger_types[item->trigger_info->type]);
> +}
> +
> +static ssize_t iio_trigger_item_type_write(struct iio_trigger_item *item,
> +					   const char *page, size_t count)
> +{
> +	int type;
> +
> +	if (item->trigger_info->active)
> +		return -EBUSY;
> +
> +	type = iio_trigger_get_type(page);
> +	if (type < 0)
> +		return -EINVAL;
> +
> +	item->trigger_info->type = type;
> +
> +	iio_trigger_set_configfs_ops(item->trigger_info, type);
> +
> +	return count;
> +}
> +
> +static ssize_t iio_trigger_item_activate_read(struct iio_trigger_item *item,
> +					      char *page)
> +{
> +	return sprintf(page, "%d\n", item->trigger_info->active);
> +}
> +
> +static ssize_t iio_trigger_item_activate_write(struct iio_trigger_item *item,
> +					       const char *page, size_t count)
> +{
> +	bool requested_action;
> +	int ret;
> +
> +	ret = strtobool(page, &requested_action);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (requested_action == item->trigger_info->active)
> +		return -EINVAL;
> +
> +	if (requested_action)
> +		item->trigger_info->configfs_ops->probe(item->trigger_info);
> +	else
> +		item->trigger_info->configfs_ops->remove(item->trigger_info);
> +
> +	item->trigger_info->active = requested_action;
> +
> +	return count;
> +}

Do we need the activate attribute? Shouldn't the trigger be create/destroyed 
if the type field is changed? Or if we have one directory per trigger type 
when the directory for the trigger is created using mkdir? I think that 
would make a nice more natural API.

[...]

Re: [PATCH 1/3] iio: configfs: Add configfs support to IIO

[Jonathan Cameron]

On 27/03/15 17:17, Lars-Peter Clausen wrote:
> On 03/25/2015 06:00 PM, Daniel Baluta wrote:
>> This creates an IIO configfs subsystem named "iio", which has one default
>> group named "triggers". This allows us to easily create/destroy software
>> triggers. One must create a driver which implements iio_configfs_trigger.h
>> interface and then add its trigger type to IIO configfs core.
>>
>> See Documentation/iio/iio_configfs.txt for more details on how configfs
>> support for IIO works.
>>
>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> 
> Very good stuff, thanks.
> 
> [...]
>> diff --git a/drivers/iio/industrialio-configfs.c b/drivers/iio/industrialio-configfs.c
>> new file mode 100644
>> index 0000000..4d2133a
>> --- /dev/null
>> +++ b/drivers/iio/industrialio-configfs.c
>> @@ -0,0 +1,297 @@
>> +/*
>> + * 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/slab.h>
>> +
>> +#include <linux/iio/iio_configfs_trigger.h>
>> +
>> +static const char *trigger_types[] =
>> +{
>> +    "none",
>> +};
> 
> This doesn't necessarily need to be in the initial implementation, but it would be good instead of having a global registry inside the core module to have a set of functions that allow to register a configfs trigger. These functions can be called from the module that implements the trigger in the init and exit section. A helper macro similar to module_platform_driver that auto-generates those section would be helpful.
> 
> Cause right now we do have the dependencies wrong. The core module has a symbol dependency on the trigger modules implementing the trigger. That means you need to load the trigger module before the core module and also means that if at least one trigger is build as a module the core also needs to be built as a module.
> 
> E.g. lets say there are triggerA, triggerB and triggerC. All build as a module. If you want to use any of them you still have to load all of them since the core module has a dependency on all of them.
> 
>> +
>> +struct iio_configfs_ops iio_none_ops = {
>> +    .get_freq    = iio_none_get_freq,
>> +    .set_freq    = iio_none_set_freq,
>> +    .probe        = iio_none_probe,
>> +    .remove        = iio_none_remove,
>> +};
>> +
>> +struct iio_trigger_item {
>> +    struct config_item item;
>> +    struct iio_configfs_trigger_info *trigger_info;
>> +};
>> +
>> +static
>> +inline struct iio_trigger_item *to_iio_trigger_item(struct config_item *item)
>> +{
>> +    if (!item)
>> +        return NULL;
>> +    return container_of(item, struct iio_trigger_item, item);
>> +}
>> +
>> +static unsigned int iio_trigger_get_type(const char *type_str)
>> +{
>> +    int i;
>> +
>> +    for (i = 0; i < ARRAY_SIZE(trigger_types); i++) {
>> +        if (!strncmp(trigger_types[i], type_str,
>> +                strlen(trigger_types[i])))
>> +            return i;
>> +    }
>> +    return -EINVAL;
>> +}
>> +
>> +static
>> +void iio_trigger_set_configfs_ops(struct iio_configfs_trigger_info *trig_info,
>> +                  unsigned int type)
>> +{
>> +    switch (type) {
>> +    case IIO_TRIGGER_TYPE_NONE:
>> +        trig_info->configfs_ops = &iio_none_ops;
>> +        break;
>> +    default:
>> +        pr_err("Setting configfs ops failed! Unknown type %d\n", type);
>> +        break;
>> +    }
>> +}
> 

> I wonder if it is not better to have a sub-directory per trigger type
> and if you create a trigger in the sub-directory it is automatically
> of that type.
I agree, this would perhaps be more flexible / intuitive.
> 
> The advantage is that you can have e.g. trigger specific properties.
The initial set of triggers will be (I guess!)

* high resolution timer
* sysfs (userspace trigger) - in parallel with it's existing sysfs interface
  which unfortunately we'll have to keep for at least a few years.
Later on, once we have in kernel interfaces for events I'd expect we'll
want event fired triggers (grab data based on an event from the same device
or a different one for that matter)...

Anyhow, definitely going to want different properties for these!

> 
> The downside is that you no longer have a global namespace and there
> could be name collisions by creating triggers with the same name, but
> with different types. This could be solved though by making sure that
> the internal name is pre-fixed by the type. E.g. "hrtimer-foobar" or
> "hrtimer/foobar".
Yes, the prefix would be sensible. 

>> +
>> +CONFIGFS_ATTR_STRUCT(iio_trigger_item);
>> +
>> +#define IIO_TRIGGER_ITEM_ATTR(_name, _mode, _show, _store) \
>> +struct iio_trigger_item_attribute iio_trigger_item_attr_##_name = \
>> +    __CONFIGFS_ATTR(_name, _mode, _show, _store)
>> +
>> +static ssize_t iio_trigger_item_type_read(struct iio_trigger_item *item,
>> +                      char *page)
>> +{
>> +    return sprintf(page, "%s\n", trigger_types[item->trigger_info->type]);
>> +}
>> +
>> +static ssize_t iio_trigger_item_type_write(struct iio_trigger_item *item,
>> +                       const char *page, size_t count)
>> +{
>> +    int type;
>> +
>> +    if (item->trigger_info->active)
>> +        return -EBUSY;
>> +
>> +    type = iio_trigger_get_type(page);
>> +    if (type < 0)
>> +        return -EINVAL;
>> +
>> +    item->trigger_info->type = type;
>> +
>> +    iio_trigger_set_configfs_ops(item->trigger_info, type);
>> +
>> +    return count;
>> +}
>> +
>> +static ssize_t iio_trigger_item_activate_read(struct iio_trigger_item *item,
>> +                          char *page)
>> +{
>> +    return sprintf(page, "%d\n", item->trigger_info->active);
>> +}
>> +
>> +static ssize_t iio_trigger_item_activate_write(struct iio_trigger_item *item,
>> +                           const char *page, size_t count)
>> +{
>> +    bool requested_action;
>> +    int ret;
>> +
>> +    ret = strtobool(page, &requested_action);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    if (requested_action == item->trigger_info->active)
>> +        return -EINVAL;
>> +
>> +    if (requested_action)
>> +        item->trigger_info->configfs_ops->probe(item->trigger_info);
>> +    else
>> +        item->trigger_info->configfs_ops->remove(item->trigger_info);
>> +
>> +    item->trigger_info->active = requested_action;
>> +
>> +    return count;
>> +}
> 
> Do we need the activate attribute? Shouldn't the trigger be
> create/destroyed if the type field is changed? Or if we have one
> directory per trigger type when the directory for the trigger is
> created using mkdir? I think that would make a nice more natural
> API.> 
> [...]
Only exception would be a trigger that has no meaning until we have
configured it - e.g. the event triggers mentioned above.  That would
need an explicit activate.  Question is whether we want to start
out with that interface for all triggers on the basis it will be
needed later?

Actually come to think of it, there is no problem with registering
an unconfigured trigger - just with using it.  Hence we could do
so and only fault out when a buffer using the trigger is started.
That works for the complex usecase (as long as we spit out appropriate
log messages / error codes).

Hence if we can get away with doing everything on the mkdir that
would be great.
> 
> -- 
> 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: [PATCH 3/3] iio: Documentation: Add initial documentaton for IIO

[Jonathan Cameron]

On 25/03/15 19:28, Daniel Baluta wrote:
> On Wed, Mar 25, 2015 at 7:22 PM, Peter Meerwald <pmeerw@pmeerw.net> wrote:
>> On Wed, 25 Mar 2015, Daniel Baluta wrote:
>>
>>> This file wants to be a starting point document for anyone wanting
>>> to use IIO configfs support or adding new IIO configfs functionality.
>>
>> typos below; I'm just trying to understand what's going on...
> 
> Hi Peter,
> 
> Thanks a lot for your time. I agree with all your comments for this patch
> and will fix in v2.
> <snip>
> 
>>> +* IIO on IIO drivers
>>
>> ??
> 
> I will add more details about this. I was trying to describe the IIO
> elements that
> would make sense to be configured via configfs.
> 
> Have a look at Jonathan's email:
> 
> http://marc.info/?l=linux-iio&m=142693832028677&w=2
> 
> thanks,
> Daniel.
> 
Might be premature / confusing to put those in the help file!
If anyone cares about our motivation for the structure here
they can dig out the email threads!

Re: [PATCH 3/3] iio: Documentation: Add initial documentaton for IIO

[Jonathan Cameron]

On 25/03/15 17:00, Daniel Baluta wrote:
> This file wants to be a starting point document for anyone wanting
> to use IIO configfs support or adding new IIO configfs functionality.
> 
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Looks like a good flexible structure to me.  Thanks!
> ---
>  Documentation/iio/iio_configfs.txt | 74 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 74 insertions(+)
>  create mode 100644 Documentation/iio/iio_configfs.txt
> 
> diff --git a/Documentation/iio/iio_configfs.txt b/Documentation/iio/iio_configfs.txt
> new file mode 100644
> index 0000000..494f4ff
> --- /dev/null
> +++ b/Documentation/iio/iio_configfs.txt
> @@ -0,0 +1,74 @@
> +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 creatd and can be accessed under
> +/config/iio. Next chapters will describe available IIO configurable objects.
> +
> +2.1. Software triggers creation and destruction
> +
> +One of the IIO default configfs groups is the "triggers" groups. It is
> +automagically accessible when the configfs is mounted and can be found under
> +/config/iio/triggers.
> +
> +2.1.1. Trigger creation
> +
> +As simply as:
> +
> +$ mkdir /config/triggers/my_trigger
> +
> +This will create a directory associated with a trigger. To understand how this
> +works we first need to see "my_triggers"'s attributes:
> +
> +$ ls /config/triggers/my_trigger
> +activate            sampling_frequency  type
> +
> +Available types for triggers are:
> +	* none, this is a default dummy trigger that does nothing.
> +	* hrtimer, this is trigger based on high resolution timer.
> +
> +Order of operations in order to create a "hrtimer" trigger:
> +
> +$ echo hrtimer > /config/triggers/my_trigger/type
> +$ echo 1 > /config/triggers/my_trigger/activate
> +$ echo 100 > /config/triggers/my_trigger/sampling_frequency
> +
> +At this point the trigger can be used by an IIO device.
> +
> +2.1.2 Trigger destruction
> +
> +$ echo 1 > /config/triggers/my_trigger/activate
> +
> +3. Misc
> +
> +In order to add a new trigger type, one need to implement a driver that creates
> +an instance of struct iio_configfs_ops (see iio_configfs_trigger.h header file
> +in include/linux/iio) and then support it in iio_trigger_set_configfs_ops
> +function from industrialiio-configfs.c file.
> +
> +These are the existing drivers implementing new trigger types:
> +	* hrtimer => iio/trigger/iio-trig-hrtimer.c
> +
> +4. Further work
> +
> +* IIO dummy device creation
> +* Mappings to 'soft' in kernel users such as iio_input and iio_hwmon
> +* IIO on IIO drivers
> +
> 

Re: [PATCH 1/3] iio: configfs: Add configfs support to IIO

[Daniel Baluta]

On Sat, Mar 28, 2015 at 1:50 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 27/03/15 17:17, Lars-Peter Clausen wrote:
>> On 03/25/2015 06:00 PM, Daniel Baluta wrote:
>>> This creates an IIO configfs subsystem named "iio", which has one default
>>> group named "triggers". This allows us to easily create/destroy software
>>> triggers. One must create a driver which implements iio_configfs_trigger.h
>>> interface and then add its trigger type to IIO configfs core.
>>>
>>> See Documentation/iio/iio_configfs.txt for more details on how configfs
>>> support for IIO works.
>>>
>>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
>>
>> Very good stuff, thanks.
>>
>> [...]
>>> diff --git a/drivers/iio/industrialio-configfs.c b/drivers/iio/industrialio-configfs.c
>>> new file mode 100644
>>> index 0000000..4d2133a
>>> --- /dev/null
>>> +++ b/drivers/iio/industrialio-configfs.c
>>> @@ -0,0 +1,297 @@
>>> +/*
>>> + * 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/slab.h>
>>> +
>>> +#include <linux/iio/iio_configfs_trigger.h>
>>> +
>>> +static const char *trigger_types[] =
>>> +{
>>> +    "none",
>>> +};
>>
>> This doesn't necessarily need to be in the initial implementation, but it would be good instead of having a global registry inside the core module to have a set of functions that allow to register a configfs trigger. These functions can be called from the module that implements the trigger in the init and exit section. A helper macro similar to module_platform_driver that auto-generates those section would be helpful.
>>
>> Cause right now we do have the dependencies wrong. The core module has a symbol dependency on the trigger modules implementing the trigger. That means you need to load the trigger module before the core module and also means that if at least one trigger is build as a module the core also needs to be built as a module.
>>
>> E.g. lets say there are triggerA, triggerB and triggerC. All build as a module. If you want to use any of them you still have to load all of them since the core module has a dependency on all of them.

Good point. I think this is the correct solution. I will add this approach
in v2.

>>
>>> +
>>> +struct iio_configfs_ops iio_none_ops = {
>>> +    .get_freq    = iio_none_get_freq,
>>> +    .set_freq    = iio_none_set_freq,
>>> +    .probe        = iio_none_probe,
>>> +    .remove        = iio_none_remove,
>>> +};
>>> +
>>> +struct iio_trigger_item {
>>> +    struct config_item item;
>>> +    struct iio_configfs_trigger_info *trigger_info;
>>> +};
>>> +
>>> +static
>>> +inline struct iio_trigger_item *to_iio_trigger_item(struct config_item *item)
>>> +{
>>> +    if (!item)
>>> +        return NULL;
>>> +    return container_of(item, struct iio_trigger_item, item);
>>> +}
>>> +
>>> +static unsigned int iio_trigger_get_type(const char *type_str)
>>> +{
>>> +    int i;
>>> +
>>> +    for (i = 0; i < ARRAY_SIZE(trigger_types); i++) {
>>> +        if (!strncmp(trigger_types[i], type_str,
>>> +                strlen(trigger_types[i])))
>>> +            return i;
>>> +    }
>>> +    return -EINVAL;
>>> +}
>>> +
>>> +static
>>> +void iio_trigger_set_configfs_ops(struct iio_configfs_trigger_info *trig_info,
>>> +                  unsigned int type)
>>> +{
>>> +    switch (type) {
>>> +    case IIO_TRIGGER_TYPE_NONE:
>>> +        trig_info->configfs_ops = &iio_none_ops;
>>> +        break;
>>> +    default:
>>> +        pr_err("Setting configfs ops failed! Unknown type %d\n", type);
>>> +        break;
>>> +    }
>>> +}
>>
>
>> I wonder if it is not better to have a sub-directory per trigger type
>> and if you create a trigger in the sub-directory it is automatically
>> of that type.
> I agree, this would perhaps be more flexible / intuitive.

Great, this sounds more in the spirit of configfs. Will add it in v2.

>>
>> The advantage is that you can have e.g. trigger specific properties.
> The initial set of triggers will be (I guess!)
>
> * high resolution timer
> * sysfs (userspace trigger) - in parallel with it's existing sysfs interface
>   which unfortunately we'll have to keep for at least a few years.
> Later on, once we have in kernel interfaces for events I'd expect we'll
> want event fired triggers (grab data based on an event from the same device
> or a different one for that matter)...
>
> Anyhow, definitely going to want different properties for these!
>
>>
>> The downside is that you no longer have a global namespace and there
>> could be name collisions by creating triggers with the same name, but
>> with different types. This could be solved though by making sure that
>> the internal name is pre-fixed by the type. E.g. "hrtimer-foobar" or
>> "hrtimer/foobar".
> Yes, the prefix would be sensible.

It is sensible, but we will enforce it at the driver level.

>
>>> +
>>> +CONFIGFS_ATTR_STRUCT(iio_trigger_item);
>>> +
>>> +#define IIO_TRIGGER_ITEM_ATTR(_name, _mode, _show, _store) \
>>> +struct iio_trigger_item_attribute iio_trigger_item_attr_##_name = \
>>> +    __CONFIGFS_ATTR(_name, _mode, _show, _store)
>>> +
>>> +static ssize_t iio_trigger_item_type_read(struct iio_trigger_item *item,
>>> +                      char *page)
>>> +{
>>> +    return sprintf(page, "%s\n", trigger_types[item->trigger_info->type]);
>>> +}
>>> +
>>> +static ssize_t iio_trigger_item_type_write(struct iio_trigger_item *item,
>>> +                       const char *page, size_t count)
>>> +{
>>> +    int type;
>>> +
>>> +    if (item->trigger_info->active)
>>> +        return -EBUSY;
>>> +
>>> +    type = iio_trigger_get_type(page);
>>> +    if (type < 0)
>>> +        return -EINVAL;
>>> +
>>> +    item->trigger_info->type = type;
>>> +
>>> +    iio_trigger_set_configfs_ops(item->trigger_info, type);
>>> +
>>> +    return count;
>>> +}
>>> +
>>> +static ssize_t iio_trigger_item_activate_read(struct iio_trigger_item *item,
>>> +                          char *page)
>>> +{
>>> +    return sprintf(page, "%d\n", item->trigger_info->active);
>>> +}
>>> +
>>> +static ssize_t iio_trigger_item_activate_write(struct iio_trigger_item *item,
>>> +                           const char *page, size_t count)
>>> +{
>>> +    bool requested_action;
>>> +    int ret;
>>> +
>>> +    ret = strtobool(page, &requested_action);
>>> +    if (ret < 0)
>>> +        return ret;
>>> +
>>> +    if (requested_action == item->trigger_info->active)
>>> +        return -EINVAL;
>>> +
>>> +    if (requested_action)
>>> +        item->trigger_info->configfs_ops->probe(item->trigger_info);
>>> +    else
>>> +        item->trigger_info->configfs_ops->remove(item->trigger_info);
>>> +
>>> +    item->trigger_info->active = requested_action;
>>> +
>>> +    return count;
>>> +}
>>
>> Do we need the activate attribute? Shouldn't the trigger be
>> create/destroyed if the type field is changed? Or if we have one
>> directory per trigger type when the directory for the trigger is
>> created using mkdir? I think that would make a nice more natural
>> API.>
>> [...]
> Only exception would be a trigger that has no meaning until we have
> configured it - e.g. the event triggers mentioned above.  That would
> need an explicit activate.  Question is whether we want to start
> out with that interface for all triggers on the basis it will be
> needed later?

Yes, I was thinking at the case where the trigger needs first to be
configured and then activated. But for hrtimers I think we can remove
activate attribute for now.

>
> Actually come to think of it, there is no problem with registering
> an unconfigured trigger - just with using it.  Hence we could do
> so and only fault out when a buffer using the trigger is started.
> That works for the complex usecase (as long as we spit out appropriate
> log messages / error codes).
>
> Hence if we can get away with doing everything on the mkdir that
> would be great.

Thanks for the feedback. I will send v2 asap.

Daniel.

Re: [PATCH 3/3] iio: Documentation: Add initial documentaton for IIO

[Daniel Baluta]

On Sat, Mar 28, 2015 at 1:53 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 25/03/15 19:28, Daniel Baluta wrote:
>> On Wed, Mar 25, 2015 at 7:22 PM, Peter Meerwald <pmeerw@pmeerw.net> wrote:
>>> On Wed, 25 Mar 2015, Daniel Baluta wrote:
>>>
>>>> This file wants to be a starting point document for anyone wanting
>>>> to use IIO configfs support or adding new IIO configfs functionality.
>>>
>>> typos below; I'm just trying to understand what's going on...
>>
>> Hi Peter,
>>
>> Thanks a lot for your time. I agree with all your comments for this patch
>> and will fix in v2.
>> <snip>
>>
>>>> +* IIO on IIO drivers
>>>
>>> ??
>>
>> I will add more details about this. I was trying to describe the IIO
>> elements that
>> would make sense to be configured via configfs.
>>
>> Have a look at Jonathan's email:
>>
>> http://marc.info/?l=linux-iio&m=142693832028677&w=2
>>
>> thanks,
>> Daniel.
>>
> Might be premature / confusing to put those in the help file!
> If anyone cares about our motivation for the structure here
> they can dig out the email threads!

Agree. Thanks Jonathan!

Daniel.