[U-Boot] [PATCH V2 02/13] lcd: split configuration_get_cmap

[Bo Shen]

Hi Nikita Kiryanov,

On 02/03/2015 07:32 PM, Nikita Kiryanov wrote:
> configuration_get_cmap() is multiple platform-specific functions stuffed into
> one function. Split it into multiple versions, and move each version to the
> appropriate driver to reduce the #ifdef complexity.
>
> Signed-off-by: Nikita Kiryanov <nikita at compulab.co.il>
> Reviewed-by: Simon Glass <sjg at chromium.org>
> Cc: Bo Shen <voice.shen at atmel.com>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Anatolij Gustschin <agust at denx.de>
> ---
> Changes in V2:
> 	- Minor commit message update
> 	- ushort *configuration_get_cmap(void) prototype is defined only once
> 	  instead of for each #ifdef CONFIG_*
>
>   common/lcd.c                 | 19 -------------------
>   drivers/video/atmel_hlcdfb.c | 13 +++++++++++++
>   drivers/video/atmel_lcdfb.c  |  5 +++++
>   drivers/video/exynos_fb.c    |  9 +++++++++
>   drivers/video/mpc8xx_lcd.c   |  7 +++++++
>   drivers/video/pxa_lcd.c      |  6 ++++++
>   include/lcd.h                |  7 +++++++
>   7 files changed, 47 insertions(+), 19 deletions(-)
>
> diff --git a/common/lcd.c b/common/lcd.c
> index 1195a54..0f6c2e4 100644
> --- a/common/lcd.c
> +++ b/common/lcd.c
> @@ -383,25 +383,6 @@ int lcd_getbgcolor(void)
>   /************************************************************************/
>   /* ** Chipset depending Bitmap / Logo stuff...                          */
>   /************************************************************************/
> -static inline ushort *configuration_get_cmap(void)
> -{
> -#if defined CONFIG_CPU_PXA
> -	struct pxafb_info *fbi = &panel_info.pxa;
> -	return (ushort *)fbi->palette;
> -#elif defined(CONFIG_MPC823)
> -	immap_t *immr = (immap_t *) CONFIG_SYS_IMMR;
> -	cpm8xx_t *cp = &(immr->im_cpm);
> -	return (ushort *)&(cp->lcd_cmap[255 * sizeof(ushort)]);
> -#elif defined(CONFIG_ATMEL_LCD)
> -	return (ushort *)(panel_info.mmio + ATMEL_LCDC_LUT(0));
> -#elif !defined(CONFIG_ATMEL_HLCD) && !defined(CONFIG_EXYNOS_FB)
> -	return panel_info.cmap;
> -#elif defined(CONFIG_LCD_LOGO)
> -	return bmp_logo_palette;
> -#else
> -	return NULL;
> -#endif
> -}
>
>   #ifdef CONFIG_LCD_LOGO
>   void bitmap_plot(int x, int y)
> diff --git a/drivers/video/atmel_hlcdfb.c b/drivers/video/atmel_hlcdfb.c
> index 935ae42..0ce2370 100644
> --- a/drivers/video/atmel_hlcdfb.c
> +++ b/drivers/video/atmel_hlcdfb.c
> @@ -13,6 +13,10 @@
>   #include <lcd.h>
>   #include <atmel_hlcdc.h>
>
> +#if defined(CONFIG_LCD_LOGO)
> +#include <bmp_logo.h>
> +#endif

I think we can include this file directly.

>   /* configurable parameters */
>   #define ATMEL_LCDC_CVAL_DEFAULT		0xc8
>   #define ATMEL_LCDC_DMA_BURST_LEN	8
> @@ -37,6 +41,15 @@ void lcd_setcolreg(ushort regno, ushort red, ushort green, ushort blue)
>   		panel_info.mmio + ATMEL_LCDC_LUT(regno));
>   }
>
> +ushort *configuration_get_cmap(void)
> +{
> +#if defined(CONFIG_LCD_LOGO)
> +	return bmp_logo_palette;
> +#else
> +	return NULL;
> +#endif
> +}

Here, I think no need to do the CONFIG_LCD_LOGO check, return 
bmp_logo_palette directly, as it will be used when use bmp command in 8 
bit mode. However, no hardware to test LCDC work or not in 8 bit mode :(

>   void lcd_ctrl_init(void *lcdbase)
>   {
>   	unsigned long value;
> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> index 3cf008c..fa6a82c 100644
> --- a/drivers/video/atmel_lcdfb.c
> +++ b/drivers/video/atmel_lcdfb.c
> @@ -29,6 +29,11 @@
>   #define lcdc_readl(mmio, reg)		__raw_readl((mmio)+(reg))
>   #define lcdc_writel(mmio, reg, val)	__raw_writel((val), (mmio)+(reg))
>
> +ushort *configuration_get_cmap(void)
> +{
> +	return (ushort *)(panel_info.mmio + ATMEL_LCDC_LUT(0));
> +}
> +
>   void lcd_setcolreg(ushort regno, ushort red, ushort green, ushort blue)
>   {
>   #if defined(CONFIG_ATMEL_LCD_BGR555)
> diff --git a/drivers/video/exynos_fb.c b/drivers/video/exynos_fb.c
> index be35b98..c5d7330 100644
> --- a/drivers/video/exynos_fb.c
> +++ b/drivers/video/exynos_fb.c
> @@ -37,6 +37,15 @@ vidinfo_t panel_info  = {
>   };
>   #endif
>
> +ushort *configuration_get_cmap(void)
> +{
> +#if defined(CONFIG_LCD_LOGO)
> +	return bmp_logo_palette;
> +#else
> +	return NULL;
> +#endif
> +}
> +
>   static void exynos_lcd_init_mem(void *lcdbase, vidinfo_t *vid)
>   {
>   	unsigned long palette_size;
> diff --git a/drivers/video/mpc8xx_lcd.c b/drivers/video/mpc8xx_lcd.c
> index add7215..9d2e5ed 100644
> --- a/drivers/video/mpc8xx_lcd.c
> +++ b/drivers/video/mpc8xx_lcd.c
> @@ -357,6 +357,13 @@ lcd_setcolreg (ushort regno, ushort red, ushort green, ushort blue)
>
>   /*----------------------------------------------------------------------*/
>
> +ushort *configuration_get_cmap(void)
> +{
> +	immap_t *immr = (immap_t *)CONFIG_SYS_IMMR;
> +	cpm8xx_t *cp = &(immr->im_cpm);
> +	return (ushort *)&(cp->lcd_cmap[255 * sizeof(ushort)]);
> +}
> +
>   void lcd_enable (void)
>   {
>   	volatile immap_t *immr = (immap_t *) CONFIG_SYS_IMMR;
> diff --git a/drivers/video/pxa_lcd.c b/drivers/video/pxa_lcd.c
> index f66f615..04105d4 100644
> --- a/drivers/video/pxa_lcd.c
> +++ b/drivers/video/pxa_lcd.c
> @@ -342,6 +342,12 @@ static int pxafb_init (vidinfo_t *vid);
>   /* ---------------  PXA chipset specific functions  ------------------- */
>   /************************************************************************/
>
> +ushort *configuration_get_cmap(void)
> +{
> +	struct pxafb_info *fbi = &panel_info.pxa;
> +	return (ushort *)fbi->palette;
> +}
> +
>   void lcd_ctrl_init (void *lcdbase)
>   {
>   	pxafb_init_mem(lcdbase, &panel_info);
> diff --git a/include/lcd.h b/include/lcd.h
> index fbba6a2..4ada617 100644
> --- a/include/lcd.h
> +++ b/include/lcd.h
> @@ -60,8 +60,15 @@ typedef struct vidinfo {
>
>   	void	*priv;		/* Pointer to driver-specific data */
>   } vidinfo_t;
> +
> +static __maybe_unused ushort *configuration_get_cmap(void)
> +{
> +	return panel_info.cmap;
> +}
>   #endif
>
> +ushort *configuration_get_cmap(void);
> +
>   extern vidinfo_t panel_info;
>
>   /* Video functions */
>

Best Regards,
Bo Shen