On Tue, 04 Nov 2008 18:10:45 +0200
"ext Tomi Valkeinen" <[EMAIL PROTECTED]> wrote:

> Signed-off-by: Tomi Valkeinen <[EMAIL PROTECTED]>
> ---
> 
>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to