[U-Boot] [PATCH 0/1] i.MX2: Support splash screen

[U-Boot] [PATCH 0/1] i.MX2: Support splash screen

[Timo Ketola]

I based this on the work here:

http://www.imxdev.org/wiki/index.php?title=I.MX25_PDK_U-boot_SplashScreen

[U-Boot] [PATCH] i.MX2: Support splash screen

[Timo Ketola]

Signed-off-by: Timo Ketola <timo@exertus.fi>
---
 arch/arm/include/asm/arch-mx25/imx-regs.h |   29 +++++++++
 drivers/video/Makefile                    |    1 +
 drivers/video/mx2fb.c                     |   92 +++++++++++++++++++++++++++++
 include/lcd.h                             |   21 ++++++-
 include/mx2fb.h                           |   39 ++++++++++++
 5 files changed, 181 insertions(+), 1 deletions(-)
 create mode 100644 drivers/video/mx2fb.c
 create mode 100644 include/mx2fb.h

diff --git a/arch/arm/include/asm/arch-mx25/imx-regs.h b/arch/arm/include/asm/arch-mx25/imx-regs.h
index 7f9449b..af5b42e 100644
--- a/arch/arm/include/asm/arch-mx25/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx25/imx-regs.h
@@ -167,6 +167,35 @@ struct aips_regs {
 	u32 mpr_8_15;
 };
 
+struct lcdc_regs {
+	u32 lssar;
+	u32 lsr;
+	u32 lvpwr;
+	u32 lcpr;
+	u32 lcwhb;
+	u32 lccmr;
+	u32 lpcr;
+	u32 lhcr;
+	u32 lvcr;
+	u32 lpor;
+	u32 lscr;
+	u32 lpccr;
+	u32 ldcr;
+	u32 lrmcr;
+	u32 licr;
+	u32 lier;
+	u32 lisr;
+	u32 pad0[3];
+	u32 lgwsar;
+	u32 lgwsr;
+	u32 lgwvpwr;
+	u32 lgwpor;
+	u32 lgwpr;
+	u32 pad1[0x200 - 25];
+	u32 bglut[0x100];
+	u32 gwlut[0x100];
+};
+
 #endif
 
 /* AIPS 1 */
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 6252f6a..e047471 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -36,6 +36,7 @@ COBJS-$(CONFIG_VIDEO_CT69000) += ct69000.o videomodes.o
 COBJS-$(CONFIG_VIDEO_DA8XX) += da8xx-fb.o videomodes.o
 COBJS-$(CONFIG_VIDEO_MB862xx) += mb862xx.o videomodes.o
 COBJS-$(CONFIG_VIDEO_MB86R0xGDC) += mb86r0xgdc.o videomodes.o
+COBJS-$(CONFIG_VIDEO_MX2) += mx2fb.o
 COBJS-$(CONFIG_VIDEO_MX3) += mx3fb.o videomodes.o
 COBJS-$(CONFIG_VIDEO_MX5) += mxc_ipuv3_fb.o ipu_common.o ipu_disp.o
 COBJS-$(CONFIG_VIDEO_OMAP3) += omap3_dss.o videomodes.o
diff --git a/drivers/video/mx2fb.c b/drivers/video/mx2fb.c
new file mode 100644
index 0000000..9ee4a3e
--- /dev/null
+++ b/drivers/video/mx2fb.c
@@ -0,0 +1,92 @@
+#include <common.h>
+#include <lcd.h>
+#include <mx2fb.h>
+#include <asm/arch/imx-regs.h>
+#include <asm/io.h>
+#include <asm/errno.h>
+
+#if !defined(LCD_BPP) || LCD_BPP != LCD_COLOR16
+
+#error Only 16bpp is supported
+
+#endif
+
+DECLARE_GLOBAL_DATA_PTR;
+
+void *lcd_base;			/* Start of framebuffer memory	*/
+void *lcd_console_address;	/* Start of console buffer	*/
+
+int lcd_line_length;
+int lcd_color_fg;
+int lcd_color_bg;
+
+short console_col;
+short console_row;
+
+
+void lcd_initcolregs(void)
+{
+}
+
+void lcd_setcolreg(ushort regno, ushort red, ushort green, ushort blue)
+{
+}
+
+void lcd_enable(void)
+{
+}
+
+void lcd_disable(void)
+{
+}
+
+void lcd_panel_disable(void)
+{
+}
+
+void lcd_ctrl_init(void *lcdbase)
+{
+	u32 ccm_ipg_cg, pcr;
+	struct lcdc_regs *lcdc = (struct lcdc_regs *)IMX_LCDC_BASE;
+	struct ccm_regs *ccm = (struct ccm_regs *)IMX_CCM_BASE;
+
+	writel(gd->fb_base, &lcdc->lssar);
+	writel(panel_info.vl_col >> 4 << 20 | panel_info.vl_row, &lcdc->lsr);
+	writel(panel_info.vl_col / 2, &lcdc->lvpwr);
+	if (panel_info.vl_bpix != 4)
+		printf("Unsupported color depth (%d), only 16bpp supported\n",
+				NBITS(panel_info.vl_bpix));
+	pcr = LCDC_LPCR | 5 << 25;
+
+	if (panel_info.vl_sync & FB_SYNC_CLK_LAT_FALL)
+		pcr |= 0x00200000;
+	if (panel_info.vl_sync & FB_SYNC_DATA_INVERT)
+		pcr |= 0x01000000;
+	if (panel_info.vl_sync & FB_SYNC_SHARP_MODE)
+		pcr |= 0x00000040;
+	if (panel_info.vl_sync & FB_SYNC_OE_LOW_ACT)
+		pcr |= 0x00100000;
+
+	pcr |= LCDC_LPCR_PCD;
+
+	writel(pcr, &lcdc->lpcr);
+	writel((panel_info.vl_hsync - 1) << 26 |
+				(panel_info.vl_right_margin - 1) << 8 |
+				(panel_info.vl_left_margin - 3),
+			&lcdc->lhcr);
+	writel(panel_info.vl_vsync << 26 |
+				panel_info.vl_lower_margin << 8 |
+				panel_info.vl_upper_margin,
+			&lcdc->lvcr);
+	writel(LCDC_LSCR, &lcdc->lscr);
+	writel(LCDC_LRMCR, &lcdc->lrmcr);
+	writel(LCDC_LDCR, &lcdc->ldcr);
+	writel(LCDC_LPCCR, &lcdc->lpccr);
+
+	/* Off and on clock gating
+	   FIXME: Why *off* and on; What side effects does it have? */
+	ccm_ipg_cg = readl(&ccm->cgr1);
+
+	writel(ccm_ipg_cg & 0xDFFFFFFF, &ccm->cgr1);
+	writel(ccm_ipg_cg | 0x20000000, &ccm->cgr1);
+}
diff --git a/include/lcd.h b/include/lcd.h
index d95feeb..a5eeb53 100644
--- a/include/lcd.h
+++ b/include/lcd.h
@@ -183,6 +183,25 @@ typedef struct vidinfo {
 	u_long	mmio;		/* Memory mapped registers */
 } vidinfo_t;
 
+#elif defined(CONFIG_VIDEO_MX2)
+
+struct vidinfo {
+	ushort vl_row;		/* resolution in x */
+	ushort vl_col;		/* resolution in y */
+	u_long vl_pixclock;	/* pixel clock in picoseconds */
+	u_long vl_left_margin;	/* Horizontal back porch */
+	u_long vl_right_margin; /* Horizontal front porch */
+	u_long vl_upper_margin; /* Vertical back porch */
+	u_long vl_lower_margin; /* Vertical front porch */
+	u_long vl_hsync;	/* Horizontal sync pulse length */
+	u_long vl_vsync;	/* Vertical sync pulse length */
+	u_long vl_sync;		/* Polarity on data enable */
+	u_long vl_mode;		/* Video Mode */
+	u_long vl_flag;
+	u_char vl_bpix;
+	ushort *cmap;
+};
+
 #else
 
 typedef struct vidinfo {
@@ -198,7 +217,7 @@ typedef struct vidinfo {
 
 #endif /* CONFIG_MPC823, CONFIG_CPU_PXA25X, CONFIG_MCC200, CONFIG_ATMEL_LCD */
 
-extern vidinfo_t panel_info;
+extern struct vidinfo panel_info;
 
 /* Video functions */
 
diff --git a/include/mx2fb.h b/include/mx2fb.h
new file mode 100644
index 0000000..1f16a61
--- /dev/null
+++ b/include/mx2fb.h
@@ -0,0 +1,39 @@
+/*
+ * Copyright 2004-2009 Freescale Semiconductor, Inc. All Rights Reserved.
+ */
+
+/*
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#ifndef __MX2FB_H__
+#define __MX2FB_H__
+
+
+/* LCDC register settings */
+
+#define LCDC_LSCR 0x00120300
+
+#define LCDC_LRMCR 0x00000000
+
+#define LCDC_LDCR 0x00020010
+
+#define LCDC_LPCCR 0x00a9037f
+
+#define LCDC_LPCR 0xFA008B80
+
+#define LCDC_LPCR_PCD 0x4
+
+#define FB_SYNC_OE_LOW_ACT	0x80000000
+#define FB_SYNC_CLK_LAT_FALL	0x40000000
+#define FB_SYNC_DATA_INVERT	0x20000000
+#define FB_SYNC_CLK_IDLE_EN	0x10000000
+#define FB_SYNC_SHARP_MODE	0x08000000
+#define FB_SYNC_SWAP_RGB	0x04000000
+
+#endif
-- 
1.7.5.4

[U-Boot] [PATCH] i.MX2: Support splash screen

[Wolfgang Denk]

Dear "Timo Ketola",

In message <1334739261-7812-2-git-send-email-timo@exertus.fi> you wrote:
> Signed-off-by: Timo Ketola <timo@exertus.fi>
> ---
>  arch/arm/include/asm/arch-mx25/imx-regs.h |   29 +++++++++
>  drivers/video/Makefile                    |    1 +
>  drivers/video/mx2fb.c                     |   92 +++++++++++++++++++++++++++++
>  include/lcd.h                             |   21 ++++++-
>  include/mx2fb.h                           |   39 ++++++++++++
>  5 files changed, 181 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/video/mx2fb.c
>  create mode 100644 include/mx2fb.h

NAK, because proper attribution is missing.  See
http://www.denx.de/wiki/view/U-Boot/Patches#Attributing_Code_Copyrights_Sign


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"The greatest warriors are the ones who fight for peace."
- Holly Near

[U-Boot] [PATCH] i.MX2: Support splash screen

[Stefano Babic]

On 18/04/2012 10:54, Timo Ketola wrote:
> Signed-off-by: Timo Ketola <timo@exertus.fi>
> ---
>  arch/arm/include/asm/arch-mx25/imx-regs.h |   29 +++++++++
>  drivers/video/Makefile                    |    1 +
>  drivers/video/mx2fb.c                     |   92 +++++++++++++++++++++++++++++
>  include/lcd.h                             |   21 ++++++-
>  include/mx2fb.h                           |   39 ++++++++++++
>  5 files changed, 181 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/video/mx2fb.c
>  create mode 100644 include/mx2fb.h

Hi Timo,

I forward your patch to Anatolji. He is the video maintainer.

> 
> diff --git a/arch/arm/include/asm/arch-mx25/imx-regs.h b/arch/arm/include/asm/arch-mx25/imx-regs.h
> index 7f9449b..af5b42e 100644
> --- a/arch/arm/include/asm/arch-mx25/imx-regs.h
> +++ b/arch/arm/include/asm/arch-mx25/imx-regs.h
> @@ -167,6 +167,35 @@ struct aips_regs {
>  	u32 mpr_8_15;
>  };
>  
> +struct lcdc_regs {
> +	u32 lssar;
> +	u32 lsr;
> +	u32 lvpwr;
> +	u32 lcpr;
> +	u32 lcwhb;
> +	u32 lccmr;
> +	u32 lpcr;
> +	u32 lhcr;
> +	u32 lvcr;
> +	u32 lpor;
> +	u32 lscr;
> +	u32 lpccr;
> +	u32 ldcr;
> +	u32 lrmcr;
> +	u32 licr;
> +	u32 lier;
> +	u32 lisr;
> +	u32 pad0[3];
> +	u32 lgwsar;
> +	u32 lgwsr;
> +	u32 lgwvpwr;
> +	u32 lgwpor;
> +	u32 lgwpr;
> +	u32 pad1[0x200 - 25];
> +	u32 bglut[0x100];
> +	u32 gwlut[0x100];
> +};
> +
>  #endif
>  
>  /* AIPS 1 */
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index 6252f6a..e047471 100644
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -36,6 +36,7 @@ COBJS-$(CONFIG_VIDEO_CT69000) += ct69000.o videomodes.o
>  COBJS-$(CONFIG_VIDEO_DA8XX) += da8xx-fb.o videomodes.o
>  COBJS-$(CONFIG_VIDEO_MB862xx) += mb862xx.o videomodes.o
>  COBJS-$(CONFIG_VIDEO_MB86R0xGDC) += mb86r0xgdc.o videomodes.o
> +COBJS-$(CONFIG_VIDEO_MX2) += mx2fb.o
>  COBJS-$(CONFIG_VIDEO_MX3) += mx3fb.o videomodes.o
>  COBJS-$(CONFIG_VIDEO_MX5) += mxc_ipuv3_fb.o ipu_common.o ipu_disp.o
>  COBJS-$(CONFIG_VIDEO_OMAP3) += omap3_dss.o videomodes.o
> diff --git a/drivers/video/mx2fb.c b/drivers/video/mx2fb.c
> new file mode 100644
> index 0000000..9ee4a3e


IMHO it is better if you use the video API instead of the old LCD
interface, that means using CONFIG_VIDEO. You have then to implement a
video_hw_init() function for your initialisation. I think there are
advantages doing that, and recently some drivers moved to this API, for
example the driver for i.MX3 (mx3fb.c).

>  
> diff --git a/include/mx2fb.h b/include/mx2fb.h
> new file mode 100644
> index 0000000..1f16a61
> --- /dev/null
> +++ b/include/mx2fb.h
> @@ -0,0 +1,39 @@
> +/*
> + * Copyright 2004-2009 Freescale Semiconductor, Inc. All Rights Reserved.
> + */
> +
> +/*
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */

The header is not correct, I think - it does not set explicitely under
which license version is published this code, only where it is possible
to get a GPLv2 or later. Please substitute this header with a full
explained license header, as you can see generally in u-boot files.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH] i.MX25: Prepare for LCD driver in board adaptation

[Timo Ketola]

Dear Stefano, Anatolij, Scott,

On 18.04.2012 13:47, Stefano Babic wrote:
> On 18/04/2012 10:54, Timo Ketola wrote:
>> Signed-off-by: Timo Ketola <timo@exertus.fi>
...
>> diff --git a/drivers/video/mx2fb.c b/drivers/video/mx2fb.c
>> new file mode 100644
>> index 0000000..9ee4a3e
> 
> IMHO it is better if you use the video API instead of the old LCD
> interface, that means using CONFIG_VIDEO. You have then to implement a
> video_hw_init() function for your initialisation. I think there are
> advantages doing that, and recently some drivers moved to this API, for
> example the driver for i.MX3 (mx3fb.c).

I agree on that but unfortunately I'm short on time. What if I
implement the driver in my board folder instead? I suppose there is not
too much interest to get generic LCD support for i.MX2 because it is
not yet there and architecture is a little oldish.

There should still be register definitions in the imx-regs.h like I
propose in this patch. Or should I keep them too in my board folder?

Timo Ketola (1):
  i.MX25: lcdc: Add register definitions

 arch/arm/include/asm/arch-mx25/imx-regs.h |   32 +++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

-- 
1.7.5.4

[U-Boot] [PATCH] i.MX25: lcdc: Add register definitions

[Timo Ketola]

Signed-off-by: Timo Ketola <timo@exertus.fi>
---
 arch/arm/include/asm/arch-mx25/imx-regs.h |   32 +++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/arch-mx25/imx-regs.h b/arch/arm/include/asm/arch-mx25/imx-regs.h
index cf925d7..751a518 100644
--- a/arch/arm/include/asm/arch-mx25/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx25/imx-regs.h
@@ -170,6 +170,38 @@ struct aips_regs {
 	u32 mpr_8_15;
 };
 
+struct lcdc_regs {
+	u32 lssar;
+	u32 lsr;
+	u32 lvpwr;
+	u32 lcpr;
+	u32 lcwhb;
+	u32 lccmr;
+	u32 lpcr;
+	u32 lhcr;
+	u32 lvcr;
+	u32 lpor;
+	u32 lscr;
+	u32 lpccr;
+	u32 ldcr;
+	u32 lrmcr;
+	u32 licr;
+	u32 lier;
+	u32 lisr;
+	u32 pad0[3];
+	u32 lgwsar;
+	u32 lgwsr;
+	u32 lgwvpwr;
+	u32 lgwpor;
+	u32 lgwpr;
+
+	/* bglut starts at 0x800 or at word 0x200 and at this point we have
+	 * defined 25 registers */
+	u32 pad1[0x200 - 25];
+	u32 bglut[0x100];
+	u32 gwlut[0x100];
+};
+
 #endif
 
 /* AIPS 1 */
-- 
1.7.5.4

[U-Boot] [PATCH] i.MX25: lcdc: Add register definitions

[Anatolij Gustschin]

On Fri, 20 Apr 2012 14:01:49 +0300
"Timo Ketola" <timo@exertus.fi> wrote:

> Signed-off-by: Timo Ketola <timo@exertus.fi>
> ---
>  arch/arm/include/asm/arch-mx25/imx-regs.h |   32 +++++++++++++++++++++++++++++
>  1 files changed, 32 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-mx25/imx-regs.h b/arch/arm/include/asm/arch-mx25/imx-regs.h
> index cf925d7..751a518 100644
> --- a/arch/arm/include/asm/arch-mx25/imx-regs.h
> +++ b/arch/arm/include/asm/arch-mx25/imx-regs.h
> @@ -170,6 +170,38 @@ struct aips_regs {
...
> +
> +	/* bglut starts at 0x800 or at word 0x200 and at this point we have
> +	 * defined 25 registers */

Please fix style here.

Thanks,
Anatolij

[U-Boot] [PATCH] i.MX25: Prepare for LCD driver in board adaptation

[Anatolij Gustschin]

Hi Timo,

sorry for my late reply.

On Fri, 20 Apr 2012 14:01:48 +0300
"Timo Ketola" <timo@exertus.fi> wrote:

> Dear Stefano, Anatolij, Scott,
> 
> On 18.04.2012 13:47, Stefano Babic wrote:
> > On 18/04/2012 10:54, Timo Ketola wrote:
> >> Signed-off-by: Timo Ketola <timo@exertus.fi>
> ...
> >> diff --git a/drivers/video/mx2fb.c b/drivers/video/mx2fb.c
> >> new file mode 100644
> >> index 0000000..9ee4a3e
> > 
> > IMHO it is better if you use the video API instead of the old LCD
> > interface, that means using CONFIG_VIDEO. You have then to implement a
> > video_hw_init() function for your initialisation. I think there are
> > advantages doing that, and recently some drivers moved to this API, for
> > example the driver for i.MX3 (mx3fb.c).
> 
> I agree on that but unfortunately I'm short on time. What if I
> implement the driver in my board folder instead? I suppose there is not
> too much interest to get generic LCD support for i.MX2 because it is
> not yet there and architecture is a little oldish.

I would prefer not including the driver in the board code. The
disadvantage of using the video API is increased code size. If you
do not need the framebuffer console, then using the LCD interface
is okay.

> There should still be register definitions in the imx-regs.h like I
> propose in this patch. Or should I keep them too in my board folder?

No, better keep them in imx-regs.h.

If you are going to update and resubmit your mx2fb patch, then please
base your work on the u-boot-samsung tree [1] as there are another lcd
driver changes in this tree. Or wait until samsung tree will be merged
and then rebase your work on top of u-boot tree.

Thanks,
Anatolij

[1] http://git.denx.de/?p=u-boot/u-boot-samsung.git;a=summary

[U-Boot] [PATCH] i.MX25: Prepare for LCD driver in board adaptation

[Timo Ketola]

On 26.04.2012 12:57, Anatolij Gustschin wrote:
...
> On Fri, 20 Apr 2012 14:01:48 +0300 "Timo Ketola" <timo@exertus.fi>
> wrote:
...
>> What if I implement the driver in my board folder instead?
...
> I would prefer not including the driver in the board code. The 
> disadvantage of using the video API is increased code size. If you do
> not need the framebuffer console, then using the LCD interface is
> okay.

Then I might resubmit the original patch. Is there basically anything
else than the lack of proper attributions?

...
> If you are going to update and resubmit your mx2fb patch, then
> please base your work on the u-boot-samsung tree [1] as there are
> another lcd driver changes in this tree. Or wait until samsung tree
> will be merged and then rebase your work on top of u-boot tree.

OK

--

Timo

[U-Boot] [PATCH] i.MX25: Prepare for LCD driver in board adaptation

[Anatolij Gustschin]

On Thu, 26 Apr 2012 14:18:29 +0300
Timo Ketola <timo@exertus.fi> wrote:
...
> Then I might resubmit the original patch. Is there basically anything
> else than the lack of proper attributions?

Some minor coding style issues. I've sent my comments to both patches.

Thanks,
Anatolij

[U-Boot] [PATCH] i.MX2: Support splash screen

[Anatolij Gustschin]

On Wed, 18 Apr 2012 11:54:21 +0300
"Timo Ketola" <timo@exertus.fi> wrote:

> Signed-off-by: Timo Ketola <timo@exertus.fi>
> ---
>  arch/arm/include/asm/arch-mx25/imx-regs.h |   29 +++++++++
>  drivers/video/Makefile                    |    1 +
>  drivers/video/mx2fb.c                     |   92 +++++++++++++++++++++++++++++
>  include/lcd.h                             |   21 ++++++-
>  include/mx2fb.h                           |   39 ++++++++++++
>  5 files changed, 181 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/video/mx2fb.c
>  create mode 100644 include/mx2fb.h

...
> diff --git a/drivers/video/mx2fb.c b/drivers/video/mx2fb.c
> new file mode 100644
> index 0000000..9ee4a3e
> --- /dev/null
> +++ b/drivers/video/mx2fb.c
> @@ -0,0 +1,92 @@

Please add Copyright info here.

> +#include <common.h>
> +#include <lcd.h>
> +#include <mx2fb.h>
> +#include <asm/arch/imx-regs.h>
> +#include <asm/io.h>
> +#include <asm/errno.h>
> +
> +#if !defined(LCD_BPP) || LCD_BPP != LCD_COLOR16
> +
> +#error Only 16bpp is supported
> +
> +#endif

Drop empty lines around #error

> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +void *lcd_base;			/* Start of framebuffer memory	*/
> +void *lcd_console_address;	/* Start of console buffer	*/
> +
> +int lcd_line_length;
> +int lcd_color_fg;
> +int lcd_color_bg;
> +
> +short console_col;
> +short console_row;
> +
> +
> +void lcd_initcolregs(void)
> +{
> +}
> +
> +void lcd_setcolreg(ushort regno, ushort red, ushort green, ushort blue)
> +{
> +}
> +
> +void lcd_enable(void)
> +{
> +}
> +
> +void lcd_disable(void)
> +{
> +}
> +
> +void lcd_panel_disable(void)
> +{
> +}
> +
> +void lcd_ctrl_init(void *lcdbase)
> +{
> +	u32 ccm_ipg_cg, pcr;
> +	struct lcdc_regs *lcdc = (struct lcdc_regs *)IMX_LCDC_BASE;
> +	struct ccm_regs *ccm = (struct ccm_regs *)IMX_CCM_BASE;
> +
> +	writel(gd->fb_base, &lcdc->lssar);
> +	writel(panel_info.vl_col >> 4 << 20 | panel_info.vl_row, &lcdc->lsr);
> +	writel(panel_info.vl_col / 2, &lcdc->lvpwr);
> +	if (panel_info.vl_bpix != 4)
> +		printf("Unsupported color depth (%d), only 16bpp supported\n",
> +				NBITS(panel_info.vl_bpix));

Style issue: multi-line if statements should use braces
	if (cond) {
		multi-line
		statement
	}

> +	pcr = LCDC_LPCR | 5 << 25;
> +
> +	if (panel_info.vl_sync & FB_SYNC_CLK_LAT_FALL)
> +		pcr |= 0x00200000;
> +	if (panel_info.vl_sync & FB_SYNC_DATA_INVERT)
> +		pcr |= 0x01000000;
> +	if (panel_info.vl_sync & FB_SYNC_SHARP_MODE)
> +		pcr |= 0x00000040;
> +	if (panel_info.vl_sync & FB_SYNC_OE_LOW_ACT)
> +		pcr |= 0x00100000;
> +
> +	pcr |= LCDC_LPCR_PCD;
> +
> +	writel(pcr, &lcdc->lpcr);
> +	writel((panel_info.vl_hsync - 1) << 26 |
> +				(panel_info.vl_right_margin - 1) << 8 |
> +				(panel_info.vl_left_margin - 3),

Please remove two tabs here,
> +			&lcdc->lhcr);

and one tab here.

> +	writel(panel_info.vl_vsync << 26 |
> +				panel_info.vl_lower_margin << 8 |
> +				panel_info.vl_upper_margin,
> +			&lcdc->lvcr);

ditto.

> +	writel(LCDC_LSCR, &lcdc->lscr);
> +	writel(LCDC_LRMCR, &lcdc->lrmcr);
> +	writel(LCDC_LDCR, &lcdc->ldcr);
> +	writel(LCDC_LPCCR, &lcdc->lpccr);
> +
> +	/* Off and on clock gating
> +	   FIXME: Why *off* and on; What side effects does it have? */

Style for multi-line comments is
	/*
	 * multi-line
	 * comment
	 */

> +	ccm_ipg_cg = readl(&ccm->cgr1);
> +
> +	writel(ccm_ipg_cg & 0xDFFFFFFF, &ccm->cgr1);
> +	writel(ccm_ipg_cg | 0x20000000, &ccm->cgr1);
> +}

...
> diff --git a/include/mx2fb.h b/include/mx2fb.h
> new file mode 100644
> index 0000000..1f16a61
> --- /dev/null
> +++ b/include/mx2fb.h
> @@ -0,0 +1,39 @@
> +/*
> + * Copyright 2004-2009 Freescale Semiconductor, Inc. All Rights Reserved.
> + */
> +
> +/*
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */

Please consider Stefano's comment regarding copyright header.

> +
> +#ifndef __MX2FB_H__
> +#define __MX2FB_H__
> +
> +

please drop one empty line here.

> +/* LCDC register settings */
> +
> +#define LCDC_LSCR 0x00120300
> +
> +#define LCDC_LRMCR 0x00000000
> +
> +#define LCDC_LDCR 0x00020010
> +
> +#define LCDC_LPCCR 0x00a9037f
> +
> +#define LCDC_LPCR 0xFA008B80
> +
> +#define LCDC_LPCR_PCD 0x4

Please use the same style here as below, so remove
empty lines and indent numbers by tab.

> +
> +#define FB_SYNC_OE_LOW_ACT	0x80000000
> +#define FB_SYNC_CLK_LAT_FALL	0x40000000
> +#define FB_SYNC_DATA_INVERT	0x20000000
> +#define FB_SYNC_CLK_IDLE_EN	0x10000000
> +#define FB_SYNC_SHARP_MODE	0x08000000
> +#define FB_SYNC_SWAP_RGB	0x04000000
> +
> +#endif

Thanks,
Anatolij

[U-Boot] [PATCH] i.MX2: Support splash screen

[Timo Ketola]

Dear Anatolij,

Thanks for the review. Everything else is clear, but...

On 26.04.2012 14:51, Anatolij Gustschin wrote:
...
>> +++ b/drivers/video/mx2fb.c
>> @@ -0,0 +1,92 @@
> 
> Please add Copyright info here.
...
>> +++ b/include/mx2fb.h
...
> Please consider Stefano's comment regarding copyright header.

I'm a little lost here. The original work is this:

http://www.imxdev.org/wiki/images/d/d0/U-boot-splash.tar.gz

from here:

http://www.imxdev.org/wiki/index.php?title=I.MX25_PDK_U-boot_SplashScreen

There is also a u-boot.spec.in file in the package which says:

License         : GPL
Vendor          : Freescale
Packager        : MAD

Now, can I really just edit the mx2fb.h copyright notice to say "V2 or
later" and copy it into the mx2fb.h? If I do that, add the imxdev.org
links into the commit message and tersely say there how I changed the
source (left out 24bpp and splash_is_in_mmc support), would that be good
enough?

--

Timo

[U-Boot] [PATCH] i.MX2: Support splash screen

[Anatolij Gustschin]

Hi Timo,

On Thu, 26 Apr 2012 17:37:31 +0300
Timo Ketola <timo@exertus.fi> wrote:
...
> Thanks for the review. Everything else is clear, but...
> 
> On 26.04.2012 14:51, Anatolij Gustschin wrote:
> ...
> >> +++ b/drivers/video/mx2fb.c
> >> @@ -0,0 +1,92 @@
> > 
> > Please add Copyright info here.
> ...
> >> +++ b/include/mx2fb.h
> ...
> > Please consider Stefano's comment regarding copyright header.
> 
> I'm a little lost here. The original work is this:
> 
> http://www.imxdev.org/wiki/images/d/d0/U-boot-splash.tar.gz
> 
> from here:
> 
> http://www.imxdev.org/wiki/index.php?title=I.MX25_PDK_U-boot_SplashScreen
> 
> There is also a u-boot.spec.in file in the package which says:
> 
> License         : GPL
> Vendor          : Freescale
> Packager        : MAD
> 
> Now, can I really just edit the mx2fb.h copyright notice to say "V2 or
> later" and copy it into the mx2fb.h? If I do that, add the imxdev.org
> links into the commit message and tersely say there how I changed the
> source (left out 24bpp and splash_is_in_mmc support), would that be good
> enough?

Yes, I think so. The original header in mx2fb.h mentions GPLv2 or later.
You should add similar header to mx2fb.c also, adding a header like
/*
 * Copyright 2004-2009 Freescale Semiconductor, Inc.
 *
 * This file is released under the terms of GPL v2 and any later version.
 * See the file COPYING in the root directory of the source tree for details.
 */

should be enough.

Thanks,
Anatolij

[U-Boot] [PATCH 0/1] i.MX2: Support splash screen

[Wolfgang Denk]

Dear "Timo Ketola",

In message <1334739261-7812-1-git-send-email-timo@exertus.fi> you wrote:
> I based this on the work here:
> 
> http://www.imxdev.org/wiki/index.php?title=I.MX25_PDK_U-boot_SplashScreen

This information belongs (with greater detail) into the commit
message.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The day-to-day travails of the IBM programmer are so amusing to  most
of us who are fortunate enough never to have been one - like watching
Charlie Chaplin trying to cook a shoe.