Hello Nikita,
On 01/23/2013 09:31 AM, Nikita Kiryanov wrote:
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.
my point is that lcd_clear will e.g. call lcd_logo. Although I haven't
tested it,
it seems you're make a side effect of a function only called once a side
effect
of another function (which could be called multiple times). So you make
things
even worse (loading an bitmap while the function is just named to
display it).
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.
then document the callback...
I don't see the improvement of this patch..
Regards,
Jeroen
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
