[PATCH 1/9] staging: most: NULL comparison style

[PATCH 1/9] staging: most: NULL comparison style

[Sudip Mukherjee]

According to the kernel coding style the NULL check should not be
written as [variable] == NULL or [variable] != NULL.

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

diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
index 0a13d8d..7b637f3 100644
--- a/drivers/staging/most/aim-cdev/cdev.c
+++ b/drivers/staging/most/aim-cdev/cdev.c
@@ -174,7 +174,7 @@ static ssize_t aim_write(struct file *filp, const char __user *buf,
 			    channel->wq,
 			    (mbo = most_get_mbo(channel->iface,
 						channel->channel_id)) ||
-			    (channel->dev == NULL)))
+			    (!channel->dev)))
 			return -ERESTARTSYS;
 	}
 
@@ -230,12 +230,12 @@ aim_read(struct file *filp, char __user *buf, size_t count, loff_t *offset)
 		goto start_copy;
 	}
 	while ((0 == kfifo_out(&channel->fifo, &mbo, 1))
-	       && (channel->dev != NULL)) {
+	       && (channel->dev)) {
 		if (filp->f_flags & O_NONBLOCK)
 			return -EAGAIN;
 		if (wait_event_interruptible(channel->wq,
 					     (!kfifo_is_empty(&channel->fifo) ||
-					      (channel->dev == NULL))))
+					      (!channel->dev))))
 			return -ERESTARTSYS;
 	}
 
@@ -300,7 +300,7 @@ static int aim_disconnect_channel(struct most_interface *iface, int channel_id)
 	}
 
 	channel = get_channel(iface, channel_id);
-	if (channel == NULL)
+	if (!channel)
 		return -ENXIO;
 
 	mutex_lock(&channel->io_mutex);
@@ -337,7 +337,7 @@ static int aim_rx_completion(struct mbo *mbo)
 		return -EINVAL;
 
 	channel = get_channel(mbo->ifp, mbo->hdm_channel_id);
-	if (channel == NULL)
+	if (!channel)
 		return -ENXIO;
 
 	kfifo_in(&channel->fifo, &mbo, 1);
@@ -370,7 +370,7 @@ static int aim_tx_completion(struct most_interface *iface, int channel_id)
 	}
 
 	channel = get_channel(iface, channel_id);
-	if (channel == NULL)
+	if (!channel)
 		return -ENXIO;
 	wake_up_interruptible(&channel->wq);
 	return 0;
-- 
1.9.1

[PATCH 2/9] staging: most: bool comparison style

[Sudip Mukherjee]

Mentioning true or false in the if comparison is error prone and also
not according to the coding style.

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

diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
index 7b637f3..e5853d0 100644
--- a/drivers/staging/most/aim-cdev/cdev.c
+++ b/drivers/staging/most/aim-cdev/cdev.c
@@ -133,7 +133,7 @@ static int aim_close(struct inode *inode, struct file *filp)
 
 	while (0 != kfifo_out((struct kfifo *)&channel->fifo, &mbo, 1))
 		most_put_mbo(mbo);
-	if (channel->keep_mbo == true)
+	if (channel->keep_mbo)
 		most_put_mbo(channel->stacked_mbo);
 	ret = most_stop_channel(channel->iface, channel->channel_id);
 	atomic_dec(&channel->access_ref);
@@ -224,7 +224,7 @@ 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 == true) {
+	if (channel->keep_mbo) {
 		mbo = channel->stacked_mbo;
 		channel->keep_mbo = false;
 		goto start_copy;
@@ -259,7 +259,7 @@ start_copy:
 
 	retval = not_copied ? proc_len - not_copied : proc_len;
 
-	if (channel->keep_mbo == true) {
+	if (channel->keep_mbo) {
 		channel->mbo_offs = retval;
 		channel->stacked_mbo = mbo;
 	} else {
-- 
1.9.1

[PATCH 3/9] staging: most: remove multiple blank line

[Sudip Mukherjee]

Multiple blank lines are not recommended in the kernel coding style.

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

diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
index e5853d0..818efc8 100644
--- a/drivers/staging/most/aim-cdev/cdev.c
+++ b/drivers/staging/most/aim-cdev/cdev.c
@@ -48,7 +48,6 @@ struct aim_channel {
 static struct list_head channel_list;
 static spinlock_t ch_list_lock;
 
-
 static struct aim_channel *get_channel(struct most_interface *iface, int id)
 {
 	struct aim_channel *channel, *tmp;
-- 
1.9.1

[PATCH 4/9] staging: most: out of memory error

[Sudip Mukherjee]

If kzalloc fails it will print lots of debugging information in the log,
no need to have another in the code.

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

diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
index 818efc8..b0a9a4a 100644
--- a/drivers/staging/most/aim-cdev/cdev.c
+++ b/drivers/staging/most/aim-cdev/cdev.c
@@ -412,7 +412,6 @@ static int aim_probe(struct most_interface *iface, int channel_id,
 
 	channel = kzalloc(sizeof(*channel), GFP_KERNEL);
 	if (!channel) {
-		pr_info("failed to alloc channel object\n");
 		retval = -ENOMEM;
 		goto error_alloc_channel;
 	}
-- 
1.9.1

[PATCH 5/9] staging: most: remove unused functions

[Sudip Mukherjee]

These functions were only defined but not used anywhere.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/most/hdm-dim2/dim2_hal.c |  5 -----
 drivers/staging/most/hdm-dim2/dim2_hal.h |  5 -----
 drivers/staging/most/hdm-dim2/dim2_hdm.c | 10 ----------
 3 files changed, 20 deletions(-)

diff --git a/drivers/staging/most/hdm-dim2/dim2_hal.c b/drivers/staging/most/hdm-dim2/dim2_hal.c
index a54cf2c..e334206 100644
--- a/drivers/staging/most/hdm-dim2/dim2_hal.c
+++ b/drivers/staging/most/hdm-dim2/dim2_hal.c
@@ -912,8 +912,3 @@ bool DIM_DetachBuffers(struct dim_channel *ch, u16 buffers_number)
 
 	return channel_detach_buffers(ch, buffers_number);
 }
-
-u32 DIM_ReadRegister(u8 register_index)
-{
-	return DIMCB_IoRead((u32 *)g.dim2 + register_index);
-}
diff --git a/drivers/staging/most/hdm-dim2/dim2_hal.h b/drivers/staging/most/hdm-dim2/dim2_hal.h
index 8929af9..ebb7d87 100644
--- a/drivers/staging/most/hdm-dim2/dim2_hal.h
+++ b/drivers/staging/most/hdm-dim2/dim2_hal.h
@@ -105,17 +105,12 @@ bool DIM_EnqueueBuffer(struct dim_channel *ch, u32 buffer_addr,
 
 bool DIM_DetachBuffers(struct dim_channel *ch, u16 buffers_number);
 
-u32 DIM_ReadRegister(u8 register_index);
-
-
 u32 DIMCB_IoRead(u32 *ptr32);
 
 void DIMCB_IoWrite(u32 *ptr32, u32 value);
 
 void DIMCB_OnError(u8 error_id, const char *error_message);
 
-void DIMCB_OnFail(const char *filename, int linenum);
-
 
 #ifdef __cplusplus
 }
diff --git a/drivers/staging/most/hdm-dim2/dim2_hdm.c b/drivers/staging/most/hdm-dim2/dim2_hdm.c
index 6a5a3a2..1ba694b 100644
--- a/drivers/staging/most/hdm-dim2/dim2_hdm.c
+++ b/drivers/staging/most/hdm-dim2/dim2_hdm.c
@@ -166,16 +166,6 @@ void DIMCB_OnError(u8 error_id, const char *error_message)
 }
 
 /**
- * DIMCB_OnFail - callback from HAL to report unrecoverable errors
- * @filename: Source file where the error happened
- * @linenum: Line number of the file where the error happened
- */
-void DIMCB_OnFail(const char *filename, int linenum)
-{
-	pr_err("DIMCB_OnFail: file - %s, line no. - %d\n", filename, linenum);
-}
-
-/**
  * startup_dim - initialize the dim2 interface
  * @pdev: platform device
  *
-- 
1.9.1

[PATCH 6/9] staging: most: make functions static

[Sudip Mukherjee]

split_arg_list() and audio_set_pcm_format() are being called from the
same file and is not referenced from outside, so make them as static. 

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/most/aim-sound/sound.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/most/aim-sound/sound.c b/drivers/staging/most/aim-sound/sound.c
index 860302e..27449d2 100644
--- a/drivers/staging/most/aim-sound/sound.c
+++ b/drivers/staging/most/aim-sound/sound.c
@@ -486,7 +486,7 @@ static struct snd_pcm_ops pcm_ops = {
 };
 
 
-int split_arg_list(char *buf, char **card_name, char **pcm_format)
+static int split_arg_list(char *buf, char **card_name, char **pcm_format)
 {
 	*card_name = strsep(&buf, ".");
 	if (!*card_name)
@@ -497,7 +497,8 @@ int split_arg_list(char *buf, char **card_name, char **pcm_format)
 	return 0;
 }
 
-int audio_set_pcm_format(char *pcm_format, struct most_channel_config *cfg)
+static int audio_set_pcm_format(char *pcm_format,
+				struct most_channel_config *cfg)
 {
 	if (!strcmp(pcm_format, "1x8")) {
 		if (cfg->subbuffer_size != 1)
-- 
1.9.1

[PATCH 7/9] staging: most: use NULL pointer

[Sudip Mukherjee]

sparse was complaining that an integer is used as NULL pointer. Fix it
by using NULL.

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

diff --git a/drivers/staging/most/hdm-dim2/dim2_hdm.c b/drivers/staging/most/hdm-dim2/dim2_hdm.c
index 1ba694b..5b0a588 100644
--- a/drivers/staging/most/hdm-dim2/dim2_hdm.c
+++ b/drivers/staging/most/hdm-dim2/dim2_hdm.c
@@ -238,7 +238,7 @@ static int try_start_dim_transfer(struct hdm_channel *hdm_ch)
 	unsigned long flags;
 	struct dim_ch_state_t st;
 
-	BUG_ON(hdm_ch == 0);
+	BUG_ON(!hdm_ch);
 	BUG_ON(!hdm_ch->is_initialized);
 
 	spin_lock_irqsave(&dim_lock, flags);
@@ -336,7 +336,7 @@ static void service_done_flag(struct dim2_hdm *dev, int ch_idx)
 	unsigned long flags;
 	u8 *data;
 
-	BUG_ON(hdm_ch == 0);
+	BUG_ON(!hdm_ch);
 	BUG_ON(!hdm_ch->is_initialized);
 
 	spin_lock_irqsave(&dim_lock, flags);
@@ -409,7 +409,7 @@ static struct dim_channel **get_active_channels(struct dim2_hdm *dev,
 		if (dev->hch[ch_idx].is_initialized)
 			buffer[idx++] = &dev->hch[ch_idx].ch;
 	}
-	buffer[idx++] = 0;
+	buffer[idx++] = NULL;
 
 	return buffer;
 }
@@ -905,7 +905,7 @@ static int dim2_remove(struct platform_device *pdev)
 	 * break link to local platform_device_id struct
 	 * to prevent crash by unload platform device module
 	 */
-	pdev->id_entry = 0;
+	pdev->id_entry = NULL;
 
 	return 0;
 }
-- 
1.9.1

[PATCH 8/9] staging: most: remove unused variable

[Sudip Mukherjee]

The variable conf was only assigned the value but was never used.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/most/hdm-usb/hdm_usb.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/most/hdm-usb/hdm_usb.c b/drivers/staging/most/hdm-usb/hdm_usb.c
index 305303f..34843b0 100644
--- a/drivers/staging/most/hdm-usb/hdm_usb.c
+++ b/drivers/staging/most/hdm-usb/hdm_usb.c
@@ -568,7 +568,6 @@ static void hdm_read_completion(struct urb *urb)
 	struct device *dev;
 	unsigned long flags;
 	unsigned int channel;
-	struct most_channel_config *conf;
 
 	mbo = urb->context;
 	anchor = mbo->priv;
@@ -582,8 +581,6 @@ static void hdm_read_completion(struct urb *urb)
 		return;
 	}
 
-	conf = &mdev->conf[channel];
-
 	if (unlikely(urb->status && !(urb->status == -ENOENT ||
 				      urb->status == -ECONNRESET ||
 				      urb->status == -ESHUTDOWN))) {
-- 
1.9.1

[PATCH 9/9] staging: most: fix Makefile

[Sudip Mukherjee]

The Makefile is including "drivers/media/video". But there is no such
directory in kernel tree. Since it is aim-v4l2 this might have been
"drivers/media/v4l2-core", but the Kconfig already mentions that it
depends on VIDEO_V4L2. So no need to mention that again in the Makefile. 

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

diff --git a/drivers/staging/most/aim-v4l2/Makefile b/drivers/staging/most/aim-v4l2/Makefile
index 28aa948..69a7524 100644
--- a/drivers/staging/most/aim-v4l2/Makefile
+++ b/drivers/staging/most/aim-v4l2/Makefile
@@ -3,4 +3,3 @@ obj-$(CONFIG_AIM_V4L2) += aim_v4l2.o
 aim_v4l2-objs := video.o
 
 ccflags-y += -Idrivers/staging/most/mostcore/
-ccflags-y += -Idrivers/media/video
-- 
1.9.1

Re: [PATCH 1/9] staging: most: NULL comparison style

[Fabio Estevam]

On Tue, Aug 18, 2015 at 12:18 PM, Sudip Mukherjee
<sudipm.mukherjee@gmail.com> wrote:
> According to the kernel coding style the NULL check should not be
> written as [variable] == NULL or [variable] != NULL.

It seems this not documented in Documentation/CodingStyle .

Re: [PATCH 1/9] staging: most: NULL comparison style

[Sudip Mukherjee]

On Tue, Aug 18, 2015 at 01:31:00PM -0300, Fabio Estevam wrote:
> On Tue, Aug 18, 2015 at 12:18 PM, Sudip Mukherjee
> <sudipm.mukherjee@gmail.com> wrote:
> > According to the kernel coding style the NULL check should not be
> > written as [variable] == NULL or [variable] != NULL.
> 
> It seems this not documented in Documentation/CodingStyle .
Yes, it is not in the CodingStyle file. But mostly it is the convention
that is followed. And in CodingStyle file if you see the "The rationale
for using gotos is:" section, you will see in the example function the
test is done like: if (!buffer).
Anyways, frankly speaking I know commit message is bad but I could not
think of anything else other than the one I wrote. Any ideas please...

regards
sudip

Re: [PATCH 1/9] staging: most: NULL comparison style

[Frans Klaver]

On Wed, Aug 19, 2015 at 12:00 PM, Sudip Mukherjee
<sudipm.mukherjee@gmail.com> wrote:
> On Tue, Aug 18, 2015 at 01:31:00PM -0300, Fabio Estevam wrote:
>> On Tue, Aug 18, 2015 at 12:18 PM, Sudip Mukherjee
>> <sudipm.mukherjee@gmail.com> wrote:
>> > According to the kernel coding style the NULL check should not be
>> > written as [variable] == NULL or [variable] != NULL.
>>
>> It seems this not documented in Documentation/CodingStyle .
> Yes, it is not in the CodingStyle file. But mostly it is the convention
> that is followed. And in CodingStyle file if you see the "The rationale
> for using gotos is:" section, you will see in the example function the
> test is done like: if (!buffer).
> Anyways, frankly speaking I know commit message is bad but I could not
> think of anything else other than the one I wrote. Any ideas please...

Maybe something like:

Although not made explicit in Documentation/CodingStyle, there seems
to be a preference for writing NULL tests as 'if (!var)' rather than
if (var == NULL). The coding style does explicitly prefer brevity, so
convert all NULL checks to follow this shorter approach.

Frans

[PATCH v2] staging: most: NULL comparison style

[Sudip Mukherjee]

checkpatch complains when a variable comparison to NULL is written as:
variable == NULL or variable != NULL.

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

diff --git a/drivers/staging/most/aim-cdev/cdev.c b/drivers/staging/most/aim-cdev/cdev.c
index 0a13d8d..7b637f3 100644
--- a/drivers/staging/most/aim-cdev/cdev.c
+++ b/drivers/staging/most/aim-cdev/cdev.c
@@ -174,7 +174,7 @@ static ssize_t aim_write(struct file *filp, const char __user *buf,
 			    channel->wq,
 			    (mbo = most_get_mbo(channel->iface,
 						channel->channel_id)) ||
-			    (channel->dev == NULL)))
+			    (!channel->dev)))
 			return -ERESTARTSYS;
 	}
 
@@ -230,12 +230,12 @@ aim_read(struct file *filp, char __user *buf, size_t count, loff_t *offset)
 		goto start_copy;
 	}
 	while ((0 == kfifo_out(&channel->fifo, &mbo, 1))
-	       && (channel->dev != NULL)) {
+	       && (channel->dev)) {
 		if (filp->f_flags & O_NONBLOCK)
 			return -EAGAIN;
 		if (wait_event_interruptible(channel->wq,
 					     (!kfifo_is_empty(&channel->fifo) ||
-					      (channel->dev == NULL))))
+					      (!channel->dev))))
 			return -ERESTARTSYS;
 	}
 
@@ -300,7 +300,7 @@ static int aim_disconnect_channel(struct most_interface *iface, int channel_id)
 	}
 
 	channel = get_channel(iface, channel_id);
-	if (channel == NULL)
+	if (!channel)
 		return -ENXIO;
 
 	mutex_lock(&channel->io_mutex);
@@ -337,7 +337,7 @@ static int aim_rx_completion(struct mbo *mbo)
 		return -EINVAL;
 
 	channel = get_channel(mbo->ifp, mbo->hdm_channel_id);
-	if (channel == NULL)
+	if (!channel)
 		return -ENXIO;
 
 	kfifo_in(&channel->fifo, &mbo, 1);
@@ -370,7 +370,7 @@ static int aim_tx_completion(struct most_interface *iface, int channel_id)
 	}
 
 	channel = get_channel(iface, channel_id);
-	if (channel == NULL)
+	if (!channel)
 		return -ENXIO;
 	wake_up_interruptible(&channel->wq);
 	return 0;
-- 
1.9.1