From mboxrd@z Thu Jan 1 00:00:00 1970 From: Minkyu Kang Date: Mon, 20 Jan 2014 16:06:45 +0900 Subject: [U-Boot] [PATCH v5 03/12] samsung: common: Add misc file and common function misc_init_r(). In-Reply-To: <52D8EB8C.3010009@samsung.com> References: <744bcfadc1429136ef62b9130c42dfae9f6b5b6f.1389352945.git.p.marczak@samsung.com> <52D541EB.8080908@samsung.com> <52D63A29.5080903@samsung.com> <52D63DFB.6030109@samsung.com> <001701cf11c8$f98ea400$ecabec00$%wilczek@samsung.com> <52D64465.9050803@samsung.com> <52D8CD13.9050903@samsung.com> <52D8EB8C.3010009@samsung.com> Message-ID: <52DCCB05.1080008@samsung.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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.