Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH 1/8] video: atmel_lcdfb: fix platform data struct
From: Nicolas Ferre @ 2013-04-16 12:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1365692422-9565-1-git-send-email-plagnioj@jcrosoft.com>

On 04/11/2013 05:00 PM, Jean-Christophe PLAGNIOL-VILLARD :
> Today we mix pdata and drivers data in the struct atmel_lcdfb_info
> Fix it and introduce a new struct atmel_lcdfb_pdata for platform data only
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Hans-Christian Egtvedt <egtvedt@samfundet.no>
> ---
>  arch/arm/mach-at91/at91sam9261_devices.c    |    6 +-
>  arch/arm/mach-at91/at91sam9263_devices.c    |    6 +-
>  arch/arm/mach-at91/at91sam9g45_devices.c    |    6 +-
>  arch/arm/mach-at91/at91sam9rl_devices.c     |    6 +-
>  arch/arm/mach-at91/board-sam9261ek.c        |    6 +-
>  arch/arm/mach-at91/board-sam9263ek.c        |    4 +-
>  arch/arm/mach-at91/board-sam9m10g45ek.c     |    4 +-
>  arch/arm/mach-at91/board-sam9rlek.c         |    4 +-
>  arch/arm/mach-at91/board.h                  |    4 +-
>  arch/avr32/boards/atngw100/evklcd10x.c      |    6 +-
>  arch/avr32/boards/atngw100/mrmt.c           |    4 +-
>  arch/avr32/boards/atstk1000/atstk1000.h     |    2 +-
>  arch/avr32/boards/atstk1000/setup.c         |    2 +-
>  arch/avr32/boards/favr-32/setup.c           |    2 +-
>  arch/avr32/boards/hammerhead/setup.c        |    2 +-
>  arch/avr32/boards/merisc/display.c          |    2 +-
>  arch/avr32/boards/mimc200/setup.c           |    4 +-
>  arch/avr32/mach-at32ap/at32ap700x.c         |    8 +--
>  arch/avr32/mach-at32ap/include/mach/board.h |    4 +-
>  drivers/video/atmel_lcdfb.c                 |  104 +++++++++++++++++----------
>  include/video/atmel_lcdc.h                  |   24 +------
>  21 files changed, 109 insertions(+), 101 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/at91sam9261_devices.c b/arch/arm/mach-at91/at91sam9261_devices.c
> index 629ea5f..b2a3474 100644
> --- a/arch/arm/mach-at91/at91sam9261_devices.c
> +++ b/arch/arm/mach-at91/at91sam9261_devices.c
> @@ -465,7 +465,7 @@ void __init at91_add_device_spi(struct spi_board_info *devices, int nr_devices)
>  
>  #if defined(CONFIG_FB_ATMEL) || defined(CONFIG_FB_ATMEL_MODULE)
>  static u64 lcdc_dmamask = DMA_BIT_MASK(32);
> -static struct atmel_lcdfb_info lcdc_data;
> +static struct atmel_lcdfb_pdata lcdc_data;
>  
>  static struct resource lcdc_resources[] = {
>  	[0] = {
> @@ -498,7 +498,7 @@ static struct platform_device at91_lcdc_device = {
>  	.num_resources	= ARRAY_SIZE(lcdc_resources),
>  };
>  
> -void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data)
> +void __init at91_add_device_lcdc(struct atmel_lcdfb_pdata *data)
>  {
>  	if (!data) {
>  		return;
> @@ -559,7 +559,7 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data)
>  	platform_device_register(&at91_lcdc_device);
>  }
>  #else
> -void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data) {}
> +void __init at91_add_device_lcdc(struct atmel_lcdfb_pdata *data) {}
>  #endif
>  
>  
> diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
> index 858c8aa..4aeaddd 100644
> --- a/arch/arm/mach-at91/at91sam9263_devices.c
> +++ b/arch/arm/mach-at91/at91sam9263_devices.c
> @@ -832,7 +832,7 @@ void __init at91_add_device_can(struct at91_can_data *data) {}
>  
>  #if defined(CONFIG_FB_ATMEL) || defined(CONFIG_FB_ATMEL_MODULE)
>  static u64 lcdc_dmamask = DMA_BIT_MASK(32);
> -static struct atmel_lcdfb_info lcdc_data;
> +static struct atmel_lcdfb_pdata lcdc_data;
>  
>  static struct resource lcdc_resources[] = {
>  	[0] = {
> @@ -859,7 +859,7 @@ static struct platform_device at91_lcdc_device = {
>  	.num_resources	= ARRAY_SIZE(lcdc_resources),
>  };
>  
> -void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data)
> +void __init at91_add_device_lcdc(struct atmel_lcdfb_pdata *data)
>  {
>  	if (!data)
>  		return;
> @@ -891,7 +891,7 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data)
>  	platform_device_register(&at91_lcdc_device);
>  }
>  #else
> -void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data) {}
> +void __init at91_add_device_lcdc(struct atmel_lcdfb_pdata *data) {}
>  #endif
>  
>  
> diff --git a/arch/arm/mach-at91/at91sam9g45_devices.c b/arch/arm/mach-at91/at91sam9g45_devices.c
> index fe626d4..82636c7 100644
> --- a/arch/arm/mach-at91/at91sam9g45_devices.c
> +++ b/arch/arm/mach-at91/at91sam9g45_devices.c
> @@ -965,7 +965,7 @@ void __init at91_add_device_isi(struct isi_platform_data *data,
>  
>  #if defined(CONFIG_FB_ATMEL) || defined(CONFIG_FB_ATMEL_MODULE)
>  static u64 lcdc_dmamask = DMA_BIT_MASK(32);
> -static struct atmel_lcdfb_info lcdc_data;
> +static struct atmel_lcdfb_pdata lcdc_data;
>  
>  static struct resource lcdc_resources[] = {
>  	[0] = {
> @@ -991,7 +991,7 @@ static struct platform_device at91_lcdc_device = {
>  	.num_resources	= ARRAY_SIZE(lcdc_resources),
>  };
>  
> -void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data)
> +void __init at91_add_device_lcdc(struct atmel_lcdfb_pdata *data)
>  {
>  	if (!data)
>  		return;
> @@ -1037,7 +1037,7 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data)
>  	platform_device_register(&at91_lcdc_device);
>  }
>  #else
> -void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data) {}
> +void __init at91_add_device_lcdc(struct atmel_lcdfb_pdata *data) {}
>  #endif
>  
>  
> diff --git a/arch/arm/mach-at91/at91sam9rl_devices.c b/arch/arm/mach-at91/at91sam9rl_devices.c
> index 352468f..a698bda 100644
> --- a/arch/arm/mach-at91/at91sam9rl_devices.c
> +++ b/arch/arm/mach-at91/at91sam9rl_devices.c
> @@ -498,7 +498,7 @@ void __init at91_add_device_ac97(struct ac97c_platform_data *data) {}
>  
>  #if defined(CONFIG_FB_ATMEL) || defined(CONFIG_FB_ATMEL_MODULE)
>  static u64 lcdc_dmamask = DMA_BIT_MASK(32);
> -static struct atmel_lcdfb_info lcdc_data;
> +static struct atmel_lcdfb_pdata lcdc_data;
>  
>  static struct resource lcdc_resources[] = {
>  	[0] = {
> @@ -525,7 +525,7 @@ static struct platform_device at91_lcdc_device = {
>  	.num_resources	= ARRAY_SIZE(lcdc_resources),
>  };
>  
> -void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data)
> +void __init at91_add_device_lcdc(struct atmel_lcdfb_pdata *data)
>  {
>  	if (!data) {
>  		return;
> @@ -557,7 +557,7 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data)
>  	platform_device_register(&at91_lcdc_device);
>  }
>  #else
> -void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data) {}
> +void __init at91_add_device_lcdc(struct atmel_lcdfb_pdata *data) {}
>  #endif
>  
>  
> diff --git a/arch/arm/mach-at91/board-sam9261ek.c b/arch/arm/mach-at91/board-sam9261ek.c
> index b446645..c819e29 100644
> --- a/arch/arm/mach-at91/board-sam9261ek.c
> +++ b/arch/arm/mach-at91/board-sam9261ek.c
> @@ -405,7 +405,7 @@ static void at91_lcdc_stn_power_control(int on)
>  	}
>  }
>  
> -static struct atmel_lcdfb_info __initdata ek_lcdc_data = {
> +static struct atmel_lcdfb_pdata __initdata ek_lcdc_data = {
>  	.default_bpp			= 1,
>  	.default_dmacon			= ATMEL_LCDC_DMAEN,
>  	.default_lcdcon2		= AT91SAM9261_DEFAULT_STN_LCDCON2,
> @@ -460,7 +460,7 @@ static void at91_lcdc_tft_power_control(int on)
>  		at91_set_gpio_value(AT91_PIN_PA12, 1);	/* power down */
>  }
>  
> -static struct atmel_lcdfb_info __initdata ek_lcdc_data = {
> +static struct atmel_lcdfb_pdata __initdata ek_lcdc_data = {
>  	.lcdcon_is_backlight		= true,
>  	.default_bpp			= 16,
>  	.default_dmacon			= ATMEL_LCDC_DMAEN,
> @@ -475,7 +475,7 @@ static struct atmel_lcdfb_info __initdata ek_lcdc_data = {
>  #endif
>  
>  #else
> -static struct atmel_lcdfb_info __initdata ek_lcdc_data;
> +static struct atmel_lcdfb_pdata __initdata ek_lcdc_data;
>  #endif
>  
>  
> diff --git a/arch/arm/mach-at91/board-sam9263ek.c b/arch/arm/mach-at91/board-sam9263ek.c
> index 3284df0..0fdae3f 100644
> --- a/arch/arm/mach-at91/board-sam9263ek.c
> +++ b/arch/arm/mach-at91/board-sam9263ek.c
> @@ -281,7 +281,7 @@ static void at91_lcdc_power_control(int on)
>  }
>  
>  /* Driver datas */
> -static struct atmel_lcdfb_info __initdata ek_lcdc_data = {
> +static struct atmel_lcdfb_pdata __initdata ek_lcdc_data = {
>  	.lcdcon_is_backlight		= true,
>  	.default_bpp			= 16,
>  	.default_dmacon			= ATMEL_LCDC_DMAEN,
> @@ -292,7 +292,7 @@ static struct atmel_lcdfb_info __initdata ek_lcdc_data = {
>  };
>  
>  #else
> -static struct atmel_lcdfb_info __initdata ek_lcdc_data;
> +static struct atmel_lcdfb_pdata __initdata ek_lcdc_data;
>  #endif
>  
>  
> diff --git a/arch/arm/mach-at91/board-sam9m10g45ek.c b/arch/arm/mach-at91/board-sam9m10g45ek.c
> index 2a94896..ef39078 100644
> --- a/arch/arm/mach-at91/board-sam9m10g45ek.c
> +++ b/arch/arm/mach-at91/board-sam9m10g45ek.c
> @@ -284,7 +284,7 @@ static struct fb_monspecs at91fb_default_monspecs = {
>  					| ATMEL_LCDC_CLKMOD_ALWAYSACTIVE)
>  
>  /* Driver datas */
> -static struct atmel_lcdfb_info __initdata ek_lcdc_data = {
> +static struct atmel_lcdfb_pdata __initdata ek_lcdc_data = {
>  	.lcdcon_is_backlight		= true,
>  	.default_bpp			= 32,
>  	.default_dmacon			= ATMEL_LCDC_DMAEN,
> @@ -295,7 +295,7 @@ static struct atmel_lcdfb_info __initdata ek_lcdc_data = {
>  };
>  
>  #else
> -static struct atmel_lcdfb_info __initdata ek_lcdc_data;
> +static struct atmel_lcdfb_pdata __initdata ek_lcdc_data;
>  #endif
>  
>  
> diff --git a/arch/arm/mach-at91/board-sam9rlek.c b/arch/arm/mach-at91/board-sam9rlek.c
> index aa265dc..b77d7a9 100644
> --- a/arch/arm/mach-at91/board-sam9rlek.c
> +++ b/arch/arm/mach-at91/board-sam9rlek.c
> @@ -179,7 +179,7 @@ static void at91_lcdc_power_control(int on)
>  }
>  
>  /* Driver datas */
> -static struct atmel_lcdfb_info __initdata ek_lcdc_data = {
> +static struct atmel_lcdfb_pdata __initdata ek_lcdc_data = {
>  	.lcdcon_is_backlight            = true,
>  	.default_bpp			= 16,
>  	.default_dmacon			= ATMEL_LCDC_DMAEN,
> @@ -191,7 +191,7 @@ static struct atmel_lcdfb_info __initdata ek_lcdc_data = {
>  };
>  
>  #else
> -static struct atmel_lcdfb_info __initdata ek_lcdc_data;
> +static struct atmel_lcdfb_pdata __initdata ek_lcdc_data;
>  #endif
>  
>  
> diff --git a/arch/arm/mach-at91/board.h b/arch/arm/mach-at91/board.h
> index 4a234fb..6c08b34 100644
> --- a/arch/arm/mach-at91/board.h
> +++ b/arch/arm/mach-at91/board.h
> @@ -107,8 +107,8 @@ extern void __init at91_add_device_pwm(u32 mask);
>  extern void __init at91_add_device_ssc(unsigned id, unsigned pins);
>  
>   /* LCD Controller */
> -struct atmel_lcdfb_info;
> -extern void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data);
> +struct atmel_lcdfb_pdata;
> +extern void __init at91_add_device_lcdc(struct atmel_lcdfb_pdata *data);
>  
>   /* AC97 */
>  extern void __init at91_add_device_ac97(struct ac97c_platform_data *data);
> diff --git a/arch/avr32/boards/atngw100/evklcd10x.c b/arch/avr32/boards/atngw100/evklcd10x.c
> index 2038875..dc42804 100644
> --- a/arch/avr32/boards/atngw100/evklcd10x.c
> +++ b/arch/avr32/boards/atngw100/evklcd10x.c
> @@ -58,7 +58,7 @@ static struct fb_monspecs __initdata atevklcd10x_default_monspecs = {
>  	.dclkmax		= 28330000,
>  };
>  
> -static struct atmel_lcdfb_info __initdata atevklcd10x_lcdc_data = {
> +static struct atmel_lcdfb_pdata __initdata atevklcd10x_lcdc_data = {
>  	.default_bpp		= 16,
>  	.default_dmacon		= ATMEL_LCDC_DMAEN | ATMEL_LCDC_DMA2DEN,
>  	.default_lcdcon2	= (ATMEL_LCDC_DISTYPE_TFT
> @@ -96,7 +96,7 @@ static struct fb_monspecs __initdata atevklcd10x_default_monspecs = {
>  	.dclkmax		= 7000000,
>  };
>  
> -static struct atmel_lcdfb_info __initdata atevklcd10x_lcdc_data = {
> +static struct atmel_lcdfb_pdata __initdata atevklcd10x_lcdc_data = {
>  	.default_bpp		= 16,
>  	.default_dmacon		= ATMEL_LCDC_DMAEN | ATMEL_LCDC_DMA2DEN,
>  	.default_lcdcon2	= (ATMEL_LCDC_DISTYPE_TFT
> @@ -134,7 +134,7 @@ static struct fb_monspecs __initdata atevklcd10x_default_monspecs = {
>  	.dclkmax		= 6400000,
>  };
>  
> -static struct atmel_lcdfb_info __initdata atevklcd10x_lcdc_data = {
> +static struct atmel_lcdfb_pdata __initdata atevklcd10x_lcdc_data = {
>  	.default_bpp		= 16,
>  	.default_dmacon		= ATMEL_LCDC_DMAEN | ATMEL_LCDC_DMA2DEN,
>  	.default_lcdcon2	= (ATMEL_LCDC_DISTYPE_TFT
> diff --git a/arch/avr32/boards/atngw100/mrmt.c b/arch/avr32/boards/atngw100/mrmt.c
> index f914319..ccc9599 100644
> --- a/arch/avr32/boards/atngw100/mrmt.c
> +++ b/arch/avr32/boards/atngw100/mrmt.c
> @@ -83,7 +83,7 @@ static struct fb_monspecs __initdata lcd_fb_default_monspecs = {
>  	.dclkmax		= 9260000,
>  };
>  
> -static struct atmel_lcdfb_info __initdata rmt_lcdc_data = {
> +static struct atmel_lcdfb_pdata __initdata rmt_lcdc_data = {
>  	.default_bpp		= 24,
>  	.default_dmacon		= ATMEL_LCDC_DMAEN | ATMEL_LCDC_DMA2DEN,
>  	.default_lcdcon2	= (ATMEL_LCDC_DISTYPE_TFT
> @@ -126,7 +126,7 @@ static struct fb_monspecs __initdata lcd_fb_default_monspecs = {
>  	.dclkmax		= 9260000,
>  };
>  
> -static struct atmel_lcdfb_info __initdata rmt_lcdc_data = {
> +static struct atmel_lcdfb_pdata __initdata rmt_lcdc_data = {
>  	.default_bpp		= 24,
>  	.default_dmacon		= ATMEL_LCDC_DMAEN | ATMEL_LCDC_DMA2DEN,
>  	.default_lcdcon2	= (ATMEL_LCDC_DISTYPE_TFT
> diff --git a/arch/avr32/boards/atstk1000/atstk1000.h b/arch/avr32/boards/atstk1000/atstk1000.h
> index 9392d32..653cc09 100644
> --- a/arch/avr32/boards/atstk1000/atstk1000.h
> +++ b/arch/avr32/boards/atstk1000/atstk1000.h
> @@ -10,7 +10,7 @@
>  #ifndef __ARCH_AVR32_BOARDS_ATSTK1000_ATSTK1000_H
>  #define __ARCH_AVR32_BOARDS_ATSTK1000_ATSTK1000_H
>  
> -extern struct atmel_lcdfb_info atstk1000_lcdc_data;
> +extern struct atmel_lcdfb_pdata atstk1000_lcdc_data;
>  
>  void atstk1000_setup_j2_leds(void);
>  
> diff --git a/arch/avr32/boards/atstk1000/setup.c b/arch/avr32/boards/atstk1000/setup.c
> index 2d6b560..b6b88f5 100644
> --- a/arch/avr32/boards/atstk1000/setup.c
> +++ b/arch/avr32/boards/atstk1000/setup.c
> @@ -55,7 +55,7 @@ static struct fb_monspecs __initdata atstk1000_default_monspecs = {
>  	.dclkmax		= 30000000,
>  };
>  
> -struct atmel_lcdfb_info __initdata atstk1000_lcdc_data = {
> +struct atmel_lcdfb_pdata __initdata atstk1000_lcdc_data = {
>  	.default_bpp		= 24,
>  	.default_dmacon		= ATMEL_LCDC_DMAEN | ATMEL_LCDC_DMA2DEN,
>  	.default_lcdcon2	= (ATMEL_LCDC_DISTYPE_TFT
> diff --git a/arch/avr32/boards/favr-32/setup.c b/arch/avr32/boards/favr-32/setup.c
> index 27bd6fb..7b1f2cd 100644
> --- a/arch/avr32/boards/favr-32/setup.c
> +++ b/arch/avr32/boards/favr-32/setup.c
> @@ -125,7 +125,7 @@ static struct fb_monspecs __initdata favr32_default_monspecs = {
>  	.dclkmax		= 28000000,
>  };
>  
> -struct atmel_lcdfb_info __initdata favr32_lcdc_data = {
> +struct atmel_lcdfb_pdata __initdata favr32_lcdc_data = {
>  	.default_bpp		= 16,
>  	.default_dmacon		= ATMEL_LCDC_DMAEN | ATMEL_LCDC_DMA2DEN,
>  	.default_lcdcon2	= (ATMEL_LCDC_DISTYPE_TFT
> diff --git a/arch/avr32/boards/hammerhead/setup.c b/arch/avr32/boards/hammerhead/setup.c
> index 9d1efd1..dc0e317 100644
> --- a/arch/avr32/boards/hammerhead/setup.c
> +++ b/arch/avr32/boards/hammerhead/setup.c
> @@ -77,7 +77,7 @@ static struct fb_monspecs __initdata hammerhead_hda350t_monspecs = {
>  	.dclkmax		= 10000000,
>  };
>  
> -struct atmel_lcdfb_info __initdata hammerhead_lcdc_data = {
> +struct atmel_lcdfb_pdata __initdata hammerhead_lcdc_data = {
>  	.default_bpp		= 24,
>  	.default_dmacon		= ATMEL_LCDC_DMAEN | ATMEL_LCDC_DMA2DEN,
>  	.default_lcdcon2	= (ATMEL_LCDC_DISTYPE_TFT
> diff --git a/arch/avr32/boards/merisc/display.c b/arch/avr32/boards/merisc/display.c
> index 85a543c..e7683ee 100644
> --- a/arch/avr32/boards/merisc/display.c
> +++ b/arch/avr32/boards/merisc/display.c
> @@ -45,7 +45,7 @@ static struct fb_monspecs merisc_fb_monspecs = {
>  	.dclkmax	= 30000000,
>  };
>  
> -struct atmel_lcdfb_info merisc_lcdc_data = {
> +struct atmel_lcdfb_pdata merisc_lcdc_data = {
>  	.default_bpp		= 24,
>  	.default_dmacon		= ATMEL_LCDC_DMAEN | ATMEL_LCDC_DMA2DEN,
>  	.default_lcdcon2	= (ATMEL_LCDC_DISTYPE_TFT
> diff --git a/arch/avr32/boards/mimc200/setup.c b/arch/avr32/boards/mimc200/setup.c
> index 05358aa..1cb8e9c 100644
> --- a/arch/avr32/boards/mimc200/setup.c
> +++ b/arch/avr32/boards/mimc200/setup.c
> @@ -8,7 +8,7 @@
>   * published by the Free Software Foundation.
>   */
>  
> -extern struct atmel_lcdfb_info mimc200_lcdc_data;
> +extern struct atmel_lcdfb_pdata mimc200_lcdc_data;
>  
>  #include <linux/clk.h>
>  #include <linux/etherdevice.h>
> @@ -71,7 +71,7 @@ static struct fb_monspecs __initdata mimc200_default_monspecs = {
>  	.dclkmax		= 25200000,
>  };
>  
> -struct atmel_lcdfb_info __initdata mimc200_lcdc_data = {
> +struct atmel_lcdfb_pdata __initdata mimc200_lcdc_data = {
>  	.default_bpp		= 16,
>  	.default_dmacon		= ATMEL_LCDC_DMAEN | ATMEL_LCDC_DMA2DEN,
>  	.default_lcdcon2	= (ATMEL_LCDC_DISTYPE_TFT
> diff --git a/arch/avr32/mach-at32ap/at32ap700x.c b/arch/avr32/mach-at32ap/at32ap700x.c
> index 7c2f668..0badb05 100644
> --- a/arch/avr32/mach-at32ap/at32ap700x.c
> +++ b/arch/avr32/mach-at32ap/at32ap700x.c
> @@ -1437,7 +1437,7 @@ fail:
>   *  LCDC
>   * -------------------------------------------------------------------- */
>  #if defined(CONFIG_CPU_AT32AP7000) || defined(CONFIG_CPU_AT32AP7002)
> -static struct atmel_lcdfb_info atmel_lcdfb0_data;
> +static struct atmel_lcdfb_pdata atmel_lcdfb0_data;
>  static struct resource atmel_lcdfb0_resource[] = {
>  	{
>  		.start		= 0xff000000,
> @@ -1465,12 +1465,12 @@ static struct clk atmel_lcdfb0_pixclk = {
>  };
>  
>  struct platform_device *__init
> -at32_add_device_lcdc(unsigned int id, struct atmel_lcdfb_info *data,
> +at32_add_device_lcdc(unsigned int id, struct atmel_lcdfb_pdata *data,
>  		     unsigned long fbmem_start, unsigned long fbmem_len,
>  		     u64 pin_mask)
>  {
>  	struct platform_device *pdev;
> -	struct atmel_lcdfb_info *info;
> +	struct atmel_lcdfb_pdata *info;
>  	struct fb_monspecs *monspecs;
>  	struct fb_videomode *modedb;
>  	unsigned int modedb_size;
> @@ -1527,7 +1527,7 @@ at32_add_device_lcdc(unsigned int id, struct atmel_lcdfb_info *data,
>  	}
>  
>  	info = pdev->dev.platform_data;
> -	memcpy(info, data, sizeof(struct atmel_lcdfb_info));
> +	memcpy(info, data, sizeof(struct atmel_lcdfb_pdata));
>  	info->default_monspecs = monspecs;
>  
>  	pdev->name = "at32ap-lcdfb";
> diff --git a/arch/avr32/mach-at32ap/include/mach/board.h b/arch/avr32/mach-at32ap/include/mach/board.h
> index d485b03..f1a316d 100644
> --- a/arch/avr32/mach-at32ap/include/mach/board.h
> +++ b/arch/avr32/mach-at32ap/include/mach/board.h
> @@ -44,9 +44,9 @@ struct platform_device *
>  at32_add_device_spi(unsigned int id, struct spi_board_info *b, unsigned int n);
>  void at32_spi_setup_slaves(unsigned int bus_num, struct spi_board_info *b, unsigned int n);
>  
> -struct atmel_lcdfb_info;
> +struct atmel_lcdfb_pdata;
>  struct platform_device *
> -at32_add_device_lcdc(unsigned int id, struct atmel_lcdfb_info *data,
> +at32_add_device_lcdc(unsigned int id, struct atmel_lcdfb_pdata *data,
>  		     unsigned long fbmem_start, unsigned long fbmem_len,
>  		     u64 pin_mask);
>  
> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> index c1a2914..98733cd4 100644
> --- a/drivers/video/atmel_lcdfb.c
> +++ b/drivers/video/atmel_lcdfb.c
> @@ -20,12 +20,45 @@
>  #include <linux/gfp.h>
>  #include <linux/module.h>
>  #include <linux/platform_data/atmel.h>
> +#include <video/of_display_timing.h>

I am not sure this one is useful for this patch, maybe place it in the
4/8 one.


>  #include <mach/cpu.h>
>  #include <asm/gpio.h>
>  
>  #include <video/atmel_lcdc.h>
>  
> +struct atmel_lcdfb_config {
> +	bool have_alt_pixclock;
> +	bool have_hozval;
> +	bool have_intensity_bit;
> +};
> +
> + /* LCD Controller info data structure, stored in device platform_data */

Is comment still relevant?

> +struct atmel_lcdfb_info {
> +	spinlock_t		lock;
> +	struct fb_info		*info;
> +	void __iomem		*mmio;
> +	int			irq_base;
> +	struct work_struct	task;
> +
> +	unsigned int		smem_len;
> +	struct platform_device	*pdev;
> +	struct clk		*bus_clk;
> +	struct clk		*lcdc_clk;
> +
> +	struct backlight_device	*backlight;
> +	u8			bl_power;
> +	bool			lcdcon_pol_negative;
> +	u8			saved_lcdcon;
> +
> +	u32			pseudo_palette[16];
> +	bool			have_intensity_bit;
> +
> +	struct atmel_lcdfb_pdata pdata;
> +
> +	struct atmel_lcdfb_config *config;
> +};
> +
>  #define lcdc_readl(sinfo, reg)		__raw_readl((sinfo)->mmio+(reg))
>  #define lcdc_writel(sinfo, reg, val)	__raw_writel((val), (sinfo)->mmio+(reg))
>  
> @@ -34,12 +67,6 @@
>  #define ATMEL_LCDC_DMA_BURST_LEN	8	/* words */
>  #define ATMEL_LCDC_FIFO_SIZE		512	/* words */
>  
> -struct atmel_lcdfb_config {
> -	bool have_alt_pixclock;
> -	bool have_hozval;
> -	bool have_intensity_bit;
> -};
> -
>  static struct atmel_lcdfb_config at91sam9261_config = {
>  	.have_hozval		= true,
>  	.have_intensity_bit	= true,
> @@ -242,6 +269,8 @@ static void exit_backlight(struct atmel_lcdfb_info *sinfo)
>  
>  static void init_contrast(struct atmel_lcdfb_info *sinfo)
>  {
> +	struct atmel_lcdfb_pdata *pdata = &sinfo->pdata;
> +
>  	/* contrast pwm can be 'inverted' */
>  	if (sinfo->lcdcon_pol_negative)
>  			contrast_ctr &= ~(ATMEL_LCDC_POL_POSITIVE);
> @@ -250,7 +279,7 @@ static void init_contrast(struct atmel_lcdfb_info *sinfo)
>  	lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, contrast_ctr);
>  	lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_VAL, ATMEL_LCDC_CVAL_DEFAULT);
>  
> -	if (sinfo->lcdcon_is_backlight)
> +	if (pdata->lcdcon_is_backlight)
>  		init_backlight(sinfo);
>  }
>  
> @@ -293,9 +322,11 @@ static unsigned long compute_hozval(struct atmel_lcdfb_info *sinfo,
>  
>  static void atmel_lcdfb_stop_nowait(struct atmel_lcdfb_info *sinfo)
>  {
> +	struct atmel_lcdfb_pdata *pdata = &sinfo->pdata;
> +
>  	/* Turn off the LCD controller and the DMA controller */
>  	lcdc_writel(sinfo, ATMEL_LCDC_PWRCON,
> -			sinfo->guard_time << ATMEL_LCDC_GUARDT_OFFSET);
> +			pdata->guard_time << ATMEL_LCDC_GUARDT_OFFSET);
>  
>  	/* Wait for the LCDC core to become idle */
>  	while (lcdc_readl(sinfo, ATMEL_LCDC_PWRCON) & ATMEL_LCDC_BUSY)
> @@ -315,9 +346,11 @@ static void atmel_lcdfb_stop(struct atmel_lcdfb_info *sinfo)
>  
>  static void atmel_lcdfb_start(struct atmel_lcdfb_info *sinfo)
>  {
> -	lcdc_writel(sinfo, ATMEL_LCDC_DMACON, sinfo->default_dmacon);
> +	struct atmel_lcdfb_pdata *pdata = &sinfo->pdata;
> +
> +	lcdc_writel(sinfo, ATMEL_LCDC_DMACON, pdata->default_dmacon);
>  	lcdc_writel(sinfo, ATMEL_LCDC_PWRCON,
> -		(sinfo->guard_time << ATMEL_LCDC_GUARDT_OFFSET)
> +		(pdata->guard_time << ATMEL_LCDC_GUARDT_OFFSET)
>  		| ATMEL_LCDC_PWR);
>  }
>  
> @@ -418,6 +451,7 @@ static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var,
>  {
>  	struct device *dev = info->device;
>  	struct atmel_lcdfb_info *sinfo = info->par;
> +	struct atmel_lcdfb_pdata *pdata = &sinfo->pdata;
>  	unsigned long clk_value_khz;
>  
>  	clk_value_khz = clk_get_rate(sinfo->lcdc_clk) / 1000;
> @@ -501,7 +535,7 @@ static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var,
>  		else
>  			var->green.length = 6;
>  
> -		if (sinfo->lcd_wiring_mode = ATMEL_LCDC_WIRING_RGB) {
> +		if (pdata->lcd_wiring_mode = ATMEL_LCDC_WIRING_RGB) {
>  			/* RGB:5X5 mode */
>  			var->red.offset = var->green.length + 5;
>  			var->blue.offset = 0;
> @@ -518,7 +552,7 @@ static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var,
>  		var->transp.length = 8;
>  		/* fall through */
>  	case 24:
> -		if (sinfo->lcd_wiring_mode = ATMEL_LCDC_WIRING_RGB) {
> +		if (pdata->lcd_wiring_mode = ATMEL_LCDC_WIRING_RGB) {
>  			/* RGB:888 mode */
>  			var->red.offset = 16;
>  			var->blue.offset = 0;
> @@ -567,6 +601,7 @@ static void atmel_lcdfb_reset(struct atmel_lcdfb_info *sinfo)
>  static int atmel_lcdfb_set_par(struct fb_info *info)
>  {
>  	struct atmel_lcdfb_info *sinfo = info->par;
> +	struct atmel_lcdfb_pdata *pdata = &sinfo->pdata;
>  	unsigned long hozval_linesz;
>  	unsigned long value;
>  	unsigned long clk_value_khz;
> @@ -628,7 +663,7 @@ static int atmel_lcdfb_set_par(struct fb_info *info)
>  
>  
>  	/* Initialize control register 2 */
> -	value = sinfo->default_lcdcon2;
> +	value = pdata->default_lcdcon2;
>  
>  	if (!(info->var.sync & FB_SYNC_HOR_HIGH_ACT))
>  		value |= ATMEL_LCDC_INVLINE_INVERTED;
> @@ -732,6 +767,7 @@ static int atmel_lcdfb_setcolreg(unsigned int regno, unsigned int red,
>  			     unsigned int transp, struct fb_info *info)
>  {
>  	struct atmel_lcdfb_info *sinfo = info->par;
> +	struct atmel_lcdfb_pdata *pdata = &sinfo->pdata;
>  	unsigned int val;
>  	u32 *pal;
>  	int ret = 1;
> @@ -768,8 +804,7 @@ static int atmel_lcdfb_setcolreg(unsigned int regno, unsigned int red,
>  				 */
>  			} else {
>  				/* new style BGR:565 / RGB:565 */
> -				if (sinfo->lcd_wiring_mode =
> -				    ATMEL_LCDC_WIRING_RGB) {
> +				if (pdata->lcd_wiring_mode = ATMEL_LCDC_WIRING_RGB) {
>  					val  = ((blue >> 11) & 0x001f);
>  					val |= ((red  >>  0) & 0xf800);
>  				} else {
> @@ -909,7 +944,7 @@ static int __init atmel_lcdfb_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct fb_info *info;
>  	struct atmel_lcdfb_info *sinfo;
> -	struct atmel_lcdfb_info *pdata_sinfo;
> +	struct atmel_lcdfb_pdata *pdata;
>  	struct fb_videomode fbmode;
>  	struct resource *regs = NULL;
>  	struct resource *map = NULL;
> @@ -927,17 +962,8 @@ static int __init atmel_lcdfb_probe(struct platform_device *pdev)
>  	sinfo = info->par;
>  
>  	if (dev->platform_data) {
> -		pdata_sinfo = (struct atmel_lcdfb_info *)dev->platform_data;
> -		sinfo->default_bpp = pdata_sinfo->default_bpp;
> -		sinfo->default_dmacon = pdata_sinfo->default_dmacon;
> -		sinfo->default_lcdcon2 = pdata_sinfo->default_lcdcon2;
> -		sinfo->default_monspecs = pdata_sinfo->default_monspecs;
> -		sinfo->atmel_lcdfb_power_control = pdata_sinfo->atmel_lcdfb_power_control;
> -		sinfo->guard_time = pdata_sinfo->guard_time;
> -		sinfo->smem_len = pdata_sinfo->smem_len;
> -		sinfo->lcdcon_is_backlight = pdata_sinfo->lcdcon_is_backlight;
> -		sinfo->lcdcon_pol_negative = pdata_sinfo->lcdcon_pol_negative;
> -		sinfo->lcd_wiring_mode = pdata_sinfo->lcd_wiring_mode;
> +		pdata = (struct atmel_lcdfb_pdata *)dev->platform_data;
> +		sinfo->pdata = *pdata;
>  	} else {
>  		dev_err(dev, "cannot get default configuration\n");
>  		goto free_info;
> @@ -953,7 +979,7 @@ static int __init atmel_lcdfb_probe(struct platform_device *pdev)
>  	info->pseudo_palette = sinfo->pseudo_palette;
>  	info->fbops = &atmel_lcdfb_ops;
>  
> -	memcpy(&info->monspecs, sinfo->default_monspecs, sizeof(info->monspecs));
> +	memcpy(&info->monspecs, pdata->default_monspecs, sizeof(info->monspecs));
>  	info->fix = atmel_lcdfb_fix;
>  
>  	/* Enable LCDC Clocks */
> @@ -971,7 +997,7 @@ static int __init atmel_lcdfb_probe(struct platform_device *pdev)
>  
>  	ret = fb_find_mode(&info->var, info, NULL, info->monspecs.modedb,
>  			info->monspecs.modedb_len, info->monspecs.modedb,
> -			sinfo->default_bpp);
> +			pdata->default_bpp);
>  	if (!ret) {
>  		dev_err(dev, "no suitable video mode found\n");
>  		goto stop_clk;
> @@ -1088,8 +1114,8 @@ static int __init atmel_lcdfb_probe(struct platform_device *pdev)
>  	fb_add_videomode(&fbmode, &info->modelist);
>  
>  	/* Power up the LCDC screen */
> -	if (sinfo->atmel_lcdfb_power_control)
> -		sinfo->atmel_lcdfb_power_control(1);
> +	if (pdata->atmel_lcdfb_power_control)
> +		pdata->atmel_lcdfb_power_control(1);
>  
>  	dev_info(dev, "fb%d: Atmel LCDC at 0x%08lx (mapped at %p), irq %d\n",
>  		       info->node, info->fix.mmio_start, sinfo->mmio, sinfo->irq_base);
> @@ -1134,15 +1160,17 @@ static int __exit atmel_lcdfb_remove(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct fb_info *info = dev_get_drvdata(dev);
>  	struct atmel_lcdfb_info *sinfo;
> +	struct atmel_lcdfb_pdata *pdata;
>  
>  	if (!info || !info->par)
>  		return 0;
>  	sinfo = info->par;
> +	pdata = &sinfo->pdata;
>  
>  	cancel_work_sync(&sinfo->task);
>  	exit_backlight(sinfo);
> -	if (sinfo->atmel_lcdfb_power_control)
> -		sinfo->atmel_lcdfb_power_control(0);
> +	if (pdata->atmel_lcdfb_power_control)
> +		pdata->atmel_lcdfb_power_control(0);
>  	unregister_framebuffer(info);
>  	atmel_lcdfb_stop_clock(sinfo);
>  	clk_put(sinfo->lcdc_clk);
> @@ -1170,6 +1198,7 @@ static int atmel_lcdfb_suspend(struct platform_device *pdev, pm_message_t mesg)
>  {
>  	struct fb_info *info = platform_get_drvdata(pdev);
>  	struct atmel_lcdfb_info *sinfo = info->par;
> +	struct atmel_lcdfb_pdata *pdata = &sinfo->pdata;
>  
>  	/*
>  	 * We don't want to handle interrupts while the clock is
> @@ -1179,8 +1208,8 @@ static int atmel_lcdfb_suspend(struct platform_device *pdev, pm_message_t mesg)
>  
>  	sinfo->saved_lcdcon = lcdc_readl(sinfo, ATMEL_LCDC_CONTRAST_CTR);
>  	lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, 0);
> -	if (sinfo->atmel_lcdfb_power_control)
> -		sinfo->atmel_lcdfb_power_control(0);
> +	if (pdata->atmel_lcdfb_power_control)
> +		pdata->atmel_lcdfb_power_control(0);
>  
>  	atmel_lcdfb_stop(sinfo);
>  	atmel_lcdfb_stop_clock(sinfo);
> @@ -1192,11 +1221,12 @@ static int atmel_lcdfb_resume(struct platform_device *pdev)
>  {
>  	struct fb_info *info = platform_get_drvdata(pdev);
>  	struct atmel_lcdfb_info *sinfo = info->par;
> +	struct atmel_lcdfb_pdata *pdata = &sinfo->pdata;
>  
>  	atmel_lcdfb_start_clock(sinfo);
>  	atmel_lcdfb_start(sinfo);
> -	if (sinfo->atmel_lcdfb_power_control)
> -		sinfo->atmel_lcdfb_power_control(1);
> +	if (pdata->atmel_lcdfb_power_control)
> +		pdata->atmel_lcdfb_power_control(1);
>  	lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, sinfo->saved_lcdcon);
>  
>  	/* Enable FIFO & DMA errors */
> diff --git a/include/video/atmel_lcdc.h b/include/video/atmel_lcdc.h
> index 0f5a2fc..2eb601c 100644
> --- a/include/video/atmel_lcdc.h
> +++ b/include/video/atmel_lcdc.h
> @@ -31,39 +31,17 @@
>  #define ATMEL_LCDC_WIRING_BGR	0
>  #define ATMEL_LCDC_WIRING_RGB	1
>  
> -struct atmel_lcdfb_config;
>  
>   /* LCD Controller info data structure, stored in device platform_data */

Wrong comment: it is not the "info" data structure, this time.

> -struct atmel_lcdfb_info {
> -	spinlock_t		lock;
> -	struct fb_info		*info;
> -	void __iomem		*mmio;
> -	int			irq_base;
> -	struct work_struct	task;
> -
> +struct atmel_lcdfb_pdata {
>  	unsigned int		guard_time;
> -	unsigned int 		smem_len;
> -	struct platform_device	*pdev;
> -	struct clk		*bus_clk;
> -	struct clk		*lcdc_clk;
> -
> -#ifdef CONFIG_BACKLIGHT_ATMEL_LCDC
> -	struct backlight_device	*backlight;
> -	u8			bl_power;
> -#endif
>  	bool			lcdcon_is_backlight;
> -	bool			lcdcon_pol_negative;
> -	u8			saved_lcdcon;
> -
>  	u8			default_bpp;
>  	u8			lcd_wiring_mode;
>  	unsigned int		default_lcdcon2;
>  	unsigned int		default_dmacon;
>  	void (*atmel_lcdfb_power_control)(int on);
>  	struct fb_monspecs	*default_monspecs;
> -	u32			pseudo_palette[16];
> -
> -	struct atmel_lcdfb_config *config;
>  };
>  
>  #define ATMEL_LCDC_DMABADDR1	0x00
> 


-- 
Nicolas Ferre

^ permalink raw reply

* Re: [PATCH 00/33] OMAPDSS: platform_enable/disable callback removal from panel drivers
From: Tomi Valkeinen @ 2013-04-16  4:20 UTC (permalink / raw)
  To: Tony Lindgren; +Cc: Archit Taneja, linux-omap, linux-fbdev
In-Reply-To: <20130415212022.GP10155@atomide.com>

[-- Attachment #1: Type: text/plain, Size: 1257 bytes --]

On 2013-04-16 00:20, Tony Lindgren wrote:

>> So, to recap, the common header changes are located in:
>>
>> git://gitorious.org/linux-omap-dss2/linux.git 3.10/0-dss-headers
>>
>> And the branch for linux-omap is:
>>
>> git://gitorious.org/linux-omap-dss2/linux.git 3.10-lo/board-cleanup
>>
>> After merging those, some displays won't start anymore until the omapdss
>> changes are in, but things should still compile.
> 
> Sounds like it's best that you merge those branches via your
> tree as the conflicts have been already resolved in linux next.

The dss changes are going through drm tree this time, as there are some
drm dependencies also, and I've already sent the pull request for those.
And when I asked Dave Airlie if he can merge the dss changes, I didn't
talk about a big chunk of arch file changes getting included.

Also, the whole division to two independent branches was done only to
make it possible for the arch changes to go through l-o tree.

There will probably be more these kind of changes in the future, so I
think we should agree how to handle those and stick to the plan.
Dividing the arch file changes to a separate branch is often quite
laborious, and I'd rather not do that for nothing.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply

* Re: [PATCH v2] backlight: platform_lcd: introduce probe callback
From: Jingoo Han @ 2013-04-16  0:53 UTC (permalink / raw)
  To: 'Andrew Morton', 'Andrew Bresticker'
  Cc: linux-fbdev, 'Richard Purdie',
	'Florian Tobias Schandinat', linux-kernel,
	'Doug Anderson', 'Jingoo Han'
In-Reply-To: <20130415160632.99e867b25418189c95fbc826@linux-foundation.org>

On Tuesday, April 16, 2013 8:07 AM, Andrew Morton wrote:
> 
> On Thu, 11 Apr 2013 13:24:50 -0700 Andrew Bresticker <abrestic@chromium.org> wrote:
> 
> > Platform LCD devices may need to do some device-specific
> > initialization before they can be used (regulator or GPIO setup,
> > for example), but currently the driver does not support any way of
> > doing this.  This patch adds a probe() callback to plat_lcd_data
> > which platform LCD devices can set to indicate that device-specific
> > initialization is needed.
> >
> > index 17a6b83..f46180e 100644
> > --- a/drivers/video/backlight/platform_lcd.c
> > +++ b/drivers/video/backlight/platform_lcd.c
> > @@ -86,6 +86,12 @@ static int platform_lcd_probe(struct platform_device *pdev)
> >  		return -EINVAL;
> >  	}
> >
> > +	if (pdata->probe) {
> > +		err = pdata->probe(pdata);
> > +		if (err)
> > +			return err;
> > +	}
> > +
> >  	plcd = devm_kzalloc(&pdev->dev, sizeof(struct platform_lcd),
> >  			    GFP_KERNEL);
> >  	if (!plcd) {
> > diff --git a/include/video/platform_lcd.h b/include/video/platform_lcd.h
> > index ad3bdfe..23864b2 100644
> > --- a/include/video/platform_lcd.h
> > +++ b/include/video/platform_lcd.h
> > @@ -15,6 +15,7 @@ struct plat_lcd_data;
> >  struct fb_info;
> >
> >  struct plat_lcd_data {
> > +	int	(*probe)(struct plat_lcd_data *);
> >  	void	(*set_power)(struct plat_lcd_data *, unsigned int power);
> >  	int	(*match_fb)(struct plat_lcd_data *, struct fb_info *);
> >  };
> 
> Sigh.  I see that this entire interface has been lovingly undocumented.
> It's an invitation for us to grow bugs, incompatibilities, leaks, etc.
> 
> Possible example: what happens if pdata->probe does some resource
> allocation or device initialisation which should be backed out if, say,
> platform_lcd_probe() later fails?

Hi Andrew,

I agree with you.
Indeed, the documentation is necessary.

Andrew Bresticker,
Would you make the document?
It would be very helpful.

Best regards,
Jingoo Han



^ permalink raw reply

* Re: [PATCH v2] backlight: platform_lcd: introduce probe callback
From: Andrew Morton @ 2013-04-15 23:06 UTC (permalink / raw)
  To: Andrew Bresticker
  Cc: linux-fbdev, Richard Purdie, Florian Tobias Schandinat,
	linux-kernel, Doug Anderson
In-Reply-To: <1365711890-15168-1-git-send-email-abrestic@chromium.org>

On Thu, 11 Apr 2013 13:24:50 -0700 Andrew Bresticker <abrestic@chromium.org> wrote:

> Platform LCD devices may need to do some device-specific
> initialization before they can be used (regulator or GPIO setup,
> for example), but currently the driver does not support any way of
> doing this.  This patch adds a probe() callback to plat_lcd_data
> which platform LCD devices can set to indicate that device-specific
> initialization is needed.
> 
> index 17a6b83..f46180e 100644
> --- a/drivers/video/backlight/platform_lcd.c
> +++ b/drivers/video/backlight/platform_lcd.c
> @@ -86,6 +86,12 @@ static int platform_lcd_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> +	if (pdata->probe) {
> +		err = pdata->probe(pdata);
> +		if (err)
> +			return err;
> +	}
> +
>  	plcd = devm_kzalloc(&pdev->dev, sizeof(struct platform_lcd),
>  			    GFP_KERNEL);
>  	if (!plcd) {
> diff --git a/include/video/platform_lcd.h b/include/video/platform_lcd.h
> index ad3bdfe..23864b2 100644
> --- a/include/video/platform_lcd.h
> +++ b/include/video/platform_lcd.h
> @@ -15,6 +15,7 @@ struct plat_lcd_data;
>  struct fb_info;
>  
>  struct plat_lcd_data {
> +	int	(*probe)(struct plat_lcd_data *);
>  	void	(*set_power)(struct plat_lcd_data *, unsigned int power);
>  	int	(*match_fb)(struct plat_lcd_data *, struct fb_info *);
>  };

Sigh.  I see that this entire interface has been lovingly undocumented.
It's an invitation for us to grow bugs, incompatibilities, leaks, etc.

Possible example: what happens if pdata->probe does some resource
allocation or device initialisation which should be backed out if, say,
platform_lcd_probe() later fails?


^ permalink raw reply

* Re: [PATCH 00/33] OMAPDSS: platform_enable/disable callback removal from panel drivers
From: Tony Lindgren @ 2013-04-15 21:20 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: Archit Taneja, linux-omap, linux-fbdev
In-Reply-To: <516BC897.5020802@ti.com>

* Tomi Valkeinen <tomi.valkeinen@ti.com> [130415 02:34]:
> Hi Tony,
> 
> On 2013-04-03 18:46, Tony Lindgren wrote:
> 
> >> Tony, I've split these patches as follows:
> >>
> >> Platform data header file changes:
> >> git://gitorious.org/linux-omap-dss2/linux.git 3.10/0-dss-headers
> >>
> >> Board file changes (based on header changes):
> >> git://gitorious.org/linux-omap-dss2/linux.git 3.10-lo/board-cleanup
> >>
> >> DSS panel changes (based on header changes):
> >> git://gitorious.org/linux-omap-dss2/linux.git 3.10/1-panel-cleanup
> >>
> >> Removing unused fields from header files (based on panel changes):
> >> git://gitorious.org/linux-omap-dss2/linux.git 3.10/2-late-panel-cleanup
> >>
> >> The 2-late-panel-cleanup breaks compilation if the arch changes are not
> >> merged, so I'll leave that until they have been merged.
> >>
> >> Do you mind if I add the board-cleanup branch temporarily to omapdss's
> >> for-next, to simplify testing? When everything looks ok, I'll remove it
> >> and pass the branch to you to be handled through l-o.
> > 
> > Sure please go ahead. There are still some board-*.c related patches
> > that I have not merged, but we'll see those merge conflicts before
> > next as I usually do a merge with next before sending out pull reqs.
> 
> The code seems to work ok, so I think we can declare the branches
> stable. I've pushed new for-next branch that does not contain the arch
> files anymore. In fact, I removed all omapdss patches also, as omapdss
> should be merged through the drm tree, and I don't want to have
> conflicts with drm's for-next branch.
> 
> So, to recap, the common header changes are located in:
> 
> git://gitorious.org/linux-omap-dss2/linux.git 3.10/0-dss-headers
> 
> And the branch for linux-omap is:
> 
> git://gitorious.org/linux-omap-dss2/linux.git 3.10-lo/board-cleanup
> 
> After merging those, some displays won't start anymore until the omapdss
> changes are in, but things should still compile.

Sounds like it's best that you merge those branches via your
tree as the conflicts have been already resolved in linux next.

Regards,

Tony

^ permalink raw reply

* Re: [PATCH v3 2/2] video: imxfb: Add DT support
From: Sascha Hauer @ 2013-04-15 19:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20130415151316.4d49e9b2@e6520eb>

On Mon, Apr 15, 2013 at 03:13:16PM +0200, Eric Bénard wrote:
> Hi,
> 
> 
> I didn't follow the thread but dmacr is not specific to our boards :
> $ grep -rn dmacr arch/arm/mach-imx/
> arch/arm/mach-imx/mach-mx27_3ds.c:446:	.dmacr		= 0x00020010,
> arch/arm/mach-imx/mach-mx21ads.c:231:	.dmacr		= 0x00020008,
> arch/arm/mach-imx/mach-mxt_td60.c:210:	.dmacr		= 0x00020010,
> arch/arm/mach-imx/eukrea_mbimx27-baseboard.c:192:	.dmacr		= 0x00040060,
> arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:165:	.dmacr		= 0x00040060,
> arch/arm/mach-imx/mach-mx27ads.c:241:	.dmacr		= 0x00020010,
> arch/arm/mach-imx/mach-pca100.c:351:	.dmacr		= 0x00020010,
> arch/arm/mach-imx/pcm970-baseboard.c:185:	.dmacr		= 0x00020010,
> arch/arm/mach-imx/mach-mx25_3ds.c:173:	.dmacr		= 0x00020010,

The reference manual has this about the dmacr register:

The dynamic mode is recommend, and also the TM and HM reset value are
recommend in the dynamic mode. The same recommendation is for the
graphic DMA control.

So maybe we should ask whether the boards really have to change this
value or if the reset value is suitable for all.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCH v3 2/2] video: imxfb: Add DT support
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-04-15 13:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20130415151316.4d49e9b2@e6520eb>

On 15:13 Mon 15 Apr     , Eric Bénard wrote:
> Hi,
> 
> Le Mon, 15 Apr 2013 14:43:21 +0200,
> Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> a écrit :
> 
> > On 16:31 Sun 14 Apr     , Markus Pargmann wrote:
> > > On Mon, Apr 08, 2013 at 09:57:42AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > > On 12:31 Fri 05 Apr     , Markus Pargmann wrote:
> > > > > Add devicetree support for imx framebuffer driver. It uses the generic
> > > > > display bindings and helper functions.
> > > > > 
> > > > > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > > > > Cc: Fabio Estevam <festevam@gmail.com>
> > > > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > > > ---
> > > > >  .../devicetree/bindings/video/fsl,imx-fb.txt       |  49 ++++++
> > > > >  drivers/video/imxfb.c                              | 192 +++++++++++++++++----
> > > > >  2 files changed, 207 insertions(+), 34 deletions(-)
> > > > >  create mode 100644 Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > > > new file mode 100644
> > > > > index 0000000..bde9c77
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > > > @@ -0,0 +1,49 @@
> > > > > +Freescale imx21 Framebuffer
> > > > > +
> > > > > +This framebuffer driver supports devices imx1, imx21, imx25, and imx27.
> > > > > +
> > > > > +Required properties:
> > > > > +- compatible : "fsl,<chip>-fb", chip should be imx1 or imx21
> > > > > +- reg : Should contain 1 register ranges(address and length)
> > > > > +- interrupts : One interrupt of the fb dev
> > > > > +
> > > > > +Required nodes:
> > > > > +- display: Phandle to a display node as described in
> > > > > +	Documentation/devicetree/bindings/video/display-timing.txt
> > > > > +	Additional, the display node has to define properties:
> > > > > +	- fsl,bpp: Bits per pixel
> > > > > +	- fsl,pcr: LCDC PCR value
> > > > > +
> > > > > +Optional properties:
> > > > > +- dmacr-eukrea: Should be set for eukrea boards.
> > > > why ?
> > > 
> > > Because eukrea boards have a different dmacr then all other boards using
> > > imxfb. The dmacr address is hardcoded as defaults in the code. I could
> > > also search for the board name in the DT, but there are no eukrea boards
> > > with DT at the moment, so I thought a bool property may be better for
> > > the moment.
> > so no if an other board come will have an other property no
> > 
> > add an optionnal option to pass the dmacr
> 
> I didn't follow the thread but dmacr is not specific to our boards :
> $ grep -rn dmacr arch/arm/mach-imx/
> arch/arm/mach-imx/mach-mx27_3ds.c:446:	.dmacr		= 0x00020010,
> arch/arm/mach-imx/mach-mx21ads.c:231:	.dmacr		= 0x00020008,
> arch/arm/mach-imx/mach-mxt_td60.c:210:	.dmacr		= 0x00020010,
> arch/arm/mach-imx/eukrea_mbimx27-baseboard.c:192:	.dmacr		= 0x00040060,
> arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:165:	.dmacr		= 0x00040060,
> arch/arm/mach-imx/mach-mx27ads.c:241:	.dmacr		= 0x00020010,
> arch/arm/mach-imx/mach-pca100.c:351:	.dmacr		= 0x00020010,
> arch/arm/mach-imx/pcm970-baseboard.c:185:	.dmacr		= 0x00020010,
> arch/arm/mach-imx/mach-mx25_3ds.c:173:	.dmacr		= 0x00020010,

with this it even more clear nack on the dmacr-eukrea property

use an optional property where we can specify the value to use

Best Regsrds,
J.

^ permalink raw reply

* Re: [PATCH v3 2/2] video: imxfb: Add DT support
From: Eric Bénard @ 2013-04-15 13:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20130415124321.GB15139@game.jcrosoft.org>

Hi,

Le Mon, 15 Apr 2013 14:43:21 +0200,
Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> a écrit :

> On 16:31 Sun 14 Apr     , Markus Pargmann wrote:
> > On Mon, Apr 08, 2013 at 09:57:42AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > On 12:31 Fri 05 Apr     , Markus Pargmann wrote:
> > > > Add devicetree support for imx framebuffer driver. It uses the generic
> > > > display bindings and helper functions.
> > > > 
> > > > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > > > Cc: Fabio Estevam <festevam@gmail.com>
> > > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > > ---
> > > >  .../devicetree/bindings/video/fsl,imx-fb.txt       |  49 ++++++
> > > >  drivers/video/imxfb.c                              | 192 +++++++++++++++++----
> > > >  2 files changed, 207 insertions(+), 34 deletions(-)
> > > >  create mode 100644 Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > > new file mode 100644
> > > > index 0000000..bde9c77
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > > @@ -0,0 +1,49 @@
> > > > +Freescale imx21 Framebuffer
> > > > +
> > > > +This framebuffer driver supports devices imx1, imx21, imx25, and imx27.
> > > > +
> > > > +Required properties:
> > > > +- compatible : "fsl,<chip>-fb", chip should be imx1 or imx21
> > > > +- reg : Should contain 1 register ranges(address and length)
> > > > +- interrupts : One interrupt of the fb dev
> > > > +
> > > > +Required nodes:
> > > > +- display: Phandle to a display node as described in
> > > > +	Documentation/devicetree/bindings/video/display-timing.txt
> > > > +	Additional, the display node has to define properties:
> > > > +	- fsl,bpp: Bits per pixel
> > > > +	- fsl,pcr: LCDC PCR value
> > > > +
> > > > +Optional properties:
> > > > +- dmacr-eukrea: Should be set for eukrea boards.
> > > why ?
> > 
> > Because eukrea boards have a different dmacr then all other boards using
> > imxfb. The dmacr address is hardcoded as defaults in the code. I could
> > also search for the board name in the DT, but there are no eukrea boards
> > with DT at the moment, so I thought a bool property may be better for
> > the moment.
> so no if an other board come will have an other property no
> 
> add an optionnal option to pass the dmacr

I didn't follow the thread but dmacr is not specific to our boards :
$ grep -rn dmacr arch/arm/mach-imx/
arch/arm/mach-imx/mach-mx27_3ds.c:446:	.dmacr		= 0x00020010,
arch/arm/mach-imx/mach-mx21ads.c:231:	.dmacr		= 0x00020008,
arch/arm/mach-imx/mach-mxt_td60.c:210:	.dmacr		= 0x00020010,
arch/arm/mach-imx/eukrea_mbimx27-baseboard.c:192:	.dmacr		= 0x00040060,
arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:165:	.dmacr		= 0x00040060,
arch/arm/mach-imx/mach-mx27ads.c:241:	.dmacr		= 0x00020010,
arch/arm/mach-imx/mach-pca100.c:351:	.dmacr		= 0x00020010,
arch/arm/mach-imx/pcm970-baseboard.c:185:	.dmacr		= 0x00020010,
arch/arm/mach-imx/mach-mx25_3ds.c:173:	.dmacr		= 0x00020010,

Best regards,
Eric

^ permalink raw reply

* Re: [PATCH v3 2/2] video: imxfb: Add DT support
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-04-15 12:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20130414143132.GD27394@pengutronix.de>

On 16:31 Sun 14 Apr     , Markus Pargmann wrote:
> On Mon, Apr 08, 2013 at 09:57:42AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 12:31 Fri 05 Apr     , Markus Pargmann wrote:
> > > Add devicetree support for imx framebuffer driver. It uses the generic
> > > display bindings and helper functions.
> > > 
> > > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > > Cc: Fabio Estevam <festevam@gmail.com>
> > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > ---
> > >  .../devicetree/bindings/video/fsl,imx-fb.txt       |  49 ++++++
> > >  drivers/video/imxfb.c                              | 192 +++++++++++++++++----
> > >  2 files changed, 207 insertions(+), 34 deletions(-)
> > >  create mode 100644 Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > new file mode 100644
> > > index 0000000..bde9c77
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > @@ -0,0 +1,49 @@
> > > +Freescale imx21 Framebuffer
> > > +
> > > +This framebuffer driver supports devices imx1, imx21, imx25, and imx27.
> > > +
> > > +Required properties:
> > > +- compatible : "fsl,<chip>-fb", chip should be imx1 or imx21
> > > +- reg : Should contain 1 register ranges(address and length)
> > > +- interrupts : One interrupt of the fb dev
> > > +
> > > +Required nodes:
> > > +- display: Phandle to a display node as described in
> > > +	Documentation/devicetree/bindings/video/display-timing.txt
> > > +	Additional, the display node has to define properties:
> > > +	- fsl,bpp: Bits per pixel
> > > +	- fsl,pcr: LCDC PCR value
> > > +
> > > +Optional properties:
> > > +- dmacr-eukrea: Should be set for eukrea boards.
> > why ?
> 
> Because eukrea boards have a different dmacr then all other boards using
> imxfb. The dmacr address is hardcoded as defaults in the code. I could
> also search for the board name in the DT, but there are no eukrea boards
> with DT at the moment, so I thought a bool property may be better for
> the moment.
so no if an other board come will have an other property no

add an optionnal option to pass the dmacr
> 
> > 
> > and This should be prefix by yhe Vendor here Eukrea
> 
> Okay.
> 
> Regards,
> 
> Markus
> 
> > > +
> > > +Example:
> > > +
> > > +	imxfb: fb@10021000 {
> > > +		compatible = "fsl,imx27-fb", "fsl,imx21-fb";
> > > +		interrupts = <61>;
> > > +		reg = <0x10021000 0x1000>;
> > > +		display = <&display0>;
> > > +	};
> > > +
> > > +	...
> > > +
> > > +	display0: display0 {
> > > +		model = "Primeview-PD050VL1";
> > > +		native-mode = <&timing_disp0>;
> > > +		fsl,bpp = <16>;		/* non-standard but required */
> > > +		fsl,pcr = <0xf0c88080>;	/* non-standard but required */
> > > +		display-timings {
> > > +			timing_disp0: 640x480 {
> > > +				hactive = <640>;
> > > +				vactive = <480>;
> > > +				hback-porch = <112>;
> > > +				hfront-porch = <36>;
> > > +				hsync-len = <32>;
> > > +				vback-porch = <33>;
> > > +				vfront-porch = <33>;
> > > +				vsync-len = <2>;
> > > +				clock-frequency = <25000000>;
> > > +			};
> > > +		};
> > > +	};
> > > diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> > > index ef2b587..5a9bc598 100644
> > > --- a/drivers/video/imxfb.c
> > > +++ b/drivers/video/imxfb.c
> > > @@ -32,6 +32,12 @@
> > >  #include <linux/io.h>
> > >  #include <linux/math64.h>
> > >  #include <linux/uaccess.h>
> > > +#include <linux/of.h>
> > > +#include <linux/of_device.h>
> > > +
> > > +#include <video/of_display_timing.h>
> > > +#include <video/of_videomode.h>
> > > +#include <video/videomode.h>
> > >  
> > >  #include <linux/platform_data/video-imxfb.h>
> > >  
> > > @@ -117,10 +123,13 @@
> > >  #define LGWCR_GWAV(alpha)	(((alpha) & 0xff) << 24)
> > >  #define LGWCR_GWE	(1 << 22)
> > >  
> > > +#define IMXFB_LSCR1_DEFAULT 0x00120300
> > > +#define IMXFB_DMACR_DEFAULT 0x00020010
> > > +#define IMXFB_DMACR_EUKREA_DEFAULT 0x00040060
> > > +
> > >  /* Used fb-mode. Can be set on kernel command line, therefore file-static. */
> > >  static const char *fb_mode;
> > >  
> > > -
> > >  /*
> > >   * These are the bitfields for each
> > >   * display depth that we support.
> > > @@ -192,6 +201,19 @@ static struct platform_device_id imxfb_devtype[] = {
> > >  };
> > >  MODULE_DEVICE_TABLE(platform, imxfb_devtype);
> > >  
> > > +static struct of_device_id imxfb_of_dev_id[] = {
> > > +	{
> > > +		.compatible = "fsl,imx1-fb",
> > > +		.data = &imxfb_devtype[IMX1_FB],
> > > +	}, {
> > > +		.compatible = "fsl,imx21-fb",
> > > +		.data = &imxfb_devtype[IMX21_FB],
> > > +	}, {
> > > +		/* sentinel */
> > > +	}
> > > +};
> > > +MODULE_DEVICE_TABLE(of, imxfb_of_dev_id);
> > > +
> > >  static inline int is_imx1_fb(struct imxfb_info *fbi)
> > >  {
> > >  	return fbi->devtype = IMX1_FB;
> > > @@ -324,6 +346,9 @@ static const struct imx_fb_videomode *imxfb_find_mode(struct imxfb_info *fbi)
> > >  	struct imx_fb_videomode *m;
> > >  	int i;
> > >  
> > > +	if (!fb_mode)
> > > +		return &fbi->mode[0];
> > > +
> > >  	for (i = 0, m = &fbi->mode[0]; i < fbi->num_modes; i++, m++) {
> > >  		if (!strcmp(m->mode.name, fb_mode))
> > >  			return m;
> > > @@ -479,6 +504,9 @@ static int imxfb_bl_update_status(struct backlight_device *bl)
> > >  	struct imxfb_info *fbi = bl_get_data(bl);
> > >  	int brightness = bl->props.brightness;
> > >  
> > > +	if (!fbi->pwmr)
> > > +		return 0;
> > > +
> > >  	if (bl->props.power != FB_BLANK_UNBLANK)
> > >  		brightness = 0;
> > >  	if (bl->props.fb_blank != FB_BLANK_UNBLANK)
> > > @@ -719,7 +747,8 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
> > >  
> > >  	writel(fbi->pcr, fbi->regs + LCDC_PCR);
> > >  #ifndef PWMR_BACKLIGHT_AVAILABLE
> > > -	writel(fbi->pwmr, fbi->regs + LCDC_PWMR);
> > > +	if (fbi->pwmr)
> > > +		writel(fbi->pwmr, fbi->regs + LCDC_PWMR);
> > >  #endif
> > >  	writel(fbi->lscr1, fbi->regs + LCDC_LSCR1);
> > >  	writel(fbi->dmacr, fbi->regs + LCDC_DMACR);
> > > @@ -758,13 +787,12 @@ static int imxfb_resume(struct platform_device *dev)
> > >  #define imxfb_resume	NULL
> > >  #endif
> > >  
> > > -static int __init imxfb_init_fbinfo(struct platform_device *pdev)
> > > +static int imxfb_init_fbinfo(struct platform_device *pdev)
> > >  {
> > >  	struct imx_fb_platform_data *pdata = pdev->dev.platform_data;
> > >  	struct fb_info *info = dev_get_drvdata(&pdev->dev);
> > >  	struct imxfb_info *fbi = info->par;
> > > -	struct imx_fb_videomode *m;
> > > -	int i;
> > > +	struct device_node *np;
> > >  
> > >  	pr_debug("%s\n",__func__);
> > >  
> > > @@ -795,41 +823,96 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
> > >  	info->fbops			= &imxfb_ops;
> > >  	info->flags			= FBINFO_FLAG_DEFAULT |
> > >  					  FBINFO_READS_FAST;
> > > -	info->var.grayscale		= pdata->cmap_greyscale;
> > > -	fbi->cmap_inverse		= pdata->cmap_inverse;
> > > -	fbi->cmap_static		= pdata->cmap_static;
> > > -	fbi->lscr1			= pdata->lscr1;
> > > -	fbi->dmacr			= pdata->dmacr;
> > > -	fbi->pwmr			= pdata->pwmr;
> > > -	fbi->lcd_power			= pdata->lcd_power;
> > > -	fbi->backlight_power		= pdata->backlight_power;
> > > -
> > > -	for (i = 0, m = &pdata->mode[0]; i < pdata->num_modes; i++, m++)
> > > -		info->fix.smem_len = max_t(size_t, info->fix.smem_len,
> > > -				m->mode.xres * m->mode.yres * m->bpp / 8);
> > > +	if (pdata) {
> > > +		info->var.grayscale		= pdata->cmap_greyscale;
> > > +		fbi->cmap_inverse		= pdata->cmap_inverse;
> > > +		fbi->cmap_static		= pdata->cmap_static;
> > > +		fbi->lscr1			= pdata->lscr1;
> > > +		fbi->dmacr			= pdata->dmacr;
> > > +		fbi->pwmr			= pdata->pwmr;
> > > +		fbi->lcd_power			= pdata->lcd_power;
> > > +		fbi->backlight_power		= pdata->backlight_power;
> > > +	} else {
> > > +		np = pdev->dev.of_node;
> > > +		info->var.grayscale = of_property_read_bool(np,
> > > +						"cmap-greyscale");
> > > +		fbi->cmap_inverse = of_property_read_bool(np, "cmap-inverse");
> > > +		fbi->cmap_static = of_property_read_bool(np, "cmap-static");
> > > +
> > > +		fbi->lscr1 = IMXFB_LSCR1_DEFAULT;
> > > +		if (of_property_read_bool(np, "dmacr-eukrea"))
> > > +			fbi->dmacr = IMXFB_DMACR_EUKREA_DEFAULT;
> > > +		else
> > > +			fbi->dmacr = IMXFB_DMACR_DEFAULT;
> > > +
> > > +		/* These two function pointers could be used by some specific
> > > +		 * platforms. */
> > > +		fbi->lcd_power = NULL;
> > > +		fbi->backlight_power = NULL;
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int imxfb_of_read_mode(struct device *dev, struct device_node *np,
> > > +		struct imx_fb_videomode *imxfb_mode)
> > > +{
> > > +	int ret;
> > > +	struct fb_videomode *of_mode = &imxfb_mode->mode;
> > > +	u32 bpp;
> > > +	u32 pcr;
> > > +
> > > +	ret = of_property_read_string(np, "model", &of_mode->name);
> > > +	if (ret)
> > > +		of_mode->name = NULL;
> > > +
> > > +	ret = of_get_fb_videomode(np, of_mode, OF_USE_NATIVE_MODE);
> > > +	if (ret) {
> > > +		dev_err(dev, "Failed to get videomode from DT\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	ret = of_property_read_u32(np, "fsl,bpp", &bpp);
> > > +	ret |= of_property_read_u32(np, "fsl,pcr", &pcr);
> > > +
> > > +	if (ret) {
> > > +		dev_err(dev, "Failed to read bpp and pcr from DT\n");
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	if (bpp < 1 || bpp > 255) {
> > > +		dev_err(dev, "Bits per pixel have to be between 1 and 255\n");
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	imxfb_mode->bpp = bpp;
> > > +	imxfb_mode->pcr = pcr;
> > >  
> > >  	return 0;
> > >  }
> > >  
> > > -static int __init imxfb_probe(struct platform_device *pdev)
> > > +static int imxfb_probe(struct platform_device *pdev)
> > >  {
> > >  	struct imxfb_info *fbi;
> > >  	struct fb_info *info;
> > >  	struct imx_fb_platform_data *pdata;
> > >  	struct resource *res;
> > > +	struct imx_fb_videomode *m;
> > > +	const struct of_device_id *of_id;
> > >  	int ret, i;
> > > +	int bytes_per_pixel;
> > >  
> > >  	dev_info(&pdev->dev, "i.MX Framebuffer driver\n");
> > >  
> > > +	of_id = of_match_device(imxfb_of_dev_id, &pdev->dev);
> > > +	if (of_id)
> > > +		pdev->id_entry = of_id->data;
> > > +
> > >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > >  	if (!res)
> > >  		return -ENODEV;
> > >  
> > >  	pdata = pdev->dev.platform_data;
> > > -	if (!pdata) {
> > > -		dev_err(&pdev->dev,"No platform_data available\n");
> > > -		return -ENOMEM;
> > > -	}
> > >  
> > >  	info = framebuffer_alloc(sizeof(struct imxfb_info), &pdev->dev);
> > >  	if (!info)
> > > @@ -837,15 +920,55 @@ static int __init imxfb_probe(struct platform_device *pdev)
> > >  
> > >  	fbi = info->par;
> > >  
> > > -	if (!fb_mode)
> > > -		fb_mode = pdata->mode[0].mode.name;
> > > -
> > >  	platform_set_drvdata(pdev, info);
> > >  
> > >  	ret = imxfb_init_fbinfo(pdev);
> > >  	if (ret < 0)
> > >  		goto failed_init;
> > >  
> > > +	if (pdata) {
> > > +		if (!fb_mode)
> > > +			fb_mode = pdata->mode[0].mode.name;
> > > +
> > > +		fbi->mode = pdata->mode;
> > > +		fbi->num_modes = pdata->num_modes;
> > > +	} else {
> > > +		struct device_node *display_np;
> > > +		fb_mode = NULL;
> > > +
> > > +		display_np = of_parse_phandle(pdev->dev.of_node, "display", 0);
> > > +		if (!display_np) {
> > > +			dev_err(&pdev->dev, "No display defined in devicetree\n");
> > > +			ret = -EINVAL;
> > > +			goto failed_of_parse;
> > > +		}
> > > +
> > > +		/*
> > > +		 * imxfb does not support more modes, we choose only the native
> > > +		 * mode.
> > > +		 */
> > > +		fbi->num_modes = 1;
> > > +
> > > +		fbi->mode = devm_kzalloc(&pdev->dev,
> > > +				sizeof(struct imx_fb_videomode), GFP_KERNEL);
> > > +		if (!fbi->mode) {
> > > +			ret = -ENOMEM;
> > > +			goto failed_of_parse;
> > > +		}
> > > +
> > > +		ret = imxfb_of_read_mode(&pdev->dev, display_np, fbi->mode);
> > > +		if (ret)
> > > +			goto failed_of_parse;
> > > +	}
> > > +
> > > +	/* Calculate maximum bytes used per pixel. In most cases this should
> > > +	 * be the same as m->bpp/8 */
> > > +	m = &fbi->mode[0];
> > > +	bytes_per_pixel = (m->bpp + 7) / 8;
> > > +	for (i = 0; i < fbi->num_modes; i++, m++)
> > > +		info->fix.smem_len = max_t(size_t, info->fix.smem_len,
> > > +				m->mode.xres * m->mode.yres * bytes_per_pixel);
> > > +
> > >  	res = request_mem_region(res->start, resource_size(res),
> > >  				DRIVER_NAME);
> > >  	if (!res) {
> > > @@ -878,7 +1001,8 @@ static int __init imxfb_probe(struct platform_device *pdev)
> > >  		goto failed_ioremap;
> > >  	}
> > >  
> > > -	if (!pdata->fixed_screen_cpu) {
> > > +	/* Seems not being used by anyone, so no support for oftree */
> > > +	if (!pdata || !pdata->fixed_screen_cpu) {
> > >  		fbi->map_size = PAGE_ALIGN(info->fix.smem_len);
> > >  		fbi->map_cpu = dma_alloc_writecombine(&pdev->dev,
> > >  				fbi->map_size, &fbi->map_dma, GFP_KERNEL);
> > > @@ -903,18 +1027,16 @@ static int __init imxfb_probe(struct platform_device *pdev)
> > >  		info->fix.smem_start = fbi->screen_dma;
> > >  	}
> > >  
> > > -	if (pdata->init) {
> > > +	if (pdata && pdata->init) {
> > >  		ret = pdata->init(fbi->pdev);
> > >  		if (ret)
> > >  			goto failed_platform_init;
> > >  	}
> > >  
> > > -	fbi->mode = pdata->mode;
> > > -	fbi->num_modes = pdata->num_modes;
> > >  
> > >  	INIT_LIST_HEAD(&info->modelist);
> > > -	for (i = 0; i < pdata->num_modes; i++)
> > > -		fb_add_videomode(&pdata->mode[i].mode, &info->modelist);
> > > +	for (i = 0; i < fbi->num_modes; i++)
> > > +		fb_add_videomode(&fbi->mode[i].mode, &info->modelist);
> > >  
> > >  	/*
> > >  	 * This makes sure that our colour bitfield
> > > @@ -944,10 +1066,10 @@ static int __init imxfb_probe(struct platform_device *pdev)
> > >  failed_register:
> > >  	fb_dealloc_cmap(&info->cmap);
> > >  failed_cmap:
> > > -	if (pdata->exit)
> > > +	if (pdata && pdata->exit)
> > >  		pdata->exit(fbi->pdev);
> > >  failed_platform_init:
> > > -	if (!pdata->fixed_screen_cpu)
> > > +	if (pdata && !pdata->fixed_screen_cpu)
> > >  		dma_free_writecombine(&pdev->dev,fbi->map_size,fbi->map_cpu,
> > >  			fbi->map_dma);
> > >  failed_map:
> > > @@ -956,6 +1078,7 @@ failed_ioremap:
> > >  failed_getclock:
> > >  	release_mem_region(res->start, resource_size(res));
> > >  failed_req:
> > > +failed_of_parse:
> > >  	kfree(info->pseudo_palette);
> > >  failed_init:
> > >  	platform_set_drvdata(pdev, NULL);
> > > @@ -980,7 +1103,7 @@ static int imxfb_remove(struct platform_device *pdev)
> > >  	unregister_framebuffer(info);
> > >  
> > >  	pdata = pdev->dev.platform_data;
> > > -	if (pdata->exit)
> > > +	if (pdata && pdata->exit)
> > >  		pdata->exit(fbi->pdev);
> > >  
> > >  	fb_dealloc_cmap(&info->cmap);
> > > @@ -1009,6 +1132,7 @@ static struct platform_driver imxfb_driver = {
> > >  	.shutdown	= imxfb_shutdown,
> > >  	.driver		= {
> > >  		.name	= DRIVER_NAME,
> > > +		.of_match_table = imxfb_of_dev_id,
> > >  	},
> > >  	.id_table	= imxfb_devtype,
> > >  };
> > > -- 
> > > 1.8.1.5
> > > 
> > > _______________________________________________
> > > devicetree-discuss mailing list
> > > devicetree-discuss@lists.ozlabs.org
> > > https://lists.ozlabs.org/listinfo/devicetree-discuss
> > 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCH 00/33] OMAPDSS: platform_enable/disable callback removal from panel drivers
From: Tomi Valkeinen @ 2013-04-15  9:29 UTC (permalink / raw)
  To: Tony Lindgren; +Cc: Archit Taneja, linux-omap, linux-fbdev
In-Reply-To: <20130403154645.GV10155@atomide.com>

[-- Attachment #1: Type: text/plain, Size: 1879 bytes --]

Hi Tony,

On 2013-04-03 18:46, Tony Lindgren wrote:

>> Tony, I've split these patches as follows:
>>
>> Platform data header file changes:
>> git://gitorious.org/linux-omap-dss2/linux.git 3.10/0-dss-headers
>>
>> Board file changes (based on header changes):
>> git://gitorious.org/linux-omap-dss2/linux.git 3.10-lo/board-cleanup
>>
>> DSS panel changes (based on header changes):
>> git://gitorious.org/linux-omap-dss2/linux.git 3.10/1-panel-cleanup
>>
>> Removing unused fields from header files (based on panel changes):
>> git://gitorious.org/linux-omap-dss2/linux.git 3.10/2-late-panel-cleanup
>>
>> The 2-late-panel-cleanup breaks compilation if the arch changes are not
>> merged, so I'll leave that until they have been merged.
>>
>> Do you mind if I add the board-cleanup branch temporarily to omapdss's
>> for-next, to simplify testing? When everything looks ok, I'll remove it
>> and pass the branch to you to be handled through l-o.
> 
> Sure please go ahead. There are still some board-*.c related patches
> that I have not merged, but we'll see those merge conflicts before
> next as I usually do a merge with next before sending out pull reqs.

The code seems to work ok, so I think we can declare the branches
stable. I've pushed new for-next branch that does not contain the arch
files anymore. In fact, I removed all omapdss patches also, as omapdss
should be merged through the drm tree, and I don't want to have
conflicts with drm's for-next branch.

So, to recap, the common header changes are located in:

git://gitorious.org/linux-omap-dss2/linux.git 3.10/0-dss-headers

And the branch for linux-omap is:

git://gitorious.org/linux-omap-dss2/linux.git 3.10-lo/board-cleanup

After merging those, some displays won't start anymore until the omapdss
changes are in, but things should still compile.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply

* Re: [PATCH v2] backlight: platform_lcd: introduce probe callback
From: Jingoo Han @ 2013-04-15  1:47 UTC (permalink / raw)
  To: 'Andrew Bresticker', 'Andrew Morton'
  Cc: linux-fbdev, 'Richard Purdie',
	'Florian Tobias Schandinat', linux-kernel,
	'Doug Anderson', 'Jingoo Han'
In-Reply-To: <1365711890-15168-1-git-send-email-abrestic@chromium.org>

On Friday, April 12, 2013 5:25 AM, Andrew Bresticker wrote:
> 
> Platform LCD devices may need to do some device-specific
> initialization before they can be used (regulator or GPIO setup,
> for example), but currently the driver does not support any way of
> doing this.  This patch adds a probe() callback to plat_lcd_data
> which platform LCD devices can set to indicate that device-specific
> initialization is needed.
> 
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>

CC'ed Andrew Morton,

It looks good.
Acked-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han


> ---
>  drivers/video/backlight/platform_lcd.c | 6 ++++++
>  include/video/platform_lcd.h           | 1 +
>  2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/video/backlight/platform_lcd.c b/drivers/video/backlight/platform_lcd.c
> index 17a6b83..f46180e 100644
> --- a/drivers/video/backlight/platform_lcd.c
> +++ b/drivers/video/backlight/platform_lcd.c
> @@ -86,6 +86,12 @@ static int platform_lcd_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
> 
> +	if (pdata->probe) {
> +		err = pdata->probe(pdata);
> +		if (err)
> +			return err;
> +	}
> +
>  	plcd = devm_kzalloc(&pdev->dev, sizeof(struct platform_lcd),
>  			    GFP_KERNEL);
>  	if (!plcd) {
> diff --git a/include/video/platform_lcd.h b/include/video/platform_lcd.h
> index ad3bdfe..23864b2 100644
> --- a/include/video/platform_lcd.h
> +++ b/include/video/platform_lcd.h
> @@ -15,6 +15,7 @@ struct plat_lcd_data;
>  struct fb_info;
> 
>  struct plat_lcd_data {
> +	int	(*probe)(struct plat_lcd_data *);
>  	void	(*set_power)(struct plat_lcd_data *, unsigned int power);
>  	int	(*match_fb)(struct plat_lcd_data *, struct fb_info *);
>  };
> --
> 1.8.1.3
> 



^ permalink raw reply

* Re: [PATCH v3 2/2] video: imxfb: Add DT support
From: Markus Pargmann @ 2013-04-14 14:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20130408075742.GR20693@game.jcrosoft.org>

On Mon, Apr 08, 2013 at 09:57:42AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 12:31 Fri 05 Apr     , Markus Pargmann wrote:
> > Add devicetree support for imx framebuffer driver. It uses the generic
> > display bindings and helper functions.
> > 
> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > Cc: Fabio Estevam <festevam@gmail.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > ---
> >  .../devicetree/bindings/video/fsl,imx-fb.txt       |  49 ++++++
> >  drivers/video/imxfb.c                              | 192 +++++++++++++++++----
> >  2 files changed, 207 insertions(+), 34 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > new file mode 100644
> > index 0000000..bde9c77
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > @@ -0,0 +1,49 @@
> > +Freescale imx21 Framebuffer
> > +
> > +This framebuffer driver supports devices imx1, imx21, imx25, and imx27.
> > +
> > +Required properties:
> > +- compatible : "fsl,<chip>-fb", chip should be imx1 or imx21
> > +- reg : Should contain 1 register ranges(address and length)
> > +- interrupts : One interrupt of the fb dev
> > +
> > +Required nodes:
> > +- display: Phandle to a display node as described in
> > +	Documentation/devicetree/bindings/video/display-timing.txt
> > +	Additional, the display node has to define properties:
> > +	- fsl,bpp: Bits per pixel
> > +	- fsl,pcr: LCDC PCR value
> > +
> > +Optional properties:
> > +- dmacr-eukrea: Should be set for eukrea boards.
> why ?

Because eukrea boards have a different dmacr then all other boards using
imxfb. The dmacr address is hardcoded as defaults in the code. I could
also search for the board name in the DT, but there are no eukrea boards
with DT at the moment, so I thought a bool property may be better for
the moment.

> 
> and This should be prefix by yhe Vendor here Eukrea

Okay.

Regards,

Markus

> > +
> > +Example:
> > +
> > +	imxfb: fb@10021000 {
> > +		compatible = "fsl,imx27-fb", "fsl,imx21-fb";
> > +		interrupts = <61>;
> > +		reg = <0x10021000 0x1000>;
> > +		display = <&display0>;
> > +	};
> > +
> > +	...
> > +
> > +	display0: display0 {
> > +		model = "Primeview-PD050VL1";
> > +		native-mode = <&timing_disp0>;
> > +		fsl,bpp = <16>;		/* non-standard but required */
> > +		fsl,pcr = <0xf0c88080>;	/* non-standard but required */
> > +		display-timings {
> > +			timing_disp0: 640x480 {
> > +				hactive = <640>;
> > +				vactive = <480>;
> > +				hback-porch = <112>;
> > +				hfront-porch = <36>;
> > +				hsync-len = <32>;
> > +				vback-porch = <33>;
> > +				vfront-porch = <33>;
> > +				vsync-len = <2>;
> > +				clock-frequency = <25000000>;
> > +			};
> > +		};
> > +	};
> > diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> > index ef2b587..5a9bc598 100644
> > --- a/drivers/video/imxfb.c
> > +++ b/drivers/video/imxfb.c
> > @@ -32,6 +32,12 @@
> >  #include <linux/io.h>
> >  #include <linux/math64.h>
> >  #include <linux/uaccess.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +
> > +#include <video/of_display_timing.h>
> > +#include <video/of_videomode.h>
> > +#include <video/videomode.h>
> >  
> >  #include <linux/platform_data/video-imxfb.h>
> >  
> > @@ -117,10 +123,13 @@
> >  #define LGWCR_GWAV(alpha)	(((alpha) & 0xff) << 24)
> >  #define LGWCR_GWE	(1 << 22)
> >  
> > +#define IMXFB_LSCR1_DEFAULT 0x00120300
> > +#define IMXFB_DMACR_DEFAULT 0x00020010
> > +#define IMXFB_DMACR_EUKREA_DEFAULT 0x00040060
> > +
> >  /* Used fb-mode. Can be set on kernel command line, therefore file-static. */
> >  static const char *fb_mode;
> >  
> > -
> >  /*
> >   * These are the bitfields for each
> >   * display depth that we support.
> > @@ -192,6 +201,19 @@ static struct platform_device_id imxfb_devtype[] = {
> >  };
> >  MODULE_DEVICE_TABLE(platform, imxfb_devtype);
> >  
> > +static struct of_device_id imxfb_of_dev_id[] = {
> > +	{
> > +		.compatible = "fsl,imx1-fb",
> > +		.data = &imxfb_devtype[IMX1_FB],
> > +	}, {
> > +		.compatible = "fsl,imx21-fb",
> > +		.data = &imxfb_devtype[IMX21_FB],
> > +	}, {
> > +		/* sentinel */
> > +	}
> > +};
> > +MODULE_DEVICE_TABLE(of, imxfb_of_dev_id);
> > +
> >  static inline int is_imx1_fb(struct imxfb_info *fbi)
> >  {
> >  	return fbi->devtype = IMX1_FB;
> > @@ -324,6 +346,9 @@ static const struct imx_fb_videomode *imxfb_find_mode(struct imxfb_info *fbi)
> >  	struct imx_fb_videomode *m;
> >  	int i;
> >  
> > +	if (!fb_mode)
> > +		return &fbi->mode[0];
> > +
> >  	for (i = 0, m = &fbi->mode[0]; i < fbi->num_modes; i++, m++) {
> >  		if (!strcmp(m->mode.name, fb_mode))
> >  			return m;
> > @@ -479,6 +504,9 @@ static int imxfb_bl_update_status(struct backlight_device *bl)
> >  	struct imxfb_info *fbi = bl_get_data(bl);
> >  	int brightness = bl->props.brightness;
> >  
> > +	if (!fbi->pwmr)
> > +		return 0;
> > +
> >  	if (bl->props.power != FB_BLANK_UNBLANK)
> >  		brightness = 0;
> >  	if (bl->props.fb_blank != FB_BLANK_UNBLANK)
> > @@ -719,7 +747,8 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
> >  
> >  	writel(fbi->pcr, fbi->regs + LCDC_PCR);
> >  #ifndef PWMR_BACKLIGHT_AVAILABLE
> > -	writel(fbi->pwmr, fbi->regs + LCDC_PWMR);
> > +	if (fbi->pwmr)
> > +		writel(fbi->pwmr, fbi->regs + LCDC_PWMR);
> >  #endif
> >  	writel(fbi->lscr1, fbi->regs + LCDC_LSCR1);
> >  	writel(fbi->dmacr, fbi->regs + LCDC_DMACR);
> > @@ -758,13 +787,12 @@ static int imxfb_resume(struct platform_device *dev)
> >  #define imxfb_resume	NULL
> >  #endif
> >  
> > -static int __init imxfb_init_fbinfo(struct platform_device *pdev)
> > +static int imxfb_init_fbinfo(struct platform_device *pdev)
> >  {
> >  	struct imx_fb_platform_data *pdata = pdev->dev.platform_data;
> >  	struct fb_info *info = dev_get_drvdata(&pdev->dev);
> >  	struct imxfb_info *fbi = info->par;
> > -	struct imx_fb_videomode *m;
> > -	int i;
> > +	struct device_node *np;
> >  
> >  	pr_debug("%s\n",__func__);
> >  
> > @@ -795,41 +823,96 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
> >  	info->fbops			= &imxfb_ops;
> >  	info->flags			= FBINFO_FLAG_DEFAULT |
> >  					  FBINFO_READS_FAST;
> > -	info->var.grayscale		= pdata->cmap_greyscale;
> > -	fbi->cmap_inverse		= pdata->cmap_inverse;
> > -	fbi->cmap_static		= pdata->cmap_static;
> > -	fbi->lscr1			= pdata->lscr1;
> > -	fbi->dmacr			= pdata->dmacr;
> > -	fbi->pwmr			= pdata->pwmr;
> > -	fbi->lcd_power			= pdata->lcd_power;
> > -	fbi->backlight_power		= pdata->backlight_power;
> > -
> > -	for (i = 0, m = &pdata->mode[0]; i < pdata->num_modes; i++, m++)
> > -		info->fix.smem_len = max_t(size_t, info->fix.smem_len,
> > -				m->mode.xres * m->mode.yres * m->bpp / 8);
> > +	if (pdata) {
> > +		info->var.grayscale		= pdata->cmap_greyscale;
> > +		fbi->cmap_inverse		= pdata->cmap_inverse;
> > +		fbi->cmap_static		= pdata->cmap_static;
> > +		fbi->lscr1			= pdata->lscr1;
> > +		fbi->dmacr			= pdata->dmacr;
> > +		fbi->pwmr			= pdata->pwmr;
> > +		fbi->lcd_power			= pdata->lcd_power;
> > +		fbi->backlight_power		= pdata->backlight_power;
> > +	} else {
> > +		np = pdev->dev.of_node;
> > +		info->var.grayscale = of_property_read_bool(np,
> > +						"cmap-greyscale");
> > +		fbi->cmap_inverse = of_property_read_bool(np, "cmap-inverse");
> > +		fbi->cmap_static = of_property_read_bool(np, "cmap-static");
> > +
> > +		fbi->lscr1 = IMXFB_LSCR1_DEFAULT;
> > +		if (of_property_read_bool(np, "dmacr-eukrea"))
> > +			fbi->dmacr = IMXFB_DMACR_EUKREA_DEFAULT;
> > +		else
> > +			fbi->dmacr = IMXFB_DMACR_DEFAULT;
> > +
> > +		/* These two function pointers could be used by some specific
> > +		 * platforms. */
> > +		fbi->lcd_power = NULL;
> > +		fbi->backlight_power = NULL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int imxfb_of_read_mode(struct device *dev, struct device_node *np,
> > +		struct imx_fb_videomode *imxfb_mode)
> > +{
> > +	int ret;
> > +	struct fb_videomode *of_mode = &imxfb_mode->mode;
> > +	u32 bpp;
> > +	u32 pcr;
> > +
> > +	ret = of_property_read_string(np, "model", &of_mode->name);
> > +	if (ret)
> > +		of_mode->name = NULL;
> > +
> > +	ret = of_get_fb_videomode(np, of_mode, OF_USE_NATIVE_MODE);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to get videomode from DT\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = of_property_read_u32(np, "fsl,bpp", &bpp);
> > +	ret |= of_property_read_u32(np, "fsl,pcr", &pcr);
> > +
> > +	if (ret) {
> > +		dev_err(dev, "Failed to read bpp and pcr from DT\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (bpp < 1 || bpp > 255) {
> > +		dev_err(dev, "Bits per pixel have to be between 1 and 255\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	imxfb_mode->bpp = bpp;
> > +	imxfb_mode->pcr = pcr;
> >  
> >  	return 0;
> >  }
> >  
> > -static int __init imxfb_probe(struct platform_device *pdev)
> > +static int imxfb_probe(struct platform_device *pdev)
> >  {
> >  	struct imxfb_info *fbi;
> >  	struct fb_info *info;
> >  	struct imx_fb_platform_data *pdata;
> >  	struct resource *res;
> > +	struct imx_fb_videomode *m;
> > +	const struct of_device_id *of_id;
> >  	int ret, i;
> > +	int bytes_per_pixel;
> >  
> >  	dev_info(&pdev->dev, "i.MX Framebuffer driver\n");
> >  
> > +	of_id = of_match_device(imxfb_of_dev_id, &pdev->dev);
> > +	if (of_id)
> > +		pdev->id_entry = of_id->data;
> > +
> >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >  	if (!res)
> >  		return -ENODEV;
> >  
> >  	pdata = pdev->dev.platform_data;
> > -	if (!pdata) {
> > -		dev_err(&pdev->dev,"No platform_data available\n");
> > -		return -ENOMEM;
> > -	}
> >  
> >  	info = framebuffer_alloc(sizeof(struct imxfb_info), &pdev->dev);
> >  	if (!info)
> > @@ -837,15 +920,55 @@ static int __init imxfb_probe(struct platform_device *pdev)
> >  
> >  	fbi = info->par;
> >  
> > -	if (!fb_mode)
> > -		fb_mode = pdata->mode[0].mode.name;
> > -
> >  	platform_set_drvdata(pdev, info);
> >  
> >  	ret = imxfb_init_fbinfo(pdev);
> >  	if (ret < 0)
> >  		goto failed_init;
> >  
> > +	if (pdata) {
> > +		if (!fb_mode)
> > +			fb_mode = pdata->mode[0].mode.name;
> > +
> > +		fbi->mode = pdata->mode;
> > +		fbi->num_modes = pdata->num_modes;
> > +	} else {
> > +		struct device_node *display_np;
> > +		fb_mode = NULL;
> > +
> > +		display_np = of_parse_phandle(pdev->dev.of_node, "display", 0);
> > +		if (!display_np) {
> > +			dev_err(&pdev->dev, "No display defined in devicetree\n");
> > +			ret = -EINVAL;
> > +			goto failed_of_parse;
> > +		}
> > +
> > +		/*
> > +		 * imxfb does not support more modes, we choose only the native
> > +		 * mode.
> > +		 */
> > +		fbi->num_modes = 1;
> > +
> > +		fbi->mode = devm_kzalloc(&pdev->dev,
> > +				sizeof(struct imx_fb_videomode), GFP_KERNEL);
> > +		if (!fbi->mode) {
> > +			ret = -ENOMEM;
> > +			goto failed_of_parse;
> > +		}
> > +
> > +		ret = imxfb_of_read_mode(&pdev->dev, display_np, fbi->mode);
> > +		if (ret)
> > +			goto failed_of_parse;
> > +	}
> > +
> > +	/* Calculate maximum bytes used per pixel. In most cases this should
> > +	 * be the same as m->bpp/8 */
> > +	m = &fbi->mode[0];
> > +	bytes_per_pixel = (m->bpp + 7) / 8;
> > +	for (i = 0; i < fbi->num_modes; i++, m++)
> > +		info->fix.smem_len = max_t(size_t, info->fix.smem_len,
> > +				m->mode.xres * m->mode.yres * bytes_per_pixel);
> > +
> >  	res = request_mem_region(res->start, resource_size(res),
> >  				DRIVER_NAME);
> >  	if (!res) {
> > @@ -878,7 +1001,8 @@ static int __init imxfb_probe(struct platform_device *pdev)
> >  		goto failed_ioremap;
> >  	}
> >  
> > -	if (!pdata->fixed_screen_cpu) {
> > +	/* Seems not being used by anyone, so no support for oftree */
> > +	if (!pdata || !pdata->fixed_screen_cpu) {
> >  		fbi->map_size = PAGE_ALIGN(info->fix.smem_len);
> >  		fbi->map_cpu = dma_alloc_writecombine(&pdev->dev,
> >  				fbi->map_size, &fbi->map_dma, GFP_KERNEL);
> > @@ -903,18 +1027,16 @@ static int __init imxfb_probe(struct platform_device *pdev)
> >  		info->fix.smem_start = fbi->screen_dma;
> >  	}
> >  
> > -	if (pdata->init) {
> > +	if (pdata && pdata->init) {
> >  		ret = pdata->init(fbi->pdev);
> >  		if (ret)
> >  			goto failed_platform_init;
> >  	}
> >  
> > -	fbi->mode = pdata->mode;
> > -	fbi->num_modes = pdata->num_modes;
> >  
> >  	INIT_LIST_HEAD(&info->modelist);
> > -	for (i = 0; i < pdata->num_modes; i++)
> > -		fb_add_videomode(&pdata->mode[i].mode, &info->modelist);
> > +	for (i = 0; i < fbi->num_modes; i++)
> > +		fb_add_videomode(&fbi->mode[i].mode, &info->modelist);
> >  
> >  	/*
> >  	 * This makes sure that our colour bitfield
> > @@ -944,10 +1066,10 @@ static int __init imxfb_probe(struct platform_device *pdev)
> >  failed_register:
> >  	fb_dealloc_cmap(&info->cmap);
> >  failed_cmap:
> > -	if (pdata->exit)
> > +	if (pdata && pdata->exit)
> >  		pdata->exit(fbi->pdev);
> >  failed_platform_init:
> > -	if (!pdata->fixed_screen_cpu)
> > +	if (pdata && !pdata->fixed_screen_cpu)
> >  		dma_free_writecombine(&pdev->dev,fbi->map_size,fbi->map_cpu,
> >  			fbi->map_dma);
> >  failed_map:
> > @@ -956,6 +1078,7 @@ failed_ioremap:
> >  failed_getclock:
> >  	release_mem_region(res->start, resource_size(res));
> >  failed_req:
> > +failed_of_parse:
> >  	kfree(info->pseudo_palette);
> >  failed_init:
> >  	platform_set_drvdata(pdev, NULL);
> > @@ -980,7 +1103,7 @@ static int imxfb_remove(struct platform_device *pdev)
> >  	unregister_framebuffer(info);
> >  
> >  	pdata = pdev->dev.platform_data;
> > -	if (pdata->exit)
> > +	if (pdata && pdata->exit)
> >  		pdata->exit(fbi->pdev);
> >  
> >  	fb_dealloc_cmap(&info->cmap);
> > @@ -1009,6 +1132,7 @@ static struct platform_driver imxfb_driver = {
> >  	.shutdown	= imxfb_shutdown,
> >  	.driver		= {
> >  		.name	= DRIVER_NAME,
> > +		.of_match_table = imxfb_of_dev_id,
> >  	},
> >  	.id_table	= imxfb_devtype,
> >  };
> > -- 
> > 1.8.1.5
> > 
> > _______________________________________________
> > devicetree-discuss mailing list
> > devicetree-discuss@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/devicetree-discuss
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCH 1/8] video: atmel_lcdfb: fix platform data struct
From: Hans-Christian Egtvedt @ 2013-04-12  9:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1365692422-9565-1-git-send-email-plagnioj@jcrosoft.com>

Around Thu 11 Apr 2013 17:00:15 +0200 or thereabout, Jean-Christophe PLAGNIOL-VILLARD wrote:
> Today we mix pdata and drivers data in the struct atmel_lcdfb_info
> Fix it and introduce a new struct atmel_lcdfb_pdata for platform data only
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Hans-Christian Egtvedt <egtvedt@samfundet.no>

For the AVR32 bits

Acked-by: Hans-Christian Egtvedt <egtvedt@samfundet.no>

> ---
>  arch/arm/mach-at91/at91sam9261_devices.c    |    6 +-
>  arch/arm/mach-at91/at91sam9263_devices.c    |    6 +-
>  arch/arm/mach-at91/at91sam9g45_devices.c    |    6 +-
>  arch/arm/mach-at91/at91sam9rl_devices.c     |    6 +-
>  arch/arm/mach-at91/board-sam9261ek.c        |    6 +-
>  arch/arm/mach-at91/board-sam9263ek.c        |    4 +-
>  arch/arm/mach-at91/board-sam9m10g45ek.c     |    4 +-
>  arch/arm/mach-at91/board-sam9rlek.c         |    4 +-
>  arch/arm/mach-at91/board.h                  |    4 +-
>  arch/avr32/boards/atngw100/evklcd10x.c      |    6 +-
>  arch/avr32/boards/atngw100/mrmt.c           |    4 +-
>  arch/avr32/boards/atstk1000/atstk1000.h     |    2 +-
>  arch/avr32/boards/atstk1000/setup.c         |    2 +-
>  arch/avr32/boards/favr-32/setup.c           |    2 +-
>  arch/avr32/boards/hammerhead/setup.c        |    2 +-
>  arch/avr32/boards/merisc/display.c          |    2 +-
>  arch/avr32/boards/mimc200/setup.c           |    4 +-
>  arch/avr32/mach-at32ap/at32ap700x.c         |    8 +--
>  arch/avr32/mach-at32ap/include/mach/board.h |    4 +-
>  drivers/video/atmel_lcdfb.c                 |  104 +++++++++++++++++----------
>  include/video/atmel_lcdc.h                  |   24 +------
>  21 files changed, 109 insertions(+), 101 deletions(-)

<snipp diff>

-- 
HcE

^ permalink raw reply

* [PATCH 2/2] ARM: dts: cfa10036: Change the OLED display to SSD1306
From: Maxime Ripard @ 2013-04-12  8:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1365753603-4438-1-git-send-email-maxime.ripard@free-electrons.com>

The SSD1307 was used in an early prototype that will never get
distributed. The final board now has a SSD1306 instead, that has its own
power generation unit and thus doesn't need any PWM. The panel attached
to it also changed.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/imx28-cfa10036.dts |   14 +++++---------
 arch/arm/boot/dts/imx28-cfa10049.dts |    4 ++--
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/arch/arm/boot/dts/imx28-cfa10036.dts b/arch/arm/boot/dts/imx28-cfa10036.dts
index 1594694..40488f5a8 100644
--- a/arch/arm/boot/dts/imx28-cfa10036.dts
+++ b/arch/arm/boot/dts/imx28-cfa10036.dts
@@ -58,12 +58,6 @@
 		};
 
 		apbx@80040000 {
-			pwm: pwm@80064000 {
-				pinctrl-names = "default";
-				pinctrl-0 = <&pwm4_pins_a>;
-				status = "okay";
-			};
-
 			duart: serial@80074000 {
 				pinctrl-names = "default";
 				pinctrl-0 = <&duart_pins_b>;
@@ -75,11 +69,13 @@
 				pinctrl-0 = <&i2c0_pins_b>;
 				status = "okay";
 
-				ssd1307: oled@3c {
-					compatible = "solomon,ssd1307fb-i2c";
+				ssd1306: oled@3c {
+					compatible = "solomon,ssd1306fb-i2c";
 					reg = <0x3c>;
-					pwms = <&pwm 4 3000>;
 					reset-gpios = <&gpio2 7 0>;
+					solomon,height = <32>;
+					solomon,width = <128>;
+					solomon,page-offset = <0>;
 				};
 			};
 		};
diff --git a/arch/arm/boot/dts/imx28-cfa10049.dts b/arch/arm/boot/dts/imx28-cfa10049.dts
index a0d3e9f..ded7784 100644
--- a/arch/arm/boot/dts/imx28-cfa10049.dts
+++ b/arch/arm/boot/dts/imx28-cfa10049.dts
@@ -132,8 +132,8 @@
 
 		apbx@80040000 {
 			pwm: pwm@80064000 {
-				pinctrl-names = "default", "default";
-				pinctrl-1 = <&pwm3_pins_b>;
+				pinctrl-names = "default";
+				pinctrl-0 = <&pwm3_pins_b>;
 				status = "okay";
 			};
 
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 1/2] video: ssd1307fb: Add support for SSD1306 OLED controller
From: Maxime Ripard @ 2013-04-12  8:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1365753603-4438-1-git-send-email-maxime.ripard@free-electrons.com>

The Solomon SSD1306 OLED controller is very similar to the SSD1307,
except for the fact that the power is given through an external PWM for
the 1307, and while the 1306 can generate its own power without any PWM.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 .../devicetree/bindings/video/ssd1307fb.txt        |   10 +-
 drivers/video/ssd1307fb.c                          |  267 ++++++++++++++------
 2 files changed, 203 insertions(+), 74 deletions(-)

diff --git a/Documentation/devicetree/bindings/video/ssd1307fb.txt b/Documentation/devicetree/bindings/video/ssd1307fb.txt
index 3d0060c..7a12542 100644
--- a/Documentation/devicetree/bindings/video/ssd1307fb.txt
+++ b/Documentation/devicetree/bindings/video/ssd1307fb.txt
@@ -1,13 +1,17 @@
 * Solomon SSD1307 Framebuffer Driver
 
 Required properties:
-  - compatible: Should be "solomon,ssd1307fb-<bus>". The only supported bus for
-    now is i2c.
+  - compatible: Should be "solomon,<chip>fb-<bus>". The only supported bus for
+    now is i2c, and the supported chips are ssd1306 and ssd1307.
   - reg: Should contain address of the controller on the I2C bus. Most likely
          0x3c or 0x3d
   - pwm: Should contain the pwm to use according to the OF device tree PWM
-         specification [0]
+         specification [0]. Only required for the ssd1307.
   - reset-gpios: Should contain the GPIO used to reset the OLED display
+  - solomon,height: Height in pixel of the screen driven by the controller
+  - solomon,width: Width in pixel of the screen driven by the controller
+  - solomon,page-offset: Offset of pages (band of 8 pixels) that the screen is
+    mapped to.
 
 Optional properties:
   - reset-active-low: Is the reset gpio is active on physical low?
diff --git a/drivers/video/ssd1307fb.c b/drivers/video/ssd1307fb.c
index 395cb6a..95f76e2 100644
--- a/drivers/video/ssd1307fb.c
+++ b/drivers/video/ssd1307fb.c
@@ -16,24 +16,39 @@
 #include <linux/pwm.h>
 #include <linux/delay.h>
 
-#define SSD1307FB_WIDTH			96
-#define SSD1307FB_HEIGHT		16
-
 #define SSD1307FB_DATA			0x40
 #define SSD1307FB_COMMAND		0x80
 
 #define SSD1307FB_CONTRAST		0x81
+#define	SSD1307FB_CHARGE_PUMP		0x8d
 #define SSD1307FB_SEG_REMAP_ON		0xa1
 #define SSD1307FB_DISPLAY_OFF		0xae
+#define SSD1307FB_SET_MULTIPLEX_RATIO	0xa8
 #define SSD1307FB_DISPLAY_ON		0xaf
 #define SSD1307FB_START_PAGE_ADDRESS	0xb0
+#define SSD1307FB_SET_DISPLAY_OFFSET	0xd3
+#define	SSD1307FB_SET_CLOCK_FREQ	0xd5
+#define	SSD1307FB_SET_PRECHARGE_PERIOD	0xd9
+#define	SSD1307FB_SET_COM_PINS_CONFIG	0xda
+#define	SSD1307FB_SET_VCOMH		0xdb
+
+struct ssd1307fb_par;
+
+struct ssd1307fb_ops {
+	int (*init)(struct ssd1307fb_par *);
+	int (*remove)(struct ssd1307fb_par *);
+};
 
 struct ssd1307fb_par {
 	struct i2c_client *client;
+	u32 height;
 	struct fb_info *info;
+	struct ssd1307fb_ops *ops;
+	u32 page_offset;
 	struct pwm_device *pwm;
 	u32 pwm_period;
 	int reset;
+	u32 width;
 };
 
 static struct fb_fix_screeninfo ssd1307fb_fix = {
@@ -43,15 +58,10 @@ static struct fb_fix_screeninfo ssd1307fb_fix = {
 	.xpanstep	= 0,
 	.ypanstep	= 0,
 	.ywrapstep	= 0,
-	.line_length	= SSD1307FB_WIDTH / 8,
 	.accel		= FB_ACCEL_NONE,
 };
 
 static struct fb_var_screeninfo ssd1307fb_var = {
-	.xres		= SSD1307FB_WIDTH,
-	.yres		= SSD1307FB_HEIGHT,
-	.xres_virtual	= SSD1307FB_WIDTH,
-	.yres_virtual	= SSD1307FB_HEIGHT,
 	.bits_per_pixel	= 1,
 };
 
@@ -134,16 +144,16 @@ static void ssd1307fb_update_display(struct ssd1307fb_par *par)
 	 *  (5) A4 B4 C4 D4 E4 F4 G4 H4
 	 */
 
-	for (i = 0; i < (SSD1307FB_HEIGHT / 8); i++) {
-		ssd1307fb_write_cmd(par->client, SSD1307FB_START_PAGE_ADDRESS + (i + 1));
+	for (i = 0; i < (par->height / 8); i++) {
+		ssd1307fb_write_cmd(par->client, SSD1307FB_START_PAGE_ADDRESS + i + par->page_offset);
 		ssd1307fb_write_cmd(par->client, 0x00);
 		ssd1307fb_write_cmd(par->client, 0x10);
 
-		for (j = 0; j < SSD1307FB_WIDTH; j++) {
+		for (j = 0; j < par->width; j++) {
 			u8 buf = 0;
 			for (k = 0; k < 8; k++) {
-				u32 page_length = SSD1307FB_WIDTH * i;
-				u32 index = page_length + (SSD1307FB_WIDTH * k + j) / 8;
+				u32 page_length = par->width * i;
+				u32 index = page_length + (par->width * k + j) / 8;
 				u8 byte = *(vmem + index);
 				u8 bit = byte & (1 << (j % 8));
 				bit = bit >> (j % 8);
@@ -227,16 +237,137 @@ static struct fb_deferred_io ssd1307fb_defio = {
 	.deferred_io	= ssd1307fb_deferred_io,
 };
 
+static int ssd1307fb_ssd1307_init(struct ssd1307fb_par *par) {
+	int ret;
+
+	par->pwm = pwm_get(&par->client->dev, NULL);
+	if (IS_ERR(par->pwm)) {
+		dev_err(&par->client->dev, "Could not get PWM from device tree!\n");
+		return PTR_ERR(par->pwm);
+	}
+
+	par->pwm_period = pwm_get_period(par->pwm);
+	/* Enable the PWM */
+	pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period);
+	pwm_enable(par->pwm);
+
+	dev_dbg(&par->client->dev, "Using PWM%d with a %dns period.\n", par->pwm->pwm, par->pwm_period);
+
+	/* Map column 127 of the OLED to segment 0 */
+	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SEG_REMAP_ON);
+	if (ret < 0)
+		return ret;
+
+	/* Turn on the display */
+	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_ON);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int ssd1307fb_ssd1307_remove(struct ssd1307fb_par *par) {
+	pwm_disable(par->pwm);
+	pwm_put(par->pwm);
+	return 0;
+}
+
+static struct ssd1307fb_ops ssd1307fb_ssd1307_ops = {
+	.init	= ssd1307fb_ssd1307_init,
+	.remove	= ssd1307fb_ssd1307_remove,
+};
+
+static int ssd1307fb_ssd1306_init(struct ssd1307fb_par *par) {
+	int ret;
+
+	/* Set initial contrast */
+	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_CONTRAST);
+	ret = ret & ssd1307fb_write_cmd(par->client, 0x7f);
+	if (ret < 0)
+		return ret;
+
+	/* Set COM direction */
+	ret = ssd1307fb_write_cmd(par->client, 0xc8);
+	if (ret < 0)
+		return ret;
+
+	/* Set segment re-map */
+	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SEG_REMAP_ON);
+	if (ret < 0)
+		return ret;
+
+	/* Set multiplex ratio value */
+	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_MULTIPLEX_RATIO);
+	ret = ret & ssd1307fb_write_cmd(par->client, par->height - 1);
+	if (ret < 0)
+		return ret;
+
+	/* set display offset value */
+	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_DISPLAY_OFFSET);
+	ret = ssd1307fb_write_cmd(par->client, 0x20);
+	if (ret < 0)
+		return ret;
+
+	/* Set clock frequency */
+	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_CLOCK_FREQ);
+	ret = ret & ssd1307fb_write_cmd(par->client, 0xf0);
+	if (ret < 0)
+		return ret;
+
+	/* Set precharge period in number of ticks from the internal clock */
+	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_PRECHARGE_PERIOD);
+	ret = ret & ssd1307fb_write_cmd(par->client, 0x22);
+	if (ret < 0)
+		return ret;
+
+	/* Set COM pins configuration */
+	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_COM_PINS_CONFIG);
+	ret = ret & ssd1307fb_write_cmd(par->client, 0x22);
+	if (ret < 0)
+		return ret;
+
+	/* Set VCOMH */
+	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_VCOMH);
+	ret = ret & ssd1307fb_write_cmd(par->client, 0x49);
+	if (ret < 0)
+		return ret;
+
+	/* Turn on the DC-DC Charge Pump */
+	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_CHARGE_PUMP);
+	ret = ret & ssd1307fb_write_cmd(par->client, 0x14);
+	if (ret < 0)
+		return ret;
+
+	/* Turn on the display */
+	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_ON);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static struct ssd1307fb_ops ssd1307fb_ssd1306_ops = {
+	.init	= ssd1307fb_ssd1306_init,
+};
+
+static const struct of_device_id ssd1307fb_of_match[] = {
+	{ .compatible = "solomon,ssd1306fb-i2c", .data = (void*)&ssd1307fb_ssd1306_ops },
+	{ .compatible = "solomon,ssd1307fb-i2c", .data = (void*)&ssd1307fb_ssd1307_ops },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ssd1307fb_of_match);
+
 static int ssd1307fb_probe(struct i2c_client *client,
 			   const struct i2c_device_id *id)
 {
 	struct fb_info *info;
-	u32 vmem_size = SSD1307FB_WIDTH * SSD1307FB_HEIGHT / 8;
+	struct device_node *node = client->dev.of_node;
+	u32 vmem_size;
 	struct ssd1307fb_par *par;
 	u8 *vmem;
 	int ret;
 
-	if (!client->dev.of_node) {
+	if (!node) {
 		dev_err(&client->dev, "No device tree data found!\n");
 		return -EINVAL;
 	}
@@ -247,6 +378,36 @@ static int ssd1307fb_probe(struct i2c_client *client,
 		return -ENOMEM;
 	}
 
+	par = info->par;
+	par->info = info;
+	par->client = client;
+
+	par->ops = (struct ssd1307fb_ops*)of_match_device(ssd1307fb_of_match, &client->dev)->data;
+
+	par->reset = of_get_named_gpio(client->dev.of_node,
+					 "reset-gpios", 0);
+	if (!gpio_is_valid(par->reset)) {
+		ret = -EINVAL;
+		goto fb_alloc_error;
+	}
+
+	if (of_property_read_u32(node, "solomon,width", &par->width))
+		par->width = 96;
+
+	printk("Width\t%u\n", par->width);
+
+	if (of_property_read_u32(node, "solomon,height", &par->height))
+		par->width = 16;
+
+	printk("Height\t%u\n", par->height);
+
+	if (of_property_read_u32(node, "solomon,page-offset", &par->page_offset))
+		par->page_offset = 1;
+
+	printk("Offset\t%u\n", par->page_offset);
+
+	vmem_size = par->width * par->height / 8;
+
 	vmem = devm_kzalloc(&client->dev, vmem_size, GFP_KERNEL);
 	if (!vmem) {
 		dev_err(&client->dev, "Couldn't allocate graphical memory.\n");
@@ -256,9 +417,15 @@ static int ssd1307fb_probe(struct i2c_client *client,
 
 	info->fbops = &ssd1307fb_ops;
 	info->fix = ssd1307fb_fix;
+	info->fix.line_length = par->width / 8;
 	info->fbdefio = &ssd1307fb_defio;
 
 	info->var = ssd1307fb_var;
+	info->var.xres = par->width;
+	info->var.xres_virtual = par->width;
+	info->var.yres = par->height;
+	info->var.yres_virtual = par->height;
+
 	info->var.red.length = 1;
 	info->var.red.offset = 0;
 	info->var.green.length = 1;
@@ -272,17 +439,6 @@ static int ssd1307fb_probe(struct i2c_client *client,
 
 	fb_deferred_io_init(info);
 
-	par = info->par;
-	par->info = info;
-	par->client = client;
-
-	par->reset = of_get_named_gpio(client->dev.of_node,
-					 "reset-gpios", 0);
-	if (!gpio_is_valid(par->reset)) {
-		ret = -EINVAL;
-		goto reset_oled_error;
-	}
-
 	ret = devm_gpio_request_one(&client->dev, par->reset,
 				    GPIOF_OUT_INIT_HIGH,
 				    "oled-reset");
@@ -293,23 +449,6 @@ static int ssd1307fb_probe(struct i2c_client *client,
 		goto reset_oled_error;
 	}
 
-	par->pwm = pwm_get(&client->dev, NULL);
-	if (IS_ERR(par->pwm)) {
-		dev_err(&client->dev, "Could not get PWM from device tree!\n");
-		ret = PTR_ERR(par->pwm);
-		goto pwm_error;
-	}
-
-	par->pwm_period = pwm_get_period(par->pwm);
-
-	dev_dbg(&client->dev, "Using PWM%d with a %dns period.\n", par->pwm->pwm, par->pwm_period);
-
-	ret = register_framebuffer(info);
-	if (ret) {
-		dev_err(&client->dev, "Couldn't register the framebuffer\n");
-		goto fbreg_error;
-	}
-
 	i2c_set_clientdata(client, info);
 
 	/* Reset the screen */
@@ -318,34 +457,25 @@ static int ssd1307fb_probe(struct i2c_client *client,
 	gpio_set_value(par->reset, 1);
 	udelay(4);
 
-	/* Enable the PWM */
-	pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period);
-	pwm_enable(par->pwm);
-
-	/* Map column 127 of the OLED to segment 0 */
-	ret = ssd1307fb_write_cmd(client, SSD1307FB_SEG_REMAP_ON);
-	if (ret < 0) {
-		dev_err(&client->dev, "Couldn't remap the screen.\n");
-		goto remap_error;
+	if (par->ops->init) {
+		ret = par->ops->init(par);
+		if (ret)
+			goto reset_oled_error;
 	}
 
-	/* Turn on the display */
-	ret = ssd1307fb_write_cmd(client, SSD1307FB_DISPLAY_ON);
-	if (ret < 0) {
-		dev_err(&client->dev, "Couldn't turn the display on.\n");
-		goto remap_error;
+	ret = register_framebuffer(info);
+	if (ret) {
+		dev_err(&client->dev, "Couldn't register the framebuffer\n");
+		goto panel_init_error;
 	}
 
 	dev_info(&client->dev, "fb%d: %s framebuffer device registered, using %d bytes of video memory\n", info->node, info->fix.id, vmem_size);
 
 	return 0;
 
-remap_error:
-	unregister_framebuffer(info);
-	pwm_disable(par->pwm);
-fbreg_error:
-	pwm_put(par->pwm);
-pwm_error:
+panel_init_error:
+	if (par->ops->remove)
+		par->ops->remove(par);
 reset_oled_error:
 	fb_deferred_io_cleanup(info);
 fb_alloc_error:
@@ -359,8 +489,8 @@ static int ssd1307fb_remove(struct i2c_client *client)
 	struct ssd1307fb_par *par = info->par;
 
 	unregister_framebuffer(info);
-	pwm_disable(par->pwm);
-	pwm_put(par->pwm);
+	if (par->ops->remove)
+		par->ops->remove(par);
 	fb_deferred_io_cleanup(info);
 	framebuffer_release(info);
 
@@ -368,17 +498,12 @@ static int ssd1307fb_remove(struct i2c_client *client)
 }
 
 static const struct i2c_device_id ssd1307fb_i2c_id[] = {
+	{ "ssd1306fb", 0 },
 	{ "ssd1307fb", 0 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, ssd1307fb_i2c_id);
 
-static const struct of_device_id ssd1307fb_of_match[] = {
-	{ .compatible = "solomon,ssd1307fb-i2c" },
-	{},
-};
-MODULE_DEVICE_TABLE(of, ssd1307fb_of_match);
-
 static struct i2c_driver ssd1307fb_driver = {
 	.probe = ssd1307fb_probe,
 	.remove = ssd1307fb_remove,
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH RESEND 0/2] Add support for the Solomon SSD1306 OLED Controller
From: Maxime Ripard @ 2013-04-12  8:00 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

This patchset adds the needed bits to the ssd1307fb driver to support the
very similar Solomon SSD1306 controller.

Since the SSD1307 has been replaced by the SSD1306 in the latest CFA-10036
production samples, we also need to change the device tree accordingly.

Thanks,
Maxime

Maxime Ripard (2):
  video: ssd1307fb: Add support for SSD1306 OLED controller
  ARM: dts: cfa10036: Change the OLED display to SSD1306

 .../devicetree/bindings/video/ssd1307fb.txt        |   10 +-
 arch/arm/boot/dts/imx28-cfa10036.dts               |   14 +-
 arch/arm/boot/dts/imx28-cfa10049.dts               |    4 +-
 drivers/video/ssd1307fb.c                          |  267 ++++++++++++++------
 4 files changed, 210 insertions(+), 85 deletions(-)

-- 
1.7.10.4


^ permalink raw reply

* Re: [PATCH V2] video: implement a simple framebuffer driver
From: Stephen Warren @ 2013-04-11 20:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1828875.4dYa7WRt67@avalon>

On 04/11/2013 02:06 PM, Laurent Pinchart wrote:
> Hi Stephen,
> 
> On Thursday 11 April 2013 10:06:49 Stephen Warren wrote:
>> On 04/11/2013 03:56 AM, Laurent Pinchart wrote:
>>> On Monday 08 April 2013 17:16:37 Andrew Morton wrote:
>>>> On Wed,  3 Apr 2013 20:39:43 -0600 Stephen Warren wrote:
>>>>> A simple frame-buffer describes a raw memory region that may be rendered
>>>>> to, with the assumption that the display hardware has already been set
>>>>> up to scan out from that buffer.
>>>>>
>>>>> This is useful in cases where a bootloader exists and has set up the
>>>>> display hardware, but a Linux driver doesn't yet exist for the display
>>>>> hardware.
>>>>>
>>>>> ...
>>>>>
>>>>> +config FB_SIMPLE
>>>>> +	bool "Simple framebuffer support"
>>>>> +	depends on (FB = y) && OF
>>>>
>>>> It's sad that this simple little thing requires Open Firmware.  Could
>>>> it be generalised in some way so that the small amount of setup info
>>>> could be provided by other means (eg, module_param) or does the
>>>> dependency go deeper than that?
>>>
>>> I second that request. I like the idea of a simple framebuffer driver if
>>> it helps deprecating fbdev in the long term, but I don't want it to offer
>>> an excuse not to implement a DRM/KMS driver. In particular adding DT
>>> bindings would force us to keep supporting the ABI for a (too) long time.
>>
>> The platforms I intend to use this with only support device tree. Adding
>> support for platform data right now would just be dead code. If somebody
>> wants to use this driver with a board file based system rather than a DT
>> based system, it would be trivial do modify the driver to add a platform
>> data structure at that time.
> 
> What about using module parameters instead of DT bindings and platform data ? 
> As this is a hack designed to work around the lack of a proper display driver 
> the frame buffer address and size could just be passed by the boot loader 
> through the kernel command line, and the device would then be instantiated by 
> the driver.

I'm afraid I strongly dislike the idea of using module parameters. One
of the things that I dislike the most about NVIDIA's downstream Android
kernels is the huge number of random parameters that are required on the
kernel command-line just to get it to boot.

I'd specifically prefer the configuration for this driver to be a device
tree binding so that it /is/ an ABI. That way, arbitrary software that
boots the Linux kernel will be able to target a common standard for how
to pass this information to the kernel. If we move to module parameters
instead, especially with the specific intent that the format of those
parameters is no longer an ABI, we run in to issues where a new kernel
requires a bootloader update in order to generate the new set of module
parameters that are required by the driver. If we say the module
parameters will never change except in a backwards-compatible fashion,
and hence that issue won't arise, then why not just make it a DT binding?

Do module parameters handle the case of multiple device instances?

Also, I really don't see this driver as a hack. It's a perfectly
reasonable situation to have some other SW set up the frame-buffer, and
Linux simply continue to use it. Perhaps that other SW even ran on a
difference CPU, or the parameters are hard-coded in HW design, and so
there's no HW that Linux ever could drive, so it just isn't possible to
create any more advanced driver.

^ permalink raw reply

* Re: [PATCH v2] backlight: platform_lcd: introduce probe callback
From: Doug Anderson @ 2013-04-11 20:30 UTC (permalink / raw)
  To: Andrew Bresticker
  Cc: linux-fbdev, Richard Purdie, Florian Tobias Schandinat,
	linux-kernel@vger.kernel.org
In-Reply-To: <1365711890-15168-1-git-send-email-abrestic@chromium.org>

Andrew,

On Thu, Apr 11, 2013 at 1:24 PM, Andrew Bresticker
<abrestic@chromium.org> wrote:
> Platform LCD devices may need to do some device-specific
> initialization before they can be used (regulator or GPIO setup,
> for example), but currently the driver does not support any way of
> doing this.  This patch adds a probe() callback to plat_lcd_data
> which platform LCD devices can set to indicate that device-specific
> initialization is needed.
>
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> ---
>  drivers/video/backlight/platform_lcd.c | 6 ++++++
>  include/video/platform_lcd.h           | 1 +
>  2 files changed, 7 insertions(+)

Reviewed-by: Doug Anderson <dianders@chromium.org>

^ permalink raw reply

* [PATCH v2] backlight: platform_lcd: introduce probe callback
From: Andrew Bresticker @ 2013-04-11 20:24 UTC (permalink / raw)
  To: linux-fbdev
  Cc: Richard Purdie, Florian Tobias Schandinat, linux-kernel,
	Doug Anderson, Andrew Bresticker
In-Reply-To: <1365705391-23181-1-git-send-email-abrestic@chromium.org>

Platform LCD devices may need to do some device-specific
initialization before they can be used (regulator or GPIO setup,
for example), but currently the driver does not support any way of
doing this.  This patch adds a probe() callback to plat_lcd_data
which platform LCD devices can set to indicate that device-specific
initialization is needed.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
---
 drivers/video/backlight/platform_lcd.c | 6 ++++++
 include/video/platform_lcd.h           | 1 +
 2 files changed, 7 insertions(+)

diff --git a/drivers/video/backlight/platform_lcd.c b/drivers/video/backlight/platform_lcd.c
index 17a6b83..f46180e 100644
--- a/drivers/video/backlight/platform_lcd.c
+++ b/drivers/video/backlight/platform_lcd.c
@@ -86,6 +86,12 @@ static int platform_lcd_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	if (pdata->probe) {
+		err = pdata->probe(pdata);
+		if (err)
+			return err;
+	}
+
 	plcd = devm_kzalloc(&pdev->dev, sizeof(struct platform_lcd),
 			    GFP_KERNEL);
 	if (!plcd) {
diff --git a/include/video/platform_lcd.h b/include/video/platform_lcd.h
index ad3bdfe..23864b2 100644
--- a/include/video/platform_lcd.h
+++ b/include/video/platform_lcd.h
@@ -15,6 +15,7 @@ struct plat_lcd_data;
 struct fb_info;
 
 struct plat_lcd_data {
+	int	(*probe)(struct plat_lcd_data *);
 	void	(*set_power)(struct plat_lcd_data *, unsigned int power);
 	int	(*match_fb)(struct plat_lcd_data *, struct fb_info *);
 };
-- 
1.8.1.3


^ permalink raw reply related

* Re: [PATCH] backlight: platform_lcd: introduce probe callback
From: Andrew Bresticker @ 2013-04-11 20:17 UTC (permalink / raw)
  To: Doug Anderson
  Cc: linux-fbdev, Richard Purdie, Florian Tobias Schandinat,
	linux-kernel@vger.kernel.org
In-Reply-To: <CAD=FV=X6BBWJMDzJR9tnjKj8xTZ3Ng9oS7PYtNdUenW7_86vqw@mail.gmail.com>

> It would probably be good not to print in the case of -EPROBE_DEFER?

Actually, I think I'll just kill the print all together since the
caller will print something anyways.

-Andrew

^ permalink raw reply

* Re: [PATCH V2] video: implement a simple framebuffer driver
From: Laurent Pinchart @ 2013-04-11 20:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5166DF99.2000308@wwwdotorg.org>

Hi Stephen,

On Thursday 11 April 2013 10:06:49 Stephen Warren wrote:
> On 04/11/2013 03:56 AM, Laurent Pinchart wrote:
> > On Monday 08 April 2013 17:16:37 Andrew Morton wrote:
> >> On Wed,  3 Apr 2013 20:39:43 -0600 Stephen Warren wrote:
> >>> A simple frame-buffer describes a raw memory region that may be rendered
> >>> to, with the assumption that the display hardware has already been set
> >>> up to scan out from that buffer.
> >>> 
> >>> This is useful in cases where a bootloader exists and has set up the
> >>> display hardware, but a Linux driver doesn't yet exist for the display
> >>> hardware.
> >>> 
> >>> ...
> >>> 
> >>> +config FB_SIMPLE
> >>> +	bool "Simple framebuffer support"
> >>> +	depends on (FB = y) && OF
> >> 
> >> It's sad that this simple little thing requires Open Firmware.  Could
> >> it be generalised in some way so that the small amount of setup info
> >> could be provided by other means (eg, module_param) or does the
> >> dependency go deeper than that?
> > 
> > I second that request. I like the idea of a simple framebuffer driver if
> > it helps deprecating fbdev in the long term, but I don't want it to offer
> > an excuse not to implement a DRM/KMS driver. In particular adding DT
> > bindings would force us to keep supporting the ABI for a (too) long time.
> 
> The platforms I intend to use this with only support device tree. Adding
> support for platform data right now would just be dead code. If somebody
> wants to use this driver with a board file based system rather than a DT
> based system, it would be trivial do modify the driver to add a platform
> data structure at that time.

What about using module parameters instead of DT bindings and platform data ? 
As this is a hack designed to work around the lack of a proper display driver 
the frame buffer address and size could just be passed by the boot loader 
through the kernel command line, and the device would then be instantiated by 
the driver.

> Adding support for a platform data structure won't remove the need for
> DT support in the driver; any platform that uses DT will always
> configure this driver through the DT binding irrespective of whether
> some other platform could configure it using platform_data.
> 
> I don't believe the DT bindings imply that they must be implemented by
> an FB driver rather than a KMS driver. It's just that it's much simpler
> to do so at present. If the whole FB subsystem goes away at some time,
> it should be possible to implement a simplest-possible KMS driver that
> supports the same DT binding. I didn't do it this way because supporting
> a pre-allocated FB in DRM/KMS apparently means implementing a custom
> memory allocator for this in the driver, which would be a lot of code
> overhead when right now the driver can just use the FB subsystem and
> simply return the address directly. The simplest possible FB driver
> appears much simpler (less code size, less maintenance) than the
> simplest possible KMS driver.
> 
> My inclination is that for many platforms, the bootloader support for
> graphics output will appear first (before the kernel's), and this driver
> will allow for the kernel to have a graphical console, allowing a more
> complete/useful system to be available earlier. In many cases, that
> window may be small; a DRM/KMS driver may appear soon after the basic
> CPU/board/... support, and then people can switch to using it if they want.
> 
> That said, I also don't really see a problem not implementing a DRM/KMS
> driver for a platform; a dumb frame-buffer works perfectly well for my
> needs. Nobody would be forced to continue using it once a better
> alternative existed.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [PATCH] backlight: platform_lcd: introduce probe callback
From: Doug Anderson @ 2013-04-11 19:14 UTC (permalink / raw)
  To: Andrew Bresticker
  Cc: linux-fbdev, Richard Purdie, Florian Tobias Schandinat,
	linux-kernel@vger.kernel.org
In-Reply-To: <1365705391-23181-1-git-send-email-abrestic@chromium.org>

Andrew,

On Thu, Apr 11, 2013 at 11:36 AM, Andrew Bresticker
<abrestic@chromium.org> wrote:
> +       if (pdata->probe) {
> +               err = pdata->probe(pdata);
> +               if (err) {
> +                       dev_err(dev, "platform probe failed: %d\n", err);

It would probably be good not to print in the case of -EPROBE_DEFER?

Otherwise this looks good to me.

-Doug

^ permalink raw reply

* [PATCH] backlight: platform_lcd: introduce probe callback
From: Andrew Bresticker @ 2013-04-11 18:36 UTC (permalink / raw)
  To: linux-fbdev
  Cc: Richard Purdie, Florian Tobias Schandinat, linux-kernel,
	Doug Anderson, Andrew Bresticker

Platform LCD devices may need to do some device-specific
initialization before they can be used (regulator or GPIO setup,
for example), but currently the driver does not support any way of
doing this.  This patch adds a probe() callback to plat_lcd_data
which platform LCD devices can set to indicate that device-specific
initialization is needed.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
---
 drivers/video/backlight/platform_lcd.c | 8 ++++++++
 include/video/platform_lcd.h           | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/video/backlight/platform_lcd.c b/drivers/video/backlight/platform_lcd.c
index 54d94de..2fb24a1 100644
--- a/drivers/video/backlight/platform_lcd.c
+++ b/drivers/video/backlight/platform_lcd.c
@@ -86,6 +86,14 @@ static int platform_lcd_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	if (pdata->probe) {
+		err = pdata->probe(pdata);
+		if (err) {
+			dev_err(dev, "platform probe failed: %d\n", err);
+			return err;
+		}
+	}
+
 	plcd = devm_kzalloc(&pdev->dev, sizeof(struct platform_lcd),
 			    GFP_KERNEL);
 	if (!plcd) {
diff --git a/include/video/platform_lcd.h b/include/video/platform_lcd.h
index ad3bdfe..23864b2 100644
--- a/include/video/platform_lcd.h
+++ b/include/video/platform_lcd.h
@@ -15,6 +15,7 @@ struct plat_lcd_data;
 struct fb_info;
 
 struct plat_lcd_data {
+	int	(*probe)(struct plat_lcd_data *);
 	void	(*set_power)(struct plat_lcd_data *, unsigned int power);
 	int	(*match_fb)(struct plat_lcd_data *, struct fb_info *);
 };
-- 
1.8.1.3


^ permalink raw reply related

* Re: [PATCH V2] video: implement a simple framebuffer driver
From: Stephen Warren @ 2013-04-11 16:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAMuHMdWFVi=AFR8prins0mees4BFUUXirCC2swyQPwGs7Tt8uQ@mail.gmail.com>

On 04/11/2013 04:42 AM, Geert Uytterhoeven wrote:
> On Thu, Apr 4, 2013 at 4:39 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> +       { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} },
> 
> Why "r5g6b5" instead of "rgb565", which is what's commonly used?

Both representations appear commonly used.

I mentioned my rationale in response to V1: "r5g6b5" is a much more
well-defined structure. It's directly algorithmically parse-able if
required, whereas you'd need somewhat more complex heuristics to
determine exactly what rgb565 or argb2101010 mean, since all the numbers
are run together without delimiters.

^ permalink raw reply

* Re: [PATCH V2] video: implement a simple framebuffer driver
From: Stephen Warren @ 2013-04-11 16:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2569832.XQDrhUGNaI@avalon>

On 04/11/2013 03:56 AM, Laurent Pinchart wrote:
> On Monday 08 April 2013 17:16:37 Andrew Morton wrote:
>> On Wed,  3 Apr 2013 20:39:43 -0600 Stephen Warren wrote:
>>> A simple frame-buffer describes a raw memory region that may be rendered
>>> to, with the assumption that the display hardware has already been set
>>> up to scan out from that buffer.
>>>
>>> This is useful in cases where a bootloader exists and has set up the
>>> display hardware, but a Linux driver doesn't yet exist for the display
>>> hardware.
>>>
>>> ...
>>>
>>> +config FB_SIMPLE
>>> +	bool "Simple framebuffer support"
>>> +	depends on (FB = y) && OF
>>
>> It's sad that this simple little thing requires Open Firmware.  Could
>> it be generalised in some way so that the small amount of setup info
>> could be provided by other means (eg, module_param) or does the
>> dependency go deeper than that?
> 
> I second that request. I like the idea of a simple framebuffer driver if it 
> helps deprecating fbdev in the long term, but I don't want it to offer an 
> excuse not to implement a DRM/KMS driver. In particular adding DT bindings 
> would force us to keep supporting the ABI for a (too) long time.

The platforms I intend to use this with only support device tree. Adding
support for platform data right now would just be dead code. If somebody
wants to use this driver with a board file based system rather than a DT
based system, it would be trivial do modify the driver to add a platform
data structure at that time.

Adding support for a platform data structure won't remove the need for
DT support in the driver; any platform that uses DT will always
configure this driver through the DT binding irrespective of whether
some other platform could configure it using platform_data.

I don't believe the DT bindings imply that they must be implemented by
an FB driver rather than a KMS driver. It's just that it's much simpler
to do so at present. If the whole FB subsystem goes away at some time,
it should be possible to implement a simplest-possible KMS driver that
supports the same DT binding. I didn't do it this way because supporting
a pre-allocated FB in DRM/KMS apparently means implementing a custom
memory allocator for this in the driver, which would be a lot of code
overhead when right now the driver can just use the FB subsystem and
simply return the address directly. The simplest possible FB driver
appears much simpler (less code size, less maintenance) than the
simplest possible KMS driver.

My inclination is that for many platforms, the bootloader support for
graphics output will appear first (before the kernel's), and this driver
will allow for the kernel to have a graphical console, allowing a more
complete/useful system to be available earlier. In many cases, that
window may be small; a DRM/KMS driver may appear soon after the basic
CPU/board/... support, and then people can switch to using it if they want.

That said, I also don't really see a problem not implementing a DRM/KMS
driver for a platform; a dumb frame-buffer works perfectly well for my
needs. Nobody would be forced to continue using it once a better
alternative existed.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox