Re: [PATCH 4/6] ARM: i.MX8MP: skov: halt startup until power is good

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hello Marco,

On 14.04.25 11:21, Marco Felsch wrote:
> On 25-04-14, Ahmad Fatoum wrote:
>> Hello Marco,
>>
>> On 14.04.25 10:40, Marco Felsch wrote:
>>> Hi,
>>>
>>> On 25-04-14, Ahmad Fatoum wrote:
>>>> The 24V regulator supplying the system can indicate via a GPIO imminent
>>>> voltage loss. The board's capacitors hold enough charge to power the system
>>>> a while longer in such a state, but eventually, unless external power is
>>>> restored, the brownout detection of the PMIC will kick in.
>>>>
>>>> For the span of time between voltage drop detection and PMIC brownout,
>>>> let's detect this situation and delay startup. This way, Linux can
>>>> detect the ongoing voltage loss, power down the eMMC, reboot into
>>>> barebox and barebox will delay boot as long as the problem persists.
>>>>
>>>> Co-developed-by: Kim Christensen <kch@skov.dk>
>>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>>> ---
>>>>  arch/arm/boards/skov-imx8mp/lowlevel.c | 60 ++++++++++++++++++++++++++
>>>>  1 file changed, 60 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boards/skov-imx8mp/lowlevel.c b/arch/arm/boards/skov-imx8mp/lowlevel.c
>>>> index 637fc50b3f30..d7bd771f259d 100644
>>>> --- a/arch/arm/boards/skov-imx8mp/lowlevel.c
>>>> +++ b/arch/arm/boards/skov-imx8mp/lowlevel.c
>>>> @@ -20,6 +20,9 @@
>>>>  
>>>>  extern char __dtb_z_imx8mp_skov_start[];
>>>>  
>>>> +#define PGOOD_PAD_CTRL  MUX_PAD_CTRL(MX8MP_PAD_CTL_PUE | \
>>>> +				     MX8MP_PAD_CTL_PE)
>>>> +
>>>>  #define UART_PAD_CTRL   MUX_PAD_CTRL(MX8MP_PAD_CTL_DSE6 | \
>>>>  				     MX8MP_PAD_CTL_FSEL | \
>>>>  				     MX8MP_PAD_CTL_PUE | \
>>>> @@ -30,6 +33,12 @@ extern char __dtb_z_imx8mp_skov_start[];
>>>>  				     MX8MP_PAD_CTL_PUE | \
>>>>  				     MX8MP_PAD_CTL_PE)
>>>>  
>>>> +static inline void led_d1_toggle(bool *on)
>>>> +{
>>>> +	imx8m_gpio_direction_output(IOMEM(MX8MP_GPIO1_BASE_ADDR), 5, *on);
>>>> +	*on = !*on;
>>>> +}
>>>> +
>>>>  static void setup_uart(void)
>>>>  {
>>>>  	void __iomem *uart = IOMEM(MX8M_UART2_BASE_ADDR);
>>>> @@ -67,6 +76,55 @@ static struct pmic_config pca9450_cfg[] = {
>>>>  	{ PCA9450_BUCK2OUT_DVS0, 0x14 },
>>>>  };
>>>>  
>>>> +static inline bool power_good(void)
>>>> +{
>>>> +	/* IMX_SHDN_MF in schematics */
>>>> +	return imx8m_gpio_val(IOMEM(MX8MP_GPIO4_BASE_ADDR), 23);
>>>> +}
>>>> +
>>>> +static void wait_for_power_good(void)
>>>> +{
>>>> +	void __iomem *gpio4 = IOMEM(MX8MP_GPIO4_BASE_ADDR);
>>>> +	int timeout_ms = 0;
>>>> +	bool led_active = true;
>>>> +
>>>> +	imx8mp_setup_pad(MX8MP_PAD_SAI2_RXD0__GPIO4_IO23 | PGOOD_PAD_CTRL);
>>>> +	imx8m_gpio_direction_input(gpio4, 23);
>>>> +
>>>> +	led_d1_toggle(&led_active);
>>>> +
>>>> +	if (power_good())
>>>> +		return;
>>>> +
>>>> +	pr_warn("\nDelaying boot until power stabilizes\n");
>>>> +
>>>> +	/* If we reach this, because Linux did a hw_protection_reboot, we don't
>>>> +	 * want to continue booting right away.
>>>> +	 *
>>>> +	 * Thus let's either wait for the condition to subscede or for voltage
>>>> +	 * to reach a low enough level for the PMIC to detect VSYS_UVLO going
>>>> +	 * lower than allowed
>>>> +	 */
>>>> +
>>>> +	while (1) {
>>>> +		if (power_good()) {
>>>> +			/* wait 10ms longer and check if it still good */
>>>> +			udelay(10000);
>>>> +			if (power_good()) {
>>>> +				pr_info("IMX_SHDN_MF stuck low for ~%ums.\n", timeout_ms);
>>>> +				break;
>>>> +			}
>>>> +		}
>>>> +		/* fast blink LED D1 */
>>>> +		if (timeout_ms % 100 == 0) {
>>>> +			pr_debug(".");
>>>> +			led_d1_toggle(&led_active);
>>>> +		}
>>>> +		udelay(1000);
>>>> +		timeout_ms++;
>>>> +	}
>>>> +}
>>>> +
>>>>  static void power_init_board(void)
>>>>  {
>>>>  	struct pbl_i2c *i2c;
>>>> @@ -77,6 +135,8 @@ static void power_init_board(void)
>>>>  	imx8mp_setup_pad(MX8MP_PAD_SAI3_TXD__GPIO5_IO01);
>>>>  	imx8m_gpio_direction_output(IOMEM(MX8MP_GPIO5_BASE_ADDR), 1, 0);
>>>>  
>>>> +	wait_for_power_good();
>>>
>>> out of curiosity, wouldn't it be more useful to do the check right
>>> before the system wants to boot the kernel and abort in that case?
>>
>> How so?
>>
>> When we have brownout, we usually never make it out of the PBL,
> 
> Okay, since your backup charge is really low already.
> 
>> so I prefer the SoC gets powered off during this loop instead
>> of during the RAM setup.
> 
> I understand that to wait for the power to become good again, but what
> if the power is getting good and changes to bad again once you entered
> barebox? Do you get notified about such an event?
> 
> IMHO the later point a bit more important since this ensures that the
> system was in power-good state before we the system starts.

Checking just before booting is insufficient anyway, because power good
may fluctuate before the Linux driver starts taking care of the power
good GPIO. Given that we need to handle this anyway, I don't think
it makes much difference whether when we check the power good GPIO
during barebox runtime.

Cheers,
Ahmad

> 
> Regards,
>   Marco
> 


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