Hi Simon, On Thu, Sep 27, 2018 at 9:42 PM Simon Glass <s...@chromium.org> wrote: > > Hi Bin, > > On 25 September 2018 at 23:39, Bin Meng <bmeng...@gmail.com> wrote: > > Hi Simon, > > > > On Wed, Sep 26, 2018 at 1:42 PM Simon Glass <s...@chromium.org> wrote: > >> > >> Hi Bin, > >> > >> On 4 September 2018 at 03:06, Bin Meng <bmeng...@gmail.com> wrote: > >> > Hi Simon, > >> > > >> > On Mon, Sep 3, 2018 at 7:02 AM Simon Glass <s...@chromium.org> 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 <s...@chromium.org> > >> >> --- > >> >> > >> >> 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.
I re-read the codes and your comments. I think what you said below: "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." makes sense. So adding 'select TIMER_EARLY' to BOOTSTAGE seems over-killing things. Let's use your approach. Reviewed-by: Bin Meng <bmeng...@gmail.com> Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
