[U-Boot] [PATCH 0/5] x86: bootstage: Fix bootstage operation on link

[U-Boot] [PATCH 0/5] x86: bootstage: Fix bootstage operation on link

[Simon Glass]

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(-)

-- 
2.13.2.932.g7449e964c-goog

[U-Boot] [PATCH 1/5] board_f: Drop the timer after relocation

[Simon Glass]

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.
-- 
2.13.2.932.g7449e964c-goog

[U-Boot] [PATCH 1/5] board_f: Drop the timer after relocation

[Bin Meng]

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

[U-Boot] [PATCH 1/5] board_f: Drop the timer after relocation

[Simon Glass]

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

[U-Boot] [PATCH 2/5] dm: x86: Allow TSC timer to be used before DM is ready

[Simon Glass]

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;
 }
-- 
2.13.2.932.g7449e964c-goog

[U-Boot] [PATCH 2/5] dm: x86: Allow TSC timer to be used before DM is ready

[Bin Meng]

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

[U-Boot] [PATCH 2/5] dm: x86: Allow TSC timer to be used before DM is ready

[Simon Glass]

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

[U-Boot] [PATCH 3/5] dm: x86: Update timer_get_boot_us to work before DM is ready

[Simon Glass]

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,
 };
-- 
2.13.2.932.g7449e964c-goog

[U-Boot] [PATCH 4/5] bootstage: Drop unused options

[Simon Glass]

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,
 };
 
-- 
2.13.2.932.g7449e964c-goog

[U-Boot] [PATCH 4/5] bootstage: Drop unused options

[Bin Meng]

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>

[U-Boot] [PATCH 5/5] bootstage: Provide a separate record count setting for SPL

[Simon Glass]

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;
 	}
-- 
2.13.2.932.g7449e964c-goog

[U-Boot] [PATCH 5/5] bootstage: Provide a separate record count setting for SPL

[Bin Meng]

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>