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

[Jonas Karlman]

Hi Peng,

On 11/3/2025 9:51 AM, Peng Fan wrote:
> On Sat, Nov 01, 2025 at 08:34:26PM +0000, Jonas Karlman 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.
>>
>> 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)
> 
> I understand it might be not proper to enable vin-supply in
> function regulator_common_of_to_plat().
> But I have a question, there is -ENOENT check in the original patch. If the
> board does not have vin-supply, there should be no issue. Or I miss something?

As mentioned in the commit message the SPL control FDT does not contain
the regulator references as a vin-supply, this result in -ENODEV to be
returned. I.e. following where the vcc_io is not included in SPL control
FDT.

  vcc_io: regulator@ {
  }
  vcc_sd: regulator@ {
  	bootph-pre-ram;
  	vin-supply = <&vcc_io>;
  }
  sdmmc: mmc@ {
  	bootph-pre-ram;
  	vmmc-supply = <&vcc_sd>;
  }

With the change introduced in this commit the core gpio/fixed regulator
behavior changed and this now result in unbootable boards.

Other reasons why I prefer a full revert instead of a workaround:
- The implementation changed core gpio/fixed regulator function without
  any review-by or acked-by.
- The patch was quickly merged after only being on the list for 1 week,
  the imx pull-request that included this did not even mention regulator
  changes.
- The implementation is not optimal, looking up vin-supply could be okay
  but not failing because it is missing, and enabling it during probe
  when we already have working enable count and regulator autoset to
  handle any needed auto-enable.
- The commit message does not give any insights on why this is needed.

My suggestion is that this is reverted and if needed it should can be
submitted as a separated patch/series with a proper implementation.

Regards,
Jonas

> 
> Thanks,
> Peng
> 
>> -			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
>>