[U-Boot] [PATCH] p1022ds: add video support

[U-Boot] [PATCH] p1022ds: add video support

[Timur Tabi]

Add support for the DIU controller.  If CONFIG_VIDEO is defined, then the
console will appear on a DVI monitor instead of the serial port.

Signed-off-by: Timur Tabi <timur@freescale.com>
---

This patch depends on:

[v3] fsl: refactor MPC8610 and MPC5121 DIU code to use existing bitmap and logo features

 board/freescale/p1022ds/Makefile |    2 +
 board/freescale/p1022ds/diu.c    |  172 ++++++++++++++++++++++++++++++++++++++
 include/configs/P1022DS.h        |    7 ++-
 3 files changed, 180 insertions(+), 1 deletions(-)
 create mode 100644 board/freescale/p1022ds/diu.c

diff --git a/board/freescale/p1022ds/Makefile b/board/freescale/p1022ds/Makefile
index 8ede2d6..678eb2a 100644
--- a/board/freescale/p1022ds/Makefile
+++ b/board/freescale/p1022ds/Makefile
@@ -16,6 +16,8 @@ COBJS-y	+= ddr.o
 COBJS-y	+= law.o
 COBJS-y	+= tlb.o
 
+COBJS-$(CONFIG_FSL_DIU_FB) += diu.o
+
 SRCS	:= $(SOBJS:.o=.S) $(COBJS-y:.o=.c)
 OBJS	:= $(addprefix $(obj),$(COBJS-y))
 SOBJS	:= $(addprefix $(obj),$(SOBJS))
diff --git a/board/freescale/p1022ds/diu.c b/board/freescale/p1022ds/diu.c
new file mode 100644
index 0000000..be6e9a8
--- /dev/null
+++ b/board/freescale/p1022ds/diu.c
@@ -0,0 +1,172 @@
+/*
+ * Copyright 2010 Freescale Semiconductor, Inc.
+ * Authors: Timur Tabi <timur@freescale.com>
+ *
+ * FSL DIU Framebuffer driver
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+#include <common.h>
+#include <command.h>
+#include <asm/io.h>
+
+#ifdef CONFIG_FSL_DIU_FB
+
+#include "../common/ngpixis.h"
+#include "../common/fsl_diu_fb.h"
+
+#ifdef CONFIG_VIDEO
+#include <stdio_dev.h>
+#include <video_fb.h>
+#endif
+
+#define PX_BRDCFG0_ELBC_DIU	0x02
+
+#define PX_BRDCFG1_DVIEN	0x80
+#define PX_BRDCFG1_DFPEN	0x40
+#define PX_BRDCFG1_BACKLIGHT	0x20
+#define PX_BRDCFG1_DDCEN	0x10
+
+/*
+ * DIU Area Descriptor
+ *
+ * Note that we need to byte-swap the value before it's written to the AD
+ * register.  So even though the registers don't look like they're in the same
+ * bit positions as they are on the MPC8610, the same value is written to the
+ * AD register on the MPC8610 and on the P1022.
+ */
+#define AD_BYTE_F		0x10000000
+#define AD_ALPHA_C_MASK		0x0E000000
+#define AD_ALPHA_C_SHIFT	25
+#define AD_BLUE_C_MASK		0x01800000
+#define AD_BLUE_C_SHIFT		23
+#define AD_GREEN_C_MASK		0x00600000
+#define AD_GREEN_C_SHIFT	21
+#define AD_RED_C_MASK		0x00180000
+#define AD_RED_C_SHIFT		19
+#define AD_PALETTE		0x00040000
+#define AD_PIXEL_S_MASK		0x00030000
+#define AD_PIXEL_S_SHIFT	16
+#define AD_COMP_3_MASK		0x0000F000
+#define AD_COMP_3_SHIFT		12
+#define AD_COMP_2_MASK		0x00000F00
+#define AD_COMP_2_SHIFT		8
+#define AD_COMP_1_MASK		0x000000F0
+#define AD_COMP_1_SHIFT		4
+#define AD_COMP_0_MASK		0x0000000F
+#define AD_COMP_0_SHIFT		0
+
+#define AD_DEFAULT
+static int xres, yres;
+
+void diu_set_pixel_clock(unsigned int pixclock)
+{
+	ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
+	unsigned long speed_ccb, temp;
+	u32 pixval;
+
+	speed_ccb = get_bus_freq(0);
+	temp = 1000000000 / pixclock;
+	temp *= 1000;
+	pixval = speed_ccb / temp;
+	debug("DIU pixval = %lu\n", pixval);
+
+	/* Modify PXCLK in GUTS CLKDVDR */
+	temp = in_be32(&gur->clkdvdr) & 0x2000FFFF;
+	out_be32(&gur->clkdvdr, temp);			/* turn off clock */
+	out_be32(&gur->clkdvdr, temp | 0x80000000 | ((pixval & 0x1F) << 16));
+}
+
+int p1022ds_diu_init(void)
+{
+	ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
+	char *monitor_port;
+	u32 pixel_format;
+	u8 temp;
+
+	pixel_format = cpu_to_le32(AD_BYTE_F | (3 << AD_ALPHA_C_SHIFT) |
+		(0 << AD_BLUE_C_SHIFT) | (1 << AD_GREEN_C_SHIFT) |
+		(2 << AD_RED_C_SHIFT) | (8 << AD_COMP_3_SHIFT) |
+		(8 << AD_COMP_2_SHIFT) | (8 << AD_COMP_1_SHIFT) |
+		(8 << AD_COMP_0_SHIFT) | (3 << AD_PIXEL_S_SHIFT));
+
+	temp = in_8(&pixis->brdcfg1);
+
+	monitor_port = getenv("monitor");
+	if (!strncmp(monitor_port, "1", 1)) { /* 1 - Single link LVDS */
+		xres = 1024;
+		yres = 768;
+		/* Enable the DFP port, disable the DVI and the backlight */
+		temp &= ~(PX_BRDCFG1_DVIEN | PX_BRDCFG1_BACKLIGHT);
+		temp |= PX_BRDCFG1_DFPEN;
+	} else {	/* DVI */
+		xres = 1280;
+		yres = 1024;
+		/* Enable the DVI port, disable the DFP and the backlight */
+		temp &= ~(PX_BRDCFG1_DFPEN | PX_BRDCFG1_BACKLIGHT);
+		temp |= PX_BRDCFG1_DVIEN;
+	}
+
+	out_8(&pixis->brdcfg1, temp);
+
+	/*
+	 * Route the LAD pins to the DIU.  This will disable access to the eLBC,
+	 * which means we won't be able to read/write any NOR flash addresses!
+	 */
+	out_8(&pixis->brdcfg0, in_8(&pixis->brdcfg0) | PX_BRDCFG0_ELBC_DIU);
+	/* we must do the dummy read from eLBC to sync the write as above */
+	in_8(&pixis->brdcfg0);
+
+	/* Setting PMUXCR to switch to DVI from ELBC */
+	/* Set pmuxcr to allow both i2c1 and i2c2 */
+	clrsetbits_be32(&gur->pmuxcr, 0xc0000000, 0x40000000);
+	in_be32(&gur->pmuxcr);
+
+	return fsl_diu_init(xres, pixel_format, 0);
+}
+
+#ifdef CONFIG_VIDEO
+
+/*
+ * The Graphic Device
+ */
+static GraphicDevice ctfb;
+
+void *video_hw_init(void)
+{
+	struct fb_info *info;
+
+	if (p1022ds_diu_init() < 0)
+		return NULL;
+
+	/* fill in Graphic device struct */
+	sprintf(ctfb.modeIdent, "%ix%ix%i %ikHz %iHz", xres, yres, 32, 64, 60);
+
+	ctfb.frameAdrs = (unsigned int)fsl_fb_open(&info);
+	ctfb.winSizeX = xres;
+	ctfb.winSizeY = yres;
+	ctfb.plnSizeX = ctfb.winSizeX;
+	ctfb.plnSizeY = ctfb.winSizeY;
+
+	ctfb.gdfBytesPP = 4;
+	ctfb.gdfIndex = GDF_32BIT_X888RGB;
+
+	ctfb.isaBase = 0;
+	ctfb.pciBase = 0;
+	ctfb.memSize = info->screen_size;
+
+	/* Cursor Start Address */
+	ctfb.dprBase = 0;
+	ctfb.vprBase = 0;
+	ctfb.cprBase = 0;
+
+	return &ctfb;
+}
+
+#endif
+
+#endif
diff --git a/include/configs/P1022DS.h b/include/configs/P1022DS.h
index dcaca2b..d518c69 100644
--- a/include/configs/P1022DS.h
+++ b/include/configs/P1022DS.h
@@ -31,6 +31,7 @@
 #define CONFIG_FSL_PCIE_RESET		/* need PCIe reset errata */
 #define CONFIG_SYS_PCI_64BIT		/* enable 64-bit PCI resources */
 #define CONFIG_SYS_HAS_SERDES		/* has SERDES */
+#define CONFIG_FSL_DIU_FB		/* Has a DIU video controller */
 
 #define CONFIG_PHYS_64BIT
 #define CONFIG_ENABLE_36BIT_PHYS
@@ -181,10 +182,14 @@
 #define CONFIG_SYS_DIU_ADDR	(CONFIG_SYS_CCSRBAR + 0x10000)
 
 /* Video */
-/* #define CONFIG_VIDEO */
+#undef CONFIG_VIDEO
+
 #ifdef CONFIG_VIDEO
+#define CONFIG_CMD_BMP
 #define CONFIG_CFB_CONSOLE
 #define CONFIG_VGA_AS_SINGLE_DEVICE
+#define CONFIG_VIDEO_LOGO
+#define CONFIG_VIDEO_BMP_LOGO
 #endif
 
 /*
-- 
1.7.2.1

[U-Boot] [PATCH] p1022ds: add video support

[Anatolij Gustschin]

Hi Timur,

On Thu, 2 Sep 2010 15:35:19 -0500
Timur Tabi <timur@freescale.com> wrote:
...
> +#include <common.h>
> +#include <command.h>
> +#include <asm/io.h>
> +
> +#ifdef CONFIG_FSL_DIU_FB

Please drop the #ifdef above. Conditional compilation is
already handled by Makefile.

...
> +#define PX_BRDCFG1_DDCEN	0x10

This one is not used, please drop.

...
> +#define AD_BYTE_F		0x10000000
> +#define AD_ALPHA_C_MASK		0x0E000000
> +#define AD_ALPHA_C_SHIFT	25
> +#define AD_BLUE_C_MASK		0x01800000
> +#define AD_BLUE_C_SHIFT		23
> +#define AD_GREEN_C_MASK		0x00600000
> +#define AD_GREEN_C_SHIFT	21
> +#define AD_RED_C_MASK		0x00180000
> +#define AD_RED_C_SHIFT		19
> +#define AD_PALETTE		0x00040000
> +#define AD_PIXEL_S_MASK		0x00030000
> +#define AD_PIXEL_S_SHIFT	16
> +#define AD_COMP_3_MASK		0x0000F000
> +#define AD_COMP_3_SHIFT		12
> +#define AD_COMP_2_MASK		0x00000F00
> +#define AD_COMP_2_SHIFT		8
> +#define AD_COMP_1_MASK		0x000000F0
> +#define AD_COMP_1_SHIFT		4
> +#define AD_COMP_0_MASK		0x0000000F
> +#define AD_COMP_0_SHIFT		0
> +
> +#define AD_DEFAULT

Some of the defines above are not used (*_MASK, AD_PALETTE,
AD_DEFAULT), please drop them, too.

...
> diff --git a/include/configs/P1022DS.h b/include/configs/P1022DS.h
> index dcaca2b..d518c69 100644
> --- a/include/configs/P1022DS.h
> +++ b/include/configs/P1022DS.h
> @@ -31,6 +31,7 @@
>  #define CONFIG_FSL_PCIE_RESET		/* need PCIe reset errata */
>  #define CONFIG_SYS_PCI_64BIT		/* enable 64-bit PCI resources */
>  #define CONFIG_SYS_HAS_SERDES		/* has SERDES */
> +#define CONFIG_FSL_DIU_FB		/* Has a DIU video controller */

Drop this CONFIG_FSL_DIU_FB define, too. It is already present
in the board config file.

>  #define CONFIG_PHYS_64BIT
>  #define CONFIG_ENABLE_36BIT_PHYS
> @@ -181,10 +182,14 @@
>  #define CONFIG_SYS_DIU_ADDR	(CONFIG_SYS_CCSRBAR + 0x10000)
>  
>  /* Video */
> -/* #define CONFIG_VIDEO */
> +#undef CONFIG_VIDEO
> +
>  #ifdef CONFIG_VIDEO
> +#define CONFIG_CMD_BMP
>  #define CONFIG_CFB_CONSOLE
>  #define CONFIG_VGA_AS_SINGLE_DEVICE
> +#define CONFIG_VIDEO_LOGO
> +#define CONFIG_VIDEO_BMP_LOGO
>  #endif

CONFIG_VIDEO definition should depend on CONFIG_FSL_DIU_FB,
I think. Otherwise the building won't succeed if CONFIG_FSL_DIU_FB
is not defined (due to absence of video_hw_init()).

Thanks,
Anatolij

[U-Boot] [PATCH] p1022ds: add video support

[Timur Tabi]

Anatolij Gustschin wrote:

>> +#define AD_BYTE_F		0x10000000
>> +#define AD_ALPHA_C_MASK		0x0E000000
>> +#define AD_ALPHA_C_SHIFT	25
>> +#define AD_BLUE_C_MASK		0x01800000
>> +#define AD_BLUE_C_SHIFT		23
>> +#define AD_GREEN_C_MASK		0x00600000
>> +#define AD_GREEN_C_SHIFT	21
>> +#define AD_RED_C_MASK		0x00180000
>> +#define AD_RED_C_SHIFT		19
>> +#define AD_PALETTE		0x00040000
>> +#define AD_PIXEL_S_MASK		0x00030000
>> +#define AD_PIXEL_S_SHIFT	16
>> +#define AD_COMP_3_MASK		0x0000F000
>> +#define AD_COMP_3_SHIFT		12
>> +#define AD_COMP_2_MASK		0x00000F00
>> +#define AD_COMP_2_SHIFT		8
>> +#define AD_COMP_1_MASK		0x000000F0
>> +#define AD_COMP_1_SHIFT		4
>> +#define AD_COMP_0_MASK		0x0000000F
>> +#define AD_COMP_0_SHIFT		0
>> +
>> +#define AD_DEFAULT
> 
> Some of the defines above are not used (*_MASK, AD_PALETTE,
> AD_DEFAULT), please drop them, too.

Does it really matter?  These are all in a .c file, so it's not like I'm
polluting the name space.

Also, I thought it would be better to provide some sort of completeness for the
bits I am using.

> CONFIG_VIDEO definition should depend on CONFIG_FSL_DIU_FB,
> I think. Otherwise the building won't succeed if CONFIG_FSL_DIU_FB
> is not defined (due to absence of video_hw_init()).

We need the DIU initialized, even if U-Boot doesn't use it.  This is what
CONFIG_FSL_DIU_FB does.  CONFIG_VIDEO then tells U-Boot to actually use the DIU.

-- 
Timur Tabi
Linux kernel developer at Freescale

[U-Boot] [PATCH] p1022ds: add video support

[Anatolij Gustschin]

On Tue, 21 Sep 2010 17:00:19 -0500
Timur Tabi <timur@freescale.com> wrote:

> Anatolij Gustschin wrote:
> 
> >> +#define AD_BYTE_F		0x10000000
> >> +#define AD_ALPHA_C_MASK		0x0E000000
> >> +#define AD_ALPHA_C_SHIFT	25
> >> +#define AD_BLUE_C_MASK		0x01800000
> >> +#define AD_BLUE_C_SHIFT		23
> >> +#define AD_GREEN_C_MASK		0x00600000
> >> +#define AD_GREEN_C_SHIFT	21
> >> +#define AD_RED_C_MASK		0x00180000
> >> +#define AD_RED_C_SHIFT		19
> >> +#define AD_PALETTE		0x00040000
> >> +#define AD_PIXEL_S_MASK		0x00030000
> >> +#define AD_PIXEL_S_SHIFT	16
> >> +#define AD_COMP_3_MASK		0x0000F000
> >> +#define AD_COMP_3_SHIFT		12
> >> +#define AD_COMP_2_MASK		0x00000F00
> >> +#define AD_COMP_2_SHIFT		8
> >> +#define AD_COMP_1_MASK		0x000000F0
> >> +#define AD_COMP_1_SHIFT		4
> >> +#define AD_COMP_0_MASK		0x0000000F
> >> +#define AD_COMP_0_SHIFT		0
> >> +
> >> +#define AD_DEFAULT
> > 
> > Some of the defines above are not used (*_MASK, AD_PALETTE,
> > AD_DEFAULT), please drop them, too.
> 
> Does it really matter?  These are all in a .c file, so it's not like I'm
> polluting the name space.
> 
> Also, I thought it would be better to provide some sort of completeness for the
> bits I am using.

We do not add unused code.

> > CONFIG_VIDEO definition should depend on CONFIG_FSL_DIU_FB,
> > I think. Otherwise the building won't succeed if CONFIG_FSL_DIU_FB
> > is not defined (due to absence of video_hw_init()).
> 
> We need the DIU initialized, even if U-Boot doesn't use it.  This is what
> CONFIG_FSL_DIU_FB does.  CONFIG_VIDEO then tells U-Boot to actually use the DIU.

I understand it. I didn't say you should make CONFIG_FSL_DIU_FB
dependent on CONFIG_VIDEO, but the opposite.

On the other site, one of the design principles states:

initialize devices only when they are needed within U-Boot.

Thanks,
Anatolij

[U-Boot] [PATCH] p1022ds: add video support

[Timur Tabi]

Anatolij Gustschin wrote:
>> Also, I thought it would be better to provide some sort of completeness for the
>> > bits I am using.
> We do not add unused code.

Well, I don't see how macros are "unused code", but I'll delete those lines.

>>> > > CONFIG_VIDEO definition should depend on CONFIG_FSL_DIU_FB,
>>> > > I think. Otherwise the building won't succeed if CONFIG_FSL_DIU_FB
>>> > > is not defined (due to absence of video_hw_init()).
>> > 
>> > We need the DIU initialized, even if U-Boot doesn't use it.  This is what
>> > CONFIG_FSL_DIU_FB does.  CONFIG_VIDEO then tells U-Boot to actually use the DIU.

> I understand it. I didn't say you should make CONFIG_FSL_DIU_FB
> dependent on CONFIG_VIDEO, but the opposite.

Ok.

> On the other site, one of the design principles states:
> 
> initialize devices only when they are needed within U-Boot.

Well, I'll try to work toward that, but I'm pretty sure there are plenty of
devices that are initialized but not used in U-Boot already.

-- 
Timur Tabi
Linux kernel developer at Freescale

[U-Boot] [PATCH] p1022ds: add video support

[Detlev Zundel]

Hi Timur,

> Anatolij Gustschin wrote:
>>> Also, I thought it would be better to provide some sort of completeness for the
>>> > bits I am using.
>> We do not add unused code.
>
> Well, I don't see how macros are "unused code", but I'll delete those lines.
>
>>>> > > CONFIG_VIDEO definition should depend on CONFIG_FSL_DIU_FB,
>>>> > > I think. Otherwise the building won't succeed if CONFIG_FSL_DIU_FB
>>>> > > is not defined (due to absence of video_hw_init()).
>>> > 
>>> > We need the DIU initialized, even if U-Boot doesn't use it.  This is what
>>> > CONFIG_FSL_DIU_FB does.  CONFIG_VIDEO then tells U-Boot to actually use the DIU.
>
>> I understand it. I didn't say you should make CONFIG_FSL_DIU_FB
>> dependent on CONFIG_VIDEO, but the opposite.
>
> Ok.
>
>> On the other site, one of the design principles states:
>> 
>> initialize devices only when they are needed within U-Boot.
>
> Well, I'll try to work toward that, but I'm pretty sure there are plenty of
> devices that are initialized but not used in U-Boot already.

This "but there are other examples doing just the same!" arguing is
getting tiresome.  Please accept that this is not an argument capable of
convincing people - the opposite is true.

Is there a technical reason why the initialization has to be in U-Boot?

Cheers
  Detlev

-- 
Less talking -- more hacking
                                        -- Olin Shivers
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH] p1022ds: add video support

[Timur Tabi]

Detlev Zundel wrote:
> This "but there are other examples doing just the same!" arguing is
> getting tiresome.  Please accept that this is not an argument capable of
> convincing people - the opposite is true.

It's also very frustrating when developers look at the way U-Boot does things,
then spend countless hours or days implementing something similar, only be told,
"Well, we know this is how it's done, but it's wrong, so you need to rewrite
your code."

Frankly, I think *that* is way more tiresome.

> Is there a technical reason why the initialization has to be in U-Boot?

I'll have to re-examine the code.  There may be a way to eliminate any such
initialization.

-- 
Timur Tabi
Linux kernel developer at Freescale

[U-Boot] [PATCH] p1022ds: add video support

[York Sun]

On Wed, 2010-09-22 at 10:10 -0500, Timur Tabi wrote:
> > Is there a technical reason why the initialization has to be in U-Boot?
> 
> I'll have to re-examine the code.  There may be a way to eliminate any such
> initialization.
> 
There is a technical reason. The DIU driver needs initialize DVI encoder
which is on I2C bus. Linux kernel starts to use console device long
before I2C is available. So it has to be done in U-boot.

York

[U-Boot] [PATCH] p1022ds: add video support

[Detlev Zundel]

Hi,

> On Wed, 2010-09-22 at 10:10 -0500, Timur Tabi wrote:
>> > Is there a technical reason why the initialization has to be in U-Boot?
>> 
>> I'll have to re-examine the code.  There may be a way to eliminate any such
>> initialization.
>> 
> There is a technical reason. The DIU driver needs initialize DVI encoder
> which is on I2C bus. Linux kernel starts to use console device long
> before I2C is available. So it has to be done in U-boot.

Ok, then why not always define CONFIG_VIDEO for this configuration and
let U-Boot initialize _and_ use it?  Actually I fail to see why U-Boot
shouldn't use it if the support is there or what am I missing?

Best wishes
  Detlev

-- 
   "Our enemies  are innovative  and resourceful, and so are we.  They
never stop thinking about new ways to harm our country and our people,
and neither do we."
       -- George W. Bush during a defense bill ceremony on 5. Aug 2004
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH] p1022ds: add video support

[Timur Tabi]

Detlev Zundel wrote:
> Ok, then why not always define CONFIG_VIDEO for this configuration and
> let U-Boot initialize _and_ use it?  Actually I fail to see why U-Boot
> shouldn't use it if the support is there or what am I missing?

How can U-Boot know whether a monitor is actually connected to the DVI port?

-- 
Timur Tabi
Linux kernel developer at Freescale

[U-Boot] [PATCH] p1022ds: add video support

[Detlev Zundel]

Hi Timur,

> Detlev Zundel wrote:
>> Ok, then why not always define CONFIG_VIDEO for this configuration and
>> let U-Boot initialize _and_ use it?  Actually I fail to see why U-Boot
>> shouldn't use it if the support is there or what am I missing?
>
> How can U-Boot know whether a monitor is actually connected to the DVI port?

I don't know - DDC?  Isn't it supposed to be used to do exactly this?
I'm pretty sure that Anatolij would welcome an implementation :)

Cheers
  Detlev

-- 
Summary [of object-oriented programming in Perl 5]
That's all about there is to it. Now you just need to go off and buy a
book about object-oriented design methodology, and bang  your forehead
with it for the next six months or so.    Larry Wall [Creator of Perl]
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH] p1022ds: add video support

[Detlev Zundel]

Hi Timur,

> Detlev Zundel wrote:
>> This "but there are other examples doing just the same!" arguing is
>> getting tiresome.  Please accept that this is not an argument capable of
>> convincing people - the opposite is true.
>
> It's also very frustrating when developers look at the way U-Boot does things,
> then spend countless hours or days implementing something similar, only be told,
> "Well, we know this is how it's done, but it's wrong, so you need to rewrite
> your code."

It is a simple fact that we cannot remove every part of U-Boot code
which is not in alignment with the (current) design principles.  The
design principles of _every_ software project will develop over its
existence and there will always be old code around which is not how it
would be done if starting today.  This will happen for every moderately
large project and should be "common sense" for software engineers.

The only way I can see around this is if the project has a sufficiently
large number of "janitors" cleaning up such things.  And you know as
well as I do that we simply do not have such people for U-Boot, although
the work would be much appreciated.

It is not the first and not the second time that you argument this way.
Who stops you from posting a small mail with the intent of your changes
to get early feedback from others if in doubt?  Not only in this context
I would advise doing this whenever possible.

Cheers
  Detlev

-- 
Another helpful hint for successful MIME processing:

application/msword; rm -f %s; description="MS Word Text";
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de