* [PATCH] staging: comedi: drivers: pcl812.c: fixed a coding style issue @ 2014-04-04 10:05 Kumar Amit Mehta 2014-04-04 11:07 ` Dan Carpenter 2014-04-04 12:39 ` walter harms 0 siblings, 2 replies; 8+ messages in thread From: Kumar Amit Mehta @ 2014-04-04 10:05 UTC (permalink / raw) To: abbotti; +Cc: hsweeten, gregkh, devel, kernel-janitors, linux-kernel Fixed a coding style issue. Reported by checkpatch.pl Signed-off-by: Kumar Amit Mehta <gmate.amit@gmail.com> --- drivers/staging/comedi/drivers/pcl812.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/comedi/drivers/pcl812.c b/drivers/staging/comedi/drivers/pcl812.c index 160eac8..5cc01fe 100644 --- a/drivers/staging/comedi/drivers/pcl812.c +++ b/drivers/staging/comedi/drivers/pcl812.c @@ -811,8 +811,9 @@ static int pcl812_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s) devpriv->ai_dma = 0; break; } - } else + } else { devpriv->ai_dma = 0; + } devpriv->ai_act_scan = 0; devpriv->ai_poll_ptr = 0; -- 1.8.5.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: comedi: drivers: pcl812.c: fixed a coding style issue 2014-04-04 10:05 [PATCH] staging: comedi: drivers: pcl812.c: fixed a coding style issue Kumar Amit Mehta @ 2014-04-04 11:07 ` Dan Carpenter 2014-04-04 11:48 ` Kumar Amit Mehta 2014-04-04 12:39 ` walter harms 1 sibling, 1 reply; 8+ messages in thread From: Dan Carpenter @ 2014-04-04 11:07 UTC (permalink / raw) To: Kumar Amit Mehta; +Cc: abbotti, devel, kernel-janitors, linux-kernel, gregkh On Fri, Apr 04, 2014 at 01:05:29PM +0300, Kumar Amit Mehta wrote: > Fixed a coding style issue. Reported by checkpatch.pl > It's better if the commit messages are more specific than this. regards, dan carpenter ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: comedi: drivers: pcl812.c: fixed a coding style issue 2014-04-04 11:07 ` Dan Carpenter @ 2014-04-04 11:48 ` Kumar Amit Mehta 2014-04-04 13:26 ` Dan Carpenter 0 siblings, 1 reply; 8+ messages in thread From: Kumar Amit Mehta @ 2014-04-04 11:48 UTC (permalink / raw) To: Dan Carpenter; +Cc: abbotti, devel, kernel-janitors, linux-kernel, gregkh On Fri, Apr 04, 2014 at 02:07:07PM +0300, Dan Carpenter wrote: > On Fri, Apr 04, 2014 at 01:05:29PM +0300, Kumar Amit Mehta wrote: > > Fixed a coding style issue. Reported by checkpatch.pl > > > > It's better if the commit messages are more specific than this. So, should I resend the patch with a more appropriate commit message ? Thanks, Kumar ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: comedi: drivers: pcl812.c: fixed a coding style issue 2014-04-04 11:48 ` Kumar Amit Mehta @ 2014-04-04 13:26 ` Dan Carpenter 2014-04-04 15:17 ` Kumar Amit Mehta 0 siblings, 1 reply; 8+ messages in thread From: Dan Carpenter @ 2014-04-04 13:26 UTC (permalink / raw) To: Kumar Amit Mehta; +Cc: devel, gregkh, abbotti, kernel-janitors, linux-kernel On Fri, Apr 04, 2014 at 02:48:52PM +0300, Kumar Amit Mehta wrote: > On Fri, Apr 04, 2014 at 02:07:07PM +0300, Dan Carpenter wrote: > > On Fri, Apr 04, 2014 at 01:05:29PM +0300, Kumar Amit Mehta wrote: > > > Fixed a coding style issue. Reported by checkpatch.pl > > > > > > > It's better if the commit messages are more specific than this. > > So, should I resend the patch with a more appropriate commit message ? [PATCH] staging: comedi: drivers: pcl812.c: add curly braces for checkpatch Kernel style is that if one side of the if else statement gets has curly braces then both side should have them. regards, dan carpenter ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: comedi: drivers: pcl812.c: fixed a coding style issue 2014-04-04 13:26 ` Dan Carpenter @ 2014-04-04 15:17 ` Kumar Amit Mehta 2014-04-04 15:28 ` Joe Perches 2014-04-04 15:32 ` Dan Carpenter 0 siblings, 2 replies; 8+ messages in thread From: Kumar Amit Mehta @ 2014-04-04 15:17 UTC (permalink / raw) To: Dan Carpenter Cc: abbotti, hsweeten, wharms, gregkh, devel, kernel-janitors, linux-kernel On Fri, Apr 04, 2014 at 04:26:51PM +0300, Dan Carpenter wrote: > On Fri, Apr 04, 2014 at 02:48:52PM +0300, Kumar Amit Mehta wrote: > > On Fri, Apr 04, 2014 at 02:07:07PM +0300, Dan Carpenter wrote: > > > On Fri, Apr 04, 2014 at 01:05:29PM +0300, Kumar Amit Mehta wrote: > > > > Fixed a coding style issue. Reported by checkpatch.pl > > > > > > > > > > It's better if the commit messages are more specific than this. > > > > So, should I resend the patch with a more appropriate commit message ? > > [PATCH] staging: comedi: drivers: pcl812.c: add curly braces for checkpatch > > Kernel style is that if one side of the if else statement gets has curly > braces then both side should have them. Dan, What Walter mentioned also makes sense, So shouldn't it be something like this: [amit@localhost linux-next]$ git diff diff --git a/drivers/staging/comedi/drivers/pcl812.c b/drivers/staging/comedi/drivers/pcl812.c index 160eac8..552b696 100644 --- a/drivers/staging/comedi/drivers/pcl812.c +++ b/drivers/staging/comedi/drivers/pcl812.c @@ -803,16 +803,14 @@ static int pcl812_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s) pcl812_ai_set_chan_range(dev, cmd->chanlist[0], 1); + devpriv->ai_dma = 0; if (devpriv->dma) { /* check if we can use DMA transfer */ devpriv->ai_dma = 1; for (i = 1; i < cmd->chanlist_len; i++) if (cmd->chanlist[0] != cmd->chanlist[i]) { /* we cann't use DMA :-( */ - devpriv->ai_dma = 0; break; } - } else - devpriv->ai_dma = 0; devpriv->ai_act_scan = 0; devpriv->ai_poll_ptr = 0; Thanks, Kumar ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: comedi: drivers: pcl812.c: fixed a coding style issue 2014-04-04 15:17 ` Kumar Amit Mehta @ 2014-04-04 15:28 ` Joe Perches 2014-04-04 15:32 ` Dan Carpenter 1 sibling, 0 replies; 8+ messages in thread From: Joe Perches @ 2014-04-04 15:28 UTC (permalink / raw) To: Kumar Amit Mehta Cc: Dan Carpenter, abbotti, hsweeten, wharms, gregkh, devel, kernel-janitors, linux-kernel On Fri, 2014-04-04 at 18:17 +0300, Kumar Amit Mehta wrote: > What Walter mentioned also makes sense, So shouldn't it be something > like this: I'm not Dan, but: > [amit@localhost linux-next]$ git diff > diff --git a/drivers/staging/comedi/drivers/pcl812.c > b/drivers/staging/comedi/drivers/pcl812.c > index 160eac8..552b696 100644 > --- a/drivers/staging/comedi/drivers/pcl812.c > +++ b/drivers/staging/comedi/drivers/pcl812.c > @@ -803,16 +803,14 @@ static int pcl812_ai_cmd(struct comedi_device > *dev, struct comedi_subdevice *s) > > pcl812_ai_set_chan_range(dev, cmd->chanlist[0], 1); > > + devpriv->ai_dma = 0; > if (devpriv->dma) { /* check if we can use DMA transfer */ > devpriv->ai_dma = 1; > for (i = 1; i < cmd->chanlist_len; i++) > if (cmd->chanlist[0] != cmd->chanlist[i]) { > /* we cann't use DMA :-( */ > - devpriv->ai_dma = 0; No. You enable unwanted DMA capability here. There's a typo of can't too. > break; > } > - } else > - devpriv->ai_dma = 0; Otherwise, I suppose either style is OK. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: comedi: drivers: pcl812.c: fixed a coding style issue 2014-04-04 15:17 ` Kumar Amit Mehta 2014-04-04 15:28 ` Joe Perches @ 2014-04-04 15:32 ` Dan Carpenter 1 sibling, 0 replies; 8+ messages in thread From: Dan Carpenter @ 2014-04-04 15:32 UTC (permalink / raw) To: Kumar Amit Mehta Cc: abbotti, hsweeten, wharms, gregkh, devel, kernel-janitors, linux-kernel On Fri, Apr 04, 2014 at 06:17:56PM +0300, Kumar Amit Mehta wrote: > On Fri, Apr 04, 2014 at 04:26:51PM +0300, Dan Carpenter wrote: > > On Fri, Apr 04, 2014 at 02:48:52PM +0300, Kumar Amit Mehta wrote: > > > On Fri, Apr 04, 2014 at 02:07:07PM +0300, Dan Carpenter wrote: > > > > On Fri, Apr 04, 2014 at 01:05:29PM +0300, Kumar Amit Mehta wrote: > > > > > Fixed a coding style issue. Reported by checkpatch.pl > > > > > > > > > > > > > It's better if the commit messages are more specific than this. > > > > > > So, should I resend the patch with a more appropriate commit message ? > > > > [PATCH] staging: comedi: drivers: pcl812.c: add curly braces for checkpatch > > > > Kernel style is that if one side of the if else statement gets has curly > > braces then both side should have them. > > Dan, > > What Walter mentioned also makes sense, So shouldn't it be something > like this: > > [amit@localhost linux-next]$ git diff > diff --git a/drivers/staging/comedi/drivers/pcl812.c > b/drivers/staging/comedi/drivers/pcl812.c > index 160eac8..552b696 100644 > --- a/drivers/staging/comedi/drivers/pcl812.c > +++ b/drivers/staging/comedi/drivers/pcl812.c > @@ -803,16 +803,14 @@ static int pcl812_ai_cmd(struct comedi_device > *dev, struct comedi_subdevice *s) > > pcl812_ai_set_chan_range(dev, cmd->chanlist[0], 1); > > + devpriv->ai_dma = 0; > if (devpriv->dma) { /* check if we can use DMA transfer */ > devpriv->ai_dma = 1; > for (i = 1; i < cmd->chanlist_len; i++) > if (cmd->chanlist[0] != cmd->chanlist[i]) { > /* we cann't use DMA :-( */ > - devpriv->ai_dma = 0; You'd still want this assignment. > break; > } > - } else > - devpriv->ai_dma = 0; regards, dan carpenter ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: comedi: drivers: pcl812.c: fixed a coding style issue 2014-04-04 10:05 [PATCH] staging: comedi: drivers: pcl812.c: fixed a coding style issue Kumar Amit Mehta 2014-04-04 11:07 ` Dan Carpenter @ 2014-04-04 12:39 ` walter harms 1 sibling, 0 replies; 8+ messages in thread From: walter harms @ 2014-04-04 12:39 UTC (permalink / raw) To: Kumar Amit Mehta Cc: abbotti, hsweeten, gregkh, devel, kernel-janitors, linux-kernel Am 04.04.2014 12:05, schrieb Kumar Amit Mehta: > Fixed a coding style issue. Reported by checkpatch.pl > > Signed-off-by: Kumar Amit Mehta <gmate.amit@gmail.com> > --- > drivers/staging/comedi/drivers/pcl812.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/comedi/drivers/pcl812.c b/drivers/staging/comedi/drivers/pcl812.c > index 160eac8..5cc01fe 100644 > --- a/drivers/staging/comedi/drivers/pcl812.c > +++ b/drivers/staging/comedi/drivers/pcl812.c > @@ -811,8 +811,9 @@ static int pcl812_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s) > devpriv->ai_dma = 0; > break; > } > - } else > + } else { > devpriv->ai_dma = 0; > + } > > devpriv->ai_act_scan = 0; > devpriv->ai_poll_ptr = 0; hi Kumar, is that else needed at all ? perhaps it is possible to devpriv->ai_dma=0 before the if ? That would reduce code and give a better readability. re, wh ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-04-04 15:33 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-04-04 10:05 [PATCH] staging: comedi: drivers: pcl812.c: fixed a coding style issue Kumar Amit Mehta 2014-04-04 11:07 ` Dan Carpenter 2014-04-04 11:48 ` Kumar Amit Mehta 2014-04-04 13:26 ` Dan Carpenter 2014-04-04 15:17 ` Kumar Amit Mehta 2014-04-04 15:28 ` Joe Perches 2014-04-04 15:32 ` Dan Carpenter 2014-04-04 12:39 ` walter harms
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).