[U-Boot] [PATCH 0/5] 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.
Simon Glass (5): 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
arch/x86/include/asm/global_data.h | 1 + common/Kconfig | 16 +++++++--------- common/board_f.c | 1 + common/bootstage.c | 6 +++--- configs/sandbox_defconfig | 1 - configs/sandbox_flattree_defconfig | 1 - configs/sandbox_noblk_defconfig | 1 - configs/sandbox_spl_defconfig | 1 - drivers/timer/tsc_timer.c | 38 ++++++++++++++++++++++++++++---------- include/bootstage.h | 6 ------ 10 files changed, 40 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 ---
common/board_f.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/common/board_f.c b/common/board_f.c index ffa84e3566..d675dc38ac 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -979,6 +979,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, Jul 16, 2017 at 7:41 AM, 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
common/board_f.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/common/board_f.c b/common/board_f.c index ffa84e3566..d675dc38ac 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -979,6 +979,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.
But there is already a config option CONFIG_TIMER_EARLY, and gd->timer is zeroed in initr_dm(). We should fix problems with existing CONFIG_TIMER_EARLY.
/* * U-Boot has been copied into SDRAM, the BSS has been cleared etc.--
Regards, Bin
Hi Bin,
On 22 July 2017 at 22:36, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sun, Jul 16, 2017 at 7:41 AM, 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
common/board_f.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/common/board_f.c b/common/board_f.c index ffa84e3566..d675dc38ac 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -979,6 +979,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.
Fixed in v2.
But there is already a config option CONFIG_TIMER_EARLY, and gd->timer is zeroed in initr_dm(). We should fix problems with existing CONFIG_TIMER_EARLY.
Yes I agree, although this patch is needed regardless, I think. We should not have invalid timers hanging around.
/* * U-Boot has been copied into SDRAM, the BSS has been cleared etc.--
Regards, Bin
Regards, Simon
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.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/include/asm/global_data.h | 1 + drivers/timer/tsc_timer.c | 18 +++++++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/arch/x86/include/asm/global_data.h b/arch/x86/include/asm/global_data.h index 93a80fe2b6..fcb6853a38 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 5c4ec0018f..9f1cfb1242 100644 --- a/drivers/timer/tsc_timer.c +++ b/drivers/timer/tsc_timer.c @@ -334,17 +334,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 = try_msr_calibrate_tsc(); @@ -354,8 +354,16 @@ 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; }
Hi Simon,
On Sun, Jul 16, 2017 at 7:41 AM, 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.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/include/asm/global_data.h | 1 + drivers/timer/tsc_timer.c | 18 +++++++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-)
Please check my comments against [1/5] in this series. We should be able to do something with existing CONFIG_TIMER_EARLY. Updating TSC timer can only solve this specific timer issue.
Regards, Bin
Hi Bin,
On 22 July 2017 at 22:36, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sun, Jul 16, 2017 at 7:41 AM, 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.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/include/asm/global_data.h | 1 + drivers/timer/tsc_timer.c | 18 +++++++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-)
Please check my comments against [1/5] in this series. We should be able to do something with existing CONFIG_TIMER_EARLY. Updating TSC timer can only solve this specific timer issue.
Yes I agree. I did wonder about that at the time, but didn't investigate it.
I'll take a look at this next week when I have access to a link.
Regards, Simon
Use the new separate init function so that we can make use of the timer before driver model is started up.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/timer/tsc_timer.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/timer/tsc_timer.c b/drivers/timer/tsc_timer.c index 9f1cfb1242..91e5cb11c1 100644 --- a/drivers/timer/tsc_timer.c +++ b/drivers/timer/tsc_timer.c @@ -301,11 +301,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(); @@ -368,6 +363,21 @@ static int tsc_timer_probe(struct udevice *dev) return 0; }
+ulong timer_get_boot_us(void) +{ + if (!gd->timer) { + u64 now_tick; + + tsc_timer_ensure_setup(); + now_tick = rdtsc(); + + now_tick -= 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, };
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 ---
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 361346b092..2b6a252d19 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 61479d7f07..efc99fc681 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 7a1b9ef052..7baba0a8d4 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 c5ef69f241..5f6157ff63 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 747d4b1bba..8865fb680b 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 6889206c1b..77f386ca6c 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 c5d93f57fd..7a524782ba 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, Jul 16, 2017 at 7:41 AM, 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
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 ---
common/Kconfig | 7 +++++++ common/bootstage.c | 6 +++--- 2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/common/Kconfig b/common/Kconfig index 2b6a252d19..7f22ea16c6 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 efc99fc681..b866e66979 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, Jul 16, 2017 at 7:41 AM, 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
common/Kconfig | 7 +++++++ common/bootstage.c | 6 +++--- 2 files changed, 10 insertions(+), 3 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
participants (2)
-
Bin Meng -
Simon Glass