[prev in list] [next in list] [prev in thread] [next in thread] 

List:       u-boot
Subject:    Re: [U-Boot] [PATCH] Add splash screen support via loading from flash
From:       Robert Winkler <robert.winkler () boundarydevices ! com>
Date:       2013-05-31 17:50:36
Message-ID: CAMDMJ5NaDV+rrTRQPgWeWHrG-60Co2HS4VLHaTfUcCLQB4f-GA () mail ! gmail ! com
[Download RAW message or body]

On Fri, May 31, 2013 at 12:49 AM, Wolfgang Denk <wd@denx.de> wrote:
> 
> Dear Robert,
> 
> In message <CAMDMJ5PJ_AqZaxTRdnvS8ju_SkVHV7zoDrBUyhgjt2jfr4Ok4Q@mail.gmail.com> you \
> wrote:
> > 
> > > > board/boundary/nitrogen6x/nitrogen6x.c | 22 ++++++++++++++++++++++
> > > > include/configs/nitrogen6x.h           | 11 ++++++++++-
> > > > 2 files changed, 32 insertions(+), 1 deletion(-)
> > > 
> > > Could you please explain why you need custom code here?
> > 
> > I originally tried to use the CONFIG_SPLASH_SCREEN_PREPARE functionality.
> > After some frustration, I figured out that without CONFIG_LCD, it is never
> > actually called.
> > http://git.denx.de/?p=u-boot/u-boot-imx.git;a=blob;f=common/stdio.c;h=5d5117c0ed838195a2caad7c28c128247771cd92;hb=HEAD#l216
> >  
> > It does seem odd that there are 2 totally different paths and the other
> > splash screen related macros are used in both but the prepare function is
> > only called in the LCD case.
> 
> OK, so this is a bug that needs to be fixed - but for all boards.
> 
> > Since we use CONFIG_VIDEO and not CONFIG_LCD, this seemed to be the best,
> > or at least the least invasive, solution.
> 
> No, this is definitely not the "best" solution, as it keeps the bug,
> so the next one who tries to do what you are doing will run into it
> again.
> 
> 
> Pleast let's fix the problem at the cause.
> 
> Please fix the video code to also run splash_screen_prepare() at the
> appropriate place.  While you are at it, please also replace this
> construct
> 
> 1071 #ifdef CONFIG_SPLASH_SCREEN_PREPARE
> 1072 static inline int splash_screen_prepare(void)
> 1073 {
> 1074         return board_splash_screen_prepare();
> 1075 }
> 1076 #else
> 1077 static inline int splash_screen_prepare(void)
> 1078 {
> 1079         return 0;
> 1080 }
> 1081 #endif
> 
> (in "common/lcd.c") and use a "weak" function for
> splash_screen_prepare().  board_splash_screen_prepare() is no longer
> needed, then (the boards can provide their own implementation of
> splash_screen_prepare()).


I'm not quite clear on how it should be.

So if I were just fixing lcd.c to use a weak function it would look like this:
#ifdef CONFIG_SPLASH_SCREEN_PREPARE
int __splash_screen_prepare(void)
{
    return 0;
}

int splash_screen_prepare(void)
    __attribute__ ((weak, alias("__splash_screen_prepare")));
#endif

But it'd also have to go in cfb_console.c.  Is it possible for a board
to ever use both CONFIG_LCD and CONFIG_VIDEO and if ?

Also are prototypes necessary?  Looking at cfb_console for example
weak functions I see a prototype for video_set_lut in video_fb.h but I
can't find any for board_video_skip.


> 
> 
> Thanks.
> 
> Best regards,
> 
> Wolfgang Denk
> 
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
> Es ist offensichtlich, dass das menschliche Gehirn wie  ein  Computer
> funktioniert.  Da  es  keine  dummen Computer gibt, gibt es also auch
> keine dummen Menschen. Nur ein paar Leute, die unter DOS laufen.
> -- <unbekannt>



Thanks,

Robert Winkler
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[prev in list] [next in list] [prev in thread] [next in thread]