[PATCH v2 00/14] staging: most: switch to configfs

[PATCH v2 00/14] staging: most: switch to configfs

[Christian Gromm]

This patch set makes the driver provide its configuration interface via
configfs. The configuration interface is being switched to simplify the
process of setting up the driver and to introduce the new feature of
speculative configuration.

v2:

Christian Gromm (14):
  staging: most: add new file configfs.c
  staging: most: change signature of function probe_channel
  staging: most: core: add configfs interface functions
  staging: most: sound: introduce new sound adapter management
  staging: most: enable configfs support
  staging: most: core: make sysfs attributes read-only
  staging: most: core: use device description as name
  staging: most: usb: remove prefix from description tag
  staging: most: core: remove attribute add_link
  staging: most: allow speculative configuration
  staging: most: configfs: make create attributes write-only
  staging: most: configfs: add code for link removal
  staging: most: configfs: rename config attributes
  staging: most: Documentation: update driver documentation

 .../most/Documentation/ABI/configfs-most.txt       | 204 +++++++
 .../staging/most/Documentation/driver_usage.txt    | 131 ++--
 drivers/staging/most/Makefile                      |   1 +
 drivers/staging/most/cdev/cdev.c                   |   8 +-
 drivers/staging/most/configfs.c                    | 676 +++++++++++++++++++++
 drivers/staging/most/core.c                        | 305 +++++-----
 drivers/staging/most/core.h                        |  20 +-
 drivers/staging/most/net/net.c                     |   3 +-
 drivers/staging/most/sound/sound.c                 |  59 +-
 drivers/staging/most/usb/usb.c                     |   2 +-
 drivers/staging/most/video/video.c                 |   3 +-
 11 files changed, 1164 insertions(+), 248 deletions(-)
 create mode 100644 drivers/staging/most/Documentation/ABI/configfs-most.txt
 create mode 100644 drivers/staging/most/configfs.c

-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 01/14] staging: most: add new file configfs.c

[Christian Gromm]

This patch adds the file configfs.c to the driver directory. The file
registers the necessary subsystems with configfs in order to move the
driver configuration from sysfs to configfs.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
v2:
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
	- remove unnecessary variable init
	- replace strcmp w/ sysfs_streq
	- prefer snprintf over sprintf
	- fix array size
	- remove empty exit function
	- use bool type for create attributes and kstrtobool function
	- remove redundant local variables
	- remove inline keyword
	- remove check for NULL pointers for struct config_item


 drivers/staging/most/configfs.c | 623 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 623 insertions(+)
 create mode 100644 drivers/staging/most/configfs.c

diff --git a/drivers/staging/most/configfs.c b/drivers/staging/most/configfs.c
new file mode 100644
index 0000000..6968b299
--- /dev/null
+++ b/drivers/staging/most/configfs.c
@@ -0,0 +1,623 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * configfs.c - Implementation of configfs interface to the driver stack
+ *
+ * Copyright (C) 2013-2015 Microchip Technology Germany II GmbH & Co. KG
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/init.h>
+#include <linux/configfs.h>
+#include <most/core.h>
+
+struct mdev_link {
+	struct config_item item;
+	bool create;
+	u16 num_buffers;
+	u16 buffer_size;
+	u16 subbuffer_size;
+	u16 packets_per_xact;
+	u16 dbr_size;
+	char datatype[PAGE_SIZE];
+	char direction[PAGE_SIZE];
+	char name[PAGE_SIZE];
+	char mdev[PAGE_SIZE];
+	char channel[PAGE_SIZE];
+	char comp[PAGE_SIZE];
+	char param[PAGE_SIZE];
+};
+
+static int set_cfg_buffer_size(struct mdev_link *link)
+{
+	if (!link->buffer_size)
+		return -ENODATA;
+	return most_set_cfg_buffer_size(link->mdev, link->channel,
+					link->buffer_size);
+}
+
+static int set_cfg_subbuffer_size(struct mdev_link *link)
+{
+	if (!link->subbuffer_size)
+		return -ENODATA;
+	return most_set_cfg_subbuffer_size(link->mdev, link->channel,
+					   link->subbuffer_size);
+}
+
+static int set_cfg_dbr_size(struct mdev_link *link)
+{
+	if (!link->dbr_size)
+		return -ENODATA;
+	return most_set_cfg_dbr_size(link->mdev, link->channel,
+				     link->dbr_size);
+}
+
+static int set_cfg_num_buffers(struct mdev_link *link)
+{
+	if (!link->num_buffers)
+		return -ENODATA;
+	return most_set_cfg_num_buffers(link->mdev, link->channel,
+					link->num_buffers);
+}
+
+static int set_cfg_packets_xact(struct mdev_link *link)
+{
+	if (!link->packets_per_xact)
+		return -ENODATA;
+	return most_set_cfg_packets_xact(link->mdev, link->channel,
+					 link->packets_per_xact);
+}
+
+static int set_cfg_direction(struct mdev_link *link)
+{
+	if (!strlen(link->direction))
+		return -ENODATA;
+	return most_set_cfg_direction(link->mdev, link->channel,
+				      link->direction);
+}
+
+static int set_cfg_datatype(struct mdev_link *link)
+{
+	if (!strlen(link->datatype))
+		return -ENODATA;
+	return most_set_cfg_datatype(link->mdev, link->channel,
+				     link->datatype);
+}
+
+static int (*set_config_val[])(struct mdev_link *link) = {
+	set_cfg_buffer_size,
+	set_cfg_subbuffer_size,
+	set_cfg_dbr_size,
+	set_cfg_num_buffers,
+	set_cfg_packets_xact,
+	set_cfg_direction,
+	set_cfg_datatype,
+};
+
+static struct mdev_link *to_mdev_link(struct config_item *item)
+{
+	return container_of(item, struct mdev_link, item);
+}
+
+static ssize_t mdev_link_create_show(struct config_item *item, char *page)
+{
+	return snprintf(page, PAGE_SIZE, "%d\n", to_mdev_link(item)->create);
+}
+
+static ssize_t mdev_link_create_store(struct config_item *item,
+				      const char *page, size_t count)
+{
+	struct mdev_link *mdev_link = to_mdev_link(item);
+	bool tmp;
+	int ret;
+	int i;
+
+	ret = kstrtobool(page, &tmp);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < ARRAY_SIZE(set_config_val); i++) {
+		ret = set_config_val[i](mdev_link);
+		if (ret < 0)
+			return ret;
+	}
+
+	if (tmp)
+		ret = most_add_link(mdev_link->mdev, mdev_link->channel,
+				    mdev_link->comp, mdev_link->name,
+				    mdev_link->param);
+	else
+		ret = most_remove_link(mdev_link->mdev, mdev_link->channel,
+				       mdev_link->comp);
+	if (ret)
+		return ret;
+	mdev_link->create = tmp;
+	return count;
+}
+
+static ssize_t mdev_link_direction_show(struct config_item *item, char *page)
+{
+	return snprintf(page, PAGE_SIZE, "%s\n", to_mdev_link(item)->direction);
+}
+
+static ssize_t mdev_link_direction_store(struct config_item *item,
+					 const char *page, size_t count)
+{
+	struct mdev_link *mdev_link = to_mdev_link(item);
+
+	if (sysfs_streq(page, "dir_rx") && sysfs_streq(page, "rx") &&
+	    sysfs_streq(page, "dir_tx") && sysfs_streq(page, "tx"))
+		return -EINVAL;
+	strcpy(mdev_link->direction, page);
+	return count;
+}
+
+static ssize_t mdev_link_datatype_show(struct config_item *item, char *page)
+{
+	return snprintf(page, PAGE_SIZE, "%s\n", to_mdev_link(item)->datatype);
+}
+
+static ssize_t mdev_link_datatype_store(struct config_item *item,
+					const char *page, size_t count)
+{
+	struct mdev_link *mdev_link = to_mdev_link(item);
+
+	if (sysfs_streq(page, "control") && sysfs_streq(page, "async") &&
+	    sysfs_streq(page, "sync") && sysfs_streq(page, "isoc") &&
+	    sysfs_streq(page, "isoc_avp"))
+		return -EINVAL;
+	strcpy(mdev_link->datatype, page);
+	return count;
+}
+
+static ssize_t mdev_link_mdev_show(struct config_item *item, char *page)
+{
+	return snprintf(page, PAGE_SIZE, "%s\n", to_mdev_link(item)->mdev);
+}
+
+static ssize_t mdev_link_mdev_store(struct config_item *item,
+				    const char *page, size_t count)
+{
+	struct mdev_link *mdev_link = to_mdev_link(item);
+
+	strcpy(mdev_link->mdev, page);
+	return count;
+}
+
+static ssize_t mdev_link_channel_show(struct config_item *item, char *page)
+{
+	return snprintf(page, PAGE_SIZE, "%s\n", to_mdev_link(item)->channel);
+}
+
+static ssize_t mdev_link_channel_store(struct config_item *item,
+				       const char *page, size_t count)
+{
+	struct mdev_link *mdev_link = to_mdev_link(item);
+
+	strcpy(mdev_link->channel, page);
+	return count;
+}
+
+static ssize_t mdev_link_comp_show(struct config_item *item, char *page)
+{
+	return snprintf(page, PAGE_SIZE, "%s\n", to_mdev_link(item)->comp);
+}
+
+static ssize_t mdev_link_comp_store(struct config_item *item,
+				    const char *page, size_t count)
+{
+	struct mdev_link *mdev_link = to_mdev_link(item);
+
+	strcpy(mdev_link->comp, page);
+	return count;
+}
+
+static ssize_t mdev_link_param_show(struct config_item *item, char *page)
+{
+	return snprintf(page, PAGE_SIZE, "%s\n", to_mdev_link(item)->param);
+}
+
+static ssize_t mdev_link_param_store(struct config_item *item,
+				     const char *page, size_t count)
+{
+	struct mdev_link *mdev_link = to_mdev_link(item);
+
+	strcpy(mdev_link->param, page);
+	return count;
+}
+
+static ssize_t mdev_link_num_buffers_show(struct config_item *item, char *page)
+{
+	return snprintf(page, PAGE_SIZE, "%d\n",
+			to_mdev_link(item)->num_buffers);
+}
+
+static ssize_t mdev_link_num_buffers_store(struct config_item *item,
+					   const char *page, size_t count)
+{
+	struct mdev_link *mdev_link = to_mdev_link(item);
+	int ret;
+
+	ret = kstrtou16(page, 0, &mdev_link->num_buffers);
+	if (ret)
+		return ret;
+	return count;
+}
+
+static ssize_t mdev_link_buffer_size_show(struct config_item *item, char *page)
+{
+	return snprintf(page, PAGE_SIZE, "%d\n",
+			to_mdev_link(item)->buffer_size);
+}
+
+static ssize_t mdev_link_buffer_size_store(struct config_item *item,
+					   const char *page, size_t count)
+{
+	struct mdev_link *mdev_link = to_mdev_link(item);
+	int ret;
+
+	ret = kstrtou16(page, 0, &mdev_link->buffer_size);
+	if (ret)
+		return ret;
+	return count;
+}
+
+static ssize_t mdev_link_subbuffer_size_show(struct config_item *item,
+					     char *page)
+{
+	return snprintf(page, PAGE_SIZE, "%d\n",
+			to_mdev_link(item)->subbuffer_size);
+}
+
+static ssize_t mdev_link_subbuffer_size_store(struct config_item *item,
+					      const char *page, size_t count)
+{
+	struct mdev_link *mdev_link = to_mdev_link(item);
+	int ret;
+
+	ret = kstrtou16(page, 0, &mdev_link->subbuffer_size);
+	if (ret)
+		return ret;
+	return count;
+}
+
+static ssize_t mdev_link_packets_per_xact_show(struct config_item *item,
+					       char *page)
+{
+	return snprintf(page, PAGE_SIZE, "%d\n",
+			to_mdev_link(item)->packets_per_xact);
+}
+
+static ssize_t mdev_link_packets_per_xact_store(struct config_item *item,
+						const char *page, size_t count)
+{
+	struct mdev_link *mdev_link = to_mdev_link(item);
+	int ret;
+
+	ret = kstrtou16(page, 0, &mdev_link->packets_per_xact);
+	if (ret)
+		return ret;
+	return count;
+}
+
+static ssize_t mdev_link_dbr_size_show(struct config_item *item, char *page)
+{
+	return snprintf(page, PAGE_SIZE, "%d\n", to_mdev_link(item)->dbr_size);
+}
+
+static ssize_t mdev_link_dbr_size_store(struct config_item *item,
+					const char *page, size_t count)
+{
+	struct mdev_link *mdev_link = to_mdev_link(item);
+	int ret;
+
+	ret = kstrtou16(page, 0, &mdev_link->dbr_size);
+	if (ret)
+		return ret;
+	return count;
+}
+
+CONFIGFS_ATTR(mdev_link_, create);
+CONFIGFS_ATTR(mdev_link_, mdev);
+CONFIGFS_ATTR(mdev_link_, channel);
+CONFIGFS_ATTR(mdev_link_, comp);
+CONFIGFS_ATTR(mdev_link_, param);
+CONFIGFS_ATTR(mdev_link_, num_buffers);
+CONFIGFS_ATTR(mdev_link_, buffer_size);
+CONFIGFS_ATTR(mdev_link_, subbuffer_size);
+CONFIGFS_ATTR(mdev_link_, packets_per_xact);
+CONFIGFS_ATTR(mdev_link_, datatype);
+CONFIGFS_ATTR(mdev_link_, direction);
+CONFIGFS_ATTR(mdev_link_, dbr_size);
+
+static struct configfs_attribute *mdev_link_attrs[] = {
+	&mdev_link_attr_create,
+	&mdev_link_attr_mdev,
+	&mdev_link_attr_channel,
+	&mdev_link_attr_comp,
+	&mdev_link_attr_param,
+	&mdev_link_attr_num_buffers,
+	&mdev_link_attr_buffer_size,
+	&mdev_link_attr_subbuffer_size,
+	&mdev_link_attr_packets_per_xact,
+	&mdev_link_attr_datatype,
+	&mdev_link_attr_direction,
+	&mdev_link_attr_dbr_size,
+	NULL,
+};
+
+static void mdev_link_release(struct config_item *item)
+{
+	kfree(to_mdev_link(item));
+}
+
+static struct configfs_item_operations mdev_link_item_ops = {
+	.release		= mdev_link_release,
+};
+
+static const struct config_item_type mdev_link_type = {
+	.ct_item_ops	= &mdev_link_item_ops,
+	.ct_attrs	= mdev_link_attrs,
+	.ct_owner	= THIS_MODULE,
+};
+
+struct most_common {
+	struct config_group group;
+};
+
+static struct most_common *to_most_common(struct config_item *item)
+{
+	return container_of(to_config_group(item), struct most_common, group);
+}
+
+static struct config_item *most_common_make_item(struct config_group *group,
+						 const char *name)
+{
+	struct mdev_link *mdev_link;
+
+	mdev_link = kzalloc(sizeof(*mdev_link), GFP_KERNEL);
+	if (!mdev_link)
+		return ERR_PTR(-ENOMEM);
+
+	config_item_init_type_name(&mdev_link->item, name,
+				   &mdev_link_type);
+
+	if (!strcmp(group->cg_item.ci_namebuf, "most_cdev"))
+		strcpy(mdev_link->comp, "cdev");
+	else if (!strcmp(group->cg_item.ci_namebuf, "most_net"))
+		strcpy(mdev_link->comp, "net");
+	else if (!strcmp(group->cg_item.ci_namebuf, "most_video"))
+		strcpy(mdev_link->comp, "video");
+	strcpy(mdev_link->name, name);
+	return &mdev_link->item;
+}
+
+static void most_common_release(struct config_item *item)
+{
+	kfree(to_most_common(item));
+}
+
+static struct configfs_item_operations most_common_item_ops = {
+	.release	= most_common_release,
+};
+
+static struct configfs_group_operations most_common_group_ops = {
+	.make_item	= most_common_make_item,
+};
+
+static const struct config_item_type most_common_type = {
+	.ct_item_ops	= &most_common_item_ops,
+	.ct_group_ops	= &most_common_group_ops,
+	.ct_owner	= THIS_MODULE,
+};
+
+static struct configfs_subsystem most_cdev_subsys = {
+	.su_group = {
+		.cg_item = {
+			.ci_namebuf = "most_cdev",
+			.ci_type = &most_common_type,
+		},
+	},
+};
+
+static struct configfs_subsystem most_net_subsys = {
+	.su_group = {
+		.cg_item = {
+			.ci_namebuf = "most_net",
+			.ci_type = &most_common_type,
+		},
+	},
+};
+
+static struct configfs_subsystem most_video_subsys = {
+	.su_group = {
+		.cg_item = {
+			.ci_namebuf = "most_video",
+			.ci_type = &most_common_type,
+		},
+	},
+};
+
+struct most_snd_grp {
+	struct config_group group;
+	bool create;
+	struct list_head list;
+};
+
+static struct most_snd_grp *to_most_snd_grp(struct config_item *item)
+{
+	return container_of(to_config_group(item), struct most_snd_grp, group);
+}
+
+static struct config_item *most_snd_grp_make_item(struct config_group *group,
+						  const char *name)
+{
+	struct mdev_link *mdev_link;
+
+	mdev_link = kzalloc(sizeof(*mdev_link), GFP_KERNEL);
+	if (!mdev_link)
+		return ERR_PTR(-ENOMEM);
+
+	config_item_init_type_name(&mdev_link->item, name, &mdev_link_type);
+	mdev_link->create = 0;
+	strcpy(mdev_link->name, name);
+	strcpy(mdev_link->comp, "sound");
+	return &mdev_link->item;
+}
+
+static ssize_t most_snd_grp_create_show(struct config_item *item, char *page)
+{
+	return snprintf(page, PAGE_SIZE, "%d\n", to_most_snd_grp(item)->create);
+}
+
+static ssize_t most_snd_grp_create_store(struct config_item *item,
+					 const char *page, size_t count)
+{
+	struct most_snd_grp *snd_grp = to_most_snd_grp(item);
+	int ret;
+	bool tmp;
+
+	ret = kstrtobool(page, &tmp);
+	if (ret)
+		return ret;
+	if (tmp) {
+		ret = most_cfg_complete("sound");
+		if (ret)
+			return ret;
+	}
+	snd_grp->create = tmp;
+	return count;
+}
+
+CONFIGFS_ATTR(most_snd_grp_, create);
+
+static struct configfs_attribute *most_snd_grp_attrs[] = {
+	&most_snd_grp_attr_create,
+	NULL,
+};
+
+static void most_snd_grp_release(struct config_item *item)
+{
+	struct most_snd_grp *group = to_most_snd_grp(item);
+
+	list_del(&group->list);
+	kfree(group);
+}
+
+static struct configfs_item_operations most_snd_grp_item_ops = {
+	.release	= most_snd_grp_release,
+};
+
+static struct configfs_group_operations most_snd_grp_group_ops = {
+	.make_item	= most_snd_grp_make_item,
+};
+
+static const struct config_item_type most_snd_grp_type = {
+	.ct_item_ops	= &most_snd_grp_item_ops,
+	.ct_group_ops	= &most_snd_grp_group_ops,
+	.ct_attrs	= most_snd_grp_attrs,
+	.ct_owner	= THIS_MODULE,
+};
+
+struct most_sound {
+	struct configfs_subsystem subsys;
+	struct list_head soundcard_list;
+};
+
+static struct config_group *most_sound_make_group(struct config_group *group,
+						  const char *name)
+{
+	struct most_snd_grp *most;
+	struct most_sound *ms = container_of(to_configfs_subsystem(group),
+					     struct most_sound, subsys);
+
+	list_for_each_entry(most, &ms->soundcard_list, list) {
+		if (!most->create) {
+			pr_info("adapter configuration still in progress.\n");
+			return ERR_PTR(-EPROTO);
+		}
+	}
+	most = kzalloc(sizeof(*most), GFP_KERNEL);
+	if (!most)
+		return ERR_PTR(-ENOMEM);
+
+	config_group_init_type_name(&most->group, name, &most_snd_grp_type);
+	list_add_tail(&most->list, &ms->soundcard_list);
+	return &most->group;
+}
+
+static struct configfs_group_operations most_sound_group_ops = {
+	.make_group	= most_sound_make_group,
+};
+
+static const struct config_item_type most_sound_type = {
+	.ct_group_ops	= &most_sound_group_ops,
+	.ct_owner	= THIS_MODULE,
+};
+
+static struct most_sound most_sound_subsys = {
+	.subsys = {
+		.su_group = {
+			.cg_item = {
+				.ci_namebuf = "most_sound",
+				.ci_type = &most_sound_type,
+			},
+		},
+	},
+};
+
+int most_register_configfs_subsys(struct core_component *c)
+{
+	int ret;
+
+	if (!strcmp(c->name, "cdev"))
+		ret = configfs_register_subsystem(&most_cdev_subsys);
+	else if (!strcmp(c->name, "net"))
+		ret = configfs_register_subsystem(&most_net_subsys);
+	else if (!strcmp(c->name, "video"))
+		ret = configfs_register_subsystem(&most_video_subsys);
+	else if (!strcmp(c->name, "sound"))
+		ret = configfs_register_subsystem(&most_sound_subsys.subsys);
+	else
+		return -ENODEV;
+
+	if (ret) {
+		pr_err("Error %d while registering subsystem %s\n",
+		       ret, c->name);
+	}
+	return ret;
+}
+EXPORT_SYMBOL_GPL(most_register_configfs_subsys);
+
+void most_deregister_configfs_subsys(struct core_component *c)
+{
+	if (!strcmp(c->name, "cdev"))
+		configfs_unregister_subsystem(&most_cdev_subsys);
+	else if (!strcmp(c->name, "net"))
+		configfs_unregister_subsystem(&most_net_subsys);
+	else if (!strcmp(c->name, "video"))
+		configfs_unregister_subsystem(&most_video_subsys);
+	else if (!strcmp(c->name, "sound"))
+		configfs_unregister_subsystem(&most_sound_subsys.subsys);
+}
+EXPORT_SYMBOL_GPL(most_deregister_configfs_subsys);
+
+int __init configfs_init(void)
+{
+	config_group_init(&most_cdev_subsys.su_group);
+	mutex_init(&most_cdev_subsys.su_mutex);
+
+	config_group_init(&most_net_subsys.su_group);
+	mutex_init(&most_net_subsys.su_mutex);
+
+	config_group_init(&most_video_subsys.su_group);
+	mutex_init(&most_video_subsys.su_mutex);
+
+	config_group_init(&most_sound_subsys.subsys.su_group);
+	mutex_init(&most_sound_subsys.subsys.su_mutex);
+
+	INIT_LIST_HEAD(&most_sound_subsys.soundcard_list);
+
+	return 0;
+}
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 02/14] staging: most: change signature of function probe_channel

[Christian Gromm]

This patch adds the param argument to the function parameter of
the call-back probe_channel. This parameter is needed to configure
the channels of an attached device.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
v2:

 drivers/staging/most/cdev/cdev.c   | 2 +-
 drivers/staging/most/core.c        | 6 ++++--
 drivers/staging/most/core.h        | 3 ++-
 drivers/staging/most/net/net.c     | 3 ++-
 drivers/staging/most/sound/sound.c | 3 +--
 drivers/staging/most/video/video.c | 3 ++-
 6 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/most/cdev/cdev.c b/drivers/staging/most/cdev/cdev.c
index f2b347c..97408ec 100644
--- a/drivers/staging/most/cdev/cdev.c
+++ b/drivers/staging/most/cdev/cdev.c
@@ -425,7 +425,7 @@ static int comp_tx_completion(struct most_interface *iface, int channel_id)
  * Returns 0 on success or error code otherwise.
  */
 static int comp_probe(struct most_interface *iface, int channel_id,
-		      struct most_channel_config *cfg, char *name)
+		      struct most_channel_config *cfg, char *name, char *args)
 {
 	struct comp_channel *c;
 	unsigned long cl_flags;
diff --git a/drivers/staging/most/core.c b/drivers/staging/most/core.c
index 18936cd..fb19b63 100644
--- a/drivers/staging/most/core.c
+++ b/drivers/staging/most/core.c
@@ -701,6 +701,7 @@ static struct most_channel *get_channel(char *mdev, char *mdev_ch)
 static
 inline int link_channel_to_component(struct most_channel *c,
 				     struct core_component *comp,
+				     char *name,
 				     char *comp_param)
 {
 	int ret;
@@ -714,7 +715,8 @@ inline int link_channel_to_component(struct most_channel *c,
 		return -ENOSPC;
 
 	*comp_ptr = comp;
-	ret = comp->probe_channel(c->iface, c->channel_id, &c->cfg, comp_param);
+	ret = comp->probe_channel(c->iface, c->channel_id, &c->cfg, name,
+				  comp_param);
 	if (ret) {
 		*comp_ptr = NULL;
 		return ret;
@@ -775,7 +777,7 @@ static ssize_t add_link_store(struct device_driver *drv,
 	if (!c)
 		return -ENODEV;
 
-	ret = link_channel_to_component(c, comp, comp_param);
+	ret = link_channel_to_component(c, comp, "name", comp_param);
 	if (ret)
 		return ret;
 	return len;
diff --git a/drivers/staging/most/core.h b/drivers/staging/most/core.h
index 64cc02f..025dd1d 100644
--- a/drivers/staging/most/core.h
+++ b/drivers/staging/most/core.h
@@ -266,7 +266,8 @@ struct core_component {
 	struct list_head list;
 	const char *name;
 	int (*probe_channel)(struct most_interface *iface, int channel_idx,
-			     struct most_channel_config *cfg, char *name);
+			     struct most_channel_config *cfg, char *name,
+			     char *param);
 	int (*disconnect_channel)(struct most_interface *iface,
 				  int channel_idx);
 	int (*rx_completion)(struct mbo *mbo);
diff --git a/drivers/staging/most/net/net.c b/drivers/staging/most/net/net.c
index e20584b..c8a64e2 100644
--- a/drivers/staging/most/net/net.c
+++ b/drivers/staging/most/net/net.c
@@ -293,7 +293,8 @@ static struct net_dev_context *get_net_dev_hold(struct most_interface *iface)
 }
 
 static int comp_probe_channel(struct most_interface *iface, int channel_idx,
-			      struct most_channel_config *ccfg, char *name)
+			      struct most_channel_config *ccfg, char *name,
+			      char *args)
 {
 	struct net_dev_context *nd;
 	struct net_dev_channel *ch;
diff --git a/drivers/staging/most/sound/sound.c b/drivers/staging/most/sound/sound.c
index 79ab3a7..02fcd32 100644
--- a/drivers/staging/most/sound/sound.c
+++ b/drivers/staging/most/sound/sound.c
@@ -579,7 +579,7 @@ static void release_adapter(struct sound_adapter *adpt)
  */
 static int audio_probe_channel(struct most_interface *iface, int channel_id,
 			       struct most_channel_config *cfg,
-			       char *arg_list)
+			       char *device_name, char *arg_list)
 {
 	struct channel *channel;
 	struct sound_adapter *adpt;
@@ -588,7 +588,6 @@ static int audio_probe_channel(struct most_interface *iface, int channel_id,
 	int capture_count = 0;
 	int ret;
 	int direction;
-	char *device_name;
 	u16 ch_num;
 	u8 create = 0;
 	char *sample_res;
diff --git a/drivers/staging/most/video/video.c b/drivers/staging/most/video/video.c
index ad7e28a..adca250 100644
--- a/drivers/staging/most/video/video.c
+++ b/drivers/staging/most/video/video.c
@@ -453,7 +453,8 @@ static void comp_v4l2_dev_release(struct v4l2_device *v4l2_dev)
 }
 
 static int comp_probe_channel(struct most_interface *iface, int channel_idx,
-			      struct most_channel_config *ccfg, char *name)
+			      struct most_channel_config *ccfg, char *name,
+			      char *args)
 {
 	int ret;
 	struct most_video_dev *mdev = get_comp_dev(iface, channel_idx);
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 03/14] staging: most: core: add configfs interface functions

[Christian Gromm]

This patch adds the core's interface to configfs file.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
v2:
	- remove configfs_exit prototype

 drivers/staging/most/core.c | 142 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/staging/most/core.h |  16 ++++-
 2 files changed, 157 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/most/core.c b/drivers/staging/most/core.c
index fb19b63..d5cf58f 100644
--- a/drivers/staging/most/core.c
+++ b/drivers/staging/most/core.c
@@ -783,6 +783,127 @@ static ssize_t add_link_store(struct device_driver *drv,
 	return len;
 }
 
+int most_set_cfg_buffer_size(char *mdev, char *mdev_ch, u16 val)
+{
+	struct most_channel *c = get_channel(mdev, mdev_ch);
+
+	if (!c)
+		return -ENODEV;
+	c->cfg.buffer_size = val;
+	return 0;
+}
+
+int most_set_cfg_subbuffer_size(char *mdev, char *mdev_ch, u16 val)
+{
+	struct most_channel *c = get_channel(mdev, mdev_ch);
+
+	if (!c)
+		return -ENODEV;
+	c->cfg.subbuffer_size = val;
+	return 0;
+}
+
+int most_set_cfg_dbr_size(char *mdev, char *mdev_ch, u16 val)
+{
+	struct most_channel *c = get_channel(mdev, mdev_ch);
+
+	if (!c)
+		return -ENODEV;
+	c->cfg.dbr_size = val;
+	return 0;
+}
+
+int most_set_cfg_num_buffers(char *mdev, char *mdev_ch, u16 val)
+{
+	struct most_channel *c = get_channel(mdev, mdev_ch);
+
+	if (!c)
+		return -ENODEV;
+	c->cfg.num_buffers = val;
+	return 0;
+}
+
+int most_set_cfg_datatype(char *mdev, char *mdev_ch, char *buf)
+{
+	int i;
+	struct most_channel *c = get_channel(mdev, mdev_ch);
+
+	if (!c)
+		return -ENODEV;
+	for (i = 0; i < ARRAY_SIZE(ch_data_type); i++) {
+		if (!strcmp(buf, ch_data_type[i].name)) {
+			c->cfg.data_type = ch_data_type[i].most_ch_data_type;
+			break;
+		}
+	}
+
+	if (i == ARRAY_SIZE(ch_data_type))
+		pr_info("WARN: invalid attribute settings\n");
+	return 0;
+}
+
+int most_set_cfg_direction(char *mdev, char *mdev_ch, char *buf)
+{
+	struct most_channel *c = get_channel(mdev, mdev_ch);
+
+	if (!c)
+		return -ENODEV;
+	if (!strcmp(buf, "dir_rx\n")) {
+		c->cfg.direction = MOST_CH_RX;
+	} else if (!strcmp(buf, "rx\n")) {
+		c->cfg.direction = MOST_CH_RX;
+	} else if (!strcmp(buf, "dir_tx\n")) {
+		c->cfg.direction = MOST_CH_TX;
+	} else if (!strcmp(buf, "tx\n")) {
+		c->cfg.direction = MOST_CH_TX;
+	} else {
+		pr_info("Invalid direction\n");
+		return -ENODATA;
+	}
+	return 0;
+}
+
+int most_set_cfg_packets_xact(char *mdev, char *mdev_ch, u16 val)
+{
+	struct most_channel *c = get_channel(mdev, mdev_ch);
+
+	if (!c)
+		return -ENODEV;
+	c->cfg.packets_per_xact = val;
+	return 0;
+}
+
+int most_cfg_complete(char *comp_name)
+{
+	struct core_component *comp;
+
+	comp = match_component(comp_name);
+	if (!comp)
+		return -ENODEV;
+
+	return comp->cfg_complete();
+}
+
+int most_add_link(char *mdev, char *mdev_ch, char *comp_name, char *link_name,
+		  char *comp_param)
+{
+	struct most_channel *c;
+	struct core_component *comp;
+	char buf[STRING_SIZE];
+
+	comp = match_component(comp_name);
+	if (!comp)
+		return -ENODEV;
+	if (!comp_param || *comp_param == 0) {
+		snprintf(buf, sizeof(buf), "%s-%s", mdev, mdev_ch);
+		comp_param = buf;
+	}
+	c = get_channel(mdev, mdev_ch);
+	if (!c)
+		return -ENODEV;
+
+	return link_channel_to_component(c, comp, link_name, comp_param);
+}
 /**
  * remove_link_store - store function for remove_link attribute
  * @drv: device driver
@@ -825,6 +946,27 @@ static ssize_t remove_link_store(struct device_driver *drv,
 	return len;
 }
 
+int most_remove_link(char *mdev, char *mdev_ch, char *comp_name)
+{
+	struct most_channel *c;
+	struct core_component *comp;
+
+	comp = match_component(comp_name);
+	if (!comp)
+		return -ENODEV;
+	c = get_channel(mdev, mdev_ch);
+	if (!c)
+		return -ENODEV;
+
+	if (comp->disconnect_channel(c->iface, c->channel_id))
+		return -EIO;
+	if (c->pipe0.comp == comp)
+		c->pipe0.comp = NULL;
+	if (c->pipe1.comp == comp)
+		c->pipe1.comp = NULL;
+	return 0;
+}
+
 #define DRV_ATTR(_name)  (&driver_attr_##_name.attr)
 
 static DRIVER_ATTR_RO(links);
diff --git a/drivers/staging/most/core.h b/drivers/staging/most/core.h
index 025dd1d..12c5992 100644
--- a/drivers/staging/most/core.h
+++ b/drivers/staging/most/core.h
@@ -272,6 +272,7 @@ struct core_component {
 				  int channel_idx);
 	int (*rx_completion)(struct mbo *mbo);
 	int (*tx_completion)(struct most_interface *iface, int channel_idx);
+	int (*cfg_complete)(void);
 };
 
 /**
@@ -319,5 +320,18 @@ int most_start_channel(struct most_interface *iface, int channel_idx,
 		       struct core_component *comp);
 int most_stop_channel(struct most_interface *iface, int channel_idx,
 		      struct core_component *comp);
-
+int __init configfs_init(void);
+int most_register_configfs_subsys(struct core_component *comp);
+void most_deregister_configfs_subsys(struct core_component *comp);
+int most_add_link(char *mdev, char *mdev_ch, char *comp_name, char *link_name,
+		  char *comp_param);
+int most_remove_link(char *mdev, char *mdev_ch, char *comp_name);
+int most_set_cfg_buffer_size(char *mdev, char *mdev_ch, u16 val);
+int most_set_cfg_subbuffer_size(char *mdev, char *mdev_ch, u16 val);
+int most_set_cfg_dbr_size(char *mdev, char *mdev_ch, u16 val);
+int most_set_cfg_num_buffers(char *mdev, char *mdev_ch, u16 val);
+int most_set_cfg_datatype(char *mdev, char *mdev_ch, char *buf);
+int most_set_cfg_direction(char *mdev, char *mdev_ch, char *buf);
+int most_set_cfg_packets_xact(char *mdev, char *mdev_ch, u16 val);
+int most_cfg_complete(char *comp_name);
 #endif /* MOST_CORE_H_ */
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 04/14] staging: most: sound: introduce new sound adapter management

[Christian Gromm]

This patch adapts the sound card management to the configfs changes.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
v2:

 drivers/staging/most/sound/sound.c | 41 +++++++++++++++++++++-----------------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/most/sound/sound.c b/drivers/staging/most/sound/sound.c
index 02fcd32..049ec44 100644
--- a/drivers/staging/most/sound/sound.c
+++ b/drivers/staging/most/sound/sound.c
@@ -471,17 +471,11 @@ static const struct snd_pcm_ops pcm_ops = {
 	.page       = snd_pcm_lib_get_vmalloc_page,
 };
 
-static int split_arg_list(char *buf, char **device_name, u16 *ch_num,
-			  char **sample_res, u8 *create)
+static int split_arg_list(char *buf, u16 *ch_num, char **sample_res)
 {
 	char *num;
 	int ret;
 
-	*device_name = strsep(&buf, ".");
-	if (!*device_name) {
-		pr_err("Missing sound card name\n");
-		return -EIO;
-	}
 	num = strsep(&buf, "x");
 	if (!num)
 		goto err;
@@ -492,8 +486,6 @@ static int split_arg_list(char *buf, char **device_name, u16 *ch_num,
 	if (!*sample_res)
 		goto err;
 
-	if (buf && !strcmp(buf, "create"))
-		*create = 1;
 	return 0;
 
 err:
@@ -589,7 +581,6 @@ static int audio_probe_channel(struct most_interface *iface, int channel_id,
 	int ret;
 	int direction;
 	u16 ch_num;
-	u8 create = 0;
 	char *sample_res;
 
 	if (!iface)
@@ -600,8 +591,7 @@ static int audio_probe_channel(struct most_interface *iface, int channel_id,
 		return -EINVAL;
 	}
 
-	ret = split_arg_list(arg_list, &device_name, &ch_num, &sample_res,
-			     &create);
+	ret = split_arg_list(arg_list, &ch_num, &sample_res);
 	if (ret < 0)
 		return ret;
 
@@ -672,12 +662,6 @@ static int audio_probe_channel(struct most_interface *iface, int channel_id,
 	strscpy(pcm->name, device_name, sizeof(pcm->name));
 	snd_pcm_set_ops(pcm, direction, &pcm_ops);
 
-	if (create) {
-		ret = snd_card_register(adpt->card);
-		if (ret < 0)
-			goto err_free_adpt;
-		adpt->registered = true;
-	}
 	return 0;
 
 err_free_adpt:
@@ -685,6 +669,26 @@ static int audio_probe_channel(struct most_interface *iface, int channel_id,
 	return ret;
 }
 
+static int audio_create_sound_card(void)
+{
+	int ret;
+	struct sound_adapter *adpt;
+
+	list_for_each_entry(adpt, &adpt_list, list) {
+		if (!adpt->registered)
+			goto adpt_alloc;
+	}
+	return -ENODEV;
+adpt_alloc:
+	ret = snd_card_register(adpt->card);
+	if (ret < 0) {
+		release_adapter(adpt);
+		return ret;
+	}
+	adpt->registered = true;
+	return ret;
+}
+
 /**
  * audio_disconnect_channel - function to disconnect a channel
  * @iface: pointer to interface instance
@@ -781,6 +785,7 @@ static struct core_component comp = {
 	.disconnect_channel = audio_disconnect_channel,
 	.rx_completion = audio_rx_completion,
 	.tx_completion = audio_tx_completion,
+	.cfg_complete = audio_create_sound_card,
 };
 
 static int __init audio_init(void)
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 05/14] staging: most: enable configfs support

[Christian Gromm]

This patch enables the configfs functionality of the driver by
registering the configfs subsystems and compiling the configfs
part of the sources.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
v2:
	- remove call to configfs_exit function

 drivers/staging/most/Makefile      |  1 +
 drivers/staging/most/cdev/cdev.c   |  6 ++++++
 drivers/staging/most/core.c        |  2 +-
 drivers/staging/most/sound/sound.c | 11 ++++++++++-
 4 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/most/Makefile b/drivers/staging/most/Makefile
index c7662f6..85ea5a4 100644
--- a/drivers/staging/most/Makefile
+++ b/drivers/staging/most/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_MOST) += most_core.o
 most_core-y := core.o
+most_core-y += configfs.o
 ccflags-y += -I $(srctree)/drivers/staging/
 
 obj-$(CONFIG_MOST_CDEV)	+= cdev/
diff --git a/drivers/staging/most/cdev/cdev.c b/drivers/staging/most/cdev/cdev.c
index 97408ec..d98977c 100644
--- a/drivers/staging/most/cdev/cdev.c
+++ b/drivers/staging/most/cdev/cdev.c
@@ -527,8 +527,13 @@ static int __init mod_init(void)
 	err = most_register_component(&comp.cc);
 	if (err)
 		goto free_cdev;
+	err = most_register_configfs_subsys(&comp.cc);
+	if (err)
+		goto deregister_comp;
 	return 0;
 
+deregister_comp:
+	most_deregister_component(&comp.cc);
 free_cdev:
 	unregister_chrdev_region(comp.devno, CHRDEV_REGION_SIZE);
 dest_ida:
@@ -543,6 +548,7 @@ static void __exit mod_exit(void)
 
 	pr_info("exit module\n");
 
+	most_deregister_configfs_subsys(&comp.cc);
 	most_deregister_component(&comp.cc);
 
 	list_for_each_entry_safe(c, tmp, &channel_list, list) {
diff --git a/drivers/staging/most/core.c b/drivers/staging/most/core.c
index d5cf58f..e7a8d89 100644
--- a/drivers/staging/most/core.c
+++ b/drivers/staging/most/core.c
@@ -1765,7 +1765,7 @@ static int __init most_init(void)
 		err = -ENOMEM;
 		goto err_unregister_driver;
 	}
-
+	configfs_init();
 	return 0;
 
 err_unregister_driver:
diff --git a/drivers/staging/most/sound/sound.c b/drivers/staging/most/sound/sound.c
index 049ec44..fd089e6 100644
--- a/drivers/staging/most/sound/sound.c
+++ b/drivers/staging/most/sound/sound.c
@@ -790,16 +790,25 @@ static struct core_component comp = {
 
 static int __init audio_init(void)
 {
+	int ret;
+
 	pr_info("init()\n");
 
 	INIT_LIST_HEAD(&adpt_list);
 
-	return most_register_component(&comp);
+	ret = most_register_component(&comp);
+	if (ret)
+		pr_err("Failed to register %s\n", comp.name);
+	ret = most_register_configfs_subsys(&comp);
+	if (ret)
+		pr_err("Failed to register %s configfs subsys\n", comp.name);
+	return ret;
 }
 
 static void __exit audio_exit(void)
 {
 	pr_info("exit()\n");
+	most_deregister_configfs_subsys(&comp);
 	most_deregister_component(&comp);
 }
 
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 06/14] staging: most: core: make sysfs attributes read-only

[Christian Gromm]

This patch changes the access flags of the channel attributes to
read-only. This is needed, because configuration is done via configfs.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
v2:

 drivers/staging/most/core.c | 122 +++-----------------------------------------
 1 file changed, 7 insertions(+), 115 deletions(-)

diff --git a/drivers/staging/most/core.c b/drivers/staging/most/core.c
index e7a8d89..a5df53e 100644
--- a/drivers/staging/most/core.c
+++ b/drivers/staging/most/core.c
@@ -272,19 +272,6 @@ static ssize_t set_number_of_buffers_show(struct device *dev,
 	return snprintf(buf, PAGE_SIZE, "%d\n", c->cfg.num_buffers);
 }
 
-static ssize_t set_number_of_buffers_store(struct device *dev,
-					   struct device_attribute *attr,
-					   const char *buf,
-					   size_t count)
-{
-	struct most_channel *c = to_channel(dev);
-	int ret = kstrtou16(buf, 0, &c->cfg.num_buffers);
-
-	if (ret)
-		return ret;
-	return count;
-}
-
 static ssize_t set_buffer_size_show(struct device *dev,
 				    struct device_attribute *attr,
 				    char *buf)
@@ -294,19 +281,6 @@ static ssize_t set_buffer_size_show(struct device *dev,
 	return snprintf(buf, PAGE_SIZE, "%d\n", c->cfg.buffer_size);
 }
 
-static ssize_t set_buffer_size_store(struct device *dev,
-				     struct device_attribute *attr,
-				     const char *buf,
-				     size_t count)
-{
-	struct most_channel *c = to_channel(dev);
-	int ret = kstrtou16(buf, 0, &c->cfg.buffer_size);
-
-	if (ret)
-		return ret;
-	return count;
-}
-
 static ssize_t set_direction_show(struct device *dev,
 				  struct device_attribute *attr,
 				  char *buf)
@@ -320,28 +294,6 @@ static ssize_t set_direction_show(struct device *dev,
 	return snprintf(buf, PAGE_SIZE, "unconfigured\n");
 }
 
-static ssize_t set_direction_store(struct device *dev,
-				   struct device_attribute *attr,
-				   const char *buf,
-				   size_t count)
-{
-	struct most_channel *c = to_channel(dev);
-
-	if (!strcmp(buf, "dir_rx\n")) {
-		c->cfg.direction = MOST_CH_RX;
-	} else if (!strcmp(buf, "rx\n")) {
-		c->cfg.direction = MOST_CH_RX;
-	} else if (!strcmp(buf, "dir_tx\n")) {
-		c->cfg.direction = MOST_CH_TX;
-	} else if (!strcmp(buf, "tx\n")) {
-		c->cfg.direction = MOST_CH_TX;
-	} else {
-		pr_info("WARN: invalid attribute settings\n");
-		return -EINVAL;
-	}
-	return count;
-}
-
 static ssize_t set_datatype_show(struct device *dev,
 				 struct device_attribute *attr,
 				 char *buf)
@@ -356,28 +308,6 @@ static ssize_t set_datatype_show(struct device *dev,
 	return snprintf(buf, PAGE_SIZE, "unconfigured\n");
 }
 
-static ssize_t set_datatype_store(struct device *dev,
-				  struct device_attribute *attr,
-				  const char *buf,
-				  size_t count)
-{
-	int i;
-	struct most_channel *c = to_channel(dev);
-
-	for (i = 0; i < ARRAY_SIZE(ch_data_type); i++) {
-		if (!strcmp(buf, ch_data_type[i].name)) {
-			c->cfg.data_type = ch_data_type[i].most_ch_data_type;
-			break;
-		}
-	}
-
-	if (i == ARRAY_SIZE(ch_data_type)) {
-		pr_info("WARN: invalid attribute settings\n");
-		return -EINVAL;
-	}
-	return count;
-}
-
 static ssize_t set_subbuffer_size_show(struct device *dev,
 				       struct device_attribute *attr,
 				       char *buf)
@@ -387,19 +317,6 @@ static ssize_t set_subbuffer_size_show(struct device *dev,
 	return snprintf(buf, PAGE_SIZE, "%d\n", c->cfg.subbuffer_size);
 }
 
-static ssize_t set_subbuffer_size_store(struct device *dev,
-					struct device_attribute *attr,
-					const char *buf,
-					size_t count)
-{
-	struct most_channel *c = to_channel(dev);
-	int ret = kstrtou16(buf, 0, &c->cfg.subbuffer_size);
-
-	if (ret)
-		return ret;
-	return count;
-}
-
 static ssize_t set_packets_per_xact_show(struct device *dev,
 					 struct device_attribute *attr,
 					 char *buf)
@@ -409,19 +326,6 @@ static ssize_t set_packets_per_xact_show(struct device *dev,
 	return snprintf(buf, PAGE_SIZE, "%d\n", c->cfg.packets_per_xact);
 }
 
-static ssize_t set_packets_per_xact_store(struct device *dev,
-					  struct device_attribute *attr,
-					  const char *buf,
-					  size_t count)
-{
-	struct most_channel *c = to_channel(dev);
-	int ret = kstrtou16(buf, 0, &c->cfg.packets_per_xact);
-
-	if (ret)
-		return ret;
-	return count;
-}
-
 static ssize_t set_dbr_size_show(struct device *dev,
 				 struct device_attribute *attr, char *buf)
 {
@@ -430,18 +334,6 @@ static ssize_t set_dbr_size_show(struct device *dev,
 	return snprintf(buf, PAGE_SIZE, "%d\n", c->cfg.dbr_size);
 }
 
-static ssize_t set_dbr_size_store(struct device *dev,
-				  struct device_attribute *attr,
-				  const char *buf, size_t count)
-{
-	struct most_channel *c = to_channel(dev);
-	int ret = kstrtou16(buf, 0, &c->cfg.dbr_size);
-
-	if (ret)
-		return ret;
-	return count;
-}
-
 #define to_dev_attr(a) container_of(a, struct device_attribute, attr)
 static umode_t channel_attr_is_visible(struct kobject *kobj,
 				       struct attribute *attr, int index)
@@ -469,13 +361,13 @@ static DEVICE_ATTR_RO(number_of_stream_buffers);
 static DEVICE_ATTR_RO(size_of_stream_buffer);
 static DEVICE_ATTR_RO(size_of_packet_buffer);
 static DEVICE_ATTR_RO(channel_starving);
-static DEVICE_ATTR_RW(set_buffer_size);
-static DEVICE_ATTR_RW(set_number_of_buffers);
-static DEVICE_ATTR_RW(set_direction);
-static DEVICE_ATTR_RW(set_datatype);
-static DEVICE_ATTR_RW(set_subbuffer_size);
-static DEVICE_ATTR_RW(set_packets_per_xact);
-static DEVICE_ATTR_RW(set_dbr_size);
+static DEVICE_ATTR_RO(set_buffer_size);
+static DEVICE_ATTR_RO(set_number_of_buffers);
+static DEVICE_ATTR_RO(set_direction);
+static DEVICE_ATTR_RO(set_datatype);
+static DEVICE_ATTR_RO(set_subbuffer_size);
+static DEVICE_ATTR_RO(set_packets_per_xact);
+static DEVICE_ATTR_RO(set_dbr_size);
 
 static struct attribute *channel_attrs[] = {
 	DEV_ATTR(available_directions),
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 07/14] staging: most: core: use device description as name

[Christian Gromm]

This patch uses the device description to clearly identity a device
attached to the bus. It is needed as the currently useed mdevX
notation is not sufficiant in case more than one network
interface controller is being used at the same time.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
v2:

 drivers/staging/most/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/most/core.c b/drivers/staging/most/core.c
index a5df53e..e0a6806 100644
--- a/drivers/staging/most/core.c
+++ b/drivers/staging/most/core.c
@@ -1467,7 +1467,7 @@ int most_register_interface(struct most_interface *iface)
 
 	INIT_LIST_HEAD(&iface->p->channel_list);
 	iface->p->dev_id = id;
-	snprintf(iface->p->name, STRING_SIZE, "mdev%d", id);
+	strcpy(iface->p->name, iface->description);
 	iface->dev.init_name = iface->p->name;
 	iface->dev.bus = &mc.bus;
 	iface->dev.parent = &mc.dev;
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 08/14] staging: most: usb: remove prefix from description tag

[Christian Gromm]

This patch cuts off the usb_device prefix of the description string.
It is not needed, as the interface type is already available with the
interface attribute of a channel.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
v2:

 drivers/staging/most/usb/usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/most/usb/usb.c b/drivers/staging/most/usb/usb.c
index c0293d8..360cb5b 100644
--- a/drivers/staging/most/usb/usb.c
+++ b/drivers/staging/most/usb/usb.c
@@ -1072,7 +1072,7 @@ hdm_probe(struct usb_interface *interface, const struct usb_device_id *id)
 	mdev->iface.num_channels = num_endpoints;
 
 	snprintf(mdev->description, sizeof(mdev->description),
-		 "usb_device %d-%s:%d.%d",
+		 "%d-%s:%d.%d",
 		 usb_dev->bus->busnum,
 		 usb_dev->devpath,
 		 usb_dev->config->desc.bConfigurationValue,
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 09/14] staging: most: core: remove attribute add_link

[Christian Gromm]

This patch removes the driver attribute add_link. It is not needed, because
the link management is now done via configfs.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
v2:

 drivers/staging/most/core.c | 61 ---------------------------------------------
 1 file changed, 61 deletions(-)

diff --git a/drivers/staging/most/core.c b/drivers/staging/most/core.c
index e0a6806..df1d774 100644
--- a/drivers/staging/most/core.c
+++ b/drivers/staging/most/core.c
@@ -616,65 +616,6 @@ inline int link_channel_to_component(struct most_channel *c,
 	return 0;
 }
 
-/**
- * add_link_store - store function for add_link attribute
- * @drv: device driver
- * @buf: buffer
- * @len: buffer length
- *
- * This parses the string given by buf and splits it into
- * four substrings. Note: last substring is optional. In case a cdev
- * component is loaded the optional 4th substring will make up the name of
- * device node in the /dev directory. If omitted, the device node will
- * inherit the channel's name within sysfs.
- *
- * Searches for (device, channel) pair and probes the component
- *
- * Example:
- * (1) echo "mdev0:ch6:cdev:my_rxchannel" >add_link
- * (2) echo "mdev1:ep81:cdev" >add_link
- *
- * (1) would create the device node /dev/my_rxchannel
- * (2) would create the device node /dev/mdev1-ep81
- */
-static ssize_t add_link_store(struct device_driver *drv,
-			      const char *buf,
-			      size_t len)
-{
-	struct most_channel *c;
-	struct core_component *comp;
-	char buffer[STRING_SIZE];
-	char *mdev;
-	char *mdev_ch;
-	char *comp_name;
-	char *comp_param;
-	char devnod_buf[STRING_SIZE];
-	int ret;
-	size_t max_len = min_t(size_t, len + 1, STRING_SIZE);
-
-	strlcpy(buffer, buf, max_len);
-	ret = split_string(buffer, &mdev, &mdev_ch, &comp_name, &comp_param);
-	if (ret)
-		return ret;
-	comp = match_component(comp_name);
-	if (!comp)
-		return -ENODEV;
-	if (!comp_param || *comp_param == 0) {
-		snprintf(devnod_buf, sizeof(devnod_buf), "%s-%s", mdev,
-			 mdev_ch);
-		comp_param = devnod_buf;
-	}
-
-	c = get_channel(mdev, mdev_ch);
-	if (!c)
-		return -ENODEV;
-
-	ret = link_channel_to_component(c, comp, "name", comp_param);
-	if (ret)
-		return ret;
-	return len;
-}
-
 int most_set_cfg_buffer_size(char *mdev, char *mdev_ch, u16 val)
 {
 	struct most_channel *c = get_channel(mdev, mdev_ch);
@@ -863,13 +804,11 @@ int most_remove_link(char *mdev, char *mdev_ch, char *comp_name)
 
 static DRIVER_ATTR_RO(links);
 static DRIVER_ATTR_RO(components);
-static DRIVER_ATTR_WO(add_link);
 static DRIVER_ATTR_WO(remove_link);
 
 static struct attribute *mc_attrs[] = {
 	DRV_ATTR(links),
 	DRV_ATTR(components),
-	DRV_ATTR(add_link),
 	DRV_ATTR(remove_link),
 	NULL,
 };
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 10/14] staging: most: allow speculative configuration

[Christian Gromm]

This patch makes the driver accept a link confiiguration eventhough no
device is attached to the bus. Instead the configuration is being applied
as soon as a device is being registered with the core.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
v2:
	- follow-up adaptions due to changes introduced w/ [PATCH v2 01/14]

 drivers/staging/most/configfs.c    | 60 ++++++++++++++++++++++++++++----------
 drivers/staging/most/core.c        | 16 +++-------
 drivers/staging/most/core.h        |  1 +
 drivers/staging/most/sound/sound.c |  6 ++--
 4 files changed, 53 insertions(+), 30 deletions(-)

diff --git a/drivers/staging/most/configfs.c b/drivers/staging/most/configfs.c
index 6968b299..3a7c54d 100644
--- a/drivers/staging/most/configfs.c
+++ b/drivers/staging/most/configfs.c
@@ -14,6 +14,7 @@
 
 struct mdev_link {
 	struct config_item item;
+	struct list_head list;
 	bool create;
 	u16 num_buffers;
 	u16 buffer_size;
@@ -29,6 +30,8 @@ struct mdev_link {
 	char param[PAGE_SIZE];
 };
 
+struct list_head mdev_link_list;
+
 static int set_cfg_buffer_size(struct mdev_link *link)
 {
 	if (!link->buffer_size)
@@ -105,33 +108,41 @@ static ssize_t mdev_link_create_show(struct config_item *item, char *page)
 	return snprintf(page, PAGE_SIZE, "%d\n", to_mdev_link(item)->create);
 }
 
+static int set_config_and_add_link(struct mdev_link *mdev_link)
+{
+	int i;
+	int ret;
+
+	for (i = 0; i < ARRAY_SIZE(set_config_val); i++) {
+		ret = set_config_val[i](mdev_link);
+		if (ret == -ENODATA) {
+			pr_err("Config failed\n");
+			return ret;
+		}
+	}
+
+	return most_add_link(mdev_link->mdev, mdev_link->channel,
+			     mdev_link->comp, mdev_link->name,
+			     mdev_link->param);
+}
+
 static ssize_t mdev_link_create_store(struct config_item *item,
 				      const char *page, size_t count)
 {
 	struct mdev_link *mdev_link = to_mdev_link(item);
 	bool tmp;
 	int ret;
-	int i;
 
 	ret = kstrtobool(page, &tmp);
 	if (ret)
 		return ret;
-
-	for (i = 0; i < ARRAY_SIZE(set_config_val); i++) {
-		ret = set_config_val[i](mdev_link);
-		if (ret < 0)
-			return ret;
-	}
-
-	if (tmp)
-		ret = most_add_link(mdev_link->mdev, mdev_link->channel,
-				    mdev_link->comp, mdev_link->name,
-				    mdev_link->param);
-	else
-		ret = most_remove_link(mdev_link->mdev, mdev_link->channel,
-				       mdev_link->comp);
-	if (ret)
+	if (!tmp)
+		return most_remove_link(mdev_link->mdev, mdev_link->channel,
+					mdev_link->comp);
+	ret = set_config_and_add_link(mdev_link);
+	if (ret && ret != -ENODEV)
 		return ret;
+	list_add_tail(&mdev_link->list, &mdev_link_list);
 	mdev_link->create = tmp;
 	return count;
 }
@@ -590,6 +601,22 @@ int most_register_configfs_subsys(struct core_component *c)
 }
 EXPORT_SYMBOL_GPL(most_register_configfs_subsys);
 
+void most_interface_register_notify(const char *mdev)
+{
+	bool register_snd_card = false;
+	struct mdev_link *mdev_link;
+
+	list_for_each_entry(mdev_link, &mdev_link_list, list) {
+		if (!strcmp(mdev_link->mdev, mdev)) {
+			set_config_and_add_link(mdev_link);
+			if (!strcmp(mdev_link->comp, "sound"))
+				register_snd_card = true;
+		}
+	}
+	if (register_snd_card)
+		most_cfg_complete("sound");
+}
+
 void most_deregister_configfs_subsys(struct core_component *c)
 {
 	if (!strcmp(c->name, "cdev"))
@@ -618,6 +645,7 @@ int __init configfs_init(void)
 	mutex_init(&most_sound_subsys.subsys.su_mutex);
 
 	INIT_LIST_HEAD(&most_sound_subsys.soundcard_list);
+	INIT_LIST_HEAD(&mdev_link_list);
 
 	return 0;
 }
diff --git a/drivers/staging/most/core.c b/drivers/staging/most/core.c
index df1d774..b1f7f70 100644
--- a/drivers/staging/most/core.c
+++ b/drivers/staging/most/core.c
@@ -720,19 +720,10 @@ int most_cfg_complete(char *comp_name)
 int most_add_link(char *mdev, char *mdev_ch, char *comp_name, char *link_name,
 		  char *comp_param)
 {
-	struct most_channel *c;
-	struct core_component *comp;
-	char buf[STRING_SIZE];
+	struct most_channel *c = get_channel(mdev, mdev_ch);
+	struct core_component *comp = match_component(comp_name);
 
-	comp = match_component(comp_name);
-	if (!comp)
-		return -ENODEV;
-	if (!comp_param || *comp_param == 0) {
-		snprintf(buf, sizeof(buf), "%s-%s", mdev, mdev_ch);
-		comp_param = buf;
-	}
-	c = get_channel(mdev, mdev_ch);
-	if (!c)
+	if (!c || !comp)
 		return -ENODEV;
 
 	return link_channel_to_component(c, comp, link_name, comp_param);
@@ -1462,6 +1453,7 @@ int most_register_interface(struct most_interface *iface)
 	}
 	pr_info("registered new device mdev%d (%s)\n",
 		id, iface->description);
+	most_interface_register_notify(iface->description);
 	return 0;
 
 err_free_most_channel:
diff --git a/drivers/staging/most/core.h b/drivers/staging/most/core.h
index 12c5992..652aaa7 100644
--- a/drivers/staging/most/core.h
+++ b/drivers/staging/most/core.h
@@ -334,4 +334,5 @@ int most_set_cfg_datatype(char *mdev, char *mdev_ch, char *buf);
 int most_set_cfg_direction(char *mdev, char *mdev_ch, char *buf);
 int most_set_cfg_packets_xact(char *mdev, char *mdev_ch, u16 val);
 int most_cfg_complete(char *comp_name);
+void most_interface_register_notify(const char *mdev_name);
 #endif /* MOST_CORE_H_ */
diff --git a/drivers/staging/most/sound/sound.c b/drivers/staging/most/sound/sound.c
index fd089e6..44c9146 100644
--- a/drivers/staging/most/sound/sound.c
+++ b/drivers/staging/most/sound/sound.c
@@ -20,6 +20,7 @@
 #include <most/core.h>
 
 #define DRIVER_NAME "sound"
+#define STRING_SIZE	80
 
 static struct core_component comp;
 
@@ -582,6 +583,7 @@ static int audio_probe_channel(struct most_interface *iface, int channel_id,
 	int direction;
 	u16 ch_num;
 	char *sample_res;
+	char arg_list_cpy[STRING_SIZE];
 
 	if (!iface)
 		return -EINVAL;
@@ -590,8 +592,8 @@ static int audio_probe_channel(struct most_interface *iface, int channel_id,
 		pr_err("Incompatible channel type\n");
 		return -EINVAL;
 	}
-
-	ret = split_arg_list(arg_list, &ch_num, &sample_res);
+	strlcpy(arg_list_cpy, arg_list, STRING_SIZE);
+	ret = split_arg_list(arg_list_cpy, &ch_num, &sample_res);
 	if (ret < 0)
 		return ret;
 
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 11/14] staging: most: configfs: make create attributes write-only

[Christian Gromm]

Reading the create attribute that triggers the creation of a link to
a certain channel is not necessary. Hence, it is being removed.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
v2:

 drivers/staging/most/configfs.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/most/configfs.c b/drivers/staging/most/configfs.c
index 3a7c54d..9d11462 100644
--- a/drivers/staging/most/configfs.c
+++ b/drivers/staging/most/configfs.c
@@ -103,11 +103,6 @@ static struct mdev_link *to_mdev_link(struct config_item *item)
 	return container_of(item, struct mdev_link, item);
 }
 
-static ssize_t mdev_link_create_show(struct config_item *item, char *page)
-{
-	return snprintf(page, PAGE_SIZE, "%d\n", to_mdev_link(item)->create);
-}
-
 static int set_config_and_add_link(struct mdev_link *mdev_link)
 {
 	int i;
@@ -329,7 +324,7 @@ static ssize_t mdev_link_dbr_size_store(struct config_item *item,
 	return count;
 }
 
-CONFIGFS_ATTR(mdev_link_, create);
+CONFIGFS_ATTR_WO(mdev_link_, create);
 CONFIGFS_ATTR(mdev_link_, mdev);
 CONFIGFS_ATTR(mdev_link_, channel);
 CONFIGFS_ATTR(mdev_link_, comp);
@@ -477,11 +472,6 @@ static struct config_item *most_snd_grp_make_item(struct config_group *group,
 	return &mdev_link->item;
 }
 
-static ssize_t most_snd_grp_create_show(struct config_item *item, char *page)
-{
-	return snprintf(page, PAGE_SIZE, "%d\n", to_most_snd_grp(item)->create);
-}
-
 static ssize_t most_snd_grp_create_store(struct config_item *item,
 					 const char *page, size_t count)
 {
@@ -501,7 +491,7 @@ static ssize_t most_snd_grp_create_store(struct config_item *item,
 	return count;
 }
 
-CONFIGFS_ATTR(most_snd_grp_, create);
+CONFIGFS_ATTR_WO(most_snd_grp_, create);
 
 static struct configfs_attribute *most_snd_grp_attrs[] = {
 	&most_snd_grp_attr_create,
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 12/14] staging: most: configfs: add code for link removal

[Christian Gromm]

This patch adds code that cleans up established links whenever the destroy
attribute is set or if the config_item (directory) is being removed.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
v2:
	- follow-up adaptions due to changes introduced w/ [PATCH v2 01/14]

 drivers/staging/most/configfs.c | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/most/configfs.c b/drivers/staging/most/configfs.c
index 9d11462..1a5d6f8 100644
--- a/drivers/staging/most/configfs.c
+++ b/drivers/staging/most/configfs.c
@@ -16,6 +16,7 @@ struct mdev_link {
 	struct config_item item;
 	struct list_head list;
 	bool create;
+	bool destroy;
 	u16 num_buffers;
 	u16 buffer_size;
 	u16 subbuffer_size;
@@ -132,8 +133,7 @@ static ssize_t mdev_link_create_store(struct config_item *item,
 	if (ret)
 		return ret;
 	if (!tmp)
-		return most_remove_link(mdev_link->mdev, mdev_link->channel,
-					mdev_link->comp);
+		return count;
 	ret = set_config_and_add_link(mdev_link);
 	if (ret && ret != -ENODEV)
 		return ret;
@@ -142,6 +142,28 @@ static ssize_t mdev_link_create_store(struct config_item *item,
 	return count;
 }
 
+static ssize_t mdev_link_destroy_store(struct config_item *item,
+				       const char *page, size_t count)
+{
+	struct mdev_link *mdev_link = to_mdev_link(item);
+	bool tmp;
+	int ret;
+
+	ret = kstrtobool(page, &tmp);
+	if (ret)
+		return ret;
+	if (!tmp)
+		return count;
+	mdev_link->destroy = tmp;
+	ret = most_remove_link(mdev_link->mdev, mdev_link->channel,
+			       mdev_link->comp);
+	if (ret)
+		return ret;
+	if (!list_empty(&mdev_link_list))
+		list_del(&mdev_link->list);
+	return count;
+}
+
 static ssize_t mdev_link_direction_show(struct config_item *item, char *page)
 {
 	return snprintf(page, PAGE_SIZE, "%s\n", to_mdev_link(item)->direction);
@@ -325,6 +347,7 @@ static ssize_t mdev_link_dbr_size_store(struct config_item *item,
 }
 
 CONFIGFS_ATTR_WO(mdev_link_, create);
+CONFIGFS_ATTR_WO(mdev_link_, destroy);
 CONFIGFS_ATTR(mdev_link_, mdev);
 CONFIGFS_ATTR(mdev_link_, channel);
 CONFIGFS_ATTR(mdev_link_, comp);
@@ -339,6 +362,7 @@ CONFIGFS_ATTR(mdev_link_, dbr_size);
 
 static struct configfs_attribute *mdev_link_attrs[] = {
 	&mdev_link_attr_create,
+	&mdev_link_attr_destroy,
 	&mdev_link_attr_mdev,
 	&mdev_link_attr_channel,
 	&mdev_link_attr_comp,
@@ -355,6 +379,16 @@ static struct configfs_attribute *mdev_link_attrs[] = {
 
 static void mdev_link_release(struct config_item *item)
 {
+	struct mdev_link *mdev_link = to_mdev_link(item);
+	int ret;
+
+	if (!list_empty(&mdev_link_list)) {
+		ret = most_remove_link(mdev_link->mdev, mdev_link->channel,
+				       mdev_link->comp);
+		if (ret && (ret != -ENODEV))
+			pr_err("Removing link failed.\n");
+		list_del(&mdev_link->list);
+	}
 	kfree(to_mdev_link(item));
 }
 
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 13/14] staging: most: configfs: rename config attributes

[Christian Gromm]

This patch introduces attribute names that are more self explaining.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
v2:
	- follow-up adaptions due to changes introduced w/ [PATCH v2 01/14]

 drivers/staging/most/configfs.c | 97 +++++++++++++++++++++--------------------
 1 file changed, 49 insertions(+), 48 deletions(-)

diff --git a/drivers/staging/most/configfs.c b/drivers/staging/most/configfs.c
index 1a5d6f8..546b658 100644
--- a/drivers/staging/most/configfs.c
+++ b/drivers/staging/most/configfs.c
@@ -15,8 +15,8 @@
 struct mdev_link {
 	struct config_item item;
 	struct list_head list;
-	bool create;
-	bool destroy;
+	bool create_link;
+	bool destroy_link;
 	u16 num_buffers;
 	u16 buffer_size;
 	u16 subbuffer_size;
@@ -25,10 +25,10 @@ struct mdev_link {
 	char datatype[PAGE_SIZE];
 	char direction[PAGE_SIZE];
 	char name[PAGE_SIZE];
-	char mdev[PAGE_SIZE];
+	char device[PAGE_SIZE];
 	char channel[PAGE_SIZE];
 	char comp[PAGE_SIZE];
-	char param[PAGE_SIZE];
+	char comp_params[PAGE_SIZE];
 };
 
 struct list_head mdev_link_list;
@@ -37,7 +37,7 @@ static int set_cfg_buffer_size(struct mdev_link *link)
 {
 	if (!link->buffer_size)
 		return -ENODATA;
-	return most_set_cfg_buffer_size(link->mdev, link->channel,
+	return most_set_cfg_buffer_size(link->device, link->channel,
 					link->buffer_size);
 }
 
@@ -45,7 +45,7 @@ static int set_cfg_subbuffer_size(struct mdev_link *link)
 {
 	if (!link->subbuffer_size)
 		return -ENODATA;
-	return most_set_cfg_subbuffer_size(link->mdev, link->channel,
+	return most_set_cfg_subbuffer_size(link->device, link->channel,
 					   link->subbuffer_size);
 }
 
@@ -53,7 +53,7 @@ static int set_cfg_dbr_size(struct mdev_link *link)
 {
 	if (!link->dbr_size)
 		return -ENODATA;
-	return most_set_cfg_dbr_size(link->mdev, link->channel,
+	return most_set_cfg_dbr_size(link->device, link->channel,
 				     link->dbr_size);
 }
 
@@ -61,7 +61,7 @@ static int set_cfg_num_buffers(struct mdev_link *link)
 {
 	if (!link->num_buffers)
 		return -ENODATA;
-	return most_set_cfg_num_buffers(link->mdev, link->channel,
+	return most_set_cfg_num_buffers(link->device, link->channel,
 					link->num_buffers);
 }
 
@@ -69,7 +69,7 @@ static int set_cfg_packets_xact(struct mdev_link *link)
 {
 	if (!link->packets_per_xact)
 		return -ENODATA;
-	return most_set_cfg_packets_xact(link->mdev, link->channel,
+	return most_set_cfg_packets_xact(link->device, link->channel,
 					 link->packets_per_xact);
 }
 
@@ -77,7 +77,7 @@ static int set_cfg_direction(struct mdev_link *link)
 {
 	if (!strlen(link->direction))
 		return -ENODATA;
-	return most_set_cfg_direction(link->mdev, link->channel,
+	return most_set_cfg_direction(link->device, link->channel,
 				      link->direction);
 }
 
@@ -85,7 +85,7 @@ static int set_cfg_datatype(struct mdev_link *link)
 {
 	if (!strlen(link->datatype))
 		return -ENODATA;
-	return most_set_cfg_datatype(link->mdev, link->channel,
+	return most_set_cfg_datatype(link->device, link->channel,
 				     link->datatype);
 }
 
@@ -117,13 +117,13 @@ static int set_config_and_add_link(struct mdev_link *mdev_link)
 		}
 	}
 
-	return most_add_link(mdev_link->mdev, mdev_link->channel,
+	return most_add_link(mdev_link->device, mdev_link->channel,
 			     mdev_link->comp, mdev_link->name,
-			     mdev_link->param);
+			     mdev_link->comp_params);
 }
 
-static ssize_t mdev_link_create_store(struct config_item *item,
-				      const char *page, size_t count)
+static ssize_t mdev_link_create_link_store(struct config_item *item,
+					   const char *page, size_t count)
 {
 	struct mdev_link *mdev_link = to_mdev_link(item);
 	bool tmp;
@@ -138,12 +138,12 @@ static ssize_t mdev_link_create_store(struct config_item *item,
 	if (ret && ret != -ENODEV)
 		return ret;
 	list_add_tail(&mdev_link->list, &mdev_link_list);
-	mdev_link->create = tmp;
+	mdev_link->create_link = tmp;
 	return count;
 }
 
-static ssize_t mdev_link_destroy_store(struct config_item *item,
-				       const char *page, size_t count)
+static ssize_t mdev_link_destroy_link_store(struct config_item *item,
+					    const char *page, size_t count)
 {
 	struct mdev_link *mdev_link = to_mdev_link(item);
 	bool tmp;
@@ -154,8 +154,8 @@ static ssize_t mdev_link_destroy_store(struct config_item *item,
 		return ret;
 	if (!tmp)
 		return count;
-	mdev_link->destroy = tmp;
-	ret = most_remove_link(mdev_link->mdev, mdev_link->channel,
+	mdev_link->destroy_link = tmp;
+	ret = most_remove_link(mdev_link->device, mdev_link->channel,
 			       mdev_link->comp);
 	if (ret)
 		return ret;
@@ -199,17 +199,17 @@ static ssize_t mdev_link_datatype_store(struct config_item *item,
 	return count;
 }
 
-static ssize_t mdev_link_mdev_show(struct config_item *item, char *page)
+static ssize_t mdev_link_device_show(struct config_item *item, char *page)
 {
-	return snprintf(page, PAGE_SIZE, "%s\n", to_mdev_link(item)->mdev);
+	return snprintf(page, PAGE_SIZE, "%s\n", to_mdev_link(item)->device);
 }
 
-static ssize_t mdev_link_mdev_store(struct config_item *item,
-				    const char *page, size_t count)
+static ssize_t mdev_link_device_store(struct config_item *item,
+				      const char *page, size_t count)
 {
 	struct mdev_link *mdev_link = to_mdev_link(item);
 
-	strcpy(mdev_link->mdev, page);
+	strcpy(mdev_link->device, page);
 	return count;
 }
 
@@ -241,17 +241,18 @@ static ssize_t mdev_link_comp_store(struct config_item *item,
 	return count;
 }
 
-static ssize_t mdev_link_param_show(struct config_item *item, char *page)
+static ssize_t mdev_link_comp_params_show(struct config_item *item, char *page)
 {
-	return snprintf(page, PAGE_SIZE, "%s\n", to_mdev_link(item)->param);
+	return snprintf(page, PAGE_SIZE, "%s\n",
+			to_mdev_link(item)->comp_params);
 }
 
-static ssize_t mdev_link_param_store(struct config_item *item,
-				     const char *page, size_t count)
+static ssize_t mdev_link_comp_params_store(struct config_item *item,
+					   const char *page, size_t count)
 {
 	struct mdev_link *mdev_link = to_mdev_link(item);
 
-	strcpy(mdev_link->param, page);
+	strcpy(mdev_link->comp_params, page);
 	return count;
 }
 
@@ -346,12 +347,12 @@ static ssize_t mdev_link_dbr_size_store(struct config_item *item,
 	return count;
 }
 
-CONFIGFS_ATTR_WO(mdev_link_, create);
-CONFIGFS_ATTR_WO(mdev_link_, destroy);
-CONFIGFS_ATTR(mdev_link_, mdev);
+CONFIGFS_ATTR_WO(mdev_link_, create_link);
+CONFIGFS_ATTR_WO(mdev_link_, destroy_link);
+CONFIGFS_ATTR(mdev_link_, device);
 CONFIGFS_ATTR(mdev_link_, channel);
 CONFIGFS_ATTR(mdev_link_, comp);
-CONFIGFS_ATTR(mdev_link_, param);
+CONFIGFS_ATTR(mdev_link_, comp_params);
 CONFIGFS_ATTR(mdev_link_, num_buffers);
 CONFIGFS_ATTR(mdev_link_, buffer_size);
 CONFIGFS_ATTR(mdev_link_, subbuffer_size);
@@ -361,12 +362,12 @@ CONFIGFS_ATTR(mdev_link_, direction);
 CONFIGFS_ATTR(mdev_link_, dbr_size);
 
 static struct configfs_attribute *mdev_link_attrs[] = {
-	&mdev_link_attr_create,
-	&mdev_link_attr_destroy,
-	&mdev_link_attr_mdev,
+	&mdev_link_attr_create_link,
+	&mdev_link_attr_destroy_link,
+	&mdev_link_attr_device,
 	&mdev_link_attr_channel,
 	&mdev_link_attr_comp,
-	&mdev_link_attr_param,
+	&mdev_link_attr_comp_params,
 	&mdev_link_attr_num_buffers,
 	&mdev_link_attr_buffer_size,
 	&mdev_link_attr_subbuffer_size,
@@ -383,7 +384,7 @@ static void mdev_link_release(struct config_item *item)
 	int ret;
 
 	if (!list_empty(&mdev_link_list)) {
-		ret = most_remove_link(mdev_link->mdev, mdev_link->channel,
+		ret = most_remove_link(mdev_link->device, mdev_link->channel,
 				       mdev_link->comp);
 		if (ret && (ret != -ENODEV))
 			pr_err("Removing link failed.\n");
@@ -481,7 +482,7 @@ static struct configfs_subsystem most_video_subsys = {
 
 struct most_snd_grp {
 	struct config_group group;
-	bool create;
+	bool create_card;
 	struct list_head list;
 };
 
@@ -500,14 +501,14 @@ static struct config_item *most_snd_grp_make_item(struct config_group *group,
 		return ERR_PTR(-ENOMEM);
 
 	config_item_init_type_name(&mdev_link->item, name, &mdev_link_type);
-	mdev_link->create = 0;
+	mdev_link->create_link = 0;
 	strcpy(mdev_link->name, name);
 	strcpy(mdev_link->comp, "sound");
 	return &mdev_link->item;
 }
 
-static ssize_t most_snd_grp_create_store(struct config_item *item,
-					 const char *page, size_t count)
+static ssize_t most_snd_grp_create_card_store(struct config_item *item,
+					      const char *page, size_t count)
 {
 	struct most_snd_grp *snd_grp = to_most_snd_grp(item);
 	int ret;
@@ -521,14 +522,14 @@ static ssize_t most_snd_grp_create_store(struct config_item *item,
 		if (ret)
 			return ret;
 	}
-	snd_grp->create = tmp;
+	snd_grp->create_card = tmp;
 	return count;
 }
 
-CONFIGFS_ATTR_WO(most_snd_grp_, create);
+CONFIGFS_ATTR_WO(most_snd_grp_, create_card);
 
 static struct configfs_attribute *most_snd_grp_attrs[] = {
-	&most_snd_grp_attr_create,
+	&most_snd_grp_attr_create_card,
 	NULL,
 };
 
@@ -568,7 +569,7 @@ static struct config_group *most_sound_make_group(struct config_group *group,
 					     struct most_sound, subsys);
 
 	list_for_each_entry(most, &ms->soundcard_list, list) {
-		if (!most->create) {
+		if (!most->create_card) {
 			pr_info("adapter configuration still in progress.\n");
 			return ERR_PTR(-EPROTO);
 		}
@@ -631,7 +632,7 @@ void most_interface_register_notify(const char *mdev)
 	struct mdev_link *mdev_link;
 
 	list_for_each_entry(mdev_link, &mdev_link_list, list) {
-		if (!strcmp(mdev_link->mdev, mdev)) {
+		if (!strcmp(mdev_link->device, mdev)) {
 			set_config_and_add_link(mdev_link);
 			if (!strcmp(mdev_link->comp, "sound"))
 				register_snd_card = true;
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 14/14] staging: most: Documentation: update driver documentation

[Christian Gromm]

This patch updates the driver documentation files to reflect the
latest changes regarding configfs.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
v2:
	- changed kernel version to 5.2

---
 .../most/Documentation/ABI/configfs-most.txt       | 204 +++++++++++++++++++++
 .../staging/most/Documentation/driver_usage.txt    | 131 +++++++------
 2 files changed, 284 insertions(+), 51 deletions(-)
 create mode 100644 drivers/staging/most/Documentation/ABI/configfs-most.txt

diff --git a/drivers/staging/most/Documentation/ABI/configfs-most.txt b/drivers/staging/most/Documentation/ABI/configfs-most.txt
new file mode 100644
index 0000000..25b3e18
--- /dev/null
+++ b/drivers/staging/most/Documentation/ABI/configfs-most.txt
@@ -0,0 +1,204 @@
+What: 		/sys/kernel/config/most_<component>
+Date: 		March 8, 2019
+KernelVersion:  5.2
+Description: 	Interface is used to configure and connect device channels
+		to component drivers.
+
+		Attributes are visible only when configfs is mounted. To mount
+		configfs in /sys/kernel/config directory use:
+		# mount -t configfs none /sys/kernel/config/
+
+
+What: 		/sys/kernel/config/most_cdev/<link>
+Date: 		March 8, 2019
+KernelVersion:  5.2
+Description:
+		The attributes:
+
+		buffer_size	configure the buffer size for this channel
+
+		subbuffer_size	configure the sub-buffer size for this channel
+				(needed for synchronous and isochrnous data)
+
+
+		num_buffers	configure number of buffers used for this
+				channel
+
+		datatype	configure type of data that will travel over
+				this channel
+
+		direction	configure whether this link will be an input
+				or output
+
+		dbr_size	configure DBR data buffer size (this is used
+				for MediaLB communiction only)
+
+		packets_per_xact
+				configure the number of packets that will be
+				collected from the network before being
+				transmitted via USB (this is used for USB
+				communiction only)
+
+		device		name of the device the link is to be attached to
+
+		channel		name of the channel the link is to be attached to
+
+		comp_params	pass parameters needed by some components
+
+		create_link	write '1' to this attribute to trigger the
+				creation of the link. In case of speculative
+				configuration, the creation is post-poned until
+				a physical device is being attached to the bus.
+
+		destroy_link	write '1' to this attribute to destroy an
+				active link
+
+What: 		/sys/kernel/config/most_video/<link>
+Date: 		March 8, 2019
+KernelVersion:  5.2
+Description:
+		The attributes:
+
+		buffer_size	configure the buffer size for this channel
+
+		subbuffer_size	configure the sub-buffer size for this channel
+				(needed for synchronous and isochrnous data)
+
+
+		num_buffers	configure number of buffers used for this
+				channel
+
+		datatype	configure type of data that will travel over
+				this channel
+
+		direction	configure whether this link will be an input
+				or output
+
+		dbr_size	configure DBR data buffer size (this is used
+				for MediaLB communiction only)
+
+		packets_per_xact
+				configure the number of packets that will be
+				collected from the network before being
+				transmitted via USB (this is used for USB
+				communiction only)
+
+		device		name of the device the link is to be attached to
+
+		channel		name of the channel the link is to be attached to
+
+		comp_params	pass parameters needed by some components
+
+		create_link	write '1' to this attribute to trigger the
+				creation of the link. In case of speculative
+				configuration, the creation is post-poned until
+				a physical device is being attached to the bus.
+
+		destroy_link	write '1' to this attribute to destroy an
+				active link
+
+What: 		/sys/kernel/config/most_net/<link>
+Date: 		March 8, 2019
+KernelVersion:  5.2
+Description:
+		The attributes:
+
+		buffer_size	configure the buffer size for this channel
+
+		subbuffer_size	configure the sub-buffer size for this channel
+				(needed for synchronous and isochrnous data)
+
+
+		num_buffers	configure number of buffers used for this
+				channel
+
+		datatype	configure type of data that will travel over
+				this channel
+
+		direction	configure whether this link will be an input
+				or output
+
+		dbr_size	configure DBR data buffer size (this is used
+				for MediaLB communiction only)
+
+		packets_per_xact
+				configure the number of packets that will be
+				collected from the network before being
+				transmitted via USB (this is used for USB
+				communiction only)
+
+		device		name of the device the link is to be attached to
+
+		channel		name of the channel the link is to be attached to
+
+		comp_params	pass parameters needed by some components
+
+		create_link	write '1' to this attribute to trigger the
+				creation of the link. In case of speculative
+				configuration, the creation is post-poned until
+				a physical device is being attached to the bus.
+
+		destroy_link	write '1' to this attribute to destroy an
+				active link
+
+What: 		/sys/kernel/config/most_sound/<card>
+Date: 		March 8, 2019
+KernelVersion:  5.2
+Description:
+		The attributes:
+
+		create_card	write '1' to this attribute to trigger the
+                                registration of the sound card with the ALSA
+				subsystem.
+
+What: 		/sys/kernel/config/most_sound/<card>/<link>
+Date: 		March 8, 2019
+KernelVersion:  5.2
+Description:
+		The attributes:
+
+		buffer_size	configure the buffer size for this channel
+
+		subbuffer_size	configure the sub-buffer size for this channel
+				(needed for synchronous and isochrnous data)
+
+
+		num_buffers	configure number of buffers used for this
+				channel
+
+		datatype	configure type of data that will travel over
+				this channel
+
+		direction	configure whether this link will be an input
+				or output
+
+		dbr_size	configure DBR data buffer size (this is used
+				for MediaLB communiction only)
+
+		packets_per_xact
+				configure the number of packets that will be
+				collected from the network before being
+				transmitted via USB (this is used for USB
+				communiction only)
+
+		device		name of the device the link is to be attached to
+
+		channel		name of the channel the link is to be attached to
+
+		comp_params	pass parameters needed by some components
+
+		create_link	write '1' to this attribute to trigger the
+				creation of the link. In case of speculative
+				configuration, the creation is post-poned until
+				a physical device is being attached to the bus.
+
+		destroy_link	write '1' to this attribute to destroy an
+				active link
+
+What: 		/sys/kernel/config/rdma_cm/<hca>/ports/<port-num>/default_roce_tos
+Date: 		March 8, 2019
+KernelVersion:  5.2
+Description: 	RDMA-CM QPs from HCA <hca> at port <port-num>
+		will be created with this TOS as default.
+		This can be overridden by using the rdma_set_option API.
+		The possible RoCE TOS values are 0-255.
diff --git a/drivers/staging/most/Documentation/driver_usage.txt b/drivers/staging/most/Documentation/driver_usage.txt
index da7a8f4..56d7919 100644
--- a/drivers/staging/most/Documentation/driver_usage.txt
+++ b/drivers/staging/most/Documentation/driver_usage.txt
@@ -115,36 +115,75 @@ following components are available
 
 		Section 2 Usage of the MOST Driver
 
-		Section 2.1 Configuration
-
-See ABI/sysfs-bus-most.txt
-
-
-		Section 2.2 Routing Channels
-
-To connect a configured channel to a certain core component and make it
-accessible for user space applications, the driver attribute 'add_link' is
-used. The configuration string passed to it has the following format:
-
-	"device_name:channel_name:component_name:link_name[.param]"
-
-It is the concatenation of up to four substrings separated by a colon. The
-substrings contain the names of the MOST interface, the channel, the
-component driver and a custom name with which the link is going to be
-referenced with. Since some components need additional information, the
-link name can be extended with a component-specific parameter (separated by
-a dot). In case the character device component is loaded, the handle would
-also appear as a device node in the /dev directory.
-
-Cdev component example:
-        $ echo "mdev0:ep_81:cdev:my_rx_channel" >$(DRV_DIR)/add_link
-
-
-Sound component example:
-
-The sound component needs additional parameters to determine the audio
-resolution that is going to be used and to trigger the registration of a
-sound card with ALSA. The following audio formats are available:
+		Section 2.1 Configuration and Data Link
+
+The driver is to be configured via configfs. Each loaded component kernel
+object (see section 1.3) registers a subsystem with configfs, which is used to
+configure and establish communiction pathways (links) to attached devices on
+the bus. To do so, the user has to descend into the component's configuration
+directory and create a new directory (child config itmes). The name of this
+directory will be used as a reference for the link and it will contain the
+following attributes:
+
+	- buffer_size
+	  configure the buffer size for this channel
+	- subbuffer_size
+	  configure the sub-buffer size for this channel (needed for
+	  synchronous and isochrnous data)
+	- num_buffers
+	  configure number of buffers used for this channel
+	- datatype
+	  configure type of data that will travel over this channel
+	- direction
+	  configure whether this link will be an input or output
+	- dbr_size
+	  configure DBR data buffer size (this is used for MediaLB communiction
+	  only)
+	- packets_per_xact
+	  configure the number of packets that will be collected from the
+	  network before being transmitted via USB (this is used for USB
+	  communiction only)
+	- device
+	  name of the device the link is to be attached to
+	- channel
+	  name of the channel the link is to be attached to
+	- comp_params
+	  pass parameters needed by some components
+	- create_link
+	  write '1' to this attribute to trigger the creation of the link. In
+	  case of speculative configuration, the creation is post-poned until
+	  a physical device is being attached to the bus.
+	- destroy_link
+	  write '1' to this attribute to destroy an already established link
+
+
+See ABI/sysfs-bus-most.txt and ABI/configfs-most.txt
+
+
+		Section 2.2 Configure a Sound Card
+
+Setting up synchronous channels to be mapped as an ALSA sound adapter is a two
+step process. Firstly, a directory (child config group) has to be created
+inside the most_sound's configuration directory. This adapter dir will
+represent the sound adapter. The name of the directory is for user reference
+only and has no further influence, as all sound adapters will be given a static
+name in ALSA. The sound adapter will have the following attribute:
+
+	- create_card
+	  write '1' to this attribute to trigger the registration of the card
+	  with the ALSA subsystem.
+	  In case of speculative configuration, the creation is post-poned
+	  until a physical device is being attached to the bus.
+
+Secondly, links will have to be created inside the adapter dir as described in
+section 2.1. These links will become the PCM devices of the sound card. The
+name of a PCM device will be inherited from the directory name. When all
+channels have been configured and created, the sound card itself can be created
+by writing '1' to the create_card attribute.
+
+The sound component needs an additional parameter to determine the audio
+resolution that is going to be used.
+The following audio formats are available:
 
 	- "1x8" (Mono)
 	- "2x16" (16-bit stereo)
@@ -152,18 +191,8 @@ sound card with ALSA. The following audio formats are available:
 	- "2x32" (32-bit stereo)
 	- "6x16" (16-bit surround 5.1)
 
-To make the sound module create a sound card and register it with ALSA the
-string "create" needs to be attached to the module parameter section of the
-configuration string. To create a sound card with with two playback devices
-(linked to channel ep01 and ep02) and one capture device (linked to channel
-ep83) the following is written to the add_link file:
-
-        $ echo "mdev0:ep01:sound:most51_playback.6x16" >$(DRV_DIR)/add_link
-        $ echo "mdev0:ep02:sound:most_playback.2x16" >$(DRV_DIR)/add_link
-        $ echo "mdev0:ep83:sound:most_capture.2x16.create" >$(DRV_DIR)/add_link
-
-The link names (most51_playback, most_playback and most_capture) will
-become the names of the PCM devices of the sound card.
+The resolution string has to be written to the link directory's comp_params
+attribute.
 
 		Section 2.3 USB Padding
 
@@ -174,13 +203,13 @@ hardware, which is for performance optimization purposes of the USB
 transmission.
 
 When transmitting synchronous data the allocated channel width needs to be
-written to 'set_subbuffer_size'. Additionally, the number of MOST frames
-that should travel to the host within one USB transaction need to be
-written to 'packets_per_xact'.
+written to 'subbuffer_size'. Additionally, the number of MOST frames that
+should travel to the host within one USB transaction need to be written to
+'packets_per_xact'.
 
 The driver, then, calculates the synchronous threshold as follows:
 
-	frame_size = set_subbuffer_size * packets_per_xact
+	frame_size = subbuffer_size * packets_per_xact
 
 In case 'packets_per_xact' is set to 0xFF the maximum number of packets,
 allocated within one MOST frame, is calculated that fit into _one_ 512 byte
@@ -192,15 +221,15 @@ This frame_size is the number of synchronous data within an USB
 transaction, which renders MTU_USB - frame_size bytes for padding.
 
 When transmitting isochronous AVP data the desired packet size needs to be
-written to 'set_subbuffer_size' and hardware will always expect two
-isochronous packets within one USB transaction. This renders
+written to 'subbuffer_size' and hardware will always expect two isochronous
+packets within one USB transaction. This renders
 
-	MTU_USB - (2 * set_subbuffer_size)
+	MTU_USB - (2 * subbuffer_size)
 
 bytes for padding.
 
-Note that at least (2 * set_subbuffer_size) bytes for isochronous data or
-(set_subbuffer_size * packts_per_xact) bytes for synchronous data need to
+Note that at least (2 * subbuffer_size) bytes for isochronous data or
+(subbuffer_size * packts_per_xact) bytes for synchronous data need to
 be put in the transmission buffer and passed to the driver.
 
 Since adapter drivers are allowed to change a chosen configuration to best
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 01/14] staging: most: add new file configfs.c

[Dan Carpenter]

On Thu, Mar 28, 2019 at 02:17:29PM +0100, Christian Gromm wrote:
> +static ssize_t mdev_link_direction_store(struct config_item *item,
> +					 const char *page, size_t count)
> +{
> +	struct mdev_link *mdev_link = to_mdev_link(item);
> +
> +	if (sysfs_streq(page, "dir_rx") && sysfs_streq(page, "rx") &&
> +	    sysfs_streq(page, "dir_tx") && sysfs_streq(page, "tx"))

These tests are reversed.  It will never return -EINVAL because one
string can't be four things.

	if (!sysfs_streq(page, "dir_rx") && !sysfs_streq(page, "rx") &&
	    !sysfs_streq(page, "dir_tx") && !sysfs_streq(page, "tx"))
		return -EINVAL;

The sysfs_streq() return true if the strings are equal.  The strcmp()
functions less intuitive and they should be used like this:

	if (strcmp(foo, bar) < 0) {  // <-- foo < bar
	if (strcmp(foo, bar) != 0) { // <-- foo != bar
	if (strcmp(foo, bar) == 0) { // <-- foo == bar

The other streq() tests have the same issue.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 03/14] staging: most: core: add configfs interface functions

[Dan Carpenter]

On Thu, Mar 28, 2019 at 02:17:31PM +0100, Christian Gromm wrote:
> +int most_set_cfg_direction(char *mdev, char *mdev_ch, char *buf)
> +{
> +	struct most_channel *c = get_channel(mdev, mdev_ch);
> +
> +	if (!c)
> +		return -ENODEV;
> +	if (!strcmp(buf, "dir_rx\n")) {
> +		c->cfg.direction = MOST_CH_RX;
> +	} else if (!strcmp(buf, "rx\n")) {
> +		c->cfg.direction = MOST_CH_RX;
> +	} else if (!strcmp(buf, "dir_tx\n")) {
> +		c->cfg.direction = MOST_CH_TX;
> +	} else if (!strcmp(buf, "tx\n")) {
> +		c->cfg.direction = MOST_CH_TX;
> +	} else {
> +		pr_info("Invalid direction\n");
> +		return -ENODATA;
> +	}

Please use sysfs_streq().

> +	return 0;
> +}

regards,
dan carpenter
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 04/14] staging: most: sound: introduce new sound adapter management

[Dan Carpenter]

On Thu, Mar 28, 2019 at 02:17:32PM +0100, Christian Gromm wrote:
> +static int audio_create_sound_card(void)
> +{
> +	int ret;
> +	struct sound_adapter *adpt;
> +
> +	list_for_each_entry(adpt, &adpt_list, list) {
> +		if (!adpt->registered)
> +			goto adpt_alloc;
> +	}
> +	return -ENODEV;
> +adpt_alloc:
> +	ret = snd_card_register(adpt->card);
> +	if (ret < 0) {
> +		release_adapter(adpt);

This doesn't feel right.  We didn't acquire "adpt" in this function so
why are we releasing it here.  Do we release it somewhere else as well?

It's still on the list...

> +		return ret;
> +	}
> +	adpt->registered = true;
> +	return ret;
> +}

regards,
dan carpenter


_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 10/14] staging: most: allow speculative configuration

[Dan Carpenter]

On Thu, Mar 28, 2019 at 02:17:38PM +0100, Christian Gromm wrote:
> This patch makes the driver accept a link confiiguration eventhough no
> device is attached to the bus. Instead the configuration is being applied
> as soon as a device is being registered with the core.
> 
> Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> ---
> v2:
> 	- follow-up adaptions due to changes introduced w/ [PATCH v2 01/14]
> 
>  drivers/staging/most/configfs.c    | 60 ++++++++++++++++++++++++++++----------
>  drivers/staging/most/core.c        | 16 +++-------
>  drivers/staging/most/core.h        |  1 +
>  drivers/staging/most/sound/sound.c |  6 ++--
>  4 files changed, 53 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/staging/most/configfs.c b/drivers/staging/most/configfs.c
> index 6968b299..3a7c54d 100644
> --- a/drivers/staging/most/configfs.c
> +++ b/drivers/staging/most/configfs.c
> @@ -14,6 +14,7 @@
>  
>  struct mdev_link {
>  	struct config_item item;
> +	struct list_head list;
>  	bool create;
>  	u16 num_buffers;
>  	u16 buffer_size;
> @@ -29,6 +30,8 @@ struct mdev_link {
>  	char param[PAGE_SIZE];
>  };
>  
> +struct list_head mdev_link_list;
> +
>  static int set_cfg_buffer_size(struct mdev_link *link)
>  {
>  	if (!link->buffer_size)
> @@ -105,33 +108,41 @@ static ssize_t mdev_link_create_show(struct config_item *item, char *page)
>  	return snprintf(page, PAGE_SIZE, "%d\n", to_mdev_link(item)->create);
>  }
>  
> +static int set_config_and_add_link(struct mdev_link *mdev_link)
> +{
> +	int i;
> +	int ret;
> +
> +	for (i = 0; i < ARRAY_SIZE(set_config_val); i++) {
> +		ret = set_config_val[i](mdev_link);
> +		if (ret == -ENODATA) {

I've read the commit description but I don't really understand the
error codes.  Here only -ENODATA is an error.  But later -ENODEV
is success.  Why not "if (ret) {" here?


> +			pr_err("Config failed\n");
> +			return ret;
> +		}
> +	}
> +
> +	return most_add_link(mdev_link->mdev, mdev_link->channel,
> +			     mdev_link->comp, mdev_link->name,
> +			     mdev_link->param);
> +}
> +
>  static ssize_t mdev_link_create_store(struct config_item *item,
>  				      const char *page, size_t count)
>  {
>  	struct mdev_link *mdev_link = to_mdev_link(item);
>  	bool tmp;
>  	int ret;
> -	int i;
>  
>  	ret = kstrtobool(page, &tmp);
>  	if (ret)
>  		return ret;
> -
> -	for (i = 0; i < ARRAY_SIZE(set_config_val); i++) {
> -		ret = set_config_val[i](mdev_link);
> -		if (ret < 0)
> -			return ret;
> -	}
> -
> -	if (tmp)
> -		ret = most_add_link(mdev_link->mdev, mdev_link->channel,
> -				    mdev_link->comp, mdev_link->name,
> -				    mdev_link->param);
> -	else
> -		ret = most_remove_link(mdev_link->mdev, mdev_link->channel,
> -				       mdev_link->comp);
> -	if (ret)
> +	if (!tmp)
> +		return most_remove_link(mdev_link->mdev, mdev_link->channel,
> +					mdev_link->comp);
> +	ret = set_config_and_add_link(mdev_link);
> +	if (ret && ret != -ENODEV)

ENODEV is success.  I feel like this needs to be explained in comments
in the code.

>  		return ret;
> +	list_add_tail(&mdev_link->list, &mdev_link_list);
>  	mdev_link->create = tmp;
>  	return count;
>  }
> @@ -590,6 +601,22 @@ int most_register_configfs_subsys(struct core_component *c)
>  }
>  EXPORT_SYMBOL_GPL(most_register_configfs_subsys);
>  
> +void most_interface_register_notify(const char *mdev)
> +{
> +	bool register_snd_card = false;
> +	struct mdev_link *mdev_link;
> +
> +	list_for_each_entry(mdev_link, &mdev_link_list, list) {
> +		if (!strcmp(mdev_link->mdev, mdev)) {
> +			set_config_and_add_link(mdev_link);

Here the errors are not checked.

> +			if (!strcmp(mdev_link->comp, "sound"))
> +				register_snd_card = true;
> +		}
> +	}
> +	if (register_snd_card)
> +		most_cfg_complete("sound");
> +}
> +

[ snip ]

> diff --git a/drivers/staging/most/sound/sound.c b/drivers/staging/most/sound/sound.c
> index fd089e6..44c9146 100644
> --- a/drivers/staging/most/sound/sound.c
> +++ b/drivers/staging/most/sound/sound.c
> @@ -20,6 +20,7 @@
>  #include <most/core.h>
>  
>  #define DRIVER_NAME "sound"
> +#define STRING_SIZE	80
>  
>  static struct core_component comp;
>  
> @@ -582,6 +583,7 @@ static int audio_probe_channel(struct most_interface *iface, int channel_id,
>  	int direction;
>  	u16 ch_num;
>  	char *sample_res;
> +	char arg_list_cpy[STRING_SIZE];
>  
>  	if (!iface)
>  		return -EINVAL;
> @@ -590,8 +592,8 @@ static int audio_probe_channel(struct most_interface *iface, int channel_id,
>  		pr_err("Incompatible channel type\n");
>  		return -EINVAL;
>  	}
> -
> -	ret = split_arg_list(arg_list, &ch_num, &sample_res);
> +	strlcpy(arg_list_cpy, arg_list, STRING_SIZE);
> +	ret = split_arg_list(arg_list_cpy, &ch_num, &sample_res);


I don't understand why we need a copy of arg_list or how this relates
to the rest of the patch.

>  	if (ret < 0)
>  		return ret;
>  

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 10/14] staging: most: allow speculative configuration

[Christian.Gromm]

On Do, 2019-03-28 at 17:12 +0300, Dan Carpenter wrote:
> External E-Mail
> 
> 
> On Thu, Mar 28, 2019 at 02:17:38PM +0100, Christian Gromm wrote:
> > 
> > This patch makes the driver accept a link confiiguration eventhough
> > no
> > device is attached to the bus. Instead the configuration is being
> > applied
> > as soon as a device is being registered with the core.
> > 
> > Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> > ---
> > v2:
> > 	- follow-up adaptions due to changes introduced w/ [PATCH v2
> > 01/14]
> > 
> >  drivers/staging/most/configfs.c    | 60
> > ++++++++++++++++++++++++++++----------
> >  drivers/staging/most/core.c        | 16 +++-------
> >  drivers/staging/most/core.h        |  1 +
> >  drivers/staging/most/sound/sound.c |  6 ++--
> >  4 files changed, 53 insertions(+), 30 deletions(-)
> > 
> > diff --git a/drivers/staging/most/configfs.c
> > b/drivers/staging/most/configfs.c
> > index 6968b299..3a7c54d 100644
> > --- a/drivers/staging/most/configfs.c
> > +++ b/drivers/staging/most/configfs.c
> > @@ -14,6 +14,7 @@
> >  
> >  struct mdev_link {
> >  	struct config_item item;
> > +	struct list_head list;
> >  	bool create;
> >  	u16 num_buffers;
> >  	u16 buffer_size;
> > @@ -29,6 +30,8 @@ struct mdev_link {
> >  	char param[PAGE_SIZE];
> >  };
> >  
> > +struct list_head mdev_link_list;
> > +
> >  static int set_cfg_buffer_size(struct mdev_link *link)
> >  {
> >  	if (!link->buffer_size)
> > @@ -105,33 +108,41 @@ static ssize_t mdev_link_create_show(struct
> > config_item *item, char *page)
> >  	return snprintf(page, PAGE_SIZE, "%d\n",
> > to_mdev_link(item)->create);
> >  }
> >  
> > +static int set_config_and_add_link(struct mdev_link *mdev_link)
> > +{
> > +	int i;
> > +	int ret;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(set_config_val); i++) {
> > +		ret = set_config_val[i](mdev_link);
> > +		if (ret == -ENODATA) {
> I've read the commit description but I don't really understand the
> error codes.  Here only -ENODATA is an error.  But later -ENODEV
> is success.  Why not "if (ret) {" here?
> 

The most_set_cfg_* functions return success, ENODEV or ENODATA.
We want to stop only in case there is something wrong with the
provided data. A missing device is not an issue here. In this
case we want to keep the configuration as is and complete stuff
once a device is being attached.
 
> 
> > 
> > +			pr_err("Config failed\n");
> > +			return ret;
> > +		}
> > +	}
> > +
> > +	return most_add_link(mdev_link->mdev, mdev_link->channel,
> > +			     mdev_link->comp, mdev_link->name,
> > +			     mdev_link->param);
> > +}
> > +
> >  static ssize_t mdev_link_create_store(struct config_item *item,
> >  				      const char *page, size_t
> > count)
> >  {
> >  	struct mdev_link *mdev_link = to_mdev_link(item);
> >  	bool tmp;
> >  	int ret;
> > -	int i;
> >  
> >  	ret = kstrtobool(page, &tmp);
> >  	if (ret)
> >  		return ret;
> > -
> > -	for (i = 0; i < ARRAY_SIZE(set_config_val); i++) {
> > -		ret = set_config_val[i](mdev_link);
> > -		if (ret < 0)
> > -			return ret;
> > -	}
> > -
> > -	if (tmp)
> > -		ret = most_add_link(mdev_link->mdev, mdev_link-
> > >channel,
> > -				    mdev_link->comp, mdev_link-
> > >name,
> > -				    mdev_link->param);
> > -	else
> > -		ret = most_remove_link(mdev_link->mdev, mdev_link-
> > >channel,
> > -				       mdev_link->comp);
> > -	if (ret)
> > +	if (!tmp)
> > +		return most_remove_link(mdev_link->mdev,
> > mdev_link->channel,
> > +					mdev_link->comp);
> > +	ret = set_config_and_add_link(mdev_link);
> > +	if (ret && ret != -ENODEV)
> ENODEV is success.  I feel like this needs to be explained in
> comments
> in the code.

ENODEV is not a show-stopper. It only postpones the configuration
process until the driver is being notified about a new device.

> 
> > 
> >  		return ret;
> > +	list_add_tail(&mdev_link->list, &mdev_link_list);
> >  	mdev_link->create = tmp;
> >  	return count;
> >  }
> > @@ -590,6 +601,22 @@ int most_register_configfs_subsys(struct
> > core_component *c)
> >  }
> >  EXPORT_SYMBOL_GPL(most_register_configfs_subsys);
> >  
> > +void most_interface_register_notify(const char *mdev)
> > +{
> > +	bool register_snd_card = false;
> > +	struct mdev_link *mdev_link;
> > +
> > +	list_for_each_entry(mdev_link, &mdev_link_list, list) {
> > +		if (!strcmp(mdev_link->mdev, mdev)) {
> > +			set_config_and_add_link(mdev_link);
> Here the errors are not checked.

We are in the notify function. That means a device is present
now. And if the list isn't empty, there is also a valid configuration
available. So, set_config_and_add_link should not fail.

> 
> > 
> > +			if (!strcmp(mdev_link->comp, "sound"))
> > +				register_snd_card = true;
> > +		}
> > +	}
> > +	if (register_snd_card)
> > +		most_cfg_complete("sound");
> > +}
> > +
> [ snip ]
> 
> > 
> > diff --git a/drivers/staging/most/sound/sound.c
> > b/drivers/staging/most/sound/sound.c
> > index fd089e6..44c9146 100644
> > --- a/drivers/staging/most/sound/sound.c
> > +++ b/drivers/staging/most/sound/sound.c
> > @@ -20,6 +20,7 @@
> >  #include <most/core.h>
> >  
> >  #define DRIVER_NAME "sound"
> > +#define STRING_SIZE	80
> >  
> >  static struct core_component comp;
> >  
> > @@ -582,6 +583,7 @@ static int audio_probe_channel(struct
> > most_interface *iface, int channel_id,
> >  	int direction;
> >  	u16 ch_num;
> >  	char *sample_res;
> > +	char arg_list_cpy[STRING_SIZE];
> >  
> >  	if (!iface)
> >  		return -EINVAL;
> > @@ -590,8 +592,8 @@ static int audio_probe_channel(struct
> > most_interface *iface, int channel_id,
> >  		pr_err("Incompatible channel type\n");
> >  		return -EINVAL;
> >  	}
> > -
> > -	ret = split_arg_list(arg_list, &ch_num, &sample_res);
> > +	strlcpy(arg_list_cpy, arg_list, STRING_SIZE);
> > +	ret = split_arg_list(arg_list_cpy, &ch_num, &sample_res);
> 
> I don't understand why we need a copy of arg_list or how this relates
> to the rest of the patch.

The function split_arg_list modifies the argument. So if we want to
read it back later in a show function, we need to have a clean copy.

> 
> > 
> >  	if (ret < 0)
> >  		return ret;
> >  
> regards,
> dan carpenter
> 
> 
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 01/14] staging: most: add new file configfs.c

[Christian.Gromm]

On Do, 2019-03-28 at 16:50 +0300, Dan Carpenter wrote:
> External E-Mail
> 
> 
> On Thu, Mar 28, 2019 at 02:17:29PM +0100, Christian Gromm wrote:
> > 
> > +static ssize_t mdev_link_direction_store(struct config_item *item,
> > +					 const char *page, size_t
> > count)
> > +{
> > +	struct mdev_link *mdev_link = to_mdev_link(item);
> > +
> > +	if (sysfs_streq(page, "dir_rx") && sysfs_streq(page, "rx")
> > &&
> > +	    sysfs_streq(page, "dir_tx") && sysfs_streq(page,
> > "tx"))
> These tests are reversed.  It will never return -EINVAL because one
> string can't be four things.

OMG, that was braindead. Sorry for the inconvenience. I'll fix that up.

thanks,
Chris

> 
> 	if (!sysfs_streq(page, "dir_rx") && !sysfs_streq(page, "rx") &&
> 	    !sysfs_streq(page, "dir_tx") && !sysfs_streq(page, "tx"))
> 		return -EINVAL;
> 
> The sysfs_streq() return true if the strings are equal.  The strcmp()
> functions less intuitive and they should be used like this:
> 
> 	if (strcmp(foo, bar) < 0) {  // <-- foo < bar
> 	if (strcmp(foo, bar) != 0) { // <-- foo != bar
> 	if (strcmp(foo, bar) == 0) { // <-- foo == bar
> 
> The other streq() tests have the same issue.
> 
> regards,
> dan carpenter
> 
> 
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 10/14] staging: most: allow speculative configuration

[Dan Carpenter]

On Fri, Mar 29, 2019 at 09:15:46AM +0000, Christian.Gromm@microchip.com wrote:
> > > +static int set_config_and_add_link(struct mdev_link *mdev_link)
> > > +{
> > > +	int i;
> > > +	int ret;
> > > +
> > > +	for (i = 0; i < ARRAY_SIZE(set_config_val); i++) {
> > > +		ret = set_config_val[i](mdev_link);
> > > +		if (ret == -ENODATA) {
> > I've read the commit description but I don't really understand the
> > error codes.  Here only -ENODATA is an error.  But later -ENODEV
> > is success.  Why not "if (ret) {" here?
> > 
> 
> The most_set_cfg_* functions return success, ENODEV or ENODATA.
> We want to stop only in case there is something wrong with the
> provided data. A missing device is not an issue here. In this
> case we want to keep the configuration as is and complete stuff
> once a device is being attached.
>  

Yeah...  But future programmers will just add whatever error codes
occur to them at the time.  Now we have two error codes which are
special instead of one (twice as much chance for them to mess up).

Just do it like:

		if (ret < 0 && ret != -ENODEV) {

That's a more standard way to handle this.

regards,
dan carpenter


_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 04/14] staging: most: sound: introduce new sound adapter management

[Christian.Gromm]

On Do, 2019-03-28 at 16:56 +0300, Dan Carpenter wrote:
> External E-Mail
> 
> 
> On Thu, Mar 28, 2019 at 02:17:32PM +0100, Christian Gromm wrote:
> > 
> > +static int audio_create_sound_card(void)
> > +{
> > +	int ret;
> > +	struct sound_adapter *adpt;
> > +
> > +	list_for_each_entry(adpt, &adpt_list, list) {
> > +		if (!adpt->registered)
> > +			goto adpt_alloc;
> > +	}
> > +	return -ENODEV;
> > +adpt_alloc:
> > +	ret = snd_card_register(adpt->card);
> > +	if (ret < 0) {
> > +		release_adapter(adpt);
> This doesn't feel right.  We didn't acquire "adpt" in this function
> so
> why are we releasing it here.  Do we release it somewhere else as
> well?
> 

We release the adapter, because it is useless if we have not been
able to register it with ALSA.

And yes, it is also being removed, when a channel gets disconnected.

> It's still on the list...

It is being removed from the list inside the function
release_adapter.

thanks,
Chris

> 
> > 
> > +		return ret;
> > +	}
> > +	adpt->registered = true;
> > +	return ret;
> > +}
> regards,
> dan carpenter
> 
> 
> 
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 04/14] staging: most: sound: introduce new sound adapter management

[Dan Carpenter]

On Thu, Mar 28, 2019 at 02:17:32PM +0100, Christian Gromm wrote:
> +static int audio_create_sound_card(void)
> +{
> +	int ret;
> +	struct sound_adapter *adpt;
> +
> +	list_for_each_entry(adpt, &adpt_list, list) {
> +		if (!adpt->registered)
> +			goto adpt_alloc;
> +	}
> +	return -ENODEV;
> +adpt_alloc:
> +	ret = snd_card_register(adpt->card);
> +	if (ret < 0) {
> +		release_adapter(adpt);
> +		return ret;
> +	}
> +	adpt->registered = true;
> +	return ret;
        ^^^^^^^^^^

	return 0;

> +}

This function is just strange to me...  We add a bunch of adapters to
"adpt_list" in audio_probe_channel().  Each adapter has it's own
adpt->card.  But here we just take the first unregistered adapter and
register it as our card.

regards,
dan carpenter
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 04/14] staging: most: sound: introduce new sound adapter management

[Christian.Gromm]

On Fr, 2019-03-29 at 13:46 +0300, Dan Carpenter wrote:
> External E-Mail
> 
> 
> On Thu, Mar 28, 2019 at 02:17:32PM +0100, Christian Gromm wrote:
> > 
> > +static int audio_create_sound_card(void)
> > +{
> > +	int ret;
> > +	struct sound_adapter *adpt;
> > +
> > +	list_for_each_entry(adpt, &adpt_list, list) {
> > +		if (!adpt->registered)
> > +			goto adpt_alloc;
> > +	}
> > +	return -ENODEV;
> > +adpt_alloc:
> > +	ret = snd_card_register(adpt->card);
> > +	if (ret < 0) {
> > +		release_adapter(adpt);
> > +		return ret;
> > +	}
> > +	adpt->registered = true;
> > +	return ret;
>         ^^^^^^^^^^
> 
> 	return 0;
> 
> > 
> > +}
> This function is just strange to me...  We add a bunch of adapters to
> "adpt_list" in audio_probe_channel().  Each adapter has it's own
> adpt->card.  But here we just take the first unregistered adapter and
> register it as our card.
> 
Unfortunately, I am not an ASCII artist, otherwise I would have put
some flowcharts in here to make things clear.
But in principle, it is like this:

A channel of a device which is intended to carry audio data,
will be represented as a PCM device on a sound card. So, if
I want my sound card

 --> mkdir /sys/kernel/config/most_sound/mycard 

to have a capture device and a playback device, I would
link two channels (one rx and on tx) and hook them on the card by
creating a directory in the card's directory.

 --> mkdir /sys/kernel/config/most_sound/mycard/pcm_capture
 --> mkdir
/sys/kernel/config/most_sound/mycard/pcm_playback		​
Then I would go ahead and configure the attributes inside and
finally activate the links:

 --> echo 1 > /sys/kernel/config/most_sound/mycard/pcm_playback/create_link

and

 --> echo 1 > /sys/kernel/config/most_sound/mycard/pcm_capture/create_link


The first time a link is being activated, the probe function gets
called and if no adapter has yet been created, one is created now
and the PCM device is added. By the time the second call happens,
we already have an adapter that is _not_ registered with ALSA.
Hence, we skip allocating another adapter and use the one we got
to add the next PCM device.

Once I have all PCM devices set up, I would do

 --> echo 1 > /sys/kernel/config/most_sound/mycard/create_card
 
And now, the "strange" audio_create_sound_card function is
called to register the adapter with ALSA. If everything is fine,
the adpt->registered flag is set and from now on I would be able
to create a second sound card on the system. An only now a new
adapter would be allocated in the probe function, because we
can't find one in our list that is waiting to get registered
with ALSA.

Unless I do the create_card thing, all channels would be added
to the same adapter and I would end up with a sound card that
has "a bunch" of PCM devices.

Hope I kind of could get the picture across.

thanks,
Chris



> regards,
> dan carpenter
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-de
> vel
> 
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 04/14] staging: most: sound: introduce new sound adapter management

[Dan Carpenter]

Thanks, I feel like I understand better now.

Sorry, I don't want to be a jerk and I'm not going to complain again
about this patchset if you resend it with the other stuff I mention
fixed.

But to me it feels like the API could be improved slightly if you
first created the adapter with a name and then linked to it.  Creating
the adapter as a side effect of linking the first audio component feels
like it seems helpful but it's going to be awkward.

I'm also concerned about the locking around the adpt_list.  Is this
something ordinary users can trigger?  Is it crash if I set my fuzz
tester to start randomly connecting and disconnecting like crazy?

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 04/14] staging: most: sound: introduce new sound adapter management

[Christian.Gromm]

On Fr, 2019-03-29 at 16:50 +0300, Dan Carpenter wrote:
> External E-Mail
> 
> 
> Thanks, I feel like I understand better now.
> 
> Sorry, I don't want to be a jerk and I'm not going to complain again
> about this patchset if you resend it with the other stuff I mention
> fixed.
> 
> But to me it feels like the API could be improved slightly if you
> first created the adapter with a name and then linked to
> it.  Creating
> the adapter as a side effect of linking the first audio component
> feels
> like it seems helpful but it's going to be awkward.

Hmm, I see. But I'm not sure if I can call snd_pcm_new() on an
already registered sound adapter.

> 
> I'm also concerned about the locking around the adpt_list.  Is this
> something ordinary users can trigger?  Is it crash if I set my fuzz
> tester to start randomly connecting and disconnecting like crazy?
> 
Well, strictly speaking yes. Because it is user space. But normally,
the driver is meant for engineered systems like a car (see Automotive
Grade Linux).

Here you will have a systemd unit that sets things up at system
startup time. 
I don't expect having to deal with multiple user space applications
that race about the sound adapters. But I will keep that in mind.

thanks,
Chris

> regards,
> dan carpenter
> 
> 
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel