[U-Boot] [PATCH v2 0/6] x86: bootstage: Fix bootstage operation on link
Recent bootstage changes have provoked problems with chromebook_link. Bootstage uses the timer before driver model is ready, but link uses driver model for the timer.
This series: - Updates the TSC timer to make the bootstage work before DM is ready - Provides a way to have a separate setting for record count in SPL to save memory - Tidies up a few bootstage options that are no-longer needed
This fixes booting on link which is currently broken.
Changes in v2: - Update to support the early timer - Rebase on top of early timer code and simplify slightly - Add new patch to enable early timer for chromebook_link
Simon Glass (6): board_f: Drop the timer after relocation dm: x86: Allow TSC timer to be used before DM is ready dm: x86: Update timer_get_boot_us to work before DM is ready bootstage: Drop unused options bootstage: Provide a separate record count setting for SPL x86: Enable early timer for chromebook_link
arch/x86/include/asm/global_data.h | 1 + common/Kconfig | 16 ++++++------ common/board_f.c | 1 + common/bootstage.c | 6 ++--- configs/chromebook_link64_defconfig | 1 + configs/chromebook_link_defconfig | 1 + configs/sandbox_defconfig | 1 - configs/sandbox_flattree_defconfig | 1 - configs/sandbox_noblk_defconfig | 1 - configs/sandbox_spl_defconfig | 1 - drivers/timer/tsc_timer.c | 49 +++++++++++++++++++++++++++++-------- include/bootstage.h | 6 ----- 12 files changed, 53 insertions(+), 32 deletions(-)
Once U-Boot relocates itself the existing driver-model timer (if any) is no-longer valid until the device is reinitialised. Any use of the device may cause a crash. To handle this, set the timer to NULL after relocation.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: None
common/board_f.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/common/board_f.c b/common/board_f.c index 104d144f41a..2e597ddf99f 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -952,6 +952,7 @@ void board_init_f_r(void) * UART if available. */ gd->flags &= ~GD_FLG_SERIAL_READY; + gd->timer = NULL;
/* * U-Boot has been copied into SDRAM, the BSS has been cleared etc.
Hi Simon,
On Sun, Aug 27, 2017 at 11:23 PM, Simon Glass sjg@chromium.org wrote:
Once U-Boot relocates itself the existing driver-model timer (if any) is no-longer valid until the device is reinitialised. Any use of the device may cause a crash. To handle this, set the timer to NULL after relocation.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2: None
common/board_f.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/common/board_f.c b/common/board_f.c index 104d144f41a..2e597ddf99f 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -952,6 +952,7 @@ void board_init_f_r(void) * UART if available. */ gd->flags &= ~GD_FLG_SERIAL_READY;
gd->timer = NULL;
This needs to be wrapped with #ifdef CONFIG_TIMER.
/* * U-Boot has been copied into SDRAM, the BSS has been cleared etc.--
Regards, Bin
With bootstage we need access to the timer before driver model is set up. To handle this, put the required state in global_data and provide a new function to set up the device, separate from the driver's probe() method.
This will be used by the 'early' timer also.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Update to support the early timer
arch/x86/include/asm/global_data.h | 1 + drivers/timer/tsc_timer.c | 30 +++++++++++++++++++++++++----- 2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/arch/x86/include/asm/global_data.h b/arch/x86/include/asm/global_data.h index 93a80fe2b6c..fcb6853a380 100644 --- a/arch/x86/include/asm/global_data.h +++ b/arch/x86/include/asm/global_data.h @@ -77,6 +77,7 @@ struct arch_global_data { uint8_t x86_mask; uint32_t x86_device; uint64_t tsc_base; /* Initial value returned by rdtsc() */ + unsigned long clock_rate; /* Clock rate of timer in Hz */ void *new_fdt; /* Relocated FDT */ uint32_t bist; /* Built-in self test value */ enum pei_boot_mode_t pei_boot_mode; diff --git a/drivers/timer/tsc_timer.c b/drivers/timer/tsc_timer.c index 4d1fc9cd137..0012fecde09 100644 --- a/drivers/timer/tsc_timer.c +++ b/drivers/timer/tsc_timer.c @@ -328,17 +328,17 @@ static int tsc_timer_get_count(struct udevice *dev, u64 *count) return 0; }
-static int tsc_timer_probe(struct udevice *dev) +static void tsc_timer_ensure_setup(void) { - struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev); - + if (gd->arch.tsc_base) + return; gd->arch.tsc_base = rdtsc();
/* * If there is no clock frequency specified in the device tree, * calibrate it by ourselves. */ - if (!uc_priv->clock_rate) { + if (!gd->arch.clock_rate) { unsigned long fast_calibrate;
fast_calibrate = cpu_mhz_from_msr(); @@ -348,12 +348,32 @@ static int tsc_timer_probe(struct udevice *dev) panic("TSC frequency is ZERO"); }
- uc_priv->clock_rate = fast_calibrate * 1000000; + gd->arch.clock_rate = fast_calibrate * 1000000; } +} + +static int tsc_timer_probe(struct udevice *dev) +{ + struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev); + + tsc_timer_ensure_setup(); + uc_priv->clock_rate = gd->arch.clock_rate;
return 0; }
+unsigned long notrace timer_early_get_rate(void) +{ + tsc_timer_ensure_setup(); + + return gd->arch.clock_rate; +} + +u64 notrace timer_early_get_count(void) +{ + return rdtsc() - gd->arch.tsc_base; +} + static const struct timer_ops tsc_timer_ops = { .get_count = tsc_timer_get_count, };
n Sun, Aug 27, 2017 at 11:23 PM, Simon Glass sjg@chromium.org wrote:
With bootstage we need access to the timer before driver model is set up. To handle this, put the required state in global_data and provide a new function to set up the device, separate from the driver's probe() method.
This will be used by the 'early' timer also.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Update to support the early timer
arch/x86/include/asm/global_data.h | 1 + drivers/timer/tsc_timer.c | 30 +++++++++++++++++++++++++----- 2 files changed, 26 insertions(+), 5 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Use the new separate init function so that we can make use of the timer before driver model is started up.
At some point we should consider adding the microsecond timer to the timer uclass interface since it would reduce the amount of plumbing here slightly.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Rebase on top of early timer code and simplify slightly
drivers/timer/tsc_timer.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/timer/tsc_timer.c b/drivers/timer/tsc_timer.c index 0012fecde09..db49de87a20 100644 --- a/drivers/timer/tsc_timer.c +++ b/drivers/timer/tsc_timer.c @@ -295,11 +295,6 @@ ulong notrace timer_get_us(void) return get_ticks() / get_tbclk_mhz(); }
-ulong timer_get_boot_us(void) -{ - return timer_get_us(); -} - void __udelay(unsigned long usec) { u64 now = get_ticks(); @@ -374,6 +369,20 @@ u64 notrace timer_early_get_count(void) return rdtsc() - gd->arch.tsc_base; }
+ulong timer_get_boot_us(void) +{ + if (!gd->timer) { + u64 now_tick; + + tsc_timer_ensure_setup(); + now_tick = rdtsc() - gd->arch.tsc_base; + + return now_tick / (gd->arch.clock_rate / 1000000); + } + + return timer_get_us(); +} + static const struct timer_ops tsc_timer_ops = { .get_count = tsc_timer_get_count, };
Hi Simon,
On Sun, Aug 27, 2017 at 11:23 PM, Simon Glass sjg@chromium.org wrote:
Use the new separate init function so that we can make use of the timer before driver model is started up.
At some point we should consider adding the microsecond timer to the timer uclass interface since it would reduce the amount of plumbing here slightly.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Rebase on top of early timer code and simplify slightly
drivers/timer/tsc_timer.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)
I don't think this patch is needed. The early timer support was already provided in the patch [2/6] in this series.
Regards, Bin
Hi Bin,
On 27 August 2017 at 23:18, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sun, Aug 27, 2017 at 11:23 PM, Simon Glass sjg@chromium.org wrote:
Use the new separate init function so that we can make use of the timer before driver model is started up.
At some point we should consider adding the microsecond timer to the timer uclass interface since it would reduce the amount of plumbing here slightly.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Rebase on top of early timer code and simplify slightly
drivers/timer/tsc_timer.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)
I don't think this patch is needed. The early timer support was already provided in the patch [2/6] in this series.
The current version of timer_get_boot_us() does not init the timer if it is not set up, so will return incorrect values at the start.
So I think this patch is needed.
Regards, Simon
Hi Simon,
On Wed, Sep 6, 2017 at 9:39 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 27 August 2017 at 23:18, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sun, Aug 27, 2017 at 11:23 PM, Simon Glass sjg@chromium.org wrote:
Use the new separate init function so that we can make use of the timer before driver model is started up.
At some point we should consider adding the microsecond timer to the timer uclass interface since it would reduce the amount of plumbing here slightly.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Rebase on top of early timer code and simplify slightly
drivers/timer/tsc_timer.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)
I don't think this patch is needed. The early timer support was already provided in the patch [2/6] in this series.
The current version of timer_get_boot_us() does not init the timer if it is not set up, so will return incorrect values at the start.
timer_get_boot_us() in tsc_timer.c calls timer_get_us():
ulong notrace timer_get_us(void) { return get_ticks() / get_tbclk_mhz(); }
get_ticks() will call timer_early_get_count() if CONFIG_TIMER_EARLY is on.
Am I missing anything? Isn't it still broken when CONFIG_TIMER_EARLY is on?
So I think this patch is needed.
Regards, Bin
Hi Bin,
On 12 September 2017 at 07:47, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Wed, Sep 6, 2017 at 9:39 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 27 August 2017 at 23:18, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sun, Aug 27, 2017 at 11:23 PM, Simon Glass sjg@chromium.org wrote:
Use the new separate init function so that we can make use of the timer before driver model is started up.
At some point we should consider adding the microsecond timer to the timer uclass interface since it would reduce the amount of plumbing here slightly.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Rebase on top of early timer code and simplify slightly
drivers/timer/tsc_timer.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)
I don't think this patch is needed. The early timer support was already provided in the patch [2/6] in this series.
The current version of timer_get_boot_us() does not init the timer if it is not set up, so will return incorrect values at the start.
timer_get_boot_us() in tsc_timer.c calls timer_get_us():
ulong notrace timer_get_us(void) { return get_ticks() / get_tbclk_mhz(); }
get_ticks() will call timer_early_get_count() if CONFIG_TIMER_EARLY is on.
Am I missing anything? Isn't it still broken when CONFIG_TIMER_EARLY is on?
So I think this patch is needed.
We don't subtract the base time in that version, but in fact we don't correctly do that in timer_get_boot_us() either. So yes, let's drop this patch from the series. It still works without it, and we are in no worse position.
Regards, Simon
The CONFIG_BOOTSTAGE_USER_COUNT option is no-longer needed since we can now support any number of user IDs. Also BOOTSTAGE_ID_COUNT is not needed now.
Drop these unused options.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: None
common/Kconfig | 9 --------- common/bootstage.c | 2 +- configs/sandbox_defconfig | 1 - configs/sandbox_flattree_defconfig | 1 - configs/sandbox_noblk_defconfig | 1 - configs/sandbox_spl_defconfig | 1 - include/bootstage.h | 6 ------ 7 files changed, 1 insertion(+), 20 deletions(-)
diff --git a/common/Kconfig b/common/Kconfig index 4d8cae96109..abd4146922e 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -46,15 +46,6 @@ config BOOTSTAGE_REPORT 29,916,167 26,005,792 bootm_start 30,361,327 445,160 start_kernel
-config BOOTSTAGE_USER_COUNT - int "Number of boot ID numbers available for user use" - default 20 - help - This is the number of available user bootstage records. - Each time you call bootstage_mark(BOOTSTAGE_ID_ALLOC, ...) - a new ID will be allocated from this stash. If you exceed - the limit, recording will stop. - config BOOTSTAGE_RECORD_COUNT int "Number of boot stage records to store" default 30 diff --git a/common/bootstage.c b/common/bootstage.c index 61479d7f079..efc99fc6810 100644 --- a/common/bootstage.c +++ b/common/bootstage.c @@ -456,7 +456,7 @@ int bootstage_unstash(const void *base, int size)
if (data->rec_count + hdr->count > RECORD_COUNT) { debug("%s: Bootstage has %d records, we have space for %d\n" - "- please increase CONFIG_BOOTSTAGE_USER_COUNT\n", + "- please increase CONFIG_BOOTSTAGE_RECORD_COUNT\n", __func__, hdr->count, RECORD_COUNT - data->rec_count); return -ENOSPC; } diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 070262fe868..4e145e83bba 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -6,7 +6,6 @@ CONFIG_FIT_SIGNATURE=y CONFIG_FIT_VERBOSE=y CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y -CONFIG_BOOTSTAGE_USER_COUNT=32 CONFIG_BOOTSTAGE_FDT=y CONFIG_BOOTSTAGE_STASH=y CONFIG_BOOTSTAGE_STASH_ADDR=0x0 diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig index 1eecf1eb9d9..124b363662a 100644 --- a/configs/sandbox_flattree_defconfig +++ b/configs/sandbox_flattree_defconfig @@ -6,7 +6,6 @@ CONFIG_FIT_SIGNATURE=y CONFIG_FIT_VERBOSE=y CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y -CONFIG_BOOTSTAGE_USER_COUNT=32 CONFIG_BOOTSTAGE_FDT=y CONFIG_BOOTSTAGE_STASH=y CONFIG_BOOTSTAGE_STASH_ADDR=0x0 diff --git a/configs/sandbox_noblk_defconfig b/configs/sandbox_noblk_defconfig index 612aed50c5a..866e5c84ec4 100644 --- a/configs/sandbox_noblk_defconfig +++ b/configs/sandbox_noblk_defconfig @@ -5,7 +5,6 @@ CONFIG_FIT_SIGNATURE=y CONFIG_FIT_VERBOSE=y CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y -CONFIG_BOOTSTAGE_USER_COUNT=32 CONFIG_BOOTSTAGE_FDT=y CONFIG_BOOTSTAGE_STASH=y CONFIG_BOOTSTAGE_STASH_ADDR=0x0 diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index d1ccdfe9d38..6620c1d12c0 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -12,7 +12,6 @@ CONFIG_FIT_VERBOSE=y CONFIG_SPL_LOAD_FIT=y CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y -CONFIG_BOOTSTAGE_USER_COUNT=32 CONFIG_BOOTSTAGE_FDT=y CONFIG_BOOTSTAGE_STASH=y CONFIG_BOOTSTAGE_STASH_ADDR=0x0 diff --git a/include/bootstage.h b/include/bootstage.h index c5d93f57fd7..7a524782bae 100644 --- a/include/bootstage.h +++ b/include/bootstage.h @@ -12,11 +12,6 @@ #ifndef _BOOTSTAGE_H #define _BOOTSTAGE_H
-/* Define this for host tools */ -#ifndef CONFIG_BOOTSTAGE_USER_COUNT -#define CONFIG_BOOTSTAGE_USER_COUNT 20 -#endif - /* Flags for each bootstage record */ enum bootstage_flags { BOOTSTAGEF_ERROR = 1 << 0, /* Error record */ @@ -208,7 +203,6 @@ enum bootstage_id {
/* a few spare for the user, from here */ BOOTSTAGE_ID_USER, - BOOTSTAGE_ID_COUNT = BOOTSTAGE_ID_USER + CONFIG_BOOTSTAGE_USER_COUNT, BOOTSTAGE_ID_ALLOC, };
On Sun, Aug 27, 2017 at 11:23 PM, Simon Glass sjg@chromium.org wrote:
The CONFIG_BOOTSTAGE_USER_COUNT option is no-longer needed since we can now support any number of user IDs. Also BOOTSTAGE_ID_COUNT is not needed now.
Drop these unused options.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2: None
common/Kconfig | 9 --------- common/bootstage.c | 2 +- configs/sandbox_defconfig | 1 - configs/sandbox_flattree_defconfig | 1 - configs/sandbox_noblk_defconfig | 1 - configs/sandbox_spl_defconfig | 1 - include/bootstage.h | 6 ------ 7 files changed, 1 insertion(+), 20 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
With SPL we often have limited memory and do not need very many bootstage records. Add a separate Kconfig option for SPL.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: None
common/Kconfig | 7 +++++++ common/bootstage.c | 6 +++--- 2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/common/Kconfig b/common/Kconfig index abd4146922e..540cc9999bc 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -53,6 +53,13 @@ config BOOTSTAGE_RECORD_COUNT This is the size of the bootstage record list and is the maximum number of bootstage records that can be recorded.
+config SPL_BOOTSTAGE_RECORD_COUNT + int "Number of boot stage records to store for SPL" + default 5 + help + This is the size of the bootstage record list and is the maximum + number of bootstage records that can be recorded. + config BOOTSTAGE_FDT bool "Store boot timing information in the OS device tree" depends on BOOTSTAGE diff --git a/common/bootstage.c b/common/bootstage.c index efc99fc6810..b866e66979e 100644 --- a/common/bootstage.c +++ b/common/bootstage.c @@ -18,7 +18,7 @@ DECLARE_GLOBAL_DATA_PTR;
enum { - RECORD_COUNT = CONFIG_BOOTSTAGE_RECORD_COUNT, + RECORD_COUNT = CONFIG_VAL(BOOTSTAGE_RECORD_COUNT), };
struct bootstage_record { @@ -327,7 +327,7 @@ void bootstage_report(void) } if (data->rec_count > RECORD_COUNT) printf("Overflowed internal boot id table by %d entries\n" - "- please increase CONFIG_BOOTSTAGE_RECORD_COUNT\n", + "Please increase CONFIG_(SPL_)BOOTSTAGE_RECORD_COUNT\n", data->rec_count - RECORD_COUNT);
puts("\nAccumulated time:\n"); @@ -456,7 +456,7 @@ int bootstage_unstash(const void *base, int size)
if (data->rec_count + hdr->count > RECORD_COUNT) { debug("%s: Bootstage has %d records, we have space for %d\n" - "- please increase CONFIG_BOOTSTAGE_RECORD_COUNT\n", + "Please increase CONFIG_(SPL_)BOOTSTAGE_RECORD_COUNT\n", __func__, hdr->count, RECORD_COUNT - data->rec_count); return -ENOSPC; }
On Sun, Aug 27, 2017 at 11:23 PM, Simon Glass sjg@chromium.org wrote:
With SPL we often have limited memory and do not need very many bootstage records. Add a separate Kconfig option for SPL.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2: None
common/Kconfig | 7 +++++++ common/bootstage.c | 6 +++--- 2 files changed, 10 insertions(+), 3 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Enable this option for link so that the timer is available earlier.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Add new patch to enable early timer for chromebook_link
configs/chromebook_link64_defconfig | 1 + configs/chromebook_link_defconfig | 1 + 2 files changed, 2 insertions(+)
diff --git a/configs/chromebook_link64_defconfig b/configs/chromebook_link64_defconfig index f058e560ef3..c3d2d120edb 100644 --- a/configs/chromebook_link64_defconfig +++ b/configs/chromebook_link64_defconfig @@ -68,6 +68,7 @@ CONFIG_DEBUG_UART_CLOCK=1843200 CONFIG_DEBUG_UART_BOARD_INIT=y CONFIG_SYS_NS16550=y CONFIG_SPL_TIMER=y +CONFIG_TIMER_EARLY=y CONFIG_TPM_TIS_LPC=y CONFIG_USB_STORAGE=y CONFIG_USB_KEYBOARD=y diff --git a/configs/chromebook_link_defconfig b/configs/chromebook_link_defconfig index 7fcddba50c8..5110bc44183 100644 --- a/configs/chromebook_link_defconfig +++ b/configs/chromebook_link_defconfig @@ -50,6 +50,7 @@ CONFIG_DEBUG_UART_BASE=0x3f8 CONFIG_DEBUG_UART_CLOCK=1843200 CONFIG_DEBUG_UART_BOARD_INIT=y CONFIG_SYS_NS16550=y +CONFIG_TIMER_EARLY=y CONFIG_TPM_TIS_LPC=y CONFIG_USB_STORAGE=y CONFIG_USB_KEYBOARD=y
On Sun, Aug 27, 2017 at 11:23 PM, Simon Glass sjg@chromium.org wrote:
Enable this option for link so that the timer is available earlier.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Add new patch to enable early timer for chromebook_link
configs/chromebook_link64_defconfig | 1 + configs/chromebook_link_defconfig | 1 + 2 files changed, 2 insertions(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
participants (2)
-
Bin Meng -
Simon Glass