On Tue, 04 Nov 2008 18:10:45 +0200
"ext Tomi Valkeinen" [off-list ref] wrote:

Signed-off-by: Tomi Valkeinen <redacted>
---

 arch/arm/mach-omap2/board-3430sdp.c |  234 +++++++++++++++++++++++++++++++++--
 drivers/video/omap2/Kconfig         |    7 +
 drivers/video/omap2/Makefile        |    1 
 drivers/video/omap2/panel-sdp3430.c |  110 ++++++++++++++++
 4 files changed, 340 insertions(+), 12 deletions(-)
 create mode 100644 drivers/video/omap2/panel-sdp3430.c

+static void __init display_init(void)

I would use unique 3430sdp_display_init even the function is static.
Cleaner symbol names and cleaner oops dumps.

+{
+	int r;
+
+	enable_gpio    = SDP3430_LCD_PANEL_ENABLE_GPIO;
+	backlight_gpio = SDP3430_LCD_PANEL_BACKLIGHT_GPIO;
+
+	r = gpio_request(enable_gpio, "OMAP SDP LCD Reset GPIO");
+	if (r) {
+		printk(KERN_ERR "failed to get LCD reset GPIO\n");
+		goto err0;
+	}
+
+	r = gpio_request(backlight_gpio, "OMAP SDP LCD Backlight GPIO");
+	if (r) {

How about labels "LCD reset" and "LCD backlight" (actually bit too
long).

+static int panel_enable_lcd(struct omap_display *display)
+static void panel_disable_lcd(struct omap_display *display)
+static int panel_enable_dvi(struct omap_display *display)

Would it better to rename these as sdp3430_enable_lcd etc since
otherwise looks somewhat confusing to see them in board-3430sdp.c where
there is also panel-sdp3430.c?


Jarkko