[U-Boot] [PATCH] ARM: mxs: spl_boot.c: make early_delay more robust

It's true that booting normally doesn't take long enough for the register to roll (which actually happens in a little over an hour, not just a few seconds). However, the counter starts at power-on, and if the board is held in reset to be booted over USB, one actually risks hitting wrap-around during boot, which can both result in too short delays (if the "st += delay" calculation makes st small) and theoretically also unbound delays (if st ends up being UINT_MAX and one just misses sampling digctl_microseconds at that point).
It doesn't take more code to DTRT, and once bitten, twice shy.
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk --- arch/arm/cpu/arm926ejs/mxs/spl_boot.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_boot.c b/arch/arm/cpu/arm926ejs/mxs/spl_boot.c index cb361ac65c..336266fe82 100644 --- a/arch/arm/cpu/arm926ejs/mxs/spl_boot.c +++ b/arch/arm/cpu/arm926ejs/mxs/spl_boot.c @@ -24,9 +24,7 @@ static bd_t bdata __section(".data");
/* * This delay function is intended to be used only in early stage of boot, where - * clock are not set up yet. The timer used here is reset on every boot and - * takes a few seconds to roll. The boot doesn't take that long, so to keep the - * code simple, it doesn't take rolling into consideration. + * clock are not set up yet. */ void early_delay(int delay) { @@ -34,8 +32,7 @@ void early_delay(int delay) (struct mxs_digctl_regs *)MXS_DIGCTL_BASE;
uint32_t st = readl(&digctl_regs->hw_digctl_microseconds); - st += delay; - while (st > readl(&digctl_regs->hw_digctl_microseconds)) + while (readl(&digctl_regs->hw_digctl_microseconds) - st <= delay) ; }

[Adding Marek]
On Tue, Sep 10, 2019 at 5:32 AM Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
It's true that booting normally doesn't take long enough for the register to roll (which actually happens in a little over an hour, not just a few seconds). However, the counter starts at power-on, and if the board is held in reset to be booted over USB, one actually risks hitting wrap-around during boot, which can both result in too short delays (if the "st += delay" calculation makes st small) and theoretically also unbound delays (if st ends up being UINT_MAX and one just misses sampling digctl_microseconds at that point).
It doesn't take more code to DTRT, and once bitten, twice shy.
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
arch/arm/cpu/arm926ejs/mxs/spl_boot.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_boot.c b/arch/arm/cpu/arm926ejs/mxs/spl_boot.c index cb361ac65c..336266fe82 100644 --- a/arch/arm/cpu/arm926ejs/mxs/spl_boot.c +++ b/arch/arm/cpu/arm926ejs/mxs/spl_boot.c @@ -24,9 +24,7 @@ static bd_t bdata __section(".data");
/*
- This delay function is intended to be used only in early stage of boot, where
- clock are not set up yet. The timer used here is reset on every boot and
- takes a few seconds to roll. The boot doesn't take that long, so to keep the
- code simple, it doesn't take rolling into consideration.
*/
- clock are not set up yet.
void early_delay(int delay) { @@ -34,8 +32,7 @@ void early_delay(int delay) (struct mxs_digctl_regs *)MXS_DIGCTL_BASE;
uint32_t st = readl(&digctl_regs->hw_digctl_microseconds);
st += delay;
while (st > readl(&digctl_regs->hw_digctl_microseconds))
while (readl(&digctl_regs->hw_digctl_microseconds) - st <= delay) ;
Looks reasonable for me.
Thanks
participants (2)
-
Fabio Estevam
-
Rasmus Villemoes