public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
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

  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