[PATCH v4 0/4] Add initial configfs support for IIO

[PATCH v4 0/4] Add initial configfs support for IIO

[Daniel Baluta]

This patchset introduces IIO software triggers, offers a way of configuring
them via configfs and adds the IIO hrtimer based interrupt source to be used
with software triggers.

The arhitecture is now split in 3 parts, to remove all IIO trigger specific
parts from IIO configfs core:

(1) IIO software triggers - are independent of configfs.
(2) IIO configfs - offers a generic way of creating IIO objects. So far we can
	create software triggers.
(3) IIO hrtimer trigger - is the first interrupt source for software triggers
	(with syfs to follow). Each trigger type can implement its own set of
	attributes.

Changes since v3:
	* addressed comments from Jonathan for previous version
	* https://lkml.org/lkml/2015/4/6/111
 
Daniel Baluta (4):
  iio: core: Introduce IIO software triggers
  iio: core: Introduce IIO configfs support
  iio: trigger: Introduce IIO hrtimer based trigger
  iio: Documentation: Add IIO configfs documentation

 Documentation/iio/iio_configfs.txt     |  67 +++++++++++
 drivers/iio/Kconfig                    |  16 +++
 drivers/iio/Makefile                   |   2 +
 drivers/iio/industrialio-configfs.c    | 117 +++++++++++++++++++
 drivers/iio/industrialio-sw-trigger.c  | 111 ++++++++++++++++++
 drivers/iio/trigger/Kconfig            |   9 ++
 drivers/iio/trigger/Makefile           |   2 +
 drivers/iio/trigger/iio-trig-hrtimer.c | 201 +++++++++++++++++++++++++++++++++
 include/linux/iio/sw_trigger.h         |  50 ++++++++
 9 files changed, 575 insertions(+)
 create mode 100644 Documentation/iio/iio_configfs.txt
 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

[PATCH v4 1/4] iio: core: Introduce IIO software triggers

[Daniel Baluta]

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.

Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
 drivers/iio/Kconfig                   |   8 +++
 drivers/iio/Makefile                  |   1 +
 drivers/iio/industrialio-sw-trigger.c | 111 ++++++++++++++++++++++++++++++++++
 include/linux/iio/sw_trigger.h        |  50 +++++++++++++++
 4 files changed, 170 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 4011eff..de7f1d9 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -58,6 +58,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
+	bool "Enable software triggers support"
+	depends on IIO_TRIGGER
+	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 698afc2..df87975 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -6,6 +6,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_TRIGGER) += industrialio-trigger.o
+industrialio-$(CONFIG_IIO_SW_TRIGGER) += industrialio-sw-trigger.o
 industrialio-$(CONFIG_IIO_BUFFER_CB) += buffer_cb.o
 
 obj-$(CONFIG_IIO_TRIGGERED_BUFFER) += industrialio-triggered-buffer.o
diff --git a/drivers/iio/industrialio-sw-trigger.c b/drivers/iio/industrialio-sw-trigger.c
new file mode 100644
index 0000000..567c675
--- /dev/null
+++ b/drivers/iio/industrialio-sw-trigger.c
@@ -0,0 +1,111 @@
+/*
+ * 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>
+
+static LIST_HEAD(iio_trigger_types_list);
+static DEFINE_RWLOCK(iio_trigger_types_lock);
+
+static
+struct iio_sw_trigger_type *iio_find_sw_trigger_type(char *name, unsigned len)
+{
+	struct iio_sw_trigger_type *t = NULL, *iter;
+
+	list_for_each_entry(iter, &iio_trigger_types_list, list)
+		if (!strncmp(iter->name, name, len)) {
+			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;
+
+	write_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);
+	write_unlock(&iio_trigger_types_lock);
+	return ret;
+}
+EXPORT_SYMBOL(iio_register_sw_trigger_type);
+
+int iio_unregister_sw_trigger_type(struct iio_sw_trigger_type *t)
+{
+	struct iio_sw_trigger_type *iter;
+	int ret = 0;
+
+	write_lock(&iio_trigger_types_lock);
+	iter = iio_find_sw_trigger_type(t->name, strlen(t->name));
+	if (!iter)
+		ret = -EINVAL;
+	else
+		list_del(&t->list);
+	write_unlock(&iio_trigger_types_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL(iio_unregister_sw_trigger_type);
+
+static
+struct iio_sw_trigger_type *iio_get_sw_trigger_type(char *name)
+{
+	struct iio_sw_trigger_type *t;
+
+	read_lock(&iio_trigger_types_lock);
+	t = iio_find_sw_trigger_type(name, strlen(name));
+	if (t && !try_module_get(t->owner))
+		t = NULL;
+	read_unlock(&iio_trigger_types_lock);
+
+	return t;
+}
+
+struct iio_sw_trigger *iio_sw_trigger_create(char *type, 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;
+
+	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);
diff --git a/include/linux/iio/sw_trigger.h b/include/linux/iio/sw_trigger.h
new file mode 100644
index 0000000..2e6659b
--- /dev/null
+++ b/include/linux/iio/sw_trigger.h
@@ -0,0 +1,50 @@
+#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)
+
+struct iio_sw_trigger_ops;
+
+struct iio_sw_trigger_type {
+	char *name;
+	struct module *owner;
+	struct iio_sw_trigger_ops *ops;
+	struct list_head list;
+};
+
+struct iio_sw_trigger {
+	struct iio_trigger *trigger;
+	struct iio_sw_trigger_type *trigger_type;
+#ifdef CONFIG_CONFIGFS_FS
+	struct config_group group;
+#endif
+};
+
+struct iio_sw_trigger_ops {
+	struct iio_sw_trigger* (*probe)(const char *);
+	int (*remove)(struct iio_sw_trigger *);
+};
+
+int iio_register_sw_trigger_type(struct iio_sw_trigger_type *);
+int iio_unregister_sw_trigger_type(struct iio_sw_trigger_type *);
+
+struct iio_sw_trigger *iio_sw_trigger_create(char *, char *);
+void iio_sw_trigger_destroy(struct iio_sw_trigger *);
+
+#ifdef CONFIG_CONFIGFS_FS
+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);
+}
+#endif
+
+#endif /* __IIO_SW_TRIGGER */
-- 
1.9.1

[PATCH v4 2/4] iio: core: Introduce IIO configfs support

[Daniel Baluta]

This creates an IIO configfs subystem named "iio", with a default "triggers"
group.

Triggers group is used for handling software triggers. To create a new software
trigger one must create a directory inside the trigger directory.

Software trigger name MUST follow the following convention:
	* <trigger-type>-<trigger-name>
Where:
	* <trigger_type>, specifies the interrupt source (e.g: hrtimer)
	* <trigger-name>, specifies the IIO device trigger name

Failing to follow this convention will result in an directory creation error.

E.g, assuming that hrtimer trigger type is registered with IIO software
trigger core:

$ mkdir /config/iio/triggers/hrtimer-instance1

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

diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index de7f1d9..c310156 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 triggers). 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 df87975..31aead3 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -10,6 +10,7 @@ industrialio-$(CONFIG_IIO_SW_TRIGGER) += industrialio-sw-trigger.o
 industrialio-$(CONFIG_IIO_BUFFER_CB) += buffer_cb.o
 
 obj-$(CONFIG_IIO_TRIGGERED_BUFFER) += industrialio-triggered-buffer.o
+obj-$(CONFIG_IIO_CONFIGFS) += industrialio-configfs.o
 obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o
 
 obj-y += accel/
diff --git a/drivers/iio/industrialio-configfs.c b/drivers/iio/industrialio-configfs.c
new file mode 100644
index 0000000..0361434
--- /dev/null
+++ b/drivers/iio/industrialio-configfs.c
@@ -0,0 +1,117 @@
+/*
+ * 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>
+#include <linux/iio/sw_trigger.h>
+
+#define MAX_NAME_LEN 32
+
+static struct config_group *trigger_make_group(struct config_group *group,
+					       const char *name)
+{
+	char *type_name;
+	char *trigger_name;
+	char buf[MAX_NAME_LEN];
+	struct iio_sw_trigger *t;
+
+	snprintf(buf, MAX_NAME_LEN, "%s", name);
+
+	/* group name should have the form <trigger-type>-<trigger-name> */
+	type_name = buf;
+	trigger_name = strchr(buf, '-');
+	if (!trigger_name) {
+		pr_err("Unable to locate '-' in %s. Use <type>-<name>.\n", buf);
+		return ERR_PTR(-EINVAL);
+	}
+
+	/* replace - with \0, this nicely separates the two strings */
+	*trigger_name = '\0';
+	trigger_name++;
+
+	t = iio_sw_trigger_create(type_name, trigger_name);
+	if (IS_ERR(t))
+		return ERR_CAST(t);
+
+	config_item_set_name(&t->group.cg_item, 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);
+
+	if (t)
+		iio_sw_trigger_destroy(t);
+	config_item_put(item);
+}
+
+static struct configfs_group_operations triggers_ops = {
+	.make_group	= &trigger_make_group,
+	.drop_item	= &trigger_drop_group,
+};
+
+static struct config_item_type iio_triggers_group_type = {
+	.ct_group_ops = &triggers_ops,
+	.ct_owner       = THIS_MODULE,
+};
+
+static struct config_group iio_triggers_group = {
+	.cg_item = {
+		.ci_namebuf = "triggers",
+		.ci_type = &iio_triggers_group_type,
+	},
+};
+
+static struct config_group *iio_root_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_root_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");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

[PATCH v4 3/4] iio: trigger: Introduce IIO hrtimer based trigger

[Daniel Baluta]

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            |   9 ++
 drivers/iio/trigger/Makefile           |   2 +
 drivers/iio/trigger/iio-trig-hrtimer.c | 201 +++++++++++++++++++++++++++++++++
 3 files changed, 212 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..454665a 100644
--- a/drivers/iio/trigger/Kconfig
+++ b/drivers/iio/trigger/Kconfig
@@ -5,6 +5,15 @@
 
 menu "Triggers - standalone"
 
+config IIO_HRTIMER_TRIGGER
+	tristate "High resolution timer trigger"
+	select IIO_SW_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..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..5077fad
--- /dev/null
+++ b/drivers/iio/trigger/iio-trig-hrtimer.c
@@ -0,0 +1,201 @@
+/**
+ * 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;
+};
+
+#ifdef CONFIG_CONFIGFS_FS
+struct iio_hrtimer_info *to_iio_hrtimer_info(struct config_item *item)
+{
+	return item ? container_of(to_iio_sw_trigger(item),
+				   struct iio_hrtimer_info, swt) : NULL;
+}
+
+CONFIGFS_ATTR_STRUCT(iio_hrtimer_info);
+
+#define IIO_HRTIMER_INFO_ATTR(_name, _mode, _show, _store) \
+	struct iio_hrtimer_info_attribute iio_hrtimer_attr_##_name = \
+	__CONFIGFS_ATTR(_name, _mode, _show, _store)
+
+static
+ssize_t iio_hrtimer_info_show_sampling_frequency(struct iio_hrtimer_info *info,
+						 char *page)
+{
+	return snprintf(page, PAGE_SIZE, "%lu\n", info->sampling_frequency);
+}
+
+static
+ssize_t iio_hrtimer_info_store_sampling_frequency(struct iio_hrtimer_info *info,
+						  const char *page,
+						  size_t count)
+{
+	unsigned long val;
+	int ret;
+
+	ret = kstrtoul(page, 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 count;
+}
+
+IIO_HRTIMER_INFO_ATTR(sampling_frequency, S_IRUGO | S_IWUSR,
+		      iio_hrtimer_info_show_sampling_frequency,
+		      iio_hrtimer_info_store_sampling_frequency);
+
+static struct configfs_attribute *iio_hrtimer_attrs[] = {
+	&iio_hrtimer_attr_sampling_frequency.attr,
+	NULL
+};
+
+CONFIGFS_ATTR_OPS(iio_hrtimer_info);
+static struct configfs_item_operations iio_hrtimer_ops = {
+	.show_attribute		= iio_hrtimer_info_attr_show,
+	.store_attribute	= iio_hrtimer_info_attr_store,
+};
+
+static struct config_item_type iio_hrtimer_type = {
+	.ct_item_ops	= &iio_hrtimer_ops,
+	.ct_attrs	= iio_hrtimer_attrs,
+	.ct_owner	= THIS_MODULE,
+};
+
+#endif /* CONFIGFS_FS */
+
+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;
+
+	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;
+#ifdef CONFIG_CONFIGFS_FS
+	config_group_init_type_name(&trig_info->swt.group, name,
+				    &iio_hrtimer_type);
+#endif
+	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);
+
+	hrtimer_cancel(&trig_info->timer);
+
+	iio_trigger_unregister(swt->trigger);
+	iio_trigger_free(swt->trigger);
+	kfree(trig_info);
+
+	return 0;
+}
+
+struct iio_sw_trigger_ops iio_trig_hrtimer_ops = {
+	.probe		= iio_trig_hrtimer_probe,
+	.remove		= iio_trig_hrtimer_remove,
+};
+
+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

[PATCH v4 4/4] iio: Documentation: Add IIO configfs documentation

[Daniel Baluta]

Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
 Documentation/iio/iio_configfs.txt | 67 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 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..1aa66d1
--- /dev/null
+++ b/Documentation/iio/iio_configfs.txt
@@ -0,0 +1,67 @@
+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" groups. It is
+automagically accessible when the configfs is mounted and can be found
+under /config/iio/triggers.
+
+Software triggers are created under /config/iio/triggers directory. A sofware
+trigger name MUST be of the following form:
+	* <trigger-type>-<trigger-name>:
+Where:
+	* <trigger-type>, specifies the interrupt source (e.g: hrtimer)
+	* <trigger-name>, spefcifies the IIO device trigger name
+
+We support now to following interrupt sources (trigger types):
+	* hrtimer, uses high resolution timers as interrupt source
+
+3.1 Software triggers creation and destruction
+
+As simply as:
+
+$ mkdir /config/triggers/<trigger-type>-<trigger-name>
+$ rmdir /config/triggers/<trigger-type>-<trigger-name>
+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 has only one attribute:
+
+$ ls /config/triggers/hrtimer-instance1
+sampling_frequency
+
+sampling_frequency - represents the period in Hz between two consecutive
+iio_trigger_poll calls. By default it is set to 100Hz.
+
+4. Further work
+
+* add "sysfs" trigger type
-- 
1.9.1

Re: [PATCH v4 1/4] iio: core: Introduce IIO software triggers

[Jonathan Cameron]

On 20/04/15 15:02, Daniel Baluta wrote:
> 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.
> 
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Looks sensible.
My only real question is if the rwlock is justified (vs a mutex).

> ---
>  drivers/iio/Kconfig                   |   8 +++
>  drivers/iio/Makefile                  |   1 +
>  drivers/iio/industrialio-sw-trigger.c | 111 ++++++++++++++++++++++++++++++++++
>  include/linux/iio/sw_trigger.h        |  50 +++++++++++++++
>  4 files changed, 170 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 4011eff..de7f1d9 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -58,6 +58,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
> +	bool "Enable software triggers support"
> +	depends on IIO_TRIGGER
> +	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 698afc2..df87975 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -6,6 +6,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_TRIGGER) += industrialio-trigger.o
> +industrialio-$(CONFIG_IIO_SW_TRIGGER) += industrialio-sw-trigger.o
>  industrialio-$(CONFIG_IIO_BUFFER_CB) += buffer_cb.o
>  
>  obj-$(CONFIG_IIO_TRIGGERED_BUFFER) += industrialio-triggered-buffer.o
> diff --git a/drivers/iio/industrialio-sw-trigger.c b/drivers/iio/industrialio-sw-trigger.c
> new file mode 100644
> index 0000000..567c675
> --- /dev/null
> +++ b/drivers/iio/industrialio-sw-trigger.c
> @@ -0,0 +1,111 @@
> +/*
> + * 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>
> +
> +static LIST_HEAD(iio_trigger_types_list);
> +static DEFINE_RWLOCK(iio_trigger_types_lock);
Can see the logic, but I'm not totally convinced a rwlock is necessary.
We don't actually create new ones terribly often...
> +
> +static
> +struct iio_sw_trigger_type *iio_find_sw_trigger_type(char *name, unsigned len)
> +{
> +	struct iio_sw_trigger_type *t = NULL, *iter;
> +
> +	list_for_each_entry(iter, &iio_trigger_types_list, list)
> +		if (!strncmp(iter->name, name, len)) {
> +			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;
> +
> +	write_lock(&iio_trigger_types_lock);
> +	iter = iio_find_sw_trigger_type(t->name, strlen(t->name));
> +	if (iter)
> +		ret = -EBUSY;
Rather than EAGAIN?  Could also do the magic attempt to autoload the
module that the usb gadget driver does (though add that as a later
feature rather than in this first posting I think to keep complexity
of patch manageable).
> +	else
> +		list_add_tail(&t->list, &iio_trigger_types_list);
> +	write_unlock(&iio_trigger_types_lock);
nitpick :) blank line here.
> +	return ret;
> +}
> +EXPORT_SYMBOL(iio_register_sw_trigger_type);
> +
> +int iio_unregister_sw_trigger_type(struct iio_sw_trigger_type *t)
> +{
> +	struct iio_sw_trigger_type *iter;
> +	int ret = 0;
> +
> +	write_lock(&iio_trigger_types_lock);
> +	iter = iio_find_sw_trigger_type(t->name, strlen(t->name));
> +	if (!iter)
> +		ret = -EINVAL;
> +	else
> +		list_del(&t->list);
> +	write_unlock(&iio_trigger_types_lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(iio_unregister_sw_trigger_type);
> +
> +static
> +struct iio_sw_trigger_type *iio_get_sw_trigger_type(char *name)
> +{
> +	struct iio_sw_trigger_type *t;
> +
> +	read_lock(&iio_trigger_types_lock);
> +	t = iio_find_sw_trigger_type(name, strlen(name));
> +	if (t && !try_module_get(t->owner))
> +		t = NULL;
> +	read_unlock(&iio_trigger_types_lock);
> +
> +	return t;
> +}
> +
> +struct iio_sw_trigger *iio_sw_trigger_create(char *type, char *name)
> +{
> +	struct iio_sw_trigger *t;
> +	struct iio_sw_trigger_type *tt;
I like the brief variable names (perfectly clear so not actually being
sarcastic ;))
> +
> +	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;
> +
> +	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);
> diff --git a/include/linux/iio/sw_trigger.h b/include/linux/iio/sw_trigger.h
> new file mode 100644
> index 0000000..2e6659b
> --- /dev/null
> +++ b/include/linux/iio/sw_trigger.h
> @@ -0,0 +1,50 @@
> +#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)
> +
> +struct iio_sw_trigger_ops;
> +
> +struct iio_sw_trigger_type {
> +	char *name;
> +	struct module *owner;
> +	struct iio_sw_trigger_ops *ops;
> +	struct list_head list;
> +};
> +
> +struct iio_sw_trigger {
> +	struct iio_trigger *trigger;
> +	struct iio_sw_trigger_type *trigger_type;
> +#ifdef CONFIG_CONFIGFS_FS
> +	struct config_group group;
> +#endif
> +};
> +
> +struct iio_sw_trigger_ops {
> +	struct iio_sw_trigger* (*probe)(const char *);
> +	int (*remove)(struct iio_sw_trigger *);
> +};
> +
> +int iio_register_sw_trigger_type(struct iio_sw_trigger_type *);
> +int iio_unregister_sw_trigger_type(struct iio_sw_trigger_type *);
> +
> +struct iio_sw_trigger *iio_sw_trigger_create(char *, char *);
> +void iio_sw_trigger_destroy(struct iio_sw_trigger *);
> +
> +#ifdef CONFIG_CONFIGFS_FS
> +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);
> +}
> +#endif
> +
> +#endif /* __IIO_SW_TRIGGER */
> 

Re: [PATCH v4 2/4] iio: core: Introduce IIO configfs support

[Jonathan Cameron]

On 20/04/15 15:02, Daniel Baluta wrote:
> This creates an IIO configfs subystem named "iio", with a default "triggers"
> group.
> 
> Triggers group is used for handling software triggers. To create a new software
> trigger one must create a directory inside the trigger directory.
> 
> Software trigger name MUST follow the following convention:
> 	* <trigger-type>-<trigger-name>
> Where:
> 	* <trigger_type>, specifies the interrupt source (e.g: hrtimer)
> 	* <trigger-name>, specifies the IIO device trigger name
> 
> Failing to follow this convention will result in an directory creation error.
> 
> E.g, assuming that hrtimer trigger type is registered with IIO software
> trigger core:
> 
> $ mkdir /config/iio/triggers/hrtimer-instance1
> 
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Looks good. Couple of little comments inline.

Jonathan
> ---
>  drivers/iio/Kconfig                 |   8 +++
>  drivers/iio/Makefile                |   1 +
>  drivers/iio/industrialio-configfs.c | 117 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 126 insertions(+)
>  create mode 100644 drivers/iio/industrialio-configfs.c
> 
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index de7f1d9..c310156 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 triggers). 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 df87975..31aead3 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -10,6 +10,7 @@ industrialio-$(CONFIG_IIO_SW_TRIGGER) += industrialio-sw-trigger.o
>  industrialio-$(CONFIG_IIO_BUFFER_CB) += buffer_cb.o
>  
>  obj-$(CONFIG_IIO_TRIGGERED_BUFFER) += industrialio-triggered-buffer.o
> +obj-$(CONFIG_IIO_CONFIGFS) += industrialio-configfs.o
>  obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o
>  
>  obj-y += accel/
> diff --git a/drivers/iio/industrialio-configfs.c b/drivers/iio/industrialio-configfs.c
> new file mode 100644
> index 0000000..0361434
> --- /dev/null
> +++ b/drivers/iio/industrialio-configfs.c
> @@ -0,0 +1,117 @@
> +/*
> + * 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>
> +#include <linux/iio/sw_trigger.h>
> +
> +#define MAX_NAME_LEN 32
Strikes me as perhaps a little short given we want to allow 'almost' arbitary names for the triggers.
> +
> +static struct config_group *trigger_make_group(struct config_group *group,
> +					       const char *name)
> +{
> +	char *type_name;
> +	char *trigger_name;
> +	char buf[MAX_NAME_LEN];
> +	struct iio_sw_trigger *t;
> +
> +	snprintf(buf, MAX_NAME_LEN, "%s", name);
> +
> +	/* group name should have the form <trigger-type>-<trigger-name> */
> +	type_name = buf;
> +	trigger_name = strchr(buf, '-');
> +	if (!trigger_name) {
> +		pr_err("Unable to locate '-' in %s. Use <type>-<name>.\n", buf);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	/* replace - with \0, this nicely separates the two strings */
> +	*trigger_name = '\0';
Dirty trick, but I like it.
> +	trigger_name++;
> +
> +	t = iio_sw_trigger_create(type_name, trigger_name);
> +	if (IS_ERR(t))
> +		return ERR_CAST(t);
> +
> +	config_item_set_name(&t->group.cg_item, 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);
> +
> +	if (t)
> +		iio_sw_trigger_destroy(t);
> +	config_item_put(item);
> +}
> +
> +static struct configfs_group_operations triggers_ops = {
> +	.make_group	= &trigger_make_group,
> +	.drop_item	= &trigger_drop_group,
> +};
> +
> +static struct config_item_type iio_triggers_group_type = {
> +	.ct_group_ops = &triggers_ops,
> +	.ct_owner       = THIS_MODULE,
> +};
> +
> +static struct config_group iio_triggers_group = {
> +	.cg_item = {
> +		.ci_namebuf = "triggers",
> +		.ci_type = &iio_triggers_group_type,
> +	},
> +};
> +
> +static struct config_group *iio_root_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_root_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");
> +MODULE_LICENSE("GPL v2");
> 

Re: [PATCH v4 3/4] iio: trigger: Introduce IIO hrtimer based trigger

[Jonathan Cameron]

On 20/04/15 15:02, Daniel Baluta wrote:
> 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>
Looks good to me - will let the series sit a little longer though
for others to take a look.
> ---
>  drivers/iio/trigger/Kconfig            |   9 ++
>  drivers/iio/trigger/Makefile           |   2 +
>  drivers/iio/trigger/iio-trig-hrtimer.c | 201 +++++++++++++++++++++++++++++++++
>  3 files changed, 212 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..454665a 100644
> --- a/drivers/iio/trigger/Kconfig
> +++ b/drivers/iio/trigger/Kconfig
> @@ -5,6 +5,15 @@
>  
>  menu "Triggers - standalone"
>  
> +config IIO_HRTIMER_TRIGGER
> +	tristate "High resolution timer trigger"
> +	select IIO_SW_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..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..5077fad
> --- /dev/null
> +++ b/drivers/iio/trigger/iio-trig-hrtimer.c
> @@ -0,0 +1,201 @@
> +/**
> + * 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;
> +};
> +
> +#ifdef CONFIG_CONFIGFS_FS
> +struct iio_hrtimer_info *to_iio_hrtimer_info(struct config_item *item)
> +{
> +	return item ? container_of(to_iio_sw_trigger(item),
> +				   struct iio_hrtimer_info, swt) : NULL;
> +}
> +
> +CONFIGFS_ATTR_STRUCT(iio_hrtimer_info);
> +
> +#define IIO_HRTIMER_INFO_ATTR(_name, _mode, _show, _store) \
> +	struct iio_hrtimer_info_attribute iio_hrtimer_attr_##_name = \
> +	__CONFIGFS_ATTR(_name, _mode, _show, _store)
> +
> +static
> +ssize_t iio_hrtimer_info_show_sampling_frequency(struct iio_hrtimer_info *info,
> +						 char *page)
> +{
> +	return snprintf(page, PAGE_SIZE, "%lu\n", info->sampling_frequency);
> +}
> +
> +static
> +ssize_t iio_hrtimer_info_store_sampling_frequency(struct iio_hrtimer_info *info,
> +						  const char *page,
> +						  size_t count)
> +{
> +	unsigned long val;
> +	int ret;
> +
> +	ret = kstrtoul(page, 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 count;
> +}
> +
> +IIO_HRTIMER_INFO_ATTR(sampling_frequency, S_IRUGO | S_IWUSR,
> +		      iio_hrtimer_info_show_sampling_frequency,
> +		      iio_hrtimer_info_store_sampling_frequency);
> +
> +static struct configfs_attribute *iio_hrtimer_attrs[] = {
> +	&iio_hrtimer_attr_sampling_frequency.attr,
> +	NULL
> +};
> +
> +CONFIGFS_ATTR_OPS(iio_hrtimer_info);
> +static struct configfs_item_operations iio_hrtimer_ops = {
> +	.show_attribute		= iio_hrtimer_info_attr_show,
> +	.store_attribute	= iio_hrtimer_info_attr_store,
> +};
> +
> +static struct config_item_type iio_hrtimer_type = {
> +	.ct_item_ops	= &iio_hrtimer_ops,
> +	.ct_attrs	= iio_hrtimer_attrs,
> +	.ct_owner	= THIS_MODULE,
> +};
> +
> +#endif /* CONFIGFS_FS */
> +
> +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;
> +
> +	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;
> +#ifdef CONFIG_CONFIGFS_FS
> +	config_group_init_type_name(&trig_info->swt.group, name,
> +				    &iio_hrtimer_type);
> +#endif
> +	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);
> +
> +	hrtimer_cancel(&trig_info->timer);
> +
> +	iio_trigger_unregister(swt->trigger);
> +	iio_trigger_free(swt->trigger);
> +	kfree(trig_info);
> +
> +	return 0;
> +}
> +
> +struct iio_sw_trigger_ops iio_trig_hrtimer_ops = {
> +	.probe		= iio_trig_hrtimer_probe,
> +	.remove		= iio_trig_hrtimer_remove,
> +};
> +
> +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");
> 

Re: [PATCH v4 4/4] iio: Documentation: Add IIO configfs documentation

[Jonathan Cameron]

On 20/04/15 15:02, Daniel Baluta wrote:
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Probably need to start Documentation/ABI/testing/configfs-iio (or something like that)
as well to document the ABI elements fully.

This plain text doc is particularly useful to hopefully get feedback on the interface
from those who might not dive into the details of the series.  Thanks!
> ---
>  Documentation/iio/iio_configfs.txt | 67 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 67 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..1aa66d1
> --- /dev/null
> +++ b/Documentation/iio/iio_configfs.txt
> @@ -0,0 +1,67 @@
> +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" groups. It is
> +automagically accessible when the configfs is mounted and can be found
> +under /config/iio/triggers.
> +
> +Software triggers are created under /config/iio/triggers directory. A sofware
> +trigger name MUST be of the following form:
> +	* <trigger-type>-<trigger-name>:
> +Where:
> +	* <trigger-type>, specifies the interrupt source (e.g: hrtimer)
> +	* <trigger-name>, spefcifies the IIO device trigger name
> +
> +We support now to following interrupt sources (trigger types):
> +	* hrtimer, uses high resolution timers as interrupt source
> +
> +3.1 Software triggers creation and destruction
> +
> +As simply as:
> +
> +$ mkdir /config/triggers/<trigger-type>-<trigger-name>
> +$ rmdir /config/triggers/<trigger-type>-<trigger-name>
> +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 has only one attribute:
> +
> +$ ls /config/triggers/hrtimer-instance1
> +sampling_frequency
> +
> +sampling_frequency - represents the period in Hz between two consecutive
> +iio_trigger_poll calls. By default it is set to 100Hz.
> +
> +4. Further work
I wouldn't bother with this section here.  
> +
> +* add "sysfs" trigger type
> 

Re: [PATCH v4 1/4] iio: core: Introduce IIO software triggers

[Daniel Baluta]

On Sun, Apr 26, 2015 at 10:21 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 20/04/15 15:02, Daniel Baluta wrote:
>> 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.
>>
>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> Looks sensible.
> My only real question is if the rwlock is justified (vs a mutex).

Hmm, a given iio_trigger_type list element is mostly read. Also, borrowed
the code from file systems core implementation.

http://lxr.linux.no/linux+v3.19.1/fs/filesystems.c#L33

Anyhow, there is no problem to use a mutex.

>
>> ---
>>  drivers/iio/Kconfig                   |   8 +++
>>  drivers/iio/Makefile                  |   1 +
>>  drivers/iio/industrialio-sw-trigger.c | 111 ++++++++++++++++++++++++++++++++++
>>  include/linux/iio/sw_trigger.h        |  50 +++++++++++++++
>>  4 files changed, 170 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 4011eff..de7f1d9 100644
>> --- a/drivers/iio/Kconfig
>> +++ b/drivers/iio/Kconfig
>> @@ -58,6 +58,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
>> +     bool "Enable software triggers support"
>> +     depends on IIO_TRIGGER
>> +     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 698afc2..df87975 100644
>> --- a/drivers/iio/Makefile
>> +++ b/drivers/iio/Makefile
>> @@ -6,6 +6,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_TRIGGER) += industrialio-trigger.o
>> +industrialio-$(CONFIG_IIO_SW_TRIGGER) += industrialio-sw-trigger.o
>>  industrialio-$(CONFIG_IIO_BUFFER_CB) += buffer_cb.o
>>
>>  obj-$(CONFIG_IIO_TRIGGERED_BUFFER) += industrialio-triggered-buffer.o
>> diff --git a/drivers/iio/industrialio-sw-trigger.c b/drivers/iio/industrialio-sw-trigger.c
>> new file mode 100644
>> index 0000000..567c675
>> --- /dev/null
>> +++ b/drivers/iio/industrialio-sw-trigger.c
>> @@ -0,0 +1,111 @@
>> +/*
>> + * 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>
>> +
>> +static LIST_HEAD(iio_trigger_types_list);
>> +static DEFINE_RWLOCK(iio_trigger_types_lock);
> Can see the logic, but I'm not totally convinced a rwlock is necessary.
> We don't actually create new ones terribly often...

I see your point. Could change the code to use a mutex.

>> +
>> +static
>> +struct iio_sw_trigger_type *iio_find_sw_trigger_type(char *name, unsigned len)
>> +{
>> +     struct iio_sw_trigger_type *t = NULL, *iter;
>> +
>> +     list_for_each_entry(iter, &iio_trigger_types_list, list)
>> +             if (!strncmp(iter->name, name, len)) {
>> +                     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;
>> +
>> +     write_lock(&iio_trigger_types_lock);
>> +     iter = iio_find_sw_trigger_type(t->name, strlen(t->name));
>> +     if (iter)
>> +             ret = -EBUSY;
> Rather than EAGAIN?  Could also do the magic attempt to autoload the
> module that the usb gadget driver does (though add that as a later
> feature rather than in this first posting I think to keep complexity
> of patch manageable).

I see, we can do this later. Anyhow, I'm not convinced that EAGAIN is a good
value here. We just want to inform the user that the module registering this
trigger type is already loaded.

Like here:

http://lxr.linux.no/linux+v3.19.1/fs/filesystems.c#L78


>> +     else
>> +             list_add_tail(&t->list, &iio_trigger_types_list);
>> +     write_unlock(&iio_trigger_types_lock);
> nitpick :) blank line here.
>> +     return ret;
>> +}
>> +EXPORT_SYMBOL(iio_register_sw_trigger_type);
>> +
>> +int iio_unregister_sw_trigger_type(struct iio_sw_trigger_type *t)
>> +{
>> +     struct iio_sw_trigger_type *iter;
>> +     int ret = 0;
>> +
>> +     write_lock(&iio_trigger_types_lock);
>> +     iter = iio_find_sw_trigger_type(t->name, strlen(t->name));
>> +     if (!iter)
>> +             ret = -EINVAL;
>> +     else
>> +             list_del(&t->list);
>> +     write_unlock(&iio_trigger_types_lock);
>> +
>> +     return ret;
>> +}
>> +EXPORT_SYMBOL(iio_unregister_sw_trigger_type);
>> +
>> +static
>> +struct iio_sw_trigger_type *iio_get_sw_trigger_type(char *name)
>> +{
>> +     struct iio_sw_trigger_type *t;
>> +
>> +     read_lock(&iio_trigger_types_lock);
>> +     t = iio_find_sw_trigger_type(name, strlen(name));
>> +     if (t && !try_module_get(t->owner))
>> +             t = NULL;
>> +     read_unlock(&iio_trigger_types_lock);
>> +
>> +     return t;
>> +}
>> +
>> +struct iio_sw_trigger *iio_sw_trigger_create(char *type, char *name)
>> +{
>> +     struct iio_sw_trigger *t;
>> +     struct iio_sw_trigger_type *tt;
> I like the brief variable names (perfectly clear so not actually being
> sarcastic ;))
>> +
>> +     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;
>> +
>> +     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);
>> diff --git a/include/linux/iio/sw_trigger.h b/include/linux/iio/sw_trigger.h
>> new file mode 100644
>> index 0000000..2e6659b
>> --- /dev/null
>> +++ b/include/linux/iio/sw_trigger.h
>> @@ -0,0 +1,50 @@
>> +#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)
>> +
>> +struct iio_sw_trigger_ops;
>> +
>> +struct iio_sw_trigger_type {
>> +     char *name;
>> +     struct module *owner;
>> +     struct iio_sw_trigger_ops *ops;
>> +     struct list_head list;
>> +};
>> +
>> +struct iio_sw_trigger {
>> +     struct iio_trigger *trigger;
>> +     struct iio_sw_trigger_type *trigger_type;
>> +#ifdef CONFIG_CONFIGFS_FS
>> +     struct config_group group;
>> +#endif
>> +};
>> +
>> +struct iio_sw_trigger_ops {
>> +     struct iio_sw_trigger* (*probe)(const char *);
>> +     int (*remove)(struct iio_sw_trigger *);
>> +};
>> +
>> +int iio_register_sw_trigger_type(struct iio_sw_trigger_type *);
>> +int iio_unregister_sw_trigger_type(struct iio_sw_trigger_type *);
>> +
>> +struct iio_sw_trigger *iio_sw_trigger_create(char *, char *);
>> +void iio_sw_trigger_destroy(struct iio_sw_trigger *);
>> +
>> +#ifdef CONFIG_CONFIGFS_FS
>> +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);
>> +}
>> +#endif
>> +
>> +#endif /* __IIO_SW_TRIGGER */
>>
>
> --
> 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 v4 0/4] Add initial configfs support for IIO

[Jonathan Cameron]

On 20/04/15 15:02, Daniel Baluta wrote:
> This patchset introduces IIO software triggers, offers a way of configuring
> them via configfs and adds the IIO hrtimer based interrupt source to be used
> with software triggers.
> 
> The arhitecture is now split in 3 parts, to remove all IIO trigger specific
> parts from IIO configfs core:
> 
> (1) IIO software triggers - are independent of configfs.
> (2) IIO configfs - offers a generic way of creating IIO objects. So far we can
> 	create software triggers.
> (3) IIO hrtimer trigger - is the first interrupt source for software triggers
> 	(with syfs to follow). Each trigger type can implement its own set of
> 	attributes.
> 
> Changes since v3:
> 	* addressed comments from Jonathan for previous version
> 	* https://lkml.org/lkml/2015/4/6/111

Hi Daniel.

Thanks for all your hard work on this.  I'm very pleased with the result.
It's clean, remarkably compact and nice and extensible.

The only reason I didn't apply it today (other than the odd nit) was because
it's major new ABI for us so I'd ideally like a few of IIOs main reviewers
to take a look before we take it.

Lars, Harmut, Peter, others (our reviewer set is growing very fast!) if you
guys have time and interest, please take a quick look at this and see if we've
missed anything.  

It's actually remarkably straightforward in the end due to all of Daniel's hard
work so shouldn't take you long unless you want to dig right down into the configfs
side of things.  If you do, then the best reference I found (Daniel may have others)
is the usb-gadget driver.

Jonathan
>  
> Daniel Baluta (4):
>   iio: core: Introduce IIO software triggers
>   iio: core: Introduce IIO configfs support
>   iio: trigger: Introduce IIO hrtimer based trigger
>   iio: Documentation: Add IIO configfs documentation
> 
>  Documentation/iio/iio_configfs.txt     |  67 +++++++++++
>  drivers/iio/Kconfig                    |  16 +++
>  drivers/iio/Makefile                   |   2 +
>  drivers/iio/industrialio-configfs.c    | 117 +++++++++++++++++++
>  drivers/iio/industrialio-sw-trigger.c  | 111 ++++++++++++++++++
>  drivers/iio/trigger/Kconfig            |   9 ++
>  drivers/iio/trigger/Makefile           |   2 +
>  drivers/iio/trigger/iio-trig-hrtimer.c | 201 +++++++++++++++++++++++++++++++++
>  include/linux/iio/sw_trigger.h         |  50 ++++++++
>  9 files changed, 575 insertions(+)
>  create mode 100644 Documentation/iio/iio_configfs.txt
>  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
> 

Re: [PATCH v4 2/4] iio: core: Introduce IIO configfs support

[Daniel Baluta]

On Sun, Apr 26, 2015 at 10:26 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 20/04/15 15:02, Daniel Baluta wrote:
>> This creates an IIO configfs subystem named "iio", with a default "triggers"
>> group.
>>
>> Triggers group is used for handling software triggers. To create a new software
>> trigger one must create a directory inside the trigger directory.
>>
>> Software trigger name MUST follow the following convention:
>>       * <trigger-type>-<trigger-name>
>> Where:
>>       * <trigger_type>, specifies the interrupt source (e.g: hrtimer)
>>       * <trigger-name>, specifies the IIO device trigger name
>>
>> Failing to follow this convention will result in an directory creation error.
>>
>> E.g, assuming that hrtimer trigger type is registered with IIO software
>> trigger core:
>>
>> $ mkdir /config/iio/triggers/hrtimer-instance1
>>
>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> Looks good. Couple of little comments inline.
>
> Jonathan
>> ---
>>  drivers/iio/Kconfig                 |   8 +++
>>  drivers/iio/Makefile                |   1 +
>>  drivers/iio/industrialio-configfs.c | 117 ++++++++++++++++++++++++++++++++++++
>>  3 files changed, 126 insertions(+)
>>  create mode 100644 drivers/iio/industrialio-configfs.c
>>
>> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
>> index de7f1d9..c310156 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 triggers). 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 df87975..31aead3 100644
>> --- a/drivers/iio/Makefile
>> +++ b/drivers/iio/Makefile
>> @@ -10,6 +10,7 @@ industrialio-$(CONFIG_IIO_SW_TRIGGER) += industrialio-sw-trigger.o
>>  industrialio-$(CONFIG_IIO_BUFFER_CB) += buffer_cb.o
>>
>>  obj-$(CONFIG_IIO_TRIGGERED_BUFFER) += industrialio-triggered-buffer.o
>> +obj-$(CONFIG_IIO_CONFIGFS) += industrialio-configfs.o
>>  obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o
>>
>>  obj-y += accel/
>> diff --git a/drivers/iio/industrialio-configfs.c b/drivers/iio/industrialio-configfs.c
>> new file mode 100644
>> index 0000000..0361434
>> --- /dev/null
>> +++ b/drivers/iio/industrialio-configfs.c
>> @@ -0,0 +1,117 @@
>> +/*
>> + * 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>
>> +#include <linux/iio/sw_trigger.h>
>> +
>> +#define MAX_NAME_LEN 32
> Strikes me as perhaps a little short given we want to allow 'almost' arbitary names for the triggers.

We are anyhow, limited by the configfs name len here.

http://lxr.linux.no/linux+v3.19.1/include/linux/configfs.h#L47


>> +
>> +static struct config_group *trigger_make_group(struct config_group *group,
>> +                                            const char *name)
>> +{
>> +     char *type_name;
>> +     char *trigger_name;
>> +     char buf[MAX_NAME_LEN];
>> +     struct iio_sw_trigger *t;
>> +
>> +     snprintf(buf, MAX_NAME_LEN, "%s", name);
>> +
>> +     /* group name should have the form <trigger-type>-<trigger-name> */
>> +     type_name = buf;
>> +     trigger_name = strchr(buf, '-');
>> +     if (!trigger_name) {
>> +             pr_err("Unable to locate '-' in %s. Use <type>-<name>.\n", buf);
>> +             return ERR_PTR(-EINVAL);
>> +     }
>> +
>> +     /* replace - with \0, this nicely separates the two strings */
>> +     *trigger_name = '\0';
> Dirty trick, but I like it.

Me too, I borrowed it from the usb gadget code. :)

>> +     trigger_name++;
>> +
>> +     t = iio_sw_trigger_create(type_name, trigger_name);
>> +     if (IS_ERR(t))
>> +             return ERR_CAST(t);
>> +
>> +     config_item_set_name(&t->group.cg_item, 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);
>> +
>> +     if (t)
>> +             iio_sw_trigger_destroy(t);
>> +     config_item_put(item);
>> +}
>> +
>> +static struct configfs_group_operations triggers_ops = {
>> +     .make_group     = &trigger_make_group,
>> +     .drop_item      = &trigger_drop_group,
>> +};
>> +
>> +static struct config_item_type iio_triggers_group_type = {
>> +     .ct_group_ops = &triggers_ops,
>> +     .ct_owner       = THIS_MODULE,
>> +};
>> +
>> +static struct config_group iio_triggers_group = {
>> +     .cg_item = {
>> +             .ci_namebuf = "triggers",
>> +             .ci_type = &iio_triggers_group_type,
>> +     },
>> +};
>> +
>> +static struct config_group *iio_root_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_root_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");
>> +MODULE_LICENSE("GPL v2");


thanks,
Daniel.

Re: [PATCH v4 4/4] iio: Documentation: Add IIO configfs documentation

[Daniel Baluta]

On Sun, Apr 26, 2015 at 10:30 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 20/04/15 15:02, Daniel Baluta wrote:
>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> Probably need to start Documentation/ABI/testing/configfs-iio (or something like that)
> as well to document the ABI elements fully.

Yes, this is a good idea.
>
> This plain text doc is particularly useful to hopefully get feedback on the interface
> from those who might not dive into the details of the series.  Thanks!
>> ---
>>  Documentation/iio/iio_configfs.txt | 67 ++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 67 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..1aa66d1
>> --- /dev/null
>> +++ b/Documentation/iio/iio_configfs.txt
>> @@ -0,0 +1,67 @@
>> +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" groups. It is
>> +automagically accessible when the configfs is mounted and can be found
>> +under /config/iio/triggers.
>> +
>> +Software triggers are created under /config/iio/triggers directory. A sofware
>> +trigger name MUST be of the following form:
>> +     * <trigger-type>-<trigger-name>:
>> +Where:
>> +     * <trigger-type>, specifies the interrupt source (e.g: hrtimer)
>> +     * <trigger-name>, spefcifies the IIO device trigger name
>> +
>> +We support now to following interrupt sources (trigger types):
>> +     * hrtimer, uses high resolution timers as interrupt source
>> +
>> +3.1 Software triggers creation and destruction
>> +
>> +As simply as:
>> +
>> +$ mkdir /config/triggers/<trigger-type>-<trigger-name>
>> +$ rmdir /config/triggers/<trigger-type>-<trigger-name>
>> +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 has only one attribute:
>> +
>> +$ ls /config/triggers/hrtimer-instance1
>> +sampling_frequency
>> +
>> +sampling_frequency - represents the period in Hz between two consecutive
>> +iio_trigger_poll calls. By default it is set to 100Hz.
>> +
>> +4. Further work
> I wouldn't bother with this section here.
>> +
>> +* add "sysfs" trigger type
>>
>
> --
> 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 v4 1/4] iio: core: Introduce IIO software triggers

[Jonathan Cameron]

On 26/04/15 20:32, Daniel Baluta wrote:
> On Sun, Apr 26, 2015 at 10:21 PM, Jonathan Cameron <jic23@kernel.org> wrote:
>> On 20/04/15 15:02, Daniel Baluta wrote:
>>> 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.
>>>
>>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
>> Looks sensible.
>> My only real question is if the rwlock is justified (vs a mutex).
> 
> Hmm, a given iio_trigger_type list element is mostly read. Also, borrowed
> the code from file systems core implementation.
> 
> http://lxr.linux.no/linux+v3.19.1/fs/filesystems.c#L33
> 
> Anyhow, there is no problem to use a mutex.
It it's the standard option for this usecase then I don't really mind.
> 
>>
>>> ---
>>>  drivers/iio/Kconfig                   |   8 +++
>>>  drivers/iio/Makefile                  |   1 +
>>>  drivers/iio/industrialio-sw-trigger.c | 111 ++++++++++++++++++++++++++++++++++
>>>  include/linux/iio/sw_trigger.h        |  50 +++++++++++++++
>>>  4 files changed, 170 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 4011eff..de7f1d9 100644
>>> --- a/drivers/iio/Kconfig
>>> +++ b/drivers/iio/Kconfig
>>> @@ -58,6 +58,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
>>> +     bool "Enable software triggers support"
>>> +     depends on IIO_TRIGGER
>>> +     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 698afc2..df87975 100644
>>> --- a/drivers/iio/Makefile
>>> +++ b/drivers/iio/Makefile
>>> @@ -6,6 +6,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_TRIGGER) += industrialio-trigger.o
>>> +industrialio-$(CONFIG_IIO_SW_TRIGGER) += industrialio-sw-trigger.o
>>>  industrialio-$(CONFIG_IIO_BUFFER_CB) += buffer_cb.o
>>>
>>>  obj-$(CONFIG_IIO_TRIGGERED_BUFFER) += industrialio-triggered-buffer.o
>>> diff --git a/drivers/iio/industrialio-sw-trigger.c b/drivers/iio/industrialio-sw-trigger.c
>>> new file mode 100644
>>> index 0000000..567c675
>>> --- /dev/null
>>> +++ b/drivers/iio/industrialio-sw-trigger.c
>>> @@ -0,0 +1,111 @@
>>> +/*
>>> + * 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>
>>> +
>>> +static LIST_HEAD(iio_trigger_types_list);
>>> +static DEFINE_RWLOCK(iio_trigger_types_lock);
>> Can see the logic, but I'm not totally convinced a rwlock is necessary.
>> We don't actually create new ones terribly often...
> 
> I see your point. Could change the code to use a mutex.
> 
>>> +
>>> +static
>>> +struct iio_sw_trigger_type *iio_find_sw_trigger_type(char *name, unsigned len)
>>> +{
>>> +     struct iio_sw_trigger_type *t = NULL, *iter;
>>> +
>>> +     list_for_each_entry(iter, &iio_trigger_types_list, list)
>>> +             if (!strncmp(iter->name, name, len)) {
>>> +                     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;
>>> +
>>> +     write_lock(&iio_trigger_types_lock);
>>> +     iter = iio_find_sw_trigger_type(t->name, strlen(t->name));
>>> +     if (iter)
>>> +             ret = -EBUSY;
>> Rather than EAGAIN?  Could also do the magic attempt to autoload the
>> module that the usb gadget driver does (though add that as a later
>> feature rather than in this first posting I think to keep complexity
>> of patch manageable).
> 
> I see, we can do this later. Anyhow, I'm not convinced that EAGAIN is a good
> value here. We just want to inform the user that the module registering this
> trigger type is already loaded.
> 
> Like here:
> 
> http://lxr.linux.no/linux+v3.19.1/fs/filesystems.c#L78
> 
Gah. I got this backwards.  Ignore me on that one.
> 
>>> +     else
>>> +             list_add_tail(&t->list, &iio_trigger_types_list);
>>> +     write_unlock(&iio_trigger_types_lock);
>> nitpick :) blank line here.
>>> +     return ret;
>>> +}
>>> +EXPORT_SYMBOL(iio_register_sw_trigger_type);
>>> +
>>> +int iio_unregister_sw_trigger_type(struct iio_sw_trigger_type *t)
>>> +{
>>> +     struct iio_sw_trigger_type *iter;
>>> +     int ret = 0;
>>> +
>>> +     write_lock(&iio_trigger_types_lock);
>>> +     iter = iio_find_sw_trigger_type(t->name, strlen(t->name));
>>> +     if (!iter)
>>> +             ret = -EINVAL;
>>> +     else
>>> +             list_del(&t->list);
>>> +     write_unlock(&iio_trigger_types_lock);
>>> +
>>> +     return ret;
>>> +}
>>> +EXPORT_SYMBOL(iio_unregister_sw_trigger_type);
>>> +
>>> +static
>>> +struct iio_sw_trigger_type *iio_get_sw_trigger_type(char *name)
>>> +{
>>> +     struct iio_sw_trigger_type *t;
>>> +
>>> +     read_lock(&iio_trigger_types_lock);
>>> +     t = iio_find_sw_trigger_type(name, strlen(name));
>>> +     if (t && !try_module_get(t->owner))
>>> +             t = NULL;
>>> +     read_unlock(&iio_trigger_types_lock);
>>> +
>>> +     return t;
>>> +}
>>> +
>>> +struct iio_sw_trigger *iio_sw_trigger_create(char *type, char *name)
>>> +{
>>> +     struct iio_sw_trigger *t;
>>> +     struct iio_sw_trigger_type *tt;
>> I like the brief variable names (perfectly clear so not actually being
>> sarcastic ;))
>>> +
>>> +     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;
>>> +
>>> +     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);
>>> diff --git a/include/linux/iio/sw_trigger.h b/include/linux/iio/sw_trigger.h
>>> new file mode 100644
>>> index 0000000..2e6659b
>>> --- /dev/null
>>> +++ b/include/linux/iio/sw_trigger.h
>>> @@ -0,0 +1,50 @@
>>> +#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)
>>> +
>>> +struct iio_sw_trigger_ops;
>>> +
>>> +struct iio_sw_trigger_type {
>>> +     char *name;
>>> +     struct module *owner;
>>> +     struct iio_sw_trigger_ops *ops;
>>> +     struct list_head list;
>>> +};
>>> +
>>> +struct iio_sw_trigger {
>>> +     struct iio_trigger *trigger;
>>> +     struct iio_sw_trigger_type *trigger_type;
>>> +#ifdef CONFIG_CONFIGFS_FS
>>> +     struct config_group group;
>>> +#endif
>>> +};
>>> +
>>> +struct iio_sw_trigger_ops {
>>> +     struct iio_sw_trigger* (*probe)(const char *);
>>> +     int (*remove)(struct iio_sw_trigger *);
>>> +};
>>> +
>>> +int iio_register_sw_trigger_type(struct iio_sw_trigger_type *);
>>> +int iio_unregister_sw_trigger_type(struct iio_sw_trigger_type *);
>>> +
>>> +struct iio_sw_trigger *iio_sw_trigger_create(char *, char *);
>>> +void iio_sw_trigger_destroy(struct iio_sw_trigger *);
>>> +
>>> +#ifdef CONFIG_CONFIGFS_FS
>>> +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);
>>> +}
>>> +#endif
>>> +
>>> +#endif /* __IIO_SW_TRIGGER */
>>>
>>
>> --
>> 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 v4 2/4] iio: core: Introduce IIO configfs support

[Jonathan Cameron]

On 26/04/15 20:36, Daniel Baluta wrote:
> On Sun, Apr 26, 2015 at 10:26 PM, Jonathan Cameron <jic23@kernel.org> wrote:
>> On 20/04/15 15:02, Daniel Baluta wrote:
>>> This creates an IIO configfs subystem named "iio", with a default "triggers"
>>> group.
>>>
>>> Triggers group is used for handling software triggers. To create a new software
>>> trigger one must create a directory inside the trigger directory.
>>>
>>> Software trigger name MUST follow the following convention:
>>>       * <trigger-type>-<trigger-name>
>>> Where:
>>>       * <trigger_type>, specifies the interrupt source (e.g: hrtimer)
>>>       * <trigger-name>, specifies the IIO device trigger name
>>>
>>> Failing to follow this convention will result in an directory creation error.
>>>
>>> E.g, assuming that hrtimer trigger type is registered with IIO software
>>> trigger core:
>>>
>>> $ mkdir /config/iio/triggers/hrtimer-instance1
>>>
>>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
>> Looks good. Couple of little comments inline.
>>
>> Jonathan
>>> ---
>>>  drivers/iio/Kconfig                 |   8 +++
>>>  drivers/iio/Makefile                |   1 +
>>>  drivers/iio/industrialio-configfs.c | 117 ++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 126 insertions(+)
>>>  create mode 100644 drivers/iio/industrialio-configfs.c
>>>
>>> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
>>> index de7f1d9..c310156 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 triggers). 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 df87975..31aead3 100644
>>> --- a/drivers/iio/Makefile
>>> +++ b/drivers/iio/Makefile
>>> @@ -10,6 +10,7 @@ industrialio-$(CONFIG_IIO_SW_TRIGGER) += industrialio-sw-trigger.o
>>>  industrialio-$(CONFIG_IIO_BUFFER_CB) += buffer_cb.o
>>>
>>>  obj-$(CONFIG_IIO_TRIGGERED_BUFFER) += industrialio-triggered-buffer.o
>>> +obj-$(CONFIG_IIO_CONFIGFS) += industrialio-configfs.o
>>>  obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o
>>>
>>>  obj-y += accel/
>>> diff --git a/drivers/iio/industrialio-configfs.c b/drivers/iio/industrialio-configfs.c
>>> new file mode 100644
>>> index 0000000..0361434
>>> --- /dev/null
>>> +++ b/drivers/iio/industrialio-configfs.c
>>> @@ -0,0 +1,117 @@
>>> +/*
>>> + * 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>
>>> +#include <linux/iio/sw_trigger.h>
>>> +
>>> +#define MAX_NAME_LEN 32
>> Strikes me as perhaps a little short given we want to allow 'almost' arbitary names for the triggers.
> 
> We are anyhow, limited by the configfs name len here.
> 
> http://lxr.linux.no/linux+v3.19.1/include/linux/configfs.h#L47
> 
Fair enough. Missed that.
> 
>>> +
>>> +static struct config_group *trigger_make_group(struct config_group *group,
>>> +                                            const char *name)
>>> +{
>>> +     char *type_name;
>>> +     char *trigger_name;
>>> +     char buf[MAX_NAME_LEN];
>>> +     struct iio_sw_trigger *t;
>>> +
>>> +     snprintf(buf, MAX_NAME_LEN, "%s", name);
>>> +
>>> +     /* group name should have the form <trigger-type>-<trigger-name> */
>>> +     type_name = buf;
>>> +     trigger_name = strchr(buf, '-');
>>> +     if (!trigger_name) {
>>> +             pr_err("Unable to locate '-' in %s. Use <type>-<name>.\n", buf);
>>> +             return ERR_PTR(-EINVAL);
>>> +     }
>>> +
>>> +     /* replace - with \0, this nicely separates the two strings */
>>> +     *trigger_name = '\0';
>> Dirty trick, but I like it.
> 
> Me too, I borrowed it from the usb gadget code. :)
> 
>>> +     trigger_name++;
>>> +
>>> +     t = iio_sw_trigger_create(type_name, trigger_name);
>>> +     if (IS_ERR(t))
>>> +             return ERR_CAST(t);
>>> +
>>> +     config_item_set_name(&t->group.cg_item, 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);
>>> +
>>> +     if (t)
>>> +             iio_sw_trigger_destroy(t);
>>> +     config_item_put(item);
>>> +}
>>> +
>>> +static struct configfs_group_operations triggers_ops = {
>>> +     .make_group     = &trigger_make_group,
>>> +     .drop_item      = &trigger_drop_group,
>>> +};
>>> +
>>> +static struct config_item_type iio_triggers_group_type = {
>>> +     .ct_group_ops = &triggers_ops,
>>> +     .ct_owner       = THIS_MODULE,
>>> +};
>>> +
>>> +static struct config_group iio_triggers_group = {
>>> +     .cg_item = {
>>> +             .ci_namebuf = "triggers",
>>> +             .ci_type = &iio_triggers_group_type,
>>> +     },
>>> +};
>>> +
>>> +static struct config_group *iio_root_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_root_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");
>>> +MODULE_LICENSE("GPL v2");
> 
> 
> thanks,
> Daniel.
> 

Re: [PATCH v4 0/4] Add initial configfs support for IIO

[Jonathan Cameron]

On 26/04/15 20:35, Jonathan Cameron wrote:
> On 20/04/15 15:02, Daniel Baluta wrote:
>> This patchset introduces IIO software triggers, offers a way of configuring
>> them via configfs and adds the IIO hrtimer based interrupt source to be used
>> with software triggers.
>>
>> The arhitecture is now split in 3 parts, to remove all IIO trigger specific
>> parts from IIO configfs core:
>>
>> (1) IIO software triggers - are independent of configfs.
>> (2) IIO configfs - offers a generic way of creating IIO objects. So far we can
>> 	create software triggers.
>> (3) IIO hrtimer trigger - is the first interrupt source for software triggers
>> 	(with syfs to follow). Each trigger type can implement its own set of
>> 	attributes.
>>
>> Changes since v3:
>> 	* addressed comments from Jonathan for previous version
>> 	* https://lkml.org/lkml/2015/4/6/111
> 
> Hi Daniel.
> 
> Thanks for all your hard work on this.  I'm very pleased with the result.
> It's clean, remarkably compact and nice and extensible.
> 
> The only reason I didn't apply it today (other than the odd nit) was because
> it's major new ABI for us so I'd ideally like a few of IIOs main reviewers
> to take a look before we take it.
> 
> Lars, Harmut, Peter, others (our reviewer set is growing very fast!) if you
> guys have time and interest, please take a quick look at this and see if we've
> missed anything.  

Anyone intending to look at this?  I'm inclined to take it as is, but know that
Lars for instance had a particular interest in this support (it was his suggestion
in the first place I think!) so if you want more time to have a look, then
let me know. 

I'm travelling for the next couple of days, so should have plenty of time in airports
to get some reviewing done.
> 
> It's actually remarkably straightforward in the end due to all of Daniel's hard
> work so shouldn't take you long unless you want to dig right down into the configfs
> side of things.  If you do, then the best reference I found (Daniel may have others)
> is the usb-gadget driver.
> 
> Jonathan
>>  
>> Daniel Baluta (4):
>>   iio: core: Introduce IIO software triggers
>>   iio: core: Introduce IIO configfs support
>>   iio: trigger: Introduce IIO hrtimer based trigger
>>   iio: Documentation: Add IIO configfs documentation
>>
>>  Documentation/iio/iio_configfs.txt     |  67 +++++++++++
>>  drivers/iio/Kconfig                    |  16 +++
>>  drivers/iio/Makefile                   |   2 +
>>  drivers/iio/industrialio-configfs.c    | 117 +++++++++++++++++++
>>  drivers/iio/industrialio-sw-trigger.c  | 111 ++++++++++++++++++
>>  drivers/iio/trigger/Kconfig            |   9 ++
>>  drivers/iio/trigger/Makefile           |   2 +
>>  drivers/iio/trigger/iio-trig-hrtimer.c | 201 +++++++++++++++++++++++++++++++++
>>  include/linux/iio/sw_trigger.h         |  50 ++++++++
>>  9 files changed, 575 insertions(+)
>>  create mode 100644 Documentation/iio/iio_configfs.txt
>>  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
>>
> 
> --
> 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 v4 0/4] Add initial configfs support for IIO

[Lars-Peter Clausen]

On 05/03/2015 09:20 PM, Jonathan Cameron wrote:
> On 26/04/15 20:35, Jonathan Cameron wrote:
>> On 20/04/15 15:02, Daniel Baluta wrote:
>>> This patchset introduces IIO software triggers, offers a way of configuring
>>> them via configfs and adds the IIO hrtimer based interrupt source to be used
>>> with software triggers.
>>>
>>> The arhitecture is now split in 3 parts, to remove all IIO trigger specific
>>> parts from IIO configfs core:
>>>
>>> (1) IIO software triggers - are independent of configfs.
>>> (2) IIO configfs - offers a generic way of creating IIO objects. So far we can
>>> 	create software triggers.
>>> (3) IIO hrtimer trigger - is the first interrupt source for software triggers
>>> 	(with syfs to follow). Each trigger type can implement its own set of
>>> 	attributes.
>>>
>>> Changes since v3:
>>> 	* addressed comments from Jonathan for previous version
>>> 	* https://lkml.org/lkml/2015/4/6/111
>>
>> Hi Daniel.
>>
>> Thanks for all your hard work on this.  I'm very pleased with the result.
>> It's clean, remarkably compact and nice and extensible.
>>
>> The only reason I didn't apply it today (other than the odd nit) was because
>> it's major new ABI for us so I'd ideally like a few of IIOs main reviewers
>> to take a look before we take it.
>>
>> Lars, Harmut, Peter, others (our reviewer set is growing very fast!) if you
>> guys have time and interest, please take a quick look at this and see if we've
>> missed anything.
>
> Anyone intending to look at this?  I'm inclined to take it as is, but know that
> Lars for instance had a particular interest in this support (it was his suggestion
> in the first place I think!) so if you want more time to have a look, then
> let me know.

Sorry, I know, I should have long taken a look at it. I'll try to do it 
tomorrow, if you don't hear anything else from me assume I'm ok with it.

Re: [PATCH v4 0/4] Add initial configfs support for IIO

[Daniel Baluta]

On 05/03/2015 10:22 PM, Lars-Peter Clausen wrote:
> On 05/03/2015 09:20 PM, Jonathan Cameron wrote:
>> On 26/04/15 20:35, Jonathan Cameron wrote:
>>> On 20/04/15 15:02, Daniel Baluta wrote:
>>>> This patchset introduces IIO software triggers, offers a way of
>>>> configuring
>>>> them via configfs and adds the IIO hrtimer based interrupt source to
>>>> be used
>>>> with software triggers.
>>>>
>>>> The arhitecture is now split in 3 parts, to remove all IIO trigger
>>>> specific
>>>> parts from IIO configfs core:
>>>>
>>>> (1) IIO software triggers - are independent of configfs.
>>>> (2) IIO configfs - offers a generic way of creating IIO objects. So
>>>> far we can
>>>>     create software triggers.
>>>> (3) IIO hrtimer trigger - is the first interrupt source for software
>>>> triggers
>>>>     (with syfs to follow). Each trigger type can implement its own
>>>> set of
>>>>     attributes.
>>>>
>>>> Changes since v3:
>>>>     * addressed comments from Jonathan for previous version
>>>>     * https://lkml.org/lkml/2015/4/6/111
>>>
>>> Hi Daniel.
>>>
>>> Thanks for all your hard work on this.  I'm very pleased with the
>>> result.
>>> It's clean, remarkably compact and nice and extensible.
>>>
>>> The only reason I didn't apply it today (other than the odd nit) was
>>> because
>>> it's major new ABI for us so I'd ideally like a few of IIOs main
>>> reviewers
>>> to take a look before we take it.
>>>
>>> Lars, Harmut, Peter, others (our reviewer set is growing very fast!)
>>> if you
>>> guys have time and interest, please take a quick look at this and see
>>> if we've
>>> missed anything.
>>
>> Anyone intending to look at this?  I'm inclined to take it as is, but
>> know that
>> Lars for instance had a particular interest in this support (it was
>> his suggestion
>> in the first place I think!) so if you want more time to have a look,
>> then
>> let me know.
>
> Sorry, I know, I should have long taken a look at it. I'll try to do it
> tomorrow, if you don't hear anything else from me assume I'm ok with it.

I've sent v5 with fixes after Jonathan's last review. Lars please use
this for review :)

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

thanks,
Daniel.