i.MX Framebuffer patches

i.MX Framebuffer patches

[Sascha Hauer]

Here are some patches for the i.MX framebuffer driver. They mainly prepare
the driver for use with other i.MX processors like the i.MX27.

Sascha



-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

[PATCH] i.MX Framebuffer: remove gpio setup function

[Sascha Hauer]

Remove the gpio mux setup function from i.MX framebuffer driver.
This function is platform specific and thus should be done by
the board setup. As there are currently no in-kernel users
of this driver we do not break anything.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/video/imxfb.c |   53 -------------------------------------------------
 1 files changed, 0 insertions(+), 53 deletions(-)

diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index ccd9861..a990d43 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -357,57 +357,6 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
 	return 0;
 }
 
-static void imxfb_setup_gpio(struct imxfb_info *fbi)
-{
-	int width;
-
-	LCDC_RMCR	&= ~(RMCR_LCDC_EN | RMCR_SELF_REF);
-
-	if( fbi->pcr & PCR_TFT )
-		width = 16;
-	else
-		width = 1 << ((fbi->pcr >> 28) & 0x3);
-
-	switch(width) {
-	case 16:
-		imx_gpio_mode(PD30_PF_LD15);
-		imx_gpio_mode(PD29_PF_LD14);
-		imx_gpio_mode(PD28_PF_LD13);
-		imx_gpio_mode(PD27_PF_LD12);
-		imx_gpio_mode(PD26_PF_LD11);
-		imx_gpio_mode(PD25_PF_LD10);
-		imx_gpio_mode(PD24_PF_LD9);
-		imx_gpio_mode(PD23_PF_LD8);
-	case 8:
-		imx_gpio_mode(PD22_PF_LD7);
-		imx_gpio_mode(PD21_PF_LD6);
-		imx_gpio_mode(PD20_PF_LD5);
-		imx_gpio_mode(PD19_PF_LD4);
-	case 4:
-		imx_gpio_mode(PD18_PF_LD3);
-		imx_gpio_mode(PD17_PF_LD2);
-	case 2:
-		imx_gpio_mode(PD16_PF_LD1);
-	case 1:
-		imx_gpio_mode(PD15_PF_LD0);
-	}
-
-	/* initialize GPIOs */
-	imx_gpio_mode(PD6_PF_LSCLK);
-	imx_gpio_mode(PD11_PF_CONTRAST);
-	imx_gpio_mode(PD14_PF_FLM_VSYNC);
-	imx_gpio_mode(PD13_PF_LP_HSYNC);
-	imx_gpio_mode(PD12_PF_ACD_OE);
-
-	/* These are only needed for Sharp HR TFT displays */
-	if (fbi->pcr & PCR_SHARP) {
-		imx_gpio_mode(PD7_PF_REV);
-		imx_gpio_mode(PD8_PF_CLS);
-		imx_gpio_mode(PD9_PF_PS);
-		imx_gpio_mode(PD10_PF_SPL_SPR);
-	}
-}
-
 #ifdef CONFIG_PM
 /*
  * Power management hooks.  Note that we won't be called from IRQ context,
@@ -594,8 +543,6 @@ static int __init imxfb_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto failed_cmap;
 
-	imxfb_setup_gpio(fbi);
-
 	imxfb_set_par(info);
 	ret = register_framebuffer(info);
 	if (ret < 0) {
-- 
1.5.6.3


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [PATCH] i.MX Framebuffer: remove gpio setup function

[Krzysztof Helt]

On Tue, 19 Aug 2008 17:06:40 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> Remove the gpio mux setup function from i.MX framebuffer driver.
> This function is platform specific and thus should be done by
> the board setup. As there are currently no in-kernel users
> of this driver we do not break anything.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---

Acked-by: Krzysztof Helt <krzysztof.h1@wp.pl>

This is patch type I like the most: only deletions.

Please number your patches with [PATCH 1/4], [PATCH 2/4], etc.
in next repost. Please also add line From: and description
to each patch.

>  drivers/video/imxfb.c |   53 -------------------------------------------------
>  1 files changed, 0 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> index ccd9861..a990d43 100644
> --- a/drivers/video/imxfb.c
> +++ b/drivers/video/imxfb.c
> @@ -357,57 +357,6 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
>  	return 0;
>  }
>  
> -static void imxfb_setup_gpio(struct imxfb_info *fbi)
> -{
> -	int width;
> -
> -	LCDC_RMCR	&= ~(RMCR_LCDC_EN | RMCR_SELF_REF);
> -
> -	if( fbi->pcr & PCR_TFT )
> -		width = 16;
> -	else
> -		width = 1 << ((fbi->pcr >> 28) & 0x3);
> -
> -	switch(width) {
> -	case 16:
> -		imx_gpio_mode(PD30_PF_LD15);
> -		imx_gpio_mode(PD29_PF_LD14);
> -		imx_gpio_mode(PD28_PF_LD13);
> -		imx_gpio_mode(PD27_PF_LD12);
> -		imx_gpio_mode(PD26_PF_LD11);
> -		imx_gpio_mode(PD25_PF_LD10);
> -		imx_gpio_mode(PD24_PF_LD9);
> -		imx_gpio_mode(PD23_PF_LD8);
> -	case 8:
> -		imx_gpio_mode(PD22_PF_LD7);
> -		imx_gpio_mode(PD21_PF_LD6);
> -		imx_gpio_mode(PD20_PF_LD5);
> -		imx_gpio_mode(PD19_PF_LD4);
> -	case 4:
> -		imx_gpio_mode(PD18_PF_LD3);
> -		imx_gpio_mode(PD17_PF_LD2);
> -	case 2:
> -		imx_gpio_mode(PD16_PF_LD1);
> -	case 1:
> -		imx_gpio_mode(PD15_PF_LD0);
> -	}
> -
> -	/* initialize GPIOs */
> -	imx_gpio_mode(PD6_PF_LSCLK);
> -	imx_gpio_mode(PD11_PF_CONTRAST);
> -	imx_gpio_mode(PD14_PF_FLM_VSYNC);
> -	imx_gpio_mode(PD13_PF_LP_HSYNC);
> -	imx_gpio_mode(PD12_PF_ACD_OE);
> -
> -	/* These are only needed for Sharp HR TFT displays */
> -	if (fbi->pcr & PCR_SHARP) {
> -		imx_gpio_mode(PD7_PF_REV);
> -		imx_gpio_mode(PD8_PF_CLS);
> -		imx_gpio_mode(PD9_PF_PS);
> -		imx_gpio_mode(PD10_PF_SPL_SPR);
> -	}
> -}
> -
>  #ifdef CONFIG_PM
>  /*
>   * Power management hooks.  Note that we won't be called from IRQ context,
> @@ -594,8 +543,6 @@ static int __init imxfb_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		goto failed_cmap;
>  
> -	imxfb_setup_gpio(fbi);
> -
>  	imxfb_set_par(info);
>  	ret = register_framebuffer(info);
>  	if (ret < 0) {
> -- 
> 1.5.6.3
> 
> 
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
> Build the coolest Linux based applications with Moblin SDK & win great prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Linux-fbdev-devel mailing list
> Linux-fbdev-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel
> 

----------------------------------------------------------------------
Taniej na zagraniczne komorki!
Sprawdz >>>  http://link.interia.pl/f1ee3 


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

[PATCH] i.MX Framebuffer: Use iowrite/ioread instead of direct pointer deref

[Sascha Hauer]

From: Juergen Beisert <j.beisert@pengutronix.de>

This patch prepares the current i.MX1 framebuffer driver for usage in the
whole i.MX family. It switches to iowrite/ioread for register accesses.
Also it moves the register definitions to the driver where they belong.

Signed-off-by: Juergen Beisert <j.beisert@pengutronix.de>
---
 drivers/video/imxfb.c |  208 ++++++++++++++++++++++++++++++++++++++++---------
 drivers/video/imxfb.h |    4 +-
 2 files changed, 173 insertions(+), 39 deletions(-)

diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index a990d43..0622589 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -16,7 +16,6 @@
  *	linux-arm-kernel@lists.arm.linux.org.uk
  */
 
-//#define DEBUG 1
 
 #include <linux/module.h>
 #include <linux/kernel.h>
@@ -33,7 +32,6 @@
 #include <linux/platform_device.h>
 #include <linux/dma-mapping.h>
 
-#include <mach/hardware.h>
 #include <asm/io.h>
 #include <mach/imxfb.h>
 
@@ -44,6 +42,124 @@
 
 #include "imxfb.h"
 
+#define DRIVER_NAME "imx-fb"
+
+#define LCDC_SSA	0x00
+
+#define LCDC_SIZE	0x04
+#define SIZE_XMAX(x)	((((x) >> 4) & 0x3f) << 20)
+#ifdef CONFIG_ARCH_IMX
+#define SIZE_YMAX(y)	((y) & 0x1ff)
+#else
+#define SIZE_YMAX(y)	((y) & 0x3ff)
+#endif
+
+#define LCDC_VPW	0x08
+#define VPW_VPW(x)	((x) & 0x3ff)
+
+#define LCDC_CPOS	0x0C
+#define CPOS_CC1	(1<<31)
+#define CPOS_CC0	(1<<30)
+#define CPOS_OP		(1<<28)
+#define CPOS_CXP(x)	(((x) & 3ff) << 16)
+#ifdef CONFIG_ARCH_IMX
+#define CPOS_CYP(y)	((y) & 0x1ff)
+#else
+#define CPOS_CYP(y)	((y) & 0x3ff)
+#endif
+
+#define LCDC_LCWHB	0x10
+#define LCWHB_BK_EN	(1<<31)
+#define LCWHB_CW(w)	(((w) & 0x1f) << 24)
+#define LCWHB_CH(h)	(((h) & 0x1f) << 16)
+#define LCWHB_BD(x)	((x) & 0xff)
+
+#define LCDC_LCHCC	0x14
+#ifdef CONFIG_ARCH_IMX
+#define LCHCC_CUR_COL_R(r) (((r) & 0x1f) << 11)
+#define LCHCC_CUR_COL_G(g) (((g) & 0x3f) << 5)
+#define LCHCC_CUR_COL_B(b) ((b) & 0x1f)
+#else
+#define LCHCC_CUR_COL_R(r) (((r) & 0x3f) << 12)
+#define LCHCC_CUR_COL_G(g) (((g) & 0x3f) << 6)
+#define LCHCC_CUR_COL_B(b) ((b) & 0x3f)
+#endif
+
+#define LCDC_PCR	0x18
+#define PCR_TFT		(1<<31)
+#define PCR_COLOR	(1<<30)
+#define PCR_PBSIZ_1	(0<<28)
+#define PCR_PBSIZ_2	(1<<28)
+#define PCR_PBSIZ_4	(2<<28)
+#define PCR_PBSIZ_8	(3<<28)
+#define PCR_BPIX_1	(0<<25)
+#define PCR_BPIX_2	(1<<25)
+#define PCR_BPIX_4	(2<<25)
+#define PCR_BPIX_8	(3<<25)
+#define PCR_BPIX_12	(4<<25)
+#define PCR_BPIX_16	(4<<25)
+#define PCR_PIXPOL	(1<<24)
+#define PCR_FLMPOL	(1<<23)
+#define PCR_LPPOL	(1<<22)
+#define PCR_CLKPOL	(1<<21)
+#define PCR_OEPOL	(1<<20)
+#define PCR_SCLKIDLE	(1<<19)
+#define PCR_END_SEL	(1<<18)
+#define PCR_END_BYTE_SWAP (1<<17)
+#define PCR_REV_VS	(1<<16)
+#define PCR_ACD_SEL	(1<<15)
+#define PCR_ACD(x)	(((x) & 0x7f) << 8)
+#define PCR_SCLK_SEL	(1<<7)
+#define PCR_SHARP	(1<<6)
+#define PCR_PCD(x)	((x) & 0x3f)
+
+#define LCDC_HCR	0x1C
+#define HCR_H_WIDTH(x)	(((x) & 0x3f) << 26)
+#define HCR_H_WAIT_1(x)	(((x) & 0xff) << 8)
+#define HCR_H_WAIT_2(x)	((x) & 0xff)
+
+#define LCDC_VCR	0x20
+#define VCR_V_WIDTH(x)	(((x) & 0x3f) << 26)
+#define VCR_V_WAIT_1(x)	(((x) & 0xff) << 8)
+#define VCR_V_WAIT_2(x)	((x) & 0xff)
+
+#define LCDC_POS	0x24
+#define POS_POS(x)	((x) & 1f)
+
+#define LCDC_LSCR1	0x28
+#define LSCR1_PS_RISE_DELAY(x)    (((x) & 0x7f) << 26)
+#define LSCR1_CLS_RISE_DELAY(x)   (((x) & 0x3f) << 16)
+#define LSCR1_REV_TOGGLE_DELAY(x) (((x) & 0xf) << 8)
+#define LSCR1_GRAY2(x)            (((x) & 0xf) << 4)
+#define LSCR1_GRAY1(x)            (((x) & 0xf))
+
+#define LCDC_PWMR	0x2C
+#define PWMR_CLS(x)	(((x) & 0x1ff) << 16)
+#define PWMR_LDMSK	(1<<15)
+#define PWMR_SCR1	(1<<10)
+#define PWMR_SCR0	(1<<9)
+#define PWMR_CC_EN	(1<<8)
+#define PWMR_PW(x)	((x) & 0xff)
+
+#define LCDC_DMACR	0x30
+#define DMACR_BURST	(1<<31)
+#define DMACR_HM(x)	(((x) & 0xf) << 16)
+#define DMACR_TM(x)	((x) &0xf)
+
+#define LCDC_RMCR	0x34
+#define RMCR_LCDC_EN	(1<<1)
+#define RMCR_SELF_REF	(1<<0)
+
+#define LCDC_LCDICR	0x38
+#define LCDICR_INT_SYN	(1<<2)
+#define LCDICR_INT_CON	(1)
+
+#define LCDC_LCDISR	0x40
+#define LCDISR_UDR_ERR	(1<<3)
+#define LCDISR_ERR_RES	(1<<2)
+#define LCDISR_EOF	(1<<1)
+#define LCDISR_BOF	(1<<0)
+
 static struct imxfb_rgb def_rgb_16 = {
 	.red	= { .offset = 8,  .length = 4, },
 	.green	= { .offset = 4,  .length = 4, },
@@ -67,7 +183,6 @@ static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)
 	return chan << bf->offset;
 }
 
-#define LCDC_PALETTE(x) __REG2(IMX_LCDC_BASE+0x800, (x)<<2)
 static int
 imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
 		       u_int trans, struct fb_info *info)
@@ -81,7 +196,7 @@ imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
 		      (CNVT_TOHW(green,4) << 4) |
 		      CNVT_TOHW(blue,  4);
 
-		LCDC_PALETTE(regno) = val;
+		iowrite32(val, fbi->regs + 0x800 + (regno << 2));
 		ret = 0;
 	}
 	return ret;
@@ -235,18 +350,23 @@ static void imxfb_enable_controller(struct imxfb_info *fbi)
 	pr_debug("Enabling LCD controller\n");
 
 	/* initialize LCDC */
-	LCDC_RMCR &= ~RMCR_LCDC_EN;		/* just to be safe... */
+	iowrite32(ioread32(fbi->regs + LCDC_RMCR) & ~RMCR_LCDC_EN,
+		fbi->regs + LCDC_RMCR);	/* just to be safe... */
+
+	iowrite32(fbi->screen_dma, fbi->regs + LCDC_SSA);
 
-	LCDC_SSA	= fbi->screen_dma;
 	/* physical screen start address	    */
-	LCDC_VPW	= VPW_VPW(fbi->max_xres * fbi->max_bpp / 8 / 4);
+	iowrite32(VPW_VPW(fbi->max_xres * fbi->max_bpp / 8 / 4),
+		fbi->regs + LCDC_VPW);
 
-	LCDC_POS	= 0x00000000;   /* panning offset 0 (0 pixel offset)        */
+	/* panning offset 0 (0 pixel offset)        */
+	iowrite32(0x00000000, fbi->regs + LCDC_POS);
 
 	/* disable hardware cursor */
-	LCDC_CPOS	&= ~(CPOS_CC0 | CPOS_CC1);
+	iowrite32(ioread32(fbi->regs + LCDC_CPOS) & ~(CPOS_CC0 | CPOS_CC1),
+		fbi->regs + LCDC_CPOS);
 
-	LCDC_RMCR = RMCR_LCDC_EN;
+	iowrite32(RMCR_LCDC_EN, fbi->regs + LCDC_RMCR);
 
 	if(fbi->backlight_power)
 		fbi->backlight_power(1);
@@ -263,7 +383,7 @@ static void imxfb_disable_controller(struct imxfb_info *fbi)
 	if(fbi->lcd_power)
 		fbi->lcd_power(0);
 
-	LCDC_RMCR = 0;
+	iowrite32(0, fbi->regs + LCDC_RMCR);
 }
 
 static int imxfb_blank(int blank, struct fb_info *info)
@@ -340,19 +460,22 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
 			info->fix.id, var->lower_margin);
 #endif
 
-	LCDC_HCR	= HCR_H_WIDTH(var->hsync_len) |
-	                  HCR_H_WAIT_1(var->left_margin) |
-			  HCR_H_WAIT_2(var->right_margin);
+	iowrite32(HCR_H_WIDTH(var->hsync_len) |
+		HCR_H_WAIT_1(var->left_margin) |
+		HCR_H_WAIT_2(var->right_margin),
+		fbi->regs + LCDC_HCR);
 
-	LCDC_VCR	= VCR_V_WIDTH(var->vsync_len) |
-	                  VCR_V_WAIT_1(var->upper_margin) |
-			  VCR_V_WAIT_2(var->lower_margin);
+	iowrite32(VCR_V_WIDTH(var->vsync_len) |
+		VCR_V_WAIT_1(var->upper_margin) |
+		VCR_V_WAIT_2(var->lower_margin),
+		fbi->regs + LCDC_VCR);
 
-	LCDC_SIZE	= SIZE_XMAX(var->xres) | SIZE_YMAX(var->yres);
-	LCDC_PCR	= fbi->pcr;
-	LCDC_PWMR	= fbi->pwmr;
-	LCDC_LSCR1	= fbi->lscr1;
-	LCDC_DMACR	= fbi->dmacr;
+	iowrite32(SIZE_XMAX(var->xres) | SIZE_YMAX(var->yres),
+			fbi->regs + LCDC_SIZE);
+	iowrite32(fbi->pcr, fbi->regs + LCDC_PCR);
+	iowrite32(fbi->pwmr, fbi->regs + LCDC_PWMR);
+	iowrite32(fbi->lscr1, fbi->regs + LCDC_LSCR1);
+	iowrite32(fbi->dmacr, fbi->regs + LCDC_DMACR);
 
 	return 0;
 }
@@ -384,10 +507,10 @@ static int imxfb_resume(struct platform_device *dev)
 #define imxfb_resume	NULL
 #endif
 
-static int __init imxfb_init_fbinfo(struct device *dev)
+static int __init imxfb_init_fbinfo(struct platform_device *pdev)
 {
-	struct imxfb_mach_info *inf = dev->platform_data;
-	struct fb_info *info = dev_get_drvdata(dev);
+	struct imxfb_mach_info *inf = pdev->dev.platform_data;
+	struct fb_info *info = dev_get_drvdata(&pdev->dev);
 	struct imxfb_info *fbi = info->par;
 
 	pr_debug("%s\n",__func__);
@@ -397,7 +520,7 @@ static int __init imxfb_init_fbinfo(struct device *dev)
 		return -ENOMEM;
 
 	memset(fbi, 0, sizeof(struct imxfb_info));
-	fbi->dev = dev;
+	fbi->pdev = pdev;
 
 	strlcpy(info->fix.id, IMX_NAME, sizeof(info->fix.id));
 
@@ -465,7 +588,7 @@ static int __init imxfb_map_video_memory(struct fb_info *info)
 	struct imxfb_info *fbi = info->par;
 
 	fbi->map_size = PAGE_ALIGN(info->fix.smem_len);
-	fbi->map_cpu = dma_alloc_writecombine(fbi->dev, fbi->map_size,
+	fbi->map_cpu = dma_alloc_writecombine(&fbi->pdev->dev, fbi->map_size,
 					&fbi->map_dma,GFP_KERNEL);
 
 	if (fbi->map_cpu) {
@@ -506,14 +629,21 @@ static int __init imxfb_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, info);
 
-	ret = imxfb_init_fbinfo(&pdev->dev);
+	ret = imxfb_init_fbinfo(pdev);
 	if( ret < 0 )
 		goto failed_init;
 
-	res = request_mem_region(res->start, res->end - res->start + 1, "IMXFB");
+	res = request_mem_region(res->start, res->end - res->start + 1,
+				DRIVER_NAME);
 	if (!res) {
 		ret = -EBUSY;
-		goto failed_regs;
+		goto failed_req;
+	}
+
+	fbi->regs = ioremap(res->start, res->end - res->start + 1);
+	if (fbi->regs == NULL) {
+		printk(KERN_ERR"Cannot map frame buffer registers\n");
+		goto failed_ioremap;
 	}
 
 	if (!inf->fixed_screen_cpu) {
@@ -559,18 +689,20 @@ failed_register:
 failed_cmap:
 	if (!inf->fixed_screen_cpu)
 		dma_free_writecombine(&pdev->dev,fbi->map_size,fbi->map_cpu,
-		           fbi->map_dma);
+			fbi->map_dma);
 failed_map:
-	kfree(info->pseudo_palette);
-failed_regs:
+	iounmap(fbi->regs);
+failed_ioremap:
 	release_mem_region(res->start, res->end - res->start);
+failed_req:
+	kfree(info->pseudo_palette);
 failed_init:
 	platform_set_drvdata(pdev, NULL);
 	framebuffer_release(info);
 	return ret;
 }
 
-static int imxfb_remove(struct platform_device *pdev)
+static int __devexit imxfb_remove(struct platform_device *pdev)
 {
 	struct fb_info *info = platform_get_drvdata(pdev);
 	struct imxfb_info *fbi = info->par;
@@ -586,6 +718,7 @@ static int imxfb_remove(struct platform_device *pdev)
 	kfree(info->pseudo_palette);
 	framebuffer_release(info);
 
+	iounmap(fbi->regs);
 	release_mem_region(res->start, res->end - res->start + 1);
 	platform_set_drvdata(pdev, NULL);
 
@@ -600,19 +733,18 @@ void  imxfb_shutdown(struct platform_device * dev)
 }
 
 static struct platform_driver imxfb_driver = {
-	.probe		= imxfb_probe,
 	.suspend	= imxfb_suspend,
 	.resume		= imxfb_resume,
-	.remove		= imxfb_remove,
+	.remove		= __devexit_p(imxfb_remove),
 	.shutdown	= imxfb_shutdown,
 	.driver		= {
-		.name	= "imx-fb",
+		.name	= DRIVER_NAME,
 	},
 };
 
 int __init imxfb_init(void)
 {
-	return platform_driver_register(&imxfb_driver);
+	return platform_driver_probe(&imxfb_driver, imxfb_probe);
 }
 
 static void __exit imxfb_cleanup(void)
diff --git a/drivers/video/imxfb.h b/drivers/video/imxfb.h
index e837a8b..766297e 100644
--- a/drivers/video/imxfb.h
+++ b/drivers/video/imxfb.h
@@ -29,7 +29,9 @@ struct imxfb_rgb {
 #define NR_RGB	2
 
 struct imxfb_info {
-	struct device		*dev;
+	struct platform_device  *pdev;
+	void __iomem		*regs;
+
 	struct imxfb_rgb	*rgb[NR_RGB];
 
 	u_int			max_bpp;
-- 
1.5.6.3


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [PATCH] i.MX Framebuffer: Use iowrite/ioread instead of direct pointer deref

[Krzysztof Helt]

On Tue, 19 Aug 2008 17:06:41 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> From: Juergen Beisert <j.beisert@pengutronix.de>
> 
> This patch prepares the current i.MX1 framebuffer driver for usage in the
> whole i.MX family. It switches to iowrite/ioread for register accesses.
> Also it moves the register definitions to the driver where they belong.
> 
> Signed-off-by: Juergen Beisert <j.beisert@pengutronix.de>
> ---
>  drivers/video/imxfb.c |  208 ++++++++++++++++++++++++++++++++++++++++---------
>  drivers/video/imxfb.h |    4 +-
>  2 files changed, 173 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> index a990d43..0622589 100644
> --- a/drivers/video/imxfb.c
> +++ b/drivers/video/imxfb.c
> @@ -16,7 +16,6 @@
>   *	linux-arm-kernel@lists.arm.linux.org.uk
>   */
>  
> -//#define DEBUG 1
>  
>  #include <linux/module.h>
>  #include <linux/kernel.h>
> @@ -33,7 +32,6 @@
>  #include <linux/platform_device.h>
>  #include <linux/dma-mapping.h>
>  
> -#include <mach/hardware.h>
>  #include <asm/io.h>
>  #include <mach/imxfb.h>
>  
> @@ -44,6 +42,124 @@
>  
>  #include "imxfb.h"
>  
> +#define DRIVER_NAME "imx-fb"
> +

All constants below should really be put into the imxfb.h. There is already such a file.

> +#define LCDC_SSA	0x00
> +
> +#define LCDC_SIZE	0x04
> +#define SIZE_XMAX(x)	((((x) >> 4) & 0x3f) << 20)
> +#ifdef CONFIG_ARCH_IMX

Is it possible to use this driver without CONFIG_ARCH_IMX defined?

> +#define SIZE_YMAX(y)	((y) & 0x1ff)
> +#else
> +#define SIZE_YMAX(y)	((y) & 0x3ff)
> +#endif
> +
> +#define LCDC_VPW	0x08
> +#define VPW_VPW(x)	((x) & 0x3ff)
> +
> +#define LCDC_CPOS	0x0C
> +#define CPOS_CC1	(1<<31)
> +#define CPOS_CC0	(1<<30)
> +#define CPOS_OP		(1<<28)
> +#define CPOS_CXP(x)	(((x) & 3ff) << 16)
> +#ifdef CONFIG_ARCH_IMX
> +#define CPOS_CYP(y)	((y) & 0x1ff)
> +#else
> +#define CPOS_CYP(y)	((y) & 0x3ff)
> +#endif
> +
> +#define LCDC_LCWHB	0x10
> +#define LCWHB_BK_EN	(1<<31)
> +#define LCWHB_CW(w)	(((w) & 0x1f) << 24)
> +#define LCWHB_CH(h)	(((h) & 0x1f) << 16)
> +#define LCWHB_BD(x)	((x) & 0xff)
> +
> +#define LCDC_LCHCC	0x14
> +#ifdef CONFIG_ARCH_IMX
> +#define LCHCC_CUR_COL_R(r) (((r) & 0x1f) << 11)
> +#define LCHCC_CUR_COL_G(g) (((g) & 0x3f) << 5)
> +#define LCHCC_CUR_COL_B(b) ((b) & 0x1f)
> +#else
> +#define LCHCC_CUR_COL_R(r) (((r) & 0x3f) << 12)
> +#define LCHCC_CUR_COL_G(g) (((g) & 0x3f) << 6)
> +#define LCHCC_CUR_COL_B(b) ((b) & 0x3f)
> +#endif
> +
> +#define LCDC_PCR	0x18
> +#define PCR_TFT		(1<<31)
> +#define PCR_COLOR	(1<<30)
> +#define PCR_PBSIZ_1	(0<<28)
> +#define PCR_PBSIZ_2	(1<<28)
> +#define PCR_PBSIZ_4	(2<<28)
> +#define PCR_PBSIZ_8	(3<<28)
> +#define PCR_BPIX_1	(0<<25)
> +#define PCR_BPIX_2	(1<<25)
> +#define PCR_BPIX_4	(2<<25)
> +#define PCR_BPIX_8	(3<<25)
> +#define PCR_BPIX_12	(4<<25)
> +#define PCR_BPIX_16	(4<<25)
> +#define PCR_PIXPOL	(1<<24)
> +#define PCR_FLMPOL	(1<<23)
> +#define PCR_LPPOL	(1<<22)
> +#define PCR_CLKPOL	(1<<21)
> +#define PCR_OEPOL	(1<<20)
> +#define PCR_SCLKIDLE	(1<<19)
> +#define PCR_END_SEL	(1<<18)
> +#define PCR_END_BYTE_SWAP (1<<17)
> +#define PCR_REV_VS	(1<<16)
> +#define PCR_ACD_SEL	(1<<15)
> +#define PCR_ACD(x)	(((x) & 0x7f) << 8)
> +#define PCR_SCLK_SEL	(1<<7)
> +#define PCR_SHARP	(1<<6)
> +#define PCR_PCD(x)	((x) & 0x3f)
> +
> +#define LCDC_HCR	0x1C
> +#define HCR_H_WIDTH(x)	(((x) & 0x3f) << 26)
> +#define HCR_H_WAIT_1(x)	(((x) & 0xff) << 8)
> +#define HCR_H_WAIT_2(x)	((x) & 0xff)
> +
> +#define LCDC_VCR	0x20
> +#define VCR_V_WIDTH(x)	(((x) & 0x3f) << 26)
> +#define VCR_V_WAIT_1(x)	(((x) & 0xff) << 8)
> +#define VCR_V_WAIT_2(x)	((x) & 0xff)
> +
> +#define LCDC_POS	0x24
> +#define POS_POS(x)	((x) & 1f)
> +
> +#define LCDC_LSCR1	0x28
> +#define LSCR1_PS_RISE_DELAY(x)    (((x) & 0x7f) << 26)
> +#define LSCR1_CLS_RISE_DELAY(x)   (((x) & 0x3f) << 16)
> +#define LSCR1_REV_TOGGLE_DELAY(x) (((x) & 0xf) << 8)
> +#define LSCR1_GRAY2(x)            (((x) & 0xf) << 4)
> +#define LSCR1_GRAY1(x)            (((x) & 0xf))
> +
> +#define LCDC_PWMR	0x2C
> +#define PWMR_CLS(x)	(((x) & 0x1ff) << 16)
> +#define PWMR_LDMSK	(1<<15)
> +#define PWMR_SCR1	(1<<10)
> +#define PWMR_SCR0	(1<<9)
> +#define PWMR_CC_EN	(1<<8)
> +#define PWMR_PW(x)	((x) & 0xff)
> +
> +#define LCDC_DMACR	0x30
> +#define DMACR_BURST	(1<<31)
> +#define DMACR_HM(x)	(((x) & 0xf) << 16)
> +#define DMACR_TM(x)	((x) &0xf)
> +
> +#define LCDC_RMCR	0x34
> +#define RMCR_LCDC_EN	(1<<1)
> +#define RMCR_SELF_REF	(1<<0)
> +
> +#define LCDC_LCDICR	0x38
> +#define LCDICR_INT_SYN	(1<<2)
> +#define LCDICR_INT_CON	(1)
> +
> +#define LCDC_LCDISR	0x40
> +#define LCDISR_UDR_ERR	(1<<3)
> +#define LCDISR_ERR_RES	(1<<2)
> +#define LCDISR_EOF	(1<<1)
> +#define LCDISR_BOF	(1<<0)
> +
>  static struct imxfb_rgb def_rgb_16 = {
>  	.red	= { .offset = 8,  .length = 4, },
>  	.green	= { .offset = 4,  .length = 4, },
> @@ -67,7 +183,6 @@ static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)
>  	return chan << bf->offset;
>  }
>  
> -#define LCDC_PALETTE(x) __REG2(IMX_LCDC_BASE+0x800, (x)<<2)
>  static int
>  imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
>  		       u_int trans, struct fb_info *info)
> @@ -81,7 +196,7 @@ imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
>  		      (CNVT_TOHW(green,4) << 4) |
>  		      CNVT_TOHW(blue,  4);
>  
> -		LCDC_PALETTE(regno) = val;
> +		iowrite32(val, fbi->regs + 0x800 + (regno << 2));

One may define an inline function imxfb_write(fbi, val, reg) :

 imxfb_iowrite(fbi, val, reg) 
{
	iowrite32(val, fbi->regs + reg);
}

It would make the code more readable. The imxfb_ioread() could be used as well.

>  		ret = 0;
>  	}
>  	return ret;
> @@ -235,18 +350,23 @@ static void imxfb_enable_controller(struct imxfb_info *fbi)
>  	pr_debug("Enabling LCD controller\n");
>  
>  	/* initialize LCDC */
> -	LCDC_RMCR &= ~RMCR_LCDC_EN;		/* just to be safe... */
> +	iowrite32(ioread32(fbi->regs + LCDC_RMCR) & ~RMCR_LCDC_EN,
> +		fbi->regs + LCDC_RMCR);	/* just to be safe... */
> +
> +	iowrite32(fbi->screen_dma, fbi->regs + LCDC_SSA);
>  
> -	LCDC_SSA	= fbi->screen_dma;
>  	/* physical screen start address	    */
> -	LCDC_VPW	= VPW_VPW(fbi->max_xres * fbi->max_bpp / 8 / 4);
> +	iowrite32(VPW_VPW(fbi->max_xres * fbi->max_bpp / 8 / 4),
> +		fbi->regs + LCDC_VPW);
>  
> -	LCDC_POS	= 0x00000000;   /* panning offset 0 (0 pixel offset)        */
> +	/* panning offset 0 (0 pixel offset)        */
> +	iowrite32(0x00000000, fbi->regs + LCDC_POS);
>  
>  	/* disable hardware cursor */
> -	LCDC_CPOS	&= ~(CPOS_CC0 | CPOS_CC1);
> +	iowrite32(ioread32(fbi->regs + LCDC_CPOS) & ~(CPOS_CC0 | CPOS_CC1),
> +		fbi->regs + LCDC_CPOS);
>  
> -	LCDC_RMCR = RMCR_LCDC_EN;
> +	iowrite32(RMCR_LCDC_EN, fbi->regs + LCDC_RMCR);
>  
>  	if(fbi->backlight_power)
>  		fbi->backlight_power(1);
> @@ -263,7 +383,7 @@ static void imxfb_disable_controller(struct imxfb_info *fbi)
>  	if(fbi->lcd_power)
>  		fbi->lcd_power(0);
>  
> -	LCDC_RMCR = 0;
> +	iowrite32(0, fbi->regs + LCDC_RMCR);
>  }
>  
>  static int imxfb_blank(int blank, struct fb_info *info)
> @@ -340,19 +460,22 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
>  			info->fix.id, var->lower_margin);
>  #endif
>  
> -	LCDC_HCR	= HCR_H_WIDTH(var->hsync_len) |
> -	                  HCR_H_WAIT_1(var->left_margin) |
> -			  HCR_H_WAIT_2(var->right_margin);
> +	iowrite32(HCR_H_WIDTH(var->hsync_len) |
> +		HCR_H_WAIT_1(var->left_margin) |
> +		HCR_H_WAIT_2(var->right_margin),
> +		fbi->regs + LCDC_HCR);
>  
> -	LCDC_VCR	= VCR_V_WIDTH(var->vsync_len) |
> -	                  VCR_V_WAIT_1(var->upper_margin) |
> -			  VCR_V_WAIT_2(var->lower_margin);
> +	iowrite32(VCR_V_WIDTH(var->vsync_len) |
> +		VCR_V_WAIT_1(var->upper_margin) |
> +		VCR_V_WAIT_2(var->lower_margin),
> +		fbi->regs + LCDC_VCR);
>  
> -	LCDC_SIZE	= SIZE_XMAX(var->xres) | SIZE_YMAX(var->yres);
> -	LCDC_PCR	= fbi->pcr;
> -	LCDC_PWMR	= fbi->pwmr;
> -	LCDC_LSCR1	= fbi->lscr1;
> -	LCDC_DMACR	= fbi->dmacr;
> +	iowrite32(SIZE_XMAX(var->xres) | SIZE_YMAX(var->yres),
> +			fbi->regs + LCDC_SIZE);
> +	iowrite32(fbi->pcr, fbi->regs + LCDC_PCR);
> +	iowrite32(fbi->pwmr, fbi->regs + LCDC_PWMR);
> +	iowrite32(fbi->lscr1, fbi->regs + LCDC_LSCR1);
> +	iowrite32(fbi->dmacr, fbi->regs + LCDC_DMACR);
>  
>  	return 0;
>  }
> @@ -384,10 +507,10 @@ static int imxfb_resume(struct platform_device *dev)
>  #define imxfb_resume	NULL
>  #endif
>  
> -static int __init imxfb_init_fbinfo(struct device *dev)
> +static int __init imxfb_init_fbinfo(struct platform_device *pdev)
>  {
> -	struct imxfb_mach_info *inf = dev->platform_data;
> -	struct fb_info *info = dev_get_drvdata(dev);
> +	struct imxfb_mach_info *inf = pdev->dev.platform_data;
> +	struct fb_info *info = dev_get_drvdata(&pdev->dev);
>  	struct imxfb_info *fbi = info->par;
>  
>  	pr_debug("%s\n",__func__);
> @@ -397,7 +520,7 @@ static int __init imxfb_init_fbinfo(struct device *dev)
>  		return -ENOMEM;
>  
>  	memset(fbi, 0, sizeof(struct imxfb_info));
> -	fbi->dev = dev;
> +	fbi->pdev = pdev;
>  
>  	strlcpy(info->fix.id, IMX_NAME, sizeof(info->fix.id));
>  
> @@ -465,7 +588,7 @@ static int __init imxfb_map_video_memory(struct fb_info *info)
>  	struct imxfb_info *fbi = info->par;
>  
>  	fbi->map_size = PAGE_ALIGN(info->fix.smem_len);
> -	fbi->map_cpu = dma_alloc_writecombine(fbi->dev, fbi->map_size,
> +	fbi->map_cpu = dma_alloc_writecombine(&fbi->pdev->dev, fbi->map_size,

Use &pdev->dev instead &fbi->pdev->dev

>  					&fbi->map_dma,GFP_KERNEL);
>  
>  	if (fbi->map_cpu) {
> @@ -506,14 +629,21 @@ static int __init imxfb_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, info);
>  
> -	ret = imxfb_init_fbinfo(&pdev->dev);
> +	ret = imxfb_init_fbinfo(pdev);
>  	if( ret < 0 )
>  		goto failed_init;
>  
> -	res = request_mem_region(res->start, res->end - res->start + 1, "IMXFB");
> +	res = request_mem_region(res->start, res->end - res->start + 1,
> +				DRIVER_NAME);

There is a function resource_size(res) which does (res->end - res->start + 1).
It seems useful here and at few places below.

>  	if (!res) {
>  		ret = -EBUSY;
> -		goto failed_regs;
> +		goto failed_req;
> +	}
> +
> +	fbi->regs = ioremap(res->start, res->end - res->start + 1);
> +	if (fbi->regs == NULL) {
> +		printk(KERN_ERR"Cannot map frame buffer registers\n");
> +		goto failed_ioremap;
>  	}
>  
>  	if (!inf->fixed_screen_cpu) {
> @@ -559,18 +689,20 @@ failed_register:
>  failed_cmap:
>  	if (!inf->fixed_screen_cpu)
>  		dma_free_writecombine(&pdev->dev,fbi->map_size,fbi->map_cpu,
> -		           fbi->map_dma);
> +			fbi->map_dma);
>  failed_map:
> -	kfree(info->pseudo_palette);
> -failed_regs:
> +	iounmap(fbi->regs);
> +failed_ioremap:
>  	release_mem_region(res->start, res->end - res->start);
> +failed_req:
> +	kfree(info->pseudo_palette);
>  failed_init:
>  	platform_set_drvdata(pdev, NULL);
>  	framebuffer_release(info);
>  	return ret;
>  }
>  
> -static int imxfb_remove(struct platform_device *pdev)
> +static int __devexit imxfb_remove(struct platform_device *pdev)
>  {
>  	struct fb_info *info = platform_get_drvdata(pdev);
>  	struct imxfb_info *fbi = info->par;
> @@ -586,6 +718,7 @@ static int imxfb_remove(struct platform_device *pdev)
>  	kfree(info->pseudo_palette);
>  	framebuffer_release(info);
>  
> +	iounmap(fbi->regs);

The fbi pointed does not exist here (after framebuffer_release(info)).

>  	release_mem_region(res->start, res->end - res->start + 1);
>  	platform_set_drvdata(pdev, NULL);
>  
> @@ -600,19 +733,18 @@ void  imxfb_shutdown(struct platform_device * dev)
>  }
>  
>  static struct platform_driver imxfb_driver = {
> -	.probe		= imxfb_probe,
>  	.suspend	= imxfb_suspend,
>  	.resume		= imxfb_resume,
> -	.remove		= imxfb_remove,
> +	.remove		= __devexit_p(imxfb_remove),
>  	.shutdown	= imxfb_shutdown,
>  	.driver		= {
> -		.name	= "imx-fb",
> +		.name	= DRIVER_NAME,
>  	},
>  };
>  
>  int __init imxfb_init(void)
>  {
> -	return platform_driver_register(&imxfb_driver);
> +	return platform_driver_probe(&imxfb_driver, imxfb_probe);
>  }
>  
>  static void __exit imxfb_cleanup(void)
> diff --git a/drivers/video/imxfb.h b/drivers/video/imxfb.h
> index e837a8b..766297e 100644
> --- a/drivers/video/imxfb.h
> +++ b/drivers/video/imxfb.h
> @@ -29,7 +29,9 @@ struct imxfb_rgb {
>  #define NR_RGB	2
>  
>  struct imxfb_info {
> -	struct device		*dev;
> +	struct platform_device  *pdev;

Is this field really useful? It can be dropped.

> +	void __iomem		*regs;
> +
>  	struct imxfb_rgb	*rgb[NR_RGB];
>  
>  	u_int			max_bpp;
> -- 
> 1.5.6.3
> 
> 
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
> Build the coolest Linux based applications with Moblin SDK & win great prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Linux-fbdev-devel mailing list
> Linux-fbdev-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel
> 

---------------------------------------------------------------------- 
Igrzyska na Dzikim Zachodzie!
Sprawdz >> http://link.interia.pl/f1edc 
	


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [PATCH] i.MX Framebuffer: Use iowrite/ioread instead of direct pointer deref

[Sascha Hauer]

On Wed, Aug 20, 2008 at 05:31:59PM +0200, Krzysztof Helt wrote:
> On Tue, 19 Aug 2008 17:06:41 +0200
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > From: Juergen Beisert <j.beisert@pengutronix.de>
> > 
> > This patch prepares the current i.MX1 framebuffer driver for usage in the
> > whole i.MX family. It switches to iowrite/ioread for register accesses.
> > Also it moves the register definitions to the driver where they belong.
> > 
> > Signed-off-by: Juergen Beisert <j.beisert@pengutronix.de>
> > ---
> >  drivers/video/imxfb.c |  208 ++++++++++++++++++++++++++++++++++++++++---------
> >  drivers/video/imxfb.h |    4 +-
> >  2 files changed, 173 insertions(+), 39 deletions(-)
> > 
> > diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> > index a990d43..0622589 100644
> > --- a/drivers/video/imxfb.c
> > +++ b/drivers/video/imxfb.c
> > @@ -16,7 +16,6 @@
> >   *	linux-arm-kernel@lists.arm.linux.org.uk
> >   */
> >  
> > -//#define DEBUG 1
> >  
> >  #include <linux/module.h>
> >  #include <linux/kernel.h>
> > @@ -33,7 +32,6 @@
> >  #include <linux/platform_device.h>
> >  #include <linux/dma-mapping.h>
> >  
> > -#include <mach/hardware.h>
> >  #include <asm/io.h>
> >  #include <mach/imxfb.h>
> >  
> > @@ -44,6 +42,124 @@
> >  
> >  #include "imxfb.h"
> >  
> > +#define DRIVER_NAME "imx-fb"
> > +
> 
> All constants below should really be put into the imxfb.h. There is already such a file.

AFAIK the-new-beautiful-way is not to have these header files next to
the .c files at all and put this stuff into the .c file. So maybe it's
better to move the rest of imxfb.h here aswell.


> 
> > +#define LCDC_SSA	0x00
> > +
> > +#define LCDC_SIZE	0x04
> > +#define SIZE_XMAX(x)	((((x) >> 4) & 0x3f) << 20)
> > +#ifdef CONFIG_ARCH_IMX
> 
> Is it possible to use this driver without CONFIG_ARCH_IMX defined?

Oops, not yet. These are the differences between IMX and MX2, the
architecture I'm currently preparing this driver for. This shouldn't
show up in this patch of course.


 >  
> > -#define LCDC_PALETTE(x) __REG2(IMX_LCDC_BASE+0x800, (x)<<2)
> >  static int
> >  imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
> >  		       u_int trans, struct fb_info *info)
> > @@ -81,7 +196,7 @@ imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
> >  		      (CNVT_TOHW(green,4) << 4) |
> >  		      CNVT_TOHW(blue,  4);
> >  
> > -		LCDC_PALETTE(regno) = val;
> > +		iowrite32(val, fbi->regs + 0x800 + (regno << 2));
> 
> One may define an inline function imxfb_write(fbi, val, reg) :
> 
>  imxfb_iowrite(fbi, val, reg) 
> {
> 	iowrite32(val, fbi->regs + reg);
> }
> 
> It would make the code more readable. The imxfb_ioread() could be used as well.
>

Well, I think this is a matter of taste, I prefer not using wrappers
around the access functions.



>  	strlcpy(info->fix.id, IMX_NAME, sizeof(info->fix.id));
> >  
> > @@ -465,7 +588,7 @@ static int __init imxfb_map_video_memory(struct fb_info *info)
> >  	struct imxfb_info *fbi = info->par;
> >  
> >  	fbi->map_size = PAGE_ALIGN(info->fix.smem_len);
> > -	fbi->map_cpu = dma_alloc_writecombine(fbi->dev, fbi->map_size,
> > +	fbi->map_cpu = dma_alloc_writecombine(&fbi->pdev->dev, fbi->map_size,
> 
> Use &pdev->dev instead &fbi->pdev->dev

ok

> 
> >  					&fbi->map_dma,GFP_KERNEL);
> >  
> >  	if (fbi->map_cpu) {
> > @@ -506,14 +629,21 @@ static int __init imxfb_probe(struct platform_device *pdev)
> >  
> >  	platform_set_drvdata(pdev, info);
> >  
> > -	ret = imxfb_init_fbinfo(&pdev->dev);
> > +	ret = imxfb_init_fbinfo(pdev);
> >  	if( ret < 0 )
> >  		goto failed_init;
> >  
> > -	res = request_mem_region(res->start, res->end - res->start + 1, "IMXFB");
> > +	res = request_mem_region(res->start, res->end - res->start + 1,
> > +				DRIVER_NAME);
> 
> There is a function resource_size(res) which does (res->end - res->start + 1).
> It seems useful here and at few places below.

ok, haven't seen it so far, thanks

 >  
> > -static int imxfb_remove(struct platform_device *pdev)
> > +static int __devexit imxfb_remove(struct platform_device *pdev)
> >  {
> >  	struct fb_info *info = platform_get_drvdata(pdev);
> >  	struct imxfb_info *fbi = info->par;
> > @@ -586,6 +718,7 @@ static int imxfb_remove(struct platform_device *pdev)
> >  	kfree(info->pseudo_palette);
> >  	framebuffer_release(info);
> >  
> > +	iounmap(fbi->regs);
> 
> The fbi pointed does not exist here (after framebuffer_release(info)).

Ack

 >  
> >  struct imxfb_info {
> > -	struct device		*dev;
> > +	struct platform_device  *pdev;
> 
> Is this field really useful? It can be dropped.

ok

-- 
 Pengutronix - Linux Solutions for Science and Industry
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
     Hannoversche Str. 2, 31134 Hildesheim, Germany
   Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [PATCH] i.MX Framebuffer: Use iowrite/ioread instead of direct pointer deref

[Krzysztof Helt]

On Wed, 20 Aug 2008 18:31:09 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> On Wed, Aug 20, 2008 at 05:31:59PM +0200, Krzysztof Helt wrote:
> > On Tue, 19 Aug 2008 17:06:41 +0200
> > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > 
> > > From: Juergen Beisert <j.beisert@pengutronix.de>
> > > 
> > > This patch prepares the current i.MX1 framebuffer driver for usage in the
> > > whole i.MX family. It switches to iowrite/ioread for register accesses.
> > > Also it moves the register definitions to the driver where they belong.
> > > 
> > > Signed-off-by: Juergen Beisert <j.beisert@pengutronix.de>
> > > ---
> > >  drivers/video/imxfb.c |  208 ++++++++++++++++++++++++++++++++++++++++---------
> > >  drivers/video/imxfb.h |    4 +-
> > >  2 files changed, 173 insertions(+), 39 deletions(-)
> > > 
> > > diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> > > index a990d43..0622589 100644
> > > --- a/drivers/video/imxfb.c
> > > +++ b/drivers/video/imxfb.c
> > > @@ -16,7 +16,6 @@
> > >   *	linux-arm-kernel@lists.arm.linux.org.uk
> > >   */
> > >  
> > > -//#define DEBUG 1
> > >  
> > >  #include <linux/module.h>
> > >  #include <linux/kernel.h>
> > > @@ -33,7 +32,6 @@
> > >  #include <linux/platform_device.h>
> > >  #include <linux/dma-mapping.h>
> > >  
> > > -#include <mach/hardware.h>
> > >  #include <asm/io.h>
> > >  #include <mach/imxfb.h>
> > >  
> > > @@ -44,6 +42,124 @@
> > >  
> > >  #include "imxfb.h"
> > >  
> > > +#define DRIVER_NAME "imx-fb"
> > > +
> > 
> > All constants below should really be put into the imxfb.h. There is already such a file.
> 
> AFAIK the-new-beautiful-way is not to have these header files next to
> the .c files at all and put this stuff into the .c file. So maybe it's
> better to move the rest of imxfb.h here aswell.
> 

I was not aware about "the-new-beautiful-way". I am just curious about this.
Normally, the headers for frame buffers are located at include/video. 

It can stay this way (no header) if you insist.

> 
>  >  
> > > -#define LCDC_PALETTE(x) __REG2(IMX_LCDC_BASE+0x800, (x)<<2)
> > >  static int
> > >  imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
> > >  		       u_int trans, struct fb_info *info)
> > > @@ -81,7 +196,7 @@ imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
> > >  		      (CNVT_TOHW(green,4) << 4) |
> > >  		      CNVT_TOHW(blue,  4);
> > >  
> > > -		LCDC_PALETTE(regno) = val;
> > > +		iowrite32(val, fbi->regs + 0x800 + (regno << 2));
> > 
> > One may define an inline function imxfb_write(fbi, val, reg) :
> > 
> >  imxfb_iowrite(fbi, val, reg) 
> > {
> > 	iowrite32(val, fbi->regs + reg);
> > }
> > 
> > It would make the code more readable. The imxfb_ioread() could be used as well.
> >
> 
> Well, I think this is a matter of taste, I prefer not using wrappers
> around the access functions.
> 

Ok.

Regards,
Krzysztof

---------------------------------------------------------------------- 	
Prosty kredyt na wszystkie potrzeby. 
Od 1000 zl do 120 000 zl. 
Bez poreczycieli i zabezpieczen >>> http://link.interia.pl/f1edf


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [PATCH] i.MX Framebuffer: Use iowrite/ioread instead of direct pointer deref

[Geert Uytterhoeven]

On Wed, 20 Aug 2008, Krzysztof Helt wrote:
> On Wed, 20 Aug 2008 18:31:09 +0200
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Wed, Aug 20, 2008 at 05:31:59PM +0200, Krzysztof Helt wrote:
> > > On Tue, 19 Aug 2008 17:06:41 +0200
> > > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > > > +#define DRIVER_NAME "imx-fb"
> > > > +
> > > 
> > > All constants below should really be put into the imxfb.h. There is already such a file.
> > 
> > AFAIK the-new-beautiful-way is not to have these header files next to
> > the .c files at all and put this stuff into the .c file. So maybe it's
> > better to move the rest of imxfb.h here aswell.
> 
> I was not aware about "the-new-beautiful-way". I am just curious about this.
> Normally, the headers for frame buffers are located at include/video. 
> 
> It can stay this way (no header) if you insist.

If the header file is included by a single source file only, it doesn't
make much sense to have the separate header file.

include/video/ is for header files that are included by multiple source
files, some outside drivers/video/.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

[PATCH] i.MX Framebuffer: Cleanup Coding style

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/video/imxfb.c |   78 +++++++++++++++++++++++++------------------------
 1 files changed, 40 insertions(+), 38 deletions(-)

diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index 0622589..e450e6b 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -1,6 +1,4 @@
 /*
- *  linux/drivers/video/imxfb.c
- *
  *  Freescale i.MX Frame Buffer device driver
  *
  *  Copyright (C) 2004 Sascha Hauer, Pengutronix
@@ -161,20 +159,21 @@
 #define LCDISR_BOF	(1<<0)
 
 static struct imxfb_rgb def_rgb_16 = {
-	.red	= { .offset = 8,  .length = 4, },
-	.green	= { .offset = 4,  .length = 4, },
-	.blue	= { .offset = 0,  .length = 4, },
-	.transp = { .offset = 0,  .length = 0, },
+	.red	= {.offset = 8, .length = 4,},
+	.green	= {.offset = 4, .length = 4,},
+	.blue	= {.offset = 0, .length = 4,},
+	.transp = {.offset = 0, .length = 0,},
 };
 
 static struct imxfb_rgb def_rgb_8 = {
-	.red	= { .offset = 0,  .length = 8, },
-	.green	= { .offset = 0,  .length = 8, },
-	.blue	= { .offset = 0,  .length = 8, },
-	.transp = { .offset = 0,  .length = 0, },
+	.red	= {.offset = 0, .length = 8,},
+	.green	= {.offset = 0, .length = 8,},
+	.blue	= {.offset = 0, .length = 8,},
+	.transp = {.offset = 0, .length = 0,},
 };
 
-static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *info);
+static int imxfb_activate_var(struct fb_var_screeninfo *var,
+		struct fb_info *info);
 
 static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)
 {
@@ -185,7 +184,7 @@ static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)
 
 static int
 imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
-		       u_int trans, struct fb_info *info)
+		u_int trans, struct fb_info *info)
 {
 	struct imxfb_info *fbi = info->par;
 	u_int val, ret = 1;
@@ -250,7 +249,8 @@ imxfb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
 
 	case FB_VISUAL_STATIC_PSEUDOCOLOR:
 	case FB_VISUAL_PSEUDOCOLOR:
-		ret = imxfb_setpalettereg(regno, red, green, blue, trans, info);
+		ret = imxfb_setpalettereg(regno, red, green, blue, trans,
+				info);
 		break;
 	}
 
@@ -336,8 +336,7 @@ static int imxfb_set_par(struct fb_info *info)
 		info->fix.visual = FB_VISUAL_STATIC_PSEUDOCOLOR;
 	}
 
-	info->fix.line_length = var->xres_virtual *
-				  var->bits_per_pixel / 8;
+	info->fix.line_length = var->xres_virtual * var->bits_per_pixel / 8;
 	fbi->palette_size = var->bits_per_pixel == 8 ? 256 : 16;
 
 	imxfb_activate_var(var, info);
@@ -368,9 +367,9 @@ static void imxfb_enable_controller(struct imxfb_info *fbi)
 
 	iowrite32(RMCR_LCDC_EN, fbi->regs + LCDC_RMCR);
 
-	if(fbi->backlight_power)
+	if (fbi->backlight_power)
 		fbi->backlight_power(1);
-	if(fbi->lcd_power)
+	if (fbi->lcd_power)
 		fbi->lcd_power(1);
 }
 
@@ -378,9 +377,9 @@ static void imxfb_disable_controller(struct imxfb_info *fbi)
 {
 	pr_debug("Disabling LCD controller\n");
 
-	if(fbi->backlight_power)
+	if (fbi->backlight_power)
 		fbi->backlight_power(0);
-	if(fbi->lcd_power)
+	if (fbi->lcd_power)
 		fbi->lcd_power(0);
 
 	iowrite32(0, fbi->regs + LCDC_RMCR);
@@ -488,7 +487,8 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
 static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
 {
 	struct imxfb_info *fbi = platform_get_drvdata(dev);
-	pr_debug("%s\n",__func__);
+
+	pr_debug("%s\n", __func__);
 
 	imxfb_disable_controller(fbi);
 	return 0;
@@ -497,7 +497,8 @@ static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
 static int imxfb_resume(struct platform_device *dev)
 {
 	struct imxfb_info *fbi = platform_get_drvdata(dev);
-	pr_debug("%s\n",__func__);
+
+	pr_debug("%s\n", __func__);
 
 	imxfb_enable_controller(fbi);
 	return 0;
@@ -515,31 +516,32 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
 
 	pr_debug("%s\n",__func__);
 
-	info->pseudo_palette = kmalloc( sizeof(u32) * 16, GFP_KERNEL);
+	info->pseudo_palette = kmalloc(sizeof (u32) * 16, GFP_KERNEL);
 	if (!info->pseudo_palette)
 		return -ENOMEM;
 
-	memset(fbi, 0, sizeof(struct imxfb_info));
+	memset(fbi, 0, sizeof (struct imxfb_info));
 	fbi->pdev = pdev;
 
-	strlcpy(info->fix.id, IMX_NAME, sizeof(info->fix.id));
+	strlcpy(info->fix.id, IMX_NAME, sizeof (info->fix.id));
 
-	info->fix.type	= FB_TYPE_PACKED_PIXELS;
+	info->fix.type			= FB_TYPE_PACKED_PIXELS;
 	info->fix.type_aux		= 0;
 	info->fix.xpanstep		= 0;
 	info->fix.ypanstep		= 0;
 	info->fix.ywrapstep		= 0;
-	info->fix.accel	= FB_ACCEL_NONE;
+	info->fix.accel			= FB_ACCEL_NONE;
 
 	info->var.nonstd		= 0;
 	info->var.activate		= FB_ACTIVATE_NOW;
 	info->var.height		= -1;
 	info->var.width	= -1;
 	info->var.accel_flags		= 0;
-	info->var.vmode	= FB_VMODE_NONINTERLACED;
+	info->var.vmode			= FB_VMODE_NONINTERLACED;
 
 	info->fbops			= &imxfb_ops;
-	info->flags			= FBINFO_FLAG_DEFAULT | FBINFO_READS_FAST;
+	info->flags			= FBINFO_FLAG_DEFAULT |
+					  FBINFO_READS_FAST;
 
 	fbi->rgb[RGB_16]		= &def_rgb_16;
 	fbi->rgb[RGB_8]			= &def_rgb_8;
@@ -577,11 +579,11 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
 }
 
 /*
- *      Allocates the DRAM memory for the frame buffer.  This buffer is
- *	remapped into a non-cached, non-buffered, memory region to
- *      allow pixel writes to occur without flushing the cache.
- *      Once this area is remapped, all virtual memory access to the
- *      video memory should occur at the new region.
+ * Allocates the DRAM memory for the frame buffer.  This buffer is
+ * remapped into a non-cached, non-buffered, memory region to
+ * allow pixel writes to occur without flushing the cache.
+ * Once this area is remapped, all virtual memory access to the
+ * video memory should occur at the new region.
  */
 static int __init imxfb_map_video_memory(struct fb_info *info)
 {
@@ -612,17 +614,17 @@ static int __init imxfb_probe(struct platform_device *pdev)
 	printk("i.MX Framebuffer driver\n");
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if(!res)
+	if (!res)
 		return -ENODEV;
 
 	inf = pdev->dev.platform_data;
-	if(!inf) {
+	if (!inf) {
 		dev_err(&pdev->dev,"No platform_data available\n");
 		return -ENOMEM;
 	}
 
 	info = framebuffer_alloc(sizeof(struct imxfb_info), &pdev->dev);
-	if(!info)
+	if (!info)
 		return -ENOMEM;
 
 	fbi = info->par;
@@ -630,7 +632,7 @@ static int __init imxfb_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, info);
 
 	ret = imxfb_init_fbinfo(pdev);
-	if( ret < 0 )
+	if (ret < 0)
 		goto failed_init;
 
 	res = request_mem_region(res->start, res->end - res->start + 1,
@@ -669,7 +671,7 @@ static int __init imxfb_probe(struct platform_device *pdev)
 	 */
 	imxfb_check_var(&info->var, info);
 
-	ret = fb_alloc_cmap(&info->cmap, 1<<info->var.bits_per_pixel, 0);
+	ret = fb_alloc_cmap(&info->cmap, 1 << info->var.bits_per_pixel, 0);
 	if (ret < 0)
 		goto failed_cmap;
 
-- 
1.5.6.3


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [PATCH] i.MX Framebuffer: Cleanup Coding style

[Krzysztof Helt]

On Tue, 19 Aug 2008 17:06:42 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/video/imxfb.c |   78 +++++++++++++++++++++++++------------------------
>  1 files changed, 40 insertions(+), 38 deletions(-)
> 

I don't like some of the changes like wrapping lines
which are not longer than 80 characters or removing
alignment vs previous line because it contains spaces.
There are also positive CS changes so not everything is bad.

> diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> index 0622589..e450e6b 100644
> --- a/drivers/video/imxfb.c
> +++ b/drivers/video/imxfb.c
> @@ -1,6 +1,4 @@
>  /*
> - *  linux/drivers/video/imxfb.c
> - *
>   *  Freescale i.MX Frame Buffer device driver
>   *
>   *  Copyright (C) 2004 Sascha Hauer, Pengutronix
> @@ -161,20 +159,21 @@
>  #define LCDISR_BOF	(1<<0)
>  
>  static struct imxfb_rgb def_rgb_16 = {
> -	.red	= { .offset = 8,  .length = 4, },
> -	.green	= { .offset = 4,  .length = 4, },
> -	.blue	= { .offset = 0,  .length = 4, },
> -	.transp = { .offset = 0,  .length = 0, },
> +	.red	= {.offset = 8, .length = 4,},
> +	.green	= {.offset = 4, .length = 4,},
> +	.blue	= {.offset = 0, .length = 4,},
> +	.transp = {.offset = 0, .length = 0,},
>  };
>  
>  static struct imxfb_rgb def_rgb_8 = {
> -	.red	= { .offset = 0,  .length = 8, },
> -	.green	= { .offset = 0,  .length = 8, },
> -	.blue	= { .offset = 0,  .length = 8, },
> -	.transp = { .offset = 0,  .length = 0, },
> +	.red	= {.offset = 0, .length = 8,},
> +	.green	= {.offset = 0, .length = 8,},
> +	.blue	= {.offset = 0, .length = 8,},
> +	.transp = {.offset = 0, .length = 0,},
>  };
>  
> -static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *info);
> +static int imxfb_activate_var(struct fb_var_screeninfo *var,
> +		struct fb_info *info);
>  
>  static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)
>  {
> @@ -185,7 +184,7 @@ static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)
>  
>  static int
>  imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
> -		       u_int trans, struct fb_info *info)
> +		u_int trans, struct fb_info *info)
>  {
>  	struct imxfb_info *fbi = info->par;
>  	u_int val, ret = 1;
> @@ -250,7 +249,8 @@ imxfb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
>  
>  	case FB_VISUAL_STATIC_PSEUDOCOLOR:
>  	case FB_VISUAL_PSEUDOCOLOR:
> -		ret = imxfb_setpalettereg(regno, red, green, blue, trans, info);
> +		ret = imxfb_setpalettereg(regno, red, green, blue, trans,
> +				info);
>  		break;
>  	}
>  
> @@ -336,8 +336,7 @@ static int imxfb_set_par(struct fb_info *info)
>  		info->fix.visual = FB_VISUAL_STATIC_PSEUDOCOLOR;
>  	}
>  
> -	info->fix.line_length = var->xres_virtual *
> -				  var->bits_per_pixel / 8;
> +	info->fix.line_length = var->xres_virtual * var->bits_per_pixel / 8;
>  	fbi->palette_size = var->bits_per_pixel == 8 ? 256 : 16;
>  
>  	imxfb_activate_var(var, info);
> @@ -368,9 +367,9 @@ static void imxfb_enable_controller(struct imxfb_info *fbi)
>  
>  	iowrite32(RMCR_LCDC_EN, fbi->regs + LCDC_RMCR);
>  
> -	if(fbi->backlight_power)
> +	if (fbi->backlight_power)
>  		fbi->backlight_power(1);
> -	if(fbi->lcd_power)
> +	if (fbi->lcd_power)
>  		fbi->lcd_power(1);
>  }
>  
> @@ -378,9 +377,9 @@ static void imxfb_disable_controller(struct imxfb_info *fbi)
>  {
>  	pr_debug("Disabling LCD controller\n");
>  
> -	if(fbi->backlight_power)
> +	if (fbi->backlight_power)
>  		fbi->backlight_power(0);
> -	if(fbi->lcd_power)
> +	if (fbi->lcd_power)
>  		fbi->lcd_power(0);
>  
>  	iowrite32(0, fbi->regs + LCDC_RMCR);
> @@ -488,7 +487,8 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
>  static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
>  {
>  	struct imxfb_info *fbi = platform_get_drvdata(dev);
> -	pr_debug("%s\n",__func__);
> +
> +	pr_debug("%s\n", __func__);
>  
>  	imxfb_disable_controller(fbi);
>  	return 0;
> @@ -497,7 +497,8 @@ static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
>  static int imxfb_resume(struct platform_device *dev)
>  {
>  	struct imxfb_info *fbi = platform_get_drvdata(dev);
> -	pr_debug("%s\n",__func__);
> +
> +	pr_debug("%s\n", __func__);
>  
>  	imxfb_enable_controller(fbi);
>  	return 0;
> @@ -515,31 +516,32 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
>  
>  	pr_debug("%s\n",__func__);
>  
> -	info->pseudo_palette = kmalloc( sizeof(u32) * 16, GFP_KERNEL);
> +	info->pseudo_palette = kmalloc(sizeof (u32) * 16, GFP_KERNEL);
>  	if (!info->pseudo_palette)
>  		return -ENOMEM;
>  
> -	memset(fbi, 0, sizeof(struct imxfb_info));
> +	memset(fbi, 0, sizeof (struct imxfb_info));
>  	fbi->pdev = pdev;
>  
> -	strlcpy(info->fix.id, IMX_NAME, sizeof(info->fix.id));
> +	strlcpy(info->fix.id, IMX_NAME, sizeof (info->fix.id));
>  
> -	info->fix.type	= FB_TYPE_PACKED_PIXELS;
> +	info->fix.type			= FB_TYPE_PACKED_PIXELS;
>  	info->fix.type_aux		= 0;
>  	info->fix.xpanstep		= 0;
>  	info->fix.ypanstep		= 0;
>  	info->fix.ywrapstep		= 0;
> -	info->fix.accel	= FB_ACCEL_NONE;
> +	info->fix.accel			= FB_ACCEL_NONE;
>  
>  	info->var.nonstd		= 0;
>  	info->var.activate		= FB_ACTIVATE_NOW;
>  	info->var.height		= -1;
>  	info->var.width	= -1;
>  	info->var.accel_flags		= 0;
> -	info->var.vmode	= FB_VMODE_NONINTERLACED;
> +	info->var.vmode			= FB_VMODE_NONINTERLACED;
>  
>  	info->fbops			= &imxfb_ops;
> -	info->flags			= FBINFO_FLAG_DEFAULT | FBINFO_READS_FAST;
> +	info->flags			= FBINFO_FLAG_DEFAULT |
> +					  FBINFO_READS_FAST;
>  
>  	fbi->rgb[RGB_16]		= &def_rgb_16;
>  	fbi->rgb[RGB_8]			= &def_rgb_8;
> @@ -577,11 +579,11 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
>  }
>  
>  /*
> - *      Allocates the DRAM memory for the frame buffer.  This buffer is
> - *	remapped into a non-cached, non-buffered, memory region to
> - *      allow pixel writes to occur without flushing the cache.
> - *      Once this area is remapped, all virtual memory access to the
> - *      video memory should occur at the new region.
> + * Allocates the DRAM memory for the frame buffer.  This buffer is
> + * remapped into a non-cached, non-buffered, memory region to
> + * allow pixel writes to occur without flushing the cache.
> + * Once this area is remapped, all virtual memory access to the
> + * video memory should occur at the new region.
>   */
>  static int __init imxfb_map_video_memory(struct fb_info *info)
>  {
> @@ -612,17 +614,17 @@ static int __init imxfb_probe(struct platform_device *pdev)
>  	printk("i.MX Framebuffer driver\n");
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if(!res)
> +	if (!res)
>  		return -ENODEV;
>  
>  	inf = pdev->dev.platform_data;
> -	if(!inf) {
> +	if (!inf) {
>  		dev_err(&pdev->dev,"No platform_data available\n");
>  		return -ENOMEM;
>  	}
>  
>  	info = framebuffer_alloc(sizeof(struct imxfb_info), &pdev->dev);
> -	if(!info)
> +	if (!info)
>  		return -ENOMEM;
>  
>  	fbi = info->par;
> @@ -630,7 +632,7 @@ static int __init imxfb_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, info);
>  
>  	ret = imxfb_init_fbinfo(pdev);
> -	if( ret < 0 )
> +	if (ret < 0)
>  		goto failed_init;
>  
>  	res = request_mem_region(res->start, res->end - res->start + 1,
> @@ -669,7 +671,7 @@ static int __init imxfb_probe(struct platform_device *pdev)
>  	 */
>  	imxfb_check_var(&info->var, info);
>  
> -	ret = fb_alloc_cmap(&info->cmap, 1<<info->var.bits_per_pixel, 0);
> +	ret = fb_alloc_cmap(&info->cmap, 1 << info->var.bits_per_pixel, 0);
>  	if (ret < 0)
>  		goto failed_cmap;
>  
> -- 
> 1.5.6.3
> 
> 
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
> Build the coolest Linux based applications with Moblin SDK & win great prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Linux-fbdev-devel mailing list
> Linux-fbdev-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel
> 

----------------------------------------------------------------------
Taniej na zagraniczne komorki!
Sprawdz >>>  http://link.interia.pl/f1ee3 


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [PATCH] i.MX Framebuffer: Cleanup Coding style

[Sascha Hauer]

On Wed, Aug 20, 2008 at 05:35:01PM +0200, Krzysztof Helt wrote:
> On Tue, 19 Aug 2008 17:06:42 +0200
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/video/imxfb.c |   78 +++++++++++++++++++++++++------------------------
> >  1 files changed, 40 insertions(+), 38 deletions(-)
> > 
> 
> I don't like some of the changes like wrapping lines
> which are not longer than 80 characters or removing
> alignment vs previous line because it contains spaces.
> There are also positive CS changes so not everything is bad.

Hm, this patch only wraps lines longer than 80 characters (in one case
it even merges two lines to one because it's still shorter than 80
chars). It also does not remove alignment vs the previous line, instead
it aligns some lines against the previous line. Maybe reading whitespace
changes in patches is a bit confusing because the +/- at line starts
mess it up.

Sascha

-- 
 Pengutronix - Linux Solutions for Science and Industry
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
     Hannoversche Str. 2, 31134 Hildesheim, Germany
   Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [PATCH] i.MX Framebuffer: Cleanup Coding style

[Krzysztof Helt]

On Wed, 20 Aug 2008 18:15:21 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> On Wed, Aug 20, 2008 at 05:35:01PM +0200, Krzysztof Helt wrote:
> > On Tue, 19 Aug 2008 17:06:42 +0200
> > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > 
> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > ---
> > >  drivers/video/imxfb.c |   78 +++++++++++++++++++++++++------------------------
> > >  1 files changed, 40 insertions(+), 38 deletions(-)
> > > 
> > 
> > I don't like some of the changes like wrapping lines
> > which are not longer than 80 characters or removing
> > alignment vs previous line because it contains spaces.
> > There are also positive CS changes so not everything is bad.
> 
> Hm, this patch only wraps lines longer than 80 characters (in one case
> it even merges two lines to one because it's still shorter than 80
> chars).

The change around line 250 wraps 80 chars line.

@@ -250,7 +249,8 @@ imxfb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
 
 	case FB_VISUAL_STATIC_PSEUDOCOLOR:
 	case FB_VISUAL_PSEUDOCOLOR:
-		ret = imxfb_setpalettereg(regno, red, green, blue, trans, info);
+		ret = imxfb_setpalettereg(regno, red, green, blue, trans,
+				info);
 		break;
 	}


> It also does not remove alignment vs the previous line, instead
> it aligns some lines against the previous line. 


@@ -185,7 +184,7 @@ static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)

 static int
 imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
-                      u_int trans, struct fb_info *info)
+               u_int trans, struct fb_info *info)
 {
        struct imxfb_info *fbi = info->par;
        u_int val, ret = 1;

The removed line was previously aligned to start after the "(" in the previous line.
One could move "static int" into the same line.

You have added spaces between sizeof operator and the opening parenthesis.
It is pointed out by the checkpatch script.

> Maybe reading whitespace
> changes in patches is a bit confusing because the +/- at line starts
> mess it up.
>
 
The changes I pointed above are ones I don't like. They are not
bringing any benefit (nor better CS conformance nor readability).

However, I recognize your patch does more positive changes then these
pointed above. Also, my feelings toward some changes are my personal
opinion and I won't block the patch you have posted. I would like
to see the mentioned changes dropped but it is up to you.

Friendly regards,
Krzysztof

---------------------------------------------------------------------- 
Igrzyska na Dzikim Zachodzie!
Sprawdz >> http://link.interia.pl/f1edc 
	


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [PATCH] i.MX Framebuffer: Cleanup Coding style

[Sascha Hauer]

On Wed, Aug 20, 2008 at 07:35:43PM +0200, Krzysztof Helt wrote:
> On Wed, 20 Aug 2008 18:15:21 +0200
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > On Wed, Aug 20, 2008 at 05:35:01PM +0200, Krzysztof Helt wrote:
> > > On Tue, 19 Aug 2008 17:06:42 +0200
> > > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > > 
> > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > > ---
> > > >  drivers/video/imxfb.c |   78 +++++++++++++++++++++++++------------------------
> > > >  1 files changed, 40 insertions(+), 38 deletions(-)
> > > > 
> > > 
> > > I don't like some of the changes like wrapping lines
> > > which are not longer than 80 characters or removing
> > > alignment vs previous line because it contains spaces.
> > > There are also positive CS changes so not everything is bad.
> > 
> > Hm, this patch only wraps lines longer than 80 characters (in one case
> > it even merges two lines to one because it's still shorter than 80
> > chars).
> 
> The change around line 250 wraps 80 chars line.
> 
> @@ -250,7 +249,8 @@ imxfb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
>  
>  	case FB_VISUAL_STATIC_PSEUDOCOLOR:
>  	case FB_VISUAL_PSEUDOCOLOR:
> -		ret = imxfb_setpalettereg(regno, red, green, blue, trans, info);
> +		ret = imxfb_setpalettereg(regno, red, green, blue, trans,
> +				info);

Ok, you're right. I just saw my cursor blinking on position 81, but of
course that means the line is 80 characters long. My bad

>  		break;
>  	}
> 
> 
> > It also does not remove alignment vs the previous line, instead
> > it aligns some lines against the previous line. 
> 
> 
> @@ -185,7 +184,7 @@ static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)
> 
>  static int
>  imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
> -                      u_int trans, struct fb_info *info)
> +               u_int trans, struct fb_info *info)
>  {
>         struct imxfb_info *fbi = info->par;
>         u_int val, ret = 1;
> 
> The removed line was previously aligned to start after the "(" in the previous line.
> One could move "static int" into the same line.
> 
> You have added spaces between sizeof operator and the opening parenthesis.
> It is pointed out by the checkpatch script.

Ah, yes. This one I have copied from a Lindent run.

> 
> > Maybe reading whitespace
> > changes in patches is a bit confusing because the +/- at line starts
> > mess it up.
> >
>  
> The changes I pointed above are ones I don't like. They are not
> bringing any benefit (nor better CS conformance nor readability).
> 
> However, I recognize your patch does more positive changes then these
> pointed above. Also, my feelings toward some changes are my personal
> opinion and I won't block the patch you have posted. I would like
> to see the mentioned changes dropped but it is up to you.

I will rework it.

Regards,
  Sascha


-- 
 Pengutronix - Linux Solutions for Science and Industry
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
     Hannoversche Str. 2, 31134 Hildesheim, Germany
   Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

[PATCH] i.MX Framebuffer: rename imxfb_mach_info to imx_fb_platform_data

[Sascha Hauer]

rename imxfb_mach_info to a name more common to kernel hackers

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/mach-imx/include/mach/imxfb.h |    5 +-
 drivers/video/imxfb.c                  |   68 ++++++++++++++++----------------
 2 files changed, 37 insertions(+), 36 deletions(-)

diff --git a/arch/arm/mach-imx/include/mach/imxfb.h b/arch/arm/mach-imx/include/mach/imxfb.h
index 3ed9ec8..ffcff5a 100644
--- a/arch/arm/mach-imx/include/mach/imxfb.h
+++ b/arch/arm/mach-imx/include/mach/imxfb.h
@@ -1,7 +1,7 @@
 /*
  * This structure describes the machine which we are running on.
  */
-struct imxfb_mach_info {
+struct imx_fb_platform_data {
 	u_long		pixclock;
 
 	u_short		xres;
@@ -34,4 +34,5 @@ struct imxfb_mach_info {
 	void (*lcd_power)(int);
 	void (*backlight_power)(int);
 };
-void set_imx_fb_info(struct imxfb_mach_info *hard_imx_fb_info);
+
+void set_imx_fb_info(struct imx_fb_platform_data *);
diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index e450e6b..a4a4940 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -510,7 +510,7 @@ static int imxfb_resume(struct platform_device *dev)
 
 static int __init imxfb_init_fbinfo(struct platform_device *pdev)
 {
-	struct imxfb_mach_info *inf = pdev->dev.platform_data;
+	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;
 
@@ -546,32 +546,32 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
 	fbi->rgb[RGB_16]		= &def_rgb_16;
 	fbi->rgb[RGB_8]			= &def_rgb_8;
 
-	fbi->max_xres			= inf->xres;
-	info->var.xres			= inf->xres;
-	info->var.xres_virtual		= inf->xres;
-	fbi->max_yres			= inf->yres;
-	info->var.yres			= inf->yres;
-	info->var.yres_virtual		= inf->yres;
-	fbi->max_bpp			= inf->bpp;
-	info->var.bits_per_pixel	= inf->bpp;
-	info->var.nonstd		= inf->nonstd;
-	info->var.pixclock		= inf->pixclock;
-	info->var.hsync_len		= inf->hsync_len;
-	info->var.left_margin		= inf->left_margin;
-	info->var.right_margin		= inf->right_margin;
-	info->var.vsync_len		= inf->vsync_len;
-	info->var.upper_margin		= inf->upper_margin;
-	info->var.lower_margin		= inf->lower_margin;
-	info->var.sync			= inf->sync;
-	info->var.grayscale		= inf->cmap_greyscale;
-	fbi->cmap_inverse		= inf->cmap_inverse;
-	fbi->cmap_static		= inf->cmap_static;
-	fbi->pcr			= inf->pcr;
-	fbi->lscr1			= inf->lscr1;
-	fbi->dmacr			= inf->dmacr;
-	fbi->pwmr			= inf->pwmr;
-	fbi->lcd_power			= inf->lcd_power;
-	fbi->backlight_power		= inf->backlight_power;
+	fbi->max_xres			= pdata->xres;
+	info->var.xres			= pdata->xres;
+	info->var.xres_virtual		= pdata->xres;
+	fbi->max_yres			= pdata->yres;
+	info->var.yres			= pdata->yres;
+	info->var.yres_virtual		= pdata->yres;
+	fbi->max_bpp			= pdata->bpp;
+	info->var.bits_per_pixel	= pdata->bpp;
+	info->var.nonstd		= pdata->nonstd;
+	info->var.pixclock		= pdata->pixclock;
+	info->var.hsync_len		= pdata->hsync_len;
+	info->var.left_margin		= pdata->left_margin;
+	info->var.right_margin		= pdata->right_margin;
+	info->var.vsync_len		= pdata->vsync_len;
+	info->var.upper_margin		= pdata->upper_margin;
+	info->var.lower_margin		= pdata->lower_margin;
+	info->var.sync			= pdata->sync;
+	info->var.grayscale		= pdata->cmap_greyscale;
+	fbi->cmap_inverse		= pdata->cmap_inverse;
+	fbi->cmap_static		= pdata->cmap_static;
+	fbi->pcr			= pdata->pcr;
+	fbi->lscr1			= pdata->lscr1;
+	fbi->dmacr			= pdata->dmacr;
+	fbi->pwmr			= pdata->pwmr;
+	fbi->lcd_power			= pdata->lcd_power;
+	fbi->backlight_power		= pdata->backlight_power;
 	info->fix.smem_len		= fbi->max_xres * fbi->max_yres *
 					  fbi->max_bpp / 8;
 
@@ -607,7 +607,7 @@ static int __init imxfb_probe(struct platform_device *pdev)
 {
 	struct imxfb_info *fbi;
 	struct fb_info *info;
-	struct imxfb_mach_info *inf;
+	struct imx_fb_platform_data *pdata;
 	struct resource *res;
 	int ret;
 
@@ -617,8 +617,8 @@ static int __init imxfb_probe(struct platform_device *pdev)
 	if (!res)
 		return -ENODEV;
 
-	inf = pdev->dev.platform_data;
-	if (!inf) {
+	pdata = pdev->dev.platform_data;
+	if (!pdata) {
 		dev_err(&pdev->dev,"No platform_data available\n");
 		return -ENOMEM;
 	}
@@ -648,7 +648,7 @@ static int __init imxfb_probe(struct platform_device *pdev)
 		goto failed_ioremap;
 	}
 
-	if (!inf->fixed_screen_cpu) {
+	if (!pdata->fixed_screen_cpu) {
 		ret = imxfb_map_video_memory(info);
 		if (ret) {
 			dev_err(&pdev->dev, "Failed to allocate video RAM: %d\n", ret);
@@ -657,8 +657,8 @@ static int __init imxfb_probe(struct platform_device *pdev)
 		}
 	} else {
 		/* Fixed framebuffer mapping enables location of the screen in eSRAM */
-		fbi->map_cpu = inf->fixed_screen_cpu;
-		fbi->map_dma = inf->fixed_screen_dma;
+		fbi->map_cpu = pdata->fixed_screen_cpu;
+		fbi->map_dma = pdata->fixed_screen_dma;
 		info->screen_base = fbi->map_cpu;
 		fbi->screen_cpu = fbi->map_cpu;
 		fbi->screen_dma = fbi->map_dma;
@@ -689,7 +689,7 @@ static int __init imxfb_probe(struct platform_device *pdev)
 failed_register:
 	fb_dealloc_cmap(&info->cmap);
 failed_cmap:
-	if (!inf->fixed_screen_cpu)
+	if (!pdata->fixed_screen_cpu)
 		dma_free_writecombine(&pdev->dev,fbi->map_size,fbi->map_cpu,
 			fbi->map_dma);
 failed_map:
-- 
1.5.6.3


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [PATCH] i.MX Framebuffer: rename imxfb_mach_info to imx_fb_platform_data

[Krzysztof Helt]

On Tue, 19 Aug 2008 17:06:43 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> rename imxfb_mach_info to a name more common to kernel hackers
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  arch/arm/mach-imx/include/mach/imxfb.h |    5 +-
>  drivers/video/imxfb.c                  |   68 ++++++++++++++++----------------
>  2 files changed, 37 insertions(+), 36 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/include/mach/imxfb.h b/arch/arm/mach-imx/include/mach/imxfb.h
> index 3ed9ec8..ffcff5a 100644
> --- a/arch/arm/mach-imx/include/mach/imxfb.h
> +++ b/arch/arm/mach-imx/include/mach/imxfb.h
> @@ -1,7 +1,7 @@
>  /*
>   * This structure describes the machine which we are running on.
>   */
> -struct imxfb_mach_info {
> +struct imx_fb_platform_data {
>  	u_long		pixclock;
>  
>  	u_short		xres;
> @@ -34,4 +34,5 @@ struct imxfb_mach_info {
>  	void (*lcd_power)(int);
>  	void (*backlight_power)(int);
>  };
> -void set_imx_fb_info(struct imxfb_mach_info *hard_imx_fb_info);
> +
> +void set_imx_fb_info(struct imx_fb_platform_data *);
> diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> index e450e6b..a4a4940 100644
> --- a/drivers/video/imxfb.c
> +++ b/drivers/video/imxfb.c
> @@ -510,7 +510,7 @@ static int imxfb_resume(struct platform_device *dev)
>  
>  static int __init imxfb_init_fbinfo(struct platform_device *pdev)
>  {
> -	struct imxfb_mach_info *inf = pdev->dev.platform_data;
> +	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;
>  
> @@ -546,32 +546,32 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
>  	fbi->rgb[RGB_16]		= &def_rgb_16;
>  	fbi->rgb[RGB_8]			= &def_rgb_8;
>  
> -	fbi->max_xres			= inf->xres;
> -	info->var.xres			= inf->xres;
> -	info->var.xres_virtual		= inf->xres;
> -	fbi->max_yres			= inf->yres;
> -	info->var.yres			= inf->yres;
> -	info->var.yres_virtual		= inf->yres;
> -	fbi->max_bpp			= inf->bpp;
> -	info->var.bits_per_pixel	= inf->bpp;
> -	info->var.nonstd		= inf->nonstd;
> -	info->var.pixclock		= inf->pixclock;
> -	info->var.hsync_len		= inf->hsync_len;
> -	info->var.left_margin		= inf->left_margin;
> -	info->var.right_margin		= inf->right_margin;
> -	info->var.vsync_len		= inf->vsync_len;
> -	info->var.upper_margin		= inf->upper_margin;
> -	info->var.lower_margin		= inf->lower_margin;
> -	info->var.sync			= inf->sync;
> -	info->var.grayscale		= inf->cmap_greyscale;
> -	fbi->cmap_inverse		= inf->cmap_inverse;
> -	fbi->cmap_static		= inf->cmap_static;
> -	fbi->pcr			= inf->pcr;
> -	fbi->lscr1			= inf->lscr1;
> -	fbi->dmacr			= inf->dmacr;
> -	fbi->pwmr			= inf->pwmr;
> -	fbi->lcd_power			= inf->lcd_power;
> -	fbi->backlight_power		= inf->backlight_power;
> +	fbi->max_xres			= pdata->xres;
> +	info->var.xres			= pdata->xres;
> +	info->var.xres_virtual		= pdata->xres;
> +	fbi->max_yres			= pdata->yres;
> +	info->var.yres			= pdata->yres;
> +	info->var.yres_virtual		= pdata->yres;
> +	fbi->max_bpp			= pdata->bpp;
> +	info->var.bits_per_pixel	= pdata->bpp;

These max_foo fields seem redundant. Especially, the max_bpp.
Most ARM chips can handle any bpp regardless the connected 
display (I don't know the iMX chips). Also, if resolution switching
is not allowed (usual for LCD displays) max_xres/yres is also 
redundant.

Otherwise, the patch is fine.

Regards,
Krzysztof

----------------------------------------------------------------------
Podbij Dziki Zachod, wygraj laptopa lub iPoda 
>> http://link.interia.pl/f1ede


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [PATCH] i.MX Framebuffer: rename imxfb_mach_info to imx_fb_platform_data

[Sascha Hauer]

On Wed, Aug 20, 2008 at 05:51:12PM +0200, Krzysztof Helt wrote:
> On Tue, 19 Aug 2008 17:06:43 +0200
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > rename imxfb_mach_info to a name more common to kernel hackers
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  arch/arm/mach-imx/include/mach/imxfb.h |    5 +-
> >  drivers/video/imxfb.c                  |   68 ++++++++++++++++----------------
> >  2 files changed, 37 insertions(+), 36 deletions(-)
> > 
> > diff --git a/arch/arm/mach-imx/include/mach/imxfb.h b/arch/arm/mach-imx/include/mach/imxfb.h
> > index 3ed9ec8..ffcff5a 100644
> > --- a/arch/arm/mach-imx/include/mach/imxfb.h
> > +++ b/arch/arm/mach-imx/include/mach/imxfb.h
> > @@ -1,7 +1,7 @@
> >  /*
> >   * This structure describes the machine which we are running on.
> >   */
> > -struct imxfb_mach_info {
> > +struct imx_fb_platform_data {
> >  	u_long		pixclock;
> >  
> >  	u_short		xres;
> > @@ -34,4 +34,5 @@ struct imxfb_mach_info {
> >  	void (*lcd_power)(int);
> >  	void (*backlight_power)(int);
> >  };
> > -void set_imx_fb_info(struct imxfb_mach_info *hard_imx_fb_info);
> > +
> > +void set_imx_fb_info(struct imx_fb_platform_data *);
> > diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> > index e450e6b..a4a4940 100644
> > --- a/drivers/video/imxfb.c
> > +++ b/drivers/video/imxfb.c
> > @@ -510,7 +510,7 @@ static int imxfb_resume(struct platform_device *dev)
> >  
> >  static int __init imxfb_init_fbinfo(struct platform_device *pdev)
> >  {
> > -	struct imxfb_mach_info *inf = pdev->dev.platform_data;
> > +	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;
> >  
> > @@ -546,32 +546,32 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
> >  	fbi->rgb[RGB_16]		= &def_rgb_16;
> >  	fbi->rgb[RGB_8]			= &def_rgb_8;
> >  
> > -	fbi->max_xres			= inf->xres;
> > -	info->var.xres			= inf->xres;
> > -	info->var.xres_virtual		= inf->xres;
> > -	fbi->max_yres			= inf->yres;
> > -	info->var.yres			= inf->yres;
> > -	info->var.yres_virtual		= inf->yres;
> > -	fbi->max_bpp			= inf->bpp;
> > -	info->var.bits_per_pixel	= inf->bpp;
> > -	info->var.nonstd		= inf->nonstd;
> > -	info->var.pixclock		= inf->pixclock;
> > -	info->var.hsync_len		= inf->hsync_len;
> > -	info->var.left_margin		= inf->left_margin;
> > -	info->var.right_margin		= inf->right_margin;
> > -	info->var.vsync_len		= inf->vsync_len;
> > -	info->var.upper_margin		= inf->upper_margin;
> > -	info->var.lower_margin		= inf->lower_margin;
> > -	info->var.sync			= inf->sync;
> > -	info->var.grayscale		= inf->cmap_greyscale;
> > -	fbi->cmap_inverse		= inf->cmap_inverse;
> > -	fbi->cmap_static		= inf->cmap_static;
> > -	fbi->pcr			= inf->pcr;
> > -	fbi->lscr1			= inf->lscr1;
> > -	fbi->dmacr			= inf->dmacr;
> > -	fbi->pwmr			= inf->pwmr;
> > -	fbi->lcd_power			= inf->lcd_power;
> > -	fbi->backlight_power		= inf->backlight_power;
> > +	fbi->max_xres			= pdata->xres;
> > +	info->var.xres			= pdata->xres;
> > +	info->var.xres_virtual		= pdata->xres;
> > +	fbi->max_yres			= pdata->yres;
> > +	info->var.yres			= pdata->yres;
> > +	info->var.yres_virtual		= pdata->yres;
> > +	fbi->max_bpp			= pdata->bpp;
> > +	info->var.bits_per_pixel	= pdata->bpp;
> 
> These max_foo fields seem redundant. Especially, the max_bpp.
> Most ARM chips can handle any bpp regardless the connected 
> display (I don't know the iMX chips). Also, if resolution switching
> is not allowed (usual for LCD displays) max_xres/yres is also 
> redundant.

Ok, I'll prepare another patch for this and let this one as is so that
it still only contains renaming.

Sascha

> 
> Otherwise, the patch is fine.
> 
> Regards,
> Krzysztof
> 
> ----------------------------------------------------------------------
> Podbij Dziki Zachod, wygraj laptopa lub iPoda 
> >> http://link.interia.pl/f1ede
> 
> 

-- 
 Pengutronix - Linux Solutions for Science and Industry
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
     Hannoversche Str. 2, 31134 Hildesheim, Germany
   Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

i.MX framebuffer patches

[Sascha Hauer]

These patches are the rework of the patches I sent some time ago based
on comments from Krzysztof Helt, Andrew Dyer and Russell King. The patches
are also available as git:

git://pasiphae.extern.pengutronix.de/git/imx/linux-2.6.git imxfb

Sascha



-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

[PATCH] i.MX Framebuffer: Use iowrite/ioread instead of direct pointer deref

[Sascha Hauer]

From: Juergen Beisert <j.beisert@pengutronix.de>

This patch prepares the current i.MX1 framebuffer driver for usage in the
whole i.MX family. It switches to readl/writel for register accesses.
Also it moves the register definitions to the driver where they belong.

Signed-off-by: Juergen Beisert <j.beisert@pengutronix.de>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/mach-imx/include/mach/imxfb.h |   45 ++++++++
 drivers/video/imxfb.c                  |  192 +++++++++++++++++++++-----------
 drivers/video/imxfb.h                  |    3 +-
 3 files changed, 174 insertions(+), 66 deletions(-)

diff --git a/arch/arm/mach-imx/include/mach/imxfb.h b/arch/arm/mach-imx/include/mach/imxfb.h
index 3ed9ec8..3f24f74 100644
--- a/arch/arm/mach-imx/include/mach/imxfb.h
+++ b/arch/arm/mach-imx/include/mach/imxfb.h
@@ -1,6 +1,51 @@
 /*
  * This structure describes the machine which we are running on.
  */
+
+#define PCR_TFT		(1 << 31)
+#define PCR_COLOR	(1 << 30)
+#define PCR_PBSIZ_1	(0 << 28)
+#define PCR_PBSIZ_2	(1 << 28)
+#define PCR_PBSIZ_4	(2 << 28)
+#define PCR_PBSIZ_8	(3 << 28)
+#define PCR_BPIX_1	(0 << 25)
+#define PCR_BPIX_2	(1 << 25)
+#define PCR_BPIX_4	(2 << 25)
+#define PCR_BPIX_8	(3 << 25)
+#define PCR_BPIX_12	(4 << 25)
+#define PCR_BPIX_16	(4 << 25)
+#define PCR_PIXPOL	(1 << 24)
+#define PCR_FLMPOL	(1 << 23)
+#define PCR_LPPOL	(1 << 22)
+#define PCR_CLKPOL	(1 << 21)
+#define PCR_OEPOL	(1 << 20)
+#define PCR_SCLKIDLE	(1 << 19)
+#define PCR_END_SEL	(1 << 18)
+#define PCR_END_BYTE_SWAP (1 << 17)
+#define PCR_REV_VS	(1 << 16)
+#define PCR_ACD_SEL	(1 << 15)
+#define PCR_ACD(x)	(((x) & 0x7f) << 8)
+#define PCR_SCLK_SEL	(1 << 7)
+#define PCR_SHARP	(1 << 6)
+#define PCR_PCD(x)	((x) & 0x3f)
+
+#define PWMR_CLS(x)	(((x) & 0x1ff) << 16)
+#define PWMR_LDMSK	(1 << 15)
+#define PWMR_SCR1	(1 << 10)
+#define PWMR_SCR0	(1 << 9)
+#define PWMR_CC_EN	(1 << 8)
+#define PWMR_PW(x)	((x) & 0xff)
+
+#define LSCR1_PS_RISE_DELAY(x)    (((x) & 0x7f) << 26)
+#define LSCR1_CLS_RISE_DELAY(x)   (((x) & 0x3f) << 16)
+#define LSCR1_REV_TOGGLE_DELAY(x) (((x) & 0xf) << 8)
+#define LSCR1_GRAY2(x)            (((x) & 0xf) << 4)
+#define LSCR1_GRAY1(x)            (((x) & 0xf))
+
+#define DMACR_BURST	(1 << 31)
+#define DMACR_HM(x)	(((x) & 0xf) << 16)
+#define DMACR_TM(x)	((x) & 0xf)
+
 struct imxfb_mach_info {
 	u_long		pixclock;
 
diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index bfc537f..af48044 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -16,7 +16,6 @@
  *	linux-arm-kernel@lists.arm.linux.org.uk
  */
 
-//#define DEBUG 1
 
 #include <linux/module.h>
 #include <linux/kernel.h>
@@ -33,10 +32,8 @@
 #include <linux/platform_device.h>
 #include <linux/dma-mapping.h>
 
-#include <mach/hardware.h>
 #include <asm/io.h>
 #include <mach/imxfb.h>
-#include <mach/imx-regs.h>
 
 /*
  * Complain if VAR is out of range.
@@ -45,6 +42,73 @@
 
 #include "imxfb.h"
 
+#define DRIVER_NAME "imx-fb"
+
+#define LCDC_SSA	0x00
+
+#define LCDC_SIZE	0x04
+#define SIZE_XMAX(x)	((((x) >> 4) & 0x3f) << 20)
+#define SIZE_YMAX(y)	((y) & 0x1ff)
+
+#define LCDC_VPW	0x08
+#define VPW_VPW(x)	((x) & 0x3ff)
+
+#define LCDC_CPOS	0x0C
+#define CPOS_CC1	(1<<31)
+#define CPOS_CC0	(1<<30)
+#define CPOS_OP		(1<<28)
+#define CPOS_CXP(x)	(((x) & 3ff) << 16)
+#define CPOS_CYP(y)	((y) & 0x1ff)
+
+#define LCDC_LCWHB	0x10
+#define LCWHB_BK_EN	(1<<31)
+#define LCWHB_CW(w)	(((w) & 0x1f) << 24)
+#define LCWHB_CH(h)	(((h) & 0x1f) << 16)
+#define LCWHB_BD(x)	((x) & 0xff)
+
+#define LCDC_LCHCC	0x14
+#define LCHCC_CUR_COL_R(r) (((r) & 0x1f) << 11)
+#define LCHCC_CUR_COL_G(g) (((g) & 0x3f) << 5)
+#define LCHCC_CUR_COL_B(b) ((b) & 0x1f)
+
+#define LCDC_PCR	0x18
+
+#define LCDC_HCR	0x1C
+#define HCR_H_WIDTH(x)	(((x) & 0x3f) << 26)
+#define HCR_H_WAIT_1(x)	(((x) & 0xff) << 8)
+#define HCR_H_WAIT_2(x)	((x) & 0xff)
+
+#define LCDC_VCR	0x20
+#define VCR_V_WIDTH(x)	(((x) & 0x3f) << 26)
+#define VCR_V_WAIT_1(x)	(((x) & 0xff) << 8)
+#define VCR_V_WAIT_2(x)	((x) & 0xff)
+
+#define LCDC_POS	0x24
+#define POS_POS(x)	((x) & 1f)
+
+#define LCDC_LSCR1	0x28
+/* bit fields in imxfb.h */
+
+#define LCDC_PWMR	0x2C
+/* bit fields in imxfb.h */
+
+#define LCDC_DMACR	0x30
+/* bit fields in imxfb.h */
+
+#define LCDC_RMCR	0x34
+#define RMCR_LCDC_EN	(1<<1)
+#define RMCR_SELF_REF	(1<<0)
+
+#define LCDC_LCDICR	0x38
+#define LCDICR_INT_SYN	(1<<2)
+#define LCDICR_INT_CON	(1)
+
+#define LCDC_LCDISR	0x40
+#define LCDISR_UDR_ERR	(1<<3)
+#define LCDISR_ERR_RES	(1<<2)
+#define LCDISR_EOF	(1<<1)
+#define LCDISR_BOF	(1<<0)
+
 static struct imxfb_rgb def_rgb_16 = {
 	.red	= { .offset = 8,  .length = 4, },
 	.green	= { .offset = 4,  .length = 4, },
@@ -68,7 +132,6 @@ static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)
 	return chan << bf->offset;
 }
 
-#define LCDC_PALETTE(x) __REG2(IMX_LCDC_BASE+0x800, (x)<<2)
 static int
 imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
 		       u_int trans, struct fb_info *info)
@@ -82,7 +145,7 @@ imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
 		      (CNVT_TOHW(green,4) << 4) |
 		      CNVT_TOHW(blue,  4);
 
-		LCDC_PALETTE(regno) = val;
+		writel(val, fbi->regs + 0x800 + (regno << 2));
 		ret = 0;
 	}
 	return ret;
@@ -236,18 +299,23 @@ static void imxfb_enable_controller(struct imxfb_info *fbi)
 	pr_debug("Enabling LCD controller\n");
 
 	/* initialize LCDC */
-	LCDC_RMCR &= ~RMCR_LCDC_EN;		/* just to be safe... */
+	writel(readl(fbi->regs + LCDC_RMCR) & ~RMCR_LCDC_EN,
+		fbi->regs + LCDC_RMCR);	/* just to be safe... */
+
+	writel(fbi->screen_dma, fbi->regs + LCDC_SSA);
 
-	LCDC_SSA	= fbi->screen_dma;
 	/* physical screen start address	    */
-	LCDC_VPW	= VPW_VPW(fbi->max_xres * fbi->max_bpp / 8 / 4);
+	writel(VPW_VPW(fbi->max_xres * fbi->max_bpp / 8 / 4),
+		fbi->regs + LCDC_VPW);
 
-	LCDC_POS	= 0x00000000;   /* panning offset 0 (0 pixel offset)        */
+	/* panning offset 0 (0 pixel offset)        */
+	writel(0x00000000, fbi->regs + LCDC_POS);
 
 	/* disable hardware cursor */
-	LCDC_CPOS	&= ~(CPOS_CC0 | CPOS_CC1);
+	writel(readl(fbi->regs + LCDC_CPOS) & ~(CPOS_CC0 | CPOS_CC1),
+		fbi->regs + LCDC_CPOS);
 
-	LCDC_RMCR = RMCR_LCDC_EN;
+	writel(RMCR_LCDC_EN, fbi->regs + LCDC_RMCR);
 
 	if(fbi->backlight_power)
 		fbi->backlight_power(1);
@@ -264,7 +332,7 @@ static void imxfb_disable_controller(struct imxfb_info *fbi)
 	if(fbi->lcd_power)
 		fbi->lcd_power(0);
 
-	LCDC_RMCR = 0;
+	writel(0, fbi->regs + LCDC_RMCR);
 }
 
 static int imxfb_blank(int blank, struct fb_info *info)
@@ -341,19 +409,22 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
 			info->fix.id, var->lower_margin);
 #endif
 
-	LCDC_HCR	= HCR_H_WIDTH(var->hsync_len) |
-	                  HCR_H_WAIT_1(var->left_margin) |
-			  HCR_H_WAIT_2(var->right_margin);
+	writel(HCR_H_WIDTH(var->hsync_len) |
+		HCR_H_WAIT_1(var->left_margin) |
+		HCR_H_WAIT_2(var->right_margin),
+		fbi->regs + LCDC_HCR);
 
-	LCDC_VCR	= VCR_V_WIDTH(var->vsync_len) |
-	                  VCR_V_WAIT_1(var->upper_margin) |
-			  VCR_V_WAIT_2(var->lower_margin);
+	writel(VCR_V_WIDTH(var->vsync_len) |
+		VCR_V_WAIT_1(var->upper_margin) |
+		VCR_V_WAIT_2(var->lower_margin),
+		fbi->regs + LCDC_VCR);
 
-	LCDC_SIZE	= SIZE_XMAX(var->xres) | SIZE_YMAX(var->yres);
-	LCDC_PCR	= fbi->pcr;
-	LCDC_PWMR	= fbi->pwmr;
-	LCDC_LSCR1	= fbi->lscr1;
-	LCDC_DMACR	= fbi->dmacr;
+	writel(SIZE_XMAX(var->xres) | SIZE_YMAX(var->yres),
+			fbi->regs + LCDC_SIZE);
+	writel(fbi->pcr, fbi->regs + LCDC_PCR);
+	writel(fbi->pwmr, fbi->regs + LCDC_PWMR);
+	writel(fbi->lscr1, fbi->regs + LCDC_LSCR1);
+	writel(fbi->dmacr, fbi->regs + LCDC_DMACR);
 
 	return 0;
 }
@@ -385,10 +456,10 @@ static int imxfb_resume(struct platform_device *dev)
 #define imxfb_resume	NULL
 #endif
 
-static int __init imxfb_init_fbinfo(struct device *dev)
+static int __init imxfb_init_fbinfo(struct platform_device *pdev)
 {
-	struct imxfb_mach_info *inf = dev->platform_data;
-	struct fb_info *info = dev_get_drvdata(dev);
+	struct imxfb_mach_info *inf = pdev->dev.platform_data;
+	struct fb_info *info = dev_get_drvdata(&pdev->dev);
 	struct imxfb_info *fbi = info->par;
 
 	pr_debug("%s\n",__func__);
@@ -398,7 +469,6 @@ static int __init imxfb_init_fbinfo(struct device *dev)
 		return -ENOMEM;
 
 	memset(fbi, 0, sizeof(struct imxfb_info));
-	fbi->dev = dev;
 
 	strlcpy(info->fix.id, IMX_NAME, sizeof(info->fix.id));
 
@@ -454,31 +524,6 @@ static int __init imxfb_init_fbinfo(struct device *dev)
 	return 0;
 }
 
-/*
- *      Allocates the DRAM memory for the frame buffer.  This buffer is
- *	remapped into a non-cached, non-buffered, memory region to
- *      allow pixel writes to occur without flushing the cache.
- *      Once this area is remapped, all virtual memory access to the
- *      video memory should occur at the new region.
- */
-static int __init imxfb_map_video_memory(struct fb_info *info)
-{
-	struct imxfb_info *fbi = info->par;
-
-	fbi->map_size = PAGE_ALIGN(info->fix.smem_len);
-	fbi->map_cpu = dma_alloc_writecombine(fbi->dev, fbi->map_size,
-					&fbi->map_dma,GFP_KERNEL);
-
-	if (fbi->map_cpu) {
-		info->screen_base = fbi->map_cpu;
-		fbi->screen_cpu = fbi->map_cpu;
-		fbi->screen_dma = fbi->map_dma;
-		info->fix.smem_start = fbi->screen_dma;
-	}
-
-	return fbi->map_cpu ? 0 : -ENOMEM;
-}
-
 static int __init imxfb_probe(struct platform_device *pdev)
 {
 	struct imxfb_info *fbi;
@@ -507,23 +552,38 @@ static int __init imxfb_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, info);
 
-	ret = imxfb_init_fbinfo(&pdev->dev);
+	ret = imxfb_init_fbinfo(pdev);
 	if( ret < 0 )
 		goto failed_init;
 
-	res = request_mem_region(res->start, res->end - res->start + 1, "IMXFB");
+	res = request_mem_region(res->start, resource_size(res),
+				DRIVER_NAME);
 	if (!res) {
 		ret = -EBUSY;
-		goto failed_regs;
+		goto failed_req;
+	}
+
+	fbi->regs = ioremap(res->start, resource_size(res));
+	if (fbi->regs == NULL) {
+		printk(KERN_ERR"Cannot map frame buffer registers\n");
+		goto failed_ioremap;
 	}
 
 	if (!inf->fixed_screen_cpu) {
-		ret = imxfb_map_video_memory(info);
-		if (ret) {
+		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);
+
+		if (!fbi->map_cpu) {
 			dev_err(&pdev->dev, "Failed to allocate video RAM: %d\n", ret);
 			ret = -ENOMEM;
 			goto failed_map;
 		}
+
+		info->screen_base = fbi->map_cpu;
+		fbi->screen_cpu = fbi->map_cpu;
+		fbi->screen_dma = fbi->map_dma;
+		info->fix.smem_start = fbi->screen_dma;
 	} else {
 		/* Fixed framebuffer mapping enables location of the screen in eSRAM */
 		fbi->map_cpu = inf->fixed_screen_cpu;
@@ -560,18 +620,20 @@ failed_register:
 failed_cmap:
 	if (!inf->fixed_screen_cpu)
 		dma_free_writecombine(&pdev->dev,fbi->map_size,fbi->map_cpu,
-		           fbi->map_dma);
+			fbi->map_dma);
 failed_map:
-	kfree(info->pseudo_palette);
-failed_regs:
+	iounmap(fbi->regs);
+failed_ioremap:
 	release_mem_region(res->start, res->end - res->start);
+failed_req:
+	kfree(info->pseudo_palette);
 failed_init:
 	platform_set_drvdata(pdev, NULL);
 	framebuffer_release(info);
 	return ret;
 }
 
-static int imxfb_remove(struct platform_device *pdev)
+static int __devexit imxfb_remove(struct platform_device *pdev)
 {
 	struct fb_info *info = platform_get_drvdata(pdev);
 	struct imxfb_info *fbi = info->par;
@@ -587,6 +649,7 @@ static int imxfb_remove(struct platform_device *pdev)
 	kfree(info->pseudo_palette);
 	framebuffer_release(info);
 
+	iounmap(fbi->regs);
 	release_mem_region(res->start, res->end - res->start + 1);
 	platform_set_drvdata(pdev, NULL);
 
@@ -601,19 +664,18 @@ void  imxfb_shutdown(struct platform_device * dev)
 }
 
 static struct platform_driver imxfb_driver = {
-	.probe		= imxfb_probe,
 	.suspend	= imxfb_suspend,
 	.resume		= imxfb_resume,
-	.remove		= imxfb_remove,
+	.remove		= __devexit_p(imxfb_remove),
 	.shutdown	= imxfb_shutdown,
 	.driver		= {
-		.name	= "imx-fb",
+		.name	= DRIVER_NAME,
 	},
 };
 
 int __init imxfb_init(void)
 {
-	return platform_driver_register(&imxfb_driver);
+	return platform_driver_probe(&imxfb_driver, imxfb_probe);
 }
 
 static void __exit imxfb_cleanup(void)
diff --git a/drivers/video/imxfb.h b/drivers/video/imxfb.h
index e837a8b..baa86a0 100644
--- a/drivers/video/imxfb.h
+++ b/drivers/video/imxfb.h
@@ -29,7 +29,8 @@ struct imxfb_rgb {
 #define NR_RGB	2
 
 struct imxfb_info {
-	struct device		*dev;
+	void __iomem		*regs;
+
 	struct imxfb_rgb	*rgb[NR_RGB];
 
 	u_int			max_bpp;
-- 
1.5.6.3


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/