According to the kernel coding style the NULL check should not be
written as [variable] == NULL or [variable] != NULL.
Signed-off-by: Sudip Mukherjee <[email protected]>
---
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
Mentioning true or false in the if comparison is error prone and also
not according to the coding style.
Signed-off-by: Sudip Mukherjee <[email protected]>
---
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
Multiple blank lines are not recommended in the kernel coding style.
Signed-off-by: Sudip Mukherjee <[email protected]>
---
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
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 <[email protected]>
---
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
These functions were only defined but not used anywhere.
Signed-off-by: Sudip Mukherjee <[email protected]>
---
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
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 <[email protected]>
---
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
sparse was complaining that an integer is used as NULL pointer. Fix it
by using NULL.
Signed-off-by: Sudip Mukherjee <[email protected]>
---
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
The variable conf was only assigned the value but was never used.
Signed-off-by: Sudip Mukherjee <[email protected]>
---
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
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 <[email protected]>
---
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
On Tue, Aug 18, 2015 at 12:18 PM, Sudip Mukherjee
<[email protected]> 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 .
On Tue, Aug 18, 2015 at 01:31:00PM -0300, Fabio Estevam wrote:
> On Tue, Aug 18, 2015 at 12:18 PM, Sudip Mukherjee
> <[email protected]> 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
On Wed, Aug 19, 2015 at 12:00 PM, Sudip Mukherjee
<[email protected]> 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
>> <[email protected]> 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
checkpatch complains when a variable comparison to NULL is written as:
variable == NULL or variable != NULL.
Signed-off-by: Sudip Mukherjee <[email protected]>
---
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