On 17/01/14 17:36, Przemyslaw Marczak wrote: > On 01/17/2014 07:26 AM, Minkyu Kang wrote: >> On 15/01/14 17:18, Przemyslaw Marczak wrote: >>>>>>> >>>>>>> In this way we can add other functions in the future even without >>>>> CONFIG_MISC_INIT_R. >>>>>> >>>>>> partly agree. >>>>>> But, I doubt what is the role of misc.c file. >>>>>> because of the meaning of miscellaneous is ambiguous, this file have >>>>>> possibility to be messy. >>>>>> So, please let me know what is your plan to this file. >>>>>> >>>>> >>>>> I first planned put there only implementation of misc_init_r() and it's >>>>> subfunctions - as the easy way to display logo and menu for Samsung >>>>> boards. >>>>> Piotr has suggested to change the purpose of this file as misc not only >>>>> for misc_init_r implementation... >>>> Przemyslaw, I asked you question: what is the misc.c file for? >>>> If for misc_init_r only then I think the file name "misc.c" is confusing. >>>> If also other common functions can be put there, then the define >>>> MISC_INIT_R >>>> to compile this file is wrong. >>>> >>> >>> Yes, and next I said that maybe I will change this config dependency, and >>> now I try to do it. >>> >> >> Actually, I feel negative to this changes. >> Because misc_init_r is a board specific. >> How you can support if someone want to do something (board specific things) >> on misc_init_r? >> I totally understand why you add misc_init_r to common directory. - It means >> you don't have to explain why you added it :) >> but it looks little weird. >> So we will discuss that misc_init_r should go to each boards or stay here? >> (misc_init_r function only, not including key, menu, logo.. etc) >> >> Please let me know your opinions. >> >> Thanks, >> Minkyu Kang. >> >> > > The reason why I used misc_init_r for a common purposes is that it is called > after all hardware initialization and before u-boot main_loop(), then I don't > need to introduce another generic function just to check buttons - this is > the only reason. > > Moreover at this time misc_init_r() is implemented only in Trats2, and there > are easy to move things. > > You're right - misc_init_r is board specific, but if we make it as a common > function, then we also can add board specific code, called here but > implemented in board files.
No. it looks wrong architecture. > > If this is wrong, then where is the better place for check keys, display logo > and any more vendor common things? Stay at common directory. > > Or maybe the better solution is just add new function callback to > board_init_r() for some vendor specific purposes - and then it can be used > for other vendors platforms too. No. Here, I've made pseudo code for it. How you think? diff --git a/board/samsung/common/misc.c b/board/samsung/common/misc.c new file mode 100644 index 0000000..cdc48bb --- /dev/null +++ b/board/samsung/common/misc.c @@ -0,0 +1,18 @@ +#ifdef CONFIG_LCD_MENU +void keys_init(void) +{ + /* TODO */ +} + +void check_boot_mode(void) +{ + /* TODO */ +} +#endif + +#ifdef CONFIG_CMD_BMP +void draw_logo(void) +{ + /* TODO */ +} +#endif diff --git a/board/samsung/trats2/trats2.c b/board/samsung/trats2/trats2.c index be15357..eb4f4dc 100644 --- a/board/samsung/trats2/trats2.c +++ b/board/samsung/trats2/trats2.c @@ -623,6 +623,16 @@ int misc_init_r(void) show_hw_revision(); +#ifdef CONFIG_LCD_MENU + keys_init(); + check_boot_mode(); +#endif + +#ifdef CONFIG_CMD_BMP + if (panel_info.logo_on) + draw_logo(); +#endif + return 0; } #endif Thanks, Minkyu Kang. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot