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

[Marco Felsch <m.felsch@pengutronix.de>]

On 25-04-14, Ahmad Fatoum wrote:
> 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.

I know, but still it is good to check and to wait for power-good state
before you cont. booting. Like you do for barebox: you wait till the
power is good or die within the PBL.

Regards,
  Marco


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