Re: [PATCH] drivers: video: Fix 80 character limit and remove unnecessary space

2 messages, 2 authors, 2017-08-23 · open the first message on its own page

Re: [PATCH] drivers: video: Fix 80 character limit and remove unnecessary space

From: Bartlomiej Zolnierkiewicz <hidden>
Date: 2017-08-23 16:12:49

Hi,

Please use a more descriptive patch subject, i.e.
"fbdev: vgastate: fix checkpatch.pl reported warnings".

Please also cc: fbdev mailing list (linux-fbdev@vger.kernel.org
on fbdev patches).

On Monday, August 21, 2017 07:33:51 PM Himanshu Jha wrote:
quoted hunk
Fixes the following checkpatch warnings:
WARNING: line over 80 characters
WARNING: space prohibited between function name and open parenthesis '('
WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable

Signed-off-by: Himanshu Jha <redacted>
---
 drivers/video/vgastate.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/video/vgastate.c b/drivers/video/vgastate.c
index 548c751..a52283d 100644
--- a/drivers/video/vgastate.c
+++ b/drivers/video/vgastate.c
@@ -31,8 +31,9 @@ struct regstate {
 	__u8 misc;
 };
 
-static inline unsigned char vga_rcrtcs(void __iomem *regbase, unsigned short iobase,
-				       unsigned char reg)
+static inline unsigned char vga_rcrtcs(void __iomem *regbase,
+						unsigned short iobase,
+							unsigned char reg)
This is not an improvement and doesn't conform to the kernel CodingStyle.

It should be:

static inline unsigned char vga_rcrtcs(void __iomem *regbase,
				       unsigned short iobase,
				       unsigned char reg)
quoted hunk
 {
 	vga_w(regbase, iobase + 0x4, reg);
 	return vga_r(regbase, iobase + 0x5);
@@ -454,8 +455,9 @@ int save_vga(struct vgastate *state)
 	}
 	return 0;
 }
+EXPORT_SYMBOL(save_vga);
 
-int restore_vga (struct vgastate *state)
+int restore_vga(struct vgastate *state)
 {
 	if (state->vidstate = NULL)
 		return 1;
@@ -480,8 +482,6 @@ int restore_vga (struct vgastate *state)
 	vga_cleanup(state);
 	return 0;
 }
-
-EXPORT_SYMBOL(save_vga);
 EXPORT_SYMBOL(restore_vga);
 
 MODULE_AUTHOR("James Simmons <jsimmons@users.sf.net>");
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH] drivers: video: Fix 80 character limit and remove unnecessary space

From: Joe Perches <joe@perches.com>
Date: 2017-08-23 23:16:09

On Wed, 2017-08-23 at 18:12 +0200, Bartlomiej Zolnierkiewicz wrote:
Hi,

Please use a more descriptive patch subject, i.e.
"fbdev: vgastate: fix checkpatch.pl reported warnings".

Please also cc: fbdev mailing list (linux-fbdev@vger.kernel.org
on fbdev patches).

On Monday, August 21, 2017 07:33:51 PM Himanshu Jha wrote:
quoted
Fixes the following checkpatch warnings:
WARNING: line over 80 characters
WARNING: space prohibited between function name and open parenthesis '('
WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
[]
quoted
diff --git a/drivers/video/vgastate.c b/drivers/video/vgastate.c
[]
quoted
@@ -31,8 +31,9 @@ struct regstate {
 	__u8 misc;
 };
 
-static inline unsigned char vga_rcrtcs(void __iomem *regbase, unsigned short iobase,
-				       unsigned char reg)
+static inline unsigned char vga_rcrtcs(void __iomem *regbase,
+						unsigned short iobase,
+							unsigned char reg)
This is not an improvement and doesn't conform to the kernel CodingStyle.

It should be:

static inline unsigned char vga_rcrtcs(void __iomem *regbase,
				       unsigned short iobase,
				       unsigned char reg)
quoted
 {
 	vga_w(regbase, iobase + 0x4, reg);
 	return vga_r(regbase, iobase + 0x5);
Better to remove the inline and let the compiler decide too.

Perhaps:

static unsigned char vga_rcrtcs(void __iomem *regbase, unsigned short iobase,
				unsigned char reg)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help