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

[PATCH v6 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 v5: (after Lars comments)
	* the most important change is that we moved sampling_frequency attribute
	from configfs to trigger's directory in /sys.
	* couple of const added to strings
	* documentation to public API in sw_trigger.h
	* replace pr_err with WARN_ONCE in trigger_make_group to avoid spamming
	kernel log, but without leaving user clueless in case of errors.
	* we still need to decide if we get a real gain by adding min/max limits
	for sampling frequency in /config dir. Anyhow, this can be done in a later
	patch.
	* fix race in hrtimer_remove

Changes since v4:
	* patch 1/4
		- fixed "new line" nit in industrialio-sw-trigger.c
		- added license header in sw_trigger.h\x02o
	* patch 2/4
		- none
	* patch 3/4 
		- none
	* patch 4/4
		- removed "Further work" chapter in iio_configfs.txt
		- added configfs-iio file in Documentation/ABI/testing

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/ABI/testing/configfs-iio |  20 ++++
 Documentation/iio/iio_configfs.txt     |  58 ++++++++++
 drivers/iio/Kconfig                    |  16 +++
 drivers/iio/Makefile                   |   2 +
 drivers/iio/industrialio-configfs.c    | 117 ++++++++++++++++++++
 drivers/iio/industrialio-sw-trigger.c  | 115 +++++++++++++++++++
 drivers/iio/trigger/Kconfig            |  10 ++
 drivers/iio/trigger/Makefile           |   2 +
 drivers/iio/trigger/iio-trig-hrtimer.c | 194 +++++++++++++++++++++++++++++++++
 include/linux/iio/sw_trigger.h         |  85 +++++++++++++++
 10 files changed, 619 insertions(+)
 create mode 100644 Documentation/ABI/testing/configfs-iio
 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 v6 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 | 115 ++++++++++++++++++++++++++++++++++
 include/linux/iio/sw_trigger.h        |  58 +++++++++++++++++
 4 files changed, 182 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..fa1ec51
--- /dev/null
+++ b/drivers/iio/industrialio-sw-trigger.c
@@ -0,0 +1,115 @@
+/*
+ * 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(const 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(const 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(const char *type, const char *name)
+{
+	struct iio_sw_trigger *t;
+	struct iio_sw_trigger_type *tt;
+
+	tt = iio_get_sw_trigger_type(type);
+	if (!tt) {
+		pr_err("Invalid trigger type: %s\n", type);
+		return ERR_PTR(-EINVAL);
+	}
+	t = tt->ops->probe(name);
+	if (IS_ERR(t))
+		goto out_module_put;
+
+	t->trigger_type = tt;
+
+	return t;
+out_module_put:
+	module_put(tt->owner);
+	return t;
+}
+EXPORT_SYMBOL(iio_sw_trigger_create);
+
+void iio_sw_trigger_destroy(struct iio_sw_trigger *t)
+{
+	struct iio_sw_trigger_type *tt = t->trigger_type;
+
+	tt->ops->remove(t);
+	module_put(tt->owner);
+}
+EXPORT_SYMBOL(iio_sw_trigger_destroy);
diff --git a/include/linux/iio/sw_trigger.h b/include/linux/iio/sw_trigger.h
new file mode 100644
index 0000000..2771a25
--- /dev/null
+++ b/include/linux/iio/sw_trigger.h
@@ -0,0 +1,58 @@
+/*
+ * Industrial I/O software trigger interface
+ *
+ * Copyright (c) 2015 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#ifndef __IIO_SW_TRIGGER
+#define __IIO_SW_TRIGGER
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/iio/iio.h>
+
+#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 {
+	const char *name;
+	struct module *owner;
+	const 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;
+};
+
+struct iio_sw_trigger_ops {
+	struct iio_sw_trigger* (*probe)(const char *);
+	int (*remove)(struct iio_sw_trigger *);
+};
+
+/**
+ * iio_register_sw_trigger_type() - register a software trigger type
+ *
+ * @tt: software trigger type to be registered
+ */
+int iio_register_sw_trigger_type(struct iio_sw_trigger_type *tt);
+
+/**
+ * iio_unregister_sw_trigger_type() - unregister a software trigger type
+ *
+ * @tt: software trigger type to be unregistered
+ */
+int iio_unregister_sw_trigger_type(struct iio_sw_trigger_type *tt);
+
+struct iio_sw_trigger *iio_sw_trigger_create(const char *, const char *);
+void iio_sw_trigger_destroy(struct iio_sw_trigger *);
+
+#endif /* __IIO_SW_TRIGGER */
-- 
1.9.1

[PATCH v6 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 ++++++++++++++++++++++++++++++++++++
 include/linux/iio/sw_trigger.h      |  27 +++++++++
 4 files changed, 153 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..daf318c
--- /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) {
+		WARN_ONCE(1, "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);
+
+	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");
diff --git a/include/linux/iio/sw_trigger.h b/include/linux/iio/sw_trigger.h
index 2771a25..87803c7 100644
--- a/include/linux/iio/sw_trigger.h
+++ b/include/linux/iio/sw_trigger.h
@@ -14,6 +14,7 @@
 #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, \
@@ -31,6 +32,9 @@ struct iio_sw_trigger_type {
 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 {
@@ -55,4 +59,27 @@ int iio_unregister_sw_trigger_type(struct iio_sw_trigger_type *tt);
 struct iio_sw_trigger *iio_sw_trigger_create(const char *, const char *);
 void iio_sw_trigger_destroy(struct iio_sw_trigger *);
 
+#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);
+}
+
+static inline
+void iio_config_group_init_type_name(struct config_group *group,
+				     const char *name,
+				     struct config_item_type *type)
+{
+	config_group_init_type_name(group, name, type);
+}
+#else
+static inline
+void iio_config_group_init_type_name(struct config_group *group,
+				     const char *name,
+				     struct config_item_type *type)
+{ }
+#endif
+
 #endif /* __IIO_SW_TRIGGER */
-- 
1.9.1

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

[Daniel Baluta]

Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
 Documentation/ABI/testing/configfs-iio | 20 ++++++++++++
 Documentation/iio/iio_configfs.txt     | 58 ++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+)
 create mode 100644 Documentation/ABI/testing/configfs-iio
 create mode 100644 Documentation/iio/iio_configfs.txt

diff --git a/Documentation/ABI/testing/configfs-iio b/Documentation/ABI/testing/configfs-iio
new file mode 100644
index 0000000..28ad893
--- /dev/null
+++ b/Documentation/ABI/testing/configfs-iio
@@ -0,0 +1,20 @@
+What:		/config/iio
+Date:		May 2015
+KernelVersion:	4.2
+Contact:	linux-iio@vger.kernel.org
+Description:
+		This represents Industrial IO configuration entry point
+		directory. It contains sub-groups corresponding to IIO
+		objects.
+
+What:		/config/iio/triggers
+Date:		May 2015
+KernelVersion:	4.2
+Description:
+		Industrial IO software triggers directory.
+
+What:		/config/iio/triggers/hrtimer-name
+Date:		May 2015
+KernelVersion:	4.2
+Description:
+		High resolution timer instance.
diff --git a/Documentation/iio/iio_configfs.txt b/Documentation/iio/iio_configfs.txt
new file mode 100644
index 0000000..3c8014a
--- /dev/null
+++ b/Documentation/iio/iio_configfs.txt
@@ -0,0 +1,58 @@
+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 doesn't have any configurable attribute from /config dir.
+It does introduce the sampling_frequency attribute to trigger directory.
-- 
1.9.1

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

[Jonathan Cameron]

On 08/05/15 09:33, 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.
I'm happy with the whole series, but given Lars' involvement would like to
give him a bit of time for another look / reviewed-by / acked-by or similar.

Obviously comments from anyone else welcome as well.

Now all we need to do is all the other uses for configfs that we've
discussed in earlier threads :) Easy really ;)
> 
> Changes since v5: (after Lars comments)
> 	* the most important change is that we moved sampling_frequency attribute
> 	from configfs to trigger's directory in /sys.
> 	* couple of const added to strings
> 	* documentation to public API in sw_trigger.h
> 	* replace pr_err with WARN_ONCE in trigger_make_group to avoid spamming
> 	kernel log, but without leaving user clueless in case of errors.
> 	* we still need to decide if we get a real gain by adding min/max limits
> 	for sampling frequency in /config dir. Anyhow, this can be done in a later
> 	patch.
> 	* fix race in hrtimer_remove
> 
> Changes since v4:
> 	* patch 1/4
> 		- fixed "new line" nit in industrialio-sw-trigger.c
> 		- added license header in sw_trigger.h\x02o
> 	* patch 2/4
> 		- none
> 	* patch 3/4 
> 		- none
> 	* patch 4/4
> 		- removed "Further work" chapter in iio_configfs.txt
> 		- added configfs-iio file in Documentation/ABI/testing
> 
> 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/ABI/testing/configfs-iio |  20 ++++
>  Documentation/iio/iio_configfs.txt     |  58 ++++++++++
>  drivers/iio/Kconfig                    |  16 +++
>  drivers/iio/Makefile                   |   2 +
>  drivers/iio/industrialio-configfs.c    | 117 ++++++++++++++++++++
>  drivers/iio/industrialio-sw-trigger.c  | 115 +++++++++++++++++++
>  drivers/iio/trigger/Kconfig            |  10 ++
>  drivers/iio/trigger/Makefile           |   2 +
>  drivers/iio/trigger/iio-trig-hrtimer.c | 194 +++++++++++++++++++++++++++++++++
>  include/linux/iio/sw_trigger.h         |  85 +++++++++++++++
>  10 files changed, 619 insertions(+)
>  create mode 100644 Documentation/ABI/testing/configfs-iio
>  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 v6 0/4] Add initial configfs support for IIO

[Daniel Baluta]

On Fri, May 8, 2015 at 9:41 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 08/05/15 09:33, 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.
> I'm happy with the whole series, but given Lars' involvement would like to
> give him a bit of time for another look / reviewed-by / acked-by or similar.
>
> Obviously comments from anyone else welcome as well.
>
> Now all we need to do is all the other uses for configfs that we've
> discussed in earlier threads :) Easy really ;)

Agree. :)

The important thing now is to get Acked-by/Reviewed-by on the interface because
once accepted this cannot be changed.

For the rest of the code we can send patches later.

>>
>> Changes since v5: (after Lars comments)
>>       * the most important change is that we moved sampling_frequency attribute
>>       from configfs to trigger's directory in /sys.
>>       * couple of const added to strings
>>       * documentation to public API in sw_trigger.h
>>       * replace pr_err with WARN_ONCE in trigger_make_group to avoid spamming
>>       kernel log, but without leaving user clueless in case of errors.
>>       * we still need to decide if we get a real gain by adding min/max limits
>>       for sampling frequency in /config dir. Anyhow, this can be done in a later
>>       patch.
>>       * fix race in hrtimer_remove
>>
>> Changes since v4:
>>       * patch 1/4
>>               - fixed "new line" nit in industrialio-sw-trigger.c
>>               - added license header in sw_trigger.h o
>>       * patch 2/4
>>               - none
>>       * patch 3/4
>>               - none
>>       * patch 4/4
>>               - removed "Further work" chapter in iio_configfs.txt
>>               - added configfs-iio file in Documentation/ABI/testing
>>
>> 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/ABI/testing/configfs-iio |  20 ++++
>>  Documentation/iio/iio_configfs.txt     |  58 ++++++++++
>>  drivers/iio/Kconfig                    |  16 +++
>>  drivers/iio/Makefile                   |   2 +
>>  drivers/iio/industrialio-configfs.c    | 117 ++++++++++++++++++++
>>  drivers/iio/industrialio-sw-trigger.c  | 115 +++++++++++++++++++
>>  drivers/iio/trigger/Kconfig            |  10 ++
>>  drivers/iio/trigger/Makefile           |   2 +
>>  drivers/iio/trigger/iio-trig-hrtimer.c | 194 +++++++++++++++++++++++++++++++++
>>  include/linux/iio/sw_trigger.h         |  85 +++++++++++++++
>>  10 files changed, 619 insertions(+)
>>  create mode 100644 Documentation/ABI/testing/configfs-iio
>>  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 v6 0/4] Add initial configfs support for IIO

[Lars-Peter Clausen]

Hi,

I'd like to go back to the issue of having one folder per trigger type and
create triggers for a type in their respective folder.

I'm not convinced that it is an intentional design restriction of configfs
that we can't do this, but rather a case of it not being implemented
because nobody needed it so far.

See the proof-of-concept patch below. With this we get one sub-folder per
trigger type in /config/iio/triggers. It's not complete and doesn't do
proper locking yet and also unregister is not yet implemented but it shows
the concept how it could be done.

I'm not saying that we definitely should go down that road, but it feels
cleaner to me. And another advantage is that it is discoverable which types
of triggers are available. Thoughts? 

- Lars

---
 drivers/iio/industrialio-configfs.c   | 43 ++++++++++++++++++-----------------
 drivers/iio/industrialio-sw-trigger.c | 12 ++++++----
 fs/configfs/dir.c                     | 28 +++++++++++++++++++++--
 include/linux/configfs.h              |  4 ++++
 include/linux/iio/sw_trigger.h        | 19 ++++++++++++++++
 5 files changed, 79 insertions(+), 27 deletions(-)

diff --git a/drivers/iio/industrialio-configfs.c b/drivers/iio/industrialio-configfs.c
index daf318c..c1fa14f 100644
--- a/drivers/iio/industrialio-configfs.c
+++ b/drivers/iio/industrialio-configfs.c
@@ -22,27 +22,9 @@
 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) {
-		WARN_ONCE(1, "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);
+	t = iio_sw_trigger_create(group->cg_item.ci_name, name);
 	if (IS_ERR(t))
 		return ERR_CAST(t);
 
@@ -60,13 +42,17 @@ static void trigger_drop_group(struct config_group *group,
 	config_item_put(item);
 }
 
-static struct configfs_group_operations triggers_ops = {
+static struct configfs_group_operations trigger_ops = {
 	.make_group	= &trigger_make_group,
 	.drop_item	= &trigger_drop_group,
 };
 
+static struct config_item_type iio_trigger_group_type = {
+	.ct_group_ops = &trigger_ops,
+	.ct_owner       = THIS_MODULE,
+};
+
 static struct config_item_type iio_triggers_group_type = {
-	.ct_group_ops = &triggers_ops,
 	.ct_owner       = THIS_MODULE,
 };
 
@@ -97,6 +83,21 @@ static struct configfs_subsystem iio_configfs_subsys = {
 	.su_mutex = __MUTEX_INITIALIZER(iio_configfs_subsys.su_mutex),
 };
 
+int iio_sw_trigger_type_configs_register(struct iio_sw_trigger_type *tt)
+{
+	config_group_init_type_name(&tt->group, tt->name,
+		&iio_trigger_group_type);
+
+	return configfs_register_group(&iio_triggers_group, &tt->group);
+}
+EXPORT_SYMBOL_GPL(iio_sw_trigger_type_configs_register);
+
+void iio_sw_trigger_type_configs_unregister(struct iio_sw_trigger_type *tt)
+{
+	configfs_unregister_group(&tt->group);
+}
+EXPORT_SYMBOL_GPL(iio_sw_trigger_type_configs_unregister);
+
 static int __init iio_configfs_init(void)
 {
 	config_group_init(&iio_triggers_group);
diff --git a/drivers/iio/industrialio-sw-trigger.c b/drivers/iio/industrialio-sw-trigger.c
index fa1ec51..312c33c 100644
--- a/drivers/iio/industrialio-sw-trigger.c
+++ b/drivers/iio/industrialio-sw-trigger.c
@@ -41,10 +41,12 @@ int iio_register_sw_trigger_type(struct iio_sw_trigger_type *t)
 
 	write_lock(&iio_trigger_types_lock);
 	iter = __iio_find_sw_trigger_type(t->name, strlen(t->name));
-	if (iter)
+	if (iter) {
 		ret = -EBUSY;
-	else
+	} else {
 		list_add_tail(&t->list, &iio_trigger_types_list);
+		iio_sw_trigger_type_configs_register(t);
+	}
 	write_unlock(&iio_trigger_types_lock);
 
 	return ret;
@@ -58,10 +60,12 @@ int iio_unregister_sw_trigger_type(struct iio_sw_trigger_type *t)
 
 	write_lock(&iio_trigger_types_lock);
 	iter = __iio_find_sw_trigger_type(t->name, strlen(t->name));
-	if (!iter)
+	if (!iter) {
 		ret = -EINVAL;
-	else
+	} else {
+		iio_sw_trigger_type_configs_unregister(t);
 		list_del(&t->list);
+	}
 	write_unlock(&iio_trigger_types_lock);
 
 	return ret;
diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index c9c298b..0e2e0e6 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -659,7 +659,7 @@ static void detach_groups(struct config_group *group)
  * try using vfs_mkdir.  Just a thought.
  */
 static int create_default_group(struct config_group *parent_group,
-				struct config_group *group)
+				struct config_group *group, struct dentry **dentry)
 {
 	int ret;
 	struct configfs_dirent *sd;
@@ -679,6 +679,8 @@ static int create_default_group(struct config_group *parent_group,
 		if (!ret) {
 			sd = child->d_fsdata;
 			sd->s_type |= CONFIGFS_USET_DEFAULT;
+			if (dentry)
+				*dentry = child;
 		} else {
 			BUG_ON(child->d_inode);
 			d_drop(child);
@@ -699,7 +701,7 @@ static int populate_groups(struct config_group *group)
 		for (i = 0; group->default_groups[i]; i++) {
 			new_group = group->default_groups[i];
 
-			ret = create_default_group(group, new_group);
+			ret = create_default_group(group, new_group, NULL);
 			if (ret) {
 				detach_groups(group);
 				break;
@@ -1644,6 +1646,28 @@ const struct file_operations configfs_dir_operations = {
 	.iterate	= configfs_readdir,
 };
 
+int configfs_register_group(struct config_group *parent_group,
+	struct config_group *group)
+{
+	struct dentry *dentry;
+	int ret;
+
+	link_group(parent_group, group);
+
+	ret = create_default_group(parent_group, group, &dentry);
+	if (ret == 0)
+		configfs_dir_set_ready(dentry->d_fsdata);
+
+	return ret;
+}
+EXPORT_SYMBOL(configfs_register_group);
+
+void configfs_unregister_group(struct config_group *group)
+{
+	/* TODO */
+}
+EXPORT_SYMBOL(configfs_unregister_group);
+
 int configfs_register_subsystem(struct configfs_subsystem *subsys)
 {
 	int err;
diff --git a/include/linux/configfs.h b/include/linux/configfs.h
index 34025df..a641bea 100644
--- a/include/linux/configfs.h
+++ b/include/linux/configfs.h
@@ -252,6 +252,10 @@ static inline struct configfs_subsystem *to_configfs_subsystem(struct config_gro
 int configfs_register_subsystem(struct configfs_subsystem *subsys);
 void configfs_unregister_subsystem(struct configfs_subsystem *subsys);
 
+int configfs_register_group(struct config_group *parent_group,
+	struct config_group *group);
+void configfs_unregister_group(struct config_group *group);
+
 /* These functions can sleep and can alloc with GFP_KERNEL */
 /* WARNING: These cannot be called underneath configfs callbacks!! */
 int configfs_depend_item(struct configfs_subsystem *subsys, struct config_item *target);
diff --git a/include/linux/iio/sw_trigger.h b/include/linux/iio/sw_trigger.h
index 87803c7..5551a11 100644
--- a/include/linux/iio/sw_trigger.h
+++ b/include/linux/iio/sw_trigger.h
@@ -27,6 +27,10 @@ struct iio_sw_trigger_type {
 	struct module *owner;
 	const struct iio_sw_trigger_ops *ops;
 	struct list_head list;
+#ifdef CONFIG_CONFIGFS_FS
+	struct config_group group;
+#endif
+
 };
 
 struct iio_sw_trigger {
@@ -60,6 +64,10 @@ struct iio_sw_trigger *iio_sw_trigger_create(const char *, const char *);
 void iio_sw_trigger_destroy(struct iio_sw_trigger *);
 
 #ifdef CONFIG_CONFIGFS_FS
+
+int iio_sw_trigger_type_configs_register(struct iio_sw_trigger_type *tt);
+void iio_sw_trigger_type_configs_unregister(struct iio_sw_trigger_type *tt);
+
 static inline
 struct iio_sw_trigger *to_iio_sw_trigger(struct config_item *item)
 {
@@ -75,6 +83,17 @@ void iio_config_group_init_type_name(struct config_group *group,
 	config_group_init_type_name(group, name, type);
 }
 #else
+static inline int iio_sw_trigger_type_configs_register(
+	struct iio_sw_trigger_type *tt)
+{
+	return 0;
+}
+
+static inline void iio_sw_trigger_type_configs_unregister(
+	struct iio_sw_trigger_type *tt)
+{
+}
+
 static inline
 void iio_config_group_init_type_name(struct config_group *group,
 				     const char *name,
-- 
1.8.0

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

[Jonathan Cameron]

On 13/05/15 12:14, Lars-Peter Clausen wrote:
> Hi,
> 
> I'd like to go back to the issue of having one folder per trigger type and
> create triggers for a type in their respective folder.
> 
> I'm not convinced that it is an intentional design restriction of configfs
> that we can't do this, but rather a case of it not being implemented
> because nobody needed it so far.
> 
> See the proof-of-concept patch below. With this we get one sub-folder per
> trigger type in /config/iio/triggers. It's not complete and doesn't do
> proper locking yet and also unregister is not yet implemented but it shows
> the concept how it could be done.
> 
> I'm not saying that we definitely should go down that road, but it feels
> cleaner to me. And another advantage is that it is discoverable which types
> of triggers are available. Thoughts? 
Nice iff it doesn't break from the intent of configfs.  I know both Daniel and
I looked for how to do this before taking the non directory based approach that
the usbgadget driver took.  

What you loose is the possibility for auto probing modules based on trigger naming
(equivalent of what the usbgadget driver does).  I undecided on whether we would
want to do that anyway.

Joel, it's your filesystem.  Is doing this acceptable to you?
> 
> - Lars
> 
> ---
>  drivers/iio/industrialio-configfs.c   | 43 ++++++++++++++++++-----------------
>  drivers/iio/industrialio-sw-trigger.c | 12 ++++++----
>  fs/configfs/dir.c                     | 28 +++++++++++++++++++++--
>  include/linux/configfs.h              |  4 ++++
>  include/linux/iio/sw_trigger.h        | 19 ++++++++++++++++
>  5 files changed, 79 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-configfs.c b/drivers/iio/industrialio-configfs.c
> index daf318c..c1fa14f 100644
> --- a/drivers/iio/industrialio-configfs.c
> +++ b/drivers/iio/industrialio-configfs.c
> @@ -22,27 +22,9 @@
>  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) {
> -		WARN_ONCE(1, "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);
> +	t = iio_sw_trigger_create(group->cg_item.ci_name, name);
>  	if (IS_ERR(t))
>  		return ERR_CAST(t);
>  
> @@ -60,13 +42,17 @@ static void trigger_drop_group(struct config_group *group,
>  	config_item_put(item);
>  }
>  
> -static struct configfs_group_operations triggers_ops = {
> +static struct configfs_group_operations trigger_ops = {
>  	.make_group	= &trigger_make_group,
>  	.drop_item	= &trigger_drop_group,
>  };
>  
> +static struct config_item_type iio_trigger_group_type = {
> +	.ct_group_ops = &trigger_ops,
> +	.ct_owner       = THIS_MODULE,
> +};
> +
>  static struct config_item_type iio_triggers_group_type = {
> -	.ct_group_ops = &triggers_ops,
>  	.ct_owner       = THIS_MODULE,
>  };
>  
> @@ -97,6 +83,21 @@ static struct configfs_subsystem iio_configfs_subsys = {
>  	.su_mutex = __MUTEX_INITIALIZER(iio_configfs_subsys.su_mutex),
>  };
>  
> +int iio_sw_trigger_type_configs_register(struct iio_sw_trigger_type *tt)
> +{
> +	config_group_init_type_name(&tt->group, tt->name,
> +		&iio_trigger_group_type);
> +
> +	return configfs_register_group(&iio_triggers_group, &tt->group);
> +}
> +EXPORT_SYMBOL_GPL(iio_sw_trigger_type_configs_register);
> +
> +void iio_sw_trigger_type_configs_unregister(struct iio_sw_trigger_type *tt)
> +{
> +	configfs_unregister_group(&tt->group);
> +}
> +EXPORT_SYMBOL_GPL(iio_sw_trigger_type_configs_unregister);
> +
>  static int __init iio_configfs_init(void)
>  {
>  	config_group_init(&iio_triggers_group);
> diff --git a/drivers/iio/industrialio-sw-trigger.c b/drivers/iio/industrialio-sw-trigger.c
> index fa1ec51..312c33c 100644
> --- a/drivers/iio/industrialio-sw-trigger.c
> +++ b/drivers/iio/industrialio-sw-trigger.c
> @@ -41,10 +41,12 @@ int iio_register_sw_trigger_type(struct iio_sw_trigger_type *t)
>  
>  	write_lock(&iio_trigger_types_lock);
>  	iter = __iio_find_sw_trigger_type(t->name, strlen(t->name));
> -	if (iter)
> +	if (iter) {
>  		ret = -EBUSY;
> -	else
> +	} else {
>  		list_add_tail(&t->list, &iio_trigger_types_list);
> +		iio_sw_trigger_type_configs_register(t);
> +	}
>  	write_unlock(&iio_trigger_types_lock);
>  
>  	return ret;
> @@ -58,10 +60,12 @@ int iio_unregister_sw_trigger_type(struct iio_sw_trigger_type *t)
>  
>  	write_lock(&iio_trigger_types_lock);
>  	iter = __iio_find_sw_trigger_type(t->name, strlen(t->name));
> -	if (!iter)
> +	if (!iter) {
>  		ret = -EINVAL;
> -	else
> +	} else {
> +		iio_sw_trigger_type_configs_unregister(t);
>  		list_del(&t->list);
> +	}
>  	write_unlock(&iio_trigger_types_lock);
>  
>  	return ret;
> diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
> index c9c298b..0e2e0e6 100644
> --- a/fs/configfs/dir.c
> +++ b/fs/configfs/dir.c
> @@ -659,7 +659,7 @@ static void detach_groups(struct config_group *group)
>   * try using vfs_mkdir.  Just a thought.
>   */
>  static int create_default_group(struct config_group *parent_group,
> -				struct config_group *group)
> +				struct config_group *group, struct dentry **dentry)
>  {
>  	int ret;
>  	struct configfs_dirent *sd;
> @@ -679,6 +679,8 @@ static int create_default_group(struct config_group *parent_group,
>  		if (!ret) {
>  			sd = child->d_fsdata;
>  			sd->s_type |= CONFIGFS_USET_DEFAULT;
> +			if (dentry)
> +				*dentry = child;
>  		} else {
>  			BUG_ON(child->d_inode);
>  			d_drop(child);
> @@ -699,7 +701,7 @@ static int populate_groups(struct config_group *group)
>  		for (i = 0; group->default_groups[i]; i++) {
>  			new_group = group->default_groups[i];
>  
> -			ret = create_default_group(group, new_group);
> +			ret = create_default_group(group, new_group, NULL);
>  			if (ret) {
>  				detach_groups(group);
>  				break;
> @@ -1644,6 +1646,28 @@ const struct file_operations configfs_dir_operations = {
>  	.iterate	= configfs_readdir,
>  };
>  
> +int configfs_register_group(struct config_group *parent_group,
> +	struct config_group *group)
> +{
> +	struct dentry *dentry;
> +	int ret;
> +
> +	link_group(parent_group, group);
> +
> +	ret = create_default_group(parent_group, group, &dentry);
> +	if (ret == 0)
> +		configfs_dir_set_ready(dentry->d_fsdata);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(configfs_register_group);
> +
> +void configfs_unregister_group(struct config_group *group)
> +{
> +	/* TODO */
> +}
> +EXPORT_SYMBOL(configfs_unregister_group);
> +
>  int configfs_register_subsystem(struct configfs_subsystem *subsys)
>  {
>  	int err;
> diff --git a/include/linux/configfs.h b/include/linux/configfs.h
> index 34025df..a641bea 100644
> --- a/include/linux/configfs.h
> +++ b/include/linux/configfs.h
> @@ -252,6 +252,10 @@ static inline struct configfs_subsystem *to_configfs_subsystem(struct config_gro
>  int configfs_register_subsystem(struct configfs_subsystem *subsys);
>  void configfs_unregister_subsystem(struct configfs_subsystem *subsys);
>  
> +int configfs_register_group(struct config_group *parent_group,
> +	struct config_group *group);
> +void configfs_unregister_group(struct config_group *group);
> +
>  /* These functions can sleep and can alloc with GFP_KERNEL */
>  /* WARNING: These cannot be called underneath configfs callbacks!! */
>  int configfs_depend_item(struct configfs_subsystem *subsys, struct config_item *target);
> diff --git a/include/linux/iio/sw_trigger.h b/include/linux/iio/sw_trigger.h
> index 87803c7..5551a11 100644
> --- a/include/linux/iio/sw_trigger.h
> +++ b/include/linux/iio/sw_trigger.h
> @@ -27,6 +27,10 @@ struct iio_sw_trigger_type {
>  	struct module *owner;
>  	const struct iio_sw_trigger_ops *ops;
>  	struct list_head list;
> +#ifdef CONFIG_CONFIGFS_FS
> +	struct config_group group;
> +#endif
> +
>  };
>  
>  struct iio_sw_trigger {
> @@ -60,6 +64,10 @@ struct iio_sw_trigger *iio_sw_trigger_create(const char *, const char *);
>  void iio_sw_trigger_destroy(struct iio_sw_trigger *);
>  
>  #ifdef CONFIG_CONFIGFS_FS
> +
> +int iio_sw_trigger_type_configs_register(struct iio_sw_trigger_type *tt);
> +void iio_sw_trigger_type_configs_unregister(struct iio_sw_trigger_type *tt);
> +
>  static inline
>  struct iio_sw_trigger *to_iio_sw_trigger(struct config_item *item)
>  {
> @@ -75,6 +83,17 @@ void iio_config_group_init_type_name(struct config_group *group,
>  	config_group_init_type_name(group, name, type);
>  }
>  #else
> +static inline int iio_sw_trigger_type_configs_register(
> +	struct iio_sw_trigger_type *tt)
> +{
> +	return 0;
> +}
> +
> +static inline void iio_sw_trigger_type_configs_unregister(
> +	struct iio_sw_trigger_type *tt)
> +{
> +}
> +
>  static inline
>  void iio_config_group_init_type_name(struct config_group *group,
>  				     const char *name,
> 

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

[Daniel Baluta]

On Wed, May 13, 2015 at 8:32 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 13/05/15 12:14, Lars-Peter Clausen wrote:
>> Hi,
>>
>> I'd like to go back to the issue of having one folder per trigger type and
>> create triggers for a type in their respective folder.
>>
>> I'm not convinced that it is an intentional design restriction of configfs
>> that we can't do this, but rather a case of it not being implemented
>> because nobody needed it so far.
>>
>> See the proof-of-concept patch below. With this we get one sub-folder per
>> trigger type in /config/iio/triggers. It's not complete and doesn't do
>> proper locking yet and also unregister is not yet implemented but it shows
>> the concept how it could be done.
>>
>> I'm not saying that we definitely should go down that road, but it feels
>> cleaner to me. And another advantage is that it is discoverable which types
>> of triggers are available. Thoughts?
> Nice iff it doesn't break from the intent of configfs.  I know both Daniel and
> I looked for how to do this before taking the non directory based approach that
> the usbgadget driver took.
>
> What you loose is the possibility for auto probing modules based on trigger naming
> (equivalent of what the usbgadget driver does).  I undecided on whether we would
> want to do that anyway.

Ok, I will have a closer look.
>
> Joel, it's your filesystem.  Is doing this acceptable to you?

Adding fsdevel and ocfs2 people. Joel seems to be idle for an awful
long period of time.
I'm not sure if there is anyone maintaining configfs now :).

>>
>> - Lars
>>
>> ---
>>  drivers/iio/industrialio-configfs.c   | 43 ++++++++++++++++++-----------------
>>  drivers/iio/industrialio-sw-trigger.c | 12 ++++++----
>>  fs/configfs/dir.c                     | 28 +++++++++++++++++++++--
>>  include/linux/configfs.h              |  4 ++++
>>  include/linux/iio/sw_trigger.h        | 19 ++++++++++++++++
>>  5 files changed, 79 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/iio/industrialio-configfs.c b/drivers/iio/industrialio-configfs.c
>> index daf318c..c1fa14f 100644
>> --- a/drivers/iio/industrialio-configfs.c
>> +++ b/drivers/iio/industrialio-configfs.c
>> @@ -22,27 +22,9 @@
>>  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) {
>> -             WARN_ONCE(1, "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);
>> +     t = iio_sw_trigger_create(group->cg_item.ci_name, name);
>>       if (IS_ERR(t))
>>               return ERR_CAST(t);
>>
>> @@ -60,13 +42,17 @@ static void trigger_drop_group(struct config_group *group,
>>       config_item_put(item);
>>  }
>>
>> -static struct configfs_group_operations triggers_ops = {
>> +static struct configfs_group_operations trigger_ops = {
>>       .make_group     = &trigger_make_group,
>>       .drop_item      = &trigger_drop_group,
>>  };
>>
>> +static struct config_item_type iio_trigger_group_type = {
>> +     .ct_group_ops = &trigger_ops,
>> +     .ct_owner       = THIS_MODULE,
>> +};
>> +
>>  static struct config_item_type iio_triggers_group_type = {
>> -     .ct_group_ops = &triggers_ops,
>>       .ct_owner       = THIS_MODULE,
>>  };
>>
>> @@ -97,6 +83,21 @@ static struct configfs_subsystem iio_configfs_subsys = {
>>       .su_mutex = __MUTEX_INITIALIZER(iio_configfs_subsys.su_mutex),
>>  };
>>
>> +int iio_sw_trigger_type_configs_register(struct iio_sw_trigger_type *tt)
>> +{
>> +     config_group_init_type_name(&tt->group, tt->name,
>> +             &iio_trigger_group_type);
>> +
>> +     return configfs_register_group(&iio_triggers_group, &tt->group);
>> +}
>> +EXPORT_SYMBOL_GPL(iio_sw_trigger_type_configs_register);
>> +
>> +void iio_sw_trigger_type_configs_unregister(struct iio_sw_trigger_type *tt)
>> +{
>> +     configfs_unregister_group(&tt->group);
>> +}
>> +EXPORT_SYMBOL_GPL(iio_sw_trigger_type_configs_unregister);
>> +
>>  static int __init iio_configfs_init(void)
>>  {
>>       config_group_init(&iio_triggers_group);
>> diff --git a/drivers/iio/industrialio-sw-trigger.c b/drivers/iio/industrialio-sw-trigger.c
>> index fa1ec51..312c33c 100644
>> --- a/drivers/iio/industrialio-sw-trigger.c
>> +++ b/drivers/iio/industrialio-sw-trigger.c
>> @@ -41,10 +41,12 @@ int iio_register_sw_trigger_type(struct iio_sw_trigger_type *t)
>>
>>       write_lock(&iio_trigger_types_lock);
>>       iter = __iio_find_sw_trigger_type(t->name, strlen(t->name));
>> -     if (iter)
>> +     if (iter) {
>>               ret = -EBUSY;
>> -     else
>> +     } else {
>>               list_add_tail(&t->list, &iio_trigger_types_list);
>> +             iio_sw_trigger_type_configs_register(t);
>> +     }
>>       write_unlock(&iio_trigger_types_lock);
>>
>>       return ret;
>> @@ -58,10 +60,12 @@ int iio_unregister_sw_trigger_type(struct iio_sw_trigger_type *t)
>>
>>       write_lock(&iio_trigger_types_lock);
>>       iter = __iio_find_sw_trigger_type(t->name, strlen(t->name));
>> -     if (!iter)
>> +     if (!iter) {
>>               ret = -EINVAL;
>> -     else
>> +     } else {
>> +             iio_sw_trigger_type_configs_unregister(t);
>>               list_del(&t->list);
>> +     }
>>       write_unlock(&iio_trigger_types_lock);
>>
>>       return ret;
>> diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
>> index c9c298b..0e2e0e6 100644
>> --- a/fs/configfs/dir.c
>> +++ b/fs/configfs/dir.c
>> @@ -659,7 +659,7 @@ static void detach_groups(struct config_group *group)
>>   * try using vfs_mkdir.  Just a thought.
>>   */
>>  static int create_default_group(struct config_group *parent_group,
>> -                             struct config_group *group)
>> +                             struct config_group *group, struct dentry **dentry)
>>  {
>>       int ret;
>>       struct configfs_dirent *sd;
>> @@ -679,6 +679,8 @@ static int create_default_group(struct config_group *parent_group,
>>               if (!ret) {
>>                       sd = child->d_fsdata;
>>                       sd->s_type |= CONFIGFS_USET_DEFAULT;
>> +                     if (dentry)
>> +                             *dentry = child;
>>               } else {
>>                       BUG_ON(child->d_inode);
>>                       d_drop(child);
>> @@ -699,7 +701,7 @@ static int populate_groups(struct config_group *group)
>>               for (i = 0; group->default_groups[i]; i++) {
>>                       new_group = group->default_groups[i];
>>
>> -                     ret = create_default_group(group, new_group);
>> +                     ret = create_default_group(group, new_group, NULL);
>>                       if (ret) {
>>                               detach_groups(group);
>>                               break;
>> @@ -1644,6 +1646,28 @@ const struct file_operations configfs_dir_operations = {
>>       .iterate        = configfs_readdir,
>>  };
>>
>> +int configfs_register_group(struct config_group *parent_group,
>> +     struct config_group *group)
>> +{
>> +     struct dentry *dentry;
>> +     int ret;
>> +
>> +     link_group(parent_group, group);
>> +
>> +     ret = create_default_group(parent_group, group, &dentry);
>> +     if (ret == 0)
>> +             configfs_dir_set_ready(dentry->d_fsdata);
>> +
>> +     return ret;
>> +}
>> +EXPORT_SYMBOL(configfs_register_group);
>> +
>> +void configfs_unregister_group(struct config_group *group)
>> +{
>> +     /* TODO */
>> +}
>> +EXPORT_SYMBOL(configfs_unregister_group);
>> +
>>  int configfs_register_subsystem(struct configfs_subsystem *subsys)
>>  {
>>       int err;
>> diff --git a/include/linux/configfs.h b/include/linux/configfs.h
>> index 34025df..a641bea 100644
>> --- a/include/linux/configfs.h
>> +++ b/include/linux/configfs.h
>> @@ -252,6 +252,10 @@ static inline struct configfs_subsystem *to_configfs_subsystem(struct config_gro
>>  int configfs_register_subsystem(struct configfs_subsystem *subsys);
>>  void configfs_unregister_subsystem(struct configfs_subsystem *subsys);
>>
>> +int configfs_register_group(struct config_group *parent_group,
>> +     struct config_group *group);
>> +void configfs_unregister_group(struct config_group *group);
>> +
>>  /* These functions can sleep and can alloc with GFP_KERNEL */
>>  /* WARNING: These cannot be called underneath configfs callbacks!! */
>>  int configfs_depend_item(struct configfs_subsystem *subsys, struct config_item *target);
>> diff --git a/include/linux/iio/sw_trigger.h b/include/linux/iio/sw_trigger.h
>> index 87803c7..5551a11 100644
>> --- a/include/linux/iio/sw_trigger.h
>> +++ b/include/linux/iio/sw_trigger.h
>> @@ -27,6 +27,10 @@ struct iio_sw_trigger_type {
>>       struct module *owner;
>>       const struct iio_sw_trigger_ops *ops;
>>       struct list_head list;
>> +#ifdef CONFIG_CONFIGFS_FS
>> +     struct config_group group;
>> +#endif
>> +
>>  };
>>
>>  struct iio_sw_trigger {
>> @@ -60,6 +64,10 @@ struct iio_sw_trigger *iio_sw_trigger_create(const char *, const char *);
>>  void iio_sw_trigger_destroy(struct iio_sw_trigger *);
>>
>>  #ifdef CONFIG_CONFIGFS_FS
>> +
>> +int iio_sw_trigger_type_configs_register(struct iio_sw_trigger_type *tt);
>> +void iio_sw_trigger_type_configs_unregister(struct iio_sw_trigger_type *tt);
>> +
>>  static inline
>>  struct iio_sw_trigger *to_iio_sw_trigger(struct config_item *item)
>>  {
>> @@ -75,6 +83,17 @@ void iio_config_group_init_type_name(struct config_group *group,
>>       config_group_init_type_name(group, name, type);
>>  }
>>  #else
>> +static inline int iio_sw_trigger_type_configs_register(
>> +     struct iio_sw_trigger_type *tt)
>> +{
>> +     return 0;
>> +}
>> +
>> +static inline void iio_sw_trigger_type_configs_unregister(
>> +     struct iio_sw_trigger_type *tt)
>> +{
>> +}
>> +
>>  static inline
>>  void iio_config_group_init_type_name(struct config_group *group,
>>                                    const char *name,
>>
>
> --
> 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 v6 0/4] Add initial configfs support for IIO

[Daniel Baluta]

On Wed, May 13, 2015 at 8:32 PM, Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On 13/05/15 12:14, Lars-Peter Clausen wrote:
>> Hi,
>>
>> I'd like to go back to the issue of having one folder per trigger type and
>> create triggers for a type in their respective folder.
>>
>> I'm not convinced that it is an intentional design restriction of configfs
>> that we can't do this, but rather a case of it not being implemented
>> because nobody needed it so far.
>>
>> See the proof-of-concept patch below. With this we get one sub-folder per
>> trigger type in /config/iio/triggers. It's not complete and doesn't do
>> proper locking yet and also unregister is not yet implemented but it shows
>> the concept how it could be done.
>>
>> I'm not saying that we definitely should go down that road, but it feels
>> cleaner to me. And another advantage is that it is discoverable which types
>> of triggers are available. Thoughts?
> Nice iff it doesn't break from the intent of configfs.  I know both Daniel and
> I looked for how to do this before taking the non directory based approach that
> the usbgadget driver took.
>
> What you loose is the possibility for auto probing modules based on trigger naming
> (equivalent of what the usbgadget driver does).  I undecided on whether we would
> want to do that anyway.

Ok, I will have a closer look.
>
> Joel, it's your filesystem.  Is doing this acceptable to you?

Adding fsdevel and ocfs2 people. Joel seems to be idle for an awful
long period of time.
I'm not sure if there is anyone maintaining configfs now :).

>>
>> - Lars
>>
>> ---
>>  drivers/iio/industrialio-configfs.c   | 43 ++++++++++++++++++-----------------
>>  drivers/iio/industrialio-sw-trigger.c | 12 ++++++----
>>  fs/configfs/dir.c                     | 28 +++++++++++++++++++++--
>>  include/linux/configfs.h              |  4 ++++
>>  include/linux/iio/sw_trigger.h        | 19 ++++++++++++++++
>>  5 files changed, 79 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/iio/industrialio-configfs.c b/drivers/iio/industrialio-configfs.c
>> index daf318c..c1fa14f 100644
>> --- a/drivers/iio/industrialio-configfs.c
>> +++ b/drivers/iio/industrialio-configfs.c
>> @@ -22,27 +22,9 @@
>>  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) {
>> -             WARN_ONCE(1, "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);
>> +     t = iio_sw_trigger_create(group->cg_item.ci_name, name);
>>       if (IS_ERR(t))
>>               return ERR_CAST(t);
>>
>> @@ -60,13 +42,17 @@ static void trigger_drop_group(struct config_group *group,
>>       config_item_put(item);
>>  }
>>
>> -static struct configfs_group_operations triggers_ops = {
>> +static struct configfs_group_operations trigger_ops = {
>>       .make_group     = &trigger_make_group,
>>       .drop_item      = &trigger_drop_group,
>>  };
>>
>> +static struct config_item_type iio_trigger_group_type = {
>> +     .ct_group_ops = &trigger_ops,
>> +     .ct_owner       = THIS_MODULE,
>> +};
>> +
>>  static struct config_item_type iio_triggers_group_type = {
>> -     .ct_group_ops = &triggers_ops,
>>       .ct_owner       = THIS_MODULE,
>>  };
>>
>> @@ -97,6 +83,21 @@ static struct configfs_subsystem iio_configfs_subsys = {
>>       .su_mutex = __MUTEX_INITIALIZER(iio_configfs_subsys.su_mutex),
>>  };
>>
>> +int iio_sw_trigger_type_configs_register(struct iio_sw_trigger_type *tt)
>> +{
>> +     config_group_init_type_name(&tt->group, tt->name,
>> +             &iio_trigger_group_type);
>> +
>> +     return configfs_register_group(&iio_triggers_group, &tt->group);
>> +}
>> +EXPORT_SYMBOL_GPL(iio_sw_trigger_type_configs_register);
>> +
>> +void iio_sw_trigger_type_configs_unregister(struct iio_sw_trigger_type *tt)
>> +{
>> +     configfs_unregister_group(&tt->group);
>> +}
>> +EXPORT_SYMBOL_GPL(iio_sw_trigger_type_configs_unregister);
>> +
>>  static int __init iio_configfs_init(void)
>>  {
>>       config_group_init(&iio_triggers_group);
>> diff --git a/drivers/iio/industrialio-sw-trigger.c b/drivers/iio/industrialio-sw-trigger.c
>> index fa1ec51..312c33c 100644
>> --- a/drivers/iio/industrialio-sw-trigger.c
>> +++ b/drivers/iio/industrialio-sw-trigger.c
>> @@ -41,10 +41,12 @@ int iio_register_sw_trigger_type(struct iio_sw_trigger_type *t)
>>
>>       write_lock(&iio_trigger_types_lock);
>>       iter = __iio_find_sw_trigger_type(t->name, strlen(t->name));
>> -     if (iter)
>> +     if (iter) {
>>               ret = -EBUSY;
>> -     else
>> +     } else {
>>               list_add_tail(&t->list, &iio_trigger_types_list);
>> +             iio_sw_trigger_type_configs_register(t);
>> +     }
>>       write_unlock(&iio_trigger_types_lock);
>>
>>       return ret;
>> @@ -58,10 +60,12 @@ int iio_unregister_sw_trigger_type(struct iio_sw_trigger_type *t)
>>
>>       write_lock(&iio_trigger_types_lock);
>>       iter = __iio_find_sw_trigger_type(t->name, strlen(t->name));
>> -     if (!iter)
>> +     if (!iter) {
>>               ret = -EINVAL;
>> -     else
>> +     } else {
>> +             iio_sw_trigger_type_configs_unregister(t);
>>               list_del(&t->list);
>> +     }
>>       write_unlock(&iio_trigger_types_lock);
>>
>>       return ret;
>> diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
>> index c9c298b..0e2e0e6 100644
>> --- a/fs/configfs/dir.c
>> +++ b/fs/configfs/dir.c
>> @@ -659,7 +659,7 @@ static void detach_groups(struct config_group *group)
>>   * try using vfs_mkdir.  Just a thought.
>>   */
>>  static int create_default_group(struct config_group *parent_group,
>> -                             struct config_group *group)
>> +                             struct config_group *group, struct dentry **dentry)
>>  {
>>       int ret;
>>       struct configfs_dirent *sd;
>> @@ -679,6 +679,8 @@ static int create_default_group(struct config_group *parent_group,
>>               if (!ret) {
>>                       sd = child->d_fsdata;
>>                       sd->s_type |= CONFIGFS_USET_DEFAULT;
>> +                     if (dentry)
>> +                             *dentry = child;
>>               } else {
>>                       BUG_ON(child->d_inode);
>>                       d_drop(child);
>> @@ -699,7 +701,7 @@ static int populate_groups(struct config_group *group)
>>               for (i = 0; group->default_groups[i]; i++) {
>>                       new_group = group->default_groups[i];
>>
>> -                     ret = create_default_group(group, new_group);
>> +                     ret = create_default_group(group, new_group, NULL);
>>                       if (ret) {
>>                               detach_groups(group);
>>                               break;
>> @@ -1644,6 +1646,28 @@ const struct file_operations configfs_dir_operations = {
>>       .iterate        = configfs_readdir,
>>  };
>>
>> +int configfs_register_group(struct config_group *parent_group,
>> +     struct config_group *group)
>> +{
>> +     struct dentry *dentry;
>> +     int ret;
>> +
>> +     link_group(parent_group, group);
>> +
>> +     ret = create_default_group(parent_group, group, &dentry);
>> +     if (ret == 0)
>> +             configfs_dir_set_ready(dentry->d_fsdata);
>> +
>> +     return ret;
>> +}
>> +EXPORT_SYMBOL(configfs_register_group);
>> +
>> +void configfs_unregister_group(struct config_group *group)
>> +{
>> +     /* TODO */
>> +}
>> +EXPORT_SYMBOL(configfs_unregister_group);
>> +
>>  int configfs_register_subsystem(struct configfs_subsystem *subsys)
>>  {
>>       int err;
>> diff --git a/include/linux/configfs.h b/include/linux/configfs.h
>> index 34025df..a641bea 100644
>> --- a/include/linux/configfs.h
>> +++ b/include/linux/configfs.h
>> @@ -252,6 +252,10 @@ static inline struct configfs_subsystem *to_configfs_subsystem(struct config_gro
>>  int configfs_register_subsystem(struct configfs_subsystem *subsys);
>>  void configfs_unregister_subsystem(struct configfs_subsystem *subsys);
>>
>> +int configfs_register_group(struct config_group *parent_group,
>> +     struct config_group *group);
>> +void configfs_unregister_group(struct config_group *group);
>> +
>>  /* These functions can sleep and can alloc with GFP_KERNEL */
>>  /* WARNING: These cannot be called underneath configfs callbacks!! */
>>  int configfs_depend_item(struct configfs_subsystem *subsys, struct config_item *target);
>> diff --git a/include/linux/iio/sw_trigger.h b/include/linux/iio/sw_trigger.h
>> index 87803c7..5551a11 100644
>> --- a/include/linux/iio/sw_trigger.h
>> +++ b/include/linux/iio/sw_trigger.h
>> @@ -27,6 +27,10 @@ struct iio_sw_trigger_type {
>>       struct module *owner;
>>       const struct iio_sw_trigger_ops *ops;
>>       struct list_head list;
>> +#ifdef CONFIG_CONFIGFS_FS
>> +     struct config_group group;
>> +#endif
>> +
>>  };
>>
>>  struct iio_sw_trigger {
>> @@ -60,6 +64,10 @@ struct iio_sw_trigger *iio_sw_trigger_create(const char *, const char *);
>>  void iio_sw_trigger_destroy(struct iio_sw_trigger *);
>>
>>  #ifdef CONFIG_CONFIGFS_FS
>> +
>> +int iio_sw_trigger_type_configs_register(struct iio_sw_trigger_type *tt);
>> +void iio_sw_trigger_type_configs_unregister(struct iio_sw_trigger_type *tt);
>> +
>>  static inline
>>  struct iio_sw_trigger *to_iio_sw_trigger(struct config_item *item)
>>  {
>> @@ -75,6 +83,17 @@ void iio_config_group_init_type_name(struct config_group *group,
>>       config_group_init_type_name(group, name, type);
>>  }
>>  #else
>> +static inline int iio_sw_trigger_type_configs_register(
>> +     struct iio_sw_trigger_type *tt)
>> +{
>> +     return 0;
>> +}
>> +
>> +static inline void iio_sw_trigger_type_configs_unregister(
>> +     struct iio_sw_trigger_type *tt)
>> +{
>> +}
>> +
>>  static inline
>>  void iio_config_group_init_type_name(struct config_group *group,
>>                                    const char *name,
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Ocfs2-devel] [PATCH v6 0/4] Add initial configfs support for IIO

[Daniel Baluta]

On Wed, May 13, 2015 at 8:32 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 13/05/15 12:14, Lars-Peter Clausen wrote:
>> Hi,
>>
>> I'd like to go back to the issue of having one folder per trigger type and
>> create triggers for a type in their respective folder.
>>
>> I'm not convinced that it is an intentional design restriction of configfs
>> that we can't do this, but rather a case of it not being implemented
>> because nobody needed it so far.
>>
>> See the proof-of-concept patch below. With this we get one sub-folder per
>> trigger type in /config/iio/triggers. It's not complete and doesn't do
>> proper locking yet and also unregister is not yet implemented but it shows
>> the concept how it could be done.
>>
>> I'm not saying that we definitely should go down that road, but it feels
>> cleaner to me. And another advantage is that it is discoverable which types
>> of triggers are available. Thoughts?
> Nice iff it doesn't break from the intent of configfs.  I know both Daniel and
> I looked for how to do this before taking the non directory based approach that
> the usbgadget driver took.
>
> What you loose is the possibility for auto probing modules based on trigger naming
> (equivalent of what the usbgadget driver does).  I undecided on whether we would
> want to do that anyway.

Ok, I will have a closer look.
>
> Joel, it's your filesystem.  Is doing this acceptable to you?

Adding fsdevel and ocfs2 people. Joel seems to be idle for an awful
long period of time.
I'm not sure if there is anyone maintaining configfs now :).

>>
>> - Lars
>>
>> ---
>>  drivers/iio/industrialio-configfs.c   | 43 ++++++++++++++++++-----------------
>>  drivers/iio/industrialio-sw-trigger.c | 12 ++++++----
>>  fs/configfs/dir.c                     | 28 +++++++++++++++++++++--
>>  include/linux/configfs.h              |  4 ++++
>>  include/linux/iio/sw_trigger.h        | 19 ++++++++++++++++
>>  5 files changed, 79 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/iio/industrialio-configfs.c b/drivers/iio/industrialio-configfs.c
>> index daf318c..c1fa14f 100644
>> --- a/drivers/iio/industrialio-configfs.c
>> +++ b/drivers/iio/industrialio-configfs.c
>> @@ -22,27 +22,9 @@
>>  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) {
>> -             WARN_ONCE(1, "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);
>> +     t = iio_sw_trigger_create(group->cg_item.ci_name, name);
>>       if (IS_ERR(t))
>>               return ERR_CAST(t);
>>
>> @@ -60,13 +42,17 @@ static void trigger_drop_group(struct config_group *group,
>>       config_item_put(item);
>>  }
>>
>> -static struct configfs_group_operations triggers_ops = {
>> +static struct configfs_group_operations trigger_ops = {
>>       .make_group     = &trigger_make_group,
>>       .drop_item      = &trigger_drop_group,
>>  };
>>
>> +static struct config_item_type iio_trigger_group_type = {
>> +     .ct_group_ops = &trigger_ops,
>> +     .ct_owner       = THIS_MODULE,
>> +};
>> +
>>  static struct config_item_type iio_triggers_group_type = {
>> -     .ct_group_ops = &triggers_ops,
>>       .ct_owner       = THIS_MODULE,
>>  };
>>
>> @@ -97,6 +83,21 @@ static struct configfs_subsystem iio_configfs_subsys = {
>>       .su_mutex = __MUTEX_INITIALIZER(iio_configfs_subsys.su_mutex),
>>  };
>>
>> +int iio_sw_trigger_type_configs_register(struct iio_sw_trigger_type *tt)
>> +{
>> +     config_group_init_type_name(&tt->group, tt->name,
>> +             &iio_trigger_group_type);
>> +
>> +     return configfs_register_group(&iio_triggers_group, &tt->group);
>> +}
>> +EXPORT_SYMBOL_GPL(iio_sw_trigger_type_configs_register);
>> +
>> +void iio_sw_trigger_type_configs_unregister(struct iio_sw_trigger_type *tt)
>> +{
>> +     configfs_unregister_group(&tt->group);
>> +}
>> +EXPORT_SYMBOL_GPL(iio_sw_trigger_type_configs_unregister);
>> +
>>  static int __init iio_configfs_init(void)
>>  {
>>       config_group_init(&iio_triggers_group);
>> diff --git a/drivers/iio/industrialio-sw-trigger.c b/drivers/iio/industrialio-sw-trigger.c
>> index fa1ec51..312c33c 100644
>> --- a/drivers/iio/industrialio-sw-trigger.c
>> +++ b/drivers/iio/industrialio-sw-trigger.c
>> @@ -41,10 +41,12 @@ int iio_register_sw_trigger_type(struct iio_sw_trigger_type *t)
>>
>>       write_lock(&iio_trigger_types_lock);
>>       iter = __iio_find_sw_trigger_type(t->name, strlen(t->name));
>> -     if (iter)
>> +     if (iter) {
>>               ret = -EBUSY;
>> -     else
>> +     } else {
>>               list_add_tail(&t->list, &iio_trigger_types_list);
>> +             iio_sw_trigger_type_configs_register(t);
>> +     }
>>       write_unlock(&iio_trigger_types_lock);
>>
>>       return ret;
>> @@ -58,10 +60,12 @@ int iio_unregister_sw_trigger_type(struct iio_sw_trigger_type *t)
>>
>>       write_lock(&iio_trigger_types_lock);
>>       iter = __iio_find_sw_trigger_type(t->name, strlen(t->name));
>> -     if (!iter)
>> +     if (!iter) {
>>               ret = -EINVAL;
>> -     else
>> +     } else {
>> +             iio_sw_trigger_type_configs_unregister(t);
>>               list_del(&t->list);
>> +     }
>>       write_unlock(&iio_trigger_types_lock);
>>
>>       return ret;
>> diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
>> index c9c298b..0e2e0e6 100644
>> --- a/fs/configfs/dir.c
>> +++ b/fs/configfs/dir.c
>> @@ -659,7 +659,7 @@ static void detach_groups(struct config_group *group)
>>   * try using vfs_mkdir.  Just a thought.
>>   */
>>  static int create_default_group(struct config_group *parent_group,
>> -                             struct config_group *group)
>> +                             struct config_group *group, struct dentry **dentry)
>>  {
>>       int ret;
>>       struct configfs_dirent *sd;
>> @@ -679,6 +679,8 @@ static int create_default_group(struct config_group *parent_group,
>>               if (!ret) {
>>                       sd = child->d_fsdata;
>>                       sd->s_type |= CONFIGFS_USET_DEFAULT;
>> +                     if (dentry)
>> +                             *dentry = child;
>>               } else {
>>                       BUG_ON(child->d_inode);
>>                       d_drop(child);
>> @@ -699,7 +701,7 @@ static int populate_groups(struct config_group *group)
>>               for (i = 0; group->default_groups[i]; i++) {
>>                       new_group = group->default_groups[i];
>>
>> -                     ret = create_default_group(group, new_group);
>> +                     ret = create_default_group(group, new_group, NULL);
>>                       if (ret) {
>>                               detach_groups(group);
>>                               break;
>> @@ -1644,6 +1646,28 @@ const struct file_operations configfs_dir_operations = {
>>       .iterate        = configfs_readdir,
>>  };
>>
>> +int configfs_register_group(struct config_group *parent_group,
>> +     struct config_group *group)
>> +{
>> +     struct dentry *dentry;
>> +     int ret;
>> +
>> +     link_group(parent_group, group);
>> +
>> +     ret = create_default_group(parent_group, group, &dentry);
>> +     if (ret == 0)
>> +             configfs_dir_set_ready(dentry->d_fsdata);
>> +
>> +     return ret;
>> +}
>> +EXPORT_SYMBOL(configfs_register_group);
>> +
>> +void configfs_unregister_group(struct config_group *group)
>> +{
>> +     /* TODO */
>> +}
>> +EXPORT_SYMBOL(configfs_unregister_group);
>> +
>>  int configfs_register_subsystem(struct configfs_subsystem *subsys)
>>  {
>>       int err;
>> diff --git a/include/linux/configfs.h b/include/linux/configfs.h
>> index 34025df..a641bea 100644
>> --- a/include/linux/configfs.h
>> +++ b/include/linux/configfs.h
>> @@ -252,6 +252,10 @@ static inline struct configfs_subsystem *to_configfs_subsystem(struct config_gro
>>  int configfs_register_subsystem(struct configfs_subsystem *subsys);
>>  void configfs_unregister_subsystem(struct configfs_subsystem *subsys);
>>
>> +int configfs_register_group(struct config_group *parent_group,
>> +     struct config_group *group);
>> +void configfs_unregister_group(struct config_group *group);
>> +
>>  /* These functions can sleep and can alloc with GFP_KERNEL */
>>  /* WARNING: These cannot be called underneath configfs callbacks!! */
>>  int configfs_depend_item(struct configfs_subsystem *subsys, struct config_item *target);
>> diff --git a/include/linux/iio/sw_trigger.h b/include/linux/iio/sw_trigger.h
>> index 87803c7..5551a11 100644
>> --- a/include/linux/iio/sw_trigger.h
>> +++ b/include/linux/iio/sw_trigger.h
>> @@ -27,6 +27,10 @@ struct iio_sw_trigger_type {
>>       struct module *owner;
>>       const struct iio_sw_trigger_ops *ops;
>>       struct list_head list;
>> +#ifdef CONFIG_CONFIGFS_FS
>> +     struct config_group group;
>> +#endif
>> +
>>  };
>>
>>  struct iio_sw_trigger {
>> @@ -60,6 +64,10 @@ struct iio_sw_trigger *iio_sw_trigger_create(const char *, const char *);
>>  void iio_sw_trigger_destroy(struct iio_sw_trigger *);
>>
>>  #ifdef CONFIG_CONFIGFS_FS
>> +
>> +int iio_sw_trigger_type_configs_register(struct iio_sw_trigger_type *tt);
>> +void iio_sw_trigger_type_configs_unregister(struct iio_sw_trigger_type *tt);
>> +
>>  static inline
>>  struct iio_sw_trigger *to_iio_sw_trigger(struct config_item *item)
>>  {
>> @@ -75,6 +83,17 @@ void iio_config_group_init_type_name(struct config_group *group,
>>       config_group_init_type_name(group, name, type);
>>  }
>>  #else
>> +static inline int iio_sw_trigger_type_configs_register(
>> +     struct iio_sw_trigger_type *tt)
>> +{
>> +     return 0;
>> +}
>> +
>> +static inline void iio_sw_trigger_type_configs_unregister(
>> +     struct iio_sw_trigger_type *tt)
>> +{
>> +}
>> +
>>  static inline
>>  void iio_config_group_init_type_name(struct config_group *group,
>>                                    const char *name,
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html