[U-Boot] [PATCH v5 04/12] samsung: misc: move display logo function to misc.c file.

[Przemyslaw Marczak]

Hi Minkyu,

On 01/14/2014 09:55 AM, Minkyu Kang wrote:
> Dear Przemyslaw Marczak,
>
> On 14/01/14 17:02, Przemyslaw Marczak wrote:
>> Hello Jaehoon,
>>
>> On 01/14/2014 04:41 AM, Jaehoon Chung wrote:
>>> Dear Przemyslaw,
>>>
>>> On 01/10/2014 11:31 PM, Przemyslaw Marczak wrote:
>>>> board/samsung/common/misc.c:
>>>> - move draw_logo() function from exynos_fb.c
>>>> - add get_tizen_logo_info() function call removed from board files
>>>>
>>>> boards:
>>>> - update board files
>>>> - add CONFIG_MISC_INIT_R to Universal, Trats and Trats2
>>>>
>>>> Signed-off-by: Przemyslaw Marczak <p.marczak at samsung.com>
>>>> Tested-by: Hyungwon Hwang <human.hwang at samsung.com>
>>>> ---
>>>> changes v2:
>>>> - configs cleanup
>>>> - add check logo address before display
>>>>
>>>> Changes v3:
>>>> - none
>>>>
>>>> Changes v4:
>>>> - none
>>>>
>>>> Changes v5:
>>>> - none
>>>>
>>>>    board/samsung/common/misc.c              |   42 ++++++++++++++++++++++++++++++
>>>>    board/samsung/trats/trats.c              |    3 ---
>>>>    board/samsung/trats2/trats2.c            |    4 ---
>>>>    board/samsung/universal_c210/universal.c |    4 ---
>>>>    drivers/video/exynos_fb.c                |   28 --------------------
>>>>    include/configs/s5pc210_universal.h      |    3 +++
>>>>    include/configs/trats.h                  |    3 +++
>>>>    include/configs/trats2.h                 |    3 +++
>>>>    8 files changed, 51 insertions(+), 39 deletions(-)
>>>>
>>>> diff --git a/board/samsung/common/misc.c b/board/samsung/common/misc.c
>>>> index 3764d12..6188e29 100644
>>>> --- a/board/samsung/common/misc.c
>>>> +++ b/board/samsung/common/misc.c
>>>> @@ -6,9 +6,51 @@
>>>>     */
>>>>
>>>>    #include <common.h>
>>>> +#include <lcd.h>
>>>> +#include <libtizen.h>
>>>> +
>>>> +#ifdef CONFIG_CMD_BMP
>>>> +static void draw_logo(void)
>>>> +{
>>>> +    int x, y;
>>>> +    ulong addr;
>>>> +
>>>> +#ifdef CONFIG_TIZEN
>>>> +    get_tizen_logo_info(&panel_info);
>>>> +#else
>>>> +    return;
>>> if CONFIG_TINZE didn't set, draw_logo should be just return, right?
>>> Then I think this point could be changed more readable.
>>>
>>> #ifdef CONFIG_TIZEN
>>>      int x, y;
>>>      ulong addr;
>>>
>>>      get_tizen_logo_info(...);
>>>
>>>      add = panel_info.logo_addr;
>>>      ...
>>>      bmp_display(addr, x, y);
>>> #endif
>>>
>>> how about?
>>>
>>> Best Regards,
>>> Jaehoon Chung
>>>
>>>
>>
>> You know, this file is common for all Samsung platforms,and I think this function should not depends only on Tizen logo. In this case user can choose other logo just by adding function like "get_logo" instead of the return statement. I also think that there is need for some common function which allows set proper logo by board config, but maybe not in this patch set?
>
> I agreed with you.
>
> You said that "there is need for some common function which allows set proper logo by board config".
> At previous version of board files, you already have common function (init_panel_info - even though this function is not for the logo but I think, it's enough).
>
> @@ -484,10 +484,6 @@ void init_panel_info(vidinfo_t *vid)
>   	vid->resolution	= HD_RESOLUTION;
>   	vid->rgb_mode	= MODE_RGB_P;
>
> -#ifdef CONFIG_TIZEN
> -	get_tizen_logo_info(vid);
> -#endif
> -
>   	/* for LD9040. */
>   	vid->pclk_name = 1;	/* MPLL */
>   	vid->sclk_div = 1;
>
> If you remove this change at each boards, then you don't have add ifdef CONFIG_TIZEN at draw_logo function.
> It can be split common stuffs and board specific stuffs efficiently.
> How you think?
>
> Thanks,
> Minkyu Kang.
>

Actually I had something else on my mind but this is more simply, so I 
will change this back.

Thank you,
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com