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

[Piotr Wilczek]

Dear Przemyslaw,

> -----Original Message-----
> From: Przemyslaw Marczak [mailto:p.marczak at samsung.com]
> Sent: Wednesday, January 15, 2014 8:51 AM
> To: Minkyu Kang
> Cc: u-boot at lists.denx.de; jh80.chung at samsung.com;
> human.hwang at samsung.com; dh09.lee at samsung.com; ideal.song at 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 at samsung.com>
> >>> Acked-by: Jaehoon Chung <jh80.chung at 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 at 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 at samsung.com

Best regards,
Piotr Wilczek