Hi Svyatoslav,
On Thu, 27 Jun 2024 at 11:34, Svyatoslav Ryhel clamor95@gmail.com wrote:
чт, 27 черв. 2024 р. о 12:26 Simon Glass sjg@chromium.org пише:
Hi Svyatoslav,
On Thu, 27 Jun 2024 at 10:09, Svyatoslav clamor95@gmail.com wrote:
27 червня 2024 р. 11:48:46 GMT+03:00, Caleb Connolly caleb.connolly@linaro.org написав(-ла):
On 27/06/2024 10:37, Simon Glass wrote:
Hi Marek,
On Thu, 27 Jun 2024 at 00:57, Marek Vasut marex@denx.de wrote:
In case a regulator DT node contains regulator-always-on or regulator-boot-on property, make sure the regulator gets correctly configured by U-Boot on start up. Unconditionally probe such regulator drivers. This is a preparatory patch for introduction of .regulator_post_probe() which would trigger the regulator configuration.
Parsing of regulator-always-on and regulator-boot-on DT property has been moved to regulator_post_bind() as the information is required early, the rest of the DT parsing has been kept in regulator_pre_probe() to avoid slowing down the boot process.
Signed-off-by: Marek Vasut marex@denx.de
Cc: Ben Wolsieffer benwolsieffer@gmail.com Cc: Caleb Connolly caleb.connolly@linaro.org Cc: Chris Morgan macromorgan@hotmail.com Cc: Dragan Simic dsimic@manjaro.org Cc: Eugen Hristev eugen.hristev@collabora.com Cc: Francesco Dolcini francesco.dolcini@toradex.com Cc: Heinrich Schuchardt xypron.glpk@gmx.de Cc: Jaehoon Chung jh80.chung@samsung.com Cc: Jagan Teki jagan@amarulasolutions.com Cc: Jonas Karlman jonas@kwiboo.se Cc: Kever Yang kever.yang@rock-chips.com Cc: Kostya Porotchkin kostap@marvell.com Cc: Matteo Lisi matteo.lisi@engicam.com Cc: Mattijs Korpershoek mkorpershoek@baylibre.com Cc: Max Krummenacher max.krummenacher@toradex.com Cc: Neil Armstrong neil.armstrong@linaro.org Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@foss.st.com Cc: Philipp Tomsich philipp.tomsich@vrull.eu Cc: Quentin Schulz quentin.schulz@cherry.de Cc: Sam Day me@samcday.com Cc: Simon Glass sjg@chromium.org Cc: Sumit Garg sumit.garg@linaro.org Cc: Svyatoslav Ryhel clamor95@gmail.com Cc: Thierry Reding treding@nvidia.com Cc: Tom Rini trini@konsulko.com Cc: Volodymyr Babchuk Volodymyr_Babchuk@epam.com Cc: u-boot-amlogic@groups.io Cc: u-boot-qcom@groups.io Cc: u-boot@dh-electronics.com Cc: u-boot@lists.denx.de Cc: uboot-stm32@st-md-mailman.stormreply.com
drivers/power/regulator/regulator-uclass.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c index 66fd531da04..ccc4ef33d83 100644 --- a/drivers/power/regulator/regulator-uclass.c +++ b/drivers/power/regulator/regulator-uclass.c @@ -433,6 +433,8 @@ static int regulator_post_bind(struct udevice *dev) const char *property = "regulator-name";
uc_pdata = dev_get_uclass_plat(dev);
uc_pdata->always_on = dev_read_bool(dev, "regulator-always-on");uc_pdata->boot_on = dev_read_bool(dev, "regulator-boot-on"); /* Regulator's mandatory constraint */ uc_pdata->name = dev_read_string(dev, property);@@ -444,13 +446,21 @@ static int regulator_post_bind(struct udevice *dev) return -EINVAL; }
if (regulator_name_is_unique(dev, uc_pdata->name))return 0;
if (!regulator_name_is_unique(dev, uc_pdata->name)) {debug("'%s' of dev: '%s', has nonunique value: '%s\n",property, dev->name, uc_pdata->name);return -EINVAL;}
debug("'%s' of dev: '%s', has nonunique value: '%s\n",property, dev->name, uc_pdata->name);
/** In case the regulator has regulator-always-on or* regulator-boot-on DT property, trigger probe() to* configure its default state during startup.*/if (uc_pdata->always_on && uc_pdata->boot_on)dev_or_flags(dev, DM_FLAG_PROBE_AFTER_BIND);
return -EINVAL;
return 0;}
static int regulator_pre_probe(struct udevice *dev)
@@ -473,8 +483,6 @@ static int regulator_pre_probe(struct udevice *dev) -ENODATA); uc_pdata->max_uA = dev_read_u32_default(dev, "regulator-max-microamp", -ENODATA);
uc_pdata->always_on = dev_read_bool(dev, "regulator-always-on");uc_pdata->boot_on = dev_read_bool(dev, "regulator-boot-on"); uc_pdata->ramp_delay = dev_read_u32_default(dev, "regulator-ramp-delay", 0); uc_pdata->force_off = dev_read_bool(dev, "regulator-force-boot-off");-- 2.43.0
This is reading a lot of DT stuff very early, which may be slow. It is also reading from the DT in the bind() step which we sometimes have to do, but try to avoid.
Could we set up the livetree pre-bind? What about MMU? On armv8 at least this would have a huge impact on performance. I've done some measurements and there is at least 1 order of magnitude difference between parsing FDT with no caches vs parsing livetree with, it's huge.
Also this seems to happen in SPL and again pre-reloc and again in U-Boot post-reloc?
Not so long ago I proposed a similar patchset with the same goal and I have discovered massive issues with SPL and relocation interfering with driver loading.
The issue which I have personally encountered was i2c driver failure due to double probing. This behavior was triggered by always-on/boot-on regulators provided by pmic which in most cases is an i2c device.
At that time everyone just ignored me, so idk if tegra i2c is the only driver which has this response or there are more, but be aware that this patch set may cause cascade failure on many devices.
I'm not sure if I remember this, but I can certainly see some problems with it. Did we have drivers that probed in the bind() function, perhaps?
It is expected not to remember all patchsets sent, not an issue. As for drivers probed after bind there are several, but they are quite essential. Core GPIO and pinmux drivers are probed as early as possible but in most cases they have no dependencies among complex subsystems (like i2c).
OK, well I suppose that you managed to find a solution.
From my side I just want to avoid doing too much such stuff
'automatically' in driver model. As you say, it can lead to difficult race conditions / bugs.
Regards, Simon