[PATCH] ARM: beaglebone: add delay in lowlevel.c

[PATCH] ARM: beaglebone: add delay in lowlevel.c

[Konstantin Kletschke]

Some Beaglebone Black devices are not able to cope with a warm start.
When system is powered up and booted hitting reset button (S1) or
issuing "reset" at barebox prompt the new instance of first stage gets
stuck after PLL init.

Adding a delay of ~1.8ms in lowlevel.c after PLL init solves this.

Link: https://lore.barebox.org/barebox/Z0gywL2hLcIDoLQ8@hephaistos/

Signed-off-by: Konstantin Kletschke <konstantin.kletschke at inside-m2m.de>

--- 
 arch/arm/boards/beaglebone/lowlevel.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boards/beaglebone/lowlevel.c b/arch/arm/boards/beaglebone/lowlevel.c
index 5dc49dfaaf..7dc338d03a 100644
--- a/arch/arm/boards/beaglebone/lowlevel.c
+++ b/arch/arm/boards/beaglebone/lowlevel.c
@@ -97,6 +97,12 @@ extern char __dtb_z_am335x_boneblack_start[];
 extern char __dtb_z_am335x_bone_common_start[];
 extern char __dtb_z_am335x_bone_start[];

+static void __udelay(int us)
+{
+        volatile int i;
+       for (i = 0; i < us; i++);
+}
+
 /**
  * @brief The basic entry point for board initialization.
  *
@@ -135,6 +141,12 @@ static noinline int beaglebone_sram_init(void)
        am33xx_enable_uart0_pin_mux();
        omap_debug_ll_init();
        putc_ll('>');
+       /*
+        * Some (~5%) Beaglebone Black received from SEEED in batches
+        * after autumn 2024 require a delay to be able to warm start
+        * after reset
+        */
+       __udelay(3000);

        barebox_arm_entry(0x80000000, sdram_size, fdt);
 }
-- 

[PATCH] ARM: beaglebone: add delay in lowlevel.c

[Konstantin Kletschke]

Some Beaglebone Black devices are not able to cope with a warm start.
When system is powered up and booted hitting reset button (S1) or
issuing "reset" at barebox prompt the new instance of first stage gets
stuck after PLL init.

Adding a delay of ~1.8ms in lowlevel.c after PLL init solves this.

Signed-off-by: Konstantin Kletschke <konstantin.kletschke@inside-m2m.de>
---
 arch/arm/boards/beaglebone/lowlevel.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boards/beaglebone/lowlevel.c b/arch/arm/boards/beaglebone/lowlevel.c
index 5dc49dfaaf..ccae1d1122 100644
--- a/arch/arm/boards/beaglebone/lowlevel.c
+++ b/arch/arm/boards/beaglebone/lowlevel.c
@@ -97,6 +97,12 @@ extern char __dtb_z_am335x_boneblack_start[];
 extern char __dtb_z_am335x_bone_common_start[];
 extern char __dtb_z_am335x_bone_start[];

+static void __udelay(int us)
+{
+        volatile int i;
+       for (i = 0; i < us; i++);
+}
+
 /**
  * @brief The basic entry point for board initialization.
  *
@@ -135,6 +141,7 @@ static noinline int beaglebone_sram_init(void)
        am33xx_enable_uart0_pin_mux();
        omap_debug_ll_init();
        putc_ll('>');
+       __udelay(3000); // Fix for Beaglebone Black warm start after reset

        barebox_arm_entry(0x80000000, sdram_size, fdt);
 }

-- 

[PATCH] ARM: beaglebone: add delay in lowlevel.c

[Konstantin Kletschke]

Some Beaglebone Black devices are not able to cope with a warm start.
When system is powered up and booted hitting reset button (S1) or
issuing "reset" at barebox prompt the new instance of first stage gets
stuck after PLL init.

Adding a delay of ~1.8ms in lowlevel.c after PLL init solves this.

Signed-off-by: Konstantin Kletschke <konstantin.kletschke@inside-m2m.de>
---
 arch/arm/boards/beaglebone/lowlevel.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boards/beaglebone/lowlevel.c b/arch/arm/boards/beaglebone/lowlevel.c
index 5dc49dfaaf..ccae1d1122 100644
--- a/arch/arm/boards/beaglebone/lowlevel.c
+++ b/arch/arm/boards/beaglebone/lowlevel.c
@@ -97,6 +97,12 @@ extern char __dtb_z_am335x_boneblack_start[];
 extern char __dtb_z_am335x_bone_common_start[];
 extern char __dtb_z_am335x_bone_start[];

+static void __udelay(int us)
+{
+        volatile int i;
+       for (i = 0; i < us * 3; i++);
+}
+
 /**
  * @brief The basic entry point for board initialization.
  *
@@ -135,6 +141,7 @@ static noinline int beaglebone_sram_init(void)
        am33xx_enable_uart0_pin_mux();
        omap_debug_ll_init();
        putc_ll('>');
+       __udelay(1000); // Adding delay of 1.8ms

        barebox_arm_entry(0x80000000, sdram_size, fdt);
 }

-- 

Re: [PATCH] ARM: beaglebone: add delay in lowlevel.c

[Sascha Hauer]

Hi Konsti,

On Tue, Jan 07, 2025 at 04:01:58PM +0100, Konstantin Kletschke wrote:
> Some Beaglebone Black devices are not able to cope with a warm start.
> When system is powered up and booted hitting reset button (S1) or
> issuing "reset" at barebox prompt the new instance of first stage gets
> stuck after PLL init.
> 
> Adding a delay of ~1.8ms in lowlevel.c after PLL init solves this.
> 
> Signed-off-by: Konstantin Kletschke <konstantin.kletschke@inside-m2m.de>
> ---
>  arch/arm/boards/beaglebone/lowlevel.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm/boards/beaglebone/lowlevel.c b/arch/arm/boards/beaglebone/lowlevel.c
> index 5dc49dfaaf..ccae1d1122 100644
> --- a/arch/arm/boards/beaglebone/lowlevel.c
> +++ b/arch/arm/boards/beaglebone/lowlevel.c
> @@ -97,6 +97,12 @@ extern char __dtb_z_am335x_boneblack_start[];
>  extern char __dtb_z_am335x_bone_common_start[];
>  extern char __dtb_z_am335x_bone_start[];
> 
> +static void __udelay(int us)
> +{
> +        volatile int i;
> +       for (i = 0; i < us * 3; i++);
> +}
> +
>  /**
>   * @brief The basic entry point for board initialization.
>   *
> @@ -135,6 +141,7 @@ static noinline int beaglebone_sram_init(void)
>         am33xx_enable_uart0_pin_mux();
>         omap_debug_ll_init();
>         putc_ll('>');
> +       __udelay(1000); // Adding delay of 1.8ms

Calling udelay(1000) and adding a comment saying it delays 1.8ms looks
inconsistent. Maybe better count up to 2 in __udelay() above which makes
it more accurate and then call __udelay(2000) if necessary.

Drop the comment, because it's obvious what the delay does. Better add a
comment why you are doing so (i.e. copy parts of the commit message as
comment).

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] ARM: beaglebone: add delay in lowlevel.c

[Konstantin Kletschke]

Hello Sascha,

On Wed, Jan 08, 2025 at 03:32:04PM +0100, Sascha Hauer wrote:

> Calling udelay(1000) and adding a comment saying it delays 1.8ms looks
> inconsistent. Maybe better count up to 2 in __udelay() above which makes

I completely agree somehow since the time is not even constant and depends on
the PLL settings just done before.

I suggest the following:

Removing the "* 3" fancy thingy in the function's loop, calling the
function with 3000 instead of 1000, changing comment to "needed on some
Beaglebone Black for warm start after reset".

That would be as simple as possible.

Regards
Konsti


-- 
INSIDE M2M GmbH
Konstantin Kletschke
Berenbosteler Straße 76 B
30823 Garbsen

Telefon: +49 (0) 5137 90950136
Mobil: +49 (0) 151 15256238
Fax: +49 (0) 5137 9095010

konstantin.kletschke@inside-m2m.de
http://www.inside-m2m.de 

Geschäftsführung: Michael Emmert, Derek Uhlig
HRB: 111204, AG Hannover

Re: [PATCH] ARM: beaglebone: add delay in lowlevel.c

[Ahmad Fatoum]

On 08.01.25 16:14, Konstantin Kletschke wrote:
> Hello Sascha,
> 
> On Wed, Jan 08, 2025 at 03:32:04PM +0100, Sascha Hauer wrote:
> 
>> Calling udelay(1000) and adding a comment saying it delays 1.8ms looks
>> inconsistent. Maybe better count up to 2 in __udelay() above which makes
> 
> I completely agree somehow since the time is not even constant and depends on
> the PLL settings just done before.
> 
> I suggest the following:
> 
> Removing the "* 3" fancy thingy in the function's loop, calling the
> function with 3000 instead of 1000, changing comment to "needed on some
> Beaglebone Black for warm start after reset".
> 
> That would be as simple as possible.

It would be nice to have the comment more verbose. e.g. that this issue
was observed on new Seeed-produced BBBs is an intersting info IMO.

> 
> Regards
> Konsti
> 
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] ARM: beaglebone: add delay in lowlevel.c

[Konstantin Kletschke]

More like this?

On Wed, Jan 08, 2025 at 04:19:49PM +0100, Ahmad Fatoum wrote:
> 
> It would be nice to have the comment more verbose. e.g. that this issue
> was observed on new Seeed-produced BBBs is an intersting info IMO.

 arch/arm/boards/beaglebone/lowlevel.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boards/beaglebone/lowlevel.c b/arch/arm/boards/beaglebone/lowlevel.c
index 5dc49dfaaf..7dc338d03a 100644
--- a/arch/arm/boards/beaglebone/lowlevel.c
+++ b/arch/arm/boards/beaglebone/lowlevel.c
@@ -97,6 +97,12 @@ extern char __dtb_z_am335x_boneblack_start[];
 extern char __dtb_z_am335x_bone_common_start[];
 extern char __dtb_z_am335x_bone_start[];

+static void __udelay(int us)
+{
+        volatile int i;
+       for (i = 0; i < us; i++);
+}
+
 /**
  * @brief The basic entry point for board initialization.
  *
@@ -135,6 +141,12 @@ static noinline int beaglebone_sram_init(void)
        am33xx_enable_uart0_pin_mux();
        omap_debug_ll_init();
        putc_ll('>');
+       /*
+        * Some (~5%) Beaglebone Black received from SEEED in batches
+        * after autumn 2024 require a delay to be able to warm start
+        * after reset
+        */
+       __udelay(3000);

        barebox_arm_entry(0x80000000, sdram_size, fdt);
 }

Re: [PATCH] ARM: beaglebone: add delay in lowlevel.c

[Ahmad Fatoum]

On 08.01.25 16:27, Konstantin Kletschke wrote:
> More like this?
> 
> On Wed, Jan 08, 2025 at 04:19:49PM +0100, Ahmad Fatoum wrote:
>>
>> It would be nice to have the comment more verbose. e.g. that this issue
>> was observed on new Seeed-produced BBBs is an intersting info IMO.
> 
>  arch/arm/boards/beaglebone/lowlevel.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/arm/boards/beaglebone/lowlevel.c b/arch/arm/boards/beaglebone/lowlevel.c
> index 5dc49dfaaf..7dc338d03a 100644
> --- a/arch/arm/boards/beaglebone/lowlevel.c
> +++ b/arch/arm/boards/beaglebone/lowlevel.c
> @@ -97,6 +97,12 @@ extern char __dtb_z_am335x_boneblack_start[];
>  extern char __dtb_z_am335x_bone_common_start[];
>  extern char __dtb_z_am335x_bone_start[];
> 
> +static void __udelay(int us)
> +{
> +        volatile int i;
> +       for (i = 0; i < us; i++);
> +}
> +
>  /**
>   * @brief The basic entry point for board initialization.
>   *
> @@ -135,6 +141,12 @@ static noinline int beaglebone_sram_init(void)
>         am33xx_enable_uart0_pin_mux();
>         omap_debug_ll_init();
>         putc_ll('>');
> +       /*
> +        * Some (~5%) Beaglebone Black received from SEEED in batches
> +        * after autumn 2024 require a delay to be able to warm start
> +        * after reset

Sounds good. Please add

Link: https://lore.barebox.org/barebox/Z0gywL2hLcIDoLQ8@hephaistos/

to your commit message before the S-o-b and then it's good to go, I think.

Thanks,
Ahmad

> +        */
> +       __udelay(3000);
> 
>         barebox_arm_entry(0x80000000, sdram_size, fdt);
>  }
> 
> 
> 
> 
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] ARM: beaglebone: add delay in lowlevel.c

[Konstantin Kletschke]

On Wed, Jan 08, 2025 at 04:31:40PM +0100, Ahmad Fatoum wrote:

> Sounds good. Please add
> 
> Link: https://lore.barebox.org/barebox/Z0gywL2hLcIDoLQ8@hephaistos/

All right, sent [PATCH] mail again.

-- 
INSIDE M2M GmbH
Konstantin Kletschke
Berenbosteler Straße 76 B
30823 Garbsen

Telefon: +49 (0) 5137 90950136
Mobil: +49 (0) 151 15256238
Fax: +49 (0) 5137 9095010

konstantin.kletschke@inside-m2m.de
http://www.inside-m2m.de 

Geschäftsführung: Michael Emmert, Derek Uhlig
HRB: 111204, AG Hannover

Re: [PATCH] ARM: beaglebone: add delay in lowlevel.c

[Lucas Stach]

Hi Konstantin,

Am Mittwoch, dem 08.01.2025 um 16:14 +0100 schrieb Konstantin
Kletschke:
> Hello Sascha,
> 
> On Wed, Jan 08, 2025 at 03:32:04PM +0100, Sascha Hauer wrote:
> 
> > Calling udelay(1000) and adding a comment saying it delays 1.8ms looks
> > inconsistent. Maybe better count up to 2 in __udelay() above which makes
> 
> I completely agree somehow since the time is not even constant and depends on
> the PLL settings just done before.
> 
> I suggest the following:
> 
> Removing the "* 3" fancy thingy in the function's loop, calling the
> function with 3000 instead of 1000, changing comment to "needed on some
> Beaglebone Black for warm start after reset".
> 
> That would be as simple as possible.

It would be interesting to know if any of the configured PLLs go out of
lock again during the time your delay loop runs. I.e. check if any of
CM_IDLEST_DPLL_CORE, CM_IDLEST_DPLL_DDR, CM_IDLEST_DPLL_PER or
CM_IDLEST_DPLL_MPU contain a value other than 0x1.

Premature signaling of a PLL lock during warm reboot (where the PLL
isn't in bypass before the configuration) might well explain the issue.

Regards,
Lucas