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

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

[Jonas Karlman]

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@riseup.net>
Signed-off-by: Jonas Karlman <jonas@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

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@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@manjaro.org>

> Reported-by: Dang Huynh <danct12@riseup.net>
> Signed-off-by: Jonas Karlman <jonas@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
>

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

[Mark Kettenis]

> From: Jonas Karlman <jonas@kwiboo.se>
> 
> 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.

I ran into this issue a few days ago.  Unfortunately that means U-Boot
2025.10 is broken on several Rockchip boards :(.

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

I fixed the issue with the diff below instead.  But I don't claim to
fully understand how this all is supposed to work...


diff --git a/drivers/power/regulator/regulator_common.c b/drivers/power/regulator/regulator_common.c
index c80f10c3aa3..f43879a4353 100644
--- a/drivers/power/regulator/regulator_common.c
+++ b/drivers/power/regulator/regulator_common.c
@@ -48,7 +48,7 @@ int regulator_common_of_to_plat(struct udevice *dev,
 	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)
+		if (ret != -ENOENT && ret != -ENODEV)
 			return ret;
 	}
 

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

[Dragan Simic]

Hello Mark,

On Saturday, November 01, 2025 22:17 CET, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> > From: Jonas Karlman <jonas@kwiboo.se>
> > 
> > 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.
> 
> I ran into this issue a few days ago.  Unfortunately that means U-Boot
> 2025.10 is broken on several Rockchip boards :(.

Sadly, that's true. :(

> > 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.
> 
> I fixed the issue with the diff below instead.  But I don't claim to
> fully understand how this all is supposed to work...

Unfortunately, that's just a quick workaround, not a proper fix.
Basically, the regulator_common_of_to_plat() function is a wrong
place for such enabling of additional regulators.

> diff --git a/drivers/power/regulator/regulator_common.c b/drivers/power/regulator/regulator_common.c
> index c80f10c3aa3..f43879a4353 100644
> --- a/drivers/power/regulator/regulator_common.c
> +++ b/drivers/power/regulator/regulator_common.c
> @@ -48,7 +48,7 @@ int regulator_common_of_to_plat(struct udevice *dev,
>  	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)
> +		if (ret != -ENOENT && ret != -ENODEV)
>  			return ret;
>  	}

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

[Jonas Karlman]

Hi Mark and Dragan,

On 11/1/2025 10:23 PM, Dragan Simic wrote:
> Hello Mark,
> 
> On Saturday, November 01, 2025 22:17 CET, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>>> From: Jonas Karlman <jonas@kwiboo.se>
>>>
>>> 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.
>>
>> I ran into this issue a few days ago.  Unfortunately that means U-Boot
>> 2025.10 is broken on several Rockchip boards :(.
> 
> Sadly, that's true. :(

I do not think the commit was part of 2025.10, it was merged to next and
should currently only be part of master and 2026.01-rc1.

If you experience a similar boot issue on 2025.10 then that will most
likely be a different issue.

Regards,
Jonas

> 
>>> 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.
>>
>> I fixed the issue with the diff below instead.  But I don't claim to
>> fully understand how this all is supposed to work...
> 
> Unfortunately, that's just a quick workaround, not a proper fix.
> Basically, the regulator_common_of_to_plat() function is a wrong
> place for such enabling of additional regulators.
> 
>> diff --git a/drivers/power/regulator/regulator_common.c b/drivers/power/regulator/regulator_common.c
>> index c80f10c3aa3..f43879a4353 100644
>> --- a/drivers/power/regulator/regulator_common.c
>> +++ b/drivers/power/regulator/regulator_common.c
>> @@ -48,7 +48,7 @@ int regulator_common_of_to_plat(struct udevice *dev,
>>  	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)
>> +		if (ret != -ENOENT && ret != -ENODEV)
>>  			return ret;
>>  	}
> 

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

[Dragan Simic]

Hello Jonas,

On Sunday, November 02, 2025 03:00 CET, Jonas Karlman <jonas@kwiboo.se> wrote:
> On 11/1/2025 10:23 PM, Dragan Simic wrote:
> > On Saturday, November 01, 2025 22:17 CET, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >>> From: Jonas Karlman <jonas@kwiboo.se>
> >>>
> >>> 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.
> >>
> >> I ran into this issue a few days ago.  Unfortunately that means U-Boot
> >> 2025.10 is broken on several Rockchip boards :(.
> > 
> > Sadly, that's true. :(
> 
> I do not think the commit was part of 2025.10, it was merged to next and
> should currently only be part of master and 2026.01-rc1.

You're right, the troublesome commit f98d812e5353 ("power:
regulator: Add vin-supply for GPIO and Fixed regulators") isn't
part of the U-Boot 2025.10 release, which can be verified by
checking drivers/power/regulator/regulator_common.c in the
ultimate reference, which is the 2025.10 release tarball. [1]

[1] https://ftp.denx.de/pub/u-boot/u-boot-2025.10.tar.bz2

Re: [PATCH] rockchip: remove regulator enable from device probe

[Brian Sune]

Hi Jonas,

This patch resolves the previous boot issue mentioned earlier on the mailing list:

[DEBUG] rk3399-nanopi-neo4: SPL SD init may report -5 due to SD power-on timing

Quoting the earlier observation:
"
While testing mainline U-Boot (v2026.01-rc1-00070-ge34d01d23e45) on the
FriendlyARM NanoPi NEO4 (RK3399), I observed that SPL sometimes fails to
initialize the SD card with the following message:

    spl: mmc init failed with error: -5
    SPL: failed to boot from all boot devices
"

So the NanoPi NEO4 boot stall appears to have been caused by the previous regulator patch.
However, it is puzzling why the issue only affected SD boot and not MMC.

This suggests that the delay method proposed earlier regarding power-on timing
is not the actual root cause of the RK3399 NanoPi NEO4 SD boot stall.

Tested with this patch applied, and SD boot now works normally.

Regards,  
Brian

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

[Peng Fan]

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@riseup.net>
>Signed-off-by: Jonas Karlman <jonas@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?

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
>

Re: [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@riseup.net>
>> Signed-off-by: Jonas Karlman <jonas@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
>>

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

[Quentin Schulz]

On 11/3/25 11:36 AM, Jonas Karlman wrote:
> 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@riseup.net>
>>> Signed-off-by: Jonas Karlman <jonas@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>;
>    }
> 

This needs to be fixed for SPL FDT for all impacted boards then 
(Whenever the patch adding the enabling of vin-supply is resent). We 
need the same bootph props for the node pointed at by vin-supply.

> 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

If the property is missing, then we shouldn't fail of course, since the 
property isn't required. If the property is there but it points to an 
empty node, it should fail? Since the vin-supply could be one that needs 
to do something (e.g. a gpio-regulator whose default state is off). If 
it's there but cannot be probed (e.g. gpio-regulator but the gpio 
controller of the enable GPIO isn't available in the stage), we should 
fail as well.

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

Agreed.

1) vin-supply isn't a regulator property. It is a fixed-regulator and 
gpio-regulator property so it should be handled in those drivers and not 
in the function common to all regulator drivers,
2) Parsing the property and mapping in *_of_to_plat is fine, but 
enabling should be done when enabling the regulator, not when parsing 
the FDT,

Cheers,
Quentin

Re: [PATCH] rockchip: remove regulator enable from device probe

[Sune Brian]

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

I am not sure this is the case.
In order for the boot to start the SD card must be powered.
And the 0x40 and 0x4000 is then read accordingly.
So why TPL phase is not having issue aka it is not dead.
But only after TPL aka SPL phase to break the power
between eMMC or SDMMC?

Regards,
Brian

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

[Tom Rini]

On Sat, 01 Nov 2025 20:34:26 +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.
> 
> [...]

Applied to u-boot/master, thanks!

[1/1] Revert "power: regulator: Add vin-supply for GPIO and Fixed regulators"
      commit: 32ead3c1bada6501a3d34ac7c0a7088cd6bcfbd2
-- 
Tom

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

[Ye Li]

Hi Jonas,

On 11/3/2025 6:36 PM, Jonas Karlman wrote:
> 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@riseup.net>
>>> Signed-off-by: Jonas Karlman <jonas@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

Shouldn't the reason be the broken SPL FDT? When one node is used in SPL 
FDT, the nodes referred by its properties should also add 
bootph-pre-ram. In your case, it is obviously missed for vcc_io. You 
can't assume driver never use this property.

If a new change for vin-supply is submitted in future, how can it avoid 
ubootable with these broken FDT?  It is appropriate for current 
implementation to get the vin_supply in probe and only check ENOENT. The 
ENODEV means a explicit problem that can't be ignored. The contentious 
point is if the vin-supply enablement should move to set enable. I agree 
with this proposal, but it is not related with the boot failure.

Best regard,
Ye Li>
>>
>> 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
>>>
> 

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

[Jonas Karlman]

Hi Ye Li,

On 11/7/2025 8:31 AM, Ye Li wrote:
> Hi Jonas,
> 
> On 11/3/2025 6:36 PM, Jonas Karlman wrote:
>> 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@riseup.net>
>>>> Signed-off-by: Jonas Karlman <jonas@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
> 
> Shouldn't the reason be the broken SPL FDT? When one node is used in SPL 
> FDT, the nodes referred by its properties should also add 
> bootph-pre-ram. In your case, it is obviously missed for vcc_io. You 
> can't assume driver never use this property.

Please note that my example above was not from a real board only for
illustration purposes.

The vin-supply used for Rockchip devices on affected boards are
typically only fixed-regulator that are always-on and cannot be
controlled, e.g. dc-in or some always-on step-down converter.
These are included in the DT for more accurate description of HW.

For those boards, including vin-supply regulators in the SPL control FDT
serve no purpose beside wasting space and boot time for udevice binding
and probing, because of this they been left out on purpose since U-Boot
did not require or cared about a vin-supply prior to this patch.

If and how a vin-supply should be flagged with bootph-pre-ram or if the
vin-supply prop is scrubbed from SPL control FDT is not related to the
revert of this commit, it was just the main issue that help discover a
change in behavior that deserve discussion on ML before being merged.

> 
> If a new change for vin-supply is submitted in future, how can it avoid 
> ubootable with these broken FDT?  It is appropriate for current 
> implementation to get the vin_supply in probe and only check ENOENT. The 
> ENODEV means a explicit problem that can't be ignored.

Personally I do not think there is a need to fail at probe time because
a regulator-uclass udevice for a referenced vin-supply cannot be
resolved. A debug message could be printed if that is the case, however
require a working driver for the vin-supply is a change in behavior that
should really be documented in the commit message of a patch.

> The contentious 
> point is if the vin-supply enablement should move to set enable. I agree 
> with this proposal, but it is not related with the boot failure.

Agreed, the main issue with the original patch what that it changed
probe behavior and also force-enabled any regulator that happened to 
be referenced as a vin-supply regardless if such regulator is even
probed or enabled.

This revert has now landed in master, please send a new series if you
have need for proper vin-supply handling by fixed/gpio-regulators.

Regards,
Jonas

> 
> Best regard,
> Ye Li>
>>>
>>> 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
>>>>
>>
>