[PATCH RFC v3 0/9] staging: most: move core module out of staging

[PATCH RFC v3 0/9] staging: most: move core module out of staging

[Christian Gromm]

The MOST driver was pushed to the staging area with kernel 4.3. Since then
it has encountered many refinements by the community and should be ready
for an upstream audit and to be moved out of the staging area. Since the
driver consists of multiple modules the core module is about to go first
and the other modules will follow subsequently. This patchset executes the
necessary steps to move the core module out of staging.

v2:
Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
	- use -M option to create patches
v3:
Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
	- fix date range in comment section of core.c
	- move code to free up memory to release funtions
	- remove noisy log messages
	- use dev_* functions for logging

Christian Gromm (9):
  staging: most: core: fix date in file comment
  staging: most: core: use dev_* function for logging
  staging: most: core: remove noisy log messages
  staging: most: move interface dev to private section
  staging: most: usb: check for NULL device
  staging: most: change storage class of struct mostcore
  staging: most: move core files out of the staging area
  staging: most: Documentation: update ABI description
  staging: most: Documentation: move ABI description files out of
    staging area

 .../ABI/testing/configfs-most                      |   8 -
 .../ABI/testing/sysfs-bus-most                     |  24 +--
 drivers/Kconfig                                    |   1 +
 drivers/Makefile                                   |   1 +
 drivers/most/Kconfig                               |  15 ++
 drivers/most/Makefile                              |   4 +
 drivers/{staging => }/most/configfs.c              |   2 +-
 drivers/{staging => }/most/core.c                  | 187 ++++++++++++---------
 drivers/staging/most/Kconfig                       |   6 +-
 drivers/staging/most/Makefile                      |   5 -
 drivers/staging/most/cdev/cdev.c                   |   2 +-
 drivers/staging/most/dim2/dim2.c                   |   4 +-
 drivers/staging/most/i2c/i2c.c                     |   2 +-
 drivers/staging/most/net/net.c                     |   2 +-
 drivers/staging/most/sound/sound.c                 |   2 +-
 drivers/staging/most/usb/usb.c                     |  11 +-
 drivers/staging/most/video/video.c                 |   2 +-
 {drivers/staging/most => include/linux}/most.h     |   6 +-
 18 files changed, 148 insertions(+), 136 deletions(-)
 rename drivers/staging/most/Documentation/ABI/configfs-most.txt => Documentation/ABI/testing/configfs-most (94%)
 rename drivers/staging/most/Documentation/ABI/sysfs-bus-most.txt => Documentation/ABI/testing/sysfs-bus-most (92%)
 create mode 100644 drivers/most/Kconfig
 create mode 100644 drivers/most/Makefile
 rename drivers/{staging => }/most/configfs.c (99%)
 rename drivers/{staging => }/most/core.c (90%)
 rename {drivers/staging/most => include/linux}/most.h (99%)

-- 
2.7.4

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

[PATCH RFC v3 1/9] staging: most: core: fix date in file comment

[Christian Gromm]

This patch updates the date range in the comment section.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
v3:
	This patch has been added to the series.

 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 af542ed..8c8a647 100644
--- a/drivers/staging/most/core.c
+++ b/drivers/staging/most/core.c
@@ -2,7 +2,7 @@
 /*
  * core.c - Implementation of core module of MOST Linux driver stack
  *
- * Copyright (C) 2013-2015 Microchip Technology Germany II GmbH & Co. KG
+ * Copyright (C) 2013-2020 Microchip Technology Germany II GmbH & Co. KG
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-- 
2.7.4

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

[PATCH RFC v3 2/9] staging: most: core: use dev_* function for logging

[Christian Gromm]

This patch replaces all calls to pr_* functions and uses
the dev_* functions instead.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
v3:
	This patch has been added to the series.

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

diff --git a/drivers/staging/most/core.c b/drivers/staging/most/core.c
index 8c8a647..9839b8c 100644
--- a/drivers/staging/most/core.c
+++ b/drivers/staging/most/core.c
@@ -5,7 +5,6 @@
  * Copyright (C) 2013-2020 Microchip Technology Germany II GmbH & Co. KG
  */
 
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 #include <linux/module.h>
 #include <linux/fs.h>
 #include <linux/slab.h>
@@ -151,7 +150,7 @@ static void flush_channel_fifos(struct most_channel *c)
 	spin_unlock_irqrestore(&c->fifo_lock, hf_flags);
 
 	if (unlikely((!list_empty(&c->fifo) || !list_empty(&c->halt_fifo))))
-		pr_info("WARN: fifo | trash fifo not empty\n");
+		dev_warn(&mc.dev, "fifo | trash fifo not empty\n");
 }
 
 /**
@@ -623,7 +622,7 @@ int most_set_cfg_datatype(char *mdev, char *mdev_ch, char *buf)
 	}
 
 	if (i == ARRAY_SIZE(ch_data_type))
-		pr_info("WARN: invalid attribute settings\n");
+		dev_warn(&mc.dev, "invalid attribute settings\n");
 	return 0;
 }
 
@@ -642,7 +641,7 @@ int most_set_cfg_direction(char *mdev, char *mdev_ch, char *buf)
 	} else if (!strcmp(buf, "tx")) {
 		c->cfg.direction = MOST_CH_TX;
 	} else {
-		pr_info("Invalid direction\n");
+		dev_err(&mc.dev, "Invalid direction\n");
 		return -ENODATA;
 	}
 	return 0;
@@ -795,7 +794,7 @@ static int hdm_enqueue_thread(void *data)
 		mutex_unlock(&c->nq_mutex);
 
 		if (unlikely(ret)) {
-			pr_err("hdm enqueue failed\n");
+			dev_err(&mc.dev, "hdm enqueue failed\n");
 			nq_hdm_mbo(mbo);
 			c->hdm_enqueue_task = NULL;
 			return 0;
@@ -942,7 +941,7 @@ static void most_write_completion(struct mbo *mbo)
 
 	c = mbo->context;
 	if (mbo->status == MBO_E_INVAL)
-		pr_info("WARN: Tx MBO status: invalid\n");
+		dev_warn(&mc.dev, "Tx MBO status: invalid\n");
 	if (unlikely(c->is_poisoned || (mbo->status == MBO_E_CLOSE)))
 		trash_mbo(mbo);
 	else
@@ -1101,14 +1100,14 @@ int most_start_channel(struct most_interface *iface, int id,
 		goto out; /* already started by another component */
 
 	if (!try_module_get(iface->mod)) {
-		pr_info("failed to acquire HDM lock\n");
+		dev_err(&mc.dev, "failed to acquire HDM lock\n");
 		mutex_unlock(&c->start_mutex);
 		return -ENOLCK;
 	}
 
 	c->cfg.extra_len = 0;
 	if (c->iface->configure(c->iface, c->channel_id, &c->cfg)) {
-		pr_info("channel configuration failed. Go check settings...\n");
+		dev_err(&mc.dev, "channel configuration failed. Go check settings...\n");
 		ret = -EINVAL;
 		goto err_put_module;
 	}
@@ -1162,7 +1161,7 @@ int most_stop_channel(struct most_interface *iface, int id,
 	struct most_channel *c;
 
 	if (unlikely((!iface) || (id >= iface->num_channels) || (id < 0))) {
-		pr_err("Bad interface or index out of range\n");
+		dev_err(&mc.dev, "Bad interface or index out of range\n");
 		return -EINVAL;
 	}
 	c = iface->p->channel[id];
@@ -1182,8 +1181,8 @@ int most_stop_channel(struct most_interface *iface, int id,
 
 	c->is_poisoned = true;
 	if (c->iface->poison_channel(c->iface, c->channel_id)) {
-		pr_err("Cannot stop channel %d of mdev %s\n", c->channel_id,
-		       c->iface->description);
+		dev_err(&mc.dev, "Cannot stop channel %d of mdev %s\n", c->channel_id,
+			c->iface->description);
 		mutex_unlock(&c->start_mutex);
 		return -EAGAIN;
 	}
@@ -1192,7 +1191,7 @@ int most_stop_channel(struct most_interface *iface, int id,
 
 #ifdef CMPL_INTERRUPTIBLE
 	if (wait_for_completion_interruptible(&c->cleanup)) {
-		pr_info("Interrupted while clean up ch %d\n", c->channel_id);
+		dev_err(&mc.dev, "Interrupted while clean up ch %d\n", c->channel_id);
 		mutex_unlock(&c->start_mutex);
 		return -EINTR;
 	}
@@ -1218,11 +1217,11 @@ EXPORT_SYMBOL_GPL(most_stop_channel);
 int most_register_component(struct most_component *comp)
 {
 	if (!comp) {
-		pr_err("Bad component\n");
+		dev_err(&mc.dev, "Bad component\n");
 		return -EINVAL;
 	}
 	list_add_tail(&comp->list, &mc.comp_list);
-	pr_info("registered new core component %s\n", comp->name);
+	dev_info(&mc.dev, "registered new core component %s\n", comp->name);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(most_register_component);
@@ -1252,25 +1251,25 @@ static int disconnect_channels(struct device *dev, void *data)
 int most_deregister_component(struct most_component *comp)
 {
 	if (!comp) {
-		pr_err("Bad component\n");
+		dev_err(&mc.dev, "Bad component\n");
 		return -EINVAL;
 	}
 
 	bus_for_each_dev(&mc.bus, NULL, comp, disconnect_channels);
 	list_del(&comp->list);
-	pr_info("deregistering component %s\n", comp->name);
+	dev_info(&mc.dev, "deregistering component %s\n", comp->name);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(most_deregister_component);
 
 static void release_interface(struct device *dev)
 {
-	pr_info("releasing interface dev %s...\n", dev_name(dev));
+	dev_info(&mc.dev, "releasing interface dev %s...\n", dev_name(dev));
 }
 
 static void release_channel(struct device *dev)
 {
-	pr_info("releasing channel dev %s...\n", dev_name(dev));
+	dev_info(&mc.dev, "releasing channel dev %s...\n", dev_name(dev));
 }
 
 /**
@@ -1288,13 +1287,13 @@ int most_register_interface(struct most_interface *iface)
 
 	if (!iface || !iface->enqueue || !iface->configure ||
 	    !iface->poison_channel || (iface->num_channels > MAX_CHANNELS)) {
-		pr_err("Bad interface or channel overflow\n");
+		dev_err(&mc.dev, "Bad interface or channel overflow\n");
 		return -EINVAL;
 	}
 
 	id = ida_simple_get(&mdev_id, 0, 0, GFP_KERNEL);
 	if (id < 0) {
-		pr_info("Failed to alloc mdev ID\n");
+		dev_err(&mc.dev, "Failed to alloc mdev ID\n");
 		return id;
 	}
 
@@ -1313,7 +1312,7 @@ int most_register_interface(struct most_interface *iface)
 	iface->dev.groups = interface_attr_groups;
 	iface->dev.release = release_interface;
 	if (device_register(&iface->dev)) {
-		pr_err("registering iface->dev failed\n");
+		dev_err(&mc.dev, "registering iface->dev failed\n");
 		kfree(iface->p);
 		ida_simple_remove(&mdev_id, id);
 		return -ENOMEM;
@@ -1356,12 +1355,12 @@ int most_register_interface(struct most_interface *iface)
 		mutex_init(&c->nq_mutex);
 		list_add_tail(&c->list, &iface->p->channel_list);
 		if (device_register(&c->dev)) {
-			pr_err("registering c->dev failed\n");
+			dev_err(&mc.dev, "registering c->dev failed\n");
 			goto err_free_most_channel;
 		}
 	}
-	pr_info("registered new device mdev%d (%s)\n",
-		id, iface->description);
+	dev_info(&mc.dev, "registered new device mdev%d (%s)\n",
+		 id, iface->description);
 	most_interface_register_notify(iface->description);
 	return 0;
 
@@ -1393,8 +1392,8 @@ void most_deregister_interface(struct most_interface *iface)
 	int i;
 	struct most_channel *c;
 
-	pr_info("deregistering device %s (%s)\n", dev_name(&iface->dev),
-		iface->description);
+	dev_info(&mc.dev, "deregistering device %s (%s)\n", dev_name(&iface->dev),
+		 iface->description);
 	for (i = 0; i < iface->num_channels; i++) {
 		c = iface->p->channel[i];
 		if (c->pipe0.comp)
@@ -1464,14 +1463,14 @@ EXPORT_SYMBOL_GPL(most_resume_enqueue);
 
 static void release_most_sub(struct device *dev)
 {
-	pr_info("releasing most_subsystem\n");
+	dev_info(&mc.dev, "releasing most_subsystem\n");
 }
 
 static int __init most_init(void)
 {
 	int err;
 
-	pr_info("init()\n");
+	dev_info(&mc.dev, "init()\n");
 	INIT_LIST_HEAD(&mc.comp_list);
 	ida_init(&mdev_id);
 
@@ -1483,12 +1482,12 @@ static int __init most_init(void)
 
 	err = bus_register(&mc.bus);
 	if (err) {
-		pr_info("Cannot register most bus\n");
+		dev_err(&mc.dev, "Cannot register most bus\n");
 		return err;
 	}
 	err = driver_register(&mc.drv);
 	if (err) {
-		pr_info("Cannot register core driver\n");
+		dev_err(&mc.dev, "Cannot register core driver\n");
 		goto err_unregister_bus;
 	}
 	mc.dev.init_name = "most_bus";
@@ -1509,7 +1508,7 @@ static int __init most_init(void)
 
 static void __exit most_exit(void)
 {
-	pr_info("exit core module\n");
+	dev_info(&mc.dev, "exit core module\n");
 	device_unregister(&mc.dev);
 	driver_unregister(&mc.drv);
 	bus_unregister(&mc.bus);
-- 
2.7.4

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

[PATCH RFC v3 3/9] staging: most: core: remove noisy log messages

[Christian Gromm]

In order to not generate unnecessary noise in the kernel log,this patch
removes debug log messages.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
v3:
	This patch has been added to the series.

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

diff --git a/drivers/staging/most/core.c b/drivers/staging/most/core.c
index 9839b8c..0a3ce29 100644
--- a/drivers/staging/most/core.c
+++ b/drivers/staging/most/core.c
@@ -1221,7 +1221,6 @@ int most_register_component(struct most_component *comp)
 		return -EINVAL;
 	}
 	list_add_tail(&comp->list, &mc.comp_list);
-	dev_info(&mc.dev, "registered new core component %s\n", comp->name);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(most_register_component);
@@ -1257,7 +1256,6 @@ int most_deregister_component(struct most_component *comp)
 
 	bus_for_each_dev(&mc.bus, NULL, comp, disconnect_channels);
 	list_del(&comp->list);
-	dev_info(&mc.dev, "deregistering component %s\n", comp->name);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(most_deregister_component);
@@ -1359,8 +1357,6 @@ int most_register_interface(struct most_interface *iface)
 			goto err_free_most_channel;
 		}
 	}
-	dev_info(&mc.dev, "registered new device mdev%d (%s)\n",
-		 id, iface->description);
 	most_interface_register_notify(iface->description);
 	return 0;
 
@@ -1392,8 +1388,6 @@ void most_deregister_interface(struct most_interface *iface)
 	int i;
 	struct most_channel *c;
 
-	dev_info(&mc.dev, "deregistering device %s (%s)\n", dev_name(&iface->dev),
-		 iface->description);
 	for (i = 0; i < iface->num_channels; i++) {
 		c = iface->p->channel[i];
 		if (c->pipe0.comp)
@@ -1470,7 +1464,6 @@ static int __init most_init(void)
 {
 	int err;
 
-	dev_info(&mc.dev, "init()\n");
 	INIT_LIST_HEAD(&mc.comp_list);
 	ida_init(&mdev_id);
 
@@ -1508,7 +1501,6 @@ static int __init most_init(void)
 
 static void __exit most_exit(void)
 {
-	dev_info(&mc.dev, "exit core module\n");
 	device_unregister(&mc.dev);
 	driver_unregister(&mc.drv);
 	bus_unregister(&mc.bus);
-- 
2.7.4

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

[PATCH RFC v3 4/9] staging: most: move interface dev to private section

[Christian Gromm]

This patch moves the struct device of the interface structure to its
private section, because only the core should access it directly. For
other entities an API is provided.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
v3:
	This patch has been added to the series.

 drivers/staging/most/core.c      | 68 ++++++++++++++++++++++++++--------------
 drivers/staging/most/dim2/dim2.c |  2 +-
 drivers/staging/most/most.h      |  6 ++--
 drivers/staging/most/usb/usb.c   |  5 +--
 4 files changed, 51 insertions(+), 30 deletions(-)

diff --git a/drivers/staging/most/core.c b/drivers/staging/most/core.c
index 0a3ce29..4f60c09 100644
--- a/drivers/staging/most/core.c
+++ b/drivers/staging/most/core.c
@@ -74,11 +74,15 @@ struct most_channel {
 
 struct interface_private {
 	int dev_id;
+	struct device dev;
+	struct most_interface *iface;
 	char name[STRING_SIZE];
 	struct most_channel *channel[MAX_CHANNELS];
 	struct list_head channel_list;
 };
 
+#define to_iface_priv(d) container_of(d, struct interface_private, dev)
+
 static const struct {
 	int most_ch_data_type;
 	const char *name;
@@ -401,7 +405,7 @@ static ssize_t description_show(struct device *dev,
 				struct device_attribute *attr,
 				char *buf)
 {
-	struct most_interface *iface = to_most_interface(dev);
+	struct most_interface *iface = to_iface_priv(dev)->iface;
 
 	return snprintf(buf, PAGE_SIZE, "%s\n", iface->description);
 }
@@ -410,7 +414,7 @@ static ssize_t interface_show(struct device *dev,
 			      struct device_attribute *attr,
 			      char *buf)
 {
-	struct most_interface *iface = to_most_interface(dev);
+	struct most_interface *iface = to_iface_priv(dev)->iface;
 
 	switch (iface->interface) {
 	case ITYPE_LOOPBACK:
@@ -475,7 +479,7 @@ static int print_links(struct device *dev, void *data)
 	int offs = d->offs;
 	char *buf = d->buf;
 	struct most_channel *c;
-	struct most_interface *iface = to_most_interface(dev);
+	struct most_interface *iface = to_iface_priv(dev)->iface;
 
 	list_for_each_entry(c, &iface->p->channel_list, list) {
 		if (c->pipe0.comp) {
@@ -483,7 +487,7 @@ static int print_links(struct device *dev, void *data)
 					 PAGE_SIZE - offs,
 					 "%s:%s:%s\n",
 					 c->pipe0.comp->name,
-					 dev_name(&iface->dev),
+					 dev_name(&iface->p->dev),
 					 dev_name(&c->dev));
 		}
 		if (c->pipe1.comp) {
@@ -491,7 +495,7 @@ static int print_links(struct device *dev, void *data)
 					 PAGE_SIZE - offs,
 					 "%s:%s:%s\n",
 					 c->pipe1.comp->name,
-					 dev_name(&iface->dev),
+					 dev_name(&iface->p->dev),
 					 dev_name(&c->dev));
 		}
 	}
@@ -533,7 +537,7 @@ static struct most_channel *get_channel(char *mdev, char *mdev_ch)
 	dev = bus_find_device_by_name(&mc.bus, NULL, mdev);
 	if (!dev)
 		return NULL;
-	iface = to_most_interface(dev);
+	iface = to_iface_priv(dev)->iface;
 	list_for_each_entry_safe(c, tmp, &iface->p->channel_list, list) {
 		if (!strcmp(dev_name(&c->dev), mdev_ch))
 			return c;
@@ -1231,7 +1235,7 @@ static int disconnect_channels(struct device *dev, void *data)
 	struct most_channel *c, *tmp;
 	struct most_component *comp = data;
 
-	iface = to_most_interface(dev);
+	iface = to_iface_priv(dev)->iface;
 	list_for_each_entry_safe(c, tmp, &iface->p->channel_list, list) {
 		if (c->pipe0.comp == comp || c->pipe1.comp == comp)
 			comp->disconnect_channel(c->iface, c->channel_id);
@@ -1260,16 +1264,35 @@ int most_deregister_component(struct most_component *comp)
 }
 EXPORT_SYMBOL_GPL(most_deregister_component);
 
-static void release_interface(struct device *dev)
+static void release_interface_priv(struct device *dev)
 {
+	struct interface_private *p = to_iface_priv(dev);
+
 	dev_info(&mc.dev, "releasing interface dev %s...\n", dev_name(dev));
+	kfree(p);
 }
 
 static void release_channel(struct device *dev)
 {
+	struct most_channel *c = to_channel(dev);
+
 	dev_info(&mc.dev, "releasing channel dev %s...\n", dev_name(dev));
+	kfree(c);
 }
 
+struct device *most_get_iface_dev(struct most_interface *iface)
+{
+	get_device(&iface->p->dev);
+	return &iface->p->dev;
+}
+EXPORT_SYMBOL_GPL(most_get_iface_dev);
+
+void most_put_iface_dev(struct device *dev)
+{
+	put_device(dev);
+}
+EXPORT_SYMBOL_GPL(most_put_iface_dev);
+
 /**
  * most_register_interface - registers an interface with core
  * @iface: device interface
@@ -1302,16 +1325,17 @@ int most_register_interface(struct most_interface *iface)
 	}
 
 	INIT_LIST_HEAD(&iface->p->channel_list);
+	iface->p->iface = iface;
 	iface->p->dev_id = id;
 	strscpy(iface->p->name, iface->description, sizeof(iface->p->name));
-	iface->dev.init_name = iface->p->name;
-	iface->dev.bus = &mc.bus;
-	iface->dev.parent = &mc.dev;
-	iface->dev.groups = interface_attr_groups;
-	iface->dev.release = release_interface;
-	if (device_register(&iface->dev)) {
-		dev_err(&mc.dev, "registering iface->dev failed\n");
-		kfree(iface->p);
+	iface->p->dev.init_name = iface->p->name;
+	iface->p->dev.bus = &mc.bus;
+	iface->p->dev.parent = &mc.dev;
+	iface->p->dev.groups = interface_attr_groups;
+	iface->p->dev.release = release_interface_priv;
+	if (device_register(&iface->p->dev)) {
+		dev_err(&mc.dev, "registering iface->p->dev failed\n");
+		put_device(&iface->p->dev);
 		ida_simple_remove(&mdev_id, id);
 		return -ENOMEM;
 	}
@@ -1327,7 +1351,7 @@ int most_register_interface(struct most_interface *iface)
 		else
 			snprintf(c->name, STRING_SIZE, "%s", name_suffix);
 		c->dev.init_name = c->name;
-		c->dev.parent = &iface->dev;
+		c->dev.parent = &iface->p->dev;
 		c->dev.groups = channel_attr_groups;
 		c->dev.release = release_channel;
 		iface->p->channel[i] = c;
@@ -1361,16 +1385,14 @@ int most_register_interface(struct most_interface *iface)
 	return 0;
 
 err_free_most_channel:
-	kfree(c);
+	put_device(&c->dev);
 
 err_free_resources:
 	while (i > 0) {
 		c = iface->p->channel[--i];
 		device_unregister(&c->dev);
-		kfree(c);
 	}
-	kfree(iface->p);
-	device_unregister(&iface->dev);
+	device_unregister(&iface->p->dev);
 	ida_simple_remove(&mdev_id, id);
 	return -ENOMEM;
 }
@@ -1400,12 +1422,10 @@ void most_deregister_interface(struct most_interface *iface)
 		c->pipe1.comp = NULL;
 		list_del(&c->list);
 		device_unregister(&c->dev);
-		kfree(c);
 	}
 
 	ida_simple_remove(&mdev_id, iface->p->dev_id);
-	kfree(iface->p);
-	device_unregister(&iface->dev);
+	device_unregister(&iface->p->dev);
 }
 EXPORT_SYMBOL_GPL(most_deregister_interface);
 
diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c
index 9eb10fc..b4d231c 100644
--- a/drivers/staging/most/dim2/dim2.c
+++ b/drivers/staging/most/dim2/dim2.c
@@ -855,7 +855,7 @@ static int dim2_probe(struct platform_device *pdev)
 	dev->most_iface.request_netinfo = request_netinfo;
 	dev->most_iface.driver_dev = &pdev->dev;
 	dev->dev.init_name = "dim2_state";
-	dev->dev.parent = &dev->most_iface.dev;
+	dev->dev.parent = most_get_iface_dev(&dev->most_iface);
 
 	ret = most_register_interface(&dev->most_iface);
 	if (ret) {
diff --git a/drivers/staging/most/most.h b/drivers/staging/most/most.h
index d93c6ce..26fdbab 100644
--- a/drivers/staging/most/most.h
+++ b/drivers/staging/most/most.h
@@ -229,7 +229,6 @@ struct mbo {
  * @priv Private field used by mostcore to store context information.
  */
 struct most_interface {
-	struct device dev;
 	struct device *driver_dev;
 	struct module *mod;
 	enum most_interface_type interface;
@@ -251,8 +250,6 @@ struct most_interface {
 	struct interface_private *p;
 };
 
-#define to_most_interface(d) container_of(d, struct most_interface, dev)
-
 /**
  * struct most_component - identifies a loadable component for the mostcore
  * @list: list_head
@@ -276,6 +273,9 @@ struct most_component {
 	int (*cfg_complete)(void);
 };
 
+struct device *most_get_iface_dev(struct most_interface *iface);
+void most_put_iface_dev(struct device *dev);
+
 /**
  * most_register_interface - Registers instance of the interface.
  * @iface: Pointer to the interface instance description.
diff --git a/drivers/staging/most/usb/usb.c b/drivers/staging/most/usb/usb.c
index 491b38e..fe3384a 100644
--- a/drivers/staging/most/usb/usb.c
+++ b/drivers/staging/most/usb/usb.c
@@ -1026,7 +1026,7 @@ static void release_dci(struct device *dev)
  * hdm_probe - probe function of USB device driver
  * @interface: Interface of the attached USB device
  * @id: Pointer to the USB ID table.
- *
+*
  * This allocates and initializes the device instance, adds the new
  * entry to the internal list, scans the USB descriptors and registers
  * the interface with the core.
@@ -1151,7 +1151,7 @@ hdm_probe(struct usb_interface *interface, const struct usb_device_id *id)
 		}
 
 		mdev->dci->dev.init_name = "dci";
-		mdev->dci->dev.parent = &mdev->iface.dev;
+		mdev->dci->dev.parent = most_get_iface_dev(&mdev->iface);
 		mdev->dci->dev.groups = dci_attr_groups;
 		mdev->dci->dev.release = release_dci;
 		if (device_register(&mdev->dci->dev)) {
@@ -1205,6 +1205,7 @@ static void hdm_disconnect(struct usb_interface *interface)
 	del_timer_sync(&mdev->link_stat_timer);
 	cancel_work_sync(&mdev->poll_work_obj);
 
+	most_put_iface_dev(mdev->dci->dev.parent);
 	device_unregister(&mdev->dci->dev);
 	most_deregister_interface(&mdev->iface);
 
-- 
2.7.4

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

Re: [PATCH RFC v3 4/9] staging: most: move interface dev to private section

[Greg KH]

On Tue, Jan 14, 2020 at 04:57:53PM +0100, Christian Gromm wrote:
> This patch moves the struct device of the interface structure to its
> private section, because only the core should access it directly. For
> other entities an API is provided.

This feels "wrong".

Shouldn't the struct device in this structure be the thing that is
handling the reference counting and sysfs integration for this
structure?  To put it as a "pointer" in a private section of the
structure feels like it is now backwards.

What is this helping with?  Who was messing with the device structure
here that needed to not mess with it?

It's good that you are now releasing the memory for the device structure
properly, but this still feels really wrong.  What is keeping the
lifetime of this structure now that the device is removed from it?

thanks,

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

Re: [PATCH RFC v3 4/9] staging: most: move interface dev to private section

[Christian.Gromm]

On Wed, 2020-01-15 at 13:17 +0100, Greg KH wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Tue, Jan 14, 2020 at 04:57:53PM +0100, Christian Gromm wrote:
> > This patch moves the struct device of the interface structure to
> > its
> > private section, because only the core should access it directly.
> > For
> > other entities an API is provided.
> 
> This feels "wrong".
> 
> Shouldn't the struct device in this structure be the thing that is
> handling the reference counting and sysfs integration for this
> structure?

Yes, that's right.

>   To put it as a "pointer" in a private section of the
> structure feels like it is now backwards.
> 
> What is this helping with?  Who was messing with the device structure
> here that needed to not mess with it?

Well, it's not that somebody was messing with it. It's just the
fact that somebody could. 

> 
> It's good that you are now releasing the memory for the device
> structure
> properly, but this still feels really wrong.  What is keeping the
> lifetime of this structure now that the device is removed from it?

It's the most_dev structure of the of USB module (or any other lower
adapter driver), which embeds the most_interface sturcture that 
contained the dev struct (which I moved to the private section).

The thing that might be confusing is that the place, where the parent 
structure with the device is being allocated, is not the same where
the device actually gets registered with the kernel. These are two
different kernel modules actually.

thanks,
Chris

> 
> thanks,
> 
> greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH RFC v3 5/9] staging: most: usb: check for NULL device

[Christian Gromm]

Check if the dci structer has been allocated before trying to release it.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
v3:
	This patch has been added to the series.

 drivers/staging/most/usb/usb.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/most/usb/usb.c b/drivers/staging/most/usb/usb.c
index fe3384a..cae7553 100644
--- a/drivers/staging/most/usb/usb.c
+++ b/drivers/staging/most/usb/usb.c
@@ -1205,8 +1205,10 @@ static void hdm_disconnect(struct usb_interface *interface)
 	del_timer_sync(&mdev->link_stat_timer);
 	cancel_work_sync(&mdev->poll_work_obj);
 
-	most_put_iface_dev(mdev->dci->dev.parent);
-	device_unregister(&mdev->dci->dev);
+	if (mdev->dci) {
+		most_put_iface_dev(mdev->dci->dev.parent);
+		device_unregister(&mdev->dci->dev);
+	}
 	most_deregister_interface(&mdev->iface);
 
 	kfree(mdev->busy_urbs);
-- 
2.7.4

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

Re: [PATCH RFC v3 5/9] staging: most: usb: check for NULL device

[Greg KH]

On Tue, Jan 14, 2020 at 04:57:54PM +0100, Christian Gromm wrote:
> Check if the dci structer has been allocated before trying to release it.
> 
> Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> ---
> v3:
> 	This patch has been added to the series.
> 
>  drivers/staging/most/usb/usb.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/most/usb/usb.c b/drivers/staging/most/usb/usb.c
> index fe3384a..cae7553 100644
> --- a/drivers/staging/most/usb/usb.c
> +++ b/drivers/staging/most/usb/usb.c
> @@ -1205,8 +1205,10 @@ static void hdm_disconnect(struct usb_interface *interface)
>  	del_timer_sync(&mdev->link_stat_timer);
>  	cancel_work_sync(&mdev->poll_work_obj);
>  
> -	most_put_iface_dev(mdev->dci->dev.parent);
> -	device_unregister(&mdev->dci->dev);
> +	if (mdev->dci) {
> +		most_put_iface_dev(mdev->dci->dev.parent);
> +		device_unregister(&mdev->dci->dev);
> +	}

How can this happen?

And why is it up to the child function here to unregister the device,
that feels wrong, and is further justification to me that your previous
patch is going the wrong way here.

thanks,

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

Re: [PATCH RFC v3 5/9] staging: most: usb: check for NULL device

[Christian.Gromm]

On Wed, 2020-01-15 at 13:18 +0100, Greg KH wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Tue, Jan 14, 2020 at 04:57:54PM +0100, Christian Gromm wrote:
> > Check if the dci structer has been allocated before trying to
> > release it.
> > 
> > Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> > ---
> > v3:
> >       This patch has been added to the series.
> > 
> >  drivers/staging/most/usb/usb.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/most/usb/usb.c
> > b/drivers/staging/most/usb/usb.c
> > index fe3384a..cae7553 100644
> > --- a/drivers/staging/most/usb/usb.c
> > +++ b/drivers/staging/most/usb/usb.c
> > @@ -1205,8 +1205,10 @@ static void hdm_disconnect(struct
> > usb_interface *interface)
> >       del_timer_sync(&mdev->link_stat_timer);
> >       cancel_work_sync(&mdev->poll_work_obj);
> > 
> > -     most_put_iface_dev(mdev->dci->dev.parent);
> > -     device_unregister(&mdev->dci->dev);
> > +     if (mdev->dci) {
> > +             most_put_iface_dev(mdev->dci->dev.parent);
> > +             device_unregister(&mdev->dci->dev);
> > +     }
> 
> How can this happen?

Depending on the Vendor/Product ID pair of the device that
probes the USB driver, this 'dci' structure is being allocated
and registered or not. And that is why this check is necessary. 

> 
> And why is it up to the child function here to unregister the device,

Child function? The device is being registered in the probe function
and unregistered in the disconnect function. What is wrong here?

thanks,
Chris

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

Re: [PATCH RFC v3 5/9] staging: most: usb: check for NULL device

[Greg KH]

On Wed, Jan 15, 2020 at 03:32:48PM +0000, Christian.Gromm@microchip.com wrote:
> On Wed, 2020-01-15 at 13:18 +0100, Greg KH wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> > 
> > On Tue, Jan 14, 2020 at 04:57:54PM +0100, Christian Gromm wrote:
> > > Check if the dci structer has been allocated before trying to
> > > release it.
> > > 
> > > Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> > > ---
> > > v3:
> > >       This patch has been added to the series.
> > > 
> > >  drivers/staging/most/usb/usb.c | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/staging/most/usb/usb.c
> > > b/drivers/staging/most/usb/usb.c
> > > index fe3384a..cae7553 100644
> > > --- a/drivers/staging/most/usb/usb.c
> > > +++ b/drivers/staging/most/usb/usb.c
> > > @@ -1205,8 +1205,10 @@ static void hdm_disconnect(struct
> > > usb_interface *interface)
> > >       del_timer_sync(&mdev->link_stat_timer);
> > >       cancel_work_sync(&mdev->poll_work_obj);
> > > 
> > > -     most_put_iface_dev(mdev->dci->dev.parent);
> > > -     device_unregister(&mdev->dci->dev);
> > > +     if (mdev->dci) {
> > > +             most_put_iface_dev(mdev->dci->dev.parent);
> > > +             device_unregister(&mdev->dci->dev);
> > > +     }
> > 
> > How can this happen?
> 
> Depending on the Vendor/Product ID pair of the device that
> probes the USB driver, this 'dci' structure is being allocated
> and registered or not. And that is why this check is necessary. 
> 
> > 
> > And why is it up to the child function here to unregister the device,
> 
> Child function? The device is being registered in the probe function
> and unregistered in the disconnect function. What is wrong here?

Ah, ok, I got confused here with the previous patch, sorry.

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

[PATCH RFC v3 6/9] staging: most: change storage class of struct mostcore

[Christian Gromm]

This patch allocated the mostcore structure dynamically and releases
the memory in the dedicated release function.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
v3:
	This patch has been added to the series.

 drivers/staging/most/core.c | 110 ++++++++++++++++++++++++--------------------
 1 file changed, 59 insertions(+), 51 deletions(-)

diff --git a/drivers/staging/most/core.c b/drivers/staging/most/core.c
index 4f60c09..92303d0 100644
--- a/drivers/staging/most/core.c
+++ b/drivers/staging/most/core.c
@@ -33,9 +33,10 @@ static struct mostcore {
 	struct device_driver drv;
 	struct bus_type bus;
 	struct list_head comp_list;
-} mc;
+} *mc;
 
 #define to_driver(d) container_of(d, struct mostcore, drv)
+#define to_mostcore(d) container_of(d, struct mostcore, dev)
 
 struct pipe {
 	struct most_component *comp;
@@ -154,7 +155,7 @@ static void flush_channel_fifos(struct most_channel *c)
 	spin_unlock_irqrestore(&c->fifo_lock, hf_flags);
 
 	if (unlikely((!list_empty(&c->fifo) || !list_empty(&c->halt_fifo))))
-		dev_warn(&mc.dev, "fifo | trash fifo not empty\n");
+		dev_warn(&mc->dev, "fifo | trash fifo not empty\n");
 }
 
 /**
@@ -461,7 +462,7 @@ static struct most_component *match_component(char *name)
 {
 	struct most_component *comp;
 
-	list_for_each_entry(comp, &mc.comp_list, list) {
+	list_for_each_entry(comp, &mc->comp_list, list) {
 		if (!strcmp(comp->name, name))
 			return comp;
 	}
@@ -507,7 +508,7 @@ static ssize_t links_show(struct device_driver *drv, char *buf)
 {
 	struct show_links_data d = { .buf = buf };
 
-	bus_for_each_dev(&mc.bus, NULL, &d, print_links);
+	bus_for_each_dev(&mc->bus, NULL, &d, print_links);
 	return d.offs;
 }
 
@@ -516,7 +517,7 @@ static ssize_t components_show(struct device_driver *drv, char *buf)
 	struct most_component *comp;
 	int offs = 0;
 
-	list_for_each_entry(comp, &mc.comp_list, list) {
+	list_for_each_entry(comp, &mc->comp_list, list) {
 		offs += snprintf(buf + offs, PAGE_SIZE - offs, "%s\n",
 				 comp->name);
 	}
@@ -534,7 +535,7 @@ static struct most_channel *get_channel(char *mdev, char *mdev_ch)
 	struct most_interface *iface;
 	struct most_channel *c, *tmp;
 
-	dev = bus_find_device_by_name(&mc.bus, NULL, mdev);
+	dev = bus_find_device_by_name(&mc->bus, NULL, mdev);
 	if (!dev)
 		return NULL;
 	iface = to_iface_priv(dev)->iface;
@@ -626,7 +627,7 @@ int most_set_cfg_datatype(char *mdev, char *mdev_ch, char *buf)
 	}
 
 	if (i == ARRAY_SIZE(ch_data_type))
-		dev_warn(&mc.dev, "invalid attribute settings\n");
+		dev_warn(&mc->dev, "invalid attribute settings\n");
 	return 0;
 }
 
@@ -645,7 +646,7 @@ int most_set_cfg_direction(char *mdev, char *mdev_ch, char *buf)
 	} else if (!strcmp(buf, "tx")) {
 		c->cfg.direction = MOST_CH_TX;
 	} else {
-		dev_err(&mc.dev, "Invalid direction\n");
+		dev_err(&mc->dev, "Invalid direction\n");
 		return -ENODATA;
 	}
 	return 0;
@@ -798,7 +799,7 @@ static int hdm_enqueue_thread(void *data)
 		mutex_unlock(&c->nq_mutex);
 
 		if (unlikely(ret)) {
-			dev_err(&mc.dev, "hdm enqueue failed\n");
+			dev_err(&mc->dev, "hdm enqueue failed\n");
 			nq_hdm_mbo(mbo);
 			c->hdm_enqueue_task = NULL;
 			return 0;
@@ -945,7 +946,7 @@ static void most_write_completion(struct mbo *mbo)
 
 	c = mbo->context;
 	if (mbo->status == MBO_E_INVAL)
-		dev_warn(&mc.dev, "Tx MBO status: invalid\n");
+		dev_warn(&mc->dev, "Tx MBO status: invalid\n");
 	if (unlikely(c->is_poisoned || (mbo->status == MBO_E_CLOSE)))
 		trash_mbo(mbo);
 	else
@@ -1104,14 +1105,14 @@ int most_start_channel(struct most_interface *iface, int id,
 		goto out; /* already started by another component */
 
 	if (!try_module_get(iface->mod)) {
-		dev_err(&mc.dev, "failed to acquire HDM lock\n");
+		dev_err(&mc->dev, "failed to acquire HDM lock\n");
 		mutex_unlock(&c->start_mutex);
 		return -ENOLCK;
 	}
 
 	c->cfg.extra_len = 0;
 	if (c->iface->configure(c->iface, c->channel_id, &c->cfg)) {
-		dev_err(&mc.dev, "channel configuration failed. Go check settings...\n");
+		dev_err(&mc->dev, "channel configuration failed. Go check settings...\n");
 		ret = -EINVAL;
 		goto err_put_module;
 	}
@@ -1165,7 +1166,7 @@ int most_stop_channel(struct most_interface *iface, int id,
 	struct most_channel *c;
 
 	if (unlikely((!iface) || (id >= iface->num_channels) || (id < 0))) {
-		dev_err(&mc.dev, "Bad interface or index out of range\n");
+		dev_err(&mc->dev, "Bad interface or index out of range\n");
 		return -EINVAL;
 	}
 	c = iface->p->channel[id];
@@ -1185,7 +1186,7 @@ int most_stop_channel(struct most_interface *iface, int id,
 
 	c->is_poisoned = true;
 	if (c->iface->poison_channel(c->iface, c->channel_id)) {
-		dev_err(&mc.dev, "Cannot stop channel %d of mdev %s\n", c->channel_id,
+		dev_err(&mc->dev, "Cannot stop channel %d of mdev %s\n", c->channel_id,
 			c->iface->description);
 		mutex_unlock(&c->start_mutex);
 		return -EAGAIN;
@@ -1195,7 +1196,7 @@ int most_stop_channel(struct most_interface *iface, int id,
 
 #ifdef CMPL_INTERRUPTIBLE
 	if (wait_for_completion_interruptible(&c->cleanup)) {
-		dev_err(&mc.dev, "Interrupted while clean up ch %d\n", c->channel_id);
+		dev_err(&mc->dev, "Interrupted while clean up ch %d\n", c->channel_id);
 		mutex_unlock(&c->start_mutex);
 		return -EINTR;
 	}
@@ -1221,10 +1222,10 @@ EXPORT_SYMBOL_GPL(most_stop_channel);
 int most_register_component(struct most_component *comp)
 {
 	if (!comp) {
-		dev_err(&mc.dev, "Bad component\n");
+		dev_err(&mc->dev, "Bad component\n");
 		return -EINVAL;
 	}
-	list_add_tail(&comp->list, &mc.comp_list);
+	list_add_tail(&comp->list, &mc->comp_list);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(most_register_component);
@@ -1254,11 +1255,11 @@ static int disconnect_channels(struct device *dev, void *data)
 int most_deregister_component(struct most_component *comp)
 {
 	if (!comp) {
-		dev_err(&mc.dev, "Bad component\n");
+		dev_err(&mc->dev, "Bad component\n");
 		return -EINVAL;
 	}
 
-	bus_for_each_dev(&mc.bus, NULL, comp, disconnect_channels);
+	bus_for_each_dev(&mc->bus, NULL, comp, disconnect_channels);
 	list_del(&comp->list);
 	return 0;
 }
@@ -1268,7 +1269,6 @@ static void release_interface_priv(struct device *dev)
 {
 	struct interface_private *p = to_iface_priv(dev);
 
-	dev_info(&mc.dev, "releasing interface dev %s...\n", dev_name(dev));
 	kfree(p);
 }
 
@@ -1276,7 +1276,6 @@ static void release_channel(struct device *dev)
 {
 	struct most_channel *c = to_channel(dev);
 
-	dev_info(&mc.dev, "releasing channel dev %s...\n", dev_name(dev));
 	kfree(c);
 }
 
@@ -1308,13 +1307,13 @@ int most_register_interface(struct most_interface *iface)
 
 	if (!iface || !iface->enqueue || !iface->configure ||
 	    !iface->poison_channel || (iface->num_channels > MAX_CHANNELS)) {
-		dev_err(&mc.dev, "Bad interface or channel overflow\n");
+		dev_err(&mc->dev, "Bad interface or channel overflow\n");
 		return -EINVAL;
 	}
 
 	id = ida_simple_get(&mdev_id, 0, 0, GFP_KERNEL);
 	if (id < 0) {
-		dev_err(&mc.dev, "Failed to alloc mdev ID\n");
+		dev_err(&mc->dev, "Failed to alloc mdev ID\n");
 		return id;
 	}
 
@@ -1329,12 +1328,12 @@ int most_register_interface(struct most_interface *iface)
 	iface->p->dev_id = id;
 	strscpy(iface->p->name, iface->description, sizeof(iface->p->name));
 	iface->p->dev.init_name = iface->p->name;
-	iface->p->dev.bus = &mc.bus;
-	iface->p->dev.parent = &mc.dev;
+	iface->p->dev.bus = &mc->bus;
+	iface->p->dev.parent = &mc->dev;
 	iface->p->dev.groups = interface_attr_groups;
 	iface->p->dev.release = release_interface_priv;
 	if (device_register(&iface->p->dev)) {
-		dev_err(&mc.dev, "registering iface->p->dev failed\n");
+		dev_err(&mc->dev, "registering iface->p->dev failed\n");
 		put_device(&iface->p->dev);
 		ida_simple_remove(&mdev_id, id);
 		return -ENOMEM;
@@ -1377,7 +1376,7 @@ int most_register_interface(struct most_interface *iface)
 		mutex_init(&c->nq_mutex);
 		list_add_tail(&c->list, &iface->p->channel_list);
 		if (device_register(&c->dev)) {
-			dev_err(&mc.dev, "registering c->dev failed\n");
+			dev_err(&mc->dev, "registering c->dev failed\n");
 			goto err_free_most_channel;
 		}
 	}
@@ -1477,53 +1476,62 @@ EXPORT_SYMBOL_GPL(most_resume_enqueue);
 
 static void release_most_sub(struct device *dev)
 {
-	dev_info(&mc.dev, "releasing most_subsystem\n");
+	struct mostcore *mc = to_mostcore(dev);
+
+	kfree(mc);
 }
 
 static int __init most_init(void)
 {
 	int err;
 
-	INIT_LIST_HEAD(&mc.comp_list);
-	ida_init(&mdev_id);
+	mc = kzalloc(sizeof(*mc), GFP_KERNEL);
+	if (!mc)
+		return -ENOMEM;
 
-	mc.bus.name = "most",
-	mc.bus.match = most_match,
-	mc.drv.name = "most_core",
-	mc.drv.bus = &mc.bus,
-	mc.drv.groups = mc_attr_groups;
+	INIT_LIST_HEAD(&mc->comp_list);
+	ida_init(&mdev_id);
 
-	err = bus_register(&mc.bus);
+	mc->bus.name = "most",
+	mc->bus.match = most_match,
+	mc->drv.name = "most_core",
+	mc->drv.bus = &mc->bus,
+	mc->drv.groups = mc_attr_groups;
+	mc->dev.init_name = "most_bus";
+	mc->dev.release = release_most_sub;
+	err = device_register(&mc->dev);
 	if (err) {
-		dev_err(&mc.dev, "Cannot register most bus\n");
+		ida_destroy(&mdev_id);
+		put_device(&mc->dev);
 		return err;
 	}
-	err = driver_register(&mc.drv);
+
+	err = bus_register(&mc->bus);
 	if (err) {
-		dev_err(&mc.dev, "Cannot register core driver\n");
-		goto err_unregister_bus;
+		dev_err(&mc->dev, "Cannot register most bus\n");
+		goto err_unregister_device;
 	}
-	mc.dev.init_name = "most_bus";
-	mc.dev.release = release_most_sub;
-	if (device_register(&mc.dev)) {
-		err = -ENOMEM;
-		goto err_unregister_driver;
+	err = driver_register(&mc->drv);
+	if (err) {
+		dev_err(&mc->dev, "Cannot register core driver\n");
+		goto err_unregister_bus;
 	}
 	configfs_init();
 	return 0;
 
-err_unregister_driver:
-	driver_unregister(&mc.drv);
 err_unregister_bus:
-	bus_unregister(&mc.bus);
+	bus_unregister(&mc->bus);
+err_unregister_device:
+	device_unregister(&mc->dev);
+	ida_destroy(&mdev_id);
 	return err;
 }
 
 static void __exit most_exit(void)
 {
-	device_unregister(&mc.dev);
-	driver_unregister(&mc.drv);
-	bus_unregister(&mc.bus);
+	driver_unregister(&mc->drv);
+	bus_unregister(&mc->bus);
+	device_unregister(&mc->dev);
 	ida_destroy(&mdev_id);
 }
 
-- 
2.7.4

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

Re: [PATCH RFC v3 6/9] staging: most: change storage class of struct mostcore

[Greg KH]

On Tue, Jan 14, 2020 at 04:57:55PM +0100, Christian Gromm wrote:
> This patch allocated the mostcore structure dynamically and releases
> the memory in the dedicated release function.
> 
> Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> ---
> v3:
> 	This patch has been added to the series.
> 
>  drivers/staging/most/core.c | 110 ++++++++++++++++++++++++--------------------
>  1 file changed, 59 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/staging/most/core.c b/drivers/staging/most/core.c
> index 4f60c09..92303d0 100644
> --- a/drivers/staging/most/core.c
> +++ b/drivers/staging/most/core.c
> @@ -33,9 +33,10 @@ static struct mostcore {
>  	struct device_driver drv;
>  	struct bus_type bus;
>  	struct list_head comp_list;
> -} mc;
> +} *mc;

Why do you need one single global variable for all of this?  Shouldn't
there be one per "bus controller" that you have in the system?  No other
driver/bus subsystem has a single global state for everything, why is
this unique?

It's good you are making this dynamic, but I would go further and not
make there be one at all, this state should be per-bus.

thanks,

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

[PATCH RFC v3 7/9] staging: most: move core files out of the staging area

[Christian Gromm]

This patch moves the core module to the /drivers/most directory
and makes all necessary changes in order to not break the build.

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

 drivers/Kconfig                                |  1 +
 drivers/Makefile                               |  1 +
 drivers/most/Kconfig                           | 15 +++++++++++++++
 drivers/most/Makefile                          |  4 ++++
 drivers/{staging => }/most/configfs.c          |  2 +-
 drivers/{staging => }/most/core.c              |  2 +-
 drivers/staging/most/Kconfig                   |  6 +++---
 drivers/staging/most/Makefile                  |  5 -----
 drivers/staging/most/cdev/cdev.c               |  2 +-
 drivers/staging/most/dim2/dim2.c               |  2 +-
 drivers/staging/most/i2c/i2c.c                 |  2 +-
 drivers/staging/most/net/net.c                 |  2 +-
 drivers/staging/most/sound/sound.c             |  2 +-
 drivers/staging/most/usb/usb.c                 |  2 +-
 drivers/staging/most/video/video.c             |  2 +-
 {drivers/staging/most => include/linux}/most.h |  0
 16 files changed, 33 insertions(+), 17 deletions(-)
 create mode 100644 drivers/most/Kconfig
 create mode 100644 drivers/most/Makefile
 rename drivers/{staging => }/most/configfs.c (99%)
 rename drivers/{staging => }/most/core.c (99%)
 rename {drivers/staging/most => include/linux}/most.h (100%)

diff --git a/drivers/Kconfig b/drivers/Kconfig
index 8befa53..c739665 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -228,4 +228,5 @@ source "drivers/interconnect/Kconfig"
 
 source "drivers/counter/Kconfig"
 
+source "drivers/most/Kconfig"
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index aaef17c..0e1dcbc 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -186,3 +186,4 @@ obj-$(CONFIG_SIOX)		+= siox/
 obj-$(CONFIG_GNSS)		+= gnss/
 obj-$(CONFIG_INTERCONNECT)	+= interconnect/
 obj-$(CONFIG_COUNTER)		+= counter/
+obj-$(CONFIG_MOST)		+= most/
diff --git a/drivers/most/Kconfig b/drivers/most/Kconfig
new file mode 100644
index 0000000..58d7999
--- /dev/null
+++ b/drivers/most/Kconfig
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0
+menuconfig MOST
+	tristate "MOST support"
+	depends on HAS_DMA && CONFIGFS_FS
+	default n
+	help
+	  Say Y here if you want to enable MOST support.
+	  This driver needs at least one additional component to enable the
+	  desired access from userspace (e.g. character devices) and one that
+	  matches the network controller's hardware interface (e.g. USB).
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called most_core.
+
+	  If in doubt, say N here.
diff --git a/drivers/most/Makefile b/drivers/most/Makefile
new file mode 100644
index 0000000..e810cd3
--- /dev/null
+++ b/drivers/most/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_MOST) += most_core.o
+most_core-y :=	core.o \
+		configfs.o
diff --git a/drivers/staging/most/configfs.c b/drivers/most/configfs.c
similarity index 99%
rename from drivers/staging/most/configfs.c
rename to drivers/most/configfs.c
index 9818f6c..77b0855 100644
--- a/drivers/staging/most/configfs.c
+++ b/drivers/most/configfs.c
@@ -10,7 +10,7 @@
 #include <linux/slab.h>
 #include <linux/init.h>
 #include <linux/configfs.h>
-#include <most/most.h>
+#include <linux/most.h>
 
 #define MAX_STRING_SIZE 80
 
diff --git a/drivers/staging/most/core.c b/drivers/most/core.c
similarity index 99%
rename from drivers/staging/most/core.c
rename to drivers/most/core.c
index 92303d0..ec1153d 100644
--- a/drivers/staging/most/core.c
+++ b/drivers/most/core.c
@@ -20,7 +20,7 @@
 #include <linux/kthread.h>
 #include <linux/dma-mapping.h>
 #include <linux/idr.h>
-#include <most/most.h>
+#include <linux/most.h>
 
 #define MAX_CHANNELS	64
 #define STRING_SIZE	80
diff --git a/drivers/staging/most/Kconfig b/drivers/staging/most/Kconfig
index 6262eb2..c5a99f7 100644
--- a/drivers/staging/most/Kconfig
+++ b/drivers/staging/most/Kconfig
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
-menuconfig MOST
+menuconfig MOST_COMPONENTS
 	tristate "MOST support"
-	depends on HAS_DMA && CONFIGFS_FS
+	depends on HAS_DMA && CONFIGFS_FS && MOST
 	default n
 	help
 	  Say Y here if you want to enable MOST support.
@@ -16,7 +16,7 @@ menuconfig MOST
 
 
 
-if MOST
+if MOST_COMPONENTS
 
 source "drivers/staging/most/cdev/Kconfig"
 
diff --git a/drivers/staging/most/Makefile b/drivers/staging/most/Makefile
index 85ea5a4..3c446bb 100644
--- a/drivers/staging/most/Makefile
+++ b/drivers/staging/most/Makefile
@@ -1,9 +1,4 @@
 # 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/
 obj-$(CONFIG_MOST_NET)	+= net/
 obj-$(CONFIG_MOST_SOUND)	+= sound/
diff --git a/drivers/staging/most/cdev/cdev.c b/drivers/staging/most/cdev/cdev.c
index 59f346d..cc1e3de 100644
--- a/drivers/staging/most/cdev/cdev.c
+++ b/drivers/staging/most/cdev/cdev.c
@@ -16,7 +16,7 @@
 #include <linux/kfifo.h>
 #include <linux/uaccess.h>
 #include <linux/idr.h>
-#include <most/most.h>
+#include <linux/most.h>
 
 #define CHRDEV_REGION_SIZE 50
 
diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c
index b4d231c..8129613 100644
--- a/drivers/staging/most/dim2/dim2.c
+++ b/drivers/staging/most/dim2/dim2.c
@@ -21,7 +21,7 @@
 #include <linux/sched.h>
 #include <linux/kthread.h>
 
-#include <most/most.h>
+#include <linux/most.h>
 #include "hal.h"
 #include "errors.h"
 #include "sysfs.h"
diff --git a/drivers/staging/most/i2c/i2c.c b/drivers/staging/most/i2c/i2c.c
index d07719c..d4a9cd8 100644
--- a/drivers/staging/most/i2c/i2c.c
+++ b/drivers/staging/most/i2c/i2c.c
@@ -14,7 +14,7 @@
 #include <linux/interrupt.h>
 #include <linux/err.h>
 
-#include <most/most.h>
+#include <linux/most.h>
 
 enum { CH_RX, CH_TX, NUM_CHANNELS };
 
diff --git a/drivers/staging/most/net/net.c b/drivers/staging/most/net/net.c
index db42732..d6d5b2a 100644
--- a/drivers/staging/most/net/net.c
+++ b/drivers/staging/most/net/net.c
@@ -15,7 +15,7 @@
 #include <linux/list.h>
 #include <linux/wait.h>
 #include <linux/kobject.h>
-#include <most/most.h>
+#include <linux/most.h>
 
 #define MEP_HDR_LEN 8
 #define MDP_HDR_LEN 16
diff --git a/drivers/staging/most/sound/sound.c b/drivers/staging/most/sound/sound.c
index 23baf4b..1527f41 100644
--- a/drivers/staging/most/sound/sound.c
+++ b/drivers/staging/most/sound/sound.c
@@ -17,7 +17,7 @@
 #include <sound/pcm_params.h>
 #include <linux/sched.h>
 #include <linux/kthread.h>
-#include <most/most.h>
+#include <linux/most.h>
 
 #define DRIVER_NAME "sound"
 #define STRING_SIZE	80
diff --git a/drivers/staging/most/usb/usb.c b/drivers/staging/most/usb/usb.c
index cae7553..4716950 100644
--- a/drivers/staging/most/usb/usb.c
+++ b/drivers/staging/most/usb/usb.c
@@ -23,7 +23,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/etherdevice.h>
 #include <linux/uaccess.h>
-#include <most/most.h>
+#include <linux/most.h>
 
 #define USB_MTU			512
 #define NO_ISOCHRONOUS_URB	0
diff --git a/drivers/staging/most/video/video.c b/drivers/staging/most/video/video.c
index 9e9e45a..ce3a44a 100644
--- a/drivers/staging/most/video/video.c
+++ b/drivers/staging/most/video/video.c
@@ -21,7 +21,7 @@
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-fh.h>
 
-#include <most/most.h>
+#include <linux/most.h>
 
 #define V4L2_CMP_MAX_INPUT  1
 
diff --git a/drivers/staging/most/most.h b/include/linux/most.h
similarity index 100%
rename from drivers/staging/most/most.h
rename to include/linux/most.h
-- 
2.7.4

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

[PATCH RFC v3 8/9] staging: most: Documentation: update ABI description

[Christian Gromm]

This patch updates the ABI description files to be in sync
with current implementation.

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

 .../most/Documentation/ABI/configfs-most.txt       |  8 --------
 .../most/Documentation/ABI/sysfs-bus-most.txt      | 24 +++-------------------
 2 files changed, 3 insertions(+), 29 deletions(-)

diff --git a/drivers/staging/most/Documentation/ABI/configfs-most.txt b/drivers/staging/most/Documentation/ABI/configfs-most.txt
index 2bf8114..ed67a4d 100644
--- a/drivers/staging/most/Documentation/ABI/configfs-most.txt
+++ b/drivers/staging/most/Documentation/ABI/configfs-most.txt
@@ -194,11 +194,3 @@ Description:
 
 		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/ABI/sysfs-bus-most.txt b/drivers/staging/most/Documentation/ABI/sysfs-bus-most.txt
index d8fa841..6b1d06e 100644
--- a/drivers/staging/most/Documentation/ABI/sysfs-bus-most.txt
+++ b/drivers/staging/most/Documentation/ABI/sysfs-bus-most.txt
@@ -5,7 +5,7 @@ Contact:	Christian Gromm <christian.gromm@microchip.com>
 Description:
 		Provides information about the interface type and the physical
 		location of the device. Hardware attached via USB, for instance,
-		might return <usb_device 1-1.1:1.0>
+		might return <1-1.1:1.0>
 Users:
 
 What:		/sys/bus/most/devices/.../interface
@@ -278,25 +278,7 @@ Description:
 		Indicates whether current channel ran out of buffers.
 Users:
 
-What:		/sys/bus/most/drivers/mostcore/add_link
-Date:		March 2017
-KernelVersion:	4.15
-Contact:	Christian Gromm <christian.gromm@microchip.com>
-Description:
-		This is used to link a channel to a component of the
-		mostcore. A link created by writing to this file is
-		referred to as pipe.
-Users:
-
-What:		/sys/bus/most/drivers/mostcore/remove_link
-Date:		March 2017
-KernelVersion:	4.15
-Contact:	Christian Gromm <christian.gromm@microchip.com>
-Description:
-		This is used to unlink a channel from a component.
-Users:
-
-What:		/sys/bus/most/drivers/mostcore/components
+What:		/sys/bus/most/drivers/most_core/components
 Date:		March 2017
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
@@ -304,7 +286,7 @@ Description:
 		This is used to retrieve a list of registered components.
 Users:
 
-What:		/sys/bus/most/drivers/mostcore/links
+What:		/sys/bus/most/drivers/most_core/links
 Date:		March 2017
 KernelVersion:	4.15
 Contact:	Christian Gromm <christian.gromm@microchip.com>
-- 
2.7.4

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

[PATCH RFC v3 9/9] staging: most: Documentation: move ABI description files out of staging area

[Christian Gromm]

This patch moves the ABI description fils sysfs-bus-most and
configfs-most to the kernel's documentation folder.

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

 .../ABI/configfs-most.txt => Documentation/ABI/testing/configfs-most      | 0
 .../ABI/sysfs-bus-most.txt => Documentation/ABI/testing/sysfs-bus-most    | 0
 2 files changed, 0 insertions(+), 0 deletions(-)
 rename drivers/staging/most/Documentation/ABI/configfs-most.txt => Documentation/ABI/testing/configfs-most (100%)
 rename drivers/staging/most/Documentation/ABI/sysfs-bus-most.txt => Documentation/ABI/testing/sysfs-bus-most (100%)

diff --git a/drivers/staging/most/Documentation/ABI/configfs-most.txt b/Documentation/ABI/testing/configfs-most
similarity index 100%
rename from drivers/staging/most/Documentation/ABI/configfs-most.txt
rename to Documentation/ABI/testing/configfs-most
diff --git a/drivers/staging/most/Documentation/ABI/sysfs-bus-most.txt b/Documentation/ABI/testing/sysfs-bus-most
similarity index 100%
rename from drivers/staging/most/Documentation/ABI/sysfs-bus-most.txt
rename to Documentation/ABI/testing/sysfs-bus-most
-- 
2.7.4

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

Re: [PATCH RFC v3 0/9] staging: most: move core module out of staging

[Greg KH]

On Tue, Jan 14, 2020 at 04:57:49PM +0100, Christian Gromm wrote:
> The MOST driver was pushed to the staging area with kernel 4.3. Since then
> it has encountered many refinements by the community and should be ready
> for an upstream audit and to be moved out of the staging area. Since the
> driver consists of multiple modules the core module is about to go first
> and the other modules will follow subsequently. This patchset executes the
> necessary steps to move the core module out of staging.
> 
> v2:
> Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 	- use -M option to create patches
> v3:
> Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 	- fix date range in comment section of core.c
> 	- move code to free up memory to release funtions
> 	- remove noisy log messages
> 	- use dev_* functions for logging

I applied the first 3 patches in this series, and commented on a few
others that I wouldn't take.  Please work on that, rebase, and resend.

thanks,

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