Patchwork [U-Boot,v5,03/12] samsung: common: Add misc file and common function misc_init_r().

login
register
mail settings
Submitter Przemyslaw Marczak
Date Jan. 10, 2014, 2:31 p.m.
Message ID <744bcfadc1429136ef62b9130c42dfae9f6b5b6f.1389352945.git.p.marczak@samsung.com>
Download mbox | patch
Permalink /patch/309300/
State Changes Requested
Delegated to: Minkyu Kang
Headers show

Comments

Przemyslaw Marczak - Jan. 10, 2014, 2:31 p.m.
Config: CONFIG_MISC_INIT_R enables implementation of misc_init_r()
in common file::
- board/samsung/common/misc.c

Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
---
Changes v2:
- change CONFIG_SAMSUNG to CONFIG_MISC_INIT_R

Changes v3:
- fix merge conflict in board/samsung/common/Makefile

Changes v4:
- none

Changes v5:
- add acked-by

 board/samsung/common/Makefile |    1 +
 board/samsung/common/misc.c   |   14 ++++++++++++++
 2 files changed, 15 insertions(+)
 create mode 100644 board/samsung/common/misc.c
Przemyslaw Marczak - Jan. 14, 2014, 1:55 p.m.
Hello,
In case of discussion with Piotr Wilczek maybe it is better to make some 
changes in this patch.

On 01/10/2014 03:31 PM, Przemyslaw Marczak wrote:
> Config: CONFIG_MISC_INIT_R enables implementation of misc_init_r()
> in common file::
> - board/samsung/common/misc.c
>
> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
> Changes v2:
> - change CONFIG_SAMSUNG to CONFIG_MISC_INIT_R
>
> Changes v3:
> - fix merge conflict in board/samsung/common/Makefile
>
> Changes v4:
> - none
>
> Changes v5:
> - add acked-by
>
>   board/samsung/common/Makefile |    1 +
>   board/samsung/common/misc.c   |   14 ++++++++++++++
>   2 files changed, 15 insertions(+)
>   create mode 100644 board/samsung/common/misc.c
>
> diff --git a/board/samsung/common/Makefile b/board/samsung/common/Makefile
> index 22bd6b1..79547a3 100644
> --- a/board/samsung/common/Makefile
> +++ b/board/samsung/common/Makefile
> @@ -8,6 +8,7 @@
>   obj-$(CONFIG_SOFT_I2C_MULTI_BUS) += multi_i2c.o
>   obj-$(CONFIG_THOR_FUNCTION) += thor.o
>   obj-$(CONFIG_CMD_USB_MASS_STORAGE) += ums.o
> +obj-$(CONFIG_MISC_INIT_R) += misc.o
here change to:
obj-y += misc.o

>
>   ifndef CONFIG_SPL_BUILD
>   obj-$(CONFIG_BOARD_COMMON)	+= board.o
> diff --git a/board/samsung/common/misc.c b/board/samsung/common/misc.c
> new file mode 100644
> index 0000000..3764d12
> --- /dev/null
> +++ b/board/samsung/common/misc.c
> @@ -0,0 +1,14 @@
> +/*
> + * Copyright (C) 2013 Samsung Electronics
> + * Przemyslaw Marczak <p.marczak@samsung.com>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <common.h>
> +

and here:
#ifdef CONFIG_MISC_INIT_R

> +/* Common for Samsung boards */
> +int misc_init_r(void)
> +{
> +	return 0;
> +}
>
#endif

In this way we can add other functions in the future even without 
CONFIG_MISC_INIT_R.

Is it better solution?

Thank you,
Minkyu Kang - Jan. 15, 2014, 7:35 a.m.
On 14/01/14 22:55, Przemyslaw Marczak wrote:
> Hello,
> In case of discussion with Piotr Wilczek maybe it is better to make some changes in this patch.
> 
> On 01/10/2014 03:31 PM, Przemyslaw Marczak wrote:
>> Config: CONFIG_MISC_INIT_R enables implementation of misc_init_r()
>> in common file::
>> - board/samsung/common/misc.c
>>
>> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
>> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>> Changes v2:
>> - change CONFIG_SAMSUNG to CONFIG_MISC_INIT_R
>>
>> Changes v3:
>> - fix merge conflict in board/samsung/common/Makefile
>>
>> Changes v4:
>> - none
>>
>> Changes v5:
>> - add acked-by
>>
>>   board/samsung/common/Makefile |    1 +
>>   board/samsung/common/misc.c   |   14 ++++++++++++++
>>   2 files changed, 15 insertions(+)
>>   create mode 100644 board/samsung/common/misc.c
>>
>> diff --git a/board/samsung/common/Makefile b/board/samsung/common/Makefile
>> index 22bd6b1..79547a3 100644
>> --- a/board/samsung/common/Makefile
>> +++ b/board/samsung/common/Makefile
>> @@ -8,6 +8,7 @@
>>   obj-$(CONFIG_SOFT_I2C_MULTI_BUS) += multi_i2c.o
>>   obj-$(CONFIG_THOR_FUNCTION) += thor.o
>>   obj-$(CONFIG_CMD_USB_MASS_STORAGE) += ums.o
>> +obj-$(CONFIG_MISC_INIT_R) += misc.o
> here change to:
> obj-y += misc.o
> 
>>
>>   ifndef CONFIG_SPL_BUILD
>>   obj-$(CONFIG_BOARD_COMMON)    += board.o
>> diff --git a/board/samsung/common/misc.c b/board/samsung/common/misc.c
>> new file mode 100644
>> index 0000000..3764d12
>> --- /dev/null
>> +++ b/board/samsung/common/misc.c
>> @@ -0,0 +1,14 @@
>> +/*
>> + * Copyright (C) 2013 Samsung Electronics
>> + * Przemyslaw Marczak <p.marczak@samsung.com>
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +
> 
> and here:
> #ifdef CONFIG_MISC_INIT_R
> 
>> +/* Common for Samsung boards */
>> +int misc_init_r(void)
>> +{
>> +    return 0;
>> +}
>>
> #endif
> 
> 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.

> 
> Is it better solution?
> 
> Thank you,

Thanks,
Minkyu Kang.
Przemyslaw Marczak - Jan. 15, 2014, 7:51 a.m.
Hello,

On 01/15/2014 08:35 AM, Minkyu Kang wrote:
> On 14/01/14 22:55, Przemyslaw Marczak wrote:
>> Hello,
>> In case of discussion with Piotr Wilczek maybe it is better to make some changes in this patch.
>>
>> On 01/10/2014 03:31 PM, Przemyslaw Marczak wrote:
>>> Config: CONFIG_MISC_INIT_R enables implementation of misc_init_r()
>>> in common file::
>>> - board/samsung/common/misc.c
>>>
>>> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
>>> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
>>> ---
>>> Changes v2:
>>> - change CONFIG_SAMSUNG to CONFIG_MISC_INIT_R
>>>
>>> Changes v3:
>>> - fix merge conflict in board/samsung/common/Makefile
>>>
>>> Changes v4:
>>> - none
>>>
>>> Changes v5:
>>> - add acked-by
>>>
>>>    board/samsung/common/Makefile |    1 +
>>>    board/samsung/common/misc.c   |   14 ++++++++++++++
>>>    2 files changed, 15 insertions(+)
>>>    create mode 100644 board/samsung/common/misc.c
>>>
>>> diff --git a/board/samsung/common/Makefile b/board/samsung/common/Makefile
>>> index 22bd6b1..79547a3 100644
>>> --- a/board/samsung/common/Makefile
>>> +++ b/board/samsung/common/Makefile
>>> @@ -8,6 +8,7 @@
>>>    obj-$(CONFIG_SOFT_I2C_MULTI_BUS) += multi_i2c.o
>>>    obj-$(CONFIG_THOR_FUNCTION) += thor.o
>>>    obj-$(CONFIG_CMD_USB_MASS_STORAGE) += ums.o
>>> +obj-$(CONFIG_MISC_INIT_R) += misc.o
>> here change to:
>> obj-y += misc.o
>>
>>>
>>>    ifndef CONFIG_SPL_BUILD
>>>    obj-$(CONFIG_BOARD_COMMON)    += board.o
>>> diff --git a/board/samsung/common/misc.c b/board/samsung/common/misc.c
>>> new file mode 100644
>>> index 0000000..3764d12
>>> --- /dev/null
>>> +++ b/board/samsung/common/misc.c
>>> @@ -0,0 +1,14 @@
>>> +/*
>>> + * Copyright (C) 2013 Samsung Electronics
>>> + * Przemyslaw Marczak <p.marczak@samsung.com>
>>> + *
>>> + * SPDX-License-Identifier:    GPL-2.0+
>>> + */
>>> +
>>> +#include <common.h>
>>> +
>>
>> and here:
>> #ifdef CONFIG_MISC_INIT_R
>>
>>> +/* Common for Samsung boards */
>>> +int misc_init_r(void)
>>> +{
>>> +    return 0;
>>> +}
>>>
>> #endif
>>
>> 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...

>>
>> Is it better solution?
>>
>> Thank you,
>
> Thanks,
> Minkyu Kang.
>

Thank you,
Piotr Wilczek - Jan. 15, 2014, 8:08 a.m.
Dear Przemyslaw,

> -----Original Message-----
> From: Przemyslaw Marczak [mailto:p.marczak@samsung.com]
> Sent: Wednesday, January 15, 2014 8:51 AM
> To: Minkyu Kang
> Cc: u-boot@lists.denx.de; jh80.chung@samsung.com;
> human.hwang@samsung.com; dh09.lee@samsung.com; ideal.song@samsung.com;
> Piotr Wilczek; Lukasz Majewski
> Subject: Re: [PATCH v5 03/12] samsung: common: Add misc file and common
> function misc_init_r().
> 
> Hello,
> 
> On 01/15/2014 08:35 AM, Minkyu Kang wrote:
> > On 14/01/14 22:55, Przemyslaw Marczak wrote:
> >> Hello,
> >> In case of discussion with Piotr Wilczek maybe it is better to make
> some changes in this patch.
> >>
> >> On 01/10/2014 03:31 PM, Przemyslaw Marczak wrote:
> >>> Config: CONFIG_MISC_INIT_R enables implementation of misc_init_r()
> >>> in common file::
> >>> - board/samsung/common/misc.c
> >>>
> >>> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
> >>> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
> >>> ---
> >>> Changes v2:
> >>> - change CONFIG_SAMSUNG to CONFIG_MISC_INIT_R
> >>>
> >>> Changes v3:
> >>> - fix merge conflict in board/samsung/common/Makefile
> >>>
> >>> Changes v4:
> >>> - none
> >>>
> >>> Changes v5:
> >>> - add acked-by
> >>>
> >>>    board/samsung/common/Makefile |    1 +
> >>>    board/samsung/common/misc.c   |   14 ++++++++++++++
> >>>    2 files changed, 15 insertions(+)
> >>>    create mode 100644 board/samsung/common/misc.c
> >>>
> >>> diff --git a/board/samsung/common/Makefile
> >>> b/board/samsung/common/Makefile index 22bd6b1..79547a3 100644
> >>> --- a/board/samsung/common/Makefile
> >>> +++ b/board/samsung/common/Makefile
> >>> @@ -8,6 +8,7 @@
> >>>    obj-$(CONFIG_SOFT_I2C_MULTI_BUS) += multi_i2c.o
> >>>    obj-$(CONFIG_THOR_FUNCTION) += thor.o
> >>>    obj-$(CONFIG_CMD_USB_MASS_STORAGE) += ums.o
> >>> +obj-$(CONFIG_MISC_INIT_R) += misc.o
> >> here change to:
> >> obj-y += misc.o
> >>
> >>>
> >>>    ifndef CONFIG_SPL_BUILD
> >>>    obj-$(CONFIG_BOARD_COMMON)    += board.o
> >>> diff --git a/board/samsung/common/misc.c
> >>> b/board/samsung/common/misc.c new file mode 100644 index
> >>> 0000000..3764d12
> >>> --- /dev/null
> >>> +++ b/board/samsung/common/misc.c
> >>> @@ -0,0 +1,14 @@
> >>> +/*
> >>> + * Copyright (C) 2013 Samsung Electronics
> >>> + * Przemyslaw Marczak <p.marczak@samsung.com>
> >>> + *
> >>> + * SPDX-License-Identifier:    GPL-2.0+
> >>> + */
> >>> +
> >>> +#include <common.h>
> >>> +
> >>
> >> and here:
> >> #ifdef CONFIG_MISC_INIT_R
> >>
> >>> +/* Common for Samsung boards */
> >>> +int misc_init_r(void)
> >>> +{
> >>> +    return 0;
> >>> +}
> >>>
> >> #endif
> >>
> >> 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.

> 
> >>
> >> Is it better solution?
> >>
> >> Thank you,
> >
> > Thanks,
> > Minkyu Kang.
> >
> 
> Thank you,
> --
> Przemyslaw Marczak
> Samsung R&D Institute Poland
> Samsung Electronics
> p.marczak@samsung.com

Best regards,
Piotr Wilczek
Przemyslaw Marczak - Jan. 15, 2014, 8:18 a.m.
Hello Piotr,

On 01/15/2014 09:08 AM, Piotr Wilczek wrote:
> Dear Przemyslaw,
>
>> -----Original Message-----
>> From: Przemyslaw Marczak [mailto:p.marczak@samsung.com]
>> Sent: Wednesday, January 15, 2014 8:51 AM
>> To: Minkyu Kang
>> Cc: u-boot@lists.denx.de; jh80.chung@samsung.com;
>> human.hwang@samsung.com; dh09.lee@samsung.com; ideal.song@samsung.com;
>> Piotr Wilczek; Lukasz Majewski
>> Subject: Re: [PATCH v5 03/12] samsung: common: Add misc file and common
>> function misc_init_r().
>>
>> Hello,
>>
>> On 01/15/2014 08:35 AM, Minkyu Kang wrote:
>>> On 14/01/14 22:55, Przemyslaw Marczak wrote:
>>>> Hello,
>>>> In case of discussion with Piotr Wilczek maybe it is better to make
>> some changes in this patch.
>>>>
>>>> On 01/10/2014 03:31 PM, Przemyslaw Marczak wrote:
>>>>> Config: CONFIG_MISC_INIT_R enables implementation of misc_init_r()
>>>>> in common file::
>>>>> - board/samsung/common/misc.c
>>>>>
>>>>> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
>>>>> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
>>>>> ---
>>>>> Changes v2:
>>>>> - change CONFIG_SAMSUNG to CONFIG_MISC_INIT_R
>>>>>
>>>>> Changes v3:
>>>>> - fix merge conflict in board/samsung/common/Makefile
>>>>>
>>>>> Changes v4:
>>>>> - none
>>>>>
>>>>> Changes v5:
>>>>> - add acked-by
>>>>>
>>>>>     board/samsung/common/Makefile |    1 +
>>>>>     board/samsung/common/misc.c   |   14 ++++++++++++++
>>>>>     2 files changed, 15 insertions(+)
>>>>>     create mode 100644 board/samsung/common/misc.c
>>>>>
>>>>> diff --git a/board/samsung/common/Makefile
>>>>> b/board/samsung/common/Makefile index 22bd6b1..79547a3 100644
>>>>> --- a/board/samsung/common/Makefile
>>>>> +++ b/board/samsung/common/Makefile
>>>>> @@ -8,6 +8,7 @@
>>>>>     obj-$(CONFIG_SOFT_I2C_MULTI_BUS) += multi_i2c.o
>>>>>     obj-$(CONFIG_THOR_FUNCTION) += thor.o
>>>>>     obj-$(CONFIG_CMD_USB_MASS_STORAGE) += ums.o
>>>>> +obj-$(CONFIG_MISC_INIT_R) += misc.o
>>>> here change to:
>>>> obj-y += misc.o
>>>>
>>>>>
>>>>>     ifndef CONFIG_SPL_BUILD
>>>>>     obj-$(CONFIG_BOARD_COMMON)    += board.o
>>>>> diff --git a/board/samsung/common/misc.c
>>>>> b/board/samsung/common/misc.c new file mode 100644 index
>>>>> 0000000..3764d12
>>>>> --- /dev/null
>>>>> +++ b/board/samsung/common/misc.c
>>>>> @@ -0,0 +1,14 @@
>>>>> +/*
>>>>> + * Copyright (C) 2013 Samsung Electronics
>>>>> + * Przemyslaw Marczak <p.marczak@samsung.com>
>>>>> + *
>>>>> + * SPDX-License-Identifier:    GPL-2.0+
>>>>> + */
>>>>> +
>>>>> +#include <common.h>
>>>>> +
>>>>
>>>> and here:
>>>> #ifdef CONFIG_MISC_INIT_R
>>>>
>>>>> +/* Common for Samsung boards */
>>>>> +int misc_init_r(void)
>>>>> +{
>>>>> +    return 0;
>>>>> +}
>>>>>
>>>> #endif
>>>>
>>>> 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.

>>
>>>>
>>>> Is it better solution?
>>>>
>>>> Thank you,
>>>
>>> Thanks,
>>> Minkyu Kang.
>>>
>>
>> Thank you,
>> --
>> Przemyslaw Marczak
>> Samsung R&D Institute Poland
>> Samsung Electronics
>> p.marczak@samsung.com
>
> Best regards,
> Piotr Wilczek
>
>
>
>

Thank you,
Minkyu Kang - Jan. 17, 2014, 6:26 a.m.
On 15/01/14 17:18, Przemyslaw Marczak wrote:
> Hello Piotr,
> 
> On 01/15/2014 09:08 AM, Piotr Wilczek wrote:
>> Dear Przemyslaw,
>>
>>> -----Original Message-----
>>> From: Przemyslaw Marczak [mailto:p.marczak@samsung.com]
>>> Sent: Wednesday, January 15, 2014 8:51 AM
>>> To: Minkyu Kang
>>> Cc: u-boot@lists.denx.de; jh80.chung@samsung.com;
>>> human.hwang@samsung.com; dh09.lee@samsung.com; ideal.song@samsung.com;
>>> Piotr Wilczek; Lukasz Majewski
>>> Subject: Re: [PATCH v5 03/12] samsung: common: Add misc file and common
>>> function misc_init_r().
>>>
>>> Hello,
>>>
>>> On 01/15/2014 08:35 AM, Minkyu Kang wrote:
>>>> On 14/01/14 22:55, Przemyslaw Marczak wrote:
>>>>> Hello,
>>>>> In case of discussion with Piotr Wilczek maybe it is better to make
>>> some changes in this patch.
>>>>>
>>>>> On 01/10/2014 03:31 PM, Przemyslaw Marczak wrote:
>>>>>> Config: CONFIG_MISC_INIT_R enables implementation of misc_init_r()
>>>>>> in common file::
>>>>>> - board/samsung/common/misc.c
>>>>>>
>>>>>> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
>>>>>> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
>>>>>> ---
>>>>>> Changes v2:
>>>>>> - change CONFIG_SAMSUNG to CONFIG_MISC_INIT_R
>>>>>>
>>>>>> Changes v3:
>>>>>> - fix merge conflict in board/samsung/common/Makefile
>>>>>>
>>>>>> Changes v4:
>>>>>> - none
>>>>>>
>>>>>> Changes v5:
>>>>>> - add acked-by
>>>>>>
>>>>>>     board/samsung/common/Makefile |    1 +
>>>>>>     board/samsung/common/misc.c   |   14 ++++++++++++++
>>>>>>     2 files changed, 15 insertions(+)
>>>>>>     create mode 100644 board/samsung/common/misc.c
>>>>>>
>>>>>> diff --git a/board/samsung/common/Makefile
>>>>>> b/board/samsung/common/Makefile index 22bd6b1..79547a3 100644
>>>>>> --- a/board/samsung/common/Makefile
>>>>>> +++ b/board/samsung/common/Makefile
>>>>>> @@ -8,6 +8,7 @@
>>>>>>     obj-$(CONFIG_SOFT_I2C_MULTI_BUS) += multi_i2c.o
>>>>>>     obj-$(CONFIG_THOR_FUNCTION) += thor.o
>>>>>>     obj-$(CONFIG_CMD_USB_MASS_STORAGE) += ums.o
>>>>>> +obj-$(CONFIG_MISC_INIT_R) += misc.o
>>>>> here change to:
>>>>> obj-y += misc.o
>>>>>
>>>>>>
>>>>>>     ifndef CONFIG_SPL_BUILD
>>>>>>     obj-$(CONFIG_BOARD_COMMON)    += board.o
>>>>>> diff --git a/board/samsung/common/misc.c
>>>>>> b/board/samsung/common/misc.c new file mode 100644 index
>>>>>> 0000000..3764d12
>>>>>> --- /dev/null
>>>>>> +++ b/board/samsung/common/misc.c
>>>>>> @@ -0,0 +1,14 @@
>>>>>> +/*
>>>>>> + * Copyright (C) 2013 Samsung Electronics
>>>>>> + * Przemyslaw Marczak <p.marczak@samsung.com>
>>>>>> + *
>>>>>> + * SPDX-License-Identifier:    GPL-2.0+
>>>>>> + */
>>>>>> +
>>>>>> +#include <common.h>
>>>>>> +
>>>>>
>>>>> and here:
>>>>> #ifdef CONFIG_MISC_INIT_R
>>>>>
>>>>>> +/* Common for Samsung boards */
>>>>>> +int misc_init_r(void)
>>>>>> +{
>>>>>> +    return 0;
>>>>>> +}
>>>>>>
>>>>> #endif
>>>>>
>>>>> 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.
Przemyslaw Marczak - Jan. 17, 2014, 8:36 a.m.
On 01/17/2014 07:26 AM, Minkyu Kang wrote:
> On 15/01/14 17:18, Przemyslaw Marczak wrote:
>> Hello Piotr,
>>
>> On 01/15/2014 09:08 AM, Piotr Wilczek wrote:
>>> Dear Przemyslaw,
>>>
>>>> -----Original Message-----
>>>> From: Przemyslaw Marczak [mailto:p.marczak@samsung.com]
>>>> Sent: Wednesday, January 15, 2014 8:51 AM
>>>> To: Minkyu Kang
>>>> Cc: u-boot@lists.denx.de; jh80.chung@samsung.com;
>>>> human.hwang@samsung.com; dh09.lee@samsung.com; ideal.song@samsung.com;
>>>> Piotr Wilczek; Lukasz Majewski
>>>> Subject: Re: [PATCH v5 03/12] samsung: common: Add misc file and common
>>>> function misc_init_r().
>>>>
>>>> Hello,
>>>>
>>>> On 01/15/2014 08:35 AM, Minkyu Kang wrote:
>>>>> On 14/01/14 22:55, Przemyslaw Marczak wrote:
>>>>>> Hello,
>>>>>> In case of discussion with Piotr Wilczek maybe it is better to make
>>>> some changes in this patch.
>>>>>>
>>>>>> On 01/10/2014 03:31 PM, Przemyslaw Marczak wrote:
>>>>>>> Config: CONFIG_MISC_INIT_R enables implementation of misc_init_r()
>>>>>>> in common file::
>>>>>>> - board/samsung/common/misc.c
>>>>>>>
>>>>>>> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
>>>>>>> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
>>>>>>> ---
>>>>>>> Changes v2:
>>>>>>> - change CONFIG_SAMSUNG to CONFIG_MISC_INIT_R
>>>>>>>
>>>>>>> Changes v3:
>>>>>>> - fix merge conflict in board/samsung/common/Makefile
>>>>>>>
>>>>>>> Changes v4:
>>>>>>> - none
>>>>>>>
>>>>>>> Changes v5:
>>>>>>> - add acked-by
>>>>>>>
>>>>>>>      board/samsung/common/Makefile |    1 +
>>>>>>>      board/samsung/common/misc.c   |   14 ++++++++++++++
>>>>>>>      2 files changed, 15 insertions(+)
>>>>>>>      create mode 100644 board/samsung/common/misc.c
>>>>>>>
>>>>>>> diff --git a/board/samsung/common/Makefile
>>>>>>> b/board/samsung/common/Makefile index 22bd6b1..79547a3 100644
>>>>>>> --- a/board/samsung/common/Makefile
>>>>>>> +++ b/board/samsung/common/Makefile
>>>>>>> @@ -8,6 +8,7 @@
>>>>>>>      obj-$(CONFIG_SOFT_I2C_MULTI_BUS) += multi_i2c.o
>>>>>>>      obj-$(CONFIG_THOR_FUNCTION) += thor.o
>>>>>>>      obj-$(CONFIG_CMD_USB_MASS_STORAGE) += ums.o
>>>>>>> +obj-$(CONFIG_MISC_INIT_R) += misc.o
>>>>>> here change to:
>>>>>> obj-y += misc.o
>>>>>>
>>>>>>>
>>>>>>>      ifndef CONFIG_SPL_BUILD
>>>>>>>      obj-$(CONFIG_BOARD_COMMON)    += board.o
>>>>>>> diff --git a/board/samsung/common/misc.c
>>>>>>> b/board/samsung/common/misc.c new file mode 100644 index
>>>>>>> 0000000..3764d12
>>>>>>> --- /dev/null
>>>>>>> +++ b/board/samsung/common/misc.c
>>>>>>> @@ -0,0 +1,14 @@
>>>>>>> +/*
>>>>>>> + * Copyright (C) 2013 Samsung Electronics
>>>>>>> + * Przemyslaw Marczak <p.marczak@samsung.com>
>>>>>>> + *
>>>>>>> + * SPDX-License-Identifier:    GPL-2.0+
>>>>>>> + */
>>>>>>> +
>>>>>>> +#include <common.h>
>>>>>>> +
>>>>>>
>>>>>> and here:
>>>>>> #ifdef CONFIG_MISC_INIT_R
>>>>>>
>>>>>>> +/* Common for Samsung boards */
>>>>>>> +int misc_init_r(void)
>>>>>>> +{
>>>>>>> +    return 0;
>>>>>>> +}
>>>>>>>
>>>>>> #endif
>>>>>>
>>>>>> 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.

If this is wrong, then where is the better place for check keys, display 
logo and any more vendor common things?

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.

Thank you,

Patch

diff --git a/board/samsung/common/Makefile b/board/samsung/common/Makefile
index 22bd6b1..79547a3 100644
--- a/board/samsung/common/Makefile
+++ b/board/samsung/common/Makefile
@@ -8,6 +8,7 @@ 
 obj-$(CONFIG_SOFT_I2C_MULTI_BUS) += multi_i2c.o
 obj-$(CONFIG_THOR_FUNCTION) += thor.o
 obj-$(CONFIG_CMD_USB_MASS_STORAGE) += ums.o
+obj-$(CONFIG_MISC_INIT_R) += misc.o
 
 ifndef CONFIG_SPL_BUILD
 obj-$(CONFIG_BOARD_COMMON)	+= board.o
diff --git a/board/samsung/common/misc.c b/board/samsung/common/misc.c
new file mode 100644
index 0000000..3764d12
--- /dev/null
+++ b/board/samsung/common/misc.c
@@ -0,0 +1,14 @@ 
+/*
+ * Copyright (C) 2013 Samsung Electronics
+ * Przemyslaw Marczak <p.marczak@samsung.com>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+
+/* Common for Samsung boards */
+int misc_init_r(void)
+{
+	return 0;
+}