On Tue, Oct 2, 2018 at 9:25 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Thu, Sep 27, 2018 at 9:42 PM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 25 September 2018 at 23:39, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Wed, Sep 26, 2018 at 1:42 PM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 4 September 2018 at 03:06, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Sep 3, 2018 at 7:02 AM Simon Glass sjg@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 sjg@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 availabledefault y if X86 && BOOTSTAGESince 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.cn@gmail.com
applied to u-boot-x86, thanks!