2015-08-10 22:30:33

by Daniel Baluta

[permalink] [raw]
Subject: [PATCH v7 0/5] Add initial configfs support for IIO

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 configfs - offers a generic way of creating IIO objects. So far there
is one default group for triggers.
(2) IIO software triggers - software trigger implementation.
(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.

Also this patchset introduces configfs_(un)register_group to the configfs core
in order to allow drivers to dynamically create groups on demand.

Changes since v6:
* implemented Lars-Peter's idea (https://lkml.org/lkml/2015/5/13/302)
to switch from /config/iio/triggers/hrtimer-instance1 to
/config/iio/triggers/instance1.

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 (5):
configfs: Allow dynamic group (un)registration
iio: core: Introduce IIO configfs support
iio: core: Introduce IIO software triggers
iio: trigger: Introduce IIO hrtimer based trigger
iio: Documentation: Add IIO configfs documentation

Documentation/ABI/testing/configfs-iio | 20 ++++
Documentation/iio/iio_configfs.txt | 57 ++++++++++
drivers/iio/Kconfig | 16 +++
drivers/iio/Makefile | 2 +
drivers/iio/industrialio-configfs.c | 116 ++++++++++++++++++++
drivers/iio/industrialio-sw-trigger.c | 119 ++++++++++++++++++++
drivers/iio/trigger/Kconfig | 10 ++
drivers/iio/trigger/Makefile | 2 +
drivers/iio/trigger/iio-trig-hrtimer.c | 193 +++++++++++++++++++++++++++++++++
fs/configfs/dir.c | 39 ++++++-
include/linux/configfs.h | 4 +
include/linux/iio/sw_trigger.h | 99 +++++++++++++++++
12 files changed, 675 insertions(+), 2 deletions(-)
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

--
2.1.4


2015-08-10 22:30:36

by Daniel Baluta

[permalink] [raw]
Subject: [PATCH v7 1/5] configfs: Allow dynamic group (un)registration

We don't want to hardcode default groups at subsystem
creation time. Thus, we export:
* configfs_register_group
* configfs_unregister_group
that allows drivers to programatically create/destroy groups.

Signed-off-by: Daniel Baluta <[email protected]>
---
I am not sure about the locking scheme here, Joel can you help? :)

fs/configfs/dir.c | 39 +++++++++++++++++++++++++++++++++++++--
include/linux/configfs.h | 4 ++++
2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index c81ce7f..dde3251 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -651,7 +651,8 @@ 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;
@@ -671,6 +672,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(d_inode(child));
d_drop(child);
@@ -691,7 +694,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;
@@ -1636,6 +1639,38 @@ 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)
+{
+ struct dentry *dentry = group->cg_item.ci_dentry;
+
+ mutex_lock(&d_inode(dentry)->i_mutex);
+ configfs_detach_group(&group->cg_item);
+ d_inode(dentry)->i_flags |= S_DEAD;
+ dont_mount(dentry);
+ mutex_unlock(&d_inode(dentry)->i_mutex);
+
+ d_delete(dentry);
+ dput(dentry);
+ unlink_group(group);
+}
+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 c9e5c57..726980e 100644
--- a/include/linux/configfs.h
+++ b/include/linux/configfs.h
@@ -251,6 +251,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);
--
2.1.4

2015-08-10 22:30:39

by Daniel Baluta

[permalink] [raw]
Subject: [PATCH v7 2/5] iio: core: Introduce IIO configfs support

This creates an IIO configfs subystem named "iio", with a default "triggers"
group. Next patches will add trigger types support, allowing new triggers
creation using configfs.

Signed-off-by: Daniel Baluta <[email protected]>
---
drivers/iio/Kconfig | 8 +++++
drivers/iio/Makefile | 1 +
drivers/iio/industrialio-configfs.c | 67 +++++++++++++++++++++++++++++++++++++
3 files changed, 76 insertions(+)
create mode 100644 drivers/iio/industrialio-configfs.c

diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index 4011eff..f2b0d77 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
+ bool "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 698afc2..dbf5f9a 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -9,6 +9,7 @@ industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o
industrialio-$(CONFIG_IIO_BUFFER_CB) += buffer_cb.o

obj-$(CONFIG_IIO_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..3edee90
--- /dev/null
+++ b/drivers/iio/industrialio-configfs.c
@@ -0,0 +1,67 @@
+/*
+ * Industrial I/O configfs bits
+ *
+ * Copyright (c) 2015 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#include <linux/configfs.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kmod.h>
+#include <linux/slab.h>
+
+#include <linux/iio/iio.h>
+
+static struct config_item_type iio_triggers_group_type = {
+ .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 <[email protected]>");
+MODULE_DESCRIPTION("Industrial I/O configfs support");
+MODULE_LICENSE("GPL v2");
--
2.1.4

2015-08-10 22:31:34

by Daniel Baluta

[permalink] [raw]
Subject: [PATCH v7 3/5] iio: core: Introduce IIO software triggers

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 <[email protected]>
---
drivers/iio/Kconfig | 8 +++
drivers/iio/Makefile | 1 +
drivers/iio/industrialio-configfs.c | 49 ++++++++++++++
drivers/iio/industrialio-sw-trigger.c | 119 ++++++++++++++++++++++++++++++++++
include/linux/iio/sw_trigger.h | 99 ++++++++++++++++++++++++++++
5 files changed, 276 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 f2b0d77..117c05a 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -66,6 +66,14 @@ config IIO_CONSUMERS_PER_TRIGGER
This value controls the maximum number of consumers that a
given trigger may handle. Default is 2.

+config IIO_SW_TRIGGER
+ 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 dbf5f9a..31aead3 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-configfs.c b/drivers/iio/industrialio-configfs.c
index 3edee90..ced7115 100644
--- a/drivers/iio/industrialio-configfs.c
+++ b/drivers/iio/industrialio-configfs.c
@@ -15,6 +15,40 @@
#include <linux/slab.h>

#include <linux/iio/iio.h>
+#include <linux/iio/sw_trigger.h>
+
+static struct config_group *trigger_make_group(struct config_group *group,
+ const char *name)
+{
+ struct iio_sw_trigger *t;
+
+ t = iio_sw_trigger_create(group->cg_item.ci_name, name);
+ if (IS_ERR(t))
+ return ERR_CAST(t);
+
+ config_item_set_name(&t->group.cg_item, name);
+
+ return &t->group;
+}
+
+static void trigger_drop_group(struct config_group *group,
+ struct config_item *item)
+{
+ struct iio_sw_trigger *t = to_iio_sw_trigger(item);
+
+ iio_sw_trigger_destroy(t);
+ config_item_put(item);
+}
+
+static struct configfs_group_operations trigger_ops = {
+ .make_group = &trigger_make_group,
+ .drop_item = &trigger_drop_group,
+};
+
+static struct config_item_type iio_trigger_type_group_type = {
+ .ct_group_ops = &trigger_ops,
+ .ct_owner = THIS_MODULE,
+};

static struct config_item_type iio_triggers_group_type = {
.ct_owner = THIS_MODULE,
@@ -47,6 +81,21 @@ static struct configfs_subsystem iio_configfs_subsys = {
.su_mutex = __MUTEX_INITIALIZER(iio_configfs_subsys.su_mutex),
};

+int iio_sw_trigger_type_configfs_register(struct iio_sw_trigger_type *tt)
+{
+ config_group_init_type_name(&tt->group, tt->name,
+ &iio_trigger_type_group_type);
+
+ return configfs_register_group(&iio_triggers_group, &tt->group);
+}
+EXPORT_SYMBOL_GPL(iio_sw_trigger_type_configfs_register);
+
+void iio_sw_trigger_type_configfs_unregister(struct iio_sw_trigger_type *tt)
+{
+ configfs_unregister_group(&tt->group);
+}
+EXPORT_SYMBOL_GPL(iio_sw_trigger_type_configfs_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
new file mode 100644
index 0000000..761418e
--- /dev/null
+++ b/drivers/iio/industrialio-sw-trigger.c
@@ -0,0 +1,119 @@
+/*
+ * 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_MUTEX(iio_trigger_types_lock);
+
+static
+struct iio_sw_trigger_type *__iio_find_sw_trigger_type(const char *name,
+ unsigned len)
+{
+ struct iio_sw_trigger_type *t = NULL, *iter;
+
+ list_for_each_entry(iter, &iio_trigger_types_list, list)
+ if (!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;
+
+ mutex_lock(&iio_trigger_types_lock);
+ iter = __iio_find_sw_trigger_type(t->name, strlen(t->name));
+ if (iter) {
+ ret = -EBUSY;
+ } else {
+ list_add_tail(&t->list, &iio_trigger_types_list);
+ iio_sw_trigger_type_configfs_register(t);
+ }
+ mutex_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;
+
+ mutex_lock(&iio_trigger_types_lock);
+ iter = __iio_find_sw_trigger_type(t->name, strlen(t->name));
+ if (!iter) {
+ ret = -EINVAL;
+ } else {
+ iio_sw_trigger_type_configfs_unregister(t);
+ list_del(&t->list);
+ }
+ mutex_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;
+
+ mutex_lock(&iio_trigger_types_lock);
+ t = __iio_find_sw_trigger_type(name, strlen(name));
+ if (t && !try_module_get(t->owner))
+ t = NULL;
+ mutex_unlock(&iio_trigger_types_lock);
+
+ return t;
+}
+
+struct iio_sw_trigger *iio_sw_trigger_create(const char *type, const char *name)
+{
+ struct iio_sw_trigger *t;
+ struct iio_sw_trigger_type *tt;
+
+ tt = iio_get_sw_trigger_type(type);
+ if (!tt) {
+ pr_err("Invalid trigger type: %s\n", type);
+ return ERR_PTR(-EINVAL);
+ }
+ t = tt->ops->probe(name);
+ if (IS_ERR(t))
+ goto out_module_put;
+
+ t->trigger_type = tt;
+
+ return t;
+out_module_put:
+ module_put(tt->owner);
+ return t;
+}
+EXPORT_SYMBOL(iio_sw_trigger_create);
+
+void iio_sw_trigger_destroy(struct iio_sw_trigger *t)
+{
+ struct iio_sw_trigger_type *tt = t->trigger_type;
+
+ tt->ops->remove(t);
+ module_put(tt->owner);
+}
+EXPORT_SYMBOL(iio_sw_trigger_destroy);
diff --git a/include/linux/iio/sw_trigger.h b/include/linux/iio/sw_trigger.h
new file mode 100644
index 0000000..762a3b7
--- /dev/null
+++ b/include/linux/iio/sw_trigger.h
@@ -0,0 +1,99 @@
+/*
+ * Industrial I/O software trigger interface
+ *
+ * Copyright (c) 2015 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#ifndef __IIO_SW_TRIGGER
+#define __IIO_SW_TRIGGER
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/iio/iio.h>
+#include <linux/configfs.h>
+
+#define module_iio_sw_trigger_driver(__iio_sw_trigger_type) \
+ module_driver(__iio_sw_trigger_type, iio_register_sw_trigger_type, \
+ iio_unregister_sw_trigger_type)
+
+struct iio_sw_trigger_ops;
+
+struct iio_sw_trigger_type {
+ const char *name;
+ struct module *owner;
+ const struct iio_sw_trigger_ops *ops;
+ struct list_head list;
+ struct config_group group;
+};
+
+struct iio_sw_trigger {
+ struct iio_trigger *trigger;
+ struct iio_sw_trigger_type *trigger_type;
+ struct config_group group;
+};
+
+struct iio_sw_trigger_ops {
+ struct iio_sw_trigger* (*probe)(const char *);
+ int (*remove)(struct iio_sw_trigger *);
+};
+
+/**
+ * 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 *);
+
+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);
+}
+
+#ifdef CONFIG_CONFIGFS_FS
+int iio_sw_trigger_type_configfs_register(struct iio_sw_trigger_type *tt);
+void iio_sw_trigger_type_configfs_unregister(struct iio_sw_trigger_type *tt);
+
+static inline
+void iio_swt_group_init_type_name(struct iio_sw_trigger *t,
+ const char *name,
+ struct config_item_type *type)
+{
+ config_group_init_type_name(&t->group, name, type);
+}
+#else
+static inline
+int iio_sw_trigger_type_configfs_register(struct iio_sw_trigger_type *tt)
+{
+ return 0;
+}
+
+static inline
+void iio_sw_trigger_type_configfs_unregister(struct iio_sw_trigger_type *tt)
+{ }
+
+static inline
+void iio_swt_group_init_type_name(struct iio_sw_trigger *t,
+ const char *name,
+ struct config_item_type *type)
+{ }
+#endif
+
+#endif /* __IIO_SW_TRIGGER */
--
2.1.4

2015-08-10 22:30:43

by Daniel Baluta

[permalink] [raw]
Subject: [PATCH v7 4/5] iio: trigger: Introduce IIO hrtimer based trigger

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 <[email protected]>
Signed-off-by: Lars-Peter Clausen <[email protected]>
Signed-off-by: Daniel Baluta <[email protected]>
---
drivers/iio/trigger/Kconfig | 10 ++
drivers/iio/trigger/Makefile | 2 +
drivers/iio/trigger/iio-trig-hrtimer.c | 193 +++++++++++++++++++++++++++++++++
3 files changed, 205 insertions(+)
create mode 100644 drivers/iio/trigger/iio-trig-hrtimer.c

diff --git a/drivers/iio/trigger/Kconfig b/drivers/iio/trigger/Kconfig
index 7999612..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..4631c7a
--- /dev/null
+++ b/drivers/iio/trigger/iio-trig-hrtimer.c
@@ -0,0 +1,193 @@
+/**
+ * The industrial I/O periodic hrtimer trigger driver
+ *
+ * Copyright (C) Intuitive Aerial AB
+ * Written by Marten Svanfeldt, [email protected]
+ * Copyright (C) 2012, Analog Device Inc.
+ * Author: Lars-Peter Clausen <[email protected]>
+ * Copyright (C) 2015, Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ */
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/hrtimer.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/sw_trigger.h>
+
+/* default sampling frequency - 100Hz */
+#define HRTIMER_DEFAULT_SAMPLING_FREQUENCY 100
+
+struct iio_hrtimer_info {
+ struct iio_sw_trigger swt;
+ struct hrtimer timer;
+ unsigned long sampling_frequency;
+ ktime_t period;
+};
+
+static struct config_item_type iio_hrtimer_type = {
+ .ct_owner = THIS_MODULE,
+};
+
+static
+ssize_t iio_hrtimer_show_sampling_frequency(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct iio_trigger *trig = to_iio_trigger(dev);
+ struct iio_hrtimer_info *info = iio_trigger_get_drvdata(trig);
+
+ return snprintf(buf, PAGE_SIZE, "%lu\n", info->sampling_frequency);
+}
+
+static
+ssize_t iio_hrtimer_store_sampling_frequency(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct iio_trigger *trig = to_iio_trigger(dev);
+ struct iio_hrtimer_info *info = iio_trigger_get_drvdata(trig);
+ unsigned long val;
+ int ret;
+
+ ret = kstrtoul(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (!val || val > NSEC_PER_SEC)
+ return -EINVAL;
+
+ info->sampling_frequency = val;
+ info->period = ktime_set(0, NSEC_PER_SEC / val);
+
+ return len;
+}
+
+static DEVICE_ATTR(sampling_frequency, S_IRUGO | S_IWUSR,
+ iio_hrtimer_show_sampling_frequency,
+ iio_hrtimer_store_sampling_frequency);
+
+static struct attribute *iio_hrtimer_attrs[] = {
+ &dev_attr_sampling_frequency.attr,
+ NULL
+};
+
+static const struct attribute_group iio_hrtimer_attr_group = {
+ .attrs = iio_hrtimer_attrs,
+};
+
+static const struct attribute_group *iio_hrtimer_attr_groups[] = {
+ &iio_hrtimer_attr_group,
+ NULL
+};
+
+static enum hrtimer_restart iio_hrtimer_trig_handler(struct hrtimer *timer)
+{
+ struct iio_hrtimer_info *info;
+
+ info = container_of(timer, struct iio_hrtimer_info, timer);
+
+ hrtimer_forward_now(timer, info->period);
+ iio_trigger_poll(info->swt.trigger);
+
+ return HRTIMER_RESTART;
+}
+
+static int iio_trig_hrtimer_set_state(struct iio_trigger *trig, bool state)
+{
+ struct iio_hrtimer_info *trig_info;
+
+ trig_info = iio_trigger_get_drvdata(trig);
+
+ if (state)
+ hrtimer_start(&trig_info->timer, trig_info->period,
+ HRTIMER_MODE_REL);
+ else
+ hrtimer_cancel(&trig_info->timer);
+
+ return 0;
+}
+
+static const struct iio_trigger_ops iio_hrtimer_trigger_ops = {
+ .owner = THIS_MODULE,
+ .set_trigger_state = iio_trig_hrtimer_set_state,
+};
+
+static struct iio_sw_trigger *iio_trig_hrtimer_probe(const char *name)
+{
+ struct iio_hrtimer_info *trig_info;
+ int ret;
+
+ trig_info = kzalloc(sizeof(*trig_info), GFP_KERNEL);
+ if (!trig_info)
+ return ERR_PTR(-ENOMEM);
+
+ trig_info->swt.trigger = iio_trigger_alloc("%s", name);
+ if (!trig_info->swt.trigger) {
+ ret = -ENOMEM;
+ goto err_free_trig_info;
+ }
+
+ iio_trigger_set_drvdata(trig_info->swt.trigger, trig_info);
+ trig_info->swt.trigger->ops = &iio_hrtimer_trigger_ops;
+ trig_info->swt.trigger->dev.groups = iio_hrtimer_attr_groups;
+
+ hrtimer_init(&trig_info->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ trig_info->timer.function = iio_hrtimer_trig_handler;
+
+ trig_info->sampling_frequency = HRTIMER_DEFAULT_SAMPLING_FREQUENCY;
+ trig_info->period = ktime_set(0, NSEC_PER_SEC /
+ trig_info->sampling_frequency);
+
+ ret = iio_trigger_register(trig_info->swt.trigger);
+ if (ret)
+ goto err_free_trigger;
+
+ iio_swt_group_init_type_name(&trig_info->swt, name, &iio_hrtimer_type);
+ return &trig_info->swt;
+err_free_trigger:
+ iio_trigger_free(trig_info->swt.trigger);
+err_free_trig_info:
+ kfree(trig_info);
+
+ return ERR_PTR(ret);
+}
+
+static int iio_trig_hrtimer_remove(struct iio_sw_trigger *swt)
+{
+ struct iio_hrtimer_info *trig_info;
+
+ trig_info = iio_trigger_get_drvdata(swt->trigger);
+
+ iio_trigger_unregister(swt->trigger);
+
+ /* cancel the timer after unreg to make sure no one rearms it */
+ hrtimer_cancel(&trig_info->timer);
+ iio_trigger_free(swt->trigger);
+ kfree(trig_info);
+
+ return 0;
+}
+
+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 <[email protected]>");
+MODULE_AUTHOR("Daniel Baluta <[email protected]>");
+MODULE_DESCRIPTION("Periodic hrtimer trigger for the IIO subsystem");
+MODULE_LICENSE("GPL v2");
--
2.1.4

2015-08-10 22:30:49

by Daniel Baluta

[permalink] [raw]
Subject: [PATCH v7 5/5] iio: Documentation: Add IIO configfs documentation

Signed-off-by: Daniel Baluta <[email protected]>
---
Documentation/ABI/testing/configfs-iio | 20 ++++++++++++
Documentation/iio/iio_configfs.txt | 57 ++++++++++++++++++++++++++++++++++
2 files changed, 77 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..1fd7cf4
--- /dev/null
+++ b/Documentation/ABI/testing/configfs-iio
@@ -0,0 +1,20 @@
+What: /config/iio
+Date: May 2015
+KernelVersion: 4.2
+Contact: [email protected]
+Description:
+ This represents Industrial IO configuration entry point
+ directory. It contains sub-groups corresponding to IIO
+ objects.
+
+What: /config/iio/triggers
+Date: August 2015
+KernelVersion: 4.2
+Description:
+ Industrial IO software triggers directory.
+
+What: /config/iio/triggers/hrtimer
+Date: August 2015
+KernelVersion: 4.2
+Description:
+ Industrial IO hrtimer based software triggers directory.
diff --git a/Documentation/iio/iio_configfs.txt b/Documentation/iio/iio_configfs.txt
new file mode 100644
index 0000000..7c56997
--- /dev/null
+++ b/Documentation/iio/iio_configfs.txt
@@ -0,0 +1,57 @@
+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. There
+must exist an associated kernel module that implements a software trigger type.
+
+We support now the following software trigger types:
+ * hrtimer, uses high resolution timers as interrupt source implemened
+ by the iio-trig-hrtimer.c module.
+
+3.1 Software triggers creation and destruction
+
+As simply as:
+
+$ insmod iio-trig-hrtimer.ko
+$ ls /config/triggers
+hrtimer
+$ mkdir /config/triggers/hrtimer/trig1
+
+$ rmdir /config/triggers/hrtimer/trig1
+$ rmmod iio-trig-hrtimer
+
+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,
+but hrtimer triggers have the sampling_frequency attribute under /sys/bus/iio/devices/triggerX
+directory.
--
2.1.4

2015-08-17 11:32:00

by Vladimir Barinov

[permalink] [raw]
Subject: Re: [PATCH v7 3/5] iio: core: Introduce IIO software triggers

Hi Daniel,

Minor comments since I need your hrtimer trigger support.

1) The drivers/iio/industrialio-sw-trigger.c should probably
come to drivers/iio/trigger/ folder which is under ifdef condition
in drivers/iio/Kconfig

2) it breaks compilation
enabling IIO_HRTIMER_TRIGGER selects IIO_SW_TRIGGER.
But IIO_SW_TRIGGER fail to compile without IIO_CONFIGFS

3) Probably the IIO_SW_TRIGGER should be tristate like other
standalone triggers

I will report test results when I try hrtimer trigger.

Regards,
Vladimir

On 11.08.2015 01:42, 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 <[email protected]>
> ---
> drivers/iio/Kconfig | 8 +++
> drivers/iio/Makefile | 1 +
> drivers/iio/industrialio-configfs.c | 49 ++++++++++++++
> drivers/iio/industrialio-sw-trigger.c | 119 ++++++++++++++++++++++++++++++++++
> include/linux/iio/sw_trigger.h | 99 ++++++++++++++++++++++++++++
> 5 files changed, 276 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 f2b0d77..117c05a 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -66,6 +66,14 @@ config IIO_CONSUMERS_PER_TRIGGER
> This value controls the maximum number of consumers that a
> given trigger may handle. Default is 2.
>
> +config IIO_SW_TRIGGER
> + 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 dbf5f9a..31aead3 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-configfs.c b/drivers/iio/industrialio-configfs.c
> index 3edee90..ced7115 100644
> --- a/drivers/iio/industrialio-configfs.c
> +++ b/drivers/iio/industrialio-configfs.c
> @@ -15,6 +15,40 @@
> #include <linux/slab.h>
>
> #include <linux/iio/iio.h>
> +#include <linux/iio/sw_trigger.h>
> +
> +static struct config_group *trigger_make_group(struct config_group *group,
> + const char *name)
> +{
> + struct iio_sw_trigger *t;
> +
> + t = iio_sw_trigger_create(group->cg_item.ci_name, name);
> + if (IS_ERR(t))
> + return ERR_CAST(t);
> +
> + config_item_set_name(&t->group.cg_item, name);
> +
> + return &t->group;
> +}
> +
> +static void trigger_drop_group(struct config_group *group,
> + struct config_item *item)
> +{
> + struct iio_sw_trigger *t = to_iio_sw_trigger(item);
> +
> + iio_sw_trigger_destroy(t);
> + config_item_put(item);
> +}
> +
> +static struct configfs_group_operations trigger_ops = {
> + .make_group = &trigger_make_group,
> + .drop_item = &trigger_drop_group,
> +};
> +
> +static struct config_item_type iio_trigger_type_group_type = {
> + .ct_group_ops = &trigger_ops,
> + .ct_owner = THIS_MODULE,
> +};
>
> static struct config_item_type iio_triggers_group_type = {
> .ct_owner = THIS_MODULE,
> @@ -47,6 +81,21 @@ static struct configfs_subsystem iio_configfs_subsys = {
> .su_mutex = __MUTEX_INITIALIZER(iio_configfs_subsys.su_mutex),
> };
>
> +int iio_sw_trigger_type_configfs_register(struct iio_sw_trigger_type *tt)
> +{
> + config_group_init_type_name(&tt->group, tt->name,
> + &iio_trigger_type_group_type);
> +
> + return configfs_register_group(&iio_triggers_group, &tt->group);
> +}
> +EXPORT_SYMBOL_GPL(iio_sw_trigger_type_configfs_register);
> +
> +void iio_sw_trigger_type_configfs_unregister(struct iio_sw_trigger_type *tt)
> +{
> + configfs_unregister_group(&tt->group);
> +}
> +EXPORT_SYMBOL_GPL(iio_sw_trigger_type_configfs_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
> new file mode 100644
> index 0000000..761418e
> --- /dev/null
> +++ b/drivers/iio/industrialio-sw-trigger.c
> @@ -0,0 +1,119 @@
> +/*
> + * 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_MUTEX(iio_trigger_types_lock);
> +
> +static
> +struct iio_sw_trigger_type *__iio_find_sw_trigger_type(const char *name,
> + unsigned len)
> +{
> + struct iio_sw_trigger_type *t = NULL, *iter;
> +
> + list_for_each_entry(iter, &iio_trigger_types_list, list)
> + if (!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;
> +
> + mutex_lock(&iio_trigger_types_lock);
> + iter = __iio_find_sw_trigger_type(t->name, strlen(t->name));
> + if (iter) {
> + ret = -EBUSY;
> + } else {
> + list_add_tail(&t->list, &iio_trigger_types_list);
> + iio_sw_trigger_type_configfs_register(t);
> + }
> + mutex_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;
> +
> + mutex_lock(&iio_trigger_types_lock);
> + iter = __iio_find_sw_trigger_type(t->name, strlen(t->name));
> + if (!iter) {
> + ret = -EINVAL;
> + } else {
> + iio_sw_trigger_type_configfs_unregister(t);
> + list_del(&t->list);
> + }
> + mutex_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;
> +
> + mutex_lock(&iio_trigger_types_lock);
> + t = __iio_find_sw_trigger_type(name, strlen(name));
> + if (t && !try_module_get(t->owner))
> + t = NULL;
> + mutex_unlock(&iio_trigger_types_lock);
> +
> + return t;
> +}
> +
> +struct iio_sw_trigger *iio_sw_trigger_create(const char *type, const char *name)
> +{
> + struct iio_sw_trigger *t;
> + struct iio_sw_trigger_type *tt;
> +
> + tt = iio_get_sw_trigger_type(type);
> + if (!tt) {
> + pr_err("Invalid trigger type: %s\n", type);
> + return ERR_PTR(-EINVAL);
> + }
> + t = tt->ops->probe(name);
> + if (IS_ERR(t))
> + goto out_module_put;
> +
> + t->trigger_type = tt;
> +
> + return t;
> +out_module_put:
> + module_put(tt->owner);
> + return t;
> +}
> +EXPORT_SYMBOL(iio_sw_trigger_create);
> +
> +void iio_sw_trigger_destroy(struct iio_sw_trigger *t)
> +{
> + struct iio_sw_trigger_type *tt = t->trigger_type;
> +
> + tt->ops->remove(t);
> + module_put(tt->owner);
> +}
> +EXPORT_SYMBOL(iio_sw_trigger_destroy);
> diff --git a/include/linux/iio/sw_trigger.h b/include/linux/iio/sw_trigger.h
> new file mode 100644
> index 0000000..762a3b7
> --- /dev/null
> +++ b/include/linux/iio/sw_trigger.h
> @@ -0,0 +1,99 @@
> +/*
> + * Industrial I/O software trigger interface
> + *
> + * Copyright (c) 2015 Intel Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + */
> +
> +#ifndef __IIO_SW_TRIGGER
> +#define __IIO_SW_TRIGGER
> +
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/iio/iio.h>
> +#include <linux/configfs.h>
> +
> +#define module_iio_sw_trigger_driver(__iio_sw_trigger_type) \
> + module_driver(__iio_sw_trigger_type, iio_register_sw_trigger_type, \
> + iio_unregister_sw_trigger_type)
> +
> +struct iio_sw_trigger_ops;
> +
> +struct iio_sw_trigger_type {
> + const char *name;
> + struct module *owner;
> + const struct iio_sw_trigger_ops *ops;
> + struct list_head list;
> + struct config_group group;
> +};
> +
> +struct iio_sw_trigger {
> + struct iio_trigger *trigger;
> + struct iio_sw_trigger_type *trigger_type;
> + struct config_group group;
> +};
> +
> +struct iio_sw_trigger_ops {
> + struct iio_sw_trigger* (*probe)(const char *);
> + int (*remove)(struct iio_sw_trigger *);
> +};
> +
> +/**
> + * 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 *);
> +
> +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);
> +}
> +
> +#ifdef CONFIG_CONFIGFS_FS
> +int iio_sw_trigger_type_configfs_register(struct iio_sw_trigger_type *tt);
> +void iio_sw_trigger_type_configfs_unregister(struct iio_sw_trigger_type *tt);
> +
> +static inline
> +void iio_swt_group_init_type_name(struct iio_sw_trigger *t,
> + const char *name,
> + struct config_item_type *type)
> +{
> + config_group_init_type_name(&t->group, name, type);
> +}
> +#else
> +static inline
> +int iio_sw_trigger_type_configfs_register(struct iio_sw_trigger_type *tt)
> +{
> + return 0;
> +}
> +
> +static inline
> +void iio_sw_trigger_type_configfs_unregister(struct iio_sw_trigger_type *tt)
> +{ }
> +
> +static inline
> +void iio_swt_group_init_type_name(struct iio_sw_trigger *t,
> + const char *name,
> + struct config_item_type *type)
> +{ }
> +#endif
> +
> +#endif /* __IIO_SW_TRIGGER */

2015-08-17 12:02:03

by Vladimir Barinov

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] iio: Documentation: Add IIO configfs documentation

Hi Daniel,

Find minor comments.

Regards,
Vladimir

On 11.08.2015 01:42, Daniel Baluta wrote:
> Signed-off-by: Daniel Baluta <[email protected]>
> ---
> Documentation/ABI/testing/configfs-iio | 20 ++++++++++++
> Documentation/iio/iio_configfs.txt | 57 ++++++++++++++++++++++++++++++++++
> 2 files changed, 77 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..1fd7cf4
> --- /dev/null
> +++ b/Documentation/ABI/testing/configfs-iio
> @@ -0,0 +1,20 @@
> +What: /config/iio
> +Date: May 2015
> +KernelVersion: 4.2
> +Contact: [email protected]
> +Description:
> + This represents Industrial IO configuration entry point
> + directory. It contains sub-groups corresponding to IIO
> + objects.
> +
> +What: /config/iio/triggers
> +Date: August 2015
> +KernelVersion: 4.2
> +Description:
> + Industrial IO software triggers directory.
> +
> +What: /config/iio/triggers/hrtimer
> +Date: August 2015
> +KernelVersion: 4.2
> +Description:
> + Industrial IO hrtimer based software triggers directory.
> diff --git a/Documentation/iio/iio_configfs.txt b/Documentation/iio/iio_configfs.txt
> new file mode 100644
> index 0000000..7c56997
> --- /dev/null
> +++ b/Documentation/iio/iio_configfs.txt
> @@ -0,0 +1,57 @@
> +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. There
> +must exist an associated kernel module that implements a software trigger type.
> +
> +We support now the following software trigger types:
> + * hrtimer, uses high resolution timers as interrupt source implemened
> + by the iio-trig-hrtimer.c module.
> +
> +3.1 Software triggers creation and destruction
> +
> +As simply as:
> +
> +$ insmod iio-trig-hrtimer.ko
> +$ ls /config/triggers
ls /config/iio/triggers/

> +hrtimer
> +$ mkdir /config/triggers/hrtimer/trig1
mkdir /config/iio/triggers/hrtimer/trig1

This is confusing:
$ cat /sys/bus//iio/devices/trigger0/name
trig1

How may I know that this is a hrtimer trigger in case I've enabled
couple of triggers or when sw_trigger support extends with other
then hrtimer trigger?

> +
> +$ rmdir /config/triggers/hrtimer/trig1
ditto


> +$ rmmod iio-trig-hrtimer
> +
> +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,
> +but hrtimer triggers have the sampling_frequency attribute under /sys/bus/iio/devices/triggerX
> +directory.

2015-08-17 12:46:10

by Vladimir Barinov

[permalink] [raw]
Subject: Re: [PATCH v7 4/5] iio: trigger: Introduce IIO hrtimer based trigger

Hello Daniel,

I've verified that your hrtimer trigger works.

I have minor suggestions.

Will not it be useful to have ability to stop/start hrtimer polling
not only during attach/detach trigger.

f.e. writing 0 to sampling_frequency will hrtimer_cancel
and writing any valid frequency will hrtimer_start.

Regards,
Vladimir

On 11.08.2015 01:42, 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 <[email protected]>
> Signed-off-by: Lars-Peter Clausen <[email protected]>
> Signed-off-by: Daniel Baluta <[email protected]>
> ---
> drivers/iio/trigger/Kconfig | 10 ++
> drivers/iio/trigger/Makefile | 2 +
> drivers/iio/trigger/iio-trig-hrtimer.c | 193 +++++++++++++++++++++++++++++++++
> 3 files changed, 205 insertions(+)
> create mode 100644 drivers/iio/trigger/iio-trig-hrtimer.c
>
> diff --git a/drivers/iio/trigger/Kconfig b/drivers/iio/trigger/Kconfig
> index 7999612..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..4631c7a
> --- /dev/null
> +++ b/drivers/iio/trigger/iio-trig-hrtimer.c
> @@ -0,0 +1,193 @@
> +/**
> + * The industrial I/O periodic hrtimer trigger driver
> + *
> + * Copyright (C) Intuitive Aerial AB
> + * Written by Marten Svanfeldt, [email protected]
> + * Copyright (C) 2012, Analog Device Inc.
> + * Author: Lars-Peter Clausen <[email protected]>
> + * Copyright (C) 2015, Intel Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + */
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/hrtimer.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/sw_trigger.h>
> +
> +/* default sampling frequency - 100Hz */
> +#define HRTIMER_DEFAULT_SAMPLING_FREQUENCY 100
> +
> +struct iio_hrtimer_info {
> + struct iio_sw_trigger swt;
> + struct hrtimer timer;
> + unsigned long sampling_frequency;
> + ktime_t period;
> +};
> +
> +static struct config_item_type iio_hrtimer_type = {
> + .ct_owner = THIS_MODULE,
> +};
> +
> +static
> +ssize_t iio_hrtimer_show_sampling_frequency(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct iio_trigger *trig = to_iio_trigger(dev);
> + struct iio_hrtimer_info *info = iio_trigger_get_drvdata(trig);
> +
> + return snprintf(buf, PAGE_SIZE, "%lu\n", info->sampling_frequency);
> +}
> +
> +static
> +ssize_t iio_hrtimer_store_sampling_frequency(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct iio_trigger *trig = to_iio_trigger(dev);
> + struct iio_hrtimer_info *info = iio_trigger_get_drvdata(trig);
> + unsigned long val;
> + int ret;
> +
> + ret = kstrtoul(buf, 10, &val);
> + if (ret)
> + return ret;
> +
> + if (!val || val > NSEC_PER_SEC)
> + return -EINVAL;
> +
> + info->sampling_frequency = val;
> + info->period = ktime_set(0, NSEC_PER_SEC / val);
> +
> + return len;
> +}
> +
> +static DEVICE_ATTR(sampling_frequency, S_IRUGO | S_IWUSR,
> + iio_hrtimer_show_sampling_frequency,
> + iio_hrtimer_store_sampling_frequency);
> +
> +static struct attribute *iio_hrtimer_attrs[] = {
> + &dev_attr_sampling_frequency.attr,
> + NULL
> +};
> +
> +static const struct attribute_group iio_hrtimer_attr_group = {
> + .attrs = iio_hrtimer_attrs,
> +};
> +
> +static const struct attribute_group *iio_hrtimer_attr_groups[] = {
> + &iio_hrtimer_attr_group,
> + NULL
> +};
> +
> +static enum hrtimer_restart iio_hrtimer_trig_handler(struct hrtimer *timer)
> +{
> + struct iio_hrtimer_info *info;
> +
> + info = container_of(timer, struct iio_hrtimer_info, timer);
> +
> + hrtimer_forward_now(timer, info->period);
> + iio_trigger_poll(info->swt.trigger);
> +
> + return HRTIMER_RESTART;
> +}
> +
> +static int iio_trig_hrtimer_set_state(struct iio_trigger *trig, bool state)
> +{
> + struct iio_hrtimer_info *trig_info;
> +
> + trig_info = iio_trigger_get_drvdata(trig);
> +
> + if (state)
> + hrtimer_start(&trig_info->timer, trig_info->period,
> + HRTIMER_MODE_REL);
> + else
> + hrtimer_cancel(&trig_info->timer);
> +
> + return 0;
> +}
> +
> +static const struct iio_trigger_ops iio_hrtimer_trigger_ops = {
> + .owner = THIS_MODULE,
> + .set_trigger_state = iio_trig_hrtimer_set_state,
> +};
> +
> +static struct iio_sw_trigger *iio_trig_hrtimer_probe(const char *name)
> +{
> + struct iio_hrtimer_info *trig_info;
> + int ret;
> +
> + trig_info = kzalloc(sizeof(*trig_info), GFP_KERNEL);
> + if (!trig_info)
> + return ERR_PTR(-ENOMEM);
> +
> + trig_info->swt.trigger = iio_trigger_alloc("%s", name);
> + if (!trig_info->swt.trigger) {
> + ret = -ENOMEM;
> + goto err_free_trig_info;
> + }
> +
> + iio_trigger_set_drvdata(trig_info->swt.trigger, trig_info);
> + trig_info->swt.trigger->ops = &iio_hrtimer_trigger_ops;
> + trig_info->swt.trigger->dev.groups = iio_hrtimer_attr_groups;
> +
> + hrtimer_init(&trig_info->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> + trig_info->timer.function = iio_hrtimer_trig_handler;
> +
> + trig_info->sampling_frequency = HRTIMER_DEFAULT_SAMPLING_FREQUENCY;
> + trig_info->period = ktime_set(0, NSEC_PER_SEC /
> + trig_info->sampling_frequency);
> +
> + ret = iio_trigger_register(trig_info->swt.trigger);
> + if (ret)
> + goto err_free_trigger;
> +
> + iio_swt_group_init_type_name(&trig_info->swt, name, &iio_hrtimer_type);
> + return &trig_info->swt;
> +err_free_trigger:
> + iio_trigger_free(trig_info->swt.trigger);
> +err_free_trig_info:
> + kfree(trig_info);
> +
> + return ERR_PTR(ret);
> +}
> +
> +static int iio_trig_hrtimer_remove(struct iio_sw_trigger *swt)
> +{
> + struct iio_hrtimer_info *trig_info;
> +
> + trig_info = iio_trigger_get_drvdata(swt->trigger);
> +
> + iio_trigger_unregister(swt->trigger);
> +
> + /* cancel the timer after unreg to make sure no one rearms it */
> + hrtimer_cancel(&trig_info->timer);
> + iio_trigger_free(swt->trigger);
> + kfree(trig_info);
> +
> + return 0;
> +}
> +
> +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 <[email protected]>");
> +MODULE_AUTHOR("Daniel Baluta <[email protected]>");
> +MODULE_DESCRIPTION("Periodic hrtimer trigger for the IIO subsystem");
> +MODULE_LICENSE("GPL v2");

2015-08-17 12:49:49

by Daniel Baluta

[permalink] [raw]
Subject: Re: [PATCH v7 4/5] iio: trigger: Introduce IIO hrtimer based trigger

On Mon, Aug 17, 2015 at 3:45 PM, Vladimir Barinov
<[email protected]> wrote:
> Hello Daniel,
>
> I've verified that your hrtimer trigger works.
>
> I have minor suggestions.
>
> Will not it be useful to have ability to stop/start hrtimer polling
> not only during attach/detach trigger.
>
> f.e. writing 0 to sampling_frequency will hrtimer_cancel
> and writing any valid frequency will hrtimer_start.

Thanks a lot Vladimir for looking at this. I will try to address your comments
this week.

Daniel.

2015-08-31 14:42:06

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH v7 3/5] iio: core: Introduce IIO software triggers

On 08/11/2015 12:42 AM, 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 <[email protected]>

Sorry for the delay.

[...]
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index dbf5f9a..31aead3 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-configfs.c b/drivers/iio/industrialio-configfs.c
> index 3edee90..ced7115 100644
> --- a/drivers/iio/industrialio-configfs.c
> +++ b/drivers/iio/industrialio-configfs.c
> @@ -15,6 +15,40 @@
> #include <linux/slab.h>
>
> #include <linux/iio/iio.h>
> +#include <linux/iio/sw_trigger.h>
> +
> +static struct config_group *trigger_make_group(struct config_group *group,
> + const char *name)
> +{
> + struct iio_sw_trigger *t;
> +
> + t = iio_sw_trigger_create(group->cg_item.ci_name, name);
> + if (IS_ERR(t))
> + return ERR_CAST(t);
> +
> + config_item_set_name(&t->group.cg_item, name);

config_item_set_name() takes a format string, since name is user supplied
this should be

config_item_set_name(&t->group.cg_item, "%s", name);

> +
> + return &t->group;
> +}
> +
> +static void trigger_drop_group(struct config_group *group,
> + struct config_item *item)
> +{
> + struct iio_sw_trigger *t = to_iio_sw_trigger(item);
> +
> + iio_sw_trigger_destroy(t);
> + config_item_put(item);
> +}

I get compile errors when I build this without software trigger support enabled:

drivers/built-in.o: In function `trigger_drop_group':
/opt/linux/drivers/iio/industrialio-configfs.c:39: undefined reference to
`iio_sw_trigger_destroy'
drivers/built-in.o: In function `trigger_make_group':
/opt/linux/drivers/iio/industrialio-configfs.c:25: undefined reference to
`iio_sw_trigger_create'

Since it is bool ifdefing it out should work. But I wonder if it shouldn't
be the other way around and this should go into the sw-trigger module?

[...]
> diff --git a/drivers/iio/industrialio-sw-trigger.c b/drivers/iio/industrialio-sw-trigger.c
> new file mode 100644
> index 0000000..761418e
> --- /dev/null
> +++ b/drivers/iio/industrialio-sw-trigger.c
> @@ -0,0 +1,119 @@
> +/*
> + * 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_MUTEX(iio_trigger_types_lock);
> +
> +static
> +struct iio_sw_trigger_type *__iio_find_sw_trigger_type(const char *name,
> + unsigned len)
> +{
> + struct iio_sw_trigger_type *t = NULL, *iter;

Maybe add a lockdep_assert() here to make it explicit that this requires the
types_lock.

> +
> + list_for_each_entry(iter, &iio_trigger_types_list, list)
> + if (!strncmp(iter->name, name, len)) {

What's the reason for doing a prefix match? E.g. if name is "h" this will
return the "hrtimer" trigger.

> + t = iter;
> + break;
> + }
> +
> + return t;
> +}
> +
> +int iio_register_sw_trigger_type(struct iio_sw_trigger_type *t)
> +{
> + struct iio_sw_trigger_type *iter;
> + int ret = 0;
> +
> + mutex_lock(&iio_trigger_types_lock);
> + iter = __iio_find_sw_trigger_type(t->name, strlen(t->name));
> + if (iter) {
> + ret = -EBUSY;
> + } else {
> + list_add_tail(&t->list, &iio_trigger_types_list);
> + iio_sw_trigger_type_configfs_register(t);
> + }
> + mutex_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)

unregister should be void. There isn't too much that can go wrong here. If
the type is registered it will be removed, if it isn't registered there is
nothing to remove and jobs already done.

The problem with having a return value of unregister function is always
where to propagate the value to. And what to do if unregister fails of one
of many in a sequence of unregister calls in case the module registered
multiple things.


> +{
> + struct iio_sw_trigger_type *iter;
> + int ret = 0;
> +
> + mutex_lock(&iio_trigger_types_lock);
> + iter = __iio_find_sw_trigger_type(t->name, strlen(t->name));
> + if (!iter) {
> + ret = -EINVAL;
> + } else {
> + iio_sw_trigger_type_configfs_unregister(t);
> + list_del(&t->list);
> + }
> + mutex_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;
> +
> + mutex_lock(&iio_trigger_types_lock);
> + t = __iio_find_sw_trigger_type(name, strlen(name));
> + if (t && !try_module_get(t->owner))
> + t = NULL;
> + mutex_unlock(&iio_trigger_types_lock);
> +
> + return t;
> +}
> +
> +struct iio_sw_trigger *iio_sw_trigger_create(const char *type, const char *name)
> +{
> + struct iio_sw_trigger *t;
> + struct iio_sw_trigger_type *tt;
> +
> + tt = iio_get_sw_trigger_type(type);
> + if (!tt) {
> + pr_err("Invalid trigger type: %s\n", type);
> + return ERR_PTR(-EINVAL);
> + }
> + t = tt->ops->probe(name);
> + if (IS_ERR(t))
> + goto out_module_put;
> +
> + t->trigger_type = tt;
> +
> + return t;
> +out_module_put:
> + module_put(tt->owner);
> + return t;
> +}
> +EXPORT_SYMBOL(iio_sw_trigger_create);
> +
> +void iio_sw_trigger_destroy(struct iio_sw_trigger *t)
> +{
> + struct iio_sw_trigger_type *tt = t->trigger_type;
> +
> + tt->ops->remove(t);
> + module_put(tt->owner);
> +}
> +EXPORT_SYMBOL(iio_sw_trigger_destroy);

2015-08-31 14:57:18

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH v7 4/5] iio: trigger: Introduce IIO hrtimer based trigger

On 08/17/2015 02:45 PM, Vladimir Barinov wrote:
> Hello Daniel,
>
> I've verified that your hrtimer trigger works.
>
> I have minor suggestions.
>
> Will not it be useful to have ability to stop/start hrtimer polling
> not only during attach/detach trigger.
>
> f.e. writing 0 to sampling_frequency will hrtimer_cancel
> and writing any valid frequency will hrtimer_start.

What's the use case for this?

2015-08-31 15:13:18

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH v7 4/5] iio: trigger: Introduce IIO hrtimer based trigger

On 08/11/2015 12:42 AM, Daniel Baluta wrote:
[...]
> +
> +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);

I think it makes sense to move the register and unregister to the swt core.
This is something you'd always want to do as the last step in the probe and
the first step in the remove function.

> + if (ret)
> + goto err_free_trigger;
> +
> + iio_swt_group_init_type_name(&trig_info->swt, name, &iio_hrtimer_type);
> + return &trig_info->swt;
> +err_free_trigger:
> + iio_trigger_free(trig_info->swt.trigger);
> +err_free_trig_info:
> + kfree(trig_info);
> +
> + return ERR_PTR(ret);
> +}
> +
> +static int iio_trig_hrtimer_remove(struct iio_sw_trigger *swt)
> +{
> + struct iio_hrtimer_info *trig_info;
> +
> + trig_info = iio_trigger_get_drvdata(swt->trigger);
> +
> + iio_trigger_unregister(swt->trigger);
> +
> + /* cancel the timer after unreg to make sure no one rearms it */
> + hrtimer_cancel(&trig_info->timer);
> + iio_trigger_free(swt->trigger);
> + kfree(trig_info);
> +
> + return 0;
> +}
> +
> +const struct iio_sw_trigger_ops iio_trig_hrtimer_ops = {

static

> + .probe = iio_trig_hrtimer_probe,
> + .remove = iio_trig_hrtimer_remove,
> +};
> +
> +struct iio_sw_trigger_type iio_trig_hrtimer = {

static

> + .name = "hrtimer",
> + .owner = THIS_MODULE,
> + .ops = &iio_trig_hrtimer_ops,
> +};
> +
> +module_iio_sw_trigger_driver(iio_trig_hrtimer);
> +
> +MODULE_AUTHOR("Marten Svanfeldt <[email protected]>");
> +MODULE_AUTHOR("Daniel Baluta <[email protected]>");
> +MODULE_DESCRIPTION("Periodic hrtimer trigger for the IIO subsystem");
> +MODULE_LICENSE("GPL v2");
>

2015-08-31 16:02:52

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH v7 1/5] configfs: Allow dynamic group (un)registration

[...]
> +int configfs_register_group(struct config_group *parent_group,
> + struct config_group *group)
> +{
> + struct dentry *dentry;
> + int ret;
> +
> + link_group(parent_group, group);

This needs to hold subsys->su_mutex

I think from here to before the return we need to hold the parent dentry
mutex with I_MUTEX_PARENT

> +
> + ret = create_default_group(parent_group, group, &dentry);
> + if (ret == 0)
> + configfs_dir_set_ready(dentry->d_fsdata);

This needs to hold configfs_dirent_lock

> +
> + return ret;
> +}
> +EXPORT_SYMBOL(configfs_register_group);
> +
> +void configfs_unregister_group(struct config_group *group)
> +{
> + struct dentry *dentry = group->cg_item.ci_dentry;
> +

We probably need to do the detach_prep stuff somewhere in here.


I think this needs to hold the parent dentry mutex with I_MUTEX_PARENT from
here to ...

> + mutex_lock(&d_inode(dentry)->i_mutex);
> + configfs_detach_group(&group->cg_item);
> + d_inode(dentry)->i_flags |= S_DEAD;
> + dont_mount(dentry);
> + mutex_unlock(&d_inode(dentry)->i_mutex);
> +
> + d_delete(dentry);

... here

> + dput(dentry);
> + unlink_group(group);

needs subsys->su_mutex

> +}
> +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 c9e5c57..726980e 100644
> --- a/include/linux/configfs.h
> +++ b/include/linux/configfs.h
> @@ -251,6 +251,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);
>