From: Anatolij Gustschin <agust@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] p1022ds: add video support
Date: Wed, 22 Sep 2010 00:57:00 +0200 [thread overview]
Message-ID: <20100922005700.499ce44d@wker> (raw)
In-Reply-To: <4C992AF3.5030001@freescale.com>
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
next prev parent reply other threads:[~2010-09-21 22:57 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-09-02 20:35 [U-Boot] [PATCH] p1022ds: add video support Timur Tabi
2010-09-21 21:56 ` Anatolij Gustschin
2010-09-21 22:00 ` Timur Tabi
2010-09-21 22:57 ` Anatolij Gustschin [this message]
2010-09-21 23:01 ` Timur Tabi
2010-09-22 15:07 ` Detlev Zundel
2010-09-22 15:10 ` Timur Tabi
2010-09-22 15:16 ` York Sun
2010-09-22 15:33 ` Detlev Zundel
2010-09-22 15:35 ` Timur Tabi
2010-09-22 15:45 ` Detlev Zundel
2010-09-22 15:30 ` Detlev Zundel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100922005700.499ce44d@wker \
--to=agust@denx.de \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox