Skip to contentSkip to conversation

ARM: beaglebone: add delay in lowlevel.c

PATCH Applied Konstantin Kletschke 4 participants 10 messages Jan 7, 2025 – Jan 10, 2025

Subsystems (2)
Arm (32-bit) Arm/actions semi architecture
lore
Share & tools
lore

Konstantin Kletschke

author
#0 lore raw

PATCH 1/1 ARM: beaglebone: add delay in lowlevel.c

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 <[email protected]>
---
 arch/arm/boards/beaglebone/lowlevel.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/arm/boards/beaglebone/lowlevel.c +7 -0
index 5dc49dfaaf..ccae1d1122 100644
@@ -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);
 }
#1 lore raw

Hi Konsti,

Quoted 30 lines · 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 <[email protected]>
---
 arch/arm/boards/beaglebone/lowlevel.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/arm/boards/beaglebone/lowlevel.c +6 -0
index 5dc49dfaaf..ccae1d1122 100644
@@ -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.
  *

arch/arm/boards/beaglebone/lowlevel.c static noinline int beaglebone_sram_init(void) · line 144

Quoted 4 lines
arch/arm/boards/beaglebone/lowlevel.c +1 -0
@@ -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

Signature · 4 lines
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 |
author
#2 lore raw

Hello Sascha,

Quoted 2 lines · Sascha Hauer

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

Signature · 14 lines
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

[email protected]
http://www.inside-m2m.de 

Geschäftsführung: Michael Emmert, Derek Uhlig
HRB: 111204, AG Hannover
#3 lore raw
Quoted 10 lines · Konstantin Kletschke

Hello Sascha,

Quoted 2 lines · Sascha Hauer

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.

Quoted 7 lines

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.

Quoted 5 lines

Regards
Konsti

Signature · 4 lines
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 |
author
#4 lore raw

More like this?

Quoted 3 lines · Ahmad Fatoum

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(+)
arch/arm/boards/beaglebone/lowlevel.c +12 -0
index 5dc49dfaaf..7dc338d03a 100644
@@ -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);
 }
#5 lore raw
Quoted 28 lines · Konstantin Kletschke

More like this?

Quoted 3 lines · Ahmad Fatoum

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(+)
arch/arm/boards/beaglebone/lowlevel.c +6 -0
index 5dc49dfaaf..7dc338d03a 100644
@@ -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.
  *

arch/arm/boards/beaglebone/lowlevel.c static noinline int beaglebone_sram_init(void) · line 147

Quoted 7 lines
arch/arm/boards/beaglebone/lowlevel.c +4 -0
@@ -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

Quoted 10 lines
arch/arm/boards/beaglebone/lowlevel.c +2 -0
+        */
+       __udelay(3000);
 
        barebox_arm_entry(0x80000000, sdram_size, fdt);
 }
Signature · 4 lines
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 |
author
#6 lore raw
Quoted 3 lines · Ahmad Fatoum

Sounds good. Please add

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

All right, sent [PATCH] mail again.

Signature · 14 lines
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

[email protected]
http://www.inside-m2m.de 

Geschäftsführung: Michael Emmert, Derek Uhlig
HRB: 111204, AG Hannover
#7 lore raw
In reply to Konstantin Kletschke

Hi Konstantin,

Am Mittwoch, dem 08.01.2025 um 16:14 +0100 schrieb Konstantin Kletschke:

Quoted 10 lines

Hello Sascha,

Quoted 2 lines · Sascha Hauer

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.

Quoted 7 lines

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

author
#8 lore raw

Hi Lucas,

this is an interesting idea and I did poke around a bit.

Quoted 4 lines · Lucas Stach

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.

I inserted this into the delay loop:

static void __udelay(int us)

{
    volatile int i;
        for (i = 0; i < us; i++) {
                (__raw_readl(CM_IDLEST_DPLL_MPU) != 0x1) ? putc_ll('m') : (void)0;
                (__raw_readl(CM_IDLEST_DPLL_CORE) != 0x1) ? putc_ll('c') : (void)0;
                (__raw_readl(CM_IDLEST_DPLL_PER) != 0x1) ? putc_ll('p') : (void)0;
                (__raw_readl(CM_IDLEST_DPLL_DDR) != 0x1) ? putc_ll('d') : (void)0;
        }
}

but none was triggered. Additionaly I tried this instead of the above before entering the delay loop in beaglebone_sram_init(void):

        while(__raw_readl(CM_IDLEST_DPLL_DDR) != 0x0);
        //while(__raw_readl(CM_IDLEST_DPLL_CORE) != 0x0);
        //while(__raw_readl(CM_IDLEST_DPLL_PER) != 0x0);
        //while(__raw_readl(CM_IDLEST_DPLL_MPU) != 0x0);
        __udelay(3000);

When I suspect the bit being stable I expect to be being trapped so I booted with a version without this, replaced it with the trap and rebooted, I did this 4 times for each DDR, CORE, PER and MPU. If the bit toggles back to zero I would exit the while, delay and boot normal. So if trapped == no bit toggle.

Either the putc_ll inserted into the loop did not appear nor the while trap was left in any case so I suspect this bit is stable at 0x1 for each type of PLL.

Kind Regards
Konsti

Signature · 14 lines
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

[email protected]
http://www.inside-m2m.de 

Geschäftsführung: Michael Emmert, Derek Uhlig
HRB: 111204, AG Hannover
Applied
#9 lore raw
In reply to Konstantin Kletschke
Quoted 8 lines · 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.

[...]

Applied, thanks!

[1/1] ARM: beaglebone: add delay in lowlevel.c
https://git.pengutronix.de/cgit/barebox/commit/?id=e6f598d9ceff (link may not be stable)

Best regards,

Signature · 1 line
Sascha Hauer <[email protected]>