Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754479AbbEMLOL (ORCPT ); Wed, 13 May 2015 07:14:11 -0400 Received: from smtp-out-163.synserver.de ([212.40.185.163]:1065 "EHLO smtp-out-163.synserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751418AbbEMLOI (ORCPT ); Wed, 13 May 2015 07:14:08 -0400 X-SynServer-TrustedSrc: 1 X-SynServer-AuthUser: lars@metafoo.de X-SynServer-PPID: 11248 From: Lars-Peter Clausen To: Daniel Baluta , Jonathan Cameron Cc: Joel Becker , Hartmut Knaack , Linux Kernel Mailing List , "linux-iio@vger.kernel.org" , Paul Bolle , constantin.musca@intel.com, marten@intuitiveaerial.com, Lars-Peter Clausen Subject: Re: [PATCH v6 0/4] Add initial configfs support for IIO Date: Wed, 13 May 2015 13:14:01 +0200 Message-Id: <1431515641-19984-1-git-send-email-lars@metafoo.de> X-Mailer: git-send-email 1.8.0 In-Reply-To: References: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8472 Lines: 264 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 - */ - type_name = buf; - trigger_name = strchr(buf, '-'); - if (!trigger_name) { - WARN_ONCE(1, "Unable to locate '-' in %s. Use -.\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 -- 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/