From: Anatolij Gustschin <agust@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] Added support for splash screen positioning adding by adding
Date: Wed, 01 Jul 2009 11:18:24 +0200 [thread overview]
Message-ID: <4A4B29E0.60505@denx.de> (raw)
In-Reply-To: <1246368064-4957-3-git-send-email-matthias.weisser@graf-syteco.de>
Dear Matthias,
Thanks for this patch, some comments however. Please use short
patch subject, e.g. "[PATCH] Add support for splash screen positioning"
is enough. And than please add more descriptive patch commit message
above your "Signed-off-by" line. In this commit message you can shortly
explain how it is done (e.g. what you mean with "adding by adding").
This is a new feature and also we should document it in the README
where the original "splashimage" environment variable is documented.
But if this feature will be documented, we also have to support it
in other places, it is used in "common/lcd.c".
Also please see other comments below.
Matthias Weisser wrote:
> Signed-off-by: Matthias Weisser <matthias.weisser@graf-syteco.de>
> ---
> drivers/video/cfb_console.c | 12 ++++++++++--
> 1 files changed, 10 insertions(+), 2 deletions(-)
> mode change 100644 => 100755 drivers/video/cfb_console.c
>
> diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c
> old mode 100644
> new mode 100755
> index bcafb27..15b99cb
> --- a/drivers/video/cfb_console.c
> +++ b/drivers/video/cfb_console.c
> @@ -314,7 +314,7 @@ void console_cursor (int state);
> #else
> #define SWAP16(x) (x)
> #define SWAP32(x) (x)
> -#if defined(VIDEO_FB_16BPP_PIXEL_SWAP)
> +#if defined(VIDEO_FB_16BPP_PIXEL_SWAP) || defined (CONFIG_VIDEO_JADEGDC)
Here you added CONFIG_VIDEO_JADEGDC case. Please do this in your
other patch where you add support for JADE GDC. This patch addresses
extension of "splashimage" and should do only this logical change.
> #define SHORTSWAP32(x) ( ((x) >> 16) | ((x) << 16) )
> #else
> #define SHORTSWAP32(x) (x)
> @@ -1188,9 +1188,17 @@ static void *video_logo (void)
> ulong addr;
>
> if ((s = getenv ("splashimage")) != NULL) {
> + int x = 0, y = 0;
> +
----^^^^^^^^^^^^
please remove tabs in the empty line above.
> addr = simple_strtoul (s, NULL, 16);
>
> - if (video_display_bitmap (addr, 0, 0) == 0) {
> + if ((s = strchr (s, ' ')) != NULL) {
> + x = simple_strtoul (s + 1, NULL, 0);
> + if ((s = strchr (s + 1, ' ')) != NULL)
------------------------------------------------------------^^^^
remove trailing white space here, please.
> + y = simple_strtoul (s + 1, NULL, 0);
> + }
> +
-----^^^^^^^^^^^
please remove tabs here, too.
> + if (video_display_bitmap (addr, x, y) == 0) {
> return ((void *) (video_fb_address));
> }
> }
Please fix and resubmit, thanks!
Anatolij
next prev parent reply other threads:[~2009-07-01 9:18 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-06-30 13:21 [U-Boot] [PATCH] This patch adds support for Fujitsu MB86R01 'JADE' SOC Matthias Weisser
2009-06-30 13:21 ` [U-Boot] [PATCH] Add suport for the graphic controller included in the JADE SOC Matthias Weisser
2009-06-30 13:21 ` [U-Boot] [PATCH] Added support for splash screen positioning adding by adding Matthias Weisser
2009-06-30 13:21 ` [U-Boot] [PATCH] Added support for jadecpu board using the MB86R01 'Jade' SOC from Fujitsu Matthias Weisser
2009-07-01 9:18 ` Anatolij Gustschin [this message]
2009-07-01 13:40 ` [U-Boot] [PATCH] Add suport for the graphic controller included in the JADE SOC Anatolij Gustschin
2009-07-02 6:02 ` [U-Boot] Antw: " Matthias Weisser
2009-07-02 9:27 ` [U-Boot] " Anatolij Gustschin
2009-07-04 23:30 ` Wolfgang Denk
2009-07-07 12:29 ` Anatolij Gustschin
2009-07-19 20:12 ` [U-Boot] [PATCH] This patch adds support for Fujitsu MB86R01 'JADE' SOC Wolfgang Denk
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=4A4B29E0.60505@denx.de \
--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