Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755157AbbHQLcA (ORCPT ); Mon, 17 Aug 2015 07:32:00 -0400 Received: from mail-wi0-f169.google.com ([209.85.212.169]:36941 "EHLO mail-wi0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752600AbbHQLb5 (ORCPT ); Mon, 17 Aug 2015 07:31:57 -0400 Message-ID: <55D1C629.3090103@cogentembedded.com> Date: Mon, 17 Aug 2015 14:31:53 +0300 From: Vladimir Barinov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.8.0 MIME-Version: 1.0 To: Daniel Baluta , jic23@kernel.org, jlbec@evilplan.org, linux-iio@vger.kernel.org, linux-fsdevel@vger.kernel.org CC: lars@metafoo.de, knaack.h@gmx.de, linux-kernel@vger.kernel.org, octavian.purdila@intel.com, pebolle@tiscali.nl, patrick.porlan@intel.com, adriana.reus@intel.com, constantin.musca@intel.com, marten@intuitiveaerial.com, cristina.opriceana@gmail.com, pmeerw@pmeerw.net, hch@lst.de, viro@zeniv.linux.org.uk, akpm@linux-foundation.org Subject: Re: [PATCH v7 3/5] iio: core: Introduce IIO software triggers References: <1439246562-17515-1-git-send-email-daniel.baluta@intel.com> <1439246562-17515-4-git-send-email-daniel.baluta@intel.com> In-Reply-To: <1439246562-17515-4-git-send-email-daniel.baluta@intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 11448 Lines: 376 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 > --- > 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 > > #include > +#include > + > +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 > +#include > +#include > +#include > +#include > + > +#include > + > +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 > +#include > +#include > +#include > + > +#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 */ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/