Re: [PATCH 27/57] fbdev: sh_mobile_lcdc: Pass a video mode to the

On Fri, 16 Dec 2011, Laurent Pinchart wrote:

Hi Guennadi,

On Thursday 15 December 2011 20:01:54 Guennadi Liakhovetski wrote:

quoted

On Tue, 13 Dec 2011, Laurent Pinchart wrote:

quoted

Pass pointers to struct fb_videomode and struct fb_monspecs instead of
struct fb_var_screeninfo to the notify callback.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---

 drivers/video/sh_mobile_hdmi.c   |   59
 +++++++++++++++++-------------------- drivers/video/sh_mobile_lcdcfb.c
 |   40 ++++++++++++++----------- drivers/video/sh_mobile_lcdcfb.h |   
 3 +-
 3 files changed, 51 insertions(+), 51 deletions(-)

[snip]

quoted

diff --git a/drivers/video/sh_mobile_lcdcfb.c

b/drivers/video/sh_mobile_lcdcfb.c index 21e5f10..4ec216e 100644

--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c

[ditto]

quoted

@@ -433,12 +432,17 @@ static int sh_mobile_lcdc_display_notify(struct

sh_mobile_lcdc_chan *ch,

 		}
 		break;

-	case SH_MOBILE_LCDC_EVENT_DISPLAY_MODE:
+	case SH_MOBILE_LCDC_EVENT_DISPLAY_MODE: {
+		struct fb_var_screeninfo var;
+

 		/* Validate a proposed new mode */

-		var->bits_per_pixel = info->var.bits_per_pixel;
-		ret = info->fbops->fb_check_var(var, info);
+		fb_videomode_to_var(&var, mode);
+		var.bits_per_pixel = info->var.bits_per_pixel;
+		var.grayscale = info->var.grayscale;
+		ret = info->fbops->fb_check_var(&var, info);

 		break;
 	
 	}

+	}

nitpick - please, realign:-)

It's common in driver code not to increase the identation level twice in a 
case statement if braces are needed. However, those "braced" statements are 
usually not the last ones in the switch. In this particular case this creates 
an alignment issue at the end, as your correctly pointed out. However, I'd 
like to avoid increasing the indentation of the whole block. Increasing the 
indentation of the closing brace only also causes an alignment issue. I can 
add a default case that returns an error to fix this, but that's adding code 
to fix a cosmetic problem :-) What's your preference.

Personally, I don't like just using braces without a related statement 
like "if," "do," etc. In such "case" cases I usually avoid adding own 
braces and just put any declarations, that are needed there in the 
enclosing block, e.g., the function. If you really don't want either, I 
would probably prefer a style like

	switch (x) {
	case X:
		{
			int y;
			my_statement(y);
			break;
		}
	}

You could also do

	switch (x) {
	case X:
		do {
			int y;
			my_statement(y);
			break;
		} while (0);
	}

So, choose your poison:-)

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/