[U-Boot] [PATCH v2 05/21] pmic: Introduce power_board_init() method at ./lib/board.c file
Lukasz Majewski
l.majewski at samsung.com
Tue Oct 9 12:25:48 CEST 2012
Hi Stefano,
> On 05/10/2012 10:16, Lukasz Majewski wrote:
> > It is necessary to introduce a new system wide function-
> > power_board_init()
> >
> > It turns out, that power initialization must be done as early as
> > possible. In the case of PMIC framework redesign, which aims to
> > support multiple instances of PMIC devices the initialization shall
> > be performed just after malloc configuration.
> >
> > The arch_early_init_r() could be used instead, but it doesn't
> > reflect that the "power" subsystem is going to be initialized.
> >
> > Signed-off-by: Lukasz Majewski <l.majewski at samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
> > ---
> > Changes for v2:
> > - None
>
> Hi Lucasz,
>
> only to make some order. We have already several entries that calls
> board specific functions.
>
> There is at least board_init() and board_late_init(). The last one was
> use mostly for the pmic initialization up nowm because it is called
> after malloc_init().
>
> Now you want to add a new one. This means we have cases where the PMIC
> must be initialized before flash, right ?
The case is that PMIC (and other devices) needs to be setup after
malloc init and before MMC (in our case).
The board_init() is before malloc init and board_late_init() is after
MMC initialization for which working PMIC is necessary.
PMIC_v1 works since there is a single, static instance of PMIC device
(no lists needed).
Filling the above gap was my motivation to define special callback
for power.
In the ./arch/arm/lib/board.c there is
#ifdef CONFIG_ARCH_EARLY_INIT_R
arch_early_init_r();
#endif
Which could be used and defined at e.g. ./boards/samsung/trats.c
However it is only ARM specific (not available at PPC) and the name is
a bit misleading. (frankly speaking it is a dead code).
>
> We use often the "weak" mechanism to avoid that a board maintainer is
> constrained to implement an empty function only to make happy the
> linker. It is better to declare the function as weak and call it with
> the same schema, such as board_power_init() (several board_* are weak)
Correct me if I'm wrong, but weren't we recently trying to remove
functions defined as weak? One of arguments was that weak function will
allow to build a u-boot.bin, which then crash at null pointer
dereference when proper CONFIG not selected.
>
> > ---
> > arch/arm/lib/board.c | 4 ++++
> > include/common.h | 3 +++
> > 2 files changed, 7 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> > index 109a1ac..431ef5b 100644
> > --- a/arch/arm/lib/board.c
> > +++ b/arch/arm/lib/board.c
> > @@ -513,6 +513,10 @@ void board_init_r(gd_t *id, ulong dest_addr)
> > arch_early_init_r();
> > #endif
> >
> > +#ifdef CONFIG_POWER_INIT
> > + power_board_init();
>
> Do we need a config option only for calling this ? I think that the
> decision can be taken with CONFIG_PMIC, declaring the function as
> weak.
This is one of options -> __weak power_board_init() would be defined
when no CONFIG_PMIC defined.
Good idea, since one CONFIG_POWER_* option would be removed.
>
> > +#endif
> > +
> > #if !defined(CONFIG_SYS_NO_FLASH)
> > puts("Flash: ");
> >
> > diff --git a/include/common.h b/include/common.h
> > index a7fb05e..5cc859f 100644
> > --- a/include/common.h
> > +++ b/include/common.h
> > @@ -487,6 +487,9 @@ int board_late_init (void);
> > int board_postclk_init (void); /* after clocks/timebase, before
> > env/serial */ int board_early_init_r (void);
> > void board_poweroff (void);
> > +#ifdef CONFIG_POWER_INIT
> > +int power_board_init(void);
> > +#endif
> >
> > #if defined(CONFIG_SYS_DRAM_TEST)
> > int testdram(void);
> >
>
> Best regards,
> Stefano Babic
>
--
Best regards,
Lukasz Majewski
Samsung Poland R&D Center | Linux Platform Group
More information about the U-Boot
mailing list