* [PATCH 2/3] drivers/fbtft: Remove unnecessary braces from if/else
@ 2018-01-08 10:06 Luis Gerhorst
2018-01-08 15:03 ` Dan Carpenter
0 siblings, 1 reply; 6+ messages in thread
From: Luis Gerhorst @ 2018-01-08 10:06 UTC (permalink / raw)
To: Thomas Petazzoni, Greg Kroah-Hartman, devel, linux-kernel
Cc: Jonny Schäfer, linux-kernel, Luis Gerhorst
The Linux kernel coding style states that braces should only be used
when necessary.
This fixes the checkpatch warning
WARNING: line over 80 characters
+ } else if (display->regwidth == 8 && display->buswidth == 9 && par->spi) {
introduced by patch #1.
Signed-off-by: Luis Gerhorst <linux-kernel@luisgerhorst.de>
Acked-by: Jonny Schaefer <schaefer.jonny@gmail.com>
Acked-by: Alexander Wuerstlein <arw@cs.fau.de>
---
drivers/staging/fbtft/fbtft-core.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 66b46b2..34b1c81 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -1367,19 +1367,18 @@ int fbtft_probe_common(struct fbtft_display *display,
}
/* write register functions */
- if (display->regwidth == 8 && display->buswidth == 8) {
+ if (display->regwidth == 8 && display->buswidth == 8)
par->fbtftops.write_register = fbtft_write_reg8_bus8;
- } else if (display->regwidth == 8 && display->buswidth == 9 && par->spi) {
+ else if (display->regwidth == 8 && display->buswidth == 9 && par->spi)
par->fbtftops.write_register = fbtft_write_reg8_bus9;
- } else if (display->regwidth == 16 && display->buswidth == 8) {
+ else if (display->regwidth == 16 && display->buswidth == 8)
par->fbtftops.write_register = fbtft_write_reg16_bus8;
- } else if (display->regwidth == 16 && display->buswidth == 16) {
+ else if (display->regwidth == 16 && display->buswidth == 16)
par->fbtftops.write_register = fbtft_write_reg16_bus16;
- } else {
+ else
dev_warn(dev,
"no default functions for regwidth=%d and buswidth=%d\n",
display->regwidth, display->buswidth);
- }
/* write_vmem() functions */
if (display->buswidth == 8)
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/3] drivers/fbtft: Remove unnecessary braces from if/else
2018-01-08 10:06 [PATCH 2/3] drivers/fbtft: Remove unnecessary braces from if/else Luis Gerhorst
@ 2018-01-08 15:03 ` Dan Carpenter
2018-01-08 18:27 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2018-01-08 15:03 UTC (permalink / raw)
To: Luis Gerhorst
Cc: Thomas Petazzoni, Greg Kroah-Hartman, devel, linux-kernel,
Jonny Schäfer, linux-kernel
On Mon, Jan 08, 2018 at 11:06:37AM +0100, Luis Gerhorst wrote:
> The Linux kernel coding style states that braces should only be used
> when necessary.
>
> This fixes the checkpatch warning
>
> WARNING: line over 80 characters
> + } else if (display->regwidth == 8 && display->buswidth == 9 && par->spi) {
>
> introduced by patch #1.
>
Don't introduce warnings and then fix them in later patches.
Anyway there is another unwritten rule that multi-line indents get curly
braces. Probably it should be:
} else if (display->regwidth == 8 && display->buswidth == 9 &&
par->spi) {
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/3] drivers/fbtft: Remove unnecessary braces from if/else
2018-01-08 15:03 ` Dan Carpenter
@ 2018-01-08 18:27 ` Joe Perches
2018-01-08 19:26 ` Dan Carpenter
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2018-01-08 18:27 UTC (permalink / raw)
To: Dan Carpenter, Luis Gerhorst
Cc: Thomas Petazzoni, Greg Kroah-Hartman, devel, linux-kernel,
Jonny Schäfer, linux-kernel
On Mon, 2018-01-08 at 18:03 +0300, Dan Carpenter wrote:
> On Mon, Jan 08, 2018 at 11:06:37AM +0100, Luis Gerhorst wrote:
> > The Linux kernel coding style states that braces should only be used
> > when necessary.
> >
> > This fixes the checkpatch warning
> >
> > WARNING: line over 80 characters
> > + } else if (display->regwidth == 8 && display->buswidth == 9 && par->spi) {
> >
> > introduced by patch #1.
> >
>
> Don't introduce warnings and then fix them in later patches.
>
> Anyway there is another unwritten rule that multi-line indents get curly
> braces. Probably it should be:
Nope. That'd be your own preferred style.
If you want it to be followed by others,
please try and get it added to CodingStyle
and use examples to show why it's better
than allowing maintainer preference.
> } else if (display->regwidth == 8 && display->buswidth == 9 &&
> par->spi) {
My own preferred style here would be to align
the display->regwidth and display->buswidth so
I can differentiate the similarity in naming a
bit more easily.
} else if (display->regwidth == 8 &&
display->buswidth == 9 && par->spi) {
I'm not sure at all there is a single best
style for this and it can easily become
situation dependent.
And so I suggest not adding anything about this
style nit to CodingStyle.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/3] drivers/fbtft: Remove unnecessary braces from if/else
2018-01-08 18:27 ` Joe Perches
@ 2018-01-08 19:26 ` Dan Carpenter
2018-01-08 19:48 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2018-01-08 19:26 UTC (permalink / raw)
To: Joe Perches
Cc: Luis Gerhorst, devel, linux-kernel, Greg Kroah-Hartman,
linux-kernel, Jonny Schäfer
On Mon, Jan 08, 2018 at 10:27:01AM -0800, Joe Perches wrote:
> On Mon, 2018-01-08 at 18:03 +0300, Dan Carpenter wrote:
> > On Mon, Jan 08, 2018 at 11:06:37AM +0100, Luis Gerhorst wrote:
> > > The Linux kernel coding style states that braces should only be used
> > > when necessary.
> > >
> > > This fixes the checkpatch warning
> > >
> > > WARNING: line over 80 characters
> > > + } else if (display->regwidth == 8 && display->buswidth == 9 && par->spi) {
> > >
> > > introduced by patch #1.
> > >
> >
> > Don't introduce warnings and then fix them in later patches.
> >
> > Anyway there is another unwritten rule that multi-line indents get curly
> > braces. Probably it should be:
>
> Nope. That'd be your own preferred style.
>
I copied it from Greg so it's the subsystem style.
> If you want it to be followed by others,
> please try and get it added to CodingStyle
> and use examples to show why it's better
> than allowing maintainer preference.
>
I'm not going to NACK patches which don't follow that rule, so it's not
something which is helpful to put in checkpatch.pl. The reason I'm
complaining about this patch is that it fixes a patch the author
introduces earlier in the patchset.
> > } else if (display->regwidth == 8 && display->buswidth == 9 &&
> > par->spi) {
>
> My own preferred style here would be to align
> the display->regwidth and display->buswidth so
> I can differentiate the similarity in naming a
> bit more easily.
Yeah. I thought about that as well but then it takes up 3 lines...
Either way is fine.
>
> } else if (display->regwidth == 8 &&
> display->buswidth == 9 && par->spi) {
>
> I'm not sure at all there is a single best
> style for this and it can easily become
> situation dependent.
>
> And so I suggest not adding anything about this
> style nit to CodingStyle.
Agreed.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/3] drivers/fbtft: Remove unnecessary braces from if/else
2018-01-08 19:26 ` Dan Carpenter
@ 2018-01-08 19:48 ` Joe Perches
2018-01-08 20:35 ` Dan Carpenter
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2018-01-08 19:48 UTC (permalink / raw)
To: Dan Carpenter
Cc: Luis Gerhorst, devel, linux-kernel, Greg Kroah-Hartman,
linux-kernel, Jonny Schäfer
On Mon, 2018-01-08 at 22:26 +0300, Dan Carpenter wrote:
> On Mon, Jan 08, 2018 at 10:27:01AM -0800, Joe Perches wrote:
> > On Mon, 2018-01-08 at 18:03 +0300, Dan Carpenter wrote:
> > > On Mon, Jan 08, 2018 at 11:06:37AM +0100, Luis Gerhorst wrote:
> > > > The Linux kernel coding style states that braces should only be used
> > > > when necessary.
> > > >
> > > > This fixes the checkpatch warning
> > > >
> > > > WARNING: line over 80 characters
> > > > + } else if (display->regwidth == 8 && display->buswidth == 9 && par->spi) {
> > > >
> > > > introduced by patch #1.
> > > >
> > >
> > > Don't introduce warnings and then fix them in later patches.
Hey Dan.
btw: I completely agree with this
> > > Anyway there is another unwritten rule that multi-line indents get curly
> > > braces. Probably it should be:
> >
> > Nope. That'd be your own preferred style.
> >
> I copied it from Greg so it's the subsystem style.
I don't fine any examples that match your
suggestion above in drivers/staging/fbtft.
Perhaps because there aren't many multi line
if statements.
All of the blocks that use braces are required
because one or more of the if/else blocks
contain multiple statements.
There are counterexamples in the subsystem.
cheers, Joe
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-01-08 20:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-08 10:06 [PATCH 2/3] drivers/fbtft: Remove unnecessary braces from if/else Luis Gerhorst
2018-01-08 15:03 ` Dan Carpenter
2018-01-08 18:27 ` Joe Perches
2018-01-08 19:26 ` Dan Carpenter
2018-01-08 19:48 ` Joe Perches
2018-01-08 20:35 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox