[PATCH 00/28] staging: most: bug-fixes and clean-up

[PATCH 00/28] staging: most: bug-fixes and clean-up

[Christian Gromm]

This patchset is needed to have open issues of the MOST driver fixed
and the code cleaned up.

Christian Gromm (28):
  staging: most: remove unnecessary keep_mbo variable
  staging: most: rename variables
  staging: most: simplify expression
  staging: most: unify types
  staging: most: use min_t
  staging: most: fix mbo leak
  staging: most: fix tracking of MBO offset
  staging: most: use readl and writel functions
  staging: most: remove function destroy_most_c_obj
  staging: most: add missing call to ida_simple_remove
  staging: most: move call to disconnect_channel callback
  staging: most: move initialization of pointer
  staging: most: move mutex
  staging: most: move channel disconnect to function
    most_deregister_interface
  staging: most: remove tainted flag
  staging: most: remove reference counter
  staging: most: remove code to destroy channel
  staging: most: remove redundant mutexes
  staging: most: remove redundant call to wake_up_interruptible
  staging: most: encapsulate shared code
  staging: most: fix retrieval of buffer availability
  staging: most: rename variable channel
  staging: most: fix race conditions
  staging: most: change type of access_ref
  staging: most: remove stacked_mbo
  staging: most: rearrange function aim_write
  staging: most: add statistics for dropped packets
  staging: most: remove 2nd forward declaration of struct most_aim

 drivers/staging/most/aim-cdev/cdev.c          |  362 ++++++++++++-------------
 drivers/staging/most/aim-network/networking.c |   11 +-
 drivers/staging/most/hdm-dim2/dim2_hdm.c      |    4 +-
 drivers/staging/most/mostcore/core.c          |  125 ++-------
 drivers/staging/most/mostcore/mostcore.h      |    3 +-
 5 files changed, 218 insertions(+), 287 deletions(-)

-- 
1.7.9.5

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

[PATCH 01/28] staging: most: remove unnecessary keep_mbo variable

[Christian Gromm]

The MBO pointer stacked_mbo and the boolean variable keep_mbo are
always changed together and therefore provide the same information.
This patch removes keep_mbo and uses stacked_mbo instead.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-cdev/cdev.c |   12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
index dc3fb25..6f70ed7 100644
--- a/drivers/staging/most/aim-cdev/cdev.c
+++ b/drivers/staging/most/aim-cdev/cdev.c
@@ -39,7 +39,6 @@ struct aim_channel {
 	struct most_channel_config *cfg;
 	unsigned int channel_id;
 	dev_t devno;
-	bool keep_mbo;
 	unsigned int mbo_offs;
 	struct mbo *stacked_mbo;
 	DECLARE_KFIFO_PTR(fifo, typeof(struct mbo *));
@@ -136,7 +135,7 @@ static int aim_close(struct inode *inode, struct file *filp)
 
 	while (kfifo_out((struct kfifo *)&channel->fifo, &mbo, 1))
 		most_put_mbo(mbo);
-	if (channel->keep_mbo)
+	if (channel->stacked_mbo)
 		most_put_mbo(channel->stacked_mbo);
 	ret = most_stop_channel(channel->iface, channel->channel_id, &cdev_aim);
 	atomic_dec(&channel->access_ref);
@@ -227,9 +226,8 @@ aim_read(struct file *filp, char __user *buf, size_t count, loff_t *offset)
 	struct mbo *mbo;
 	struct aim_channel *channel = filp->private_data;
 
-	if (channel->keep_mbo) {
+	if (channel->stacked_mbo) {
 		mbo = channel->stacked_mbo;
-		channel->keep_mbo = false;
 		goto start_copy;
 	}
 	while ((!kfifo_out(&channel->fifo, &mbo, 1)) && (channel->dev)) {
@@ -249,9 +247,6 @@ start_copy:
 		return -EIO;
 	}
 
-	if (count < mbo->processed_length)
-		channel->keep_mbo = true;
-
 	proc_len = min((int)count,
 		       (int)(mbo->processed_length - channel->mbo_offs));
 
@@ -261,12 +256,13 @@ start_copy:
 
 	retval = not_copied ? proc_len - not_copied : proc_len;
 
-	if (channel->keep_mbo) {
+	if (count < mbo->processed_length) {
 		channel->mbo_offs = retval;
 		channel->stacked_mbo = mbo;
 	} else {
 		most_put_mbo(mbo);
 		channel->mbo_offs = 0;
+		channel->stacked_mbo = NULL;
 	}
 	mutex_unlock(&channel->io_mutex);
 	return retval;
-- 
1.7.9.5

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

[PATCH 02/28] staging: most: rename variables

[Christian Gromm]

This patch renames some variables for better readability.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-cdev/cdev.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
index 6f70ed7..bff891f 100644
--- a/drivers/staging/most/aim-cdev/cdev.c
+++ b/drivers/staging/most/aim-cdev/cdev.c
@@ -221,8 +221,8 @@ error:
 static ssize_t
 aim_read(struct file *filp, char __user *buf, size_t count, loff_t *offset)
 {
-	ssize_t retval;
-	size_t not_copied, proc_len;
+	ssize_t copied;
+	size_t to_copy, not_copied;
 	struct mbo *mbo;
 	struct aim_channel *channel = filp->private_data;
 
@@ -247,17 +247,17 @@ start_copy:
 		return -EIO;
 	}
 
-	proc_len = min((int)count,
-		       (int)(mbo->processed_length - channel->mbo_offs));
+	to_copy = min((int)count,
+		      (int)(mbo->processed_length - channel->mbo_offs));
 
 	not_copied = copy_to_user(buf,
 				  mbo->virt_address + channel->mbo_offs,
-				  proc_len);
+				  to_copy);
 
-	retval = not_copied ? proc_len - not_copied : proc_len;
+	copied = not_copied ? to_copy - not_copied : to_copy;
 
 	if (count < mbo->processed_length) {
-		channel->mbo_offs = retval;
+		channel->mbo_offs = copied;
 		channel->stacked_mbo = mbo;
 	} else {
 		most_put_mbo(mbo);
@@ -265,7 +265,7 @@ start_copy:
 		channel->stacked_mbo = NULL;
 	}
 	mutex_unlock(&channel->io_mutex);
-	return retval;
+	return copied;
 }
 
 static inline bool __must_check IS_ERR_OR_FALSE(int x)
-- 
1.7.9.5

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

[PATCH 03/28] staging: most: simplify expression

[Christian Gromm]

This patch replaces the ternary ?-operator with a way simpler subtraction.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-cdev/cdev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
index bff891f..b293078 100644
--- a/drivers/staging/most/aim-cdev/cdev.c
+++ b/drivers/staging/most/aim-cdev/cdev.c
@@ -254,7 +254,7 @@ start_copy:
 				  mbo->virt_address + channel->mbo_offs,
 				  to_copy);
 
-	copied = not_copied ? to_copy - not_copied : to_copy;
+	copied = to_copy - not_copied;
 
 	if (count < mbo->processed_length) {
 		channel->mbo_offs = copied;
-- 
1.7.9.5

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

[PATCH 04/28] staging: most: unify types

[Christian Gromm]

This patch unifies variable types to get less castings.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-cdev/cdev.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
index b293078..4bf0322 100644
--- a/drivers/staging/most/aim-cdev/cdev.c
+++ b/drivers/staging/most/aim-cdev/cdev.c
@@ -39,7 +39,7 @@ struct aim_channel {
 	struct most_channel_config *cfg;
 	unsigned int channel_id;
 	dev_t devno;
-	unsigned int mbo_offs;
+	size_t mbo_offs;
 	struct mbo *stacked_mbo;
 	DECLARE_KFIFO_PTR(fifo, typeof(struct mbo *));
 	atomic_t access_ref;
@@ -221,8 +221,7 @@ error:
 static ssize_t
 aim_read(struct file *filp, char __user *buf, size_t count, loff_t *offset)
 {
-	ssize_t copied;
-	size_t to_copy, not_copied;
+	size_t to_copy, not_copied, copied;
 	struct mbo *mbo;
 	struct aim_channel *channel = filp->private_data;
 
@@ -247,8 +246,7 @@ start_copy:
 		return -EIO;
 	}
 
-	to_copy = min((int)count,
-		      (int)(mbo->processed_length - channel->mbo_offs));
+	to_copy = min(count, (size_t)mbo->processed_length - channel->mbo_offs);
 
 	not_copied = copy_to_user(buf,
 				  mbo->virt_address + channel->mbo_offs,
-- 
1.7.9.5

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

[PATCH 05/28] staging: most: use min_t

[Christian Gromm]

This patch replaces min with min_t.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-cdev/cdev.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
index 4bf0322..6ee4eb2 100644
--- a/drivers/staging/most/aim-cdev/cdev.c
+++ b/drivers/staging/most/aim-cdev/cdev.c
@@ -246,7 +246,9 @@ start_copy:
 		return -EIO;
 	}
 
-	to_copy = min(count, (size_t)mbo->processed_length - channel->mbo_offs);
+	to_copy = min_t(size_t,
+			count,
+			mbo->processed_length - channel->mbo_offs);
 
 	not_copied = copy_to_user(buf,
 				  mbo->virt_address + channel->mbo_offs,
-- 
1.7.9.5

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

[PATCH 06/28] staging: most: fix mbo leak

[Christian Gromm]

This patch fixes a potential MBO leak in case function aim_read()
exits right after the MBO has been fetched from kfifo and before
it has been saved to the variable stacked_mbo.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-cdev/cdev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
index 6ee4eb2..86194ce 100644
--- a/drivers/staging/most/aim-cdev/cdev.c
+++ b/drivers/staging/most/aim-cdev/cdev.c
@@ -237,6 +237,7 @@ aim_read(struct file *filp, char __user *buf, size_t count, loff_t *offset)
 					      (!channel->dev))))
 			return -ERESTARTSYS;
 	}
+	channel->stacked_mbo = mbo;
 
 start_copy:
 	/* make sure we don't submit to gone devices */
@@ -258,7 +259,6 @@ start_copy:
 
 	if (count < mbo->processed_length) {
 		channel->mbo_offs = copied;
-		channel->stacked_mbo = mbo;
 	} else {
 		most_put_mbo(mbo);
 		channel->mbo_offs = 0;
-- 
1.7.9.5

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

Re: [PATCH 06/28] staging: most: fix mbo leak

[Sudip Mukherjee]

On Wed, Nov 18, 2015 at 01:43:30PM +0100, Christian Gromm wrote:
> This patch fixes a potential MBO leak in case function aim_read()
> exits right after the MBO has been fetched from kfifo and before
> it has been saved to the variable stacked_mbo.
> 
> Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> ---
>  drivers/staging/most/aim-cdev/cdev.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
> index 6ee4eb2..86194ce 100644
> --- a/drivers/staging/most/aim-cdev/cdev.c
> +++ b/drivers/staging/most/aim-cdev/cdev.c
> @@ -237,6 +237,7 @@ aim_read(struct file *filp, char __user *buf, size_t count, loff_t *offset)
>  					      (!channel->dev))))
>  			return -ERESTARTSYS;
>  	}
> +	channel->stacked_mbo = mbo;

You can also come out of the while loop when channel->dev is NULL.

Maybe this should have been:
	if (channel->dev)
		 channel->stacked_mbo = mbo;

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

Re: [PATCH 06/28] staging: most: fix mbo leak

[Christian Gromm]

On Thu, 19 Nov 2015 10:41:24 +0530
Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote:

> On Wed, Nov 18, 2015 at 01:43:30PM +0100, Christian Gromm wrote:
> > This patch fixes a potential MBO leak in case function aim_read()
> > exits right after the MBO has been fetched from kfifo and before
> > it has been saved to the variable stacked_mbo.
> > 
> > Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> > ---
> >  drivers/staging/most/aim-cdev/cdev.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
> > index 6ee4eb2..86194ce 100644
> > --- a/drivers/staging/most/aim-cdev/cdev.c
> > +++ b/drivers/staging/most/aim-cdev/cdev.c
> > @@ -237,6 +237,7 @@ aim_read(struct file *filp, char __user *buf, size_t count, loff_t *offset)
> >  					      (!channel->dev))))
> >  			return -ERESTARTSYS;
> >  	}
> > +	channel->stacked_mbo = mbo;
> 
> You can also come out of the while loop when channel->dev is NULL.
> 
> Maybe this should have been:
> 	if (channel->dev)
> 		 channel->stacked_mbo = mbo;

You're probably right. But the stacked_mbo pointer is going to be removed
with patch 25 of this patchset anyway.

This patch is just a part to get the whole story across.

Thanks,
Chris

> 
> regards
> sudip

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

[PATCH 07/28] staging: most: fix tracking of MBO offset

[Christian Gromm]

This patch increments mbo_offs by the number of bytes that have
been copied and resets it in case a complete mbo has been transferred
to user buffer.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-cdev/cdev.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
index 86194ce..0141293 100644
--- a/drivers/staging/most/aim-cdev/cdev.c
+++ b/drivers/staging/most/aim-cdev/cdev.c
@@ -257,9 +257,8 @@ start_copy:
 
 	copied = to_copy - not_copied;
 
-	if (count < mbo->processed_length) {
-		channel->mbo_offs = copied;
-	} else {
+	channel->mbo_offs += copied;
+	if (channel->mbo_offs >= mbo->processed_length) {
 		most_put_mbo(mbo);
 		channel->mbo_offs = 0;
 		channel->stacked_mbo = NULL;
-- 
1.7.9.5

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

[PATCH 08/28] staging: most: use readl and writel functions

[Christian Gromm]

This patch makes use of functions readl and writel instead of
the __raw_* variants.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/hdm-dim2/dim2_hdm.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/most/hdm-dim2/dim2_hdm.c b/drivers/staging/most/hdm-dim2/dim2_hdm.c
index 327d738..48ce7ab 100644
--- a/drivers/staging/most/hdm-dim2/dim2_hdm.c
+++ b/drivers/staging/most/hdm-dim2/dim2_hdm.c
@@ -140,7 +140,7 @@ bool dim2_sysfs_get_state_cb(void)
  */
 u32 dimcb_io_read(u32 *ptr32)
 {
-	return __raw_readl(ptr32);
+	return readl(ptr32);
 }
 
 /**
@@ -150,7 +150,7 @@ u32 dimcb_io_read(u32 *ptr32)
  */
 void dimcb_io_write(u32 *ptr32, u32 value)
 {
-	__raw_writel(value, ptr32);
+	writel(value, ptr32);
 }
 
 /**
-- 
1.7.9.5

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

[PATCH 09/28] staging: most: remove function destroy_most_c_obj

[Christian Gromm]

This patch removes the function destroy_most_c_obj and executes its code
within function destroy_most_inst_obj.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/mostcore/core.c |   38 +++++++++++++---------------------
 1 file changed, 14 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/most/mostcore/core.c b/drivers/staging/most/mostcore/core.c
index ed1ed25..8be6cdcf 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -551,29 +551,6 @@ create_most_c_obj(const char *name, struct kobject *parent)
 	return c;
 }
 
-/**
- * destroy_most_c_obj - channel release function
- * @c: pointer to channel object
- *
- * This decrements the reference counter of the channel object.
- * If the reference count turns zero, its release function is called.
- */
-static void destroy_most_c_obj(struct most_c_obj *c)
-{
-	if (c->aim0.ptr)
-		c->aim0.ptr->disconnect_channel(c->iface, c->channel_id);
-	if (c->aim1.ptr)
-		c->aim1.ptr->disconnect_channel(c->iface, c->channel_id);
-	c->aim0.ptr = NULL;
-	c->aim1.ptr = NULL;
-
-	mutex_lock(&deregister_mutex);
-	flush_trash_fifo(c);
-	flush_channel_fifos(c);
-	mutex_unlock(&deregister_mutex);
-	kobject_put(&c->kobj);
-}
-
 /*		     ___	       ___
  *		     ___I N S T A N C E___
  */
@@ -766,7 +743,20 @@ static void destroy_most_inst_obj(struct most_inst_obj *inst)
 	 * reference count of the inst->kobj
 	 */
 	list_for_each_entry_safe(c, tmp, &inst->channel_list, list) {
-		destroy_most_c_obj(c);
+		if (c->aim0.ptr)
+			c->aim0.ptr->disconnect_channel(c->iface,
+							c->channel_id);
+		if (c->aim1.ptr)
+			c->aim1.ptr->disconnect_channel(c->iface,
+							c->channel_id);
+		c->aim0.ptr = NULL;
+		c->aim1.ptr = NULL;
+
+		mutex_lock(&deregister_mutex);
+		flush_trash_fifo(c);
+		flush_channel_fifos(c);
+		mutex_unlock(&deregister_mutex);
+		kobject_put(&c->kobj);
 	}
 	kobject_put(&inst->kobj);
 }
-- 
1.7.9.5

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

[PATCH 10/28] staging: most: add missing call to ida_simple_remove

[Christian Gromm]

This patch adds two missing calls to function ida_simpel_remove.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/mostcore/core.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/most/mostcore/core.c b/drivers/staging/most/mostcore/core.c
index 8be6cdcf..995987d 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -1752,6 +1752,7 @@ struct kobject *most_register_interface(struct most_interface *iface)
 	inst = create_most_inst_obj(name);
 	if (!inst) {
 		pr_info("Failed to allocate interface instance\n");
+		ida_simple_remove(&mdev_id, id);
 		return ERR_PTR(-ENOMEM);
 	}
 
@@ -1808,6 +1809,7 @@ struct kobject *most_register_interface(struct most_interface *iface)
 free_instance:
 	pr_info("Failed allocate channel(s)\n");
 	list_del(&inst->list);
+	ida_simple_remove(&mdev_id, id);
 	destroy_most_inst_obj(inst);
 	return ERR_PTR(-ENOMEM);
 }
-- 
1.7.9.5

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

[PATCH 11/28] staging: most: move call to disconnect_channel callback

[Christian Gromm]

This patch invokes AIM's disconnect_channel callback before the
corresponding pointers are re-initialized to NULL.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/mostcore/core.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/most/mostcore/core.c b/drivers/staging/most/mostcore/core.c
index 995987d..d5ef61d 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -1046,12 +1046,12 @@ static ssize_t store_remove_link(struct most_aim_obj *aim_obj,
 	if (IS_ERR(c))
 		return -ENODEV;
 
+	if (aim_obj->driver->disconnect_channel(c->iface, c->channel_id))
+		return -EIO;
 	if (c->aim0.ptr == aim_obj->driver)
 		c->aim0.ptr = NULL;
 	if (c->aim1.ptr == aim_obj->driver)
 		c->aim1.ptr = NULL;
-	if (aim_obj->driver->disconnect_channel(c->iface, c->channel_id))
-		return -EIO;
 	return len;
 }
 
-- 
1.7.9.5

[PATCH 12/28] staging: most: move initialization of pointer

[Christian Gromm]

This patch makes function store_add_link initialize the pointer to an AIM
right before the channel is probed.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/mostcore/core.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/most/mostcore/core.c b/drivers/staging/most/mostcore/core.c
index d5ef61d..21c550c 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -996,11 +996,14 @@ static ssize_t store_add_link(struct most_aim_obj *aim_obj,
 	else
 		return -ENOSPC;
 
+	*aim_ptr = aim_obj->driver;
 	ret = aim_obj->driver->probe_channel(c->iface, c->channel_id,
 					     &c->cfg, &c->kobj, mdev_devnod);
-	if (ret)
+	if (ret) {
+		*aim_ptr = NULL;
 		return ret;
-	*aim_ptr = aim_obj->driver;
+	}
+
 	return len;
 }
 
-- 
1.7.9.5

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

Re: [PATCH 12/28] staging: most: move initialization of pointer

[Dan Carpenter]

On Wed, Nov 18, 2015 at 01:43:36PM +0100, Christian Gromm wrote:
> This patch makes function store_add_link initialize the pointer to an AIM
> right before the channel is probed.
> 

The patch description implies that this is a cleanup but looking at the
patch it looks like a behavior change.  Actually for quite a few of
these it definitely is a bugfix but it's not clear what the bug would
look like to the user.  Please always be explicit.  For example,
calculating the offset badly seems like it might have security
implications.  Don't be ambiguous if that's the case.

regards,
dan carpenter

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

[PATCH 13/28] staging: most: move mutex

[Christian Gromm]

This patch removes mutex from code that doesn't need any locking.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/mostcore/core.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/most/mostcore/core.c b/drivers/staging/most/mostcore/core.c
index 21c550c..ae1b577 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -1830,15 +1830,14 @@ void most_deregister_interface(struct most_interface *iface)
 	struct most_inst_obj *i = iface->priv;
 	struct most_c_obj *c;
 
-	mutex_lock(&deregister_mutex);
 	if (unlikely(!i)) {
 		pr_info("Bad Interface\n");
-		mutex_unlock(&deregister_mutex);
 		return;
 	}
 	pr_info("deregistering MOST device %s (%s)\n", i->kobj.name,
 		iface->description);
 
+	mutex_lock(&deregister_mutex);
 	atomic_set(&i->tainted, 1);
 	mutex_unlock(&deregister_mutex);
 
-- 
1.7.9.5

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

[PATCH 14/28] staging: most: move channel disconnect to function most_deregister_interface

[Christian Gromm]

This patch moves the code that disconnects linked channels. It is needed
to have cleaning things up done right.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/mostcore/core.c |   24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/most/mostcore/core.c b/drivers/staging/most/mostcore/core.c
index ae1b577..782747a 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -738,20 +738,7 @@ static void destroy_most_inst_obj(struct most_inst_obj *inst)
 {
 	struct most_c_obj *c, *tmp;
 
-	/* need to destroy channels first, since
-	 * each channel incremented the
-	 * reference count of the inst->kobj
-	 */
 	list_for_each_entry_safe(c, tmp, &inst->channel_list, list) {
-		if (c->aim0.ptr)
-			c->aim0.ptr->disconnect_channel(c->iface,
-							c->channel_id);
-		if (c->aim1.ptr)
-			c->aim1.ptr->disconnect_channel(c->iface,
-							c->channel_id);
-		c->aim0.ptr = NULL;
-		c->aim1.ptr = NULL;
-
 		mutex_lock(&deregister_mutex);
 		flush_trash_fifo(c);
 		flush_channel_fifos(c);
@@ -1837,6 +1824,17 @@ void most_deregister_interface(struct most_interface *iface)
 	pr_info("deregistering MOST device %s (%s)\n", i->kobj.name,
 		iface->description);
 
+	list_for_each_entry(c, &i->channel_list, list) {
+		if (c->aim0.ptr)
+			c->aim0.ptr->disconnect_channel(c->iface,
+							c->channel_id);
+		if (c->aim1.ptr)
+			c->aim1.ptr->disconnect_channel(c->iface,
+							c->channel_id);
+		c->aim0.ptr = NULL;
+		c->aim1.ptr = NULL;
+	}
+
 	mutex_lock(&deregister_mutex);
 	atomic_set(&i->tainted, 1);
 	mutex_unlock(&deregister_mutex);
-- 
1.7.9.5

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

[PATCH 15/28] staging: most: remove tainted flag

[Christian Gromm]

This patch removes the atomic tainted flag. It is needed to get rid of
logical overhead.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/mostcore/core.c |   33 +--------------------------------
 1 file changed, 1 insertion(+), 32 deletions(-)

diff --git a/drivers/staging/most/mostcore/core.c b/drivers/staging/most/mostcore/core.c
index 782747a..58e288b 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -74,7 +74,6 @@ struct most_c_obj {
 
 struct most_inst_obj {
 	int dev_id;
-	atomic_t tainted;
 	struct most_interface *iface;
 	struct list_head channel_list;
 	struct most_c_obj *channel[MAX_CHANNELS];
@@ -1299,18 +1298,10 @@ _exit:
  */
 int most_submit_mbo(struct mbo *mbo)
 {
-	struct most_c_obj *c;
-	struct most_inst_obj *i;
-
 	if (unlikely((!mbo) || (!mbo->context))) {
 		pr_err("Bad MBO or missing channel reference\n");
 		return -EINVAL;
 	}
-	c = mbo->context;
-	i = c->inst;
-
-	if (unlikely(atomic_read(&i->tainted)))
-		return -ENODEV;
 
 	nq_hdm_mbo(mbo);
 	return 0;
@@ -1436,17 +1427,8 @@ EXPORT_SYMBOL_GPL(most_get_mbo);
  */
 void most_put_mbo(struct mbo *mbo)
 {
-	struct most_c_obj *c;
-	struct most_inst_obj *i;
-
-	c = mbo->context;
-	i = c->inst;
+	struct most_c_obj *c = mbo->context;
 
-	if (unlikely(atomic_read(&i->tainted))) {
-		mbo->status = MBO_E_CLOSE;
-		trash_mbo(mbo);
-		return;
-	}
 	if (c->cfg.direction == MOST_CH_TX) {
 		arm_mbo(mbo);
 		return;
@@ -1602,14 +1584,6 @@ int most_stop_channel(struct most_interface *iface, int id,
 	c->hdm_enqueue_task = NULL;
 	mutex_unlock(&c->stop_task_mutex);
 
-	mutex_lock(&deregister_mutex);
-	if (atomic_read(&c->inst->tainted)) {
-		mutex_unlock(&deregister_mutex);
-		mutex_unlock(&c->start_mutex);
-		return -ENODEV;
-	}
-	mutex_unlock(&deregister_mutex);
-
 	if (iface->mod && modref) {
 		module_put(iface->mod);
 		modref--;
@@ -1750,7 +1724,6 @@ struct kobject *most_register_interface(struct most_interface *iface)
 	INIT_LIST_HEAD(&inst->channel_list);
 	inst->iface = iface;
 	inst->dev_id = id;
-	atomic_set(&inst->tainted, 0);
 	list_add_tail(&inst->list, &instance_list);
 
 	for (i = 0; i < iface->num_channels; i++) {
@@ -1835,10 +1808,6 @@ void most_deregister_interface(struct most_interface *iface)
 		c->aim1.ptr = NULL;
 	}
 
-	mutex_lock(&deregister_mutex);
-	atomic_set(&i->tainted, 1);
-	mutex_unlock(&deregister_mutex);
-
 	while (modref) {
 		if (iface->mod && modref)
 			module_put(iface->mod);
-- 
1.7.9.5

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

[PATCH 16/28] staging: most: remove reference counter

[Christian Gromm]

This patch removes the unnecessary reference conter mod_ref.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/mostcore/core.c |   13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/staging/most/mostcore/core.c b/drivers/staging/most/mostcore/core.c
index 58e288b..daae42d 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -35,7 +35,6 @@
 static struct class *most_class;
 static struct device *class_glue_dir;
 static struct ida mdev_id;
-static int modref;
 static int dummy_num_buffers;
 
 struct most_c_aim_obj {
@@ -1508,7 +1507,6 @@ int most_start_channel(struct most_interface *iface, int id,
 		mutex_unlock(&c->start_mutex);
 		return -ENOLCK;
 	}
-	modref++;
 
 	c->cfg.extra_len = 0;
 	if (c->iface->configure(c->iface, c->channel_id, &c->cfg)) {
@@ -1550,7 +1548,6 @@ out:
 
 error:
 	module_put(iface->mod);
-	modref--;
 	mutex_unlock(&c->start_mutex);
 	return ret;
 }
@@ -1584,10 +1581,8 @@ int most_stop_channel(struct most_interface *iface, int id,
 	c->hdm_enqueue_task = NULL;
 	mutex_unlock(&c->stop_task_mutex);
 
-	if (iface->mod && modref) {
+	if (iface->mod)
 		module_put(iface->mod);
-		modref--;
-	}
 
 	c->is_poisoned = true;
 	if (c->iface->poison_channel(c->iface, c->channel_id)) {
@@ -1808,12 +1803,6 @@ void most_deregister_interface(struct most_interface *iface)
 		c->aim1.ptr = NULL;
 	}
 
-	while (modref) {
-		if (iface->mod && modref)
-			module_put(iface->mod);
-		modref--;
-	}
-
 	list_for_each_entry(c, &i->channel_list, list) {
 		if (c->aim0.refs + c->aim1.refs <= 0)
 			continue;
-- 
1.7.9.5

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

[PATCH 17/28] staging: most: remove code to destroy channel

[Christian Gromm]

This patch removes unnecessary code to destroy channel objects. It is
needed, because function most_stop_channel, which is indirectly
triggered by function most_deregister_interface, already destroys the
channels.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/mostcore/core.c |   13 -------------
 1 file changed, 13 deletions(-)

diff --git a/drivers/staging/most/mostcore/core.c b/drivers/staging/most/mostcore/core.c
index daae42d..31ea3a5 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -1803,19 +1803,6 @@ void most_deregister_interface(struct most_interface *iface)
 		c->aim1.ptr = NULL;
 	}
 
-	list_for_each_entry(c, &i->channel_list, list) {
-		if (c->aim0.refs + c->aim1.refs <= 0)
-			continue;
-
-		mutex_lock(&c->stop_task_mutex);
-		if (c->hdm_enqueue_task)
-			kthread_stop(c->hdm_enqueue_task);
-		c->hdm_enqueue_task = NULL;
-		mutex_unlock(&c->stop_task_mutex);
-
-		if (iface->poison_channel(iface, c->channel_id))
-			pr_err("Can't poison channel %d\n", c->channel_id);
-	}
 	ida_simple_remove(&mdev_id, i->dev_id);
 	list_del(&i->list);
 	destroy_most_inst_obj(i);
-- 
1.7.9.5

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

[PATCH 18/28] staging: most: remove redundant mutexes

[Christian Gromm]

This patch removes the mutexes stop_task_mutex and deregister mutex,
since they can safely be left out.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/mostcore/core.c |    9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/staging/most/mostcore/core.c b/drivers/staging/most/mostcore/core.c
index 31ea3a5..b085f0a 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -65,7 +65,6 @@ struct most_c_obj {
 	struct most_c_aim_obj aim1;
 	struct list_head trash_fifo;
 	struct task_struct *hdm_enqueue_task;
-	struct mutex stop_task_mutex;
 	wait_queue_head_t hdm_fifo_wq;
 };
 
@@ -93,8 +92,6 @@ struct most_inst_obj {
 	_mbo;								\
 })
 
-static struct mutex deregister_mutex;
-
 /*		     ___	     ___
  *		     ___C H A N N E L___
  */
@@ -737,10 +734,8 @@ static void destroy_most_inst_obj(struct most_inst_obj *inst)
 	struct most_c_obj *c, *tmp;
 
 	list_for_each_entry_safe(c, tmp, &inst->channel_list, list) {
-		mutex_lock(&deregister_mutex);
 		flush_trash_fifo(c);
 		flush_channel_fifos(c);
-		mutex_unlock(&deregister_mutex);
 		kobject_put(&c->kobj);
 	}
 	kobject_put(&inst->kobj);
@@ -1575,11 +1570,9 @@ int most_stop_channel(struct most_interface *iface, int id,
 	if (c->aim0.refs + c->aim1.refs >= 2)
 		goto out;
 
-	mutex_lock(&c->stop_task_mutex);
 	if (c->hdm_enqueue_task)
 		kthread_stop(c->hdm_enqueue_task);
 	c->hdm_enqueue_task = NULL;
-	mutex_unlock(&c->stop_task_mutex);
 
 	if (iface->mod)
 		module_put(iface->mod);
@@ -1757,7 +1750,6 @@ struct kobject *most_register_interface(struct most_interface *iface)
 		init_completion(&c->cleanup);
 		atomic_set(&c->mbo_ref, 0);
 		mutex_init(&c->start_mutex);
-		mutex_init(&c->stop_task_mutex);
 		list_add_tail(&c->list, &inst->channel_list);
 	}
 	pr_info("registered new MOST device mdev%d (%s)\n",
@@ -1853,7 +1845,6 @@ static int __init most_init(void)
 	pr_info("init()\n");
 	INIT_LIST_HEAD(&instance_list);
 	INIT_LIST_HEAD(&aim_list);
-	mutex_init(&deregister_mutex);
 	ida_init(&mdev_id);
 
 	if (bus_register(&most_bus)) {
-- 
1.7.9.5

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

[PATCH 19/28] staging: most: remove redundant call to wake_up_interruptible

[Christian Gromm]

This patch prevents the cdev module from rousing the channel wait queue in
case the channel is about to be closed. It is safe to do so, because the
application can not be waiting within read or write and at the same time
be calling close.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-cdev/cdev.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
index 0141293..fd2ac89 100644
--- a/drivers/staging/most/aim-cdev/cdev.c
+++ b/drivers/staging/most/aim-cdev/cdev.c
@@ -127,7 +127,6 @@ static int aim_close(struct inode *inode, struct file *filp)
 		kfifo_free(&channel->fifo);
 		list_del(&channel->list);
 		ida_simple_remove(&minor_id, MINOR(channel->devno));
-		wake_up_interruptible(&channel->wq);
 		kfree(channel);
 		return 0;
 	}
@@ -139,7 +138,6 @@ static int aim_close(struct inode *inode, struct file *filp)
 		most_put_mbo(channel->stacked_mbo);
 	ret = most_stop_channel(channel->iface, channel->channel_id, &cdev_aim);
 	atomic_dec(&channel->access_ref);
-	wake_up_interruptible(&channel->wq);
 	return ret;
 }
 
-- 
1.7.9.5

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

[PATCH 20/28] staging: most: encapsulate shared code

[Christian Gromm]

This patch encapsulates shared code. It therefore creates the new functions
stop_channel and destroy_cdev.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-cdev/cdev.c |   55 ++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
index fd2ac89..533e290 100644
--- a/drivers/staging/most/aim-cdev/cdev.c
+++ b/drivers/staging/most/aim-cdev/cdev.c
@@ -69,6 +69,30 @@ static struct aim_channel *get_channel(struct most_interface *iface, int id)
 	return channel;
 }
 
+static void stop_channel(struct aim_channel *c)
+{
+	struct mbo *mbo;
+
+	while (kfifo_out((struct kfifo *)&c->fifo, &mbo, 1))
+		most_put_mbo(mbo);
+	if (c->stacked_mbo)
+		most_put_mbo(c->stacked_mbo);
+	most_stop_channel(c->iface, c->channel_id, &cdev_aim);
+}
+
+static void destroy_cdev(struct aim_channel *c)
+{
+	unsigned long flags;
+
+	device_destroy(aim_class, c->devno);
+	cdev_del(&c->cdev);
+	kfifo_free(&c->fifo);
+	spin_lock_irqsave(&ch_list_lock, flags);
+	list_del(&c->list);
+	spin_unlock_irqrestore(&ch_list_lock, flags);
+	ida_simple_remove(&minor_id, MINOR(c->devno));
+}
+
 /**
  * aim_open - implements the syscall to open the device
  * @inode: inode pointer
@@ -114,31 +138,21 @@ static int aim_open(struct inode *inode, struct file *filp)
  */
 static int aim_close(struct inode *inode, struct file *filp)
 {
-	int ret;
-	struct mbo *mbo;
 	struct aim_channel *channel = to_channel(inode->i_cdev);
 
 	mutex_lock(&channel->io_mutex);
 	if (!channel->dev) {
 		mutex_unlock(&channel->io_mutex);
 		atomic_dec(&channel->access_ref);
-		device_destroy(aim_class, channel->devno);
-		cdev_del(&channel->cdev);
-		kfifo_free(&channel->fifo);
-		list_del(&channel->list);
-		ida_simple_remove(&minor_id, MINOR(channel->devno));
+		destroy_cdev(channel);
 		kfree(channel);
 		return 0;
 	}
 	mutex_unlock(&channel->io_mutex);
 
-	while (kfifo_out((struct kfifo *)&channel->fifo, &mbo, 1))
-		most_put_mbo(mbo);
-	if (channel->stacked_mbo)
-		most_put_mbo(channel->stacked_mbo);
-	ret = most_stop_channel(channel->iface, channel->channel_id, &cdev_aim);
+	stop_channel(channel);
 	atomic_dec(&channel->access_ref);
-	return ret;
+	return 0;
 }
 
 /**
@@ -310,7 +324,6 @@ static const struct file_operations channel_fops = {
 static int aim_disconnect_channel(struct most_interface *iface, int channel_id)
 {
 	struct aim_channel *channel;
-	unsigned long flags;
 
 	if (!iface) {
 		pr_info("Bad interface pointer\n");
@@ -326,13 +339,7 @@ static int aim_disconnect_channel(struct most_interface *iface, int channel_id)
 	mutex_unlock(&channel->io_mutex);
 
 	if (atomic_read(&channel->access_ref)) {
-		device_destroy(aim_class, channel->devno);
-		cdev_del(&channel->cdev);
-		kfifo_free(&channel->fifo);
-		ida_simple_remove(&minor_id, MINOR(channel->devno));
-		spin_lock_irqsave(&ch_list_lock, flags);
-		list_del(&channel->list);
-		spin_unlock_irqrestore(&ch_list_lock, flags);
+		destroy_cdev(channel);
 		kfree(channel);
 	} else {
 		wake_up_interruptible(&channel->wq);
@@ -526,11 +533,7 @@ static void __exit mod_exit(void)
 	most_deregister_aim(&cdev_aim);
 
 	list_for_each_entry_safe(channel, tmp, &channel_list, list) {
-		device_destroy(aim_class, channel->devno);
-		cdev_del(&channel->cdev);
-		kfifo_free(&channel->fifo);
-		list_del(&channel->list);
-		ida_simple_remove(&minor_id, MINOR(channel->devno));
+		destroy_cdev(channel);
 		kfree(channel);
 	}
 	class_destroy(aim_class);
-- 
1.7.9.5

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

[PATCH 21/28] staging: most: fix retrieval of buffer availability

[Christian Gromm]

This patch fixes the function channel_has_mbo that delivers the false
information in case two AIMs are using the same tx channel.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-cdev/cdev.c     |   12 ++++++------
 drivers/staging/most/mostcore/core.c     |    7 ++++++-
 drivers/staging/most/mostcore/mostcore.h |    3 ++-
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
index 533e290..c4bbf7d 100644
--- a/drivers/staging/most/aim-cdev/cdev.c
+++ b/drivers/staging/most/aim-cdev/cdev.c
@@ -50,6 +50,11 @@ struct aim_channel {
 static struct list_head channel_list;
 static spinlock_t ch_list_lock;
 
+static inline bool ch_has_mbo(struct aim_channel *c)
+{
+	return channel_has_mbo(c->iface, c->channel_id, &cdev_aim) > 0;
+}
+
 static struct aim_channel *get_channel(struct most_interface *iface, int id)
 {
 	struct aim_channel *channel, *tmp;
@@ -279,11 +284,6 @@ start_copy:
 	return copied;
 }
 
-static inline bool __must_check IS_ERR_OR_FALSE(int x)
-{
-	return x <= 0;
-}
-
 static unsigned int aim_poll(struct file *filp, poll_table *wait)
 {
 	struct aim_channel *c = filp->private_data;
@@ -295,7 +295,7 @@ static unsigned int aim_poll(struct file *filp, poll_table *wait)
 		if (!kfifo_is_empty(&c->fifo))
 			mask |= POLLIN | POLLRDNORM;
 	} else {
-		if (!IS_ERR_OR_FALSE(channel_has_mbo(c->iface, c->channel_id)))
+		if (ch_has_mbo(c))
 			mask |= POLLOUT | POLLWRNORM;
 	}
 	return mask;
diff --git a/drivers/staging/most/mostcore/core.c b/drivers/staging/most/mostcore/core.c
index b085f0a..ff0e0dc 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -1352,7 +1352,7 @@ most_c_obj *get_channel_by_iface(struct most_interface *iface, int id)
 	return i->channel[id];
 }
 
-int channel_has_mbo(struct most_interface *iface, int id)
+int channel_has_mbo(struct most_interface *iface, int id, struct most_aim *aim)
 {
 	struct most_c_obj *c = get_channel_by_iface(iface, id);
 	unsigned long flags;
@@ -1361,6 +1361,11 @@ int channel_has_mbo(struct most_interface *iface, int id)
 	if (unlikely(!c))
 		return -EINVAL;
 
+	if (c->aim0.refs && c->aim1.refs &&
+	    ((aim == c->aim0.ptr && c->aim0.num_buffers <= 0) ||
+	     (aim == c->aim1.ptr && c->aim1.num_buffers <= 0)))
+		return false;
+
 	spin_lock_irqsave(&c->fifo_lock, flags);
 	empty = list_empty(&c->fifo);
 	spin_unlock_irqrestore(&c->fifo_lock, flags);
diff --git a/drivers/staging/most/mostcore/mostcore.h b/drivers/staging/most/mostcore/mostcore.h
index bda3850..60e018e 100644
--- a/drivers/staging/most/mostcore/mostcore.h
+++ b/drivers/staging/most/mostcore/mostcore.h
@@ -310,7 +310,8 @@ int most_deregister_aim(struct most_aim *aim);
 struct mbo *most_get_mbo(struct most_interface *iface, int channel_idx,
 			 struct most_aim *);
 void most_put_mbo(struct mbo *mbo);
-int channel_has_mbo(struct most_interface *iface, int channel_idx);
+int channel_has_mbo(struct most_interface *iface, int channel_idx,
+		    struct most_aim *aim);
 int most_start_channel(struct most_interface *iface, int channel_idx,
 		       struct most_aim *);
 int most_stop_channel(struct most_interface *iface, int channel_idx,
-- 
1.7.9.5

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

[PATCH 22/28] staging: most: rename variable channel

[Christian Gromm]

This patch renames the variable 'channel' to 'c'. This is needed to have
the code look more homogeneous and to prevent format violations.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-cdev/cdev.c |  216 +++++++++++++++++-----------------
 1 file changed, 108 insertions(+), 108 deletions(-)

diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
index c4bbf7d..5065139 100644
--- a/drivers/staging/most/aim-cdev/cdev.c
+++ b/drivers/staging/most/aim-cdev/cdev.c
@@ -57,13 +57,13 @@ static inline bool ch_has_mbo(struct aim_channel *c)
 
 static struct aim_channel *get_channel(struct most_interface *iface, int id)
 {
-	struct aim_channel *channel, *tmp;
+	struct aim_channel *c, *tmp;
 	unsigned long flags;
 	int found_channel = 0;
 
 	spin_lock_irqsave(&ch_list_lock, flags);
-	list_for_each_entry_safe(channel, tmp, &channel_list, list) {
-		if ((channel->iface == iface) && (channel->channel_id == id)) {
+	list_for_each_entry_safe(c, tmp, &channel_list, list) {
+		if ((c->iface == iface) && (c->channel_id == id)) {
 			found_channel = 1;
 			break;
 		}
@@ -71,7 +71,7 @@ static struct aim_channel *get_channel(struct most_interface *iface, int id)
 	spin_unlock_irqrestore(&ch_list_lock, flags);
 	if (!found_channel)
 		return NULL;
-	return channel;
+	return c;
 }
 
 static void stop_channel(struct aim_channel *c)
@@ -108,29 +108,29 @@ static void destroy_cdev(struct aim_channel *c)
  */
 static int aim_open(struct inode *inode, struct file *filp)
 {
-	struct aim_channel *channel;
+	struct aim_channel *c;
 	int ret;
 
-	channel = to_channel(inode->i_cdev);
-	filp->private_data = channel;
+	c = to_channel(inode->i_cdev);
+	filp->private_data = c;
 
-	if (((channel->cfg->direction == MOST_CH_RX) &&
+	if (((c->cfg->direction == MOST_CH_RX) &&
 	     ((filp->f_flags & O_ACCMODE) != O_RDONLY)) ||
-	     ((channel->cfg->direction == MOST_CH_TX) &&
+	     ((c->cfg->direction == MOST_CH_TX) &&
 		((filp->f_flags & O_ACCMODE) != O_WRONLY))) {
 		pr_info("WARN: Access flags mismatch\n");
 		return -EACCES;
 	}
-	if (!atomic_inc_and_test(&channel->access_ref)) {
+	if (!atomic_inc_and_test(&c->access_ref)) {
 		pr_info("WARN: Device is busy\n");
-		atomic_dec(&channel->access_ref);
+		atomic_dec(&c->access_ref);
 		return -EBUSY;
 	}
 
-	ret = most_start_channel(channel->iface, channel->channel_id,
+	ret = most_start_channel(c->iface, c->channel_id,
 				 &cdev_aim);
 	if (ret)
-		atomic_dec(&channel->access_ref);
+		atomic_dec(&c->access_ref);
 	return ret;
 }
 
@@ -143,20 +143,20 @@ static int aim_open(struct inode *inode, struct file *filp)
  */
 static int aim_close(struct inode *inode, struct file *filp)
 {
-	struct aim_channel *channel = to_channel(inode->i_cdev);
-
-	mutex_lock(&channel->io_mutex);
-	if (!channel->dev) {
-		mutex_unlock(&channel->io_mutex);
-		atomic_dec(&channel->access_ref);
-		destroy_cdev(channel);
-		kfree(channel);
+	struct aim_channel *c = to_channel(inode->i_cdev);
+
+	mutex_lock(&c->io_mutex);
+	if (!c->dev) {
+		mutex_unlock(&c->io_mutex);
+		atomic_dec(&c->access_ref);
+		destroy_cdev(c);
+		kfree(c);
 		return 0;
 	}
-	mutex_unlock(&channel->io_mutex);
+	mutex_unlock(&c->io_mutex);
 
-	stop_channel(channel);
-	atomic_dec(&channel->access_ref);
+	stop_channel(c);
+	atomic_dec(&c->access_ref);
 	return 0;
 }
 
@@ -175,38 +175,38 @@ static ssize_t aim_write(struct file *filp, const char __user *buf,
 	size_t max_len = 0;
 	ssize_t retval;
 	struct mbo *mbo;
-	struct aim_channel *channel = filp->private_data;
+	struct aim_channel *c = filp->private_data;
 
-	mutex_lock(&channel->io_mutex);
-	if (unlikely(!channel->dev)) {
-		mutex_unlock(&channel->io_mutex);
+	mutex_lock(&c->io_mutex);
+	if (unlikely(!c->dev)) {
+		mutex_unlock(&c->io_mutex);
 		return -EPIPE;
 	}
-	mutex_unlock(&channel->io_mutex);
+	mutex_unlock(&c->io_mutex);
 
-	mbo = most_get_mbo(channel->iface, channel->channel_id, &cdev_aim);
+	mbo = most_get_mbo(c->iface, c->channel_id, &cdev_aim);
 
 	if (!mbo) {
 		if ((filp->f_flags & O_NONBLOCK))
 			return -EAGAIN;
 		if (wait_event_interruptible(
-			    channel->wq,
-			    (mbo = most_get_mbo(channel->iface,
-						channel->channel_id,
+			    c->wq,
+			    (mbo = most_get_mbo(c->iface,
+						c->channel_id,
 						&cdev_aim)) ||
-			    (!channel->dev)))
+			    (!c->dev)))
 			return -ERESTARTSYS;
 	}
 
-	mutex_lock(&channel->io_mutex);
-	if (unlikely(!channel->dev)) {
-		mutex_unlock(&channel->io_mutex);
+	mutex_lock(&c->io_mutex);
+	if (unlikely(!c->dev)) {
+		mutex_unlock(&c->io_mutex);
 		err = -EPIPE;
 		goto error;
 	}
-	mutex_unlock(&channel->io_mutex);
+	mutex_unlock(&c->io_mutex);
 
-	max_len = channel->cfg->buffer_size;
+	max_len = c->cfg->buffer_size;
 	actual_len = min(count, max_len);
 	mbo->buffer_length = actual_len;
 
@@ -240,47 +240,47 @@ aim_read(struct file *filp, char __user *buf, size_t count, loff_t *offset)
 {
 	size_t to_copy, not_copied, copied;
 	struct mbo *mbo;
-	struct aim_channel *channel = filp->private_data;
+	struct aim_channel *c = filp->private_data;
 
-	if (channel->stacked_mbo) {
-		mbo = channel->stacked_mbo;
+	if (c->stacked_mbo) {
+		mbo = c->stacked_mbo;
 		goto start_copy;
 	}
-	while ((!kfifo_out(&channel->fifo, &mbo, 1)) && (channel->dev)) {
+	while ((!kfifo_out(&c->fifo, &mbo, 1)) && (c->dev)) {
 		if (filp->f_flags & O_NONBLOCK)
 			return -EAGAIN;
-		if (wait_event_interruptible(channel->wq,
-					     (!kfifo_is_empty(&channel->fifo) ||
-					      (!channel->dev))))
+		if (wait_event_interruptible(c->wq,
+					     (!kfifo_is_empty(&c->fifo) ||
+					      (!c->dev))))
 			return -ERESTARTSYS;
 	}
-	channel->stacked_mbo = mbo;
+	c->stacked_mbo = mbo;
 
 start_copy:
 	/* make sure we don't submit to gone devices */
-	mutex_lock(&channel->io_mutex);
-	if (unlikely(!channel->dev)) {
-		mutex_unlock(&channel->io_mutex);
+	mutex_lock(&c->io_mutex);
+	if (unlikely(!c->dev)) {
+		mutex_unlock(&c->io_mutex);
 		return -EIO;
 	}
 
 	to_copy = min_t(size_t,
 			count,
-			mbo->processed_length - channel->mbo_offs);
+			mbo->processed_length - c->mbo_offs);
 
 	not_copied = copy_to_user(buf,
-				  mbo->virt_address + channel->mbo_offs,
+				  mbo->virt_address + c->mbo_offs,
 				  to_copy);
 
 	copied = to_copy - not_copied;
 
-	channel->mbo_offs += copied;
-	if (channel->mbo_offs >= mbo->processed_length) {
+	c->mbo_offs += copied;
+	if (c->mbo_offs >= mbo->processed_length) {
 		most_put_mbo(mbo);
-		channel->mbo_offs = 0;
-		channel->stacked_mbo = NULL;
+		c->mbo_offs = 0;
+		c->stacked_mbo = NULL;
 	}
-	mutex_unlock(&channel->io_mutex);
+	mutex_unlock(&c->io_mutex);
 	return copied;
 }
 
@@ -323,26 +323,26 @@ static const struct file_operations channel_fops = {
  */
 static int aim_disconnect_channel(struct most_interface *iface, int channel_id)
 {
-	struct aim_channel *channel;
+	struct aim_channel *c;
 
 	if (!iface) {
 		pr_info("Bad interface pointer\n");
 		return -EINVAL;
 	}
 
-	channel = get_channel(iface, channel_id);
-	if (!channel)
+	c = get_channel(iface, channel_id);
+	if (!c)
 		return -ENXIO;
 
-	mutex_lock(&channel->io_mutex);
-	channel->dev = NULL;
-	mutex_unlock(&channel->io_mutex);
+	mutex_lock(&c->io_mutex);
+	c->dev = NULL;
+	mutex_unlock(&c->io_mutex);
 
-	if (atomic_read(&channel->access_ref)) {
-		destroy_cdev(channel);
-		kfree(channel);
+	if (atomic_read(&c->access_ref)) {
+		destroy_cdev(c);
+		kfree(c);
 	} else {
-		wake_up_interruptible(&channel->wq);
+		wake_up_interruptible(&c->wq);
 	}
 	return 0;
 }
@@ -356,21 +356,21 @@ static int aim_disconnect_channel(struct most_interface *iface, int channel_id)
  */
 static int aim_rx_completion(struct mbo *mbo)
 {
-	struct aim_channel *channel;
+	struct aim_channel *c;
 
 	if (!mbo)
 		return -EINVAL;
 
-	channel = get_channel(mbo->ifp, mbo->hdm_channel_id);
-	if (!channel)
+	c = get_channel(mbo->ifp, mbo->hdm_channel_id);
+	if (!c)
 		return -ENXIO;
 
-	kfifo_in(&channel->fifo, &mbo, 1);
+	kfifo_in(&c->fifo, &mbo, 1);
 #ifdef DEBUG_MESG
-	if (kfifo_is_full(&channel->fifo))
+	if (kfifo_is_full(&c->fifo))
 		pr_info("WARN: Fifo is full\n");
 #endif
-	wake_up_interruptible(&channel->wq);
+	wake_up_interruptible(&c->wq);
 	return 0;
 }
 
@@ -383,7 +383,7 @@ static int aim_rx_completion(struct mbo *mbo)
  */
 static int aim_tx_completion(struct most_interface *iface, int channel_id)
 {
-	struct aim_channel *channel;
+	struct aim_channel *c;
 
 	if (!iface) {
 		pr_info("Bad interface pointer\n");
@@ -394,10 +394,10 @@ static int aim_tx_completion(struct most_interface *iface, int channel_id)
 		return -EINVAL;
 	}
 
-	channel = get_channel(iface, channel_id);
-	if (!channel)
+	c = get_channel(iface, channel_id);
+	if (!c)
 		return -ENXIO;
-	wake_up_interruptible(&channel->wq);
+	wake_up_interruptible(&c->wq);
 	return 0;
 }
 
@@ -419,7 +419,7 @@ static int aim_probe(struct most_interface *iface, int channel_id,
 		     struct most_channel_config *cfg,
 		     struct kobject *parent, char *name)
 {
-	struct aim_channel *channel;
+	struct aim_channel *c;
 	unsigned long cl_flags;
 	int retval;
 	int current_minor;
@@ -428,60 +428,60 @@ static int aim_probe(struct most_interface *iface, int channel_id,
 		pr_info("Probing AIM with bad arguments");
 		return -EINVAL;
 	}
-	channel = get_channel(iface, channel_id);
-	if (channel)
+	c = get_channel(iface, channel_id);
+	if (c)
 		return -EEXIST;
 
 	current_minor = ida_simple_get(&minor_id, 0, 0, GFP_KERNEL);
 	if (current_minor < 0)
 		return current_minor;
 
-	channel = kzalloc(sizeof(*channel), GFP_KERNEL);
-	if (!channel) {
+	c = kzalloc(sizeof(*c), GFP_KERNEL);
+	if (!c) {
 		retval = -ENOMEM;
 		goto error_alloc_channel;
 	}
 
-	channel->devno = MKDEV(major, current_minor);
-	cdev_init(&channel->cdev, &channel_fops);
-	channel->cdev.owner = THIS_MODULE;
-	cdev_add(&channel->cdev, channel->devno, 1);
-	channel->iface = iface;
-	channel->cfg = cfg;
-	channel->channel_id = channel_id;
-	channel->mbo_offs = 0;
-	atomic_set(&channel->access_ref, -1);
-	INIT_KFIFO(channel->fifo);
-	retval = kfifo_alloc(&channel->fifo, cfg->num_buffers, GFP_KERNEL);
+	c->devno = MKDEV(major, current_minor);
+	cdev_init(&c->cdev, &channel_fops);
+	c->cdev.owner = THIS_MODULE;
+	cdev_add(&c->cdev, c->devno, 1);
+	c->iface = iface;
+	c->cfg = cfg;
+	c->channel_id = channel_id;
+	c->mbo_offs = 0;
+	atomic_set(&c->access_ref, -1);
+	INIT_KFIFO(c->fifo);
+	retval = kfifo_alloc(&c->fifo, cfg->num_buffers, GFP_KERNEL);
 	if (retval) {
 		pr_info("failed to alloc channel kfifo");
 		goto error_alloc_kfifo;
 	}
-	init_waitqueue_head(&channel->wq);
-	mutex_init(&channel->io_mutex);
+	init_waitqueue_head(&c->wq);
+	mutex_init(&c->io_mutex);
 	spin_lock_irqsave(&ch_list_lock, cl_flags);
-	list_add_tail(&channel->list, &channel_list);
+	list_add_tail(&c->list, &channel_list);
 	spin_unlock_irqrestore(&ch_list_lock, cl_flags);
-	channel->dev = device_create(aim_class,
+	c->dev = device_create(aim_class,
 				     NULL,
-				     channel->devno,
+				     c->devno,
 				     NULL,
 				     "%s", name);
 
-	retval = IS_ERR(channel->dev);
+	retval = IS_ERR(c->dev);
 	if (retval) {
 		pr_info("failed to create new device node %s\n", name);
 		goto error_create_device;
 	}
-	kobject_uevent(&channel->dev->kobj, KOBJ_ADD);
+	kobject_uevent(&c->dev->kobj, KOBJ_ADD);
 	return 0;
 
 error_create_device:
-	kfifo_free(&channel->fifo);
-	list_del(&channel->list);
+	kfifo_free(&c->fifo);
+	list_del(&c->list);
 error_alloc_kfifo:
-	cdev_del(&channel->cdev);
-	kfree(channel);
+	cdev_del(&c->cdev);
+	kfree(c);
 error_alloc_channel:
 	ida_simple_remove(&minor_id, current_minor);
 	return retval;
@@ -526,15 +526,15 @@ free_cdev:
 
 static void __exit mod_exit(void)
 {
-	struct aim_channel *channel, *tmp;
+	struct aim_channel *c, *tmp;
 
 	pr_info("exit module\n");
 
 	most_deregister_aim(&cdev_aim);
 
-	list_for_each_entry_safe(channel, tmp, &channel_list, list) {
-		destroy_cdev(channel);
-		kfree(channel);
+	list_for_each_entry_safe(c, tmp, &channel_list, list) {
+		destroy_cdev(c);
+		kfree(c);
 	}
 	class_destroy(aim_class);
 	unregister_chrdev_region(aim_devno, 1);
-- 
1.7.9.5

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

[PATCH 23/28] staging: most: fix race conditions

[Christian Gromm]

This patch fixes race conditions that might emerge from functions
aim_open, aim_close, aim_read, aim_write and aim_disconnect_channel
within module cdev.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-cdev/cdev.c |   89 +++++++++++++++++++++-------------
 1 file changed, 54 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
index 5065139..ade7808 100644
--- a/drivers/staging/most/aim-cdev/cdev.c
+++ b/drivers/staging/most/aim-cdev/cdev.c
@@ -32,6 +32,7 @@ static struct most_aim cdev_aim;
 
 struct aim_channel {
 	wait_queue_head_t wq;
+	spinlock_t unlink;	/* synchronization lock to unlink channels */
 	struct cdev cdev;
 	struct device *dev;
 	struct mutex io_mutex;
@@ -55,6 +56,12 @@ static inline bool ch_has_mbo(struct aim_channel *c)
 	return channel_has_mbo(c->iface, c->channel_id, &cdev_aim) > 0;
 }
 
+static inline bool ch_get_mbo(struct aim_channel *c, struct mbo **mbo)
+{
+	*mbo = most_get_mbo(c->iface, c->channel_id, &cdev_aim);
+	return *mbo;
+}
+
 static struct aim_channel *get_channel(struct most_interface *iface, int id)
 {
 	struct aim_channel *c, *tmp;
@@ -82,6 +89,7 @@ static void stop_channel(struct aim_channel *c)
 		most_put_mbo(mbo);
 	if (c->stacked_mbo)
 		most_put_mbo(c->stacked_mbo);
+	c->stacked_mbo = NULL;
 	most_stop_channel(c->iface, c->channel_id, &cdev_aim);
 }
 
@@ -121,16 +129,25 @@ static int aim_open(struct inode *inode, struct file *filp)
 		pr_info("WARN: Access flags mismatch\n");
 		return -EACCES;
 	}
+
+	mutex_lock(&c->io_mutex);
+	if (!c->dev) {
+		pr_info("WARN: Device is destroyed\n");
+		mutex_unlock(&c->io_mutex);
+		return -EBUSY;
+	}
+
 	if (!atomic_inc_and_test(&c->access_ref)) {
 		pr_info("WARN: Device is busy\n");
 		atomic_dec(&c->access_ref);
+		mutex_unlock(&c->io_mutex);
 		return -EBUSY;
 	}
 
-	ret = most_start_channel(c->iface, c->channel_id,
-				 &cdev_aim);
+	ret = most_start_channel(c->iface, c->channel_id, &cdev_aim);
 	if (ret)
 		atomic_dec(&c->access_ref);
+	mutex_unlock(&c->io_mutex);
 	return ret;
 }
 
@@ -146,17 +163,17 @@ static int aim_close(struct inode *inode, struct file *filp)
 	struct aim_channel *c = to_channel(inode->i_cdev);
 
 	mutex_lock(&c->io_mutex);
-	if (!c->dev) {
+	spin_lock(&c->unlink);
+	atomic_dec(&c->access_ref);
+	spin_unlock(&c->unlink);
+	if (c->dev) {
+		stop_channel(c);
 		mutex_unlock(&c->io_mutex);
-		atomic_dec(&c->access_ref);
+	} else {
 		destroy_cdev(c);
+		mutex_unlock(&c->io_mutex);
 		kfree(c);
-		return 0;
 	}
-	mutex_unlock(&c->io_mutex);
-
-	stop_channel(c);
-	atomic_dec(&c->access_ref);
 	return 0;
 }
 
@@ -171,40 +188,27 @@ static ssize_t aim_write(struct file *filp, const char __user *buf,
 			 size_t count, loff_t *offset)
 {
 	int ret, err;
-	size_t actual_len = 0;
-	size_t max_len = 0;
+	size_t actual_len;
+	size_t max_len;
 	ssize_t retval;
-	struct mbo *mbo;
+	struct mbo *mbo = NULL;
 	struct aim_channel *c = filp->private_data;
 
 	mutex_lock(&c->io_mutex);
-	if (unlikely(!c->dev)) {
+	while (c->dev && !ch_get_mbo(c, &mbo)) {
 		mutex_unlock(&c->io_mutex);
-		return -EPIPE;
-	}
-	mutex_unlock(&c->io_mutex);
-
-	mbo = most_get_mbo(c->iface, c->channel_id, &cdev_aim);
 
-	if (!mbo) {
 		if ((filp->f_flags & O_NONBLOCK))
 			return -EAGAIN;
-		if (wait_event_interruptible(
-			    c->wq,
-			    (mbo = most_get_mbo(c->iface,
-						c->channel_id,
-						&cdev_aim)) ||
-			    (!c->dev)))
+		if (wait_event_interruptible(c->wq, ch_has_mbo(c) || !c->dev))
 			return -ERESTARTSYS;
+		mutex_lock(&c->io_mutex);
 	}
 
-	mutex_lock(&c->io_mutex);
 	if (unlikely(!c->dev)) {
-		mutex_unlock(&c->io_mutex);
 		err = -EPIPE;
 		goto error;
 	}
-	mutex_unlock(&c->io_mutex);
 
 	max_len = c->cfg->buffer_size;
 	actual_len = min(count, max_len);
@@ -222,9 +226,12 @@ static ssize_t aim_write(struct file *filp, const char __user *buf,
 		err = ret;
 		goto error;
 	}
+	mutex_unlock(&c->io_mutex);
 	return actual_len - retval;
 error:
-	most_put_mbo(mbo);
+	if (mbo)
+		most_put_mbo(mbo);
+	mutex_unlock(&c->io_mutex);
 	return err;
 }
 
@@ -242,23 +249,25 @@ aim_read(struct file *filp, char __user *buf, size_t count, loff_t *offset)
 	struct mbo *mbo;
 	struct aim_channel *c = filp->private_data;
 
+	mutex_lock(&c->io_mutex);
 	if (c->stacked_mbo) {
 		mbo = c->stacked_mbo;
 		goto start_copy;
 	}
 	while ((!kfifo_out(&c->fifo, &mbo, 1)) && (c->dev)) {
+		mutex_unlock(&c->io_mutex);
 		if (filp->f_flags & O_NONBLOCK)
 			return -EAGAIN;
 		if (wait_event_interruptible(c->wq,
 					     (!kfifo_is_empty(&c->fifo) ||
 					      (!c->dev))))
 			return -ERESTARTSYS;
+		mutex_lock(&c->io_mutex);
 	}
 	c->stacked_mbo = mbo;
 
 start_copy:
 	/* make sure we don't submit to gone devices */
-	mutex_lock(&c->io_mutex);
 	if (unlikely(!c->dev)) {
 		mutex_unlock(&c->io_mutex);
 		return -EIO;
@@ -335,14 +344,17 @@ static int aim_disconnect_channel(struct most_interface *iface, int channel_id)
 		return -ENXIO;
 
 	mutex_lock(&c->io_mutex);
+	spin_lock(&c->unlink);
 	c->dev = NULL;
-	mutex_unlock(&c->io_mutex);
-
-	if (atomic_read(&c->access_ref)) {
+	spin_unlock(&c->unlink);
+	if (!atomic_read(&c->access_ref)) {
+		stop_channel(c);
+		wake_up_interruptible(&c->wq);
+		mutex_unlock(&c->io_mutex);
+	} else {
 		destroy_cdev(c);
+		mutex_unlock(&c->io_mutex);
 		kfree(c);
-	} else {
-		wake_up_interruptible(&c->wq);
 	}
 	return 0;
 }
@@ -365,7 +377,13 @@ static int aim_rx_completion(struct mbo *mbo)
 	if (!c)
 		return -ENXIO;
 
+	spin_lock(&c->unlink);
+	if (atomic_read(&c->access_ref) || !c->dev) {
+		spin_unlock(&c->unlink);
+		return -EFAULT;
+	}
 	kfifo_in(&c->fifo, &mbo, 1);
+	spin_unlock(&c->unlink);
 #ifdef DEBUG_MESG
 	if (kfifo_is_full(&c->fifo))
 		pr_info("WARN: Fifo is full\n");
@@ -451,6 +469,7 @@ static int aim_probe(struct most_interface *iface, int channel_id,
 	c->channel_id = channel_id;
 	c->mbo_offs = 0;
 	atomic_set(&c->access_ref, -1);
+	spin_lock_init(&c->unlink);
 	INIT_KFIFO(c->fifo);
 	retval = kfifo_alloc(&c->fifo, cfg->num_buffers, GFP_KERNEL);
 	if (retval) {
-- 
1.7.9.5

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

[PATCH 24/28] staging: most: change type of access_ref

[Christian Gromm]

This patch changes the type of the access reference from atomit_t to int.
It is needed, because the reference variable is secured by synchronization
locks and does not need to be atomic anymore.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-cdev/cdev.c |   17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
index ade7808..d9c3f56 100644
--- a/drivers/staging/most/aim-cdev/cdev.c
+++ b/drivers/staging/most/aim-cdev/cdev.c
@@ -43,7 +43,7 @@ struct aim_channel {
 	size_t mbo_offs;
 	struct mbo *stacked_mbo;
 	DECLARE_KFIFO_PTR(fifo, typeof(struct mbo *));
-	atomic_t access_ref;
+	int access_ref;
 	struct list_head list;
 };
 
@@ -137,16 +137,15 @@ static int aim_open(struct inode *inode, struct file *filp)
 		return -EBUSY;
 	}
 
-	if (!atomic_inc_and_test(&c->access_ref)) {
+	if (c->access_ref) {
 		pr_info("WARN: Device is busy\n");
-		atomic_dec(&c->access_ref);
 		mutex_unlock(&c->io_mutex);
 		return -EBUSY;
 	}
 
 	ret = most_start_channel(c->iface, c->channel_id, &cdev_aim);
-	if (ret)
-		atomic_dec(&c->access_ref);
+	if (!ret)
+		c->access_ref = 1;
 	mutex_unlock(&c->io_mutex);
 	return ret;
 }
@@ -164,7 +163,7 @@ static int aim_close(struct inode *inode, struct file *filp)
 
 	mutex_lock(&c->io_mutex);
 	spin_lock(&c->unlink);
-	atomic_dec(&c->access_ref);
+	c->access_ref = 0;
 	spin_unlock(&c->unlink);
 	if (c->dev) {
 		stop_channel(c);
@@ -347,7 +346,7 @@ static int aim_disconnect_channel(struct most_interface *iface, int channel_id)
 	spin_lock(&c->unlink);
 	c->dev = NULL;
 	spin_unlock(&c->unlink);
-	if (!atomic_read(&c->access_ref)) {
+	if (c->access_ref) {
 		stop_channel(c);
 		wake_up_interruptible(&c->wq);
 		mutex_unlock(&c->io_mutex);
@@ -378,7 +377,7 @@ static int aim_rx_completion(struct mbo *mbo)
 		return -ENXIO;
 
 	spin_lock(&c->unlink);
-	if (atomic_read(&c->access_ref) || !c->dev) {
+	if (!c->access_ref || !c->dev) {
 		spin_unlock(&c->unlink);
 		return -EFAULT;
 	}
@@ -468,7 +467,7 @@ static int aim_probe(struct most_interface *iface, int channel_id,
 	c->cfg = cfg;
 	c->channel_id = channel_id;
 	c->mbo_offs = 0;
-	atomic_set(&c->access_ref, -1);
+	c->access_ref = 0;
 	spin_lock_init(&c->unlink);
 	INIT_KFIFO(c->fifo);
 	retval = kfifo_alloc(&c->fifo, cfg->num_buffers, GFP_KERNEL);
-- 
1.7.9.5

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

[PATCH 25/28] staging: most: remove stacked_mbo

[Christian Gromm]

This patch makes use of kfifo_peek and kfifo_skip, which renders the
variable stacked_mbo useless. It is therefore removed.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-cdev/cdev.c |   16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
index d9c3f56..0ee2f08 100644
--- a/drivers/staging/most/aim-cdev/cdev.c
+++ b/drivers/staging/most/aim-cdev/cdev.c
@@ -41,7 +41,6 @@ struct aim_channel {
 	unsigned int channel_id;
 	dev_t devno;
 	size_t mbo_offs;
-	struct mbo *stacked_mbo;
 	DECLARE_KFIFO_PTR(fifo, typeof(struct mbo *));
 	int access_ref;
 	struct list_head list;
@@ -87,9 +86,6 @@ static void stop_channel(struct aim_channel *c)
 
 	while (kfifo_out((struct kfifo *)&c->fifo, &mbo, 1))
 		most_put_mbo(mbo);
-	if (c->stacked_mbo)
-		most_put_mbo(c->stacked_mbo);
-	c->stacked_mbo = NULL;
 	most_stop_channel(c->iface, c->channel_id, &cdev_aim);
 }
 
@@ -143,6 +139,7 @@ static int aim_open(struct inode *inode, struct file *filp)
 		return -EBUSY;
 	}
 
+	c->mbo_offs = 0;
 	ret = most_start_channel(c->iface, c->channel_id, &cdev_aim);
 	if (!ret)
 		c->access_ref = 1;
@@ -249,11 +246,7 @@ aim_read(struct file *filp, char __user *buf, size_t count, loff_t *offset)
 	struct aim_channel *c = filp->private_data;
 
 	mutex_lock(&c->io_mutex);
-	if (c->stacked_mbo) {
-		mbo = c->stacked_mbo;
-		goto start_copy;
-	}
-	while ((!kfifo_out(&c->fifo, &mbo, 1)) && (c->dev)) {
+	while (c->dev && !kfifo_peek(&c->fifo, &mbo)) {
 		mutex_unlock(&c->io_mutex);
 		if (filp->f_flags & O_NONBLOCK)
 			return -EAGAIN;
@@ -263,9 +256,7 @@ aim_read(struct file *filp, char __user *buf, size_t count, loff_t *offset)
 			return -ERESTARTSYS;
 		mutex_lock(&c->io_mutex);
 	}
-	c->stacked_mbo = mbo;
 
-start_copy:
 	/* make sure we don't submit to gone devices */
 	if (unlikely(!c->dev)) {
 		mutex_unlock(&c->io_mutex);
@@ -284,9 +275,9 @@ start_copy:
 
 	c->mbo_offs += copied;
 	if (c->mbo_offs >= mbo->processed_length) {
+		kfifo_skip(&c->fifo);
 		most_put_mbo(mbo);
 		c->mbo_offs = 0;
-		c->stacked_mbo = NULL;
 	}
 	mutex_unlock(&c->io_mutex);
 	return copied;
@@ -466,7 +457,6 @@ static int aim_probe(struct most_interface *iface, int channel_id,
 	c->iface = iface;
 	c->cfg = cfg;
 	c->channel_id = channel_id;
-	c->mbo_offs = 0;
 	c->access_ref = 0;
 	spin_lock_init(&c->unlink);
 	INIT_KFIFO(c->fifo);
-- 
1.7.9.5

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

[PATCH 26/28] staging: most: rearrange function aim_write

[Christian Gromm]

This patch rearranges the code of function aim_write() of module aim-cdev.
It is needed to remove the error lable and make the code straighter.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-cdev/cdev.c |   22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
index 0ee2f08..e5ceb82 100644
--- a/drivers/staging/most/aim-cdev/cdev.c
+++ b/drivers/staging/most/aim-cdev/cdev.c
@@ -183,7 +183,6 @@ static int aim_close(struct inode *inode, struct file *filp)
 static ssize_t aim_write(struct file *filp, const char __user *buf,
 			 size_t count, loff_t *offset)
 {
-	int ret, err;
 	size_t actual_len;
 	size_t max_len;
 	ssize_t retval;
@@ -202,8 +201,8 @@ static ssize_t aim_write(struct file *filp, const char __user *buf,
 	}
 
 	if (unlikely(!c->dev)) {
-		err = -EPIPE;
-		goto error;
+		mutex_unlock(&c->io_mutex);
+		return -EPIPE;
 	}
 
 	max_len = c->cfg->buffer_size;
@@ -212,23 +211,14 @@ static ssize_t aim_write(struct file *filp, const char __user *buf,
 
 	retval = copy_from_user(mbo->virt_address, buf, mbo->buffer_length);
 	if (retval) {
-		err = -EIO;
-		goto error;
+		most_put_mbo(mbo);
+		mutex_unlock(&c->io_mutex);
+		return -EIO;
 	}
 
-	ret = most_submit_mbo(mbo);
-	if (ret) {
-		pr_info("submitting MBO to core failed\n");
-		err = ret;
-		goto error;
-	}
+	most_submit_mbo(mbo);
 	mutex_unlock(&c->io_mutex);
 	return actual_len - retval;
-error:
-	if (mbo)
-		most_put_mbo(mbo);
-	mutex_unlock(&c->io_mutex);
-	return err;
 }
 
 /**
-- 
1.7.9.5

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

Re: [PATCH 26/28] staging: most: rearrange function aim_write

[Dan Carpenter]

On Wed, Nov 18, 2015 at 01:43:50PM +0100, Christian Gromm wrote:
> This patch rearranges the code of function aim_write() of module aim-cdev.
> It is needed to remove the error lable and make the code straighter.
> 

I like error labels.

> Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> ---
>  drivers/staging/most/aim-cdev/cdev.c |   22 ++++++----------------
>  1 file changed, 6 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
> index 0ee2f08..e5ceb82 100644
> --- a/drivers/staging/most/aim-cdev/cdev.c
> +++ b/drivers/staging/most/aim-cdev/cdev.c
> @@ -183,7 +183,6 @@ static int aim_close(struct inode *inode, struct file *filp)
>  static ssize_t aim_write(struct file *filp, const char __user *buf,
>  			 size_t count, loff_t *offset)
>  {
> -	int ret, err;

Keep ret.  Get rid of err.

>  	size_t actual_len;
>  	size_t max_len;
>  	ssize_t retval;

Get rid of retval.

The first part of this function uses direct returns and that's fine.


> @@ -202,8 +201,8 @@ static ssize_t aim_write(struct file *filp, const char __user *buf,
>  	}
>  
>  	if (unlikely(!c->dev)) {
> -		err = -EPIPE;
> -		goto error;
> +		mutex_unlock(&c->io_mutex);
> +		return -EPIPE;

	if (!c->dev) {
		ret = -EPIPE;
		goto unlock;
	}


>  	}
>  
>  	max_len = c->cfg->buffer_size;
> @@ -212,23 +211,14 @@ static ssize_t aim_write(struct file *filp, const char __user *buf,
>  
>  	retval = copy_from_user(mbo->virt_address, buf, mbo->buffer_length);
>  	if (retval) {
> -		err = -EIO;
> -		goto error;
> +		most_put_mbo(mbo);
> +		mutex_unlock(&c->io_mutex);
> +		return -EIO;
>  	}

The error code should be -EFAULT instead of -EIO.  It's better to not
assign the "retval = " because we don't care about the actual value, we
only care if it's non-zero.  Also it's a few characters less to write it
the normal way.

	if (copy_from_user(mbo->virt_address, buf, mbo->buffer_length) {
		ret = -EFAULT;
		goto put_mbo;
	}

>  
> -	ret = most_submit_mbo(mbo);
> -	if (ret) {
> -		pr_info("submitting MBO to core failed\n");
> -		err = ret;
> -		goto error;
> -	}
> +	most_submit_mbo(mbo);

Why remove the error handling?  Anyway,

	ret = most_submit_mbo(mbo);
	if (ret)
		goto put_mbo;

>  	mutex_unlock(&c->io_mutex);
>  	return actual_len - retval;

retval is always zero here.

> -error:
> -	if (mbo)
> -		most_put_mbo(mbo);
> -	mutex_unlock(&c->io_mutex);
> -	return err;

	mutex_unlock(&c->io_mutex);

	return actual_len;

put_mbo:
	most_put_mbo(mbo);
unlock:
	mutex_unlock(&c->io_mutex);

	return ret;


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

[PATCH 27/28] staging: most: add statistics for dropped packets

[Christian Gromm]

This patch adds a counter for dropped packets. It needed for statistical
analysis.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-network/networking.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/most/aim-network/networking.c b/drivers/staging/most/aim-network/networking.c
index 3c7beb0..a1fc8b3 100644
--- a/drivers/staging/most/aim-network/networking.c
+++ b/drivers/staging/most/aim-network/networking.c
@@ -431,6 +431,7 @@ static int aim_rx_data(struct mbo *mbo)
 	u32 len = mbo->processed_length;
 	struct sk_buff *skb;
 	struct net_device *dev;
+	typeof(skb->len) skb_len;
 
 	nd = get_net_dev_context(mbo->ifp);
 	if (!nd || !nd->channels_opened || nd->rx.ch_id != mbo->hdm_channel_id)
@@ -482,9 +483,13 @@ static int aim_rx_data(struct mbo *mbo)
 
 	memcpy(skb_put(skb, len), buf, len);
 	skb->protocol = eth_type_trans(skb, dev);
-	dev->stats.rx_packets++;
-	dev->stats.rx_bytes += skb->len;
-	netif_rx(skb);
+	skb_len = skb->len;
+	if (netif_rx(skb) == NET_RX_SUCCESS) {
+		dev->stats.rx_packets++;
+		dev->stats.rx_bytes += skb_len;
+	} else {
+		dev->stats.rx_dropped++;
+	}
 
 out:
 	most_put_mbo(mbo);
-- 
1.7.9.5

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

Re: [PATCH 27/28] staging: most: add statistics for dropped packets

[Dan Carpenter]

On Wed, Nov 18, 2015 at 01:43:51PM +0100, Christian Gromm wrote:
> This patch adds a counter for dropped packets. It needed for statistical
> analysis.
> 
> Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> ---
>  drivers/staging/most/aim-network/networking.c |   11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/most/aim-network/networking.c b/drivers/staging/most/aim-network/networking.c
> index 3c7beb0..a1fc8b3 100644
> --- a/drivers/staging/most/aim-network/networking.c
> +++ b/drivers/staging/most/aim-network/networking.c
> @@ -431,6 +431,7 @@ static int aim_rx_data(struct mbo *mbo)
>  	u32 len = mbo->processed_length;
>  	struct sk_buff *skb;
>  	struct net_device *dev;
> +	typeof(skb->len) skb_len;

Heh.  Use unsigned int.

regards,
dan carpenter

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

[PATCH 28/28] staging: most: remove 2nd forward declaration of struct most_aim

[Christian Gromm]

This patch removes the second forwared declaration of struct most_aim.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/aim-cdev/cdev.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
index e5ceb82..e9943c3 100644
--- a/drivers/staging/most/aim-cdev/cdev.c
+++ b/drivers/staging/most/aim-cdev/cdev.c
@@ -399,8 +399,6 @@ static int aim_tx_completion(struct most_interface *iface, int channel_id)
 	return 0;
 }
 
-static struct most_aim cdev_aim;
-
 /**
  * aim_probe - probe function of the driver module
  * @iface: pointer to interface instance
-- 
1.7.9.5

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