[PATCH 1/5] staging: most: remove always true comparison

[PATCH 1/5] staging: most: remove always true comparison

[Sudip Mukherjee]

channel->dev has already been checked for NULL and if it was NULL then
we have returned with -EPIPE. So at this point it can not be NULL.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 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 b0a9a4a..1a17e2a 100644
--- a/drivers/staging/most/aim-cdev/cdev.c
+++ b/drivers/staging/most/aim-cdev/cdev.c
@@ -166,7 +166,7 @@ static ssize_t aim_write(struct file *filp, const char __user *buf,
 
 	mbo = most_get_mbo(channel->iface, channel->channel_id);
 
-	if (!mbo && channel->dev) {
+	if (!mbo) {
 		if ((filp->f_flags & O_NONBLOCK))
 			return -EAGAIN;
 		if (wait_event_interruptible(
-- 
1.9.1

[PATCH 2/5] staging: most: return NULL instead of integer

[Sudip Mukherjee]

The return type of get_aim_dev() is a pointer but we were returning 0
incase of failure.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/most/aim-v4l2/video.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/most/aim-v4l2/video.c b/drivers/staging/most/aim-v4l2/video.c
index d968791..8333245 100644
--- a/drivers/staging/most/aim-v4l2/video.c
+++ b/drivers/staging/most/aim-v4l2/video.c
@@ -430,7 +430,7 @@ static struct most_video_dev *get_aim_dev(
 		}
 	}
 	spin_unlock(&list_lock);
-	return 0;
+	return NULL;
 }
 
 static int aim_rx_data(struct mbo *mbo)
-- 
1.9.1

[PATCH 3/5] staging: most: remove driver owner

[Sudip Mukherjee]

The platform driver core will set the owner value, we do not need to do
it in the module.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/most/hdm-dim2/dim2_hdm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/most/hdm-dim2/dim2_hdm.c b/drivers/staging/most/hdm-dim2/dim2_hdm.c
index 5b0a588..4481a0b 100644
--- a/drivers/staging/most/hdm-dim2/dim2_hdm.c
+++ b/drivers/staging/most/hdm-dim2/dim2_hdm.c
@@ -923,7 +923,6 @@ static struct platform_driver dim2_driver = {
 	.id_table = dim2_id,
 	.driver = {
 		.name = "hdm_dim2",
-		.owner = THIS_MODULE,
 	},
 };
 
-- 
1.9.1

[PATCH 4/5] staging: most: remove unneeded NULL check

[Sudip Mukherjee]

The loop cursor of list_for_each_entry_safe() can never be NULL.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/most/mostcore/core.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/most/mostcore/core.c b/drivers/staging/most/mostcore/core.c
index 7bb16db..87f950f 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -190,8 +190,7 @@ static void flush_channel_fifos(struct most_c_obj *c)
 	list_for_each_entry_safe(mbo, tmp, &c->fifo, list) {
 		list_del(&mbo->list);
 		spin_unlock_irqrestore(&c->fifo_lock, flags);
-		if (likely(mbo))
-			most_free_mbo_coherent(mbo);
+		most_free_mbo_coherent(mbo);
 		spin_lock_irqsave(&c->fifo_lock, flags);
 	}
 	spin_unlock_irqrestore(&c->fifo_lock, flags);
@@ -200,8 +199,7 @@ static void flush_channel_fifos(struct most_c_obj *c)
 	list_for_each_entry_safe(mbo, tmp, &c->halt_fifo, list) {
 		list_del(&mbo->list);
 		spin_unlock_irqrestore(&c->fifo_lock, hf_flags);
-		if (likely(mbo))
-			most_free_mbo_coherent(mbo);
+		most_free_mbo_coherent(mbo);
 		spin_lock_irqsave(&c->fifo_lock, hf_flags);
 	}
 	spin_unlock_irqrestore(&c->fifo_lock, hf_flags);
-- 
1.9.1

[PATCH 5/5] staging: most: style of bool comparison

[Sudip Mukherjee]

BOOLEAN tests do not need any comparison to TRUE or FALSE.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/most/hdm-usb/hdm_usb.c | 10 +++++-----
 drivers/staging/most/mostcore/core.c   |  4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/most/hdm-usb/hdm_usb.c b/drivers/staging/most/hdm-usb/hdm_usb.c
index 34843b0..9bbefaa 100644
--- a/drivers/staging/most/hdm-usb/hdm_usb.c
+++ b/drivers/staging/most/hdm-usb/hdm_usb.c
@@ -308,7 +308,7 @@ static int hdm_poison_channel(struct most_interface *iface, int channel)
 
 	mutex_lock(&mdev->io_mutex);
 	free_anchored_buffers(mdev, channel);
-	if (mdev->padding_active[channel] == true)
+	if (mdev->padding_active[channel])
 		mdev->padding_active[channel] = false;
 
 	if (mdev->conf[channel].data_type == MOST_CH_ASYNC) {
@@ -411,7 +411,7 @@ static void hdm_write_completion(struct urb *urb)
 	dev = &mdev->usb_device->dev;
 
 	if ((urb->status == -ENOENT) || (urb->status == -ECONNRESET) ||
-	    (mdev->is_channel_healthy[channel] == false)) {
+	    (!mdev->is_channel_healthy[channel])) {
 		complete(&anchor->urb_compl);
 		return;
 	}
@@ -576,7 +576,7 @@ static void hdm_read_completion(struct urb *urb)
 	dev = &mdev->usb_device->dev;
 
 	if ((urb->status == -ENOENT) || (urb->status == -ECONNRESET) ||
-	    (mdev->is_channel_healthy[channel] == false)) {
+	    (!mdev->is_channel_healthy[channel])) {
 		complete(&anchor->urb_compl);
 		return;
 	}
@@ -605,7 +605,7 @@ static void hdm_read_completion(struct urb *urb)
 		}
 	} else {
 		mbo->processed_length = urb->actual_length;
-		if (mdev->padding_active[channel] == false) {
+		if (!mdev->padding_active[channel]) {
 			mbo->status = MBO_SUCCESS;
 		} else {
 			if (hdm_remove_padding(mdev, channel, mbo)) {
@@ -685,7 +685,7 @@ static int hdm_enqueue(struct most_interface *iface, int channel, struct mbo *mb
 	list_add_tail(&anchor->list, &mdev->anchor_list[channel]);
 	spin_unlock_irqrestore(&mdev->anchor_list_lock[channel], flags);
 
-	if ((mdev->padding_active[channel] == true) &&
+	if ((mdev->padding_active[channel]) &&
 	    (conf->direction & MOST_CH_TX))
 		if (hdm_add_padding(mdev, channel, mbo)) {
 			retval = -EIO;
diff --git a/drivers/staging/most/mostcore/core.c b/drivers/staging/most/mostcore/core.c
index 87f950f..eb4e159 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -1339,7 +1339,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");
-	if (unlikely((c->is_poisoned == true) || (mbo->status == MBO_E_CLOSE)))
+	if (unlikely(c->is_poisoned || (mbo->status == MBO_E_CLOSE)))
 		trash_mbo(mbo);
 	else
 		arm_mbo(mbo);
@@ -1444,7 +1444,7 @@ static void most_read_completion(struct mbo *mbo)
 	struct most_c_obj *c;
 
 	c = mbo->context;
-	if (unlikely((c->is_poisoned == true) || (mbo->status == MBO_E_CLOSE)))
+	if (unlikely(c->is_poisoned || (mbo->status == MBO_E_CLOSE)))
 		goto release_mbo;
 
 	if (mbo->status == MBO_E_INVAL) {
-- 
1.9.1

Re: [PATCH 3/5] staging: most: remove driver owner

[Andrey Shvetsov]

On Fri, Sep 04, 2015 at 04:22:04PM +0530, Sudip Mukherjee wrote:
> The platform driver core will set the owner value, we do not need to do
> it in the module.
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
>  drivers/staging/most/hdm-dim2/dim2_hdm.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/staging/most/hdm-dim2/dim2_hdm.c b/drivers/staging/most/hdm-dim2/dim2_hdm.c
> index 5b0a588..4481a0b 100644
> --- a/drivers/staging/most/hdm-dim2/dim2_hdm.c
> +++ b/drivers/staging/most/hdm-dim2/dim2_hdm.c
> @@ -923,7 +923,6 @@ static struct platform_driver dim2_driver = {
>  	.id_table = dim2_id,
>  	.driver = {
>  		.name = "hdm_dim2",
> -		.owner = THIS_MODULE,
I cannot accept this.

This change is not significant for current kernel, but we still have customers
using kernels <= 3.10, where auto assignment of .owner does not exist.

>  	},
>  };
>  
> -- 
> 1.9.1
> 
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 3/5] staging: most: remove driver owner

[Sudip Mukherjee]

On Mon, Sep 07, 2015 at 02:13:38PM +0200, Andrey Shvetsov wrote:
> On Fri, Sep 04, 2015 at 04:22:04PM +0530, Sudip Mukherjee wrote:
> > The platform driver core will set the owner value, we do not need to do
> > it in the module.
> > 
> > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > ---
> >  drivers/staging/most/hdm-dim2/dim2_hdm.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/staging/most/hdm-dim2/dim2_hdm.c b/drivers/staging/most/hdm-dim2/dim2_hdm.c
> > index 5b0a588..4481a0b 100644
> > --- a/drivers/staging/most/hdm-dim2/dim2_hdm.c
> > +++ b/drivers/staging/most/hdm-dim2/dim2_hdm.c
> > @@ -923,7 +923,6 @@ static struct platform_driver dim2_driver = {
> >  	.id_table = dim2_id,
> >  	.driver = {
> >  		.name = "hdm_dim2",
> > -		.owner = THIS_MODULE,
> I cannot accept this.
> 
> This change is not significant for current kernel, but we still have customers
> using kernels <= 3.10, where auto assignment of .owner does not exist.
But this patch is for 4.4-rc1, and it is not marked for stable. Then how
is it going to affect your customers who are still using <=3.10?

regards
sudip

Re: [PATCH 3/5] staging: most: remove driver owner

[Andrey Shvetsov]

On Mon, Sep 07, 2015 at 06:16:21PM +0530, Sudip Mukherjee wrote:
> On Mon, Sep 07, 2015 at 02:13:38PM +0200, Andrey Shvetsov wrote:
> > On Fri, Sep 04, 2015 at 04:22:04PM +0530, Sudip Mukherjee wrote:
> > > The platform driver core will set the owner value, we do not need to do
> > > it in the module.
> > > 
> > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > > ---
> > >  drivers/staging/most/hdm-dim2/dim2_hdm.c | 1 -
> > >  1 file changed, 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/most/hdm-dim2/dim2_hdm.c b/drivers/staging/most/hdm-dim2/dim2_hdm.c
> > > index 5b0a588..4481a0b 100644
> > > --- a/drivers/staging/most/hdm-dim2/dim2_hdm.c
> > > +++ b/drivers/staging/most/hdm-dim2/dim2_hdm.c
> > > @@ -923,7 +923,6 @@ static struct platform_driver dim2_driver = {
> > >  	.id_table = dim2_id,
> > >  	.driver = {
> > >  		.name = "hdm_dim2",
> > > -		.owner = THIS_MODULE,
> > I cannot accept this.
> > 
> > This change is not significant for current kernel, but we still have customers
> > using kernels <= 3.10, where auto assignment of .owner does not exist.
> But this patch is for 4.4-rc1, and it is not marked for stable. Then how
> is it going to affect your customers who are still using <=3.10?
The customers will get MOST Linux Driver of 4.x and compile it with 3.10.
They'll not even see the warning, but missing owner is definitely not what they
want to have.  This patch prepares wasted time of cusomers expiriencing the
issue and ours supporting resolving of some problem.  But what we get instead?

-- 
regards
andy

Re: [PATCH 3/5] staging: most: remove driver owner

[Greg Kroah-Hartman]

On Tue, Sep 08, 2015 at 11:56:29AM +0200, Andrey Shvetsov wrote:
> On Mon, Sep 07, 2015 at 06:16:21PM +0530, Sudip Mukherjee wrote:
> > On Mon, Sep 07, 2015 at 02:13:38PM +0200, Andrey Shvetsov wrote:
> > > On Fri, Sep 04, 2015 at 04:22:04PM +0530, Sudip Mukherjee wrote:
> > > > The platform driver core will set the owner value, we do not need to do
> > > > it in the module.
> > > > 
> > > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > > > ---
> > > >  drivers/staging/most/hdm-dim2/dim2_hdm.c | 1 -
> > > >  1 file changed, 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/staging/most/hdm-dim2/dim2_hdm.c b/drivers/staging/most/hdm-dim2/dim2_hdm.c
> > > > index 5b0a588..4481a0b 100644
> > > > --- a/drivers/staging/most/hdm-dim2/dim2_hdm.c
> > > > +++ b/drivers/staging/most/hdm-dim2/dim2_hdm.c
> > > > @@ -923,7 +923,6 @@ static struct platform_driver dim2_driver = {
> > > >  	.id_table = dim2_id,
> > > >  	.driver = {
> > > >  		.name = "hdm_dim2",
> > > > -		.owner = THIS_MODULE,
> > > I cannot accept this.
> > > 
> > > This change is not significant for current kernel, but we still have customers
> > > using kernels <= 3.10, where auto assignment of .owner does not exist.
> > But this patch is for 4.4-rc1, and it is not marked for stable. Then how
> > is it going to affect your customers who are still using <=3.10?
> The customers will get MOST Linux Driver of 4.x and compile it with 3.10.

Really?  If they do that they are on their own and odds are, it will
break.

> They'll not even see the warning, but missing owner is definitely not what they
> want to have.  This patch prepares wasted time of cusomers expiriencing the
> issue and ours supporting resolving of some problem.  But what we get instead?

We do not support people taking a kernel driver from one kernel and
putting it into another one.  That's never been something that anyone
can ever do with Linux, sorry.

The patch is correct.

greg k-h