When updating from arm-trusted-firmware-v2.4-stm32mp-r1 to arm-trusted-firmware-v2.4-stm32mp-r2 I get PANIC on UART

[cfilipescu]

I didn't change anything in the device tree just compiled the arm-trusted-firmware-v2.4-stm32mp-r2 instead of arm-trusted-firmware-v2.4-stm32mp-r1 and I get the following on UART:

PANIC at PC : 0x2ffcfd2b

What is different between the two versions?

[cfilipescu]

Hi @Bernard PUEL​,

Thank you for the feedback, unfortunately, it will be a while before I can't get to a scope to look at the signals. However, I could try adding the delay. Would you have some sample code where in the arm-trusted-firmware source code I should add the delay?

Thank you

[Bernard PUEL · Accepted solution]

You can do this simple test:

in tf-a dts (only in tf-a), remove this line: vmmc-supply = <&v3v3>;

It will remove the switch off/on of buck4 during sdmmc init.

Please let me know if it can solve the issue on your board. Dev team is thinking about a better correction .....

[cfilipescu]

NOTICE:  Model: STMicroelectronics custom STM32CubeMX board - openstlinux-5.10-dunfell-mp1-21-11-17
NOTICE:  BL2: v2.4-r2.0(release):2021.11
NOTICE:  BL2: Built : 14:21:43, Jan 25 2022
NOTICE:  BL2: Booting BL32
NOTICE:  SP_MIN: v2.4-r2.0(release):2021.11
NOTICE:  SP_MIN: Built : 14:21:43, Jan 25 2022

Awesome that worked and it booted.

Thank you for all the help.

[Bernard PUEL]

ok great. That way you will get the same behavior as for v3.0.

But the subject is not finished on ST side and I will propose probably a better correction in the coming days.

[Bernard PUEL]

After internal discussions, we will deploy a correction that will have the same effect as the previous solution. So you can go on with it on your side.

In case of v3v3 supply of the sdmmc, you cannot do a power cycle of the sdmmc at tf-a level (because you will switch off almost all the platform). But this is not an issue because the romcode has already done it before.

[cfilipescu]

Awesome. Thank you again.

[UVV]

It seems I stumbled upon this post too late when I already solved the issue a bit differently.

I will post my patch here, but I'm not sure it's a proper fix. Could you please pass it further to the dev team?

commit 3437bda13812085cdec4b8ac5ab8c93a2e350bf9
Author: Vyacheslav Yurkov <uvv.mail@gmail.com>
Date:   Fri Mar 25 07:23:16 2022 +0100
 
    drivers: regulator: fix ALWAYS_ON handling
    
    BL2 tried to disable regulators even when ALWAYS_ON flag was set
    
    Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com>
 
diff --git a/drivers/regulator/regulator_core.c b/drivers/regulator/regulator_core.c
index 67da7d292..3738ecdfd 100644
--- a/drivers/regulator/regulator_core.c
+++ b/drivers/regulator/regulator_core.c
@@ -321,6 +321,10 @@ int regulator_disable(struct rdev *rdev)
 
        assert(rdev != NULL);
 
+       if (rdev->flags & REGUL_ALWAYS_ON) {
+               return 0;
+       }
+
        ret = __regulator_set_state(rdev, STATE_DISABLE);
 
        udelay(rdev->enable_ramp_delay);
@@ -578,8 +582,6 @@ int regulator_set_flag(struct rdev *rdev, uint16_t flag)
        return 0;
 }
 
-#if defined(IMAGE_BL32)
-
 struct regul_property {
        char *name;
        uint16_t flag;
@@ -648,6 +650,8 @@ static int parse_properties(const void *fdt, struct rdev *rdev, int node)
        return 0;
 }
 
+#if defined(IMAGE_BL32)
+
 static void parse_supply(const void *fdt, struct rdev *rdev, int node)
 {
        const char *name = rdev->desc->supply_name;
@@ -777,12 +781,12 @@ static int parse_dt(struct rdev *rdev, int node)
                return ret;
        }
 
-#if defined(IMAGE_BL32)
        ret = parse_properties(fdt, rdev, node);
        if (ret != 0) {
                return ret;
        }
 
+#if defined(IMAGE_BL32)
        parse_supply(fdt, rdev, node);
 
        parse_low_power_modes(fdt, rdev, node);
diff --git a/include/drivers/regulator.h b/include/drivers/regulator.h
index 392051f96..bbfae23da 100644
--- a/include/drivers/regulator.h
+++ b/include/drivers/regulator.h
@@ -123,11 +123,11 @@ struct rdev {
        uint16_t flags;
 
        uint32_t enable_ramp_delay;
-#if defined(IMAGE_BL32)
        const char *reg_name;
 
        uint8_t use_count;
 
+#if defined(IMAGE_BL32)
        int32_t supply_phandle;
        struct rdev *supply_dev;

The patch is based on v2.4-stm32mp-r2 tag from the downstream tf-a project (https://github.com/STMicroelectronics/arm-trusted-firmware)

Thanks,

Vyacheslav

[Bernard PUEL]

Hi, here is the fix that will be integrated in next ST releases.

[GHers.1]

hello Bernard,

I am sending this message long after this thread was closed.

I hope you can reply to it.

We encountered this issue just recently. Every few power-ons, the system is stuck and we get the messages as described at the beginning of the thread. Resetting the PMIC / STM32MP153C solves this.

At first you suggested to remove the line for vmmc-supply from the TF-A dts.

My guess it that SDMMC initialization procedure in the TF-A powers the SDMMC device off/on (maybe to bring it to some initial state) and by removing this line, the TF-A code will not know which supply to toggle, so it will not perform it.

So, the instability/oscillations in BUCK4 (due to the off/on) will not happen, thus will not affect the eMMC.

Then you wrote "In case of v3v3 supply of the sdmmc, you cannot do a power cycle of the sdmmc at tf-a level (because you will switch off almost all the platform). But this is not an issue because the romcode has already done it before."

I assume this is another justification to remove this line from the TF-A dts.

In my case, our custom board connects v3v3 (BUCK4) to both vmmc and vqmmc supplies.

So, should I remove both lines (vmmc-supply = <&v3v3>; vqmmc-supply = <&v3v3>) from the TF-A dts?

Then, you wrote: "After internal discussions, we will deploy a correction that will have the same effect as the previous solution. So you can go on with it on your side."

And, suggested a fix: a patch, to be applied to "regulator_core.c" (a file in the TF-A package).

Can you describe what this patch does?

What do you suggest we do? Apply the patch or modify the TF-A dts file?

thank you

Gil

[Bernard PUEL]

Hello Gil,

The patch "Add support for regulator-always-on at BL2 level" so that the regulator is not switched off during BL2 step if this property is in the dt. It was integrated in V3.1.1 and the following versions ....

What is the SW version you are using ?