From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nikita Kiryanov Date: Wed, 23 Jan 2013 10:31:55 +0200 Subject: [U-Boot] [PATCH 2/5] lcd: add option for board specific splash screen preparation In-Reply-To: <50FD9398.1010603@myspectrum.nl> References: <1356246228-26732-1-git-send-email-nikita@compulab.co.il> <1356246228-26732-3-git-send-email-nikita@compulab.co.il> <50FC54BC.3070101@myspectrum.nl> <50FCF38D.7070901@compulab.co.il> <50FD9398.1010603@myspectrum.nl> Message-ID: <50FF9FFB.4090806@compulab.co.il> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 01/21/2013 09:14 PM, Jeroen Hofstee wrote: > Hello Nikita, > > On 01/21/2013 08:51 AM, Nikita Kiryanov wrote: >> Hi Jeroen, >> >> On 01/20/2013 10:34 PM, Jeroen Hofstee wrote: >> [...] >>>> diff --git a/include/lcd.h b/include/lcd.h >>>> index c24164a..4ac4ddd 100644 >>>> --- a/include/lcd.h >>>> +++ b/include/lcd.h >>>> @@ -47,6 +47,7 @@ extern struct vidinfo panel_info; >>>> extern void lcd_ctrl_init (void *lcdbase); >>>> extern void lcd_enable (void); >>>> +extern int board_splash_screen_prepare(void); >>>> /* setcolreg used in 8bpp/16bpp; initcolregs used in monochrome */ >>>> extern void lcd_setcolreg (ushort regno, >>> Other boards seem to do this in lcd_enable. Perhaps that is also an >>> option. >> >> The problem with doing it in lcd_enable is that it's akin to a side >> effect, given what the function's name advertises. Preparing the splash >> image can, however, be a natural part of the process that displays it. >> > mmm, I am not so sure I agree that loading a bitmap in lcd_enable is > a _problem_, while loading it in show logo and requiring a CONFIG_* is > _natural_. Well, it is a problem. If we don't respect the abstractions we create then things like function names become meaningless. A function called "lcd_enable" should do just that- enable lcd. Not load stuff from storage to memory or manipulate BMPs. > > But anyway, can't this at least be changed to a __weak function, so the > CONFIG and ifdef stuff can be dropped? The motivation behind the CONFIG was to make it a documentable user setting, rather than an undocumented feature buried in the code. > > Regards, > Jeroen > > -- Regards, Nikita.