From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Thu, 27 Sep 2018 06:41:41 -0700 Subject: [U-Boot] [PATCH 1/4] Enable CONFIG_TIMER_EARLY with bootstage In-Reply-To: References: <20180902230227.26643-1-sjg@chromium.org> <20180902230227.26643-2-sjg@chromium.org> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Bin, On 25 September 2018 at 23:39, Bin Meng wrote: > Hi Simon, > > On Wed, Sep 26, 2018 at 1:42 PM Simon Glass wrote: >> >> Hi Bin, >> >> On 4 September 2018 at 03:06, Bin Meng wrote: >> > Hi Simon, >> > >> > On Mon, Sep 3, 2018 at 7:02 AM Simon Glass wrote: >> >> >> >> In initr_bootstage() we call bootstage_mark_name() which ends up calling >> >> timer_get_us(). This call happens before initr_dm(), which inits driver >> >> model. >> >> >> >> On x86 we set gd->timer to NULL in the transition from board_init_f() >> > >> > It's not just x86 we set gd->timer to NULL. It applied to all >> > architectures when CONFIG_TIMER is on. >> > >> >> to board_init_r(). See board_init_f_r() for this assignment. So U-Boot >> >> knows there is no timer available in the period immediately after >> >> relocation. >> >> >> >> On x86 the timer_get_us() call is implemented as calls to get_ticks() and >> >> get_tbclk(). Both of these call dm_timer_init() to set up the timer, if >> >> gd->timer is NULL and the early timer is not available. >> >> >> >> However dm_timer_init() cannot succeed before initr_dm() is called. >> >> >> >> So it seems that on x86 if we want to use CONFIG_BOOTSTAGE we must enable >> >> CONFIG_TIMER_EARLY. Update the Kconfig to handle this. >> >> >> >> Note: On most architectures we can rely on the pre-relocation memory still >> >> being available, so that gd->timer pointers to a valid timer device and >> >> everything works correctly. Admittedly this is not strictly correct since >> >> the timer device is set up by pre-relocation U-Boot, but normally this is >> >> fine. On x86 the 'CAR' (cache-as-RAM) memory used by pre-relocation U-Boot >> >> disappears in board_init_f_r() and any attempt to access it will hang. >> >> This is the reason why we must mark the timer as invalid when we get to >> >> board_init_f_r(). >> >> >> >> Signed-off-by: Simon Glass >> >> --- >> >> >> >> drivers/timer/Kconfig | 3 +++ >> >> 1 file changed, 3 insertions(+) >> >> >> >> diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig >> >> index 5ab6749193c..ff434de6f7c 100644 >> >> --- a/drivers/timer/Kconfig >> >> +++ b/drivers/timer/Kconfig >> >> @@ -30,6 +30,9 @@ config TPL_TIMER >> >> config TIMER_EARLY >> >> bool "Allow timer to be used early in U-Boot" >> >> depends on TIMER >> >> + # initr_bootstage() requires a timer and is called before initr_dm() >> >> + # so only the early timer is available >> >> + default y if X86 && BOOTSTAGE >> > >> > Since this applies not only on x86, and given without TIMER_EARLY >> > BOOTSTAGE is broken, shouldn't we do this in BOOTSTAGE config instead: >> > >> > config BOOTSTAGE >> > select TIMER_EARLY >> >> Well we could, but I suspect that would break things since the early >> timer is not supported by many boards. Also for most boards this >> doesn't actually work fine. x86 is really quite awful in that it has >> no SRAM and the CAR becomes inaccessible as soon as you turn on the >> cache! > > It's true that early timer is supported by some limited boards, but > that's a different issue. For now that patch does not fix anything. If > we add BOOTSTAGE from either defconfig or 'menuconfig' for a board, > without adding TIMER_EARLY, it will for sure break. Is this because of code called in board_f.c ? I don't quite follow. Regards, Simon