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 <[email protected]>
Signed-off-by: Jonas Karlman <[email protected]>
---
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
