From mboxrd@z Thu Jan  1 00:00:00 1970
From: Lukasz Majewski <l.majewski@samsung.com>
Date: Tue, 09 Oct 2012 12:25:48 +0200
Subject: [U-Boot] [PATCH v2 05/21] pmic: Introduce power_board_init()
 method at ./lib/board.c file
In-Reply-To: <5073E631.2030402@denx.de>
References: <1349425003-32523-1-git-send-email-l.majewski@samsung.com>
	<1349425003-32523-6-git-send-email-l.majewski@samsung.com>
	<5073E631.2030402@denx.de>
Message-ID: <20121009122548.74467dc2@amdc308.digital.local>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@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