On Fri, 2015-02-13 at 11:35 +0000, Russell King - ARM Linux wrote:

On Fri, Feb 13, 2015 at 06:21:33AM -0500, Parmeshwr Prasad wrote:

quoted

@@ -288,7 +288,7 @@ static int clcdfb_set_par(struct fb_info *info)
        struct clcd_regs regs;

        fb->fb.fix.line_length = fb->fb.var.xres_virtual *
-                                fb->fb.var.bits_per_pixel / 8;
+           fb->fb.var.bits_per_pixel / 8;

NAK on this one.  The code as it stood before is much clearer since
we align the expression with the start of it on the preceding line.

I agree with all of what Russell wrote, but maybe this;

quoted

@@ -717,7 +716,9 @@ static int clcdfb_of_vram_setup(struct clcd_fb *fb)
                return -ENOMEM;

        fb->fb.fix.smem_start = of_translate_address(memory,
-                       of_get_address(memory, 0, &size, NULL));
+                                                    of_get_address(memory, 0,
+                                                                   &size,
+                                                                   NULL));

Thi sis the exception to the rule - where scrunching an expression so that
it takes multiple lines because of lack of right-hand space is not on.
The former version was a lot better.

Perhaps this could be better as:

	fb->fb.fix.smem_start 		 of_translate_address(memory,
				      of_get_address(memory, 0, &size, NULL));

But sometimes using multiple statements instead of
embedding function calls as arguments can be simpler
and more intelligible for the reader.

	__be32 addr;

	...

	addr = of_get_address(memory, 0, &size, NULL);
	fb->fb.fix.smem_start = of_translate_address(memory, addr);