Re: [PATCH] Revert "power: regulator: Add vin-supply for GPIO and Fixed regulators"

[Dragan Simic]

Hello Jonas and Dang,

On Saturday, November 01, 2025 21:34 CET, Jonas Karlman <jonas at kwiboo.se> wrote:
> Rockchip boards may depend on a working MMC regulator in SPL to
> successfully load FIT payload from MMC. Typically, these boards only
> include the vmmc-supply regulator and not its vin-supply in SPL control
> FDT.
> 
> The commit f98d812e5353 ("power: regulator: Add vin-supply for GPIO and
> Fixed regulators") breaks loading FIT from MMC in SPL on some of these
> boards due to now requiring the vin-supply to be included in the SPL
> control FDT.
> 
> The commit also strangely enables any found vin-supply in
> regulator_common_of_to_plat() and not when a regulator is enabled or as
> part of regulator_autoset().
> 
> Revert the commit to fix FIT loading in SPL on broken boards.
> 
> If a board needs to have its vin-supply enabled, two options come to
> mind:
> 
> - Add regulator-always-on prop to the regulator in the -u-boot.dtsi for
>   any board.
> 
> - Implement full support for reference counting of regulators and then
>   update the regulator-uclass to enable any found vin-supply when a
>   regulator is enabled.
> 
> This reverts commit f98d812e5353408ef77a46bad1f1cdc793ff8a03.

Thank you, Jonas, for preparing this patch so quickly!  Of course,
huge thanks to Dang for spotting the issue and bisecting it.

I totally agree with the description of how the reverted commit
f98d812e5353 ("power: regulator: Add vin-supply for GPIO and Fixed
regulators", 2025-09-11) strangely performed the enabling of "vin-
supply" regulators in a suboptimal way.  As already described by
Jonas, a proper implementation of such regulator enabling would
require a much more complex approach.

Thus, please feel free to include:

Reviewed-by: Dragan Simic <dsimic at manjaro.org>

> Reported-by: Dang Huynh <danct12 at riseup.net>
> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
> ---
>  drivers/power/regulator/regulator_common.c | 10 ----------
>  drivers/power/regulator/regulator_common.h |  1 -
>  2 files changed, 11 deletions(-)
> 
> diff --git a/drivers/power/regulator/regulator_common.c b/drivers/power/regulator/regulator_common.c
> index 3ed713ce5019..685d8735fa5c 100644
> --- a/drivers/power/regulator/regulator_common.c
> +++ b/drivers/power/regulator/regulator_common.c
> @@ -44,16 +44,6 @@ int regulator_common_of_to_plat(struct udevice *dev,
>  			dev_read_u32_default(dev, "u-boot,off-on-delay-us", 0);
>  	}
>  
> -	ret = device_get_supply_regulator(dev, "vin-supply", &plat->vin_supply);
> -	if (ret) {
> -		debug("Regulator vin regulator not defined: %d\n", ret);
> -		if (ret != -ENOENT)
> -			return ret;
> -	}
> -
> -	if (plat->vin_supply)
> -		regulator_set_enable_if_allowed(plat->vin_supply, true);
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/power/regulator/regulator_common.h b/drivers/power/regulator/regulator_common.h
> index 799c968d0b66..d4962899d830 100644
> --- a/drivers/power/regulator/regulator_common.h
> +++ b/drivers/power/regulator/regulator_common.h
> @@ -14,7 +14,6 @@ struct regulator_common_plat {
>  	unsigned int startup_delay_us;
>  	unsigned int off_on_delay_us;
>  	unsigned int enable_count;
> -	struct udevice *vin_supply;
>  };
>  
>  int regulator_common_of_to_plat(struct udevice *dev,
> -- 
> 2.51.0
>