linux-mips
[Top] [All Lists]

Re: [Linux-fbdev-devel] [PATCH] au1200fb: make framebuffer config runtim

To: Manuel Lauss <mano@roarinelk.homelinux.net>
Subject: Re: [Linux-fbdev-devel] [PATCH] au1200fb: make framebuffer config runtime selectable.
From: Krzysztof Helt <krzysztof.h1@poczta.fm>
Date: Tue, 5 Aug 2008 17:25:47 +0200
Cc: linux-fbdev-devel@lists.sf.net, L-M-O <linux-mips@linux-mips.org>, Kevin Hickey <khickey@rmicorp.com>
In-reply-to: <20080805124522.GB27469@roarinelk.homelinux.net>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <20080805124221.GA27469@roarinelk.homelinux.net> <20080805124522.GB27469@roarinelk.homelinux.net>
Sender: linux-mips-bounce@linux-mips.org
On Tue, 5 Aug 2008 14:45:22 +0200
Manuel Lauss <mano@roarinelk.homelinux.net> wrote:

> Make the number of framebuffer windows and the window configuration
> selectable at the kernel commandline instead of hardcoding it
> in the kernel config.
> 
> This patch does not directly depend on the previous one ("au1200fb: fixup PM
> support"), but it only applies cleanly on top of it.
> 
> Signed-off-by: Manuel Lauss <mano@roarinelk.homelinux.net>
> ---
>  drivers/video/au1200fb.c |   55 ++++++++++++++++++++++++++++++++++-----------
>  1 files changed, 41 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/video/au1200fb.c b/drivers/video/au1200fb.c
> index be945ab..7127aa7 100644
> --- a/drivers/video/au1200fb.c
> +++ b/drivers/video/au1200fb.c
> @@ -45,10 +45,6 @@
>  #include <asm/mach-au1x00/au1000.h>
>  #include "au1200fb.h"
>  
> -#ifndef CONFIG_FB_AU1200_DEVS
> -#define CONFIG_FB_AU1200_DEVS 4
> -#endif
> -
>  #define DRIVER_NAME "au1200fb"
>  #define DRIVER_DESC "LCD controller driver for AU1200 processors"
>  
> @@ -153,7 +149,7 @@ struct au1200fb_device {
>       dma_addr_t              fb_phys;
>  };
>  
> -static struct au1200fb_device _au1200fb_devices[CONFIG_FB_AU1200_DEVS];
> +static struct au1200fb_device _au1200fb_devices[4];

Please use the constant here.

>  /********************************************************************/
>  
>  /* LCD controller restrictions */
> @@ -166,10 +162,17 @@ static struct au1200fb_device 
> _au1200fb_devices[CONFIG_FB_AU1200_DEVS];
>  /* Default number of visible screen buffer to allocate */
>  #define AU1200FB_NBR_VIDEO_BUFFERS 1
>  
> +/* Default number of fb devices to create */
> +#define DEFAULT_DEVICE_COUNT 4
> +

This one can be renamed to MAX_DEVICE_COUNT and used in the declaration
above and various checks. There is nothing wrong with the default value being
the maximum value.

> +/* Default window configuration entry to use (see windows[]) */
> +#define DEFAULT_WINDOW_INDEX 2
> +
>  /********************************************************************/
>  
>  static struct au1200_lcd *lcd = (struct au1200_lcd *) AU1200_LCD_ADDR;
> -static int window_index = 2; /* default is zero */
> +static int device_count = DEFAULT_DEVICE_COUNT;      /* number of fb devices 
> to create */
> +static int window_index = DEFAULT_WINDOW_INDEX;      /* default is zero */
>  static int panel_index = 2; /* default is zero */
>  static struct window_settings *win;
>  static struct panel_settings *panel;
> @@ -682,7 +685,7 @@ static int fbinfo2index (struct fb_info *fb_info)
>  {
>       int i;
>  
> -     for (i = 0; i < CONFIG_FB_AU1200_DEVS; ++i) {
> +     for (i = 0; i < device_count; ++i) {
>               if (fb_info == (struct fb_info 
> *)(&_au1200fb_devices[i].fb_info))
>                       return i;
>       }
> @@ -1594,7 +1597,7 @@ static int au1200fb_drv_probe(struct platform_device 
> *pdev)
>       /* Kickstart the panel */
>       au1200_setpanel(panel);
>  
> -     for (plane = 0; plane < CONFIG_FB_AU1200_DEVS; ++plane) {
> +     for (plane = 0; plane < device_count; ++plane) {
>               bpp = winbpp(win->w[plane].mode_winctrl1);
>               if (win->w[plane].xres == 0)
>                       win->w[plane].xres = panel->Xres;
> @@ -1686,7 +1689,7 @@ static int au1200fb_drv_remove(struct platform_device 
> *pdev)
>       /* Turn off the panel */
>       au1200_setpanel(NULL);
>  
> -     for (plane = 0; plane < CONFIG_FB_AU1200_DEVS; ++plane)
> +     for (plane = 0; plane < device_count; ++plane)
>       {
>               fbdev = &_au1200fb_devices[plane];
>  
> @@ -1727,7 +1730,7 @@ static int au1200fb_drv_resume(struct platform_device 
> *pdev)
>       /* Kickstart the panel */
>       au1200_setpanel(panel);
>  
> -     for (i = 0; i < CONFIG_FB_AU1200_DEVS; i++) {
> +     for (i = 0; i < device_count; i++) {
>               fbdev = &_au1200fb_devices[i];
>               au1200fb_fb_set_par(&fbdev->fb_info);
>       }
> @@ -1754,10 +1757,10 @@ static struct platform_driver au1200fb_driver = {
>  
>  /* Kernel driver */
>  
> -static void au1200fb_setup(void)
> +static int au1200fb_setup(void)
>  {
> -     char* options = NULL;
> -     char* this_opt;
> +     char *options = NULL;
> +     char *this_opt, *endptr;
>       int num_panels = ARRAY_SIZE(known_lcd_panels);
>       int panel_idx = -1;
>  
> @@ -1802,12 +1805,29 @@ static void au1200fb_setup(void)
>                               nohwcursor = 1;
>                       }
>  
> +                     else if (strncmp(this_opt, "devices:", 8) == 0) {
> +                             this_opt += 8;
> +                             device_count = simple_strtol(this_opt, &endptr, 
> 0);
> +                             if ((device_count < 0) || (device_count > 4))
> +                                     device_count = DEFAULT_DEVICE_COUNT;
> +                     }
> +
> +                     else if (strncmp(this_opt, "wincfg:", 7) == 0) {
> +                             this_opt += 7;
> +                             window_index = simple_strtol(this_opt, &endptr, 
> 0);
> +                             if ((window_index < 0) || (window_index >= 
> ARRAY_SIZE(windows)))
> +                                     window_index = DEFAULT_WINDOW_INDEX;
> +                     }
> +
> +                     else if (strncmp(this_opt, "off", 3) == 0)
> +                             return 1;
>                       /* Unsupported option */
>                       else {
>                               print_warn("Unsupported option \"%s\"", 
> this_opt);
>                       }
>               }
>       }
> +     return 0;
>  }
>  
>  static int __init au1200fb_init(void)
> @@ -1815,7 +1835,14 @@ static int __init au1200fb_init(void)
>       print_info("" DRIVER_DESC "");
>  
>       /* Setup driver with options */
> -     au1200fb_setup();
> +     if (au1200fb_setup())
> +             return -ENODEV;
> +
> +     if ((device_count < 0) || (device_count > 4))
> +             device_count = DEFAULT_DEVICE_COUNT;
> +

The au1200fb_setup does this check before so it is not needed
to repeat it.

The patch looks fine otherwise.

Kind regards,
Krzysztof

----------------------------------------------------------------------
Te newsy nakreca Cie na caly dzien!
Sprawdz >>> http://link.interia.pl/f1e94


<Prev in Thread] Current Thread [Next in Thread>