Re: [PATCH] i.MX Framebuffer: Cleanup Coding style

On Wed, 20 Aug 2008 18:15:21 +0200
Sascha Hauer [off-list ref] wrote:

quoted

On Wed, Aug 20, 2008 at 05:35:01PM +0200, Krzysztof Helt wrote:

quoted

On Tue, 19 Aug 2008 17:06:42 +0200
Sascha Hauer [off-list ref] wrote:

quoted

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/video/imxfb.c |   78 +++++++++++++++++++++++++------------------------
 1 files changed, 40 insertions(+), 38 deletions(-)

I don't like some of the changes like wrapping lines
which are not longer than 80 characters or removing
alignment vs previous line because it contains spaces.
There are also positive CS changes so not everything is bad.

Hm, this patch only wraps lines longer than 80 characters (in one case
it even merges two lines to one because it's still shorter than 80
chars).

The change around line 250 wraps 80 chars line.

@@ -250,7 +249,8 @@ imxfb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
 
 	case FB_VISUAL_STATIC_PSEUDOCOLOR:
 	case FB_VISUAL_PSEUDOCOLOR:
-		ret = imxfb_setpalettereg(regno, red, green, blue, trans, info);
+		ret = imxfb_setpalettereg(regno, red, green, blue, trans,
+				info);

 		break;
 	}

quoted

It also does not remove alignment vs the previous line, instead
it aligns some lines against the previous line. 

@@ -185,7 +184,7 @@ static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)

 static int
 imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
-                      u_int trans, struct fb_info *info)
+               u_int trans, struct fb_info *info)
 {
        struct imxfb_info *fbi = info->par;
        u_int val, ret = 1;

The removed line was previously aligned to start after the "(" in the previous line.
One could move "static int" into the same line.

You have added spaces between sizeof operator and the opening parenthesis.
It is pointed out by the checkpatch script.