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: > > @@ -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; > > @@ -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); -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
