Re: [PATCH 1/2]Trivial patch: to solve indentation warnings in amba-clcd.c driver

[Joe Perches]

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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/