| Message ID | 744bcfadc1429136ef62b9130c42dfae9f6b5b6f.1389352945.git.p.marczak@samsung.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Minkyu Kang |
| Headers | show |
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,
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.
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,
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
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,
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.
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,
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; +}