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

[Anatolij Gustschin]

On Tue, 21 Sep 2010 17:00:19 -0500
Timur Tabi <timur at 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